Путешествие в царство легаси. Как маленькая задача привела к большому рефакторингу
Привет! Меня зовут Артем Коньков, я frontend-разработчик в СберМаркете. А еще, я тот человек, который в фильмах ужасов спускается в темный подвал вопреки инстинкту самосохранения. Во-первых, потому что это интересно, а во-вторых — кто-то же должен это делать!
Это история о том, как мое любопытство превратило небольшую задачу в настоящий квест длиной в 5 спринтов. Я нисколько не жалею, что в него ввязался, потому что в итоге я провел крупный и полезный для компании рефакторинг. Я потратил гораздо больше времени и ресурсов, чем планировал, но оно того стоило. Надеюсь, мой пример поможет вам меньше бояться подобных задач, и даст мотивацию копать глубже.
А в конце статьи я расскажу, как правильно закреплять результаты, чтобы такая масштабная работа не была проделана впустую. Но сначала немножко контекста.
Маленькая задачка
Давным-давно мы в СберМаркете придумали, что наш api делится на три версии. Ручки к первой расположены по адресу /api/, ко второй — по адресу /api/v2/, а v3 не был расположен по адресу /api/v3/, там в запрос был зашит header с указанием версии.
И вот однажды ко мне пришли ребята из команды backend-support и сообщили, что v3 api будет продублировано в неймспейс api/v3 (раньше api v3 и api v1 располагались по одному и тому же адресу), следовательно, пропадет необходимость добавлять header к запросу.
Бэкендеры проделывала эту работу в рамках задачи
Моя задача — добавить в адреса /v3/ где надо.
В задаче был список адресов, куда мне нужно было добавить v3
Приключение на 20 минут, вошли и вышли! На первый взгляд, изян. Вроде…
Я принялся за дело и в какой-то момент мне бросилось в глаза, что где-то мы пишем /api/ в начале строки, а где-то нет. Недолго думая, я пошел смотреть в чём же разница.
И тут я заметил кое-что странное…
Я обнаружил, что мы используем на проекте разные axios клиенты. Если быть точным, то восемь клиентов api. А если быть совсем точным ВОСЕМЬ. РАЗНЫХ. КЛИЕНТОВ!
В этот момент можно было остановиться и не заглядывать в пропасть. Многие, наверное, так бы и поступили, но…
У меня возник вопрос — зачем их столько? И я пошел сравнивать!
Омерзительная восьмерка
Итак, вот они все, слева направо:
Знаете, что между ними общего и различного?
Можете сравнить сами на примере трёх клиентов
Из общего, они все так или иначе ссылаются или на старый client v1 с его config или на клиент v3. А знаете, в чём между ними разница? Только отправка дополнительного header’a.
Как так вышло? Я предполагаю, что мои коллеги просто смотрели, как организована соседняя фича, и делали по аналогии. А потом, не найдя там нужный api-клиент, создавали свой. Любой старый проект всегда обрастает слоем легаси, который нужно рефакторить или переписывать. И я решил взять эту задачу на себя.
Рефакторинг. Начало
Я прикинул, что несколько клиентов можно схлопнуть. По моей оценке эта работа должна была занять 2 спринта, что уже превышало время на изначальную задачу. Но я получил добро от руководителя и с энтузиазмом отправился в этот квест, пока не осознавая его настоящих масштабов.
Это я смотрю на себя из прошлого
Реальность сразу начала вносить коррективы — две недели ушли только на то, чтобы выстроить общую логику изменений и сделать первые попытки их внедрить.
Спустя две недели я придумал обновленный план, который должен был сработать:
rtkQueryApiV3 надо переименовать в rtkQueryApi.
clientApiVer3 надо переименовать в apiClient.
rtkQueryApiV1 удалить, использовать вместо него apiClient.
client удалить, использовать вместо него apiClient.
rtkRecsQueryApi переименовать в rtkQueryApiRecs.
clientApiBanners переименовать в apiClientBanners.
clientApiRecommendations переименовать в apiClientRecommendations.
Последние три изменения нужны для унификации нейминга. Приступим!
Унификация
Чтобы вместо удалённых клиентов использовать новый apiClient, нужно было сперва его унифицировать. Изменений в самом клиенте надо было сделать не так много — я добавил несколько констант и функцию.
Константа в данном случае выполняла роль множителя timeout:
Если действие происходит с использованием SSR.
Если используется не SSR или действие проиходит при разработке — timeout равен 0.
Функция, которую я написал, возвращала базовый URL для ручки (имею в виду определенный адрес бэкэнда, на который мы обращаемся и забираем оттуда данные). Функция принимала в себя строку, и затем происходило действие по одному из двух сценариев:
если вычисления происходили на сервере, то она соединяла сервер api url, который был прописан в environment, с путем, который был передан.
если вычисления происходили на клиенте — то возвращала тот же путь, что и был указан.
Тот самый код
Еще я написал новый config, который бы по умолчанию использовался во всех не удаленных клиентах и в котором появился baseUrl.
Что еще в него добавилось?
Новый header is-storefront-ssr — определяет, сделан этот запрос из сервера или из клиента;
Token — подмешивает header-ы, которые отвечают за безопасность;
Logger — фиксирует действия, которые выполняет api-client, сообщает об ошибках;
Modify — интерцептор, который нужен чтобы подмешивать cookie и header;
Timeout — использовали новый timeout.
Тот самый конфиг
Что ж, вот мы и унифицировали api — теперь можно использовать единый подход для всех! Осталось только поменять все строки, где использовались старые api clients, на новые.
Что это значит на практике? Это значит, что теперь мы должны сделать огромное количество изменений.
Большие изменения
В основном они выглядели вот так:
Здесь я заменил старый v1 client на api client
Здесь — заменили только конфиг
Внимательные читатели заметили еще кое-что — из роутинга было удалено слово spree.
Здесь подробнее о spree
Вот более наглядный пример:
Что это за зверь и почему я решил его удалить?
Минутка истории. Когда-то в СберМаркете frontend находился в монолите с ruby-on-rails backend. Мы жили в одном репозитории, одном проекте, и для того, чтобы запустить frontend, нужно было локально запустить и backend. Но время шло, монолит распиливался, и уже давно frontend находится отдельно.
Spree — это мультиязыковая e-commerce open-source библиотека, которая позволяла писать магазины на ruby. Для коллег из backend ее использование до сих пор актуально, но для нас указание, что какая-то конкретная ручка является ручкой spree, никакого смысла уже не имеет.
В некоторых местах пришлось поменять формат записи, т.к. не соблюдалась конвенция наименований. Где-то добавился префикс /api/ у адреса, где-то в названии роута явно появилось указание, что это api, где-то добавилось /v3/ в адрес ручки.
Догадываетесь, сколько еще изменений за собой это принесло? Мягко говоря, довольно много!
В самом файле с путями было изменено около 200 строк — это значит, что изменений по всему проекту будет как минимум 200, если считать, что один адрес используется только в одном месте. Но, к сожалению, это было далеко от реальности, потому что не зря роуты вынесены в отдельный файл — они переиспользуются минимум в двух местах по проекту.
В итоге, изменения затронули почти весь проект — всего в Merge Request оказалось замен на +1246 -1260 строк.
Пример со spree, конечно же, был не единственным. Легаси — вещь хрупкая, поэтому моя дорога к успешному завершению задачи была тернистой.
Один из сайд-квестов
Например, произошел баг на нашей платформе рекламы — после изменения api client, сервер перестал узнавать пользователя, потому что туда не отправлялись запросы, спотыкаясь об CORS. И я начал разбираться, снова.
Запрос в сервис рекламы был написан примерно 500 лет назад давно, при этом изменения в него вносились лишь частичные. Из-за этого разобраться было довольно трудно. Например, в запрос передавались параметры, которые непонятно откуда брались, и не все их поля использовались.
Skipped for now код
В общем, пройдя по ложному следу кучи ненужных констант и функций, я смог найти проблему — не перекинулся какой-то из header-ов, который позволял идентифицировать нас на сервере. Фикс — +33 -308 строк. Да, в итоге я хорошенько вычистил код и везде использовал новые подходы вместо старых.
Например, эти константы потеряли свою актуальность. Они повторяли типизацию из более нового участка кода.
Константы, которые, как оказалось были уже типизированы
Слева — как было, справа — как стало | |
[COMPONENT_ID.TOP_CAROUSEL_BANNER] | BannersPlacementMapping[BannersPlacement.MainPage]] |
COMPONENT_ID.TOP_CAROUSEL_BANNER это тоже самое, что и BannersPlacementMapping[BannersPlacement.MainPage]], просто второй взят из TS файлов с типами, а значит он более свежий.
А ещё нашел место в котором типизация расходилась с константами — пришлось дописать.
Также как и утратилась необходимость использования некоторых утилит.
Те самые некоторые утилиты
Где-то удалялись и переписывался вот такие куски
Решение этой и других проблем заняли у меня еще два спринта. Изменения затрагивали почти весь проект, поэтому много моего времени уходило на выяснение вопросов по работе разных функций и микросервисов — я еще никогда так много не общался с другими командами!
И все же, спустя полтора месяца я наконец-то был готов передать задачу в тестирование.
Не все так просто
Как вы поняли, за эти полтора месяца я проделал большую работу и много чего проверил сам. Тем не менее, я получил вот такой список багов от тестировщиков:
Таковы реалии больших сервисов — всегда есть страницы и функции, запрятанные далеко в недрах проекта, до которых ты не докопался. А что делать? Я начал фиксить новые баги, как вдруг, я получаю сообщение:
И тут меня как по голове ударило! Изначально я же должен был просто добавить в адреса /v3/, где надо…
Мое лицо, когда вспомнил об изначальной задаче
Мне повезло — пока я делал большую необязательную работу, команда backend занималась своей масштабной задачей по удалению дублирующего кода и проверки по header. Они тщательно все тестировали, фиксили баги и пришли ко мне с вопросом по моей задаче, только когда у них все было готово.
Я же так увлекся рефакторингом, что начал вносить изменения в ту же задачу вместо того, чтобы создать новую. Как результат — я забыл, что от меня требовалось в начале. Никогда так не делайте!
Тем не менее, процесс уже вышел на финишную прямую и оставалось совсем немного.
Финальные штрихи
Я быстро создал новую, более узкую задачу.
По ней требовались примерно такие изменения:
Всего +21 -19. И я внес их за 15 минут! А в течение суток задача ушла в релиз. Можно было выдохнуть.
Еще две недели мы с командой backend придумывали, что и за чем нужно изменить, чтобы ничего не сломалось. Примерно в это же время я завершил мой рефакторинг — задача была в постоянном тестировании в течение 3 недель, потому что пришлось делать полноценный регресс.
И вот, спустя 2,5 месяца, я ставлю статус RESOLVED!
Мои ощущения в финале этого квеста
Итоги и закрепление результата
О чем эта история? Формально — о том, как я вместо 20 строк кода написал 2000. Но я думаю о ней скорее как о доказательстве того факта, что самые интересные и важные проекты случаются, когда ты следуешь за своим любопытством.
Да, на пути было много ошибок и сложностей. Задача перекладывалась из спринта в спринт:
из-за проблем с большим количеством созависимого легаси-кода;
из-за сложного тестирования, которое выпадало на периодическую неработоспособность тестовых контуров;
из-за большого количества коммуникаций с другими командами, связанных со спецификой работы как отдельно взятых функций, так и микросервисов.
Всего было создано 4 MR и 6 веток. В каждой из них я продвигался чуть ближе к решению (кроме первой, которую я закопал полностью), и следующую начинал уже с наработок из предыдущей. По сути, я сохранялся, пока не прошел игру.
Что можно было сделать лучше?
Подойти к рефакторингу более осознанно и завести отдельную задачу
Осторожнее делать массовые замены, т.к. они затрагивают слишком много созависимого кода
Но все же я успешно закончил начатое, и мы получили крутые результаты. Теперь проект выглядит чище, а всем командам приятнее работать с кодом, который стал консистентнее.
Как было раньше? | Что есть сейчас? |
При разработке какой-нибудь новой фичи разработчики ходили в соседнюю папку и соседний файл, смотрели, как было сделано там, и делали также. Из-за этого вырастало огромное количество кода, не находящегося под контролем. 1) Накапливались старые apiClient’ы. 2) Подходы в клиентах были разные. 3) Где-то были какие-то преобразователи запроса и ответа, где-то не было. 4) Где-то были какие-то кастомные хедеры, где-то не было. 5) Где-то надо было писать api в начале строки адреса, где-то нет. | Стало возможно использовать apiClient и не думать о том, какими из тысячи инстансов axios мы пользуемся в том или ином случае. Мы имеем единый подход в написании адресов ручек, что позволяет нам быстрее искать какие-то конкретные адреса, а значит быстрее разбираться в багах. Мы точно знаем каким контроллером в Ruby обрабатывается ручка, потому что видим версию прямо в ссылке, И мы наконец-то избавились от надоедливого слова spree в названии роутов — я сэкономил коллегам пять символов на набор! |
Но важно не только провести рефакторинг, но и зафиксировать изменения, чтобы все не откатилось назад. Для этого у нас есть несколько инструментов.
Один из них — соглашения, свод правил и инструкций, как надо и как не надо делать. Своего рода устав. Теперь в него добавлена информация о новой схеме работы с api, и frontend разработчики всегда могут к ней обратиться.
Второй важный инструмент — фронтенд-новости. Это внутренняя встреча, где я часто сам выступаю или являюсь ведущим. Там мы рассказываем, что поменялось в проекте, и в принципе про важные события в индустрии frontend.
Третий инструмент — просто качественная коммуникация внутри команды. Мы подготовили разработчиков frontend несколькими уведомлениями, направили их смотреть merge request и готовиться морально, что у них поменяется схема работы с api — теперь все будет консистентно. А еще — я сообщил об изменениях командам, которые проводят код ревью, и теперь они не пропустят создание новых api.
Казалось бы, данная статья про то, что надо лезть делать крупные рефакторинги и не бояться бездумно лезть в болото в надежде, что выплывешь оттуда сам. Но нет. Вся эта эпопея была вам рассказана затем, чтобы в конце показать вот эту сводную табличку:
Когда лезть в колодец? | |
Не стоит: | Стоит: |
1) Вы не понимаете, где дно 2) Нет ясности в объеме 3) У вас есть другие задачи (более прогрумленые или приоритетные) | 1) Вы хотите стать бэтменом (типа он в колодец падал, а потом в нём ещё и сидел— это шутка, короче) 2) Вы понимаете, что после рефакторинга сильно поднимете DX 3) Рефакторинг повлияет на метрики 4) Вы точно понимаете объем работ 5) Нет жёстких сроков |
На самом деле мне очень повезло, что всё получилось так как получилось. Но если бы повезло чуть меньше, получилась бы жижа из кучи задач, сдвинутых сроков и конфликтов. Главное, что я не сделал в начале — адекватно, сняв розовые очки, не посмотрел на объём. Я же понял в какой-то момент, что рефакторинг будет крупным, надо было создать под него отдельную задачу. А если бы я не понимал объёма, стоило бы создать задачу на ресёрч.
Не будьте как Артём из прошлого, будьте как Артём из настоящего. Теперь я при понимании, что МР и спектр работ разрастается до размера луны, стараюсь вовремя создавать новые задачи: на ресёрч и на исполнение.
Ну, и в заключение — желаю вам быть любопытными, чтобы находить задачи с большим потенциалом, и терпеливыми, чтобы в конце концов их успешно закрывать. Удачи!
Tech-команда СберМаркета ведет соцсети с новостями и анонсами. Если хочешь узнать, что под капотом высоконагруженного e-commerce, следи за нами в Telegram и на YouTube. А также слушай подкаст «Для tech и этих» от наших it-менеджеров.
