# 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` :

```php
// 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 :

```sql
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 :**
- `/Inventory_frontend/app/composables/useComponentEdit.ts` (550 LOC)
- `/Inventory_frontend/app/composables/usePieceEdit.ts` (472 LOC)
- `/Inventory_frontend/app/composables/useMachineDetailData.ts` (468 LOC)
- `/Inventory_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 :**
- `/Inventory_frontend/app/shared/utils/customFieldFormUtils.ts` (404 LOC) - pour les pages create/edit
- `/Inventory_frontend/app/shared/utils/customFieldUtils.ts` (440 LOC) - pour la page machine detail
- `/Inventory_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 :

```php
// 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 :

```php
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 :**
- `/Inventory_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.