JavaRush /Курсы /Claude code /Архитектурный refactoring и приёмка PR

Архитектурный refactoring и приёмка PR

Claude code
20 уровень , 4 лекция
Открыта

1. Архитектурный refactor — всё ещё refactor

Правило здесь не меняется, как бы солидно ни звучало слово «архитектурный»: даже архитектурный refactoring остаётся refactoring, пока вы правите внутреннее устройство и не трогаете наблюдаемое снаружи поведение. Весь вопрос — в одном: что увидит внешний потребитель? А это не только пользователь в браузере. Это REST-клиент, соседний модуль, подписчик на событие, тест, проверяющий публичный контракт, другой сервис, которому приходит ваше сообщение. Их поведение прежнее — вы на территории refactor.

Если перенести это на Commerce OS, картина становится нагляднее: вы можете вынести публикацию события из OrderService в отдельный класс, разбить длинный метод, провести границу между бизнес-логикой и инфраструктурой. Но поменяли JSON-ответ контроллера, формат события, тему Kafka, зависимость в build.gradle или бизнес-правило скидки — вышли за пределы задачи.

Нагляднее всего это видно в такой таблице:

Изменение Это refactoring? Почему
Вынести публикацию события в OrderEventPublisher Да Внешний контракт тот же, меняется только структура
Переименовать processOrderV2 в finalizeOrder внутри модуля Да Имя стало понятнее, поведение не изменилось
Добавить поле в REST-ответ /api/orders/{id} Нет Меняется внешний контракт API
Обновить spring-kafka или spring-boot Нет Это уже изменение зависимостей, не refactor
Исправить неверное условие скидки Нет Это bugfix, то есть поведение меняется
Переименовать topic orders.finalized в orders.done Нет Внешние подписчики увидят другое поведение

Если говорить совсем по-человечески, архитектурный refactor — это не «сделать проект современным», а «сделать одну границу чище, не трогая договорённости с внешним миром». Менее героически. Зато безопаснее.

Архитектурный refactoring остаётся refactoring ровно до тех пор, пока внешнее наблюдаемое поведение не изменилось.

2. Commerce OS: OrderService знает слишком много

Самый понятный способ почувствовать архитектурный refactor — не читать определение, а посмотреть на класс, который «вроде работает», но уже слегка подозрительно похож на сотрудника, выполняющего обязанности пяти отделов сразу. В Commerce OS такой кандидат — OrderService: он и завершает заказ, и сам думает про публикацию события. Внешний flow не трогаем — чистим границу ответственности.

import org.springframework.stereotype.Service;

@Service
public class OrderService {

    public void completeOrderFinalization(Order order) {
        order.markFinalized();
        orderRepository.save(order);
        kafkaTemplate.send("orders.finalized", new OrderFinalizedEvent(order.getId()));
        // Бизнес-логика и публикация события смешаны в одном месте
    }
}

На первый взгляд всё даже прилично: заказ завершили, сохранили, событие отправили. Но проблема здесь не в том, что код не работает, а в том, что OrderService одновременно знает про состояние заказа, про сохранение в репозиторий и про механизм доставки события наружу. Класс держит в голове бизнес-правило, репозиторий, Kafka и, кажется, ещё расписание электричек — сигнал сузить его кругозор.

Посмотрим на более чистую границу. Сначала выносим публикацию события в отдельный класс:

import org.springframework.stereotype.Component;

@Component
public class OrderEventPublisher {

    public void orderFinalized(Order order) {
        kafkaTemplate.send("orders.finalized", new OrderFinalizedEvent(order.getId()));
        // Topic и payload остались прежними
    }
}

Теперь OrderService проще:

import org.springframework.stereotype.Service;

@Service
public class OrderService {

    public void completeOrderFinalization(Order order) {
        order.markFinalized();
        orderRepository.save(order);
        orderEventPublisher.orderFinalized(order); // Поведение то же, граница чище
    }
}

Снаружи ничего не поменялось: тот же вызов completeOrderFinalization(order), то же состояние заказа в базе, то же событие в тот же topic. Но внутри понятнее — OrderService координирует бизнес-действие, OrderEventPublisher отвечает за инфраструктурный побочный эффект.

Эту же мысль удобно увидеть схемой:

flowchart LR
A[OrderController] --> B[OrderService]
B --> C[OrderRepository]
B --> D[OrderEventPublisher]
D --> E[KafkaTemplate]

Именно так и выглядит хороший архитектурный refactor. Без апокалипсиса, без «новой платформы», без желания обновить полпроекта. Вы сужаете ответственность каждого элемента — чтобы следующий человек не читал бизнес-сервис с мыслью: «А почему он знает про Kafka?»

3. Направление зависимостей без мистики

Фраза «направление зависимостей» часто звучит для новичка слишком академично, будто сейчас нужно открыть три книги по архитектуре и одну чашку валерьянки. Магии нет — это ответ на один вопрос: кто о ком знает. Чем меньше бизнес-логика знает про инфраструктуру, тем проще код читать, тестировать и менять безопасно.

В нашем примере OrderService уже перестал напрямую дёргать kafkaTemplate, и это хорошо. Но можно сделать шаг ещё аккуратнее: зависеть не от низкоуровневой детали, а от понятной границы. Иногда хватает конкретного класса OrderEventPublisher, иногда полезен интерфейс — отделить бизнес-уровень от инфраструктуры явно.

public interface OrderEvents {
    void orderFinalized(Order order);
}

А publisher реализует этот интерфейс:

import org.springframework.stereotype.Component;

@Component
public class OrderEventPublisher implements OrderEvents {

    @Override
    public void orderFinalized(Order order) {
        kafkaTemplate.send("orders.finalized", new OrderFinalizedEvent(order.getId()));
    }
}

Что это даёт на практике? OrderService легче тестировать — реальный Kafka-клиент ему больше не нужен даже мысленно. И код лучше выражает намерение: сервис не «работает с Kafka», а «сообщает, что заказ завершён». Разница кажется словесной, но именно такие словесные границы потом спасают большие классы от каши. Тест после этого честнее:

import static org.mockito.Mockito.verify;
import org.junit.jupiter.api.Test;

@Test
void shouldDelegateEventPublication() {
    service.completeOrderFinalization(order);
    verify(orderEvents).orderFinalized(order); // Проверяем границу, а не транспорт
}

Заметьте, safety net из прошлых лекций мы не выбросили — наоборот, здесь он особенно нужен. Unit-тест выше проверяет, что сервис делегирует публикацию. Существующий integration test подтверждает, что внешний flow не сломался: endpoint работает, событие уходит, контракт сохранён. Так и сочетаются lightweight characterization, harness и архитектурный refactor — каждый проверяет свою границу.

И ещё одна важная деталь для тех, кто только учится: не нужно героически добавлять интерфейс всюду, где мелькает слово «зависимость». Интерфейс — это инструмент, а не религия: вводим его там, где он делает границу яснее, а не ради чувства архитектурной духовности.

4. Красная линия: refactor или уже другая задача

Самый опасный момент refactor наступает не в коде, а в голове. Вы вынесли OrderEventPublisher, всё гладко, и тут где-то сбоку возникает соблазн: «Раз уж я здесь, давайте заодно обновлю Kafka, переименую topic, поправлю странный if и почищу соседний пакет». Вот в этот момент refactor и превращается в подозрительный комбайн из трёх задач.

Чтобы этого не произошло, полезно задавать себе очень приземлённый вопрос: если reviewer увидит это в PR, он объяснит одним предложением, что произошло? Если ответ звучит как «тут и refactor, и fix, и чуть-чуть инфраструктуры» — пора разделять работу.

Вот удобная таблица для таких пограничных случаев:

Что вам захотелось сделать «заодно» Что это на самом деле Что делать правильно
Поменять topic события или его payload Изменение внешнего контракта Отдельная задача, не refactor
Поднять версию spring-kafka Обновление зависимости Отдельный PR
Исправить ошибку в условии скидки Bugfix Отдельный PR с regression check
Переименовать поля в REST DTO Изменение API Отдельная задача
Вынести side effect в отдельный collaborator Refactor Можно делать сейчас
Убрать пару посторонних предупреждений в соседнем модуле Побочная уборка по соседству Не делать в этом PR

Особенно коварная история — «скрытая миграция». Выглядит она безобидно: вроде почистили архитектуру, но заодно поменяли конфиг, сигнатуру события, формат сериализации или поведение зависимости. Снаружи PR называется refactor, а по факту поведение системы уже изменилось. Это ломает главное обещание работы: behavior preserved.

Поэтому у архитектурного refactor есть очень полезная дисциплина: всплыл настоящий bug, dependency upgrade или изменение контракта — останавливаете текущий PR и открываете отдельную задачу. Не бюрократия — способ держать diff понятным, rollback дешёвым, review честным. Иначе получите красивое название PR и некрасивый риск внутри.

5. Критерии приёмки для refactor PR

Пока refactor живёт только в вашей голове, он кажется понятным почти всегда. Настоящая проверка начинается в тот момент, когда PR открывает другой человек — или reviewer-agent из Workflow Kit. Поэтому критерии приёмки здесь должны быть не абстрактными, а вполне приземлёнными: что осталось тем же, что стало лучше, чем это подтверждено.

Для архитектурного refactor PR в Commerce OS удобно использовать такую таблицу:

Критерий Чем его доказываем
Внешнее поведение не изменилось Integration/API checks зелёные, topic и payload события прежние, публичные сигнатуры не менялись
Тесты и локальные проверки зелёные Harness из CLAUDE.md: unit, integration, build, lint, type-check, если они применимы
Diff понятен за один проход PR держится вокруг одной идеи и ограниченного набора файлов
Сложность стала ниже Метод короче, меньше ветвлений, ответственность уже
Дублирование стало меньше Повторяющийся код вынесен в отдельный collaborator
Имена стали яснее Новые названия объясняют намерение, а не скрывают его
Случайных изменений зависимостей нет build.gradle, package.json, конфиги зависимостей не тронуты
Скрытой миграции нет Не менялись версии, транспортные настройки, внешний формат событий и API
Нет широкого постороннего cleanup В diff нет лавины постороннего форматирования и случайных переименований по соседству
Откат возможен дешево PR можно откатить одним revert без каскада побочных действий

Обратите внимание: критерии приёмки проверяют не только безопасность, но и пользу. Если diff огромный, а ответственность не стала яснее, reviewer вполне вправе не радоваться. Refactor PR отвечает на два вопроса разом: ничего ли мы не сломали и стало ли реально легче жить с этим кодом дальше.

Хороший PR description для нашего примера может выглядеть так:

## Цель
Изолировать публикацию событий из `OrderService`, не меняя внешний контракт сервиса.

## Область изменений
`OrderService`
`OrderEventPublisher`
`OrderServiceTest`

## Что не менялось
REST `/api/orders/{id}/finalize`
topic `orders.finalized`
payload `OrderFinalizedEvent`
зависимости проекта

## Проверки
`./gradlew test --tests '*OrdersUnitTest'`
`./gradlew test --tests '*OrdersIntegrationTest'`

А reviewer.md из Workflow Kit может вернуть, например, такой краткий итог в REVIEW_NOTES.md:

Статус: BEHAVIOR_PRESERVED
Публичные символы: без изменений
Проверки: OrdersUnitTest, OrdersIntegrationTest
Риск: низкий

Такой вывод полезен именно потому, что он короткий и проверяемый. Reviewer не читает роман на пять экранов — он видит: контракт на месте, проверки прогнали, изменение обратимое.

6. Анатомия понятного refactor PR

Многие проблемы архитектурного refactor возникают не потому, что код плохой, а потому, что PR плохо упакован. Автор знает, что «всего лишь вынес publisher». Reviewer видит три файла, новое имя, старое, пару тестов — и нервничает, не закопали ли под видом cleanup маленькую революцию. Секция «что не менялось» успокаивает его быстрее всего. Изменение маленькое — один commit; шагов два — они отдельно обратимы, но живут в одном PR, а не размазываются по истории как сериал на шесть сезонов.

И вот тут вся схема наконец собирается в одно целое. Harness даёт техническое доказательство. Lightweight characterization и существующие integration tests страхуют поведение. Incremental loop защищает от broad rewrite. Критерии приёмки превращают всё это в PR, который не требует от reviewer телепатии.

Если reviewer открывает такой PR и за один проход понимает: business service больше не знает о Kafka напрямую, REST не изменился, event не изменился, тесты зелёные, откат дешёвый — значит, вы сделали ровно то, чему посвящена эта лекция. Не эффектное «перерождение архитектуры», а спокойное, точное и обратимое улучшение системы — иногда это ценнее всего остального, потому что именно так код становится лучше без лишней драмы.

1
Задача
Claude code, 20 уровень, 4 лекция
Недоступна
Выделение OrderEventPublisher из OrderService
Выделение OrderEventPublisher из OrderService
1
Задача
Claude code, 20 уровень, 4 лекция
Недоступна
PR description и acceptance criteria для architecture refactor
PR description и acceptance criteria для architecture refactor
1
Опрос
Safe refactoring с Claude, 20 уровень, 4 лекция
Недоступен
Safe refactoring с Claude
Safe refactoring с Claude
Комментарии
ЧТОБЫ ПОСМОТРЕТЬ ВСЕ КОММЕНТАРИИ ИЛИ ОСТАВИТЬ КОММЕНТАРИЙ,
ПЕРЕЙДИТЕ В ПОЛНУЮ ВЕРСИЮ