# routerとserviceの責務
## 対象レビュー
単一ページのmdファイルエクスポート機能のサーバー側（ページデータへのアクセス部分）の実装 ([PR](https://github.com/weseek/growi/pull/2512))

## ここで言いたかったこと
> 答え自体は普通に考えればわかることだけど、一度冷静になってその処理は本当にそこに書くべきなのかを考える癖をつけるのが大事だと思いました。

## 議題
どこまでrouter (`routes` 配下のファイル)がやるべきでどっから service (`/services` 配下のファイル) がやるべきか（あくまでGROWI開発において）

### 実装物の処理の流れ
1. サーバーがフロントからの page の body 取得リクエストを受け取る
2. ログインチェック等を経て、　`/router` ディレクトリ内の page の body 取得用処理に飛ぶ
3. 要求されているページの存在チェック
4. リクエスト送信ユーザーの要求されているページへのアクセス権限チェック
5. リクエストパラメータ内の revision id から revision を取得（revision id が指定されていなかったら同じくリクエストパラメータ内の page id から 最新の revision を取得）
6. revision.body を取得し、適切なデータ形式に変換（当時はpdfエクスポート機能も予定していたため変換処理や送受信データの contents-type の調整などが必要だった）
7. 変換を終えたデータをフロント側に送信

### レビュー前
全て `/routes` 配下のファイルに書いていた

### レビュー内容
[レビューコメント](https://github.com/weseek/growi/pull/2512#discussion_r466972622)

- page data, revision data へのアクセス権があるかどうか、そのエラー処理までは route でやる
- revision を引数に、stream を返すところは service でやる
- res に対しての処理は route でやる

このコメントに答えが出揃ってしまってはいるが、大事なのはrouter の役割を意識すること。

router がすべきなのは
- リクエストを受け取ったら、そのリクエストの正当性をチェック
- パラメータを処理しやすいように変換しておく
- そのリクエストを達成できるような処理に案内する
- リクエストにふさわしいレスポンスを返す

(あくまで個人的な考えです。また開発しているアプリによって変わってくると思います)

これを踏まえて今回の実装内容を見てみると

- ページの存在チェックやアクセス権限チェックは一個目に該当するのでrouterでやるべき
    ```javascript:routes/page.js
    const page = await Page.findByIdAndViewer(pageId, req.user);
    const isPageExist = await Page.count({ _id: pageId }) > 0;
    if (page == null) {
        const isPageExist = await Page.count({ _id: pageId }) > 0;
    if (isPageExist) {
        // This page exists but req.user has not read permission
        return res.apiv3Err(new ErrorV3(`Haven't the right to see the page ${pageId}.`), 403);
    }
    ```
- リクエストパラメータの revision id から revision を取得しておくのもrouterでやる（ここは場合によってはserviceに任せても良さそう）
    ```javascript:routes/page.js
    const revision = await Revision.findById(revisionIdForFind);
    ```
- 書くまでもないが、service の処理を呼び出すのも router がやる
    ```javascript:routes/page.js
    const stream = exportService.getReadStreamFromRevision(revision);
    ```
- フロントに返すデータの準備はservice がやるべき(ここでDBへのアクセスがない時は servise、ある時はrepositoryという分け方をする考え方もあるんですね。知りませんでした）
    ```javascript:services/export.js
    getReadStreamFromRevision(revision) {
        const markdown = revision.body;

        const Readable = require('stream').Readable;
        const readable = new Readable();
        readable._read = () => {};
        readable.push(markdown);
        readable.push(null);

        return readable;
  }
  ```
- データをフロントに返す部分はrouterがやるべき
    ```javascript:routes/page.js
    res.set({
        'Content-Disposition': `attachment;filename*=UTF-8''${fileName}.${format}`,
    });
    return stream.pipe(res);
    ```

# まとめ
このディレクトリ配下のファイルはどこまでの作業をすべきかを開発者同士の共通認識に沿って考慮しておくことが大事。今回は別に問題が生じたわけではないが、このように責務を明確に切り分けていない状態（ある権限チェックはrouterに書かれ、ある権限チェックはserviceに書かれているような状態）だとどこにどうゆう目的の処理が書かれているのか把握しづらく変更が大変なソースコードになってしまいそう。

---

## Page Navigation

- Canonical URL: https://tips.weseek.co.jp/今日のレビューから/routerとserviceの責務
- Permalink: https://tips.weseek.co.jp/5f323bfc55d58b0048c43b00
- Parent: [今日のレビューから](/5e38deb8e848c600487d69a0.md)
- Children: 0 total
- Total descendants: 0
- Siblings: 4 total
  - [Requestに適正な値が入ってなかったら400エラーを返す](/5e38f144e848c600487d69a5.md)
  - [それって車輪の再発明じゃない？](/5e425d62decd0a0049953aef.md)
  - [イテレータを利用してスマートな実装しよう](/5e3f9b11f5921400485be6ad.md)
  - [関数名定義(getHoge)](/5f4883eb0802bb00498f41d9.md)
- Last updated: 2020-08-27T07:16:03.847Z by deleted_at_1672138409189
- Full page listing (all children regardless of count): https://tips.weseek.co.jp/_api/v3/page-listing/children?id=5f323bfc55d58b0048c43b00