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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

/** * @see \WordsBundle\Controller\Api\GetLearnedMeanings\Controller::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

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