1. Checklist pour la couche de persistance
Si dans du code Java classique, on voit souvent la cause et l'effet presque directement (on appelle une méthode — on obtient un résultat), Hibernate aime les surprises : on modifie un champ — UPDATE apparaîtra plus tard, pas « tout de suite » ; on prend order.getCustomer() — et soudain un SQL s'exécute ; on fait une mise à jour bulk — et le persistence context reste tranquillement dans sa réalité parallèle. Par conséquent, la revue de code de la couche de données ne concerne pas seulement « est-ce bien écrit », mais aussi « ce code va réellement faire quoi dans la BDD ?».
La matrice de choix est utile tant que vous regardez le scénario d'en haut. Mais dans une PR, personne ne marque le code avec des mots comme « ici c'est un list-read via entity » ou « ici j'ai accidentellement créé un stale state ». Dans la revue, vous voyez d'abord le service, le mapper, saveAndFlush(), le parcours du graphe, et ensuite seulement vous faites le lien avec la matrice. La checklist est nécessaire précisément pour cela : reconnaître rapidement un red flag et comprendre quelle question poser.
La checklist fonctionne ici comme la liste de vérification pré-vol d'un pilote. Le pilote sait aussi voler sans papiers, mais les papiers protègent du facteur humain : fatigue, précipitation, « mais c'est évident ». En ORM, tout est compliqué par le fait qu'une partie du comportement est implicite : transactions, flush, proxies, cascades, lazy loading, versions. La checklist aide à transformer la revue de « opinions » en un processus reproductible : nous posons les bonnes questions, cherchons les red flags et, si nécessaire, les transformons en finding avec un risque clair.
2. Une bonne checklist ORM sans interdits
Une checklist pour le code Hibernate n'est pas une « liste noire d'annotations » ni une tentative d'interdire tout sauf select * from .... Sa tâche est plus simple et plus utile : transformer les axes d'audit en questions pour une PR spécifique. Chaque question doit mener à une décision d'ingénierie : soit nous nous assurons que tout va bien (et nous fusionnons tranquillement), soit nous identifions un risque et demandons des éclaircissements/modifications.
Il est utile de considérer un point de checklist comme une petite construction « signal → question → risque ». Le signal est un red flag, c'est-à-dire un motif reconnaissable dans le code. La question est ce que nous posons à l'auteur (ou à nous-mêmes) pour déterminer s'il y a une raison de faire ainsi. Le risque est ce qui se produira à l'exécution s'il n'y a pas de raison. Et ensuite, nous choisissons l'action : laisser tel quel, demander une correction, proposer une alternative de la matrice de décisions.
Ci-dessous se trouve une « carte » compacte des blocs qui composent généralement la checklist pour notre Commerce Persistence Lab.
| Bloc de la checklist | Sur quoi nous regardons dans le code | Risque typique si c'est « comme ça » |
|---|---|---|
| Transitions d'état | save(), saveAndFlush(), merge(), clear(), detach() | SQL superflus, flush prématuré, pièges de merge, graphe imprévisible |
| Limite de transaction | où se trouve @Transactional, où il n'y en a pas, et où il est trop large | LazyInitializationException, lectures incohérentes, transactions longues, SQL « flottant » |
| Fetch & modèle de lecture | findAll()+stream(), parcours de graphe, EAGER, toString() sur entity | N+1, overfetching, requêtes aléatoires, dirty checking lourd sur le chemin de lecture |
| Qualité du mapping | méthodes helper, mappedBy, cascade, orphanRemoval, equals/hashCode | FK non mis à jour comme attendu, delete/insert superflus, collections cassées, traîtrise des proxies |
| Concurrence | @Version, modes de verrouillage, sections critiques | perte de mise à jour, blocages, deadlock – pensée « ça viendra plus tard (non) » |
| Bulk & read-only | @Modifying bulk, hints/transactions read-only | persistence context obsolète, « nous avons mis à jour la BDD, mais l'objet pense différemment », effets étranges lors de la lecture |
Pour que cela ne reste pas une théorie, il est utile de garder à l'esprit un itinéraire de revue simple et pratique :
flowchart TD
A["Choisir le use case / hot path de la PR"] --> B["Comprendre : lecture ou écriture ?"]
B --> C["Parcourir les blocs de la checklist"]
C --> D["Identifier les red flags comme des questions"]
D --> E["Si le risque est réel : proposer une solution de la matrice"]
Remarquez : il n'y a pas d'étape « commencer immédiatement à se disputer à propos des annotations ». Nous comprenons d'abord le scénario, puis nous vérifions dans quelle mesure le code lui correspond.
Mais la checklist en elle-même n'est pas encore une preuve. Elle est utile jusqu'au moment où vous dites : « ça ressemble à N+1 », « ça ressemble à un flush superflu », « ça ressemble à un stale state ». Ensuite, ce soupçon doit être confirmé par les SQL, les compteurs et les tests, sinon la revue restera au niveau de l'intuition.
3. Transitions d'état : save, flush, merge
Dans ce bloc, nous vérifions que l'auteur de la PR comprend dans quel état vit l'entité et pourquoi il appelle exactement telle ou telle méthode. Hibernate n'est pas « j'ai appelé save() — c'est sauvegardé », mais plutôt une unité de travail : l'entité gérée (managed) change en mémoire, et le SQL est envoyé en BDD lors des moments de flush/commit. Par conséquent, toute action « superflue » autour de l'état est un drapeau rouge : elle cache presque toujours une incompréhension, ou corrige un symptôme au lieu de la cause.
Red flag : saveAndFlush() à l'intérieur d'une transaction « au cas où ».
C'est l'un des signaux les plus fréquents. saveAndFlush() ne fait pas que « sauvegarder », il force le flush immédiatement. Si ce n'est pas nécessaire pour la correction (par exemple, nous n'avons pas besoin que les données deviennent visibles en BDD avant la requête suivante), alors c'est un SQL superflu et une potentielle imprévisibilité.
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
@Service
class OrderAdminService {
@Transactional
public void changeStatus(PurchaseOrder order, OrderStatus status) {
// L'entité est managed dans la transaction : Hibernate verra les modifications lui-même (dirty checking)
order.setStatus(status);
// Red flag : on force le flush maintenant.
// Nécessaire seulement si plus loin dans la même méthode il y a une requête/contrainte
// qui doit voir les modifications avant le commit.
// orderRepository.saveAndFlush(order);
}
}
Dans la revue, une bonne question n'est pas « enlève ça », mais « pourquoi as-tu besoin d'un flush précoce ici ? y a-t-il une requête/contrainte qui nécessite une synchronisation avant la fin de la transaction ? ». S'il n'y a pas de réponse, la méthode est probablement superflue.
Red flag : save() après findById() et modification d'une entité managed.
Psychologiquement, on a envie de « confirmer la sauvegarde ». Mais Hibernate « tient » déjà l'entité : le dirty checking fera son travail. save() ici ne fait rien, ou (selon la nouveauté/l'identifiant) peut nous entraîner indirectement dans une sémantique de merge, ce qui est particulièrement désagréable sur les graphes.
import org.springframework.transaction.annotation.Transactional;
@Transactional
public void renameProduct(long id, String newName) {
// On charge l'entité : elle sera managed dans le cadre de la transaction
Product product = productRepository.findById(id).orElseThrow();
// Il suffit de modifier le champ : l'UPDATE sera envoyé au flush/commit automatiquement
product.setName(newName);
// Red flag : le plus souvent, opération superflue qui crée l'illusion « j'ai sauvegardé manuellement ».
// productRepository.save(product);
}
Red flag : « DTO → entity → save » pour le chemin de mise à jour.
Si vous voyez du code qui crée un nouvel objet Product, lui attribue un id, copie les champs du DTO et fait save(), c'est presque toujours une invitation à la fête du merge. Et le merge, comme nous le savons, adore les SELECT cachés, la copie d'état et les surprises sur les graphes.
Dans la revue, il faut demander : « pourquoi pas un find + mutation ? ». C'est généralement plus simple et plus prévisible.
import org.springframework.transaction.annotation.Transactional;
@Transactional
public void updateProduct(long id, String name) {
// Chemin de mise à jour prévisible : on trouve l'entité managed
Product product = productRepository.findById(id).orElseThrow();
// Et on modifie ponctuellement les champs nécessaires (sans merge et ses surprises)
product.setName(name);
}
Red flag : clear() / detach() sans raison très claire.
Ces méthodes ne sont pas une « optimisation », mais un changement des règles du jeu : vous désactivez manuellement la surveillance des objets par Hibernate. Cela peut être nécessaire dans des scénarios bulk ou dans des laboratoires spécifiques, mais dans un flux CRUD normal, sans explication, cela semble suspect.
Si vous voyez entityManager.clear() au milieu d'un service, posez une question simple : « quel problème de stale-state ou de mémoire a dicté cela ? ». Si la réponse ressemble à « pour que ça marche », c'est presque garanti être une dette d'architecture.
4. Limite de transaction et Session
Ensuite, nous vérifions que le code s'exécute bien dans un cadre transactionnel correct. Dans notre cours et dans le projet de base, open-in-view=false, donc on ne peut pas compter sur « eh bien, dans le contrôleur, il y avait encore une session ». Tout accès lazy en dehors d'une transaction soit échouera, soit vous obligera à activer OSIV et à cacher le problème (ce que nous ne faisons pas).
Red flag : une méthode de service qui effectue plusieurs opérations d'écriture sans @Transactional.
Parfois, l'auteur a « dispersé » la logique entre les repositories et espère que chaque save() est « atomique » en soi. Mais l'unité de travail doit être au niveau de l'opération métier.
import org.springframework.stereotype.Service;
@Service
class InventoryService {
public void reserve(long itemId, int qty) {
// Sans @Transactional : l'objet peut devenir detached immédiatement après la méthode du repository
InventoryItem item = inventoryRepository.findById(itemId).orElseThrow();
// Red flag : les modifications peuvent ne pas arriver en BDD, car il n'y a pas d'unité de travail ni de flush/commit
item.reserve(qty);
}
}
Lors de la revue, il est important de demander : « quelle est la limite de l'unité de travail ? où est @Transactional et quel mode (read-only ou write) ? ». Si la méthode est un chemin d'écriture, la transaction doit être explicite.
Red flag : self-invocation et illusion de transaction.
C'est un bug subtil qui a l'air « tout correct », mais la transaction ne s'ouvre pas, car Spring ne peut pas appliquer le proxy à un appel à l'intérieur du même objet.
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
@Service
class OrderService {
public void cancel(long id) {
// Red flag : l'appel d'une méthode à l'intérieur de la même classe contourne le proxy Spring,
// ce qui peut empêcher @Transactional de fonctionner
cancelTx(id);
}
@Transactional
void cancelTx(long id) {
// La transaction s'ouvrira garantie seulement si la méthode est appelée via le proxy
/* ... */
}
}
La question de la checklist ici est simple : « la méthode est-elle bien appelée via le proxy Spring ? ». Souvent, la solution est de déplacer la méthode transactionnelle dans un bean séparé ou de restructurer l'orchestration.
Red flag : transaction trop large sur le chemin de lecture.
Oui, une transaction est nécessaire même pour la lecture (pour la cohérence et le chargement lazy prévisible), mais « garder une transaction autour du monde entier » est également dangereux. Lors de la revue, il faut prêter attention au fait qu'à l'intérieur d'un scénario read-only, il ne se produit pas de modifications accidentelles d'entités managed. Si vous voyez @Transactional(readOnly=true), et à l'intérieur quelqu'un fait setName(), ce n'est plus seulement une question de « style », c'est un risque de comportement inattendu.
5. Fetching et modèle de lecture : motifs N+1
Ce bloc provoque souvent la surprise chez les débutants : « comment puis-je parler de N+1 si je n'ai pas encore regardé le SQL ? ». Spoiler : nous n'affirmons pas que N+1 est exactement présent, mais nous savons voir les motifs qui y mènent presque toujours. C'est comme voir dans le code while(true) sans break : théoriquement, ça peut être normal, mais il vaut mieux demander.
Red flag : findAll() + stream() + navigation dans les associations.
C'est le classique « cacheur de requêtes » : dans le code, une seule méthode, mais en BDD — toute une série de SELECT.
import java.util.List;
import org.springframework.transaction.annotation.Transactional;
@Transactional(readOnly = true)
public List<String> findOrderEmails() {
// Red flag : findAll charge les entités racines, puis nous naviguons dans les associations
return orderRepository.findAll().stream()
// Potentiel N+1 : getCustomer() peut être LAZY et déclencher un SELECT séparé pour chaque ligne
.map(o -> o.getCustomer().getEmail())
.toList();
}
Dans la revue, la question de la checklist est : « est-ce un cas d'utilisation de liste (list use case) ? avons-nous vraiment besoin d'entités ? pourquoi pas une projection ou un plan de fetch explicite ? ». Et notez : c'est une question sur la solution, pas « sur les goûts ».
L'une des réponses normales à ce red flag est la projection.
Par exemple, au lieu de charger les entités, on peut faire une requête qui retourne directement les valeurs nécessaires. Ce sera plus simple pour Hibernate et pour votre cerveau.
import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
interface PurchaseOrderRepository extends JpaRepository<PurchaseOrder, Long> {
// Projection : on retourne directement des données « plates » sans charger le graphe d'entités
@Query("select o.customer.email from PurchaseOrder o")
List<String> findAllCustomerEmails(); // Une seule requête comme attendu
}
Red flag : « modèle d'entité universel » pour les listes.
Si une PR ajoute une méthode du genre List<PurchaseOrder> findByStatus(...) et que dans le service/mapper on commence à extraire de la commande tout et n'importe quoi (client, lignes, montants, adresses), c'est presque toujours le signe que le modèle de lecture et le modèle d'écriture sont mélangés. Question de checklist : « quelle forme de sortie est nécessaire ? peut-on séparer le DTO de ligne de liste de la vue détaillée ? ».
Red flag : toString()/logging qui touche aux associations lazy.
Si vous voyez un toString() qui imprime items, addresses, categories — c'est une mine ORM. Au mieux, il y aura du SQL bruyant, au pire une exception si toString() est appelé en dehors de la transaction.
@Override
public String toString() {
// Red flag : l'accès à une collection lazy dans toString() peut :
// - déclencher soudainement des requêtes SQL (dans le logging !)
// - échouer avec LazyInitializationException en dehors de la transaction
return "PurchaseOrder{id=" + id + ", items=" + items + "}";
}
Question de checklist : « toString() est-il vraiment sûr ? ne touche-t-il pas aux collections lazy ? ». Dans un projet pédagogique, il est préférable de faire un toString() minimal : seulement id et quelques champs simples.
6. Qualité du mapping : associations, cascades, equals
Passons à ce qui casse souvent le runtime, même si les requêtes sont parfaitement écrites : le mapping. C'est comme les fondations d'une maison : à l'extérieur, tout est beau, mais si les fondations sont « sur la bonne parole », la maison commence à se fissurer au moment le plus inopportun. Hibernate y ajoute sa spécificité : le owning side gère la FK, la cascade reflète le cycle de vie, equals/hashCode interagit avec les proxies, et les collections ne sont pas « un simple ArrayList », mais des collections persistantes.
Red flag : une association bidirectionnelle n'est pas synchronisée par une méthode helper.
Si dans une PR vous voyez un accès direct à la collection order.getItems().add(item) — c'est une raison de s'inquiéter. Il est très facile d'oublier de définir le owning side, et alors la clé étrangère soit ne sera pas mise à jour, soit le sera « de travers ».
import java.util.ArrayList;
import java.util.List;
class PurchaseOrder {
private final List<OrderItem> items = new ArrayList<>();
public void addItem(OrderItem item) {
// On modifie la collection du côté parent (inverse/non owning dans le mapping typique)
items.add(item);
// Important : on synchronise le owning side, sinon la FK pourrait ne pas être mise à jour comme attendu
item.setOrder(this);
}
}
Question de checklist : « par quoi l'association est-elle gérée : par un helper ou par la collection nue ? ». Dans notre projet, la bonne réponse est presque toujours : par un helper.
Red flag : cascade « ALL » sur des associations partagées.
Si PurchaseOrder contient une référence à Customer, alors Customer n'est certainement pas une entité enfant de la commande. Une cascade sur ManyToOne vers Customer conduit facilement à des conséquences inattendues : des persist inopinés à « oh, il semble que nous ayons supprimé le client avec la commande ».
import jakarta.persistence.CascadeType;
import jakarta.persistence.ManyToOne;
class PurchaseOrder {
// Red flag : Customer est une entité partagée. CascadeType.ALL peut entraîner :
// - un persist/merge accidentel sur le client
// - et dans les pires configurations, même un remove (ce qui n'est pas du tout ce que nous voulons)
@ManyToOne(cascade = CascadeType.ALL)
private Customer customer;
}
Lors de la revue, nous demandons : « quel est le cycle de vie ? l'enfant doit-il vivre et mourir avec le parent ? ». C'est une excellente question car elle oblige à parler du modèle métier, et non de la magie des annotations.
Red flag : equals() via getClass() (piège du proxy).
Hibernate peut fournir une classe proxy, et la comparaison getClass() casse l'égalité. Ce n'est pas de la « théorie », c'est une vraie raison de bugs étranges dans Set, le orphan removal et la comparaison d'entités à l'intérieur du persistence context.
import org.hibernate.Hibernate;
@Override
public boolean equals(Object o) {
if (this == o) return true;
// Important : la comparaison via Hibernate.getClass prend en compte les classes proxy
if (o == null || Hibernate.getClass(this) != Hibernate.getClass(o)) return false;
Product other = (Product) o;
// Stratégie de base : comparaison par id, mais seulement s'il a déjà été attribué
return id != null && id.equals(other.id);
}
Question de checklist : « l'égalité prend-elle en compte la présence de proxy ? ne dépend-elle pas de champs mutables ? ». Et oui, c'est le moment où @Data de Lombok devient votre ennemi, pas votre ami.
7. Concurrence : versions et verrous
La concurrence dans la couche de persistance n'est pas des « mathématiques supérieures », c'est une réalité normale. Dans notre projet, le chemin d'écriture contesté le plus évident est l'inventaire : plusieurs opérations peuvent réserver le même produit. Par conséquent, la checklist doit répondre rapidement à la question : sommes-nous protégés contre la perte de mise à jour et ne transformons-nous pas la BDD en champ de bataille de verrous ?
Red flag : chemin d'écriture sur des données contestées sans @Version.
Si une PR modifie availableQty/reservedQty, et que l'entité n'a pas de version, vous obtiendrez presque à coup sûr une perte silencieuse de mises à jour lors du travail parallèle.
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Version;
@Entity
class InventoryItem {
@Id
private Long id;
// Optimistic locking : Hibernate ajoutera une vérification de version lors de l'UPDATE
@Version
private long version;
}
Question de checklist : « y a-t-il un versioning là où des mises à jour parallèles sont possibles ? ».
Red flag : verrou pessimiste sans conscience du coût et de la limite.
Le verrouillage pessimiste est nécessaire, mais c'est un outil lourd : il doit être dans une transaction courte et avec une politique d'attente claire. Si une PR ajoute PESSIMISTIC_WRITE « pour être sûr », demandons : « pourquoi l'optimiste est-il insuffisant ? à quelle fréquence les conflits se produisent-ils ? ».
import java.util.Optional;
import jakarta.persistence.LockModeType;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Lock;
interface InventoryItemRepository extends JpaRepository<InventoryItem, Long> {
// Verrou pessimiste : on verrouille la ligne en BDD (coûteux, mais parfois nécessaire)
@Lock(LockModeType.PESSIMISTIC_WRITE)
Optional<InventoryItem> findLockedById(Long id); // Red flag s'il n'y a pas de raison
}
Red flag : actions longues à l'intérieur d'une section verrouillée.
Si vous voyez qu'après avoir pris un verrou à l'intérieur d'une transaction, on fait « quelque chose de long » (calculs complexes, attentes, grandes boucles), c'est une raison de s'arrêter. Même si c'est un projet d'apprentissage, l'habitude est dangereuse.
import org.springframework.transaction.annotation.Transactional;
@Transactional
public void reserve(long itemId, int qty) {
// On a pris le verrou : la section critique a commencé
InventoryItem item = inventoryRepository.findLockedById(itemId).orElseThrow();
// Red flag : une action longue sous verrou augmente les risques de contention/deadlock
slowBusinessLogic();
// Il est préférable de ne garder sous verrou que la partie minimale nécessaire
item.reserve(qty);
}
Lors de la revue, la question : « peut-on réduire la section critique ? ». Souvent, la réponse est oui.
8. Opérations bulk et read-only : stale state
Ce bloc est nécessaire car les mises à jour/suppressions bulk constituent une « sortie consciente du modèle d'entité », et elles ont un effet secondaire : le persistence context n'est pas synchronisé automatiquement. Même si le code ressemble à « nous avons tout mis à jour », les objets managed peuvent rester obsolètes. Et ce n'est pas un bug de Hibernate — c'est le prix de l'approche bulk.
Red flag : @Modifying bulk update et poursuite du travail avec des entités précédemment chargées.
Si un service fait d'abord un find(), puis une mise à jour bulk, puis lit les champs de l'objet managed — c'est pratiquement un manuel du persistence context obsolète.
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
interface ProductRepository {
// Bulk update : on contourne le persistence context et le dirty checking
@Modifying
@Query("update Product p set p.status = :status where p.deleted = false")
int changeStatusForCatalog(ProductStatus status); // Après cela, les entités managed peuvent devenir stale
}
La question de checklist pour l'endroit où le bulk est utilisé : « que se passe-t-il avec le persistence context après le bulk ? est-il vidé ? rechargeons-nous les données ? ». Ici, il est important que nous ne demandions pas « comment faire correctement », mais « comment contrôles-tu cela ? ».
Red flag : transaction read-only, mais à l'intérieur on mute une entité.
Oui, cela semble être « qui fait ça ? », mais dans la vraie vie, cela arrive plus souvent qu'on ne le souhaite. Une personne a mis readOnly=true comme « optimisation », puis a ajouté setX() et a oublié.
import org.springframework.transaction.annotation.Transactional;
@Transactional(readOnly = true)
public void accidentalWrite(long productId) {
// Transaction read-only : on suppose « lecture seule »
Product p = productRepository.findById(productId).orElseThrow();
// Red flag : mutation à l'intérieur d'un scénario read-only crée des surprises
p.setName("Oops");
}
Dans la revue, la question : « est-ce vraiment un cas d'utilisation read-only ? ». Si c'est une écriture, il faut honnêtement faire une transaction d'écriture et ne pas masquer.
9. Commentaires constructifs de revue ORM
La checklist la plus puissante peut devenir un outil inutile si elle est utilisée comme une massue. Surtout dans le domaine d'Hibernate, où chacun a ses traumatismes : quelqu'un a vu un LazyInitializationException et a maintenant peur du mot LAZY, quelqu'un a vécu N+1 et met maintenant JOIN FETCH même pour la lecture d'un répertoire des « villes de Mésopotamie » (y compris les villes qui n'existent pas encore). Par conséquent, la forme est importante.
Un bon commentaire de revue ORM ressemble généralement à une question + raison + suggestion. La question rend la discussion respectueuse et vérifie le contexte (peut-être que saveAndFlush() est vraiment nécessaire). La raison explique le risque (timing du flush, stale state, N+1). La suggestion donne une issue (projection, EntityGraph, find + mutate, @Version, clear après bulk). Et il est utile de suggérer que la solution doit être vérifiable, mais sans entrer dans les détails : « vérifions par le comportement », plutôt que « j'ai dit ».
Voici un petit tableau de ce à quoi cela pourrait ressembler :
| Ce qu'il ne faut pas écrire | Ce qu'il est préférable d'écrire |
|---|---|
| « Pourquoi saveAndFlush() ici ? Enlève-le. » | « Je vois saveAndFlush(). Avons-nous besoin d'un flush avant la fin de la méthode ? Sinon, on peut l'enlever et compter sur le dirty checking au commit — il y aura moins de SQL imprévu. » |
| « Encore du findAll()… » | « findAll() + navigation dans les associations ressemble à un candidat pour N+1. Est-ce un cas d'utilisation de liste (list use case) ? Peut-être qu'une projection/un plan de fetch explicite serait mieux adapté au scénario ? » |
| « CascadeType.ALL — c'est le mal. » | « Ici, CascadeType.ALL sur ManyToOne. Customer est une entité partagée, la cascade peut donner des effets de bord inattendus. Quel cycle de vie voulais-tu exprimer ? » |
10. Erreurs typiques lors de l'utilisation d'une checklist ORM
Erreur n°1 : Transformer la checklist en religion au lieu d'un outil.
Lorsqu'un développeur voit saveAndFlush() et écrit automatiquement « interdit », il cesse de réfléchir aux raisons. Et parfois, il y a une raison : par exemple, il est nécessaire que les contraintes de la BDD s'appliquent avant la requête suivante, ou d'obtenir l'id avec une certaine stratégie. La checklist doit forcer à poser des questions, pas à éteindre le cerveau.
Erreur n°2 : Vérifier uniquement les repositories et ignorer le scénario du service.
Les problèmes Hibernate ne vivent pas dans les « méthodes de repository en elles-mêmes », mais dans la combinaison : cas d'utilisation du service → transaction → lecture/écriture → navigation dans le graphe. Si la revue se résume aux fichiers Repository, vous passerez à côté de la moitié des risques réels, en particulier les transactions trop larges et les parcours lazy cachés.
Erreur n°3 : Ne détecter les drapeaux rouges que dans les annotations des entités.
Bien sûr, EAGER, CascadeType.ALL et les mappedBy étranges sont importants. Mais beaucoup de drapeaux rouges vivent dans le « code Java ordinaire » : stream().map(o -> o.getCustomer()...), toString() avec des collections, des mappers qui parcourent tout le graphe, une mise à jour bulk sans nettoyage du contexte. La checklist doit regarder le code dans son ensemble.
Erreur n°4 : Mélanger les exigences de lecture et d'écriture dans le même sac.
Pour le chemin de lecture, il est normal de vouloir une projection et un minimum d'entités managed. Pour le chemin d'écriture, il est normal de vouloir une entité racine managed et un find + mutation. Si dans la revue nous « assassinons » à la fois les lectures et les écritures avec le même ensemble de griefs, le chaos apparaît : l'auteur ne comprend pas ce qu'on attend de lui, et la couche de données devient incohérente.
Erreur n°5 : Oublier qu'un drapeau rouge n'est pas encore un bug, mais c'est presque toujours une dette.
Parfois, une PR avec un red flag « fonctionne » effectivement. Mais un red flag signifie que le comportement est soit coûteux, soit fragile, soit imprévisible. Si vous le laissez sans discussion, vous ne fusionnez pas seulement du code — vous fusionnez de futures gardes de nuit avec des logs SQL. Hibernate, bien sûr, est un outil puissant, mais il n'est pas obligé d'être votre interlocuteur nocturne.
GO TO FULL VERSION