feat(security): harden the API, trust Hiboutik for deletions, lock syncs (#3)

- Sync deletions: a 2xx Hiboutik answer is the truth, even an empty list; a failed call (HTTP error or non-list body) deletes nothing (HiboutikClient.getList).
- Validate the environment at startup (HIBOUTIK_BASE_URL must be HTTPS), global ValidationPipe, Helmet, CORS opt-in, HTTP timeouts, rate limiting (60 req/min/IP, 5 on POST /sync), AllExceptionsFilter.
- Postgres advisory lock so only one synchronization runs at a time across instances; syncAll is private, syncs start through startSync.
- Fix the Jest moduleNameMapper; document the validation and security rules in CLAUDE.md.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-11-08 09:36:50 +00:00
parent 8b341c4933
commit 8c8a2ccf15
31 changed files with 923 additions and 499 deletions

View File

@@ -16,14 +16,14 @@ describe('AppController', () => {
controller = module.get(AppController);
});
it('starts a synchronization', () => {
hiboutikService.startSync.mockReturnValue(true);
expect(controller.hiboutikSync()).toEqual({ message: 'Hiboutik synchronization started' });
it('starts a synchronization', async () => {
hiboutikService.startSync.mockResolvedValue(true);
await expect(controller.hiboutikSync()).resolves.toEqual({ message: 'Hiboutik synchronization started' });
expect(hiboutikService.startSync).toHaveBeenCalledTimes(1);
});
it('rejects a synchronization when one is already running', () => {
hiboutikService.startSync.mockReturnValue(false);
expect(() => controller.hiboutikSync()).toThrow(ConflictException);
it('rejects a synchronization when one is already running', async () => {
hiboutikService.startSync.mockResolvedValue(false);
await expect(controller.hiboutikSync()).rejects.toThrow(ConflictException);
});
});

View File

@@ -1,15 +1,20 @@
import { ConflictException, Controller, HttpCode, HttpStatus, Post } from '@nestjs/common';
import { Throttle } from '@nestjs/throttler';
import { HiboutikService } from './services/hiboutik/hiboutik.service';
@Controller()
export class AppController {
constructor(private readonly hiboutikService: HiboutikService) {}
/** Starts the synchronization in the background and answers immediately: 202, or 409 if one is already running. */
/**
* Starts the synchronization in the background and answers immediately: 202, or 409 if one is already running.
* Rate limited to 5 calls per minute: each one can trigger thousands of Hiboutik API calls.
*/
@Post('sync')
@Throttle({ default: { limit: 5, ttl: 60_000 } })
@HttpCode(HttpStatus.ACCEPTED)
hiboutikSync() {
if (!this.hiboutikService.startSync()) {
async hiboutikSync() {
if (!(await this.hiboutikService.startSync())) {
throw new ConflictException('A Hiboutik synchronization is already in progress');
}
return { message: 'Hiboutik synchronization started' };

View File

@@ -1,6 +1,10 @@
import { Module } from '@nestjs/common';
import { APP_FILTER, APP_GUARD } from '@nestjs/core';
import { AllExceptionsFilter } from './filters/all-exceptions.filter';
import { ThrottlerGuard, ThrottlerModule } from '@nestjs/throttler';
import { AppController } from './app.controller';
import { ConfigModule } from '@nestjs/config';
import { validateEnv } from './config/env.validation';
import { HiboutikService } from './services/hiboutik/hiboutik.service';
import { CategoryHiboutikRepository } from './hiboutik/category.hiboutik.repository';
import { CategoryConverter } from './models/converter/category.converter';
@@ -32,17 +36,23 @@ import { CategoryHiboutikService } from './services/hiboutik/category.hiboutik.s
import { CustomerHiboutikService } from './services/hiboutik/customer.hiboutik.service';
import { SupplierHiboutikService } from './services/hiboutik/supplier.hiboutik.service';
import { ProductHiboutikService } from './services/hiboutik/product.hiboutik.service';
import { SyncLockDatabaseRepository } from './database/syncLock.database.repository';
@Module({
imports: [
ConfigModule.forRoot({
isGlobal: true,
validate: validateEnv,
}),
// Default rate limit for every route: 60 requests per minute per IP (stricter limits via @Throttle)
ThrottlerModule.forRoot([{ ttl: 60_000, limit: 60 }]),
HttpModule.register({ timeout: 30_000 }),
PrismaModule,
],
controllers: [AppController],
providers: [
{ provide: APP_GUARD, useClass: ThrottlerGuard },
{ provide: APP_FILTER, useClass: AllExceptionsFilter },
BrandConverter,
BrandDatabaseRepository,
BrandHiboutikRepository,
@@ -70,6 +80,7 @@ import { ProductHiboutikService } from './services/hiboutik/product.hiboutik.ser
StockHiboutikRepository,
SupplierConverter,
SupplierDatabaseRepository,
SyncLockDatabaseRepository,
SupplierHiboutikRepository,
SupplierHiboutikService,
],

View File

@@ -0,0 +1,39 @@
import 'reflect-metadata';
import { validateEnv } from './env.validation';
describe('validateEnv', () => {
const valid = {
DATABASE_URL: 'postgresql://user:secret@localhost:5432/db',
HIBOUTIK_BASE_URL: 'https://shop.hiboutik.com/api',
HIBOUTIK_USERNAME: 'user',
HIBOUTIK_PASSWORD: 'secret-password',
};
it('accepts a valid configuration and keeps every variable', () => {
const env = { ...valid, PORT: '3000', CORS_ORIGINS: 'https://shop.example.com', TRUST_PROXY: '1', OTHER: 'x' };
expect(validateEnv(env)).toBe(env);
});
it('rejects a missing required variable, naming it', () => {
expect(() => validateEnv({ ...valid, HIBOUTIK_PASSWORD: undefined })).toThrow(/HIBOUTIK_PASSWORD/);
});
it('rejects a non-HTTPS Hiboutik URL', () => {
expect(() => validateEnv({ ...valid, HIBOUTIK_BASE_URL: 'http://shop.hiboutik.com/api' })).toThrow(/HIBOUTIK_BASE_URL/);
});
it('rejects an invalid port', () => {
expect(() => validateEnv({ ...valid, PORT: 'abc' })).toThrow(/PORT/);
expect(() => validateEnv({ ...valid, PORT: '70000' })).toThrow(/PORT/);
});
it('never puts secret values in the error message', () => {
expect.assertions(2);
try {
validateEnv({ ...valid, HIBOUTIK_BASE_URL: 'http://insecure', DATABASE_URL: '' });
} catch (error) {
expect((error as Error).message).not.toContain('secret-password');
expect((error as Error).message).not.toContain('secret');
}
});
});

View File

@@ -0,0 +1,51 @@
import { plainToInstance } from 'class-transformer';
import { IsInt, IsNotEmpty, IsOptional, IsString, IsUrl, Max, Min, validateSync } from 'class-validator';
/** Environment variables read by the app. A missing or malformed required one stops the startup. */
export class EnvironmentVariables {
@IsString()
@IsNotEmpty()
DATABASE_URL!: string;
/** HTTPS only: the Hiboutik credentials are sent with HTTP Basic auth. */
@IsUrl({ protocols: ['https'], require_protocol: true })
HIBOUTIK_BASE_URL!: string;
@IsString()
@IsNotEmpty()
HIBOUTIK_USERNAME!: string;
@IsString()
@IsNotEmpty()
HIBOUTIK_PASSWORD!: string;
@IsOptional()
@IsInt()
@Min(1)
@Max(65535)
PORT?: number;
/** Comma-separated list of origins allowed by CORS. Empty or unset: CORS stays disabled. */
@IsOptional()
@IsString()
CORS_ORIGINS?: string;
/** Number of reverse proxies in front of the app (so the real client IP is used for rate limiting). Unset: no proxy. */
@IsOptional()
@IsInt()
@Min(1)
@Max(10)
TRUST_PROXY?: number;
}
/** `ConfigModule` validation hook: fails fast with the names of the faulty variables (never their values, they may be secrets). */
export function validateEnv(config: Record<string, unknown>): Record<string, unknown> {
const validated = plainToInstance(EnvironmentVariables, config, { enableImplicitConversion: true });
const errors = validateSync(validated, { whitelist: false });
if (errors.length > 0) {
const details = errors.map((error) => `${error.property}: ${Object.values(error.constraints ?? {}).join(', ')}`).join('\n ');
throw new Error(`Invalid environment configuration:\n ${details}`);
}
return config;
}

View File

@@ -0,0 +1,74 @@
import { Logger } from '@nestjs/common';
import { Client } from 'pg';
import { SyncLockDatabaseRepository } from './syncLock.database.repository';
jest.mock('pg', () => ({ Client: jest.fn() }));
describe('SyncLockDatabaseRepository', () => {
const client = { on: jest.fn(), connect: jest.fn(), query: jest.fn(), end: jest.fn() };
const configService = { get: jest.fn().mockReturnValue('postgresql://user:pass@localhost:5432/db') };
let repository: SyncLockDatabaseRepository;
beforeAll(() => {
jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined);
jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
});
beforeEach(() => {
jest.clearAllMocks();
(Client as unknown as jest.Mock).mockImplementation(() => client);
client.connect.mockResolvedValue(undefined);
client.end.mockResolvedValue(undefined);
client.query.mockResolvedValue({ rows: [{ locked: true }] });
repository = new SyncLockDatabaseRepository(configService as never);
});
it('takes the advisory lock on a dedicated connection and keeps it open', async () => {
const release = await repository.tryAcquire();
expect(release).toEqual(expect.any(Function));
expect(Client).toHaveBeenCalledWith({ connectionString: 'postgresql://user:pass@localhost:5432/db' });
expect(client.query).toHaveBeenCalledWith('SELECT pg_try_advisory_lock($1) AS locked', [expect.any(Number)]);
expect(client.end).not.toHaveBeenCalled();
});
it('returns null and closes the connection when the lock is already held', async () => {
client.query.mockResolvedValue({ rows: [{ locked: false }] });
await expect(repository.tryAcquire()).resolves.toBeNull();
expect(client.end).toHaveBeenCalledTimes(1);
});
it('unlocks then closes the connection on release', async () => {
const release = await repository.tryAcquire();
await release!();
expect(client.query).toHaveBeenLastCalledWith('SELECT pg_advisory_unlock($1)', [expect.any(Number)]);
expect(client.end).toHaveBeenCalledTimes(1);
});
it('still closes the connection, and does not throw, when the explicit unlock fails', async () => {
const release = await repository.tryAcquire();
client.query.mockRejectedValueOnce(new Error('connection lost'));
await expect(release!()).resolves.toBeUndefined();
expect(client.end).toHaveBeenCalledTimes(1);
});
it('closes the connection and rethrows when the lock query fails', async () => {
client.query.mockRejectedValue(new Error('boom'));
await expect(repository.tryAcquire()).rejects.toThrow('boom');
expect(client.end).toHaveBeenCalledTimes(1);
});
it('logs connection errors instead of leaving them unhandled', async () => {
await repository.tryAcquire();
expect(client.on).toHaveBeenCalledWith('error', expect.any(Function));
});
});

View File

@@ -0,0 +1,52 @@
import { Injectable, Logger } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { Client } from 'pg';
/** Arbitrary constant identifying the Hiboutik synchronization among the Postgres advisory locks of the database. */
const SYNC_LOCK_KEY = 727_001;
/** Function releasing a lock acquired with `tryAcquire`. Never throws. */
export type ReleaseLock = () => Promise<void>;
/**
* Cross-instance lock for the synchronization, based on a Postgres session-level advisory lock.
* It lives on its own dedicated connection (not the Prisma pool, where lock and unlock could land on different
* connections) kept open for as long as the lock is held. If the process dies, Postgres closes the connection and
* releases the lock by itself: there is never a stale lock to clean up.
*/
@Injectable()
export class SyncLockDatabaseRepository {
private readonly logger = new Logger('SyncLockDatabaseRepository');
constructor(private readonly configService: ConfigService) {}
/** @returns the function releasing the lock, or `null` if another instance (or process) already holds it. */
async tryAcquire(): Promise<ReleaseLock | null> {
const client = new Client({ connectionString: this.configService.get<string>('DATABASE_URL') });
// A lost connection means Postgres released the lock: log it instead of crashing the process on an unhandled 'error'
client.on('error', (error) => this.logger.error(`Synchronization lock connection error: ${error.message}`));
await client.connect();
try {
const { rows } = await client.query<{ locked: boolean }>('SELECT pg_try_advisory_lock($1) AS locked', [SYNC_LOCK_KEY]);
if (!rows[0]?.locked) {
await client.end();
return null;
}
} catch (error) {
await client.end().catch(() => undefined);
throw error;
}
return async () => {
try {
await client.query('SELECT pg_advisory_unlock($1)', [SYNC_LOCK_KEY]);
} catch (error) {
// Ending the connection below releases the lock anyway
this.logger.warn(`Failed to unlock the synchronization lock explicitly: ${(error as Error)?.message}`);
} finally {
await client.end().catch(() => undefined);
}
};
}
}

View File

@@ -0,0 +1,58 @@
import { BadRequestException, Logger } from '@nestjs/common';
import { AllExceptionsFilter } from './all-exceptions.filter';
describe('AllExceptionsFilter', () => {
const applicationRef = { reply: jest.fn(), isHeadersSent: jest.fn().mockReturnValue(false), end: jest.fn() };
const response = {};
const host = { getArgByIndex: jest.fn().mockReturnValue(response) };
let filter: AllExceptionsFilter;
let logError: jest.SpyInstance;
beforeEach(() => {
jest.clearAllMocks();
logError = jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
filter = new AllExceptionsFilter(applicationRef as never);
});
it('answers a generic 500 for an unexpected error, without leaking its message', () => {
filter.catch(new Error('relation "customer" does not exist'), host as never);
expect(applicationRef.reply).toHaveBeenCalledTimes(1);
const [, body, status] = applicationRef.reply.mock.calls[0] as [unknown, { message: string }, number];
expect(status).toBe(500);
expect(JSON.stringify(body)).not.toContain('customer');
});
it('logs the message and stack of an unexpected error, never the raw object', () => {
const error = Object.assign(new Error('boom'), { config: { headers: { Authorization: 'Basic c2VjcmV0' } } });
filter.catch(error, host as never);
expect(logError).toHaveBeenCalledWith('Unhandled exception: boom', error.stack);
expect(JSON.stringify(logError.mock.calls)).not.toContain('c2VjcmV0');
});
it('keeps the status and body of deliberate HTTP exceptions', () => {
filter.catch(new BadRequestException('limit must be an integer'), host as never);
const [, body, status] = applicationRef.reply.mock.calls[0] as [unknown, { message: string }, number];
expect(status).toBe(400);
expect(body.message).toBe('limit must be an integer');
expect(logError).not.toHaveBeenCalled();
});
it('keeps the 4xx of HTTP-layer errors such as a payload too large', () => {
filter.catch(Object.assign(new Error('request entity too large'), { statusCode: 413 }), host as never);
const [, , status] = applicationRef.reply.mock.calls[0] as [unknown, unknown, number];
expect(status).toBe(413);
});
it('treats a 5xx HTTP-layer error as an unexpected one', () => {
filter.catch(Object.assign(new Error('upstream detail'), { statusCode: 502 }), host as never);
const [, body, status] = applicationRef.reply.mock.calls[0] as [unknown, unknown, number];
expect(status).toBe(500);
expect(JSON.stringify(body)).not.toContain('upstream');
});
});

View File

@@ -0,0 +1,29 @@
import { ArgumentsHost, Catch, HttpException, InternalServerErrorException, Logger } from '@nestjs/common';
import { BaseExceptionFilter } from '@nestjs/core';
/** Errors raised by the HTTP layer itself (malformed JSON, payload too large...) carry a 4xx `statusCode` but are not `HttpException`s. */
function isClientHttpError(exception: unknown): boolean {
const statusCode = (exception as { statusCode?: unknown } | null)?.statusCode;
return typeof statusCode === 'number' && statusCode >= 400 && statusCode < 500;
}
/**
* Last line of defence: anything that is not a deliberate HTTP error (Prisma, axios, bug...) is logged with its stack
* (message and stack only, never the raw error object: it may carry credentials) and answered with a generic 500,
* so no internal detail (SQL, table names, URLs, stack) ever reaches the client.
*/
@Catch()
export class AllExceptionsFilter extends BaseExceptionFilter {
private readonly logger = new Logger('ExceptionsFilter');
catch(exception: unknown, host: ArgumentsHost): void {
if (exception instanceof HttpException || isClientHttpError(exception)) {
super.catch(exception, host);
return;
}
const error = exception as Error | undefined;
this.logger.error(`Unhandled exception: ${error?.message}`, error?.stack);
super.catch(new InternalServerErrorException(), host);
}
}

View File

@@ -0,0 +1,66 @@
import { of, throwError } from 'rxjs';
import { HiboutikClient } from './hiboutik.client';
describe('HiboutikClient', () => {
const httpService = { get: jest.fn() };
const configService = { get: jest.fn((key: string) => (key === 'HIBOUTIK_BASE_URL' ? 'https://shop.hiboutik.test/api' : 'x')) };
let client: HiboutikClient;
const answer = (data: unknown) => of({ data });
const page = (size: number) => Array.from({ length: size }, (_, index) => ({ id: index }));
beforeEach(() => {
jest.clearAllMocks();
client = new HiboutikClient(httpService as never, configService as never);
});
describe('getList', () => {
it('returns the list of a 2xx answer, empty or not', async () => {
httpService.get.mockReturnValueOnce(answer([{ id: 1 }])).mockReturnValueOnce(answer([]));
await expect(client.getList('/x')).resolves.toEqual([{ id: 1 }]);
await expect(client.getList('/x')).resolves.toEqual([]);
});
it('treats a 2xx answer without body as an empty list', async () => {
httpService.get.mockReturnValueOnce(answer('')).mockReturnValueOnce(answer(null));
await expect(client.getList('/x')).resolves.toEqual([]);
await expect(client.getList('/x')).resolves.toEqual([]);
});
it('rejects a 2xx answer that is not a list, without leaking its content', async () => {
httpService.get.mockReturnValueOnce(answer({ error: 'secret details' }));
const result = client.getList('/x');
await expect(result).rejects.toThrow('a list was expected');
await expect(result).rejects.not.toThrow('secret details');
});
it('lets HTTP errors through', async () => {
httpService.get.mockReturnValueOnce(throwError(() => new Error('Request failed with status code 500')));
await expect(client.getList('/x')).rejects.toThrow('500');
});
});
describe('getAllPages', () => {
it('walks the pages until a short one', async () => {
httpService.get.mockReturnValueOnce(answer(page(250))).mockReturnValueOnce(answer(page(10)));
const results = await client.getAllPages('/x');
expect(results).toHaveLength(260);
expect(httpService.get).toHaveBeenCalledTimes(2);
});
it('fails as a whole when a later page fails, instead of returning a truncated list', async () => {
httpService.get
.mockReturnValueOnce(answer(page(250)))
.mockReturnValueOnce(throwError(() => new Error('Request failed with status code 502')));
await expect(client.getAllPages('/x')).rejects.toThrow('502');
});
});
});

View File

@@ -33,6 +33,17 @@ export class HiboutikClient {
return response.data;
}
/**
* GET on a list endpoint. HTTP errors (4xx/5xx) throw, a 2xx answer without body is an empty list, and any other
* non-list body throws: an empty list triggers deletions in the sync, so it must never be inferred from a malformed answer.
*/
async getList<T>(path: string, params?: Record<string, unknown>): Promise<T[]> {
const data = await this.get<T[] | '' | null | undefined>(path, params);
if (data === '' || data == null) return [];
if (!Array.isArray(data)) throw new Error(`Unexpected answer from Hiboutik ${path}: a list was expected`);
return data;
}
/**
* Fetches every page of a paginated endpoint (`p` param, starting at 1) and concatenates the results.
* Only for paginated endpoints: on a non-paginated one, a full first page would loop forever on the same results.
@@ -42,8 +53,8 @@ export class HiboutikClient {
let page = 1;
while (true) {
const pageData = await this.get<T[]>(path, { ...baseParams, p: page });
if (!pageData || pageData.length === 0) break;
const pageData = await this.getList<T>(path, { ...baseParams, p: page });
if (pageData.length === 0) break;
results.push(...pageData);
// A page smaller than the page size is the last one: no need to request an empty page
if (pageData.length < HIBOUTIK_PAGE_SIZE) break;

View File

@@ -9,7 +9,7 @@ export class StockHiboutikRepository {
/** Stock of every product and size, all warehouses combined. Not paginated. */
async getStocks(): Promise<StockHiboutikDto[]> {
const stocks = await this.hiboutikClient.get<StockHiboutikDto[]>('/stock_available/all_wh', {});
const stocks = await this.hiboutikClient.getList<StockHiboutikDto>('/stock_available/all_wh', {});
// Hiboutik may return ids as strings: normalize them so strict comparisons work downstream
return stocks.map((stock) => ({
...stock,

View File

@@ -9,7 +9,7 @@ export class SupplierHiboutikRepository {
/** `/suppliers` is not paginated: everything comes in one call. */
async getSuppliers(): Promise<SupplierHiboutikDto[]> {
return this.hiboutikClient.get<SupplierHiboutikDto[]>('/suppliers', {
return this.hiboutikClient.getList<SupplierHiboutikDto>('/suppliers', {
order_by: 'supplier_id',
sort: 'ASC',
});

View File

@@ -1,8 +1,36 @@
import { ValidationPipe } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { NestFactory } from '@nestjs/core';
import { NestExpressApplication } from '@nestjs/platform-express';
import helmet from 'helmet';
import { AppModule } from './app.module';
async function bootstrap() {
const app = await NestFactory.create(AppModule);
await app.listen(process.env.PORT ?? 3000);
const app = await NestFactory.create<NestExpressApplication>(AppModule);
const config = app.get(ConfigService);
// Security headers (also removes X-Powered-By)
app.use(helmet());
// Behind a reverse proxy, trust its X-Forwarded-For so rate limiting sees the real client IP
const trustProxy = config.get<number>('TRUST_PROXY');
if (trustProxy) app.set('trust proxy', trustProxy);
// CORS is disabled unless origins are explicitly allowed
const origins = (config.get<string>('CORS_ORIGINS') ?? '')
.split(',')
.map((origin) => origin.trim())
.filter(Boolean);
if (origins.length > 0) app.enableCors({ origin: origins });
app.useGlobalPipes(new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true, transform: true }));
// Disconnect Prisma cleanly on SIGTERM/SIGINT
app.enableShutdownHooks();
const server = await app.listen(config.get<number>('PORT') ?? 3000);
// Drop slow clients holding sockets open (Slowloris): every endpoint answers fast, POST /sync runs in the background
server.headersTimeout = 15_000;
server.requestTimeout = 30_000;
}
void bootstrap();

View File

@@ -25,11 +25,7 @@ export class BrandHiboutikService {
(brand) => `Hiboutik brand ${brand.hiboutikId}`,
);
// Safety net: an empty answer from Hiboutik most likely means an API issue
if (hiboutikBrands.length === 0) {
this.logger.warn('Hiboutik returned no brands: deletion skipped.');
return;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.brandDatabaseRepository.deleteMissing(hiboutikBrands.map((dto) => Number(dto.brand_id)));
this.logger.log(
`Brands sync ended: ${hiboutikBrands.length - nbFailed}/${hiboutikBrands.length} synced, ${nbFailed} failed, ${nbDeleted} deleted.`,

View File

@@ -79,11 +79,19 @@ describe('CategoryHiboutikService', () => {
expect(categoryDatabaseRepository.deleteMissing).toHaveBeenCalledWith([1, 2]);
});
it('does not delete anything when Hiboutik returns no categories', async () => {
it('deletes every category when Hiboutik answers with an empty list', async () => {
categoryHiboutikRepository.getCategories.mockResolvedValue([]);
await service.syncCategories();
expect(categoryDatabaseRepository.deleteMissing).toHaveBeenCalledWith([]);
});
it('deletes nothing when the Hiboutik call fails', async () => {
categoryHiboutikRepository.getCategories.mockRejectedValue(new Error('Request failed with status code 500'));
await expect(service.syncCategories()).rejects.toThrow('500');
expect(categoryDatabaseRepository.deleteMissing).not.toHaveBeenCalled();
});
});

View File

@@ -37,11 +37,7 @@ export class CategoryHiboutikService {
(dto) => `parent of Hiboutik category ${dto.category_id}`,
);
// Safety net: an empty answer from Hiboutik most likely means an API issue
if (hiboutikCategories.length === 0) {
this.logger.warn('Hiboutik returned no categories: deletion skipped.');
return;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.categoryDatabaseRepository.deleteMissing(hiboutikCategories.map((dto) => Number(dto.category_id)));
this.logger.log(
`Categories sync ended: ${hiboutikCategories.length - nbFailed}/${hiboutikCategories.length} synced, ${nbFailed} failed, ` +

View File

@@ -48,9 +48,17 @@ describe('CustomerHiboutikService', () => {
expect(customerDatabaseRepository.upsert).toHaveBeenCalledTimes(2);
});
it('does not delete customers when Hiboutik returns none', async () => {
it('deletes every customer when Hiboutik answers with an empty list', async () => {
await service.syncCustomers();
expect(customerDatabaseRepository.deleteMissing).toHaveBeenCalledWith([]);
});
it('deletes nothing when the Hiboutik call fails', async () => {
customerHiboutikRepository.getCustomers.mockRejectedValue(new Error('Request failed with status code 500'));
await expect(service.syncCustomers()).rejects.toThrow('500');
expect(customerDatabaseRepository.deleteMissing).not.toHaveBeenCalled();
});

View File

@@ -37,13 +37,8 @@ export class CustomerHiboutikService {
async syncCustomers() {
const hiboutikCustomers = await this.customerHiboutikRepository.getCustomers();
// Safety net: an empty answer from Hiboutik most likely means an API issue
let nbDeleted = 0;
if (hiboutikCustomers.length === 0) {
this.logger.warn('Hiboutik returned no customers: deletion skipped.');
} else {
nbDeleted = await this.customerDatabaseRepository.deleteMissing(hiboutikCustomers.map((dto) => Number(dto.customers_id)));
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.customerDatabaseRepository.deleteMissing(hiboutikCustomers.map((dto) => Number(dto.customers_id)));
let nbFailed = 0;
let nbFailedAddresses = 0;

View File

@@ -7,7 +7,10 @@ describe('HiboutikService', () => {
const supplierService = { syncSuppliers: jest.fn() };
const productService = { syncProducts: jest.fn() };
const customerService = { syncCustomers: jest.fn() };
const syncLock = { tryAcquire: jest.fn() };
const release = jest.fn();
let service: HiboutikService;
const flush = () => new Promise((resolve) => setImmediate(resolve));
beforeAll(() => {
jest.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined);
@@ -22,12 +25,15 @@ describe('HiboutikService', () => {
supplierService.syncSuppliers.mockResolvedValue(undefined);
productService.syncProducts.mockResolvedValue(undefined);
customerService.syncCustomers.mockResolvedValue(undefined);
release.mockResolvedValue(undefined);
syncLock.tryAcquire.mockResolvedValue(release);
service = new HiboutikService(
brandService as never,
categoryService as never,
customerService as never,
productService as never,
supplierService as never,
syncLock as never,
);
});
@@ -56,13 +62,44 @@ 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<void>((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);
expect(service.startSync()).toBe(true);
expect(service.isSyncInProgress).toBe(true);
expect(service.startSync()).toBe(false);
await expect(service.startSync()).resolves.toBe(true);
await expect(service.startSync()).resolves.toBe(false);
expect(brandService.syncBrands).toHaveBeenCalledTimes(1);
expect(release).not.toHaveBeenCalled();
finishBrands();
await new Promise((resolve) => setImmediate(resolve));
expect(service.isSyncInProgress).toBe(false);
await flush();
expect(release).toHaveBeenCalledTimes(1);
});
it('starts nothing when another instance holds the lock', async () => {
syncLock.tryAcquire.mockResolvedValue(null);
await expect(service.startSync()).resolves.toBe(false);
expect(brandService.syncBrands).not.toHaveBeenCalled();
expect(release).not.toHaveBeenCalled();
});
it('starts nothing and can be retried when the lock cannot be requested', async () => {
syncLock.tryAcquire.mockRejectedValueOnce(new Error('database unreachable'));
await expect(service.startSync()).rejects.toThrow('database unreachable');
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 as never, 'syncAll').mockRejectedValue(new Error('crash'));
await expect(service.startSync()).resolves.toBe(true);
await flush();
expect(release).toHaveBeenCalledTimes(1);
});
});

View File

@@ -4,13 +4,12 @@ import { CategoryHiboutikService } from './category.hiboutik.service';
import { CustomerHiboutikService } from './customer.hiboutik.service';
import { SupplierHiboutikService } from './supplier.hiboutik.service';
import { ProductHiboutikService } from './product.hiboutik.service';
import { ReleaseLock, SyncLockDatabaseRepository } from '../../database/syncLock.database.repository';
/** Entry point of the Hiboutik → database synchronization: runs each entity sync in dependency order. */
@Injectable()
export class HiboutikService {
private readonly logger = new Logger('HiboutikService');
/** In-memory lock: only valid while the app runs as a single instance. */
private syncInProgress = false;
constructor(
private readonly brandHiboutikService: BrandHiboutikService,
@@ -18,28 +17,37 @@ export class HiboutikService {
private readonly customerHiboutikService: CustomerHiboutikService,
private readonly productHiboutikService: ProductHiboutikService,
private readonly supplierHiboutikService: SupplierHiboutikService,
private readonly syncLock: SyncLockDatabaseRepository,
) {}
get isSyncInProgress(): boolean {
return this.syncInProgress;
}
/**
* Starts a full synchronization in the background.
* 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).
*/
startSync(): boolean {
if (this.syncInProgress) return false;
async startSync(): Promise<boolean> {
const release = await this.syncLock.tryAcquire();
if (!release) return false;
this.syncInProgress = true;
void this.syncAll()
.catch((error) => this.logger.error('Global Hiboutik synchronization crashed', (error as Error)?.stack))
.finally(() => (this.syncInProgress = false));
void this.runLocked(release);
return true;
}
/** Runs the synchronization, then always releases the lock. */
private async runLocked(release: ReleaseLock): Promise<void> {
try {
await this.syncAll();
} catch (error) {
this.logger.error('Global Hiboutik synchronization crashed', (error as Error)?.stack);
} finally {
await release();
}
}
/** 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<void>][] = [

View File

@@ -89,12 +89,21 @@ describe('ProductHiboutikService', () => {
expect(calls[0]).toBe('deleteProducts');
});
it('does not delete anything when Hiboutik returns no products', async () => {
it('deletes every product when Hiboutik answers with an empty list', async () => {
productHiboutikRepository.getProducts.mockResolvedValue([]);
productDatabaseRepository.getAll.mockResolvedValue([{ id: 'uuid-1', hiboutikId: 1 }]);
await service.syncProducts();
expect(productDatabaseRepository.deleteAll).toHaveBeenCalledWith(['uuid-1']);
});
it('deletes nothing when the Hiboutik call fails', async () => {
productHiboutikRepository.getProducts.mockRejectedValue(new Error('Request failed with status code 500'));
productDatabaseRepository.getAll.mockResolvedValue([{ id: 'uuid-1', hiboutikId: 1 }]);
await expect(service.syncProducts()).rejects.toThrow('500');
expect(productDatabaseRepository.deleteAll).not.toHaveBeenCalled();
});

View File

@@ -27,7 +27,6 @@ export class ProductHiboutikService {
/** Mirrors Hiboutik products, their versions (sizes) and stocks. Products missing from Hiboutik are deleted. */
async syncProducts() {
this.logger.log('Starting products sync');
const [productsHiboutik, hiboutikStocks, existingProducts] = await Promise.all([
this.productHiboutikRepository.getProducts(),
this.stockHiboutikRepository.getStocks(),
@@ -56,12 +55,7 @@ export class ProductHiboutikService {
/** @returns the number of deleted products. */
private async deleteMissingProducts(productsHiboutik: ProductHiboutikDto[], existingProducts: Product[]): Promise<number> {
// Safety net: an empty answer from Hiboutik most likely means an API issue, not an empty catalog
if (productsHiboutik.length === 0 && existingProducts.length > 0) {
this.logger.warn(`Hiboutik returned no products while ${existingProducts.length} exist in database: deletion skipped.`);
return 0;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const hiboutikIds = new Set(productsHiboutik.map((product) => Number(product.product_id)));
const missingProductIds = existingProducts.filter((product) => !hiboutikIds.has(product.hiboutikId)).map((product) => product.id);
return this.productDatabaseRepository.deleteAll(missingProductIds);

View File

@@ -25,11 +25,7 @@ export class SupplierHiboutikService {
(supplier) => `Hiboutik supplier ${supplier.hiboutikId}`,
);
// Safety net: an empty answer from Hiboutik most likely means an API issue
if (hiboutikSuppliers.length === 0) {
this.logger.warn('Hiboutik returned no suppliers: deletion skipped.');
return;
}
// Hiboutik is trusted: a successful (2xx) answer, even an empty list, is the truth. An HTTP error throws before this point.
const nbDeleted = await this.supplierDatabaseRepository.deleteMissing(hiboutikSuppliers.map((dto) => Number(dto.supplier_id)));
this.logger.log(
`Suppliers sync ended: ${hiboutikSuppliers.length - nbFailed}/${hiboutikSuppliers.length} synced, ${nbFailed} failed, ${nbDeleted} deleted.`,

View File

@@ -2,6 +2,9 @@ import { Logger } from '@nestjs/common';
/**
* Runs `action` on each item, one at a time: a failing item is logged and skipped, the others are still processed.
* @param items
* @param action
* @param logger
* @param describe label of an item in the error log, e.g. "Hiboutik brand 12".
* @returns the number of failed items.
*/