From 2dde845738846e021fee6e6c3c0f625cd0ce1e47 Mon Sep 17 00:00:00 2001 From: tamaina Date: Sun, 3 Mar 2024 23:26:35 +0000 Subject: [PATCH 1/5] fix test --- packages/backend/src/core/FetchInstanceMetadataService.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/backend/src/core/FetchInstanceMetadataService.ts b/packages/backend/src/core/FetchInstanceMetadataService.ts index 7e5185e960..d8be1153ce 100644 --- a/packages/backend/src/core/FetchInstanceMetadataService.ts +++ b/packages/backend/src/core/FetchInstanceMetadataService.ts @@ -51,7 +51,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}`); @@ -63,7 +64,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}`); } From 41a461edbe1dfe347e9a8c7394c2d6ab9e869973 Mon Sep 17 00:00:00 2001 From: tamaina Date: Sun, 3 Mar 2024 23:33:08 +0000 Subject: [PATCH 2/5] fix --- .../backend/src/core/FetchInstanceMetadataService.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/backend/src/core/FetchInstanceMetadataService.ts b/packages/backend/src/core/FetchInstanceMetadataService.ts index d8be1153ce..d937605b3c 100644 --- a/packages/backend/src/core/FetchInstanceMetadataService.ts +++ b/packages/backend/src/core/FetchInstanceMetadataService.ts @@ -73,13 +73,14 @@ export class FetchInstanceMetadataService { public async fetchInstanceMetadata(instance: MiInstance, force = false): Promise { const host = instance.host; + // unlockされてしまうのでtry内でロックチェックをしない + 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() < 1000 * 60 * 60 * 24)) { From 2926f68d8e3cd15865f4969778ac4fed00cdb4bf Mon Sep 17 00:00:00 2001 From: tamaina Date: Sun, 3 Mar 2024 23:33:25 +0000 Subject: [PATCH 3/5] comment --- packages/backend/src/core/FetchInstanceMetadataService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend/src/core/FetchInstanceMetadataService.ts b/packages/backend/src/core/FetchInstanceMetadataService.ts index d937605b3c..dc3780e270 100644 --- a/packages/backend/src/core/FetchInstanceMetadataService.ts +++ b/packages/backend/src/core/FetchInstanceMetadataService.ts @@ -73,7 +73,7 @@ export class FetchInstanceMetadataService { public async fetchInstanceMetadata(instance: MiInstance, force = false): Promise { const host = instance.host; - // unlockされてしまうのでtry内でロックチェックをしない + // finallyでunlockされてしまうのでtry内でロックチェックをしない if (!force && await this.tryLock(host) === '1') { // 1が返ってきていたらロックされているという意味なので、何もしない return; From 64fcf736cccafc6d548a5bbd4bda29a4154ed686 Mon Sep 17 00:00:00 2001 From: tamaina Date: Sun, 3 Mar 2024 23:36:03 +0000 Subject: [PATCH 4/5] comment --- packages/backend/src/core/FetchInstanceMetadataService.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/backend/src/core/FetchInstanceMetadataService.ts b/packages/backend/src/core/FetchInstanceMetadataService.ts index dc3780e270..8d173855f3 100644 --- a/packages/backend/src/core/FetchInstanceMetadataService.ts +++ b/packages/backend/src/core/FetchInstanceMetadataService.ts @@ -74,6 +74,7 @@ export class FetchInstanceMetadataService { const host = instance.host; // finallyでunlockされてしまうのでtry内でロックチェックをしない + // (returnであってもfinallyは実行される) if (!force && await this.tryLock(host) === '1') { // 1が返ってきていたらロックされているという意味なので、何もしない return; From 7eb19d5a8e413ceb8f134fc53c2917e1a1a91fa2 Mon Sep 17 00:00:00 2001 From: tamaina Date: Sun, 3 Mar 2024 23:45:47 +0000 Subject: [PATCH 5/5] improve test --- .../test/unit/FetchInstanceMetadataService.ts | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) 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(); + }); });