Sylvain Joffraud l4w
Articles Tags À propos Services Archives Prendre RDV
Retour

Doublons de n° de facture : MAX+1 et advisory lock

Un SaaS de gestion locative génère des documents numérotés : factures, quittances de loyer, reçus de paiement, régularisations de charges. Le numéro affiché sur chaque PDF (IN|60, RR|12, PR|7…) est censé être unique par propriétaire et par type de document — un compteur incrémental, classique.

En production, des locataires reçoivent deux PDF distincts portant le même numéro. Deux quittances « Numéro : 60 », baux différents, même propriétaire. Le bug a l’air anodin — un compteur qui se trompe. En réalité il touche trois couches : un tri SQL sans tie-breaker, une absence d’index unique, et — la partie la plus surprenante — une dette comptable qu’on ne peut pas effacer d’un UPDATE.

Symptôme

Le numéro est extrait d’une référence de la forme XX|Y portée par l’entité Document :

Une requête d’audit suffit à mesurer l’ampleur. Un doublon = même owner_id et même reference, présents plus d’une fois :

SELECT owner_id, reference, COUNT(*) AS nb
FROM document
WHERE reference IS NOT NULL
GROUP BY owner_id, reference
HAVING COUNT(*) > 1;

Le résultat est édifiant : un propriétaire avec 16 factures mais un numéro maximum de 5. Un autre avec 75 documents partageant tous IN|5. Et l’illusion typique : une dizaine de quittances de baux différents d’un même propriétaire, toutes estampillées IN|3, toutes créées à la même seconde.

Cause : trois faiblesses qui s’additionnent

Le calcul du numéro suivant ressemblait à ça :

// "numéro suivant = numéro du dernier document créé + 1"
$last = $this->documentRepository->getLastDocumentWithReference($owner, $type);
$next = $this->extractNumber($last) + 1;
-- getLastDocumentWithReference()
SELECT * FROM document
WHERE owner_id = :owner AND reference LIKE 'IN%'
ORDER BY created_at DESC   -- aucun second critère
LIMIT 1;

Trois problèmes se combinent, et c’est leur combinaison qui produit les doublons :

1. On prend « le dernier créé », pas « le plus grand numéro ». Le tri porte sur created_at, pas sur la valeur numérique de la référence. Or les deux ne coïncident pas toujours (régénération, reprise, import → un numéro élevé peut avoir un created_at antérieur).

2. created_at est en TIMESTAMP(0) — précision à la seconde. Toutes les factures générées dans la même seconde pour un propriétaire ont un created_at identique. ORDER BY created_at DESC LIMIT 1 renvoie alors une ligne arbitraire : il n’y a aucun tie-breaker. PostgreSQL est libre de te rendre n’importe laquelle des lignes ex æquo.

3. document.reference n’a aucun index unique. La base n’oppose donc aucun garde-fou. Rien n’empêche physiquement d’insérer deux fois IN|60.

Le scénario de collision le plus courant

Le cron nocturne boucle sur toutes les recettes à notifier et traite celles d’un même propriétaire en quelques millisecondes — donc dans la même seconde :

ÉtapeORDER BY created_at DESC LIMIT 1 renvoieN° attribuécreated_at
Facture AIN|59 (la veille)6010:00:00
Facture BIN|60 (seul à 10:00:00)6110:00:00
Facture CIN|60 et IN|61 à 10:00:00, tie arbitraire → IN|6061 ❌10:00:00

→ B et C portent le numéro 61. Le doublon apparaît dès la 3ᵉ facture d’un même propriétaire dans la même seconde.

Deux autres scénarios existent : le numéro élevé créé plus tôt (déterministe, indépendant de la concurrence) et la concurrence pure (cron + génération à la demande lisant la même « dernière référence » avant que l’une ait committé). Mais le scénario « même seconde » est le plus fréquent en production.

Un détour par le git blame : l’origine était pire

En remontant l’historique, surprise : le tri d’origine n’était pas ORDER BY created_at mais ORDER BY created_by. Comme la requête filtre déjà WHERE owner_id = :owner, created_by est constant pour un propriétaire donné — et carrément NULL en contexte cron (pas d’utilisateur authentifié). Le tri était donc totalement inopérant : il renvoyait une facture arbitraire parmi toutes celles du propriétaire, pas seulement parmi celles de la même seconde.

Le passage ultérieur à created_at n’a pas aggravé le bug : il l’a réduit. Il a rétabli un vrai ordre temporel qui corrige le cas séquentiel courant, ne laissant subsister que le bug résiduel des égalités à la seconde. Contre-intuitif quand on cherche un coupable : le commit qui « touche » la ligne fautive dans le blame est en fait celui qui a atténué le problème. Le vrai coupable est le commit d’origine qui a livré les trois faiblesses d’un coup — dont l’absence d’index unique dès le départ.

La correction : MAX+1 + advisory lock + transaction enveloppante

L’instinct, c’est de poser un UNIQUE (owner_id, reference) et de basculer le calcul sur MAX(numéro) + 1. On va voir pourquoi l’index unique est impossible ici — mais d’abord le calcul.

Calculer à partir du plus grand numéro, pas du dernier créé

public function getHighestReferenceNumber(Owner $owner, DocumentReferenceType $type): int
{
    // Verrou par (propriétaire, type), tenu jusqu'au commit de la transaction
    $this->lockSequence($owner, $type);

    $references = $this->createQueryBuilder('d')
        ->select('d.reference')
        ->where('d.owner = :owner')->setParameter('owner', $owner)
        ->andWhere('d.reference LIKE :prefix')
        ->setParameter('prefix', $type->prefix() . '|%')
        ->getQuery()
        ->getSingleColumnResult();

    // MAX numérique calculé en PHP : volume faible par propriétaire,
    // et on évite un cast SQL fragile sur une référence éventuellement malformée
    return array_reduce(
        $references,
        static fn (int $max, string $ref): int => max($max, (int) substr($ref, strpos($ref, '|') + 1)),
        0,
    );
}

Deux choix volontaires :

Le verrou de concurrence : advisory lock PostgreSQL

MAX+1 corrige le tri, mais pas la course : deux process qui lisent le même MAX au même instant produisent le même +1. La parade, faute d’index unique (voir plus bas), c’est un advisory lock transactionnel, scopé au couple (propriétaire, type) :

private function lockSequence(Owner $owner, DocumentReferenceType $type): void
{
    // Une seule séquence verrouillée à la fois : (owner, type).
    // pg_advisory_xact_lock se libère automatiquement au COMMIT/ROLLBACK.
    $key = $owner->getId()->toRfc4122() . '|' . $type->prefix();

    $this->getEntityManager()->getConnection()->executeStatement(
        'SELECT pg_advisory_xact_lock(hashtext(:key))',
        ['key' => $key],
    );
}

pg_advisory_xact_lock est un verrou applicatif léger : il ne pose pas de lock de ligne, il sérialise simplement les sections critiques qui partagent la même clé. Deux générations pour des propriétaires différents ne se bloquent pas. Le _xact_ garantit la libération automatique à la fin de la transaction — pas de risque de verrou orphelin si le process meurt.

Le piège : tenir le verrou jusqu’au COMMIT

Un advisory lock transactionnel ne sert à rien si la lecture MAX et la persistance du document ne sont pas dans la même transaction. Sinon le verrou se relâche entre les deux, et la fenêtre de course se rouvre. Il faut enrober lecture + écriture dans une transaction explicite :

trait TransactionalDocumentGenerationTrait
{
    private function generateWithinTransaction(callable $generate): Document
    {
        $connection = $this->entityManager->getConnection();

        // Si on est déjà dans une transaction (ex. appel depuis le postPersist
        // d'un listener Doctrine), ne pas en imbriquer une seconde — le verrou
        // de la transaction parente couvre déjà la section critique.
        if ($connection->isTransactionActive()) {
            return $generate();
        }

        return $this->entityManager->wrapInTransaction($generate);
    }
}

Le garde isTransactionActive() est essentiel : la génération de document est appelée depuis plusieurs chemins. Certains (le cron, l’action à la demande) n’ont pas de transaction ouverte → on en crée une. D’autres (un postPersist de listener de paiement) tournent déjà dans une transaction Doctrine → imbriquer un wrapInTransaction créerait un savepoint inutile et fragiliserait le rollback. On laisse la transaction parente porter le verrou.

C’est exactement le genre de détail qu’on oublie : on écrit pg_advisory_xact_lock, les tests unitaires passent, et en prod le verrou ne protège rien parce qu’il est relâché trois lignes plus loin, avant le flush. Un verrou transactionnel n’a de valeur que si tu maîtrises où commence et où finit la transaction.

Le vrai twist : on ne peut pas créer l’index unique

Voici où l’histoire devient intéressante. La « bonne » solution, celle qui rend le bug structurellement impossible, c’est le garde-fou en base :

ALTER TABLE document
    ADD CONSTRAINT document_reference_owner_unique
    UNIQUE (owner_id, reference);

Sauf que cette migration échoue. La base contient déjà des centaines de doublons. PostgreSQL refuse de créer une contrainte d’unicité sur des données qui la violent déjà :

ERROR:  could not create unique index "document_reference_owner_unique"
DETAIL:  Key (owner_id, reference)=(…, IN|5) is duplicated.

Pour créer l’index, il faudrait d’abord nettoyer l’historique — renuméroter les doublons existants. Et c’est là que la décision produit bascule du technique au métier : « on ne touche pas à l’existant ». Les doublons historiques restent en l’état. On corrige uniquement pour les nouvelles factures.

Conséquence en cascade :

Puisque les doublons existants ne sont pas corrigés, l’index unique ne peut pas être créé. Le correctif devient applicatif uniquement, sans migration de schéma. Et l’advisory lock, qui aurait pu n’être qu’une ceinture en plus de la bretelle « index unique », devient le garde-fou principal — le seul rempart contre la concurrence.

C’est un renversement de priorités qu’il faut assumer dans le code et dans les tests. Le test qui validait l’index unique (databaseRejectsDuplicateReferenceForSameOwner) est abandonné, remplacé par des tests qui prouvent que le verrou applicatif tient :

#[Test]
public function consecutiveGenerationsProduceDistinctNumbers(): void
{
    $owner = $this->getCurrentOwner();
    $generator = self::getContainer()->get(DocumentReferenceGenerator::class);

    $first = $generator->generate($owner, DocumentReferenceType::INVOICE);
    $this->persistInvoiceDocument($owner, $first, new DateTimeImmutable('2025-07-31 10:00:00'));
    $this->getEntityManager()->flush();

    $second = $generator->generate($owner, DocumentReferenceType::INVOICE);

    self::assertNotSame($first, $second);
}

Leçon d’architecture : quand tu ne peux pas mettre la contrainte d’intégrité là où elle devrait être (la base), tu dois la reconstituer un cran plus haut (l’application) — en sachant que ce garde-fou est plus faible (il ne protège que les chemins qui passent par ton code, pas un INSERT manuel, pas un autre service). Documenter cette faiblesse n’est pas optionnel : le prochain dev qui voit pg_advisory_xact_lock au lieu d’un UNIQUE doit comprendre que c’est un choix contraint, pas une préférence.

Le volet inattendu : un numéro en double, c’est une dette comptable

On pourrait s’arrêter là : bug compris, fix mergé, nouveaux doublons impossibles. Mais le stock historique pose une question que le code ne peut pas trancher seul : un document avec un numéro en double, est-ce un problème comptable ? Et si oui, a-t-on le droit de le renuméroter en base ?

Le réflexe « je fais un UPDATE pour réparer l’historique » est un piège. Selon la nature du document, c’est soit anodin, soit illégal.

La numérotation des factures est encadrée par la loi

En France, l’article 242 nonies A du CGI impose aux factures soumises à TVA un numéro unique fondé sur une séquence chronologique et continue. Un doublon viole cette unicité et expose à des sanctions réelles (amende au pourcentage du montant facturé, jusqu’au rejet du Fichier des Écritures Comptables en cas de contrôle).

Pire : depuis la loi anti-fraude TVA (article 286 I 3° bis CGI), tout logiciel émettant des factures TVA doit garantir l’inaltérabilité des données. Une renumérotation rétroactive silencieuse en base est strictement interdite. La seule correction conforme d’une facture TVA, c’est la procédure « avoir + ré-émission » :

  1. émettre un avoir (note de crédit) référençant la facture initiale ;
  2. émettre une nouvelle facture avec le prochain numéro libre et la mention « Annule et remplace la facture n° X » ;
  3. conserver les trois documents.

Autrement dit : si ces documents étaient des factures TVA, le UPDATE document SET reference = ... qui semblait évident serait une fraude documentaire.

Sauf que… ce ne sont pas des factures TVA

Le croisement entre le préfixe de référence et le type réel du document (déduit du nom de fichier) change tout :

PréfixeType réelRégime juridique
INQuittance de loyerLoi n° 89-462 (art. 21)
IN / PRReçu de paiementPas d’obligation de numérotation
INReçu de cautionPas d’obligation de numérotation

Aucun document du périmètre n’est une facture commerciale soumise à TVA. Ce sont des quittances de loyer et des reçus, relevant du régime « gestion locative » (location nue, exonérée de TVA). Et l’article 21 de la loi de 1989 qui régit les quittances n’impose aucune numérotation chronologique unique : il impose la gratuité, le détail loyer/charges, la délivrance à la demande. Point.

Le risque fiscal direct s’effondre. Reste un risque résiduel qui se déplace :

Trois stratégies, et celle qu’on retient

Cette qualification juridique débloque une option de remédiation interne qui serait interdite sur des factures TVA :

La renumérotation de l’option C est un script idempotent, sous le même advisory lock que le fix applicatif, exécuté hors des heures de cron :

Pour chaque groupe (owner_id, reference) en doublon :
    docs = SELECT * FROM document
           WHERE owner_id = :owner AND reference = :ref
           ORDER BY created_at ASC, id ASC    -- tie-breaker sur id, cette fois

    conserve   = docs[0]      // le plus ancien garde son numéro
    a_renumeroter = docs[1..]

    next = MAX(numéro du couple owner/type) + 1
    Pour chaque doc dans a_renumeroter :
        doc.previous_reference   = doc.reference
        doc.reference            = prefix || '|' || next
        doc.cancellation_reason  = 'Rattrapage doublons historiques'
        next = next + 1

Avec une précondition non négociable avant de poser l’index unique :

-- Doit renvoyer ZÉRO ligne, sinon la migration d'index échouera
SELECT owner_id, reference, COUNT(*)
FROM document
WHERE reference IS NOT NULL
GROUP BY owner_id, reference
HAVING COUNT(*) > 1;

Le point que je retiens : la stratégie de remédiation d’un bug de données n’est pas une décision de dev. Ici, c’est la qualification juridique du document — facture TVA vs quittance de loyer — qui détermine si on a le droit de faire un UPDATE, et donc l’architecture du correctif. J’ai écrit la requête d’audit, le script de renumérotation, le test d’idempotence ; mais la ligne qui débloque tout, c’est la validation d’un expert-comptable confirmant qu’aucune pièce n’est une facture assujettie.

Lessons learned

#php #symfony #doctrine #postgresql #concurrency #advisory-lock #billing #accounting #data-integrity #troubleshooting

Partager cet article :
Partager cet article sur Whatsapp Partager cet article sur X Partager cet article sur Telegram Partager cet article par email
Article précédent
Message d’erreur : une hypothèse, pas un diagnostic
Article suivant
robots.txt et bots IA : autoriser les moteurs de réponse