From 99c77eb7b64b06c79c1e38e50eb77f6743003db1 Mon Sep 17 00:00:00 2001 From: matthieu Date: Wed, 22 Apr 2026 16:10:03 +0200 Subject: [PATCH] fix(audit-log) : applique fixes code review (precision timestamp, ESCAPE LIKE, pagination max) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - TIMESTAMP(6) WITH TIME ZONE + tie-breaker id DESC sur l'ORDER BY pour garantir un tri deterministe quand plusieurs lignes partagent la meme timestamp (batch fixture, bulk flush < 1µs). - Suppression de la clause ESCAPE '\\' redondante (`\` est deja l'echappement LIKE par defaut en PostgreSQL) et fragile sur standard_conforming_strings. Le str_replace des wildcards reste. - paginationMaximumItemsPerPage : 100 -> 50. Reduit le pire cas de reponse lourde sur un endpoint admin (changes JSONB volumineux). --- migrations/Version20260420202749.php | 2 +- .../ApiPlatform/Resource/AuditLogResource.php | 2 +- .../ApiPlatform/State/Provider/AuditLogProvider.php | 10 +++++++--- tests/Module/Core/Api/AuditLogApiTest.php | 10 ++++++---- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/migrations/Version20260420202749.php b/migrations/Version20260420202749.php index abff7c2..615dbd6 100644 --- a/migrations/Version20260420202749.php +++ b/migrations/Version20260420202749.php @@ -39,7 +39,7 @@ final class Version20260420202749 extends AbstractMigration action VARCHAR(10) NOT NULL, changes JSONB NOT NULL DEFAULT '{}'::jsonb, performed_by VARCHAR(100) NOT NULL, - performed_at TIMESTAMP(0) WITH TIME ZONE NOT NULL, + performed_at TIMESTAMP(6) WITH TIME ZONE NOT NULL, ip_address VARCHAR(45) DEFAULT NULL, request_id VARCHAR(36) DEFAULT NULL, PRIMARY KEY(id) diff --git a/src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogResource.php b/src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogResource.php index 1f428b9..23361d3 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogResource.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/Resource/AuditLogResource.php @@ -40,7 +40,7 @@ use App\Module\Core\Infrastructure\ApiPlatform\State\Provider\AuditLogProvider; uriTemplate: '/audit-logs', paginationItemsPerPage: 30, paginationClientItemsPerPage: true, - paginationMaximumItemsPerPage: 100, + paginationMaximumItemsPerPage: 50, security: "is_granted('core.audit_log.view')", provider: AuditLogProvider::class, ), diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php index f7d6137..e5fd16c 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Provider/AuditLogProvider.php @@ -75,6 +75,10 @@ final readonly class AuditLogProvider implements ProviderInterface $dataQuery = $this->buildBaseQuery() ->select('id', 'entity_type', 'entity_id', 'action', 'changes', 'performed_by', 'performed_at', 'ip_address', 'request_id') ->orderBy('performed_at', 'DESC') + // Tie-breaker sur `id` (UUID v7 monotone) : garantit un tri + // totalement deterministe quand plusieurs lignes partagent la + // meme timestamp (ex: batch fixture, bulk flush < 1µs). + ->addOrderBy('id', 'DESC') ->setFirstResult($offset) ->setMaxResults($itemsPerPage) ; @@ -168,10 +172,10 @@ final readonly class AuditLogProvider implements ProviderInterface // Recherche contains insensible a la casse pour matcher "adm" → "admin". // On echappe `%`, `_` et `\` saisis par l'utilisateur pour qu'ils soient // interpretes comme caracteres litteraux (sinon `%` matche tout, `_` - // matche n'importe quel caractere). La clause `ESCAPE '\\'` indique - // a PostgreSQL le caractere d'echappement utilise dans le motif. + // matche n'importe quel caractere). Pas de clause ESCAPE : `\` est + // deja le caractere d'echappement LIKE par defaut en PostgreSQL. $escaped = str_replace(['\\', '%', '_'], ['\\\\', '\%', '\_'], $filters['performed_by']); - $qb->andWhere("performed_by ILIKE :performed_by ESCAPE '\\'") + $qb->andWhere('performed_by ILIKE :performed_by') ->setParameter('performed_by', '%'.$escaped.'%') ; } diff --git a/tests/Module/Core/Api/AuditLogApiTest.php b/tests/Module/Core/Api/AuditLogApiTest.php index 32e66b5..a0e82f7 100644 --- a/tests/Module/Core/Api/AuditLogApiTest.php +++ b/tests/Module/Core/Api/AuditLogApiTest.php @@ -260,8 +260,9 @@ final class AuditLogApiTest extends AbstractApiTestCase /** * Les caracteres wildcard PostgreSQL (`%`, `_`) saisis par l'utilisateur * doivent etre echappes et traites comme caracteres litteraux, pas comme - * des metacaracteres LIKE. Idem pour le backslash qui doit etre double - * pour ne pas interferer avec la clause ESCAPE. + * des metacaracteres LIKE. `\` est le caractere d'echappement LIKE par + * defaut en PostgreSQL (pas de clause ESCAPE explicite) : on le double + * dans le motif pour qu'il soit aussi traite comme litteral. */ public function testFilterByPerformedByEscapesWildcards(): void { @@ -281,8 +282,9 @@ final class AuditLogApiTest extends AbstractApiTestCase $ours = array_filter($data['member'], fn (array $m) => ($m['requestId'] ?? null) === $this->runTag); self::assertCount(0, $ours, '_ doit etre traite comme literal, pas wildcard single-char.'); - // `\` (backslash) dans le motif ne doit pas casser la clause ESCAPE : - // on attend une reponse 200 (pas 500), meme si le resultat est vide. + // `\` (backslash, echappement LIKE par defaut en PG) : un seul `\` + // dans l'entree utilisateur est double en `\\` et doit etre interprete + // comme litteral. On attend une reponse 200 (pas 500), resultat vide. $response = $client->request('GET', '/api/audit-logs?performed_by=%5C&entity_id=999'); self::assertSame(200, $response->getStatusCode(), 'Un backslash dans le filtre ne doit pas produire de 500.'); }