From 38837bd388621f5ba49bd7bf0f657a08ec2f19c4 Mon Sep 17 00:00:00 2001 From: zyoshoka <107108195+zyoshoka@users.noreply.github.com> Date: Sun, 3 Mar 2024 20:15:35 +0900 Subject: test(backend): refactor tests (#13499) * test(backend): refactor tests * fix: failed test --- .../backend/test/unit/FetchInstanceMetadataService.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'packages/backend/test/unit/FetchInstanceMetadataService.ts') diff --git a/packages/backend/test/unit/FetchInstanceMetadataService.ts b/packages/backend/test/unit/FetchInstanceMetadataService.ts index e6e68ccd6d..510b84b680 100644 --- a/packages/backend/test/unit/FetchInstanceMetadataService.ts +++ b/packages/backend/test/unit/FetchInstanceMetadataService.ts @@ -19,8 +19,8 @@ import { DI } from '@/di-symbols.js'; import type { TestingModule } from '@nestjs/testing'; function mockRedis() { - const hash = {}; - const set = jest.fn((key, value) => { + const hash = {} as any; + const set = jest.fn((key: string, value) => { const ret = hash[key]; hash[key] = value; return ret; @@ -61,7 +61,7 @@ describe('FetchInstanceMetadataService', () => { app.enableShutdownHooks(); - fetchInstanceMetadataService = app.get(FetchInstanceMetadataService); + fetchInstanceMetadataService = app.get(FetchInstanceMetadataService) as jest.Mocked; federatedInstanceService = app.get(FederatedInstanceService) as jest.Mocked; redisClient = app.get(DI.redis) as jest.Mocked; httpRequestService = app.get(HttpRequestService) as jest.Mocked; @@ -74,11 +74,11 @@ describe('FetchInstanceMetadataService', () => { test('Lock and update', async () => { redisClient.set = mockRedis(); const now = Date.now(); - federatedInstanceService.fetch.mockReturnValue({ infoUpdatedAt: { getTime: () => { return now - 10 * 1000 * 60 * 60 * 24; } } }); + federatedInstanceService.fetch.mockResolvedValue({ infoUpdatedAt: { getTime: () => { return now - 10 * 1000 * 60 * 60 * 24; } } } as any); httpRequestService.getJson.mockImplementation(() => { throw Error(); }); const tryLockSpy = jest.spyOn(fetchInstanceMetadataService, 'tryLock'); const unlockSpy = jest.spyOn(fetchInstanceMetadataService, 'unlock'); - await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' }); + await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' } as any); expect(tryLockSpy).toHaveBeenCalledTimes(1); expect(unlockSpy).toHaveBeenCalledTimes(1); expect(federatedInstanceService.fetch).toHaveBeenCalledTimes(1); @@ -88,11 +88,11 @@ describe('FetchInstanceMetadataService', () => { test('Lock and don\'t update', async () => { redisClient.set = mockRedis(); const now = Date.now(); - federatedInstanceService.fetch.mockReturnValue({ infoUpdatedAt: { getTime: () => now } }); + federatedInstanceService.fetch.mockResolvedValue({ infoUpdatedAt: { getTime: () => now } } as any); httpRequestService.getJson.mockImplementation(() => { throw Error(); }); const tryLockSpy = jest.spyOn(fetchInstanceMetadataService, 'tryLock'); const unlockSpy = jest.spyOn(fetchInstanceMetadataService, 'unlock'); - await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' }); + await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' } as any); expect(tryLockSpy).toHaveBeenCalledTimes(1); expect(unlockSpy).toHaveBeenCalledTimes(1); expect(federatedInstanceService.fetch).toHaveBeenCalledTimes(1); @@ -101,12 +101,13 @@ describe('FetchInstanceMetadataService', () => { test('Do nothing when lock not acquired', async () => { redisClient.set = mockRedis(); - federatedInstanceService.fetch.mockReturnValue({ infoUpdatedAt: { getTime: () => now - 10 * 1000 * 60 * 60 * 24 } }); + const now = Date.now(); + federatedInstanceService.fetch.mockResolvedValue({ infoUpdatedAt: { getTime: () => now - 10 * 1000 * 60 * 60 * 24 } } as any); httpRequestService.getJson.mockImplementation(() => { throw Error(); }); const tryLockSpy = jest.spyOn(fetchInstanceMetadataService, 'tryLock'); const unlockSpy = jest.spyOn(fetchInstanceMetadataService, 'unlock'); await fetchInstanceMetadataService.tryLock('example.com'); - await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' }); + await fetchInstanceMetadataService.fetchInstanceMetadata({ host: 'example.com' } as any); expect(tryLockSpy).toHaveBeenCalledTimes(2); expect(unlockSpy).toHaveBeenCalledTimes(0); expect(federatedInstanceService.fetch).toHaveBeenCalledTimes(0); -- cgit v1.2.3-freya From 9542cb8d6253a93b06a68b9ac3647367f8f7354c Mon Sep 17 00:00:00 2001 From: tamaina Date: Mon, 4 Mar 2024 13:48:57 +0900 Subject: fix(backend): リモートサーバーの情報が更新できなくなっていた問題を修正 (#13507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(backend): fetchInstanceMetadataのLockが永遠に解除されない問題を修正 Co-authored-by: まっちゃとーにゅ <17376330+u1-liquid@users.noreply.github.com> * fix test * fix * comment * comment * improve test --------- Co-authored-by: まっちゃとーにゅ <17376330+u1-liquid@users.noreply.github.com> --- .../src/core/FetchInstanceMetadataService.ts | 28 ++++++++++++++++------ .../test/unit/FetchInstanceMetadataService.ts | 24 +++++++++++++++++-- 2 files changed, 43 insertions(+), 9 deletions(-) (limited to 'packages/backend/test/unit/FetchInstanceMetadataService.ts') diff --git a/packages/backend/src/core/FetchInstanceMetadataService.ts b/packages/backend/src/core/FetchInstanceMetadataService.ts index bc270bd28f..8d173855f3 100644 --- a/packages/backend/src/core/FetchInstanceMetadataService.ts +++ b/packages/backend/src/core/FetchInstanceMetadataService.ts @@ -51,21 +51,35 @@ export class FetchInstanceMetadataService { } @bindThis - public async tryLock(host: string): Promise { - const mutex = await this.redisClient.set(`fetchInstanceMetadata:mutex:${host}`, '1', 'GET'); - return mutex !== '1'; + // public for test + public async tryLock(host: string): Promise { + // TODO: マイグレーションなのであとで消す (2024.3.1) + this.redisClient.del(`fetchInstanceMetadata:mutex:${host}`); + + return await this.redisClient.set( + `fetchInstanceMetadata:mutex:v2:${host}`, '1', + 'EX', 30, // 30秒したら自動でロック解除 https://github.com/misskey-dev/misskey/issues/13506#issuecomment-1975375395 + 'GET' // 古い値を返す（なかったらnull） + ); } @bindThis - public unlock(host: string): Promise<'OK'> { - return this.redisClient.set(`fetchInstanceMetadata:mutex:${host}`, '0'); + // public for test + public unlock(host: string): Promise { + return this.redisClient.del(`fetchInstanceMetadata:mutex:v2:${host}`); } @bindThis public async fetchInstanceMetadata(instance: MiInstance, force = false): Promise { const host = instance.host; - // Acquire mutex to ensure no parallel runs - if (!await this.tryLock(host)) return; + + // finallyでunlockされてしまうのでtry内でロックチェックをしない + // （returnであってもfinallyは実行される） + if (!force && await this.tryLock(host) === '1') { + // 1が返ってきていたらロックされているという意味なので、何もしない + return; + } + try { if (!force) { const _instance = await this.federatedInstanceService.fetch(host); 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(); + }); }); -- cgit v1.2.3-freya