diff --git a/.claude/rules/backend.md b/.claude/rules/backend.md index e21a688..4079df4 100644 --- a/.claude/rules/backend.md +++ b/.claude/rules/backend.md @@ -13,6 +13,64 @@ - Le login `/login_check` est **hors** prefix `/api` (nginx reecrit `REQUEST_URI` vers `/login_check`) - **Exception** : si tu dois creer un controller custom sous `/api/`, mettre `priority: 1` sur `#[Route]` pour eviter le conflit avec API Platform `{id}` +## Pagination (obligatoire) + +**Regle** : toute collection API DOIT etre paginee. Aucun retour de collection complete cote serveur. + +### Standard global + +Pose dans `config/packages/api_platform.yaml` (section `defaults:`) et heritee par toutes les ressources : + +| Cle | Valeur | Effet | +|---|---|---| +| `pagination_enabled` | `true` | Pagination Hydra active par defaut. | +| `pagination_items_per_page` | `10` | Taille de page par defaut, aligne sur l'UI `MalioDataTable`. | +| `pagination_maximum_items_per_page` | `50` | Borne dure : `?itemsPerPage=999` โ†’ ramene a 50. Anti deep-fetch. | +| `pagination_client_items_per_page` | `true` | Le client peut envoyer `?itemsPerPage=25` (bornee par le max). | +| `pagination_client_enabled` | `true` | Le client peut envoyer `?pagination=false` pour TOUT recuperer (echappatoire selects). | + +### Override par ressource (rare) + +Si une ressource a besoin d'un autre defaut (ex: payload lourd), utiliser les attributs sur l'operation. **JAMAIS `paginationEnabled: false`** sans whitelist explicite dans `tests/Architecture/CollectionsArePaginatedTest::EXCLUDED`. + +```php +new GetCollection( + paginationItemsPerPage: 5, // override taille par defaut + paginationMaximumItemsPerPage: 20, // override borne max +) +``` + +### Selects et autocompletions + +Pour alimenter un ` ou autre + # vue "tout-en-un" โ€” c'est l'echappatoire prevue pour les ressources + # servant a la fois de datatable et de source de select (Role, + # Permission, Site, CategoryType). Override par ressource possible via + # `paginationItemsPerPage` / `paginationMaximumItemsPerPage` / + # `paginationEnabled` sur l'attribut #[ApiResource] ou sur une operation. + pagination_enabled: true + pagination_items_per_page: 10 + pagination_maximum_items_per_page: 50 + pagination_client_items_per_page: true + pagination_client_enabled: true diff --git a/src/Module/Catalog/Infrastructure/ApiPlatform/State/Provider/CategoryProvider.php b/src/Module/Catalog/Infrastructure/ApiPlatform/State/Provider/CategoryProvider.php index 97a351b..bc7f4c7 100644 --- a/src/Module/Catalog/Infrastructure/ApiPlatform/State/Provider/CategoryProvider.php +++ b/src/Module/Catalog/Infrastructure/ApiPlatform/State/Provider/CategoryProvider.php @@ -4,11 +4,14 @@ declare(strict_types=1); namespace App\Module\Catalog\Infrastructure\ApiPlatform\State\Provider; +use ApiPlatform\Doctrine\Orm\Paginator; use ApiPlatform\Metadata\CollectionOperationInterface; use ApiPlatform\Metadata\Operation; +use ApiPlatform\State\Pagination\Pagination; use ApiPlatform\State\ProviderInterface; use App\Module\Catalog\Domain\Entity\Category; use App\Module\Catalog\Domain\Repository\CategoryRepositoryInterface; +use Doctrine\ORM\Tools\Pagination\Paginator as DoctrinePaginator; use Symfony\Component\DependencyInjection\Attribute\Autowire; /** @@ -29,18 +32,32 @@ final class CategoryProvider implements ProviderInterface public function __construct( #[Autowire(service: 'App\Module\Catalog\Infrastructure\Doctrine\DoctrineCategoryRepository')] private readonly CategoryRepositoryInterface $repository, + private readonly Pagination $pagination, ) {} - public function provide(Operation $operation, array $uriVariables = [], array $context = []): Category|iterable|null + public function provide(Operation $operation, array $uriVariables = [], array $context = []): Category|iterable|Paginator|null { $includeDeleted = $this->readIncludeDeleted($context); if ($operation instanceof CollectionOperationInterface) { - return $this->repository - ->createListQueryBuilder($includeDeleted) - ->getQuery() - ->getResult() - ; + $qb = $this->repository->createListQueryBuilder($includeDeleted); + + // Echappatoire ?pagination=false : retourne la collection complete sans Paginator. + // Utile pour les drawers Role/Permission/Site/CategoryType qui alimentent un alimente par l'audit). Le flag global + // `pagination_client_enabled: true` reste donc volontairement inerte ici. + // // `page` brut peut etre <= 0 (parametre client) โ†’ OFFSET negatif โ†’ 500 PG // (`SQLSTATE[22023] OFFSET must not be negative`). API Platform clampe // `itemsPerPage` au max de la resource mais pas `page` ; on impose un diff --git a/tests/Architecture/CollectionsArePaginatedTest.php b/tests/Architecture/CollectionsArePaginatedTest.php new file mode 100644 index 0000000..54c21a7 --- /dev/null +++ b/tests/Architecture/CollectionsArePaginatedTest.php @@ -0,0 +1,126 @@ + 'justification + reference ticket/spec'` + * + * @internal + */ +final class CollectionsArePaginatedTest extends TestCase +{ + /** + * Resources API Platform dont un `GetCollection` peut desactiver la pagination. + * + * Laisser vide au demarrage. Pour ajouter une exception : + * 'App\Module\Foo\Infrastructure\ApiPlatform\Resource\BarResource' + * => 'Referentiel statique < 50 items (types de contrat). Cf. ERP-XX.', + * + * @var array + */ + private const EXCLUDED = []; + + public function testAllGetCollectionOperationsHavePaginationEnabled(): void + { + $finder = new Finder() + ->files() + ->in(__DIR__.'/../../src') + ->name('*.php') + ->contains('#[ApiResource') + ; + + // Garde : si le scan ne trouve rien, le chemin est casse โ€” le test + // deviendrait un faux positif vert. On verifie qu'il a du grain a moudre. + self::assertNotEmpty( + iterator_to_array($finder), + 'Aucun fichier #[ApiResource] trouve sous src/ : chemin invalide ou codebase vide.', + ); + + foreach ($finder as $file) { + $fqcn = $this->extractFqcn($file->getRealPath()); + if (null === $fqcn || !class_exists($fqcn)) { + continue; + } + + $reflection = new ReflectionClass($fqcn); + $apiResourceAttributes = $reflection->getAttributes(ApiResource::class); + + if ([] === $apiResourceAttributes) { + continue; + } + + foreach ($apiResourceAttributes as $attribute) { + /** @var ApiResource $apiResource */ + $apiResource = $attribute->newInstance(); + $operations = $apiResource->getOperations()?->getIterator() ?? []; + + foreach ($operations as $operation) { + if (!$operation instanceof GetCollection) { + continue; + } + + if (false !== $operation->getPaginationEnabled()) { + continue; + } + + // La pagination est explicitement desactivee : verifier + // que la resource est dans la whitelist EXCLUDED. + self::assertArrayHasKey( + $fqcn, + self::EXCLUDED, + sprintf( + "La resource %s desactive la pagination sur une operation GetCollection.\n" + ."Regle : toute collection API Platform doit etre paginee (cf. .claude/rules/backend.md).\n" + ."Si cette collection est structurellement bornee et que la desactivation est justifiee,\n" + .'ajouter une entree dans CollectionsArePaginatedTest::EXCLUDED avec une justification.', + $fqcn, + ), + ); + } + } + } + } + + /** + * Extrait le FQCN (namespace + classe) d'un fichier PHP par lecture du + * source, sans charger le fichier. + */ + private function extractFqcn(string $path): ?string + { + $source = file_get_contents($path); + if (false === $source) { + return null; + } + + if ( + 1 !== preg_match('/^namespace\s+([^;]+);/m', $source, $nsMatch) + || 1 !== preg_match('/^(?:final\s+|abstract\s+|readonly\s+)*class\s+(\w+)/m', $source, $classMatch) + ) { + return null; + } + + return trim($nsMatch[1]).'\\'.$classMatch[1]; + } +} diff --git a/tests/Module/Catalog/Api/CategoryListTest.php b/tests/Module/Catalog/Api/CategoryListTest.php index 0074e51..c1dfa41 100644 --- a/tests/Module/Catalog/Api/CategoryListTest.php +++ b/tests/Module/Catalog/Api/CategoryListTest.php @@ -29,7 +29,7 @@ final class CategoryListTest extends AbstractCatalogApiTestCase ); $client = $this->createAdminClient(); - $response = $client->request('GET', '/api/categories'); + $response = $client->request('GET', '/api/categories?pagination=false'); self::assertSame(200, $response->getStatusCode()); $data = $response->toArray(); @@ -62,7 +62,7 @@ final class CategoryListTest extends AbstractCatalogApiTestCase ); $client = $this->createAdminClient(); - $response = $client->request('GET', '/api/categories?includeDeleted=true'); + $response = $client->request('GET', '/api/categories?includeDeleted=true&pagination=false'); self::assertSame(200, $response->getStatusCode()); $names = array_values(array_filter( @@ -87,7 +87,7 @@ final class CategoryListTest extends AbstractCatalogApiTestCase $this->createCategory(self::TEST_CATEGORY_PREFIX.'mid', $type); $client = $this->createAdminClient(); - $response = $client->request('GET', '/api/categories'); + $response = $client->request('GET', '/api/categories?pagination=false'); self::assertSame(200, $response->getStatusCode()); $names = array_values(array_filter( diff --git a/tests/Module/Catalog/Api/CategoryPaginationTest.php b/tests/Module/Catalog/Api/CategoryPaginationTest.php new file mode 100644 index 0000000..b0e418b --- /dev/null +++ b/tests/Module/Catalog/Api/CategoryPaginationTest.php @@ -0,0 +1,171 @@ +createCategoryType(); + for ($i = 1; $i <= 12; ++$i) { + $this->createCategory(self::TEST_CATEGORY_PREFIX.'meta_'.$i, $type); + } + + $client = $this->createAdminClient(); + $response = $client->request('GET', '/api/categories'); + + self::assertSame(200, $response->getStatusCode()); + + $data = $response->toArray(); + self::assertArrayHasKey('totalItems', $data, 'La collection doit exposer totalItems.'); + self::assertArrayHasKey('view', $data, 'La collection doit exposer view (pagination) quand totalItems > itemsPerPage.'); + self::assertIsArray($data['member'], 'member doit etre un tableau.'); + } + + /** + * Un itemsPerPage arbitrairement grand (99999) doit etre plafonne au + * maximum global configure (50). On cree 12 categories pour etre certain + * de disposer de donnees ; le cap doit s'appliquer quelle que soit la taille + * reelle de la collection. + */ + public function testItemsPerPageIsCappedAtMaximum(): void + { + $type = $this->createCategoryType(); + for ($i = 1; $i <= 12; ++$i) { + $this->createCategory(self::TEST_CATEGORY_PREFIX.'cap_'.$i, $type); + } + + $client = $this->createAdminClient(); + $response = $client->request('GET', '/api/categories?itemsPerPage=99999'); + + self::assertSame(200, $response->getStatusCode()); + // Le cap global est 50 : jamais plus d'items par page que le maximum. + self::assertLessThanOrEqual( + 50, + count($response->toArray()['member']), + 'itemsPerPage doit etre plafonne au maximum global (50).', + ); + } + + /** + * Une page tres elevee (99999) sur une petite collection ne doit pas + * produire une 500 PG (OFFSET negatif ou depassement de capacite) mais + * retourner 200 avec un tableau member vide. + */ + public function testOutOfBoundPageReturnsEmptyCollectionNot500(): void + { + $type = $this->createCategoryType(); + $this->createCategory(self::TEST_CATEGORY_PREFIX.'oob', $type); + + $client = $this->createAdminClient(); + $response = $client->request('GET', '/api/categories?page=99999'); + + self::assertSame(200, $response->getStatusCode()); + // La page 99999 est forcement vide (on a bien moins que 99999*10 items). + self::assertSame( + [], + $response->toArray()['member'], + 'Une page hors limites doit retourner un member vide, jamais une 500.', + ); + } + + /** + * ?pagination=false permet au frontend de desactiver la pagination pour + * alimenter un select-box. On cree exactement 12 categories dont les noms + * commencent par `test_cat_select_` : le filtre sur ce prefixe isole nos + * entrees des donnees concurrentes et prouve que les 12 items sont tous + * retournes (et pas seulement les 10 premiers de la page 1). + */ + public function testClientCanDisablePaginationToFeedASelect(): void + { + $type = $this->createCategoryType(); + for ($i = 1; $i <= 12; ++$i) { + $this->createCategory(self::TEST_CATEGORY_PREFIX.'select_'.$i, $type); + } + + $client = $this->createAdminClient(); + $response = $client->request('GET', '/api/categories?pagination=false'); + + self::assertSame(200, $response->getStatusCode()); + + $members = $response->toArray()['member']; + + // Filtre sur le sous-prefixe pour ne pas comptabiliser les categories + // d'autres tests qui partagent la meme base de donnees. + $selectItems = array_values(array_filter( + $members, + fn (array $m): bool => str_starts_with($m['name'], self::TEST_CATEGORY_PREFIX.'select_'), + )); + + self::assertCount( + 12, + $selectItems, + '?pagination=false doit retourner toutes les categories (pas seulement la page 1).', + ); + } + + /** + * La pagination doit fonctionner conjointement avec le flag ?includeDeleted=true. + * On seed 3 categories actives + 2 soft-deleted, on demande itemsPerPage=5 : + * la page 1 doit contenir exactement 5 items et totalItems doit valoir >= 5. + */ + public function testPaginationCombinedWithIncludeDeletedFlag(): void + { + $type = $this->createCategoryType(); + + // 3 categories actives. + for ($i = 1; $i <= 3; ++$i) { + $this->createCategory(self::TEST_CATEGORY_PREFIX.'pag_active_'.$i, $type); + } + + // 2 categories soft-deleted. + for ($i = 1; $i <= 2; ++$i) { + $this->createCategory( + self::TEST_CATEGORY_PREFIX.'pag_deleted_'.$i, + $type, + new DateTimeImmutable(), + ); + } + + $client = $this->createAdminClient(); + $response = $client->request('GET', '/api/categories?includeDeleted=true&itemsPerPage=5'); + + self::assertSame(200, $response->getStatusCode()); + + $data = $response->toArray(); + // La page retournee ne doit pas exceder itemsPerPage=5. + self::assertCount( + 5, + $data['member'], + 'La page 1 doit contenir exactement 5 items (itemsPerPage=5 avec >= 5 categories disponibles).', + ); + self::assertGreaterThanOrEqual( + 5, + $data['totalItems'], + 'totalItems doit refleter au moins les 5 categories seedees (actives + soft-deleted).', + ); + } +} diff --git a/tests/Module/Core/Api/AuditLogPaginationRegressionTest.php b/tests/Module/Core/Api/AuditLogPaginationRegressionTest.php new file mode 100644 index 0000000..9e29da6 --- /dev/null +++ b/tests/Module/Core/Api/AuditLogPaginationRegressionTest.php @@ -0,0 +1,71 @@ +authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/audit-logs'); + + self::assertSame(200, $response->getStatusCode()); + + $data = $response->toArray(); + self::assertArrayHasKey('totalItems', $data, 'La collection audit-logs doit exposer totalItems.'); + self::assertArrayHasKey('view', $data, 'La collection audit-logs doit exposer view (pagination active).'); + self::assertIsArray($data['member'], 'member doit etre un tableau.'); + + // Le nouveau defaut global est 10 (etait 30 dans AuditLogResource avant ERP-72). + self::assertLessThanOrEqual( + 10, + count($data['member']), + 'La page par defaut ne doit pas depasser 10 items (default global ERP-72).', + ); + } + + /** + * Un itemsPerPage excessif (99999) doit etre plafonne au maximum global (50). + * Teste la regression specifique du paginator DBAL custom (DbalPaginator) qui + * pourrait ignorer la limite si la logique de cap n'est pas appliquee cote provider. + */ + public function testAuditLogItemsPerPageCappedAt50(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/audit-logs?itemsPerPage=99999'); + + self::assertSame(200, $response->getStatusCode()); + + $data = $response->toArray(); + self::assertIsArray($data['member'], 'member doit etre un tableau.'); + + // Le cap global est 50 : jamais plus d'items par page que le maximum. + self::assertLessThanOrEqual( + 50, + count($data['member']), + 'itemsPerPage=99999 doit etre plafonne a 50 (maximum global ERP-72).', + ); + } +} diff --git a/tests/Module/Core/Api/PermissionApiTest.php b/tests/Module/Core/Api/PermissionApiTest.php index 18fc34b..703ca47 100644 --- a/tests/Module/Core/Api/PermissionApiTest.php +++ b/tests/Module/Core/Api/PermissionApiTest.php @@ -71,11 +71,43 @@ final class PermissionApiTest extends AbstractApiTestCase self::assertGreaterThanOrEqual(3, $data['totalItems']); } + /** + * Verrouille le chemin paginE PAR DEFAUT (ERP-72) : sans `?pagination=false`, + * `/api/permissions` doit borner la page au defaut global (10) et exposer + * `view`. Les autres tests de filtre passent `?pagination=false` et + * n'exercent donc plus ce contrat โ€” on le reteste ici de maniere isolee. + * + * On seed 12 permissions de test pour garantir un total > 10 quelle que soit + * la quantite de permissions reelles presentes en base. + */ + public function testDefaultCollectionIsPaginatedToGlobalDefault(): void + { + $em = $this->getEm(); + for ($i = 1; $i <= 12; ++$i) { + $em->persist(new Permission(sprintf('test.core.pagination.perm_%d', $i), sprintf('Perm pagination %d (test)', $i), 'core')); + } + $em->flush(); + $em->clear(); + + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/permissions'); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + + // La page par defaut ne doit jamais depasser le maximum global (10). + self::assertLessThanOrEqual(10, count($data['member']), 'La page par defaut doit etre bornee a 10 items.'); + // Avec >= 12 permissions de test (+ reelles), le total depasse une page. + self::assertGreaterThan(10, $data['totalItems']); + // `view` n'est present que lorsque la collection est reellement paginee. + self::assertArrayHasKey('view', $data, 'La collection doit exposer view quand totalItems > itemsPerPage.'); + } + public function testCollectionFilterByModule(): void { $client = $this->authenticatedClient('admin', 'admin'); $response = $client->request('GET', '/api/permissions', [ - 'query' => ['module' => 'core'], + 'query' => ['module' => 'core', 'pagination' => 'false'], ]); self::assertResponseIsSuccessful(); @@ -94,7 +126,7 @@ final class PermissionApiTest extends AbstractApiTestCase { $client = $this->authenticatedClient('admin', 'admin'); $response = $client->request('GET', '/api/permissions', [ - 'query' => ['orphan' => 'true'], + 'query' => ['orphan' => 'true', 'pagination' => 'false'], ]); self::assertResponseIsSuccessful(); @@ -114,7 +146,7 @@ final class PermissionApiTest extends AbstractApiTestCase { $client = $this->authenticatedClient('admin', 'admin'); $response = $client->request('GET', '/api/permissions', [ - 'query' => ['orphan' => 'false'], + 'query' => ['orphan' => 'false', 'pagination' => 'false'], ]); self::assertResponseIsSuccessful(); diff --git a/tests/Module/Core/Api/RoleApiTest.php b/tests/Module/Core/Api/RoleApiTest.php index f11bfa3..62abd45 100644 --- a/tests/Module/Core/Api/RoleApiTest.php +++ b/tests/Module/Core/Api/RoleApiTest.php @@ -146,7 +146,7 @@ final class RoleApiTest extends AbstractApiTestCase public function testGetCollectionAsAdminReturnsRoles(): void { $client = $this->authenticatedClient('admin', 'admin'); - $response = $client->request('GET', '/api/roles'); + $response = $client->request('GET', '/api/roles?pagination=false'); self::assertResponseIsSuccessful(); $data = $response->toArray(); @@ -157,6 +157,35 @@ final class RoleApiTest extends AbstractApiTestCase self::assertContains('test_editor', $codes); } + /** + * Verrouille le chemin paginE PAR DEFAUT (ERP-72) : le test ci-dessus passe + * `?pagination=false` (usage