Inventory/docs/REVIEW_ARCHITECTURE.md

Revue d'architecture - Sources de complexite et effets de bord

Date : 2026-03-23 Branche analysee : develop

---

Diagnostic - Top 10 des sources de complexite

#	Source	Impact	Effort
1	Duplication massive du smartMatch dans les Sync Strategies	Bugs silencieux, maintenance triple	M
2	Custom Fields : 4 FK nullable sur une seule entite (polymorphisme pauvre)	Integrite fragile, code defensif partout	L
3	Composables frontend geants avec responsabilites multiples	Difficile a tester, refactoring risque	M
4	3 fichiers utils de custom fields frontend avec logique qui se chevauche	Incoherences, bugs de merge/dedup	M
5	pendingStructure : canal de communication cache entre deserialisation et processor	Effet de bord invisible, timing fragile	S
6	PieceProductSyncSubscriber : legacy sync dans un subscriber Doctrine	Side effect cache, recompute du changeset	S
7	Double flush dans les processors (decorated + flush manuel)	Audit logs potentiellement incomplets	S
8	MachineStructureController : God controller avec normalisation JSON manuelle	Bypass API Platform, 300+ LOC de serialisation	L
9	Chaine de dependances circulaire dans useMachineDetailData	Proxy refs, ordre d'initialisation fragile	M
10	Frontend : typage any systematique sur les entites	Pas de filet de securite TypeScript	L

---

Analyse detaillee

1. Duplication du smartMatch dans les Sync Strategies

Fichiers concernes :

• /src/Service/Sync/ComposantSyncStrategy.php (lignes 380-446)
• /src/Service/Sync/PieceSyncStrategy.php (lignes 244-308)

Probleme : smartMatch(), smartMatchPreview() et toute la logique de sync des custom field values sont copiees-collees entre ComposantSyncStrategy et PieceSyncStrategy. Le ProductSyncStrategy a une version simplifiee (pas de slots). Si un bug est corrige dans l'un, il faut penser a le corriger dans l'autre.

Effets de bord concrets :

• Un correctif sur le matching des slots dans une strategie peut etre oublie dans l'autre
• Le compteur de preview custom fields utilise orderIndex comme cle de matching, ce qui est fragile (reindexation = faux positif)

Solution proposee (effort M) : Extraire un trait ou une classe abstraite AbstractSlotSyncStrategy :

// AVANT : smartMatch() duplique dans ComposantSyncStrategy et PieceSyncStrategy

// APRES : extraire dans un trait
trait SlotSyncTrait
{
    protected function smartMatch(array $existingTypeIds, array $proposedTypeIds): array
    {
        // ... logique unique
    }

    protected function syncCustomFieldValues(
        object $entity,
        string $fkField,
        array $customFields,
        bool $confirmDeletions,
    ): array {
        // ... logique unique pour add/remove CFValues
    }
}

La methode execute() de chaque strategie ne garderait que la boucle specifique a son type de slot (piece slots, product slots, subcomponent slots), et deleguerait le matching et la gestion des CF values au trait.

---

2. Custom Fields : polymorphisme par FK nullable

Fichiers concernes :

• /src/Entity/CustomField.php - 4 FK nullable : machine, typeComposant, typePiece, typeProduct
• /src/Entity/CustomFieldValue.php - 4 FK nullable : machine, composant, piece, product
• /src/Controller/CustomFieldValueController.php - resolveTarget() fait un switch sur 4 types

Probleme : Un CustomFieldValue peut pointer vers machine OU composant OU piece OU produit via 4 colonnes nullable. Rien n'empeche au niveau DB qu'un CFV pointe vers deux entites en meme temps. Le frontend doit deviner le type cible. Chaque nouveau type d'entite necessite d'ajouter une colonne, un setter, et un cas dans tous les switches.

Effets de bord concrets :

• Le CustomFieldValueController::resolveTarget() tente 4 cles dans un ordre specifique -- si le payload a machineId ET composantId, seul machine est utilise (silent bug)
• Les audit subscribers (getOwnerFromCustomFieldValue) doivent tester chaque getter -- si getComposant() renvoie un objet alors que getMachine() aussi, le comportement est indetermine
• La serialisation API Platform expose les 4 FK meme quand 3 sont null

Solution proposee (effort L) :

Option pragmatique (pas de refactoring DB) : ajouter une colonne discriminante entityType (enum) + contrainte CHECK :

ALTER TABLE custom_field_values
  ADD COLUMN entity_type VARCHAR(20) NOT NULL DEFAULT 'machine';

ALTER TABLE custom_field_values
  ADD CONSTRAINT chk_single_fk CHECK (
    (entity_type = 'machine' AND machineId IS NOT NULL AND composantId IS NULL AND pieceId IS NULL AND productId IS NULL) OR
    (entity_type = 'composant' AND composantId IS NOT NULL AND machineId IS NULL AND pieceId IS NULL AND productId IS NULL) OR
    (entity_type = 'piece' AND pieceId IS NOT NULL AND machineId IS NULL AND composantId IS NULL AND productId IS NULL) OR
    (entity_type = 'product' AND productId IS NOT NULL AND machineId IS NULL AND composantId IS NULL AND pieceId IS NULL)
  );

Cela securise l'integrite sans changer l'architecture. Le resolveTarget et les audit subscribers pourraient ensuite brancher sur entityType au lieu de tester 4 FK.

---

3. Composables frontend geants (400-550 LOC)

Fichiers concernes :

• /frontend/app/composables/useComponentEdit.ts (550 LOC)
• /frontend/app/composables/usePieceEdit.ts (472 LOC)
• /frontend/app/composables/useMachineDetailData.ts (468 LOC)
• /frontend/app/composables/useComponentCreate.ts (417 LOC)

Probleme : Ces composables orchestrent en un seul fichier : le chargement de donnees, la gestion de formulaire, la persistence des custom fields, la gestion des documents, l'historique, la resolution de labels, et la soumission. Chacun instancie 8-12 sous-composables.

Effets de bord concrets :

• useComponentEdit instancie usePieces(), useProducts(), useComposants() a chaque montage de page, meme si ces catalogues sont deja charges -- requetes API en double
• La logique de soumission (submitEdition, submitCreation) melange la construction du payload, la validation, l'appel API, et la persistence des custom fields -- si une etape echoue, l'etat local est partiellement modifie
• Les watchers sur selectedType/selectedTypeStructure dans useComponentCreate et useComponentEdit font des choses differentes pour le meme concept -- source de divergence

Solution proposee (effort M) : Decouper chaque composable geant en sous-composables par responsabilite, comme deja fait pour useMachineDetailData (qui delegue a useMachineDetailDocuments, useMachineDetailCustomFields, etc.) :

useComponentEdit.ts (550 LOC)
  -> useComponentEditForm.ts        (~100 LOC : reactive form, validation)
  -> useComponentEditDocuments.ts   (~80 LOC : upload, preview, delete)
  -> useComponentEditSlots.ts       (~120 LOC : slot selection/save)
  -> useComponentEditCustomFields.ts (~60 LOC : build inputs, save)
  -> useComponentEdit.ts            (~150 LOC : orchestrateur)

Appliquer le meme pattern a usePieceEdit et useComponentCreate. Les blocs communs (document handling, custom field save, price formatting) deviendraient des composables partages.

---

4. Triple duplication de la logique custom fields frontend

Fichiers concernes :

• /frontend/app/shared/utils/customFieldFormUtils.ts (404 LOC) - pour les pages create/edit
• /frontend/app/shared/utils/customFieldUtils.ts (440 LOC) - pour la page machine detail
• /frontend/app/shared/utils/entityCustomFieldLogic.ts (335 LOC) - pour les composants item

Probleme : Ces 3 fichiers resolvent le meme probleme (normaliser des definitions de custom fields + merger avec des valeurs existantes) avec des implementations differentes :

• customFieldFormUtils.ts : resolveFieldName(), resolveFieldType(), buildCustomFieldInputs()
• entityCustomFieldLogic.ts : resolveFieldName() (differente!), resolveFieldType() (differente!), mergeFieldDefinitionsWithValues()
• customFieldUtils.ts : extractDefinitionName(), normalizeExistingCustomFieldDefinitions(), mergeCustomFieldValuesWithDefinitions()

Effets de bord concrets :

• Trois facons differentes de resoudre le nom d'un champ -- resolveFieldName dans customFieldFormUtils teste name, key, label ; dans entityCustomFieldLogic elle teste name seulement et retourne 'Champ' par defaut
• Trois algorithmes de merge values/definitions -- un bug corrige dans l'un n'est pas corrige dans les autres
• La deduplication par name+type dans entityCustomFieldLogic.ts et par orderIndex dans customFieldUtils.ts produit des resultats differents pour les memes donnees

Solution proposee (effort M) : Fusionner en un seul module customFields.ts avec :

1. Une seule fonction resolveFieldName(field: any): string
2. Une seule fonction mergeDefinitionsWithValues(defs, values): MergedField[]
3. Une seule fonction deduplicateFields(fields): MergedField[]

Les 3 fichiers actuels deviendraient des re-exports ou des wrappers fins. Commencer par aligner les signatures, puis remplacer les imports un par un.

---

5. pendingStructure : canal de communication cache

Fichiers concernes :

• /src/Entity/ModelType.php et /src/Entity/Composant.php -- propriete #[ApiProperty] non mappee en DB
• /src/State/ModelTypeProcessor.php (lignes 33-43)
• /src/State/ComposantProcessor.php (lignes 42-51)

Probleme : Le champ structure envoye par le frontend est intercepte par API Platform dans un champ pendingStructure (non mappe en DB), puis lu par le processor apres le persist du decorated processor. Ce mecanisme est invisible : rien dans l'entite n'indique qu'un setter a un effet de bord differe.

Effets de bord concrets :

• Si le decorated->process() leve une exception, le pendingStructure reste dans l'entite -- pas de cleanup
• Le flush() supplementaire dans le processor (ligne 43 de ModelTypeProcessor) declenche les audit subscribers une deuxieme fois pour le meme cycle de request -- les snapshots d'audit peuvent capturer un etat intermediaire
• Un developpeur qui modifie le ModelType via Doctrine directement (fixture, migration, CLI) ne beneficie pas de ce mecanisme -- les skeleton requirements ne sont pas mis a jour

Solution proposee (effort S) : Documenter explicitement ce pattern dans l'entite avec un docblock. Ajouter un try/finally pour le cleanup :

// ModelTypeProcessor::process()
try {
    $result = $this->decorated->process($data, $operation, $uriVariables, $context);
    if (null !== $pendingStructure) {
        $this->skeletonStructureService->updateSkeletonRequirements($data, $pendingStructure);
        $this->entityManager->flush();
    }
    return $result;
} finally {
    $data->clearPendingStructure();
}

---

6. PieceProductSyncSubscriber : side effect cache

Fichier concerne :

• /src/EventSubscriber/PieceProductSyncSubscriber.php

Probleme : Ce subscriber Doctrine ecoute prePersist et preUpdate pour synchroniser la relation legacy product (ManyToOne) avec la collection productIds (JSON array). Sur preUpdate, il fait un recomputeSingleEntityChangeSet (ligne 50-51), ce qui modifie le changeset en cours de flush.

Effets de bord concrets :

• Le recompute du changeset peut interferer avec les audit subscribers qui lisent ce meme changeset -- l'audit log peut capturer le changement de product comme une modification manuelle alors qu'il est automatique
• L'ordre d'execution des subscribers n'est pas garanti -- si l'audit subscriber s'execute avant le sync, il ne voit pas le changement de product
• Si productIds est vide, le subscriber ne touche pas product -- mais si product avait deja une valeur, elle reste (pas de cleanup)

Solution proposee (effort S) : Remplacer ce subscriber par une logique explicite dans le controller/processor qui traite les pieces. Le sync productIds -> product devrait etre fait AVANT le flush, pas dans un subscriber. Cela supprime l'ambiguite sur l'ordre d'execution et le recompute.

Alternativement, si la relation legacy product (ManyToOne) n'est plus utilisee par le frontend, la supprimer completement et ne garder que productIds / les product slots.

---

7. Double flush dans les processors

Fichiers concernes :

• /src/State/ModelTypeProcessor.php (ligne 36 via decorated, ligne 43 manuellement)
• /src/State/ComposantProcessor.php (ligne 45 via decorated, ligne 132 manuellement)

Probleme : Le decorated processor fait un flush() pour persister l'entite, puis un second flush() est appele pour persister les skeleton requirements ou slots. Chaque flush declenche onFlush dans tous les audit subscribers.

Effets de bord concrets :

• Le premier flush capture le create de l'entite dans l'audit log
• Le second flush peut generer un update de la meme entite si les slots ont modifie une relation qui declenche un dirty check (par ex. si $composant->incrementVersion() etait appele)
• En cas d'erreur entre les deux flush, l'entite est persistee mais ses slots ne le sont pas -- etat inconsistant

Solution proposee (effort S) : Wrapper les deux operations dans une transaction explicite, et ne faire qu'un seul flush a la fin :

public function process(mixed $data, Operation $operation, ...): mixed
{
    return $this->entityManager->wrapInTransaction(function () use ($data, $operation, ...) {
        // Ne pas flush dans le decorated -- utiliser le mode COMMIT_ON_CLOSE
        $result = $this->decorated->process($data, $operation, $uriVariables, $context);

        if (null !== $pendingStructure) {
            $this->skeletonStructureService->updateSkeletonRequirements($data, $pendingStructure);
        }
        $data->clearPendingStructure();

        // Un seul flush
        $this->entityManager->flush();
        return $result;
    });
}

Note : cela necessite de verifier que le decorated processor ne fait pas deja un flush interne non configurable. Si c'est le cas, il faudrait potentiellement ne pas utiliser le decorated et gerer le persist manuellement.

---

8. MachineStructureController : God controller

Fichier concerne :

• /src/Controller/MachineStructureController.php (300+ LOC)

Probleme : Ce controller gere GET structure, PATCH structure, et POST clone. Il contient toute la logique de normalisation JSON des links (component, piece, product), la resolution des entites, et la serialisation manuelle de la reponse -- tout ce qu'API Platform fait normalement automatiquement.

Effets de bord concrets :

• La normalisation JSON manuelle (normalizeStructureResponse) ne passe pas par les serialization groups d'API Platform -- si un champ est ajoute a une entite avec un group, il n'apparaitra pas dans la reponse structure
• Le PATCH structure fait $this->entityManager->flush() sans transaction -- si la creation d'un link echoue, les precedents sont deja persistes
• Le clone copie les custom fields mais pas les documents -- comportement potentiellement inattendu
• 8 repositories injectes dans le constructeur -- code smell

Solution proposee (effort L) :

1. Extraire la logique de normalisation dans un MachineStructureSerializer service
2. Extraire la logique de clone dans un MachineCloneService
3. Wrapper le PATCH et le clone dans des transactions
4. A terme, considerer un DTO + custom provider/processor API Platform pour le GET/PATCH structure

---

9. Dependance circulaire dans useMachineDetailData

Fichier concerne :

• /frontend/app/composables/useMachineDetailData.ts (lignes 119-187)

Probleme : useMachineDetailProducts a besoin de machineProductLinks (venant de hierarchy), et useMachineDetailHierarchy a besoin de findProductById (venant de products). La solution actuelle utilise un _machineProductLinksProxy ref avec un watcher pour synchroniser.

Effets de bord concrets :

• Le proxy ref est mis a jour de facon asynchrone via un watcher -- pendant le premier tick de rendu, _machineProductLinksProxy est vide meme si les liens sont deja charges
• L'ordre d'initialisation des sous-composables est fragile -- deplacer une ligne peut casser la boucle
• Le commentaire dans le code (lignes 119-122) admet explicitement le probleme

Solution proposee (effort M) : Inverser la dependance : le composable useMachineDetailHierarchy devrait etre le seul a gerer les links et exposer les product links. useMachineDetailProducts ne devrait recevoir que les product IDs (pas les links complets). Cela supprime la circularite.

Alternativement, creer un useMachineDetailState purement reactif (store local) qui contient tous les refs partages, et le passer aux sous-composables. Cela explicite les dependances.

---

10. Typage any systematique sur les entites frontend

Fichiers concernes : Quasi tous les composables utilisent ref<any | null>(null) pour les entites :

• useComponentEdit.ts : const component = ref<any | null>(null) (ligne 74)
• usePieceEdit.ts : const piece = ref<any | null>(null) (ligne 56)
• useMachineDetailData.ts : type AnyRecord = Record<string, unknown>

Probleme : Les reponses API ne sont jamais typees. L'acces aux proprietes se fait par convention (result.data?.structure?.pieces) sans aucune validation. TypeScript ne peut pas detecter les typos ou les acces a des proprietes inexistantes.

Effets de bord concrets :

• Un changement de nom de champ cote API ne provoque aucune erreur TypeScript -- le bug n'est decouvert qu'au runtime
• L'autocompletion IDE est inutile sur ces objets
• Les defensives checks (Array.isArray(x?.y) ? x.y : []) sont necessaires partout parce que le type ne garantit rien

Solution proposee (effort L) :

1. Creer des interfaces TypeScript pour les reponses API principales : MachineStructureResponse, ComposantResponse, PieceResponse, ProductResponse, ModelTypeResponse
2. Ajouter une couche de validation a la reception dans useApi.ts (optionnelle, avec Zod ou un type guard maison)
3. Remplacer progressivement ref<any> par ref<ComposantResponse | null>

Commencer par les entites les plus utilisees (Machine, Composant) pour obtenir un benefice immediat.

---

Plan de simplification -- Ordre recommande

Phase 1 : Quick wins (1-2 jours chacun, impact immediat)

#	Action	Source	Effort
1	Extraire smartMatch + sync CF values dans un trait partage	Source 1	S
2	Ajouter try/finally sur clearPendingStructure	Source 5	S
3	Remplacer PieceProductSyncSubscriber par logique explicite	Source 6	S
4	Wrapper les processors dans des transactions	Source 7	S

Phase 2 : Unification frontend (1-2 semaines)

#	Action	Source	Effort
5	Fusionner les 3 fichiers custom fields utils en un seul	Source 4	M
6	Decouper useComponentEdit / usePieceEdit en sous-composables	Source 3	M
7	Resoudre la circularite dans useMachineDetailData	Source 9	M

Phase 3 : Renforcement structurel (2-4 semaines)

#	Action	Source	Effort
8	Ajouter la contrainte CHECK sur custom_field_values	Source 2	M
9	Typer les reponses API principales	Source 10	L
10	Extraire services depuis MachineStructureController	Source 8	L

Principe directeur

Commencer par la phase 1 -- elle ne modifie pas les interfaces (ni API ni frontend) et supprime les effets de bord les plus dangereux. La phase 2 est une consolidation frontend qui peut etre faite page par page. La phase 3 est un investissement a plus long terme.

Ne pas tenter de tout refactorer en une fois. Chaque item peut etre un PR isole, testable independamment.