Ты приходишь в проект, а там легаси…

Привет, сегодня я хочу поговорить об ужасной кодовой базе, с которой вы, скорее всего, прямо сейчас имеете дело. Есть проект и вы нужны, чтобы добавлять новые фичи и фиксить баги. Но вы открываете IDЕ, делаете пул репозитория с проектом — и хочется плакать. Кажется, что с этим кодом невозможно работать.

Картина, конечно, немного удручающая, но… давайте отбросим эмоции. И посмотрим, что можно быстро предпринять, чтобы облегчить страдания.

Почему мы чувствуем себя несчастными, работая с легаси-кодом? 

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

Но с точки зрения старшего разработчика часто все выглядит так:

• приложение очень хрупкое: если фиксим где-то баг, в ответ прилетают два новых;

• есть regressions bugs — они как вампиры, которых невозможно убить, каждый релиз появляются снова и снова;

• новые фичи занимают все больше времени: откладываются релизы, давят дедлайны, вы начинаете сознательно увеличивать время на планировании.

Причиной этих бед, как правило, является огромный объем технического долга — и отношение к нему в команде. Тесты, рефакторинги требуют времени. Конечно, когда важнее деливерить фичи, хочется на это забить. И я думаю, мы сами допустили подобное в индустрии.

У меня есть одна история. Приходит менеджмент, говорит, что нам нужна эта фича, причем «еще вчера». Мы работаем изо всех сил, релизим как можно быстрее. Затем говорим менеджменту: «Теперь нам нужна неделя, чтобы отрефакторить, написать тесты». На что менеджер отвечает: «Что? Вы этого не сделали?».

Когда вас просят сделать что-то, от вас ожидают что-то готовое. А если фича нужна настолько срочно, что к ней даже не нужна «пожарная сигнализация» в виде тестов, это должно быть оговорено в самом начале. И наша задача как разработчиков донести это до бизнеса. Я хочу поделиться подходом, который мы применили в команде мобильного бэкенда Skyeng — это путь инкрементального улучшения легаси-проекта.

Что будет, если ничего не делать (и почему переписать не всегда выход)

Каюсь, как-то раз я работал в компании, где использовался подход «нет времени ничего менять, давайте быстро пофиксим». Мы продолжали добавлять фичи, плодить (и фиксить) всё больше багов. Мы лечили не болезнь, а лишь ее симптомы. Я на практике столкнулся с «эффектом разбитых окон». Начав делать фиксы по принципу «лишь бы просто работало» и заметив, что, в принципе, и новичкам и старичкам в команде всё равно, мы начали жить по принципу: быстрый фикс, копи-паста, костыль — сделали, зарелизили, забыли. 

Становилось всё хуже и хуже. Всё это привело к серьезному стрессу и потере мотивации. В конце концов, не дожидаясь, когда мы подойдём к моменту, что всё это станет не поддерживаемо, я ушел. 

Есть другая крайность. Мы видим старую систему. Думаем: «Если тот криворукий, кто написал этот ужас, смог заставить всё работать, то наверняка это просто. Давайте перепишем всё уже по-нормальному. Сделаем красиво». Но это ловушка. Легаси-код уже пережил много деплоев и изменений требований. Начиная с начала, мы думать не думаем и знать не знаем о всех тех трудностях, которые пережил тот код. Да, мы пытаемся делать предположения, но зачастую они неверны. Почему так?

Есть старая команда, которая поддерживает легаси-проект, и новая — у нее обычно нет большой накопленной экспертизы ни в домене, ни в легаси-коде. Новая команда будет делать слишком много предположений о системе. И не всегда угадает.

Я не хочу сказать, что это не работает. У меня есть позитивный опыт переписывания легаси-системы: но, во-первых, тот, кто писал старый проект, и переписывал его. Во-вторых, бизнес дал добро, что на определенный промежуток времени мы замораживаем систему — ничего больше не добавляем, не фиксим без крайней необходимости. Плюс у нас под рукой всегда был доменный эксперт. Но у меня до сих пор есть ощущение, что нам повезло и это было скорее исключение, подтверждающее правило.

Шаг первый: пишем smoke-тесты, которые скажут, что приложение хотя бы живое   

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

Что использовать? Нам понравился Codeception — у него приятный интерфейс, можно быстро накидать smoke-тесты на наиболее критичные эндпоинты, сами тесты выходят лаконичными и понятными. А еще у него такой выразительный API и тест можно читать прямо как user story.

Что покрывать? Правило простое: покрываем тот функционал, который отвечает за бизнес-домен. В нашем случае это были тренировки слов — мы покрыли smoke-тестами его. Например, у нас был эндпоинт, который отдавал выученные пользователем значения слов.

/**
 * @see WordsBundleControllerApiGetLearnedMeaningsController::get()
 */
final class MeaningIdsFromLearnedWordsWereReceivedCest
{
   private const URL_PATH = '/api/for-mobile/meanings/learned';
 
   public function responseContainsOkCode(SmokeTester $I): void
   { 
    
   }
}

Мы явно прописали эндпоинт, который будем тестировать, и через аннотацию @see сделали указание на контроллер. Это удобно тем, что по клику в IDE можно сразу перейти в контроллер. И наоборот, по клику на экшен контроллера можно перейти в тест.

Сам тест был максимально прост: подготавливаем данные, сохраняем в базе выученное пользователем слово, авторизуемся под этим юзером, делаем get-запрос к API и проверяем, что получили 200.

public function responseContainsOkCode(SmokeTester $I): void
{       
   $I->amBearerAuthenticated(Identity::USER);
   $I->sendGET($I->getApiUrl(self::URL_PATH));
   $I->seeResponseCodeIs(Response::HTTP_OK);
}

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

Но потом нам захотелось большего, и мы допилили тест: стали дополнительно проверять схему ответа и что в нем вернулось действительно то, что нужно. В нашем случае — выученное слово.

public function responseContainsLearnedMeaningIds(SmokeTester $I): void
{       
   $learnedWord = $I->have(
       UserWord::class, 
       ['isKnown' => true, 'userId' => $I->getUserId(Identity::USER)]
   );

   $I->amBearerAuthenticated(Identity::USER);
   $I->sendGET($I->getApiUrl(self::URL_PATH));
   $I->seeResponseCodeIs(Response::HTTP_OK);

    $I->seeResponseJsonMatchesJsonPath('$.meaningIds');
    $I->seeResponseContainsJson(
    ['meaningIds' => [$learnedWord->getMeaningId()]]
    );

Так постепенно мы покрыли все критичные части нашего приложения — и могли перейти непосредственно к коду.

Шаг второй: удаляем неиспользуемый код

В проекте не нужен код, который «потом понадобится». Потому что нас есть git, который уже его запомнил. Надо будет — восстановим. 

Как понять, что удалять, а что нет. Довольно легко разобраться с каким-то доменным или инфраструктурным кодом. Совсем другое дело — API и эндпоинты. Чтобы разобраться с ними, можно заглянуть в NewRelic и проекты на гитхабе. Но лучше прямо сходить к командам и поспрашивать — может статься, что у вас есть какой-то эндпоинт, из которого аналитика раз в год выкачивает важные данные. И будет очень некрасиво удалить его, даже если по всем признакам его никто не вызывал уже почти год. 

Шаг третий: делаем слой вывода

Очень часто в легаси-приложениях либо сущности торчат наружу, либо где-то в контроллере собирается и отдается массив. Это плохо — скорее всего, у вас нарушена инкапсуляция и у сущности есть публичные свойства. Такое тяжело рефакторить: клиенты в ответе уже ожидают определенные поля, и если мы захотим переименовать или сделать приватным какое-то свойство, то даже обнаружив и пофиксив все использования, мы имеем шанс что-то сломать и разбираться с фронтом. 

Например, у нас есть контроллер, который возвращает баланс пользователя. В этом примере мы просто достаём из репозитория сущность и как есть отдаём наружу — то есть как только мы порефакторим, API у клиента поменяется. 

final class Controller
{
   private Repository $repository;

   public function __construct(Repository $repository)
   {
       $this->repository = $repository;
   }
 
   public function getBalance(int $userId): View
   {
       $balance = $this->repository->get($userId);
       return View::create($balance);
   }
}

Чтобы решить проблему, можно вместо этого в ответах всегда использовать простую DTO — пускай у нее будут только те поля, которые нужны в ответе клиентам. Добавим маппинг из сущности и уже в контроллере это вернем.

final class Balance
{
   public int $userId;
   public string $amount;
   public string $currency;

   public function __construct(int $userId, string $amount, string $currency)
   {
       $this->userId = $userId;
       $this->amount = $amount;
       $this->currency = $currency;
   }
}

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

final class Controller
{
   private Repository $repository;

   public function __construct(Repository $repository)
   {
       $this->repository = $repository;
   }
 
   public function getBalance(int $userId): View
   {
       $balance = $this->repository->get($userId);
       return View::create(Balance::fromEntity($balance));
   }
}

Можно спокойно начинать улучшать и рефакторить, не боясь сломать обратную совместимость API.

Шаг четвертый: статический анализ кода

В современном PHP есть strict types, но даже с ними можно по-прежнему менять значение переменной внутри самого метода или функции:

declare(strict_types=1);
 
function find(int $id): Model
{
    $id = '' . $id;
 
    /*
     * This is perfectly allowed in PHP
     * `$id` is a string now.
     */
 
    // …
}
 
find('1'); // This would trigger a TypeError.
 
find(1); // This would be fine.

А так как типы проверяются в runtime, проверки могут зафейлится во время выполнения программы. Да, всегда лучше упасть, чем словить проблем из-за того, что строка внезапно превратилась в число (причем не то число, которое мы бы ожидали увидеть). Но иногда хочется просто не падать в runtime. Мы можем выполнить проверки до выполнения кода с помощью дополнительных инструментов: Psalm, PHPstan, Phan и так далее.

Мы в проекте выбрали Psalm. Возможно, у вас возник вопрос: зачем тащить в легаси-проект статический анализ, если мы и без него знаем, что код там не очень? Он поможет проверить неочевидные вещи. В легаси-проектах часто встречается так называемый array-driven development — структуры данных представлены массивами. Например, есть легаси-сервис, который регистрирует пользователей. Он принимает массив $params.

final class UserService
{
   public function register(array $params): void
   {
       // ...
   }
}


$this->registration->register($params);

Код неочевиден. Нам придется залезть в сервис, чтобы узнать, какие поля там используются. Статическим анализом мы можем явно указать, что в этот метод нужен такой-то массив с такими-то ключами, а под ключами — значения определенных типов.

final class UserService
{
   /**
    * @psalm-param array{name:string, surname:string, email:string, age:integer} $params
    */
   public function register(array $params): void
   {
       // ...
   }
}

$this->registration->register($params);

Прописав через докблоки формат и тип данных, при следующем рефакторинге мы запустим Psalm, он проверит все возможные вызовы и заранее скажет, сломает ли код. Достаточно удобно при работе с легаси.

Что дальше?

Получите контроль над существующей кодовой базой, поймите, как она работает, и попытайтесь немного привести ее в порядок. Имея в арсенале тесты, статанализ и постепенно вычищая код, вы сможете приступать к более серьезному рефакторингу и улучшать качество продукта.

А дальше — просто подумайте, что еще нужно, чтобы чувствовать себя счастливым, работая с этим кодом. Чего еще не хватает? Составьте себе план действий и постепенно улучшайте доставшийся вам в наследство код.

p.s. Несколько полезных материалов для дальнейшего изучения: 

• Пост основан на докладе с краснодарского PHP-митапа — вот его запись

• Больше о статическом анализе и Psalm

• Интересная история про переписывание проекта на PHP

• Что дальше делать с тестами

Автор: Сергей Жук

Источник