ORM checklist: red flags в Hibernate

1. Checklist для persistence layer

Если в обычном Java-коде мы часто видим причину и следствие почти напрямую (вызвал метод — получил результат), то Hibernate любит сюрпризы: изменил поле — UPDATE появится позже, а не «вот сейчас»; взял order.getCustomer() — и внезапно уехал SQL; сделал bulk update — и persistence context тихо остался жить в своей параллельной реальности. Поэтому code review слоя данных — это не только «правильно ли написано», но и «что этот код на самом деле сделает в БД».

Матрица выбора полезна, пока вы смотрите на сценарий сверху. Но в PR никто не подписывает код словами «тут list-read через entity» или «тут я случайно создал stale state». В ревью вы сначала видите service, mapper, saveAndFlush(), обход графа и только потом соотносите это с матрицей. Checklist нужен именно для этого: быстро распознать red flag и понять, какой вопрос задать.

Checklist здесь работает как предполётный список пилота. Пилот тоже умеет летать без бумажки, но бумажка спасает от человеческого фактора: усталость, спешка, «да это же очевидно». В ORM всё усложняется тем, что часть поведения неявная: транзакции, flush, прокси, каскады, lazy loading, версии. Чеклист помогает превращать ревью из «мнений» в повторяемый процесс: мы задаём правильные вопросы, ищем red flags и, если нужно, превращаем их в finding с понятным риском.

2. Хороший ORM-checklist без запретов

Checklist для Hibernate-кода — не «чёрный список аннотаций» и не попытка запретить всё, кроме select * from .... Его задача проще и полезнее: перевести оси аудита в вопросы к конкретному PR. Каждый вопрос должен вести к инженерному решению: либо мы убеждаемся, что всё ок (и спокойно мёрджим), либо фиксируем риск и просим уточнить/переделать.

Полезно мыслить пунктом чеклиста как маленькой конструкцией «сигнал → вопрос → риск». Сигнал — это red flag, то есть узнаваемый паттерн в коде. Вопрос — то, что мы задаём автору (или себе), чтобы выяснить, есть ли причина так делать. Риск — то, что случится в рантайме, если причины нет. И уже после этого мы выбираем действие: оставить как есть, попросить правку, предложить альтернативу из матрицы решений.

Ниже — компактная «карта» блоков, из которых обычно состоит checklist для нашего Commerce Persistence Lab.

Чтобы это не оставалось теорией, полезно держать в голове простой рабочий маршрут ревью:

flowchart TD
    A["Выбираем use case / hot path PR"] --> B["Понимаем: read или write?"]
    B --> C["Проходим блоки checklist"]
    C --> D["Фиксируем red flags как вопросы"]
    D --> E["Если риск реальный: предлагаем решение из матрицы"]

Заметьте: здесь нет шага «сразу спорим про аннотации». Мы сначала понимаем сценарий и только потом смотрим, насколько код под него подходит.

Но checklist сам по себе — это ещё не доказательство. Он полезен ровно до того места, где вы сказали: «похоже на N+1», «похоже на лишний flush», «похоже на stale state». Дальше это подозрение нужно подтверждать по SQL, счётчикам и тестам, иначе review останется на уровне интуиции.

3. State transitions: save, flush, merge

В этом блоке мы проверяем, что автор PR понимает, в каком состоянии живёт сущность и почему именно он вызывает те или иные методы. Hibernate не про «вызвал save() — сохранилось», а про unit of work: managed-сущность меняется в памяти, а SQL уезжает в БД в моменты flush/commit. Поэтому любое «лишнее» действие вокруг состояния — это красный флажок: оно почти всегда либо скрывает непонимание, либо чинит симптом вместо причины.

Red flag: saveAndFlush() внутри транзакции «на всякий случай».
Это один из самых частых сигналов. saveAndFlush() не просто «сохранить», он ещё и принуждает flush прямо сейчас. Если это не нужно для корректности (например, нам не надо, чтобы данные стали видны в БД до следующего запроса), то это лишний SQL и потенциальная непредсказуемость.

import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
class OrderAdminService {

    @Transactional
    public void changeStatus(PurchaseOrder order, OrderStatus status) {
        // Сущность managed внутри транзакции: Hibernate сам увидит изменения (dirty checking)
        order.setStatus(status);

        // Red flag: форсируем flush прямо сейчас.
        // Нужно только если дальше в этом же методе есть запрос/constraint,
        // которому важно видеть изменения до commit.
        // orderRepository.saveAndFlush(order);
    }
}

В ревью хороший вопрос звучит не как «убери это», а как «зачем тебе ранний flush именно здесь? есть ли запрос/constraint, который требует синхронизации до конца транзакции?». Если ответа нет — вероятнее всего, метод лишний.

Red flag: save() после findById() и изменения managed-сущности.
Чисто психологически хочется «закрепить сохранение». Но Hibernate уже «держит» сущность: dirty checking сделает свою работу. save() тут либо ничего не даёт, либо (в зависимости от newness/идентификатора) может косвенно увести нас в merge-семантику, что особенно неприятно на графах.

import org.springframework.transaction.annotation.Transactional;

@Transactional
public void renameProduct(long id, String newName) {
    // Загружаем entity: дальше она будет managed в рамках транзакции
    Product product = productRepository.findById(id).orElseThrow();

    // Достаточно изменить поле: UPDATE уйдёт на flush/commit автоматически
    product.setName(newName);

    // Red flag: чаще всего лишняя операция, создающая иллюзию «я сохранил руками».
    // productRepository.save(product);
}

Red flag: «DTO → entity → save» для update-path.
Если вы видите код, где создают новый объект Product, ставят ему id, копируют поля из DTO и делают save(), то это почти всегда приглашение на merge-вечеринку. А merge, как мы уже знаем, любит скрытые SELECT, копирование состояния и сюрпризы на графах.

В ревью здесь стоит спрашивать: «почему не find + mutate?». Обычно это и проще, и предсказуемее.

import org.springframework.transaction.annotation.Transactional;

@Transactional
public void updateProduct(long id, String name) {
    // Предсказуемый update-path: нашли managed entity
    Product product = productRepository.findById(id).orElseThrow();

    // И точечно поменяли нужные поля (без merge и его сюрпризов)
    product.setName(name);
}

Red flag: clear() / detach() без очень понятной причины.
Эти методы — не «оптимизация», а изменение правил игры: вы вручную выключаете наблюдение Hibernate за объектами. Это бывает нужно в bulk-сценариях или в специфических лабораториях, но в обычном CRUD-потоке без объяснения выглядит подозрительно.

Если вы видите entityManager.clear() посреди сервиса, задайте простой вопрос: «какой stale-state или memory-проблемой это продиктовано?». Если ответ звучит как «чтобы работало» — это почти гарантированно долг в архитектуре.

4. Транзакционная граница и Session

Дальше мы проверяем, что код вообще выполняется в корректной транзакционной рамке. В нашем курсе и в проекте baseline — open-in-view=false, а значит, надеяться на «ну в контроллере ещё была сессия» нельзя. Любой lazy доступ вне транзакции либо упадёт, либо заставит вас включить OSIV и спрятать проблему (а это мы не делаем).

Red flag: сервисный метод, который делает несколько операций записи без @Transactional.
Иногда автор «раскидал» логику по репозиториям и надеется, что каждый save() как-то сам по себе «атомарен». Но unit of work должен быть на уровне бизнес-операции.

import org.springframework.stereotype.Service;

@Service
class InventoryService {

    public void reserve(long itemId, int qty) {
        // Без @Transactional: объект может стать detached сразу после метода репозитория
        InventoryItem item = inventoryRepository.findById(itemId).orElseThrow();

        // Red flag: изменения могут не попасть в БД, потому что нет unit of work и flush/commit
        item.reserve(qty);
    }
}

На ревью здесь важно спрашивать: «какая граница unit of work? где @Transactional и какой режим (read-only или write)?». Если метод — write-path, транзакция должна быть явной.

Red flag: self-invocation и иллюзия транзакции.
Это тонкий баг, который выглядит как «всё правильно», но транзакция не открывается, потому что Spring не может применить прокси к вызову внутри того же объекта.

import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
class OrderService {

    public void cancel(long id) {
        // Red flag: вызов метода внутри того же класса обходит Spring-прокси,
        // из-за чего @Transactional может не сработать
        cancelTx(id);
    }

    @Transactional
    void cancelTx(long id) {
        // Транзакция гарантированно откроется только если метод будет вызван через прокси
        /* ... */
    }
}

Чеклистовый вопрос здесь простой: «точно ли метод вызывается через прокси Spring?». Часто решение — вынести транзакционный метод в отдельный bean или перестроить orchestration.

Red flag: слишком широкая транзакция на read-path.
Да, транзакция нужна и для чтения (ради консистентности и предсказуемой lazy-загрузки), но «держать транзакцию вокруг всего мира» тоже опасно. На ревью стоит обращать внимание на то, что внутри read-only сценария не происходит случайных изменений managed-сущностей. Если вы видите @Transactional(readOnly=true), а внутри кто-то делает setName(), это уже не просто «стиль», это риск неожиданного поведения.

5. Fetching и read-model: паттерны N+1

Этот блок часто вызывает у новичков удивление: «как я могу говорить про N+1, если я ещё не смотрел SQL?». Спойлер: мы не утверждаем, что N+1 точно есть, но мы умеем видеть паттерны, которые почти всегда к нему приводят. Это как увидеть в коде while(true) без break: теоретически может быть нормально, но лучше спросить.

Red flag: findAll() + stream() + навигация по связям.
Это классический «прятатель запросов»: в коде один метод, а в БД — целая серия SELECT.

import java.util.List;
import org.springframework.transaction.annotation.Transactional;

@Transactional(readOnly = true)
public List<String> findOrderEmails() {
    // Red flag: findAll грузит root-сущности, а дальше мы идём по связям
    return orderRepository.findAll().stream()
            // Потенциальный N+1: getCustomer() может быть LAZY и дёргать отдельный SELECT на каждую строку
            .map(o -> o.getCustomer().getEmail())
            .toList();
}

В ревью чеклистовый вопрос звучит так: «это list-use case? точно ли нам нужны entity? почему не projection или явный fetch-plan?». И обратите внимание: это вопрос про решение, а не «про вкусы».

Один из нормальных ответов на этот red flag — projection.
Например, вместо entity-loading можно сделать query, которая сразу вернёт нужные значения. Это будет проще и для Hibernate, и для вашего мозга.

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: возвращаем сразу «плоские» данные без загрузки графа сущностей
    @Query("select o.customer.email from PurchaseOrder o")
    List<String> findAllCustomerEmails(); // Ожидаемо один запрос
}

Red flag: “универсальная entity-модель” для списков.
Если PR добавляет метод вида List<PurchaseOrder> findByStatus(...) и затем в сервисе/маппере начинают вытаскивать из заказа всё подряд (клиент, позиции, суммы, адреса), это почти всегда сигнал, что read-model и write-model смешали. Чеклистовый вопрос: «какая форма выдачи нужна? можно ли отделить list-row DTO от detail-view?».

Red flag: toString()/логирование, которое трогает lazy-связи.
Если вы видите toString(), который печатает items, addresses, categories — это ORM-мина. В лучшем случае будет шумный SQL, в худшем — исключение, если toString() зовут вне транзакции.

@Override
public String toString() {
    // Red flag: обращение к lazy-коллекции в toString() может:
    // - внезапно инициировать SQL-запросы (внутри логирования!)
    // - упасть с LazyInitializationException вне транзакции
    return "PurchaseOrder{id=" + id + ", items=" + items + "}";
}

Чеклистовый вопрос: «точно ли toString() безопасен? не трогает ли он lazy-коллекции?». В учебном проекте лучше делать toString() минимальным: только id и пара простых полей.

6. Mapping quality: связи, каскады, equals

Переходим к тому, что часто ломает рантайм, даже если запросы написаны идеально: маппинг. Это как фундамент дома: снаружи всё красиво, но если фундамент «на честном слове», дом начинает трескаться в самый неудобный момент. Hibernate добавляет к этому свою специфику: owning side управляет FK, cascade отражает lifecycle, equals/hashCode взаимодействует с proxy, а коллекции — это не «просто ArrayList», а persistent collections.

Red flag: двунаправленная связь не синхронизируется helper-методом.
Если в PR вы видите прямой доступ к коллекции order.getItems().add(item) — это повод насторожиться. Очень легко забыть выставить owning side, и тогда внешний ключ либо не обновится, либо обновится «не так».

import java.util.ArrayList;
import java.util.List;

class PurchaseOrder {
    private final List<OrderItem> items = new ArrayList<>();

    public void addItem(OrderItem item) {
        // Меняем коллекцию на стороне parent (inverse/не owning в типичном маппинге)
        items.add(item);

        // Важно: синхронизируем owning side, иначе FK может не обновиться как ожидаем
        item.setOrder(this);
    }
}

Чеклистовый вопрос: «через что управляется связь: через helper или через голую коллекцию?». В нашем проекте правильный ответ почти всегда: через helper.

Red flag: cascade “ALL” на shared-ассоциации.
Если PurchaseOrder содержит ссылку на Customer, то Customer — точно не child сущность заказа. Cascade на ManyToOne к Customer легко приводит к случайным последствиям: от неожиданных persist до “ой, кажется, мы удалили клиента вместе с заказом”.

import jakarta.persistence.CascadeType;
import jakarta.persistence.ManyToOne;

class PurchaseOrder {

    // Red flag: Customer — shared сущность. CascadeType.ALL может протащить:
    // - случайный persist/merge на клиента
    // - а в худших конфигурациях и remove (что вообще не то, чего мы хотим)
    @ManyToOne(cascade = CascadeType.ALL)
    private Customer customer;
}

В ревью спрашиваем: «какой lifecycle? должен ли child жить и умирать вместе с parent?». Это отличный вопрос, потому что заставляет говорить о доменной модели, а не о магии аннотаций.

Red flag: equals() через getClass() (proxy-подстава).
Hibernate может подсовывать proxy-класс, и сравнение getClass() ломает equality. Это не «теория», это реальная причина странных багов в Set, orphan removal и в сравнении сущностей внутри persistence context.

import org.hibernate.Hibernate;

@Override
public boolean equals(Object o) {
    if (this == o) return true;

    // Важно: сравнение через Hibernate.getClass учитывает proxy-классы
    if (o == null || Hibernate.getClass(this) != Hibernate.getClass(o)) return false;

    Product other = (Product) o;

    // Базовая стратегия: сравнение по id, но только если он уже назначен
    return id != null && id.equals(other.id);
}

Чеклистовый вопрос: «учитывает ли equality наличие proxy? не зависит ли она от mutable-полей?». И да, это тот самый момент, когда @Data от Lombok становится вашим врагом, а не другом.

7. Конкурентность: версии и блокировки

Конкурентность в persistence layer — это не «высшая математика», а нормальная реальность. У нас в проекте самый очевидный contested write-path — инвентарь: несколько операций могут резервировать один и тот же товар. Поэтому чеклист должен быстро отвечать на вопрос: защищены ли мы от lost update и не превращаем ли БД в поле боя блокировок.

Red flag: write-path по contested данным без @Version.
Если PR меняет availableQty/reservedQty, а в entity нет версии, вы почти гарантированно получите тихую потерю обновлений при параллельной работе.

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Version;

@Entity
class InventoryItem {
    @Id
    private Long id;

    // Optimistic locking: Hibernate добавит проверку версии при UPDATE
    @Version
    private long version;
}

Чеклистовый вопрос: «есть ли versioning там, где возможны параллельные обновления?».

Red flag: pessimistic lock без осознания цены и границы.
Pessimistic locking нужен, но это тяжёлый инструмент: он должен быть в короткой транзакции и с понятной политикой ожидания. Если PR добавляет PESSIMISTIC_WRITE «чтобы наверняка», спрашиваем: «почему optimistic недостаточен? насколько часты конфликты?».

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> {

    // Pessimistic lock: блокируем строку в БД (дорого, но иногда необходимо)
    @Lock(LockModeType.PESSIMISTIC_WRITE)
    Optional<InventoryItem> findLockedById(Long id); // Red flag, если нет причины
}

Red flag: долгие действия внутри lock-секции.
Если вы видите, что после взятия lock внутри транзакции делается «что-то долгое» (сложные вычисления, ожидания, большие циклы), это повод остановиться. Даже если это учебный проект, привычка опасная.

import org.springframework.transaction.annotation.Transactional;

@Transactional
public void reserve(long itemId, int qty) {
    // Взяли блокировку: критическая секция началась
    InventoryItem item = inventoryRepository.findLockedById(itemId).orElseThrow();

    // Red flag: долгое действие под блокировкой повышает шанс contention/deadlock
    slowBusinessLogic();

    // Лучше держать под lock только минимально необходимую часть
    item.reserve(qty);
}

На ревью вопрос: «можно ли сократить критическую секцию?». Часто ответ — да.

8. Bulk-операции и read-only: stale state

Этот блок нужен, потому что bulk update/delete — это «осознанный выход за entity-модель», и у него есть побочный эффект: persistence context не синхронизируется автоматически. Даже если код выглядит как «мы всё обновили», managed-объекты могут оставаться старыми. И это не баг Hibernate — это цена bulk подхода.

Red flag: @Modifying bulk update и продолжение работы с ранее загруженными entity.
Если сервис сначала делает find(), потом bulk update, а затем читает поля из managed-объекта — это почти учебник по stale persistence context.

import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;

interface ProductRepository {
    // Bulk update: минуем persistence context и dirty checking
    @Modifying
    @Query("update Product p set p.status = :status where p.deleted = false")
    int changeStatusForCatalog(ProductStatus status); // После этого managed-сущности могут стать stale
}

Чеклистовый вопрос к месту использования bulk: «что происходит с persistence context после bulk? он очищается? мы перезагружаем данные?». Здесь важно, что мы спрашиваем не “как правильно”, а “как ты это контролируешь”.

Red flag: read-only транзакция, но внутри мутируют entity.
Да, это звучит как «ну кто так делает», но в реальной жизни это происходит чаще, чем хотелось бы. Человек поставил readOnly=true как “оптимизацию”, а потом добавил setX() и забыл.

import org.springframework.transaction.annotation.Transactional;

@Transactional(readOnly = true)
public void accidentalWrite(long productId) {
    // Read-only транзакция: предполагаем «только чтение»
    Product p = productRepository.findById(productId).orElseThrow();

    // Red flag: мутация внутри read-only сценария создаёт неожиданности
    p.setName("Oops");
}

В ревью вопрос: «это точно read-only use case?». Если это запись — надо честно сделать write transaction и не маскировать.

9. Конструктивные ORM-review комментарии

Самый сильный чеклист может превратиться в бесполезный инструмент, если использовать его как дубинку. Особенно в Hibernate-теме, где у каждого есть травмы: кто-то однажды увидел LazyInitializationException и теперь боится слова LAZY, кто-то пережил N+1 и теперь JOIN FETCH ставит даже на чтение справочника «городов Месопотамии » (включая города, которых ещё не существует). Поэтому важна форма.

Хороший ORM-review комментарий обычно выглядит как вопрос + причина + предложение. Вопрос делает обсуждение уважительным и проверяет контекст (вдруг saveAndFlush() реально нужен). Причина объясняет риск (flush timing, stale state, N+1). Предложение даёт выход (projection, EntityGraph, find + mutate, @Version, clear после bulk). И отдельно полезно намекнуть, что решение должно быть проверяемым, но без ухода в детали: «давай убедимся по поведению», а не «я так сказал».

Вот маленькая табличка, как это может звучать:

10. Типичные ошибки при использовании ORM-checklist

Ошибка №1: превращать чеклист в религию вместо инструмента.
Когда разработчик видит saveAndFlush() и автоматически пишет «запрещено», он перестаёт думать о причинах. А иногда причина есть: например, нужно, чтобы ограничения БД сработали до следующего запроса, или нужно получить id при определённой стратегии. Чеклист должен заставлять задавать вопросы, а не выключать мозг.

Ошибка №2: проверять только репозитории и игнорировать сервисный сценарий.
Hibernate-проблемы живут не в «методах репозитория сами по себе», а в связке: сервисный use case → транзакция → чтение/запись → навигация по графу. Если ревью сводится к Repository-файлам, вы пропустите половину реальных рисков, особенно overscoped transactions и hidden lazy traversal.

Ошибка №3: ловить красные флажки только в entity-аннотациях.
Конечно, EAGER, CascadeType.ALL и странные mappedBy — важны. Но очень много красных флажков живут в «обычном Java-коде»: stream().map(o -> o.getCustomer()...), toString() с коллекциями, мапперы, которые обходят весь граф, bulk update без очистки контекста. Чеклист должен смотреть на код целиком.

Ошибка №4: смешивать read и write требования в одну кашу.
Для read-path нормально хотеть projection и минимум managed-сущностей. Для write-path нормально хотеть managed root-entity и find + mutate. Если в ревью мы одним и тем же набором претензий «мочим» и чтения, и записи, появляется хаос: автор не понимает, что именно от него хотят, а слой данных становится непоследовательным.

Ошибка №5: забывать, что красный флажок — это ещё не баг, но почти всегда долг.
Иногда PR с red flag действительно «работает». Но red flag означает, что поведение либо дорогое, либо хрупкое, либо непредсказуемое. Если вы оставляете его без обсуждения, вы не просто мёрджите код — вы мёрджите будущие ночные дежурства с логами SQL. Hibernate, конечно, мощный инструмент, но он не обязан быть вашим ночным собеседником.