Node.js with TypeScript なプロジェクトに入ってやった最近の細々としたこと

EVO Japan 2024 のメインタイトルに MELTY BLOOD: TYPE LUMINA がなくて、う、うそだろ…って気持ちと、まあしゃーないよな…(今年のEVOで新情報が UNDER NIGHT IN-BIRTH II Sys:Celes のみだった)という気持ちでせめぎ合ってます。サーバサイドエンジニアの @mix3 です。

最近まで tonamel チームにいたのですが今年の梅雨明けぐらいに異動してタイトルな感じのプロジェクトにジョインしました。EVOJapanとMBTL EVOJapan後夜祭 で十数年ぶりに当時IRCで熱帯してた旧作メルブラのお友達と会って「@mix3 さんいま tonamel いますよね?」と tonamel にいること察せられててビビったり、後夜祭でそのお友達の武装シエルにぼこぼこにされたり、急遽後夜祭に来てくれたフランスパンのディレクター・バトルプランナーの 鴨音 さんにそのお友達経由で引き合わせて貰って(大変感謝)「tonamel でバックエンドやってまーす」と、ご挨拶したりしたばかりなのに、すまぬ…すまぬ…というお気持ちですが、私は元気です。

tonamel をよろしくお願いします🙏 tonamel.com

この記事はTech KAYAC Advent Calendar 2023の13日目の記事です。

本題

ということで現在 typescript を触ってるわけですが、tonamel までは golang をよく使っていたので、typescript (というか javascript) で number や string、Array などのプリミティブ(?)な型に便利メソッドが生えまくっていて「ほー」と感動したり、エディタ上では怒られててもトランスパイル出来れば問題なく動いちゃうなどの「え、そうなの?こわ」ってなるときがあるものの基本は型の恩恵を受けられて感謝したり、時に「型パズルは人類には早すぎないか?🤔 まあ自分が書くことはないからええか」 となりつつ「typescript もまあ悪くはないか」と思いながら日々コードを書き散らしております。

そんな感じで typescript にそれほど不満は無いものの、それ以外のところで思うところはちらほら出てくるもので

  • ORM に typeorm を使っているが、テストで typeorm をモックするのアホほど面倒臭い(アホほど面倒臭い)
  • Connection がマスタデータ用とユーザデータ用で別にあるが EntityManager は型の上では同じなので EntityManager を引き回してるところで取り違え得る状態になっている
    • Connection = typeorm v0.3 の DataSource
    • 歴史的経緯により typeorm v0.2 を使っていますが、もうすぐ v0.3 移行予定
  • API定義にOpenAPIを使っているがプロジェクトの歴史的経緯により OpenAPI で使っているプラグインがバイナリしか残っていないので一部のコード生成に手が付けられない

などなど、いくつか思うことはあったりします。なので、余裕があれば手をつけたいなぁというお気持ちでいて、最近ちょっと手を入れたので紹介します。

モック面倒くさすぎる問題

今のプロジェクトでは、テストを書く時にDB操作を担う typeorm をモックをしています。

具体的には以下のような感じで EntityManager をモックし、メソッドごとに想定される input と期待する output を用意、テストケースごとに書き並べる感じで書いています。(テーブルテストな書き方が多いだけでこういう書き方に縛っているとかではありません)

    type test = {
      name: string;
      data: {
        hoge: {
          find: HogeEntity[];
        };
      };
      expected: {
        query: {
          hoge: {
            find: FindManyOptions<HogeEntity>;
          };
        };
        result: ...;
      };
    };
    it.each([
      {
        name: 'pattern1',
        data: {
          hoge: {
            find: [
              MockFactory.Create(HogeEntity, {
                id: 1,
              }),
              MockFactory.Create(HogeEntity, {
                id: 2,
              }),
              MockFactory.Create(HogeEntity, {
                id: 3,
              }),
            ],
          },
        },
        expected: {
          query: {
            hoge: {
              find: {
                id: In([1, 2, 3]),
              },
            },
          },
          result: ...,
        },
      },
      {
        name: 'pattern2',
        ...
      },
      {
        name: 'pattern3',
        ...
      },
    ] as Partial<test>[])('$name', async (t) => {
      const entityManager = {} as EntityManager;
      entityManager.getRepository = jest.fn().mockImplementation((entity) => {
        switch (entity) {
          case HogeEntity: {
            const repo = new Repository<HogeEntity>();
            repo.find = jest.fn().mockImplementation((condition) => {
              expect(condition).toStrictEqual(t.expected.query.hoge.find);
            });
            return repo;
          }
          default:
            throw new Error(`unexpected entity type: ${entity}`);
        }
      });

      const result = await manager.hoge(entityManager);
      expect(result).toStrictEqual(t.expected.result);
    });

関わる Entity が増えると case 文が増え、テストケースが増えるとモックの input / output が増え、テストを増やすたびにこれらを都度書いていて、と正直実装してる時間よりテストでモック書いてる時間の方が長げぇぞ?ということが多く、流石にこれはなんとかしたいなと個人的になりました。

弊社的にはテストでもDBを使ってモックを避けることが多いので最終的にはそうしたいし、実はPRも既にWIPで作られているのでそのうち導入されそうではあるのですが、ただDBを使うテストはテストを充実させるとモックよりもテストの時間が伸びやすいという問題もあるのでモックを楽にする方向の対応があっても良いなと思いました。

最初は以下のような感じで配列の入れ物を持っておいて typeorm をエミュレートするモックを実装して使いまわせないかと考えました

export const MockEntityManager = (): EntityManager => {
  const hogeStore: HogeEntity[] = [];
  const entityManager = {} as EntityManager;
  entityManager.getRepository = jest.fn().mockImplementation((entity) => {
    switch (entity) {
      case HogeEntity: {
        const repo = new Repository<HogeEntity>();
        repo.find = jest.fn().mockImplementation((condition) => {
          return hogeStore.filter((v) => {
            // condition を使って良い感じに実装して…
          });
        });
        repo.insert = jest.fn().mockImplementation((entity) => {
          hogeStore.push(entity);
        });
        return repo;
      }
      default:
        throw new Error(`unexpected entity type: ${entity}`);
    }
  });
};
      ...
    ] as Partial<test>[])('$name', async (t) => {
      // EntityManager に良い感じにモックした共通実装を使うことで各テストでのモック実装を無くせないか?
      const entityManager = MockEntityManager();
      const result = await manager.hoge(entityManager);
      expect(result).toStrictEqual(t.expected.result);
    });
  });
});

ただ、typeorm のあらゆるメソッドを良い感じにモックするのは難しく selectQueryBuilder を使われたり find のオプションで Raw を使われた瞬間に破綻するので「いや、無理そうだな?」となりました。

次に typeorm を隠蔽する Repository 層を設けて typeorm を直接モックするのではなく Repository 層に対してモックするのはどうか?ということで以下のような修正を試みてみました。

// typeorm を使って必要なDB操作を定義
export class HogeRepository {
  findByIds(ids:number[]): Entity[] {
    return this.repo.find({ where: { id: In(ids) } })
  }
}
// 配列を入れ物にモック実装
// typeorm を直接モックするのではないので、基本的にモック出来ないことは無いハズ
export class MockHogeRepository implements HogeRepository {
  store: HogeEntity[] = [];

  findById(ids:number[]): {
    const set = new Set(ids)
    return this.store.filter((v) => set.has(v.id))
  }
}
// entityManager を受け取って Repository を返す君を用意
// NestJS を使ってるので DI でこれを inject する
// これ経由で各Repositoryを使う
export class RepositoryService {
  getHogeRepository(entityManager:EntityManager): HogeRepository {
    return new HogeRepository(entityManager)
  }
}
// RepositoryService をモックする君
// TestingModule で useValue にこれを new して渡して実装をモックに差し替える
export class MockRepositoryService implements RepositoryService {
  readonly mockHogeRepository = new MockHogeRepository()

  getHogeRepository(entityManager:EntityManager): HogeRepository {
    return this.mockHogeRepository
  }
}
    type test = {
      name: string;
      init: {
        hoge: HogeEntity[];
      };
      expected: {
        store: HogeEntity[];
        result: ...;
      };
    };
    it.each([
      {
        name: 'pattern1',
        init: {
          hoge: [
            MockFactory.Create(HogeEntity, {
              id: 1,
            })
            MockFactory.Create(HogeEntity, {
              id: 2,
            })
            MockFactory.Create(HogeEntity, {
              id: 3,
            })
          ],
        },
        expected: {
          store: [
            ...,
          ],
          result: ....,
        },
      },
      {
        name: 'pattern2',
        ...
      },
      {
        name: 'pattern3',
        ...
      },
    ] as Partial<test>[])('$name', async (t) => {
      // RepositoryServiceの方でモックの実装がDIされてるので
      // 初期データを入れてテストしたいメソッドを叩くだけ
      // 必要に応じてモックで保持してるデータもテストして期待通りに更新されてるか、なども見る
      const mainEntityManager = ...;
      await repositoryService.mockHogeRepository.insert(t.init?.hoge ?? []);
      const result = await manager.hoge(entityManager);
      expect(result).toStrictEqual(t.expected.result);
      expect(repositoryService.mockHogeRepository.store).toStructEqual(
        expect.arrayContaining(t.expected.store.map((v) => expect.objectContaining(v)),
      );
    });

typeorm を直接使わないということで Repository 層に対して typeorm を使って必要なメソッドを都度実装する必要があるのが面倒なところではありますが、テストで都度モックを考える必要が無くなりモックに対応する input / output について考える必要も無くなり、DBを使ったテストを書いてるのと変わらない書き心地になったのでテストを書くのは大分楽になったかなと思います。

他にも以下のような工夫をしつつ現在 RepositoryService を導入しています

  • insert, save, remove などの write 系の実装は共通化して extends するようにしていて、各Repositoryで実装が必要になるのは read 系のみになるようにしている
  • RepositoryService は getXXXRepository を並べるだけなので Entity 名や Entity のファイル名などから生成するようにしている
  • selectQueryBuilder を使って join してるような場合は他の mockRepository が持つ配列を参照することで実装可能なので、他の MockRepository を持たせられるようにしている
export class HogeRepository extends BaseRepository<HogeEntity> {
  // join 使った実装(内容は適当)
  async findWithDeletedAt(): Promise<{ entity: HogeEntity; deletedAt: Date }[]> {
    const { entities, raw } = await this.repo
      .createQueryBuilder('h')
      .withDeleted()
      .addSelect('f.deleted_at', 'deletedAt')
      .innerJoin(FugaEntity, 'f', 'f.hoge_id = h.hoge_id')
      .andWhere('f.deleted_at IS NOT NULL')
      .getRawAndEntities<{ deletedAt: Date }>();

    return entities.map((v, i) => {
      return {
        entity: v,
        deletedAt: raw[i].deletedAt,
      };
    });
  }
}
export class MockHogeRepository
  extends MockBaseRepository<HogeEntity>
  implements HogeRepository
{
  mockFugaRepository: MockFugaRepository;

  // 他の MockRepository を持たせられるメソッド用意
  with(mockFugaRepository: MockFugaRepository): MockFugaRepository {
    this.mockFugaRepository = mockFugaRepository;
    return this;
  }

  // 他の mockRespository.store を使って join を模した実装する
  async findWithDeletedAt(): Promise<{ entity: HogeEntity; deletedAt: Date }[]> {
    const deletedAtMap = new Map(
      this.mockFugaRepository.store.filter((v) => !!v.deletedAt).map((v) => [v.hogeId, v.deletedAt]),
    );
    return this.store.filter().map((v) => {
      entity: v,
      deletedAt: deletedAtMap.get(v.hogeId),
    })
  }
export class MockRepositoryService extends MockBaseRepositoryService implements RepositoryService {
  // モックで with が必要な Repository は override して対応
  getHogeRepository(entityManager: EntityManager): HogeRepository {
    return this.mockHogeRepository.with(this.mockFugaRepository);
  }
}

モックの実装ミスったらどうするの?という問題はあるものの、それは今までの都度モックを書くテストの書き方でも同様なので「リスクは同じなのでヨシ!」としています。

EntityManager 取り違える問題

このプロジェクトは初期にシャーディング(ユーザデータの水平分割)をしようとした痕跡があり、その名残から(?)かマスタデータを扱うDBとユーザデータを扱うDBが分かれていて、それに合わせて Connection が分かれています。具体的には以下のような感じになっています。

@Injectable()
export class DatabaseService {
  // ユーザデータを扱うコネクション
  mainConnection(): MainConnection {
    try {
      return getConnection();
    } catch (error) {
      throw new ...
    }
  }

  // マスタデータを扱うコネクション
  masterConnection(): MasterConnection {
    try {
      return getConnection(config.getMasterDBName());
    } catch (error) {
      throw new ...
    }
  }
}

そのためユーザデータを扱うときとマスタデータを扱うときで別の Connection から EntityManager を取り出して引き回しています。この EntityManager は typeorm の型で Connection が分かれていようと型の上では同じなので、変数名でしか判別出来ず、渡し間違えても気づけない可能性があります( no-unused-vars の eslint でも救えないパターンはある)

  async hoge() {
    await this.database.mainConnection().transaction(async(mainEntityManager) => {
      // ロックを取る
      await this.repositoryService.getHogeRepository(mainEntityManager).findByIdForUpdate(req.id)
      // ロジック呼び出し
      await this.fuga(
        mainEntityManager,
        this.database.masterConnection().manager,
      )
    });
  }

  async fuga(mainEntityManager:EntityManager, masterEntityManager:EntityManager) {
    const hogeRepo = this.repositoryService.getHoge(mainEntityManager);
    // たとえば masterEntityManager を渡すところに mainEntityManager を渡せてしまえる
    const masterHogeRepo = this.repositoryService.getFuga(mainEntityManager);
    const masterFugaRepo = this.repositoryService.getPiyo(masterEntityManager);
    ...
  }

実際に動かしたときに EntityManager 取り違えてると存在しないテーブルを操作しようとして死ぬので、本番まで素通りすることは考えづらく危険な状態ということではないですが、とは言え気持ちの良いものではないですし、出来れば実装中に間違っていることに気付きたいし、そもそも型が違っていてほしい。

ということで EntityManager をそのまま使うのでは無く以下のようにラップして別の型にしてしまう、ということを試みてみました。

@Injectable()
export class DatabaseService {
  mainConnection(): MainConnection {
    try {
      // MainConnection にラップ
      return new MainConnection(getConnection());
    } catch (error) {
      throw new ...
    }
  }

  masterConnection(): MasterConnection {
    try {
      // MasterConnection にラップ
      return new MasterConnection(getConnection(config.getMasterDBName()));
    } catch (error) {
      throw new ...
    }
  }
}
// 置き換えたことによって既存実装のところでメソッドが無い!と怒られるので、委譲という形で適宜実装する
// overload してるところが面倒だが最悪引数の個数を合わせて as any で渡せばなんとかなる…

export class MainConnection {
  readonly manager: MainEntityManager;

  isMainConnection() {} // MainConnection であることが分かるように

  constructor(readonly conn: Connection) {
    // mainConnection の entityManager は MainEntityManager でラップする
    this.manager = new MainEntityManager(conn.manager);
  }
}

export class MasterConnection extends BaseConnection {
  readonly manager: MasterEntityManager;

  isMasterConnection() {} // MasterConnection であることが分かるように

  constructor(readonly conn: Connection) {
    // masterConnection の entityManager は MasterEntityManager でラップする
    this.manager = new MainEntityManager(conn.manager);
  }
}

export class MainEntityManager {
  isMainEntityManager() {} // MainEntityManager であることが分かるように
}

export class MasterEntityManager {
  isMasterEntityManager() {} // MasterEntityManager であることが分かるように
}

既存の EntityManager 使っているところを MainEntityManager, MasterEntityManager に置き換えていき、メソッドが足りなくて落ちたら委譲で実装してを繰り返して EntityManager を駆逐、無事 Main, Master で型を分けることが出来ました。この対応により、既存の実装で一箇所ミスってるのを発見して修正できました。(渡し間違えていた変数はその先で使われていなかったので問題にならず放置されていた模様)

ミス発見現場

まとめ

  • テストで typeorm のモック書いているときの目からハイライトが消え虚無になるのをなんとかしてみました
    • トレードオフとして手間を増やしてはいるので全てが「ヨシ!」ではないけど、虚無が減った恩恵の方が大きいかなと(自分は)感じているのでやってよかったなと思っています
  • 中身は違うが型が同じものを変数名だけで区別するのは typescript を使ってる意味とは?となるので別の型になるようにしてみました
    • せっかく typescript という静的に型を扱える言語を使ってるので型の恩恵は受けましょう。もったいない

という感じで、細々業務と直接関係ない気になっていたところに手を入れたりしておりました。今後もぼちぼち業務と直接関係ない「いや〜これな〜」と思うところ見つけたら手を入れていきたいなと思います。とりあえず今は OpenAPI のコード生成に不安抱えてるのを何とかしたい

カヤックでは業務に直接関係ないことでも改善していくことに興味のあるエンジニアも募集してます hubspot.kayac.com