diff --git a/src/services/hiboutik/hiboutik.service.spec.ts b/src/services/hiboutik/hiboutik.service.spec.ts index 6da351d..87ef133 100644 --- a/src/services/hiboutik/hiboutik.service.spec.ts +++ b/src/services/hiboutik/hiboutik.service.spec.ts @@ -62,30 +62,17 @@ describe('HiboutikService', () => { it('refuses a second synchronization while one is running, and unlocks at the end', async () => { let finishBrands: () => void = () => undefined; brandService.syncBrands.mockReturnValue(new Promise((resolve) => (finishBrands = resolve))); + // The lock is the only guard: the second request does not get it, even from the same instance + syncLock.tryAcquire.mockResolvedValueOnce(release).mockResolvedValueOnce(null); await expect(service.startSync()).resolves.toBe(true); - expect(service.isSyncInProgress).toBe(true); await expect(service.startSync()).resolves.toBe(false); - expect(syncLock.tryAcquire).toHaveBeenCalledTimes(1); + expect(brandService.syncBrands).toHaveBeenCalledTimes(1); expect(release).not.toHaveBeenCalled(); finishBrands(); await flush(); expect(release).toHaveBeenCalledTimes(1); - expect(service.isSyncInProgress).toBe(false); - }); - - it('refuses concurrent calls on the same instance while the lock is being requested', async () => { - let grantLock: (value: typeof release) => void = () => undefined; - syncLock.tryAcquire.mockReturnValue(new Promise((resolve) => (grantLock = resolve))); - - const first = service.startSync(); - await expect(service.startSync()).resolves.toBe(false); - - grantLock(release); - await expect(first).resolves.toBe(true); - await flush(); - expect(syncLock.tryAcquire).toHaveBeenCalledTimes(1); }); it('starts nothing when another instance holds the lock', async () => { @@ -93,7 +80,6 @@ describe('HiboutikService', () => { await expect(service.startSync()).resolves.toBe(false); - expect(service.isSyncInProgress).toBe(false); expect(brandService.syncBrands).not.toHaveBeenCalled(); expect(release).not.toHaveBeenCalled(); }); @@ -102,19 +88,18 @@ describe('HiboutikService', () => { syncLock.tryAcquire.mockRejectedValueOnce(new Error('database unreachable')); await expect(service.startSync()).rejects.toThrow('database unreachable'); - expect(service.isSyncInProgress).toBe(false); + expect(brandService.syncBrands).not.toHaveBeenCalled(); await expect(service.startSync()).resolves.toBe(true); await flush(); }); it('releases the lock even when the synchronization crashes', async () => { - jest.spyOn(service, 'syncAll').mockRejectedValue(new Error('crash')); + jest.spyOn(service as never, 'syncAll').mockRejectedValue(new Error('crash')); await expect(service.startSync()).resolves.toBe(true); await flush(); expect(release).toHaveBeenCalledTimes(1); - expect(service.isSyncInProgress).toBe(false); }); }); diff --git a/src/services/hiboutik/hiboutik.service.ts b/src/services/hiboutik/hiboutik.service.ts index 89205df..54375ec 100644 --- a/src/services/hiboutik/hiboutik.service.ts +++ b/src/services/hiboutik/hiboutik.service.ts @@ -10,8 +10,6 @@ import { ReleaseLock, SyncLockDatabaseRepository } from '../../database/syncLock @Injectable() export class HiboutikService { private readonly logger = new Logger('HiboutikService'); - /** Cheap in-process guard, checked before the database lock (which is the one that holds across instances). */ - private syncInProgress = false; constructor( private readonly brandHiboutikService: BrandHiboutikService, @@ -22,32 +20,16 @@ export class HiboutikService { private readonly syncLock: SyncLockDatabaseRepository, ) {} - get isSyncInProgress(): boolean { - return this.syncInProgress; - } - /** - * Starts a full synchronization in the background, if no other one is running on this instance or any other - * (Postgres advisory lock, held until the synchronization ends). + * Starts a full synchronization in the background, if no other one is running on this instance or any other. + * The Postgres advisory lock is the only guard: each request takes it on its own connection (its own session), + * so it also refuses concurrent calls on this very instance. It is held until the synchronization ends. * @returns false if a synchronization is already running (nothing is started). * @throws if the lock cannot be requested (e.g. database unreachable). */ async startSync(): Promise { - if (this.syncInProgress) return false; - // Set before the first await so that concurrent calls on this instance are refused while the lock is requested - this.syncInProgress = true; - - let release: ReleaseLock | null; - try { - release = await this.syncLock.tryAcquire(); - } catch (error) { - this.syncInProgress = false; - throw error; - } - if (!release) { - this.syncInProgress = false; - return false; - } + const release = await this.syncLock.tryAcquire(); + if (!release) return false; void this.runLocked(release); return true; @@ -61,12 +43,11 @@ export class HiboutikService { this.logger.error('Global Hiboutik synchronization crashed', (error as Error)?.stack); } finally { await release(); - this.syncInProgress = false; } } /** Runs every sync step in order. A failing step is logged and does not stop the next ones. */ - async syncAll() { + private async syncAll() { this.logger.log('Starting global Hiboutik synchronization'); // Order matters: products connect to brands, categories and suppliers by hiboutikId. const steps: [string, () => Promise][] = [