refactor(commercial) : découpler l'hydratation des collections de la sélection clients (ERP-100) #50

2026-06-03T09:36:48Z

matthieu commented

2026-06-03 09:36:48 +00:00

Contexte

Issu de la review ERP-62 (#44). DoctrineClientRepository::createListQueryBuilder() portait 3 leftJoin+addSelect to-many imbriqués (categories × addresses × addresses.sites) partagés entre :

la liste paginée (ClientProvider) — bornée, OK ;
l'export XLSX et ?pagination=false — getResult() sans pagination → hydratation du produit cartésien sur tout le référentiel (1 client à 5 cat × 4 adr × 3 sites = 60 lignes SQL, × N clients).

Défaut d'altitude : un « QueryBuilder de liste » (contrat = filtres) imposait une stratégie d'hydratation à tout appelant.

Changements

createListQueryBuilder() redevient filtres + tri seuls — conforme au contrat de l'interface.
Nouvelle méthode hydrateListCollections(array $clients) : recharge les collections en 2 requêtes WHERE id IN(...) séparées (catégories d'un côté, adresses+sites de l'autre) via l'identity map Doctrine. Casse le triple cartésien en cat + (addr × site).
3 appelants branchés sur cette stratégie unique :
- liste paginée : fetchJoinCollection: false (COUNT simple) + hydratation de la page ;
- ?pagination=false : hydratation après getResult() ;
- export XLSX : hydratation après getResult().

Tests

make test : 465 OK.
Nouveau test ClientExportControllerTest::testExportPopulatesCategoryAndSiteColumns : garde-fou sur les valeurs Catégories/Sites de l'export (qu'un oubli d'hydratation rendrait silencieusement vides).
php-cs-fixer : 0 correction.

Notes

Benchmark « 1000+ clients » non exécuté (pas de jeu de données à cette échelle en dev) ; le cartésien est supprimé structurellement.
addr × site reste un join imbriqué (inévitable pour agréger les sites par adresse), désormais non multiplié par les catégories.

Closes ERP-100.

## Contexte Issu de la review ERP-62 (#44). `DoctrineClientRepository::createListQueryBuilder()` portait 3 `leftJoin+addSelect` to-many imbriqués (`categories × addresses × addresses.sites`) **partagés** entre : - la **liste paginée** (`ClientProvider`) — bornée, OK ; - l'**export XLSX** et **`?pagination=false`** — `getResult()` sans pagination → hydratation du **produit cartésien sur tout le référentiel** (1 client à 5 cat × 4 adr × 3 sites = 60 lignes SQL, × N clients). Défaut d'altitude : un « QueryBuilder de liste » (contrat = filtres) imposait une stratégie d'hydratation à tout appelant. ## Changements - **`createListQueryBuilder()`** redevient **filtres + tri seuls** — conforme au contrat de l'interface. - Nouvelle méthode **`hydrateListCollections(array $clients)`** : recharge les collections en **2 requêtes `WHERE id IN(...)` séparées** (catégories d'un côté, adresses+sites de l'autre) via l'identity map Doctrine. Casse le triple cartésien en `cat + (addr × site)`. - **3 appelants** branchés sur cette stratégie unique : - liste paginée : `fetchJoinCollection: false` (COUNT simple) + hydratation de la page ; - `?pagination=false` : hydratation après `getResult()` ; - export XLSX : hydratation après `getResult()`. ## Tests - `make test` : **465 OK**. - Nouveau test `ClientExportControllerTest::testExportPopulatesCategoryAndSiteColumns` : garde-fou sur les valeurs Catégories/Sites de l'export (qu'un oubli d'hydratation rendrait silencieusement vides). - `php-cs-fixer` : 0 correction. ## Notes - Benchmark « 1000+ clients » non exécuté (pas de jeu de données à cette échelle en dev) ; le cartésien est supprimé structurellement. - `addr × site` reste un join imbriqué (inévitable pour agréger les sites par adresse), désormais non multiplié par les catégories. Closes ERP-100.

matthieu added 1 commit 2026-06-03 09:36:49 +00:00

refactor(commercial) : découpler l'hydratation des collections de la sélection clients (ERP-100)

Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 1m55s

Details

Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m6s

Details

a97adb1dd9

createListQueryBuilder() redevient filtres + tri seuls (contrat de l'interface) :
plus de fetch-join to-many imposé à tous les appelants. L'hydratation des
collections affichées (Catégories / Site(s)) passe par la nouvelle méthode
hydrateListCollections(), appelée par la liste paginée, ?pagination=false et
l'export XLSX sur leur jeu déjà borné.

Deux requêtes IN séparées (catégories d'un côté, adresses+sites de l'autre)
remplissent les collections via l'identity map et cassent le produit cartésien
catégories × adresses × sites qui explosait sur les chemins non paginés.

Ajoute un garde-fou fonctionnel sur les colonnes Catégories/Sites de l'export.

Découvert en review ERP-62 (#44).

matthieu added the back M1-Client type/perf type/refactor labels 2026-06-03 09:37:20 +00:00

malio added 1 commit 2026-06-03 09:44:25 +00:00

Merge branch 'develop' into fix/ERP-100-decouple-list-hydration

Pull Request — Quality gate / Backend (PHP CS + PHPUnit) (pull_request) Successful in 1m47s

Details

Pull Request — Quality gate / Frontend (lint + Vitest + build) (pull_request) Successful in 1m11s

Details

59295779c1

malio merged commit 97301dcd6c into develop

2026-06-03 09:44:32 +00:00

malio deleted branch fix/ERP-100-decouple-list-hydration

2026-06-03 09:44:32 +00:00

matthieu commented

2026-06-03 09:44:42 +00:00

Review (relecture ciblée) — LGTM, non bloquant ✅

Refacto correct et même légèrement plus correct que l'existant. Synthèse :

Ce qui est bon

Le fix du cartésien est réel : 2 requêtes IN distinctes re-hydratant via l'identity map (mêmes instances managées) cassent bien cat × addr × site. Pattern Doctrine canonique.
fetchJoinCollection: false justifié : plus de jointure to-many dans le QB de sélection → COUNT trivial et exact (gain de correction, pas seulement de perf).
Pas de double exécution sur le chemin paginé : AbstractPaginator::getIterator() met l'itérateur en cache et DoctrinePaginator renvoie un ArrayIterator rewindable → la re-itération de sérialisation rejoue le même tableau (vérifié dans le vendor).
Une seule implémentation de l'interface, aucun mock → l'ajout de méthode ne casse rien. Lint PHP OK sur les 5 fichiers.
Le test garde-fou couvre le risque réel (colonnes vides si oubli d'hydratation).

2 remarques non bloquantes (info)

Export non borné + IN (:ids) : déplie en IN (?, ?, …) → plafond ~65 535 paramètres PostgreSQL. Exposition théorique (l'export chargeait déjà tout, c'est strictement mieux qu'avant) ; si l'export peut grossir, prévoir un chunk des ids. Pas à corriger ici.
?pagination=false hydrate 2 requêtes en plus alors que cette échappatoire alimente surtout des <select> ne sérialisant pas catégories/sites. Coût marginal, cohérence défensive acceptable. OK tel quel.

Rien à corriger. (Suite make test non exécutée côté review : container monté sur un autre checkout — lint + analyse statique seulement.)

**Review (relecture ciblée) — LGTM, non bloquant** ✅ Refacto correct et même légèrement plus *correct* que l'existant. Synthèse : **Ce qui est bon** - Le fix du cartésien est réel : 2 requêtes `IN` distinctes re-hydratant via l'identity map (mêmes instances managées) cassent bien `cat × addr × site`. Pattern Doctrine canonique. - `fetchJoinCollection: false` justifié : plus de jointure to-many dans le QB de sélection → `COUNT` trivial et exact (gain de correction, pas seulement de perf). - Pas de double exécution sur le chemin paginé : `AbstractPaginator::getIterator()` met l'itérateur en cache et `DoctrinePaginator` renvoie un `ArrayIterator` rewindable → la re-itération de sérialisation rejoue le même tableau (vérifié dans le vendor). - Une seule implémentation de l'interface, aucun mock → l'ajout de méthode ne casse rien. Lint PHP OK sur les 5 fichiers. - Le test garde-fou couvre le risque réel (colonnes vides si oubli d'hydratation). **2 remarques non bloquantes (info)** 1. Export non borné + `IN (:ids)` : déplie en `IN (?, ?, …)` → plafond ~65 535 paramètres PostgreSQL. Exposition théorique (l'export chargeait déjà tout, c'est strictement mieux qu'avant) ; si l'export peut grossir, prévoir un *chunk* des ids. Pas à corriger ici. 2. `?pagination=false` hydrate 2 requêtes en plus alors que cette échappatoire alimente surtout des `<select>` ne sérialisant pas catégories/sites. Coût marginal, cohérence défensive acceptable. OK tel quel. Rien à corriger. (Suite `make test` non exécutée côté review : container monté sur un autre checkout — lint + analyse statique seulement.)

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MALIO-DEV/Starseed#50