diff --git a/packages/backend/src/core/FetchInstanceMetadataService.ts b/packages/backend/src/core/FetchInstanceMetadataService.ts index 7f5543620d..54dc925540 100644 --- a/packages/backend/src/core/FetchInstanceMetadataService.ts +++ b/packages/backend/src/core/FetchInstanceMetadataService.ts @@ -56,7 +56,8 @@ export class FetchInstanceMetadataService { } @bindThis - private async tryLock(host: string): Promise { + // public for test + public async tryLock(host: string): Promise { // TODO: マイグレーションなのであとで消す (2024.3.1) this.redisClient.del(`fetchInstanceMetadata:mutex:${host}`); @@ -68,7 +69,8 @@ export class FetchInstanceMetadataService { } @bindThis - private unlock(host: string): Promise { + // public for test + public unlock(host: string): Promise { return this.redisClient.del(`fetchInstanceMetadata:mutex:v2:${host}`); } @@ -76,13 +78,15 @@ export class FetchInstanceMetadataService { public async fetchInstanceMetadata(instance: MiInstance, force = false): Promise { const host = instance.host; + // finallyでunlockされてしまうのでtry内でロックチェックをしない + // (returnであってもfinallyは実行される) + if (!force && await this.tryLock(host) === '1') { + // 1が返ってきていたらロックされているという意味なので、何もしない + return; + } + try { if (!force) { - if (await this.tryLock(host) === '1') { - // 1が返ってきていたらロックされている = 何もしない - return; - } - const _instance = await this.federatedInstanceService.fetch(host); const now = Date.now(); if (_instance && _instance.infoUpdatedAt && (now - _instance.infoUpdatedAt.getTime() < REMOTE_SERVER_CACHE_TTL)) { diff --git a/packages/backend/test/unit/FetchInstanceMetadataService.ts b/packages/backend/test/unit/FetchInstanceMetadataService.ts index 510b84b680..bf8f3ab0e3 100644 --- a/packages/backend/test/unit/FetchInstanceMetadataService.ts +++ b/packages/backend/test/unit/FetchInstanceMetadataService.ts @@ -56,6 +56,7 @@ describe('FetchInstanceMetadataService', () => { } else if (token === DI.redis) { return mockRedis; } + return null; }) .compile(); @@ -78,6 +79,7 @@ describe('FetchInstanceMetadataService', () => { httpRequestService.getJson.mockImplementation(() => { throw Error(); }); const tryLockSpy = jest.spyOn(fetchInstanceMetadataService, 'tryLock'); const unlockSpy = jest.spyOn(fetchInstanceMetadataService, 'unlock'); + await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' } as any); expect(tryLockSpy).toHaveBeenCalledTimes(1); expect(unlockSpy).toHaveBeenCalledTimes(1); @@ -92,6 +94,7 @@ describe('FetchInstanceMetadataService', () => { httpRequestService.getJson.mockImplementation(() => { throw Error(); }); const tryLockSpy = jest.spyOn(fetchInstanceMetadataService, 'tryLock'); const unlockSpy = jest.spyOn(fetchInstanceMetadataService, 'unlock'); + await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' } as any); expect(tryLockSpy).toHaveBeenCalledTimes(1); expect(unlockSpy).toHaveBeenCalledTimes(1); @@ -104,13 +107,30 @@ describe('FetchInstanceMetadataService', () => { const now = Date.now(); federatedInstanceService.fetch.mockResolvedValue({ infoUpdatedAt: { getTime: () => now - 10 * 1000 * 60 * 60 * 24 } } as any); httpRequestService.getJson.mockImplementation(() => { throw Error(); }); + await fetchInstanceMetadataService.tryLock('example.com'); const tryLockSpy = jest.spyOn(fetchInstanceMetadataService, 'tryLock'); const unlockSpy = jest.spyOn(fetchInstanceMetadataService, 'unlock'); - await fetchInstanceMetadataService.tryLock('example.com'); + await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' } as any); - expect(tryLockSpy).toHaveBeenCalledTimes(2); + expect(tryLockSpy).toHaveBeenCalledTimes(1); expect(unlockSpy).toHaveBeenCalledTimes(0); expect(federatedInstanceService.fetch).toHaveBeenCalledTimes(0); expect(httpRequestService.getJson).toHaveBeenCalledTimes(0); }); + + test('Do when lock not acquired but forced', async () => { + redisClient.set = mockRedis(); + const now = Date.now(); + federatedInstanceService.fetch.mockResolvedValue({ infoUpdatedAt: { getTime: () => now - 10 * 1000 * 60 * 60 * 24 } } as any); + httpRequestService.getJson.mockImplementation(() => { throw Error(); }); + await fetchInstanceMetadataService.tryLock('example.com'); + const tryLockSpy = jest.spyOn(fetchInstanceMetadataService, 'tryLock'); + const unlockSpy = jest.spyOn(fetchInstanceMetadataService, 'unlock'); + + await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' } as any, true); + expect(tryLockSpy).toHaveBeenCalledTimes(0); + expect(unlockSpy).toHaveBeenCalledTimes(1); + expect(federatedInstanceService.fetch).toHaveBeenCalledTimes(0); + expect(httpRequestService.getJson).toHaveBeenCalled(); + }); });