fix(audit-log) : applique fixes code review (precision timestamp, ESCAPE LIKE, pagination max)
- 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).
This commit is contained in:
@@ -39,7 +39,7 @@ final class Version20260420202749 extends AbstractMigration
|
|||||||
action VARCHAR(10) NOT NULL,
|
action VARCHAR(10) NOT NULL,
|
||||||
changes JSONB NOT NULL DEFAULT '{}'::jsonb,
|
changes JSONB NOT NULL DEFAULT '{}'::jsonb,
|
||||||
performed_by VARCHAR(100) NOT NULL,
|
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,
|
ip_address VARCHAR(45) DEFAULT NULL,
|
||||||
request_id VARCHAR(36) DEFAULT NULL,
|
request_id VARCHAR(36) DEFAULT NULL,
|
||||||
PRIMARY KEY(id)
|
PRIMARY KEY(id)
|
||||||
|
|||||||
@@ -40,7 +40,7 @@ use App\Module\Core\Infrastructure\ApiPlatform\State\Provider\AuditLogProvider;
|
|||||||
uriTemplate: '/audit-logs',
|
uriTemplate: '/audit-logs',
|
||||||
paginationItemsPerPage: 30,
|
paginationItemsPerPage: 30,
|
||||||
paginationClientItemsPerPage: true,
|
paginationClientItemsPerPage: true,
|
||||||
paginationMaximumItemsPerPage: 100,
|
paginationMaximumItemsPerPage: 50,
|
||||||
security: "is_granted('core.audit_log.view')",
|
security: "is_granted('core.audit_log.view')",
|
||||||
provider: AuditLogProvider::class,
|
provider: AuditLogProvider::class,
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -75,6 +75,10 @@ final readonly class AuditLogProvider implements ProviderInterface
|
|||||||
$dataQuery = $this->buildBaseQuery()
|
$dataQuery = $this->buildBaseQuery()
|
||||||
->select('id', 'entity_type', 'entity_id', 'action', 'changes', 'performed_by', 'performed_at', 'ip_address', 'request_id')
|
->select('id', 'entity_type', 'entity_id', 'action', 'changes', 'performed_by', 'performed_at', 'ip_address', 'request_id')
|
||||||
->orderBy('performed_at', 'DESC')
|
->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)
|
->setFirstResult($offset)
|
||||||
->setMaxResults($itemsPerPage)
|
->setMaxResults($itemsPerPage)
|
||||||
;
|
;
|
||||||
@@ -168,10 +172,10 @@ final readonly class AuditLogProvider implements ProviderInterface
|
|||||||
// Recherche contains insensible a la casse pour matcher "adm" → "admin".
|
// Recherche contains insensible a la casse pour matcher "adm" → "admin".
|
||||||
// On echappe `%`, `_` et `\` saisis par l'utilisateur pour qu'ils soient
|
// On echappe `%`, `_` et `\` saisis par l'utilisateur pour qu'ils soient
|
||||||
// interpretes comme caracteres litteraux (sinon `%` matche tout, `_`
|
// interpretes comme caracteres litteraux (sinon `%` matche tout, `_`
|
||||||
// matche n'importe quel caractere). La clause `ESCAPE '\\'` indique
|
// matche n'importe quel caractere). Pas de clause ESCAPE : `\` est
|
||||||
// a PostgreSQL le caractere d'echappement utilise dans le motif.
|
// deja le caractere d'echappement LIKE par defaut en PostgreSQL.
|
||||||
$escaped = str_replace(['\\', '%', '_'], ['\\\\', '\%', '\_'], $filters['performed_by']);
|
$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.'%')
|
->setParameter('performed_by', '%'.$escaped.'%')
|
||||||
;
|
;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -260,8 +260,9 @@ final class AuditLogApiTest extends AbstractApiTestCase
|
|||||||
/**
|
/**
|
||||||
* Les caracteres wildcard PostgreSQL (`%`, `_`) saisis par l'utilisateur
|
* Les caracteres wildcard PostgreSQL (`%`, `_`) saisis par l'utilisateur
|
||||||
* doivent etre echappes et traites comme caracteres litteraux, pas comme
|
* doivent etre echappes et traites comme caracteres litteraux, pas comme
|
||||||
* des metacaracteres LIKE. Idem pour le backslash qui doit etre double
|
* des metacaracteres LIKE. `\` est le caractere d'echappement LIKE par
|
||||||
* pour ne pas interferer avec la clause ESCAPE.
|
* 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
|
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);
|
$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.');
|
self::assertCount(0, $ours, '_ doit etre traite comme literal, pas wildcard single-char.');
|
||||||
|
|
||||||
// `\` (backslash) dans le motif ne doit pas casser la clause ESCAPE :
|
// `\` (backslash, echappement LIKE par defaut en PG) : un seul `\`
|
||||||
// on attend une reponse 200 (pas 500), meme si le resultat est vide.
|
// 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');
|
$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.');
|
self::assertSame(200, $response->getStatusCode(), 'Un backslash dans le filtre ne doit pas produire de 500.');
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user