From 38f6f6ab52f5035c60e047d5ee25d22fecb6ea56 Mon Sep 17 00:00:00 2001 From: z3n Date: Fri, 26 Jun 2026 12:00:13 +0000 Subject: [PATCH] =?UTF-8?q?back:=20validation=20d'env,=20transaction=20set?= =?UTF-8?q?Current,=20fix=20N+1=20cat=C3=A9gories,=20logging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ConfigModule.forRoot({ validate }) : schéma class-validator des variables d'env, l'app refuse de démarrer si une var manque/mal typée ; suppression des fallbacks de credentials DB (getOrThrow) - setCurrent() enveloppé dans une transaction (plus de fenêtre incohérente sur isCurrent entre les 2 updates) - browse() : compteurs de toutes les catégories en 2 requêtes + calcul en mémoire au lieu d'un N+1 récursif par enfant - console.* -> Logger Nest partout (apis, uploads, seeder, bootstrap) ; .catch(console.error) -> logger.error avec contexte Co-Authored-By: Claude Opus 4.8 --- back/src/app.module.ts | 13 ++-- back/src/config/env.validation.ts | 76 +++++++++++++++++++ back/src/main.ts | 4 +- back/src/modules/apis/apis.service.spec.ts | 8 +- back/src/modules/apis/apis.service.ts | 27 ++++--- .../modules/categories/categories.service.ts | 63 ++++++++++++--- back/src/modules/seeder/seeder.service.ts | 12 ++- .../src/modules/uploads/uploads.controller.ts | 4 +- 8 files changed, 171 insertions(+), 36 deletions(-) create mode 100644 back/src/config/env.validation.ts diff --git a/back/src/app.module.ts b/back/src/app.module.ts index 03c8aac..cf459a6 100644 --- a/back/src/app.module.ts +++ b/back/src/app.module.ts @@ -7,19 +7,20 @@ import { UploadsModule } from './modules/uploads/uploads.module'; import { DocsGenerationModule } from './modules/docs-generation/docs-generation.module'; import { CategoriesModule } from './modules/categories/categories.module'; import { SeederModule } from './modules/seeder/seeder.module'; +import { validateEnv } from './config/env.validation'; @Module({ imports: [ - ConfigModule.forRoot({ isGlobal: true }), + ConfigModule.forRoot({ isGlobal: true, validate: validateEnv }), TypeOrmModule.forRootAsync({ inject: [ConfigService], useFactory: (config: ConfigService) => ({ type: 'postgres', - host: config.get('DATABASE_HOST', 'localhost'), - port: config.get('DATABASE_PORT', 5432), - username: config.get('DATABASE_USER', 'datacat'), - password: config.get('DATABASE_PASSWORD', 'datacat'), - database: config.get('DATABASE_NAME', 'datacat'), + host: config.getOrThrow('DATABASE_HOST'), + port: config.getOrThrow('DATABASE_PORT'), + username: config.getOrThrow('DATABASE_USER'), + password: config.getOrThrow('DATABASE_PASSWORD'), + database: config.getOrThrow('DATABASE_NAME'), entities: [__dirname + '/**/*.entity{.ts,.js}'], synchronize: config.get('NODE_ENV') !== 'production', logging: config.get('NODE_ENV') === 'development', diff --git a/back/src/config/env.validation.ts b/back/src/config/env.validation.ts new file mode 100644 index 0000000..a7f9bc2 --- /dev/null +++ b/back/src/config/env.validation.ts @@ -0,0 +1,76 @@ +import { plainToInstance, Type } from 'class-transformer'; +import { + IsEnum, + IsInt, + IsNotEmpty, + IsOptional, + IsString, + Max, + Min, + validateSync, +} from 'class-validator'; + +export enum NodeEnv { + Development = 'development', + Production = 'production', + Test = 'test', +} + +/** + * Schéma des variables d'environnement. Validé au démarrage via `validateEnv` + * (branché sur `ConfigModule.forRoot({ validate })`) : l'app refuse de démarrer + * si une variable requise manque ou est mal typée, plutôt que d'utiliser + * silencieusement des valeurs par défaut (ex. credentials DB). + */ +export class EnvironmentVariables { + @IsEnum(NodeEnv) + @IsOptional() + NODE_ENV: NodeEnv = NodeEnv.Development; + + @IsString() + @IsNotEmpty() + DATABASE_HOST!: string; + + @Type(() => Number) + @IsInt() + @Min(1) + @Max(65535) + DATABASE_PORT!: number; + + @IsString() + @IsNotEmpty() + DATABASE_USER!: string; + + @IsString() + @IsNotEmpty() + DATABASE_PASSWORD!: string; + + @IsString() + @IsNotEmpty() + DATABASE_NAME!: string; + + @Type(() => Number) + @IsInt() + @Min(1) + @Max(65535) + @IsOptional() + PORT = 3000; + + @IsString() + @IsOptional() + DOCS_OUTPUT_DIR?: string; +} + +export function validateEnv(config: Record): EnvironmentVariables { + const validated = plainToInstance(EnvironmentVariables, config, { + enableImplicitConversion: true, + }); + const errors = validateSync(validated, { skipMissingProperties: false }); + if (errors.length > 0) { + const details = errors + .map((e) => Object.values(e.constraints ?? {}).join(', ')) + .join('\n - '); + throw new Error(`Configuration d'environnement invalide:\n - ${details}`); + } + return validated; +} diff --git a/back/src/main.ts b/back/src/main.ts index 405b19b..10c3d08 100644 --- a/back/src/main.ts +++ b/back/src/main.ts @@ -1,5 +1,5 @@ import { NestFactory } from '@nestjs/core'; -import { ValidationPipe, RequestMethod } from '@nestjs/common'; +import { Logger, ValidationPipe, RequestMethod } from '@nestjs/common'; import { AppModule } from './app.module'; import { AllExceptionsFilter } from './common/filters/all-exceptions.filter'; @@ -16,7 +16,7 @@ async function bootstrap() { const port = process.env.PORT ?? 3000; await app.listen(port); - console.log(`Datacat API running on port ${port}`); + Logger.log(`Datacat API running on port ${port}`, 'Bootstrap'); } bootstrap(); diff --git a/back/src/modules/apis/apis.service.spec.ts b/back/src/modules/apis/apis.service.spec.ts index 53d93df..ca5192f 100644 --- a/back/src/modules/apis/apis.service.spec.ts +++ b/back/src/modules/apis/apis.service.spec.ts @@ -170,11 +170,17 @@ const mockRepo = { const mockDocsService = { generate: vi.fn() }; +const mockDataSource = { + transaction: vi.fn(async (cb: (m: unknown) => unknown) => + cb({ getRepository: () => mockRepo }), + ), +}; + let service: ApisService; beforeEach(() => { vi.clearAllMocks(); - service = new ApisService(mockRepo as any, mockDocsService as any); + service = new ApisService(mockRepo as any, mockDataSource as any, mockDocsService as any); }); function makeEntity(overrides: Record = {}) { diff --git a/back/src/modules/apis/apis.service.ts b/back/src/modules/apis/apis.service.ts index a988027..4b47842 100644 --- a/back/src/modules/apis/apis.service.ts +++ b/back/src/modules/apis/apis.service.ts @@ -1,6 +1,6 @@ -import { Injectable, NotFoundException, ConflictException, Inject, forwardRef } from '@nestjs/common'; +import { Injectable, NotFoundException, ConflictException, Inject, forwardRef, Logger } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; +import { DataSource, Repository } from 'typeorm'; import { ApiEntryEntity } from './api-entry.entity'; import { CreateApiEntryDto } from './dto/create-api-entry.dto'; import { UpdateApiEntryDto } from './dto/update-api-entry.dto'; @@ -17,9 +17,12 @@ interface InternalUpdateFields { @Injectable() export class ApisService { + private readonly logger = new Logger(ApisService.name); + constructor( @InjectRepository(ApiEntryEntity) private readonly repo: Repository, + private readonly dataSource: DataSource, @Inject(forwardRef(() => DocsGenerationService)) private readonly docsService: DocsGenerationService, ) {} @@ -213,20 +216,24 @@ export class ApisService { async setCurrent(id: string): Promise { const entry = await this.findOneOrThrow(id); - // Mettre tous les siblings à false - await this.repo.update( - { name: entry.name, convention: entry.convention }, - { isCurrent: false }, - ); - // Mettre cet entry à true - await this.repo.update({ id }, { isCurrent: true }); + // Transaction : éviter une fenêtre où aucun (ou deux) sibling n'est courant. + await this.dataSource.transaction(async (manager) => { + const repo = manager.getRepository(ApiEntryEntity); + await repo.update( + { name: entry.name, convention: entry.convention }, + { isCurrent: false }, + ); + await repo.update({ id }, { isCurrent: true }); + }); return this.findOneOrThrow(id); } async regenerate(id: string): Promise { await this.findOneOrThrow(id); await this.updateInternal(id, { status: 'PENDING', errorMessage: null }); - this.docsService.generate(id).catch(console.error); + this.docsService + .generate(id) + .catch((err) => this.logger.error(`Génération docs échouée pour ${id}`, err)); return this.findOneOrThrow(id); } } diff --git a/back/src/modules/categories/categories.service.ts b/back/src/modules/categories/categories.service.ts index 6656069..94ff499 100644 --- a/back/src/modules/categories/categories.service.ts +++ b/back/src/modules/categories/categories.service.ts @@ -98,11 +98,51 @@ export class CategoriesService { await this.categoryRepo.delete(id); } - private async countApisRecursively(categoryId: string): Promise { - const direct = await this.apiRepo.count({ where: { categoryId } }); - const children = await this.categoryRepo.find({ where: { parentId: categoryId }, select: ['id'] }); - const childCounts = await Promise.all(children.map((c) => this.countApisRecursively(c.id))); - return direct + childCounts.reduce((sum, n) => sum + n, 0); + /** + * Comptages pour TOUTES les catégories en 2 requêtes (au lieu d'un N+1 + * récursif par enfant) : la hiérarchie et les APIs directes sont chargées en + * une fois, puis les compteurs récursifs sont calculés en mémoire. + */ + private async computeCategoryCounts(): Promise<{ + subcategoryCount: Map; + recursiveApiCount: Map; + }> { + const categories = await this.categoryRepo.find({ select: ['id', 'parentId'] }); + const directRows = await this.apiRepo + .createQueryBuilder('api') + .select('api.categoryId', 'categoryId') + .addSelect('COUNT(*)', 'count') + .where('api.categoryId IS NOT NULL') + .groupBy('api.categoryId') + .getRawMany<{ categoryId: string; count: string }>(); + + const directApiCount = new Map(directRows.map((r) => [r.categoryId, Number(r.count)])); + const childrenOf = new Map(); + for (const c of categories) { + if (c.parentId) { + const siblings = childrenOf.get(c.parentId) ?? []; + siblings.push(c.id); + childrenOf.set(c.parentId, siblings); + } + } + + const recursiveApiCount = new Map(); + const computeRecursive = (id: string): number => { + const cached = recursiveApiCount.get(id); + if (cached !== undefined) return cached; + let total = directApiCount.get(id) ?? 0; + for (const childId of childrenOf.get(id) ?? []) total += computeRecursive(childId); + recursiveApiCount.set(id, total); + return total; + }; + + const subcategoryCount = new Map(); + for (const c of categories) { + subcategoryCount.set(c.id, (childrenOf.get(c.id) ?? []).length); + computeRecursive(c.id); + } + + return { subcategoryCount, recursiveApiCount }; } async browse(categoryId: string | null): Promise { @@ -132,13 +172,12 @@ export class CategoriesService { order: { orderIndex: 'ASC', name: 'ASC' }, }); - const subcategories: CategoryWithCounts[] = await Promise.all( - directChildren.map(async (child) => { - const subcategoryCount = await this.categoryRepo.count({ where: { parentId: child.id } }); - const apiCount = await this.countApisRecursively(child.id); - return { ...toCategory(child), subcategoryCount, apiCount }; - }), - ); + const { subcategoryCount, recursiveApiCount } = await this.computeCategoryCounts(); + const subcategories: CategoryWithCounts[] = directChildren.map((child) => ({ + ...toCategory(child), + subcategoryCount: subcategoryCount.get(child.id) ?? 0, + apiCount: recursiveApiCount.get(child.id) ?? 0, + })); /* 4. APIs directement dans cette catégorie */ const apiEntities = await this.apiRepo.find({ diff --git a/back/src/modules/seeder/seeder.service.ts b/back/src/modules/seeder/seeder.service.ts index 5e09e04..67a37a5 100644 --- a/back/src/modules/seeder/seeder.service.ts +++ b/back/src/modules/seeder/seeder.service.ts @@ -1,4 +1,4 @@ -import { Injectable, OnApplicationBootstrap } from '@nestjs/common'; +import { Injectable, Logger, OnApplicationBootstrap } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository, DataSource } from 'typeorm'; import * as fs from 'fs/promises'; @@ -587,6 +587,8 @@ components: @Injectable() export class SeederService implements OnApplicationBootstrap { + private readonly logger = new Logger(SeederService.name); + constructor( @InjectRepository(ApiEntryEntity) private readonly apiRepo: Repository, @@ -648,7 +650,7 @@ export class SeederService implements OnApplicationBootstrap { const docsDir = process.env.DOCS_OUTPUT_DIR ?? '/app/docs-output'; await fs.rm(docsDir, { recursive: true, force: true }); await fs.mkdir(docsDir, { recursive: true }); - console.log('[Seeder] Reset complet — données supprimées, docs nettoyées.'); + this.logger.log('Reset complet — données supprimées, docs nettoyées.'); } private async seed(): Promise { @@ -864,11 +866,13 @@ export class SeederService implements OnApplicationBootstrap { }), ]); - console.log('[Seeder] 8 catégories et 7 APIs de démo créées — génération docs en cours...'); + this.logger.log('8 catégories et 7 APIs de démo créées — génération docs en cours...'); /* Déclenche la génération de docs en fire-and-forget pour chaque API */ for (const entry of entries) { - this.docsService.generate(entry.id).catch(console.error); + this.docsService + .generate(entry.id) + .catch((err) => this.logger.error(`Génération docs échouée pour ${entry.id}`, err)); } } } diff --git a/back/src/modules/uploads/uploads.controller.ts b/back/src/modules/uploads/uploads.controller.ts index 23fa8ef..d3aca50 100644 --- a/back/src/modules/uploads/uploads.controller.ts +++ b/back/src/modules/uploads/uploads.controller.ts @@ -253,7 +253,9 @@ export class UploadsController { /* Fire-and-forget : génération asynchrone (seulement si YAML fourni) */ if (yamlContent) { - this.docsService.generate(entry.id).catch(console.error); + this.docsService + .generate(entry.id) + .catch((err) => this.logger.error(`Génération docs échouée pour ${entry.id}`, err)); } return entry;