PD-43 — Retour d'Expérience¶

User Story : PD-43 — Implémentation du service OVH S3 Multipart Upload
Epic : Storage
Date de rédaction : 2025-12-31
Statut d'acceptabilité : ✅ ACCEPTÉ (2025-12-29)

1. Résumé exécutif¶

PD-43 visait à implémenter un service de téléversement multipart OVH S3 conforme aux spécifications strictes d'intégrité et de streaming (INV-2, INV-10). L'implémentation initiale (commit a4b9112) a été livrée avec deux écarts bloquants : violation du streaming sans buffering (E-01) et absence de vérification d'intégrité S3 (E-02). Ces écarts ont été détectés lors de la phase d'acceptabilité et corrigés dans le commit 2f04281. L'implémentation finale respecte tous les invariants, atteint une couverture de tests de 80.5% branches et 98.45% statements, et a été acceptée le 2025-12-29.

2. Points fluides¶

Architecture modulaire et séparation des responsabilités¶

La séparation en trois couches distinctes a facilité le développement, les tests et la correction des écarts :

Infrastructure (s3-multipart.adapter.ts) : Abstraction AWS SDK sans logique métier
Application (upload-session.service.ts) : Orchestration des flux N1-N4, validation des invariants
Domaine (upload-session.entity.ts, upload-part.entity.ts) : Règles métier encapsulées

Impact : Remplacement de l'adapter S3 stub par une implémentation réelle sans toucher à la logique applicative (E-02).

Streaming architecture avec Transform streams¶

L'implémentation finale utilise une vraie pipeline streaming Node.js :

HTTP Request Body → HashTransform (SHA3-256) → MD5Transform → S3 UploadPart

Code : upload-session.service.ts:152-171

Bénéfice : Support théorique de parts jusqu'à 5 GB sans limite mémoire, conformité stricte INV-2.

Double vérification d'intégrité sans overhead¶

Calcul parallèle de MD5 (S3) et SHA3-256 (ProbatioVault) dans le même pipeline streaming :

MD5 : Comparé avec ETag S3 pour détecter corruption réseau
SHA3-256 : Hash métier pour idempotence et référencement final

Code : s3-multipart.adapter.ts:200-258

Stratégie de tests avec mocking AWS SDK¶

Utilisation de aws-sdk-client-mock pour tester l'adapter S3 sans réseau :

Tests déterministes
Simulation de scénarios d'erreur (MD5 mismatch, ETag manquant)
Couverture finale : 80.5% branches, 98.45% statements

Code : s3-multipart.adapter.spec.ts:4-10

Spécification avec invariants numérotés¶

Le document de spécification avec invariants INV-1 à INV-11 a permis une vérification systématique lors de l'acceptabilité et facilité l'identification des écarts E-01 et E-02.

3. Points difficiles¶

E-01 : Violation de l'invariant de streaming (BLOQUANT)¶

Symptôme : La première implémentation (commit a4b9112) utilisait hashStreamWithBuffer() qui bufferisait entièrement la part en mémoire avant envoi S3.

Code problématique :

// src/modules/upload/services/upload-session.service.ts (commit a4b9112)
const { hash, size, buffer } = await this.hashService.hashStreamWithBuffer(
  stream,
  session.validationRules.maxPartSize
);
const s3Result = await this.s3Adapter.uploadPart({
  body: buffer, // ← Entire part in memory!
});

Cause racine : Confusion entre "hash en streaming" (calcul incrémental) et "streaming vers S3" (pas de bufferisation complète).

Violation : INV-2 (risque OOM pour une part de 5 GB).

Résolution (commit 2f04281) : Remplacement par createHashTransform() avec pipe direct vers S3.

Code corrigé :

const hashTransform = this.hashService.createHashTransform();
const hashedStream = stream.pipe(hashTransform);

const s3Result = await this.s3Adapter.uploadPart({
  body: hashedStream, // ← Direct streaming
  contentLength: undefined,
});

const incomingHash = hashTransform.getHash();
const size = hashTransform.getSize();

Temps de résolution : 2h (incluant refactorisation tests).

E-02 : Absence de vérification d'intégrité stockage (BLOQUANT)¶

Symptôme : L'adapter S3 était un stub avec ETag factice et getObjectStream retournant un flux vide.

Code problématique (commit a4b9112) :

// src/modules/upload/infrastructure/s3-multipart.adapter.ts
async uploadPart(input: UploadPartInput): Promise<UploadPartResult> {
  // Fake implementation
  return {
    etag: '"dummy-etag-' + input.partNumber + '"',
    partNumber: input.partNumber,
  };
}

async getObjectStream(input: GetObjectInput): Promise<Readable> {
  return Readable.from([]); // ← Empty stream!
}

Violation : INV-10 (pas de garantie de cohérence entre hash final et objet stocké).

Résolution (commit 2f04281) : 1. Implémentation S3Client réel avec credentials OVH 2. Calcul MD5 en streaming pendant l'upload 3. Vérification ETag S3 === MD5 calculé 4. getObjectStream retourne le flux S3 réel via GetObjectCommand

Code corrigé : s3-multipart.adapter.ts:176-266

Temps de résolution : 3h (incluant configuration credentials, tests de vérification MD5).

Timing des streams et calcul MD5 asynchrone¶

Symptôme : Le calcul MD5 se termine dans le callback flush() du Transform stream, potentiellement après le retour de s3Client.send(command). Test attendu en échec passait (MD5 non encore calculé lors de la vérification).

Cause racine : Les Transform streams sont asynchrones par nature. Le callback flush est appelé après que le stream soit consommé, mais sans garantie de synchronisation avec le code appelant.

Code problématique :

const md5Transform = new Transform({
  flush(callback) {
    md5Digest = md5Hash.digest('hex'); // ← Set asynchronously
    callback();
  },
});

const response = await this.s3Client.send(command);

// Race condition: md5Digest might still be null here!
if (md5Digest && s3Etag !== md5Digest) {
  throw new Error('Integrity check failed');
}

Résolution : Ajout d'une Promise explicite pour attendre la fin du stream finish.

Code corrigé : s3-multipart.adapter.ts:212-244

md5Promise = new Promise<void>((resolve) => {
  md5Transform.on('finish', resolve);
});

const response = await this.s3Client.send(command);

if (md5Promise) {
  await md5Promise; // ← Explicit wait
}

if (md5Digest && s3Etag !== md5Digest) {
  throw new Error('Integrity check failed');
}

Temps de résolution : 1h.

Couverture de tests initialement insuffisante (77.17%)¶

Symptôme : Première exécution des tests : 77.17% branches (seuil : 80%).

Branches manquantes : - Chemins d'erreur dans HashTransform._transform() et _flush() - Nullish coalescing dans SessionRepository (opérateur ??) - Scénarios de vérification MD5 (ETag manquant, MD5 mismatch)

Résolution : Ajout de 7 tests ciblés : - 4 tests d'erreur pour HashTransform - 3 tests de chemins alternatifs pour SessionRepository - Tests de scénarios d'intégrité S3 (s3-multipart.adapter.spec.ts:132-148)

Résultat final : 80.5% branches, 98.45% statements ✅

Temps de résolution : 1.5h.

4. Hypothèses révélées tardivement¶

Synchronisation explicite des événements Transform stream¶

Hypothèse initiale : Le calcul dans le callback flush() serait disponible immédiatement après await s3Client.send().

Réalité observée : Les événements finish/end des Transform streams sont asynchrones et doivent être attendus explicitement via Promise pour éviter les race conditions.

Impact : Nécessité d'ajouter md5Promise pour garantir la disponibilité de md5Digest avant la vérification.

Comportement de aws-sdk-client-mock vs SDK réel¶

Hypothèse initiale : Le mock S3Client retournerait automatiquement des ETag conformes.

Réalité observée : aws-sdk-client-mock requiert un mock explicite de chaque commande (CreateMultipartUploadCommand, UploadPartCommand, etc.) et ne simule pas le comportement MD5 automatique de S3.

Impact : Nécessité de mocker manuellement les ETag avec des MD5 corrects dans les tests (s3-multipart.adapter.spec.ts:110-130).

Calcul du hash final nécessite re-download¶

Hypothèse initiale : Possibilité de calculer le hash final en concaténant les hash de parts (Plan PV-1, PV-2).

Réalité observée : S3 retourne un ETag composite pour les uploads multipart (format "hash-nbparts"), incompatible avec la concaténation simple de hash de parts.

Impact : Adoption du plan PV-3 (fallback) : re-download de l'objet complet depuis S3 pour calculer le hash final.

5. Invariants complexes¶

INV-2 : Streaming sans buffering¶

Énoncé : "Les parts DOIVENT être traitées en streaming, sans chargement complet en mémoire."

Complexité : Risque élevé de régression. Toute modification du pipeline de traitement (ex: ajout d'un Transform intermédiaire) peut réintroduire une bufferisation accidentelle.

Maintien : - Tests de mémoire avec parts volumineuses (non implémenté) - Code review systématique des modifications de uploadPart - Documentation explicite du pattern streaming dans les commentaires

Fragilité : Le pattern stream.pipe(transform1).pipe(transform2) est fragile face aux modifications naïves (ex: const buffer = await streamToBuffer(stream)).

INV-10 : Vérification d'intégrité stricte¶

Énoncé : "Vérification d'intégrité avec checksums S3 (MD5) ET hash métier (SHA3-256)."

Complexité : Double calcul (MD5 + SHA3-256) en parallèle dans le même pipeline sans impacter les performances.

Maintien : - Synchronisation explicite des événements finish pour les deux Transform streams - Tests de scénarios d'intégrité (MD5 mismatch, ETag manquant)

Fragilité : Les race conditions sur md5Digest et finalHash peuvent réapparaître lors de modifications du pipeline.

INV-9 : Normativité SHA3-256¶

Énoncé : "L'algorithme de hachage est SHA3-256, hex encoding."

Complexité : Changement d'algorithme impossible sans migration des hash stockés. Toute erreur d'implémentation (ex: utilisation de SHA-256 au lieu de SHA3-256) est critique.

Maintien : - Tests avec vecteurs de test SHA3-256 connus - Interdiction de modifier HashStreamService sans validation cryptographique

6. Dette technique¶

Calcul du hash final par re-download S3 (Plan PV-3)¶

Description : Le hash final est calculé en re-téléchargeant l'objet complet depuis S3 après complétion.

Impact : - Latence supplémentaire à la complétion (proportionnelle à la taille du fichier) - Bande passante réseau doublée (upload + download)

Justification : Seule approche garantissant la cohérence entre l'objet stocké et le hash retourné au client (ETag composite S3 inutilisable).

Mitigation future : Investigation de l'API S3 pour obtenir un hash cryptographique de l'objet complet (HEAD Object metadata).

Configuration credentials S3 via ConfigService¶

Description : Les credentials S3 (access key, secret key) sont passés via ConfigService sans chiffrement côté application.

Impact : Risque d'exposition en cas de dump de configuration.

Justification : Délégation de la sécurité à HashiCorp Vault (backend récupère les credentials depuis Vault au démarrage).

Mitigation future : Rotation automatique des credentials S3 via Vault dynamic secrets.

Tests E2E absents (pas de MinIO local)¶

Description : Aucun test d'intégration E2E avec un vrai S3 (MinIO local).

Impact : Les tests unitaires avec aws-sdk-client-mock ne valident pas le comportement réseau réel (timeout, retry, corruption).

Justification : Complexité d'intégration de MinIO dans le pipeline CI GitLab.

Mitigation future : Ajout de docker-compose avec MinIO pour tests d'intégration locaux et CI.

7. Risques résiduels¶

Pas de test de charge avec parts volumineuses (1-5 GB)¶

Risque : Fuite mémoire ou timeout non détectés pour des parts proches de maxPartSize (5 GB).

Probabilité : Faible (architecture streaming validée).

Impact : ÉLEVÉ (échec en production pour gros fichiers).

Mitigation : Ajouter des tests de charge avec parts de 1 GB, 2 GB, 5 GB sur environnement de staging.

Pas de monitoring mémoire en production¶

Risque : Impossible de détecter une régression de l'invariant INV-2 (bufferisation accidentelle) en production.

Probabilité : Moyenne (modifications futures du pipeline).

Impact : ÉLEVÉ (OOM en production).

Mitigation : Ajouter des métriques Prometheus sur l'usage mémoire par session d'upload (heap size, RSS).

Gestion des erreurs réseau S3 (retry, timeout)¶

Risque : Aucune stratégie de retry pour les erreurs réseau transitoires S3 (HTTP 500, timeout).

Probabilité : Moyenne (infrastructure réseau OVH).

Impact : MOYEN (échec d'upload pour erreurs transitoires).

Mitigation : Configurer la stratégie de retry AWS SDK v3 (maxAttempts, retryMode).

Race condition sur sessions expirées¶

Risque : Une session peut expirer entre la vérification isExpired() et l'upload effectif vers S3.

Probabilité : Faible (fenêtre de quelques millisecondes).

Impact : FAIBLE (erreur client, pas de corruption).

Mitigation : Aucune (acceptable selon la spec).

8. Améliorations processus¶

Spécification : Exemples de code pour invariants critiques¶

Constat : Les invariants INV-2 et INV-10 étaient bien décrits textuellement, mais leur implémentation concrète n'était pas évidente.

Amélioration : Ajouter des exemples de code conforme et non conforme dans la spécification pour les invariants critiques.

Proposition :

### INV-2 — Streaming sans buffering

**Contrainte** : Les parts DOIVENT être traitées en streaming, sans chargement complet en mémoire.

**Exemple conforme** :
```typescript
const hashTransform = createHashTransform();
const hashedStream = inputStream.pipe(hashTransform);
await s3Adapter.uploadPart({ body: hashedStream });

Exemple NON conforme :

const buffer = await streamToBuffer(inputStream); // ❌ Full buffering
await s3Adapter.uploadPart({ body: buffer });

### Plan d'implémentation : Section "Risques techniques"

**Constat** : Le plan d'implémentation listait les tâches mais n'identifiait pas les risques critiques (streaming, timing MD5).

**Amélioration** : Ajouter une section **"Risques techniques"** avec probabilité, impact, et mitigation.

**Proposition** :
```markdown
## Risques techniques identifiés

| Risque | Probabilité | Impact | Mitigation |
|--------|-------------|--------|------------|
| Violation INV-2 (buffering accidentel) | Moyenne | BLOQUANT | Code review + test de mémoire avec part 1GB |
| Race condition MD5/ETag | Faible | BLOQUANT | Attente explicite événement `finish` |
| Couverture < 80% | Moyenne | Mineur | Tests d'erreur systématiques |

Tests : Tests d'intégration E2E obligatoires¶

Constat : Les tests unitaires avec mocking ont détecté E-01 et E-02, mais n'auraient pas détecté des problèmes réseau S3.

Amélioration : Exiger un test d'intégration E2E avec MinIO local pour valider la chaîne complète.

Proposition : - Ajouter upload-session.integration.spec.ts avec MinIO via Docker Compose - Tester upload réel → complétion → vérification hash final sur objet stocké

Acceptabilité : Code review pré-acceptabilité¶

Constat : Les écarts E-01 et E-02 auraient pu être détectés plus tôt par une code review avant la phase d'acceptabilité.

Amélioration : Ajouter une code review obligatoire avec checklist des invariants critiques avant soumission à l'acceptabilité.

Proposition : - Template de PR avec checklist INV-1 à INV-11 - Review assignée à un second développeur

9. Enseignements clés¶

1. Pattern streaming-first avec Transform streams¶

Concevoir dès le départ avec des Transform streams évite des refactorisations coûteuses. Le pattern stream.pipe(transform) garantit une mémoire constante quelle que soit la taille des données.

Actionnable : Utiliser systématiquement Transform streams pour tout traitement de flux > 100 MB.

2. Double vérification d'intégrité sans overhead¶

Le calcul parallèle de MD5 et SHA3-256 dans le même pipeline streaming n'ajoute pas de latence (traitement en transit). Coût CPU marginal comparé au bénéfice sécurité.

Actionnable : Privilégier les vérifications d'intégrité multiples (checksum transport + hash métier) pour les opérations critiques.

3. Synchronisation explicite des événements asynchrones¶

Les événements finish/end des streams doivent être gérés explicitement via Promise pour éviter les race conditions dans les calculs asynchrones.

Actionnable : Toujours attendre explicitement les événements stream avant d'utiliser les résultats calculés dans les callbacks.

4. Importance de la couverture des chemins d'erreur¶

Les chemins d'erreur sont souvent oubliés mais critiques pour la robustesse. Utiliser jest --coverage --coverageReporters=text pour identifier précisément les branches non couvertes.

Actionnable : Exiger systématiquement des tests pour les chemins d'erreur (throw, reject, invalid input).

5. Spécification avec invariants numérotés facilite l'acceptabilité¶

Les invariants numérotés (INV-1 à INV-11) permettent une vérification systématique et une traçabilité claire des écarts.

Actionnable : Systématiser la numérotation des invariants dans toutes les spécifications futures.

Conclusion¶

Métriques finales¶

Fichiers modifiés : 39 (30 production, 9 tests)
Couverture de tests : 80.5% branches, 98.45% statements
Commits : 3 (a4b9112 feat, 1e0ef9a style, 2f04281 fix E-01/E-02)
Durée totale : 3 jours (2025-12-27 au 2025-12-29)

Temps passé par phase¶

Implémentation initiale : 1.5 jour
Résolution E-01 (streaming) : 2h
Résolution E-02 (intégrité) : 3h
Couverture tests : 1.5h
Documentation : 1h

Total estimé : ~20h

Verdict final¶

✅ ACCEPTÉ (2025-12-29)
Conformité : Tous les invariants INV-1 à INV-11 respectés
Qualité : Architecture streaming robuste, double vérification d'intégrité, couverture tests > 80%
Recommandation : Ajouter tests E2E MinIO et monitoring mémoire production pour mitiger les risques résiduels

Rédacteur : Équipe ProbatioVault
Date : 2025-12-31