Крупный рефакторинг приложения. Часть 1 - Код.

У меня получилось нечто типа отпуска, так что появилось время, силы и желание на графоманию.
Хотелось бы рассказать о моём довольно интересном опыте по переписыванию половины проекта "одним куском", т.к. по частям уже не получалось (либо выходило слишком опасно, криво и долго, а времени у меня уже не оставалось).

Краткое введение

До недавнего времени я работал на проекте для онлайн перевода документов.

Т.е. упрощенно - пользователь загружает документ (docx, xlsx, indd, xml, ts, xliff и т.д. и т.п.), наше приложение его парсит, разбивает на т.н. сегменты - куски удобные для перевода (чаще всего - предложения), даёт веб-интерфейс для этого самого перевода (с редактором и уймой всяких других плюшек облегчающих жизнь переводчику) и потом пользователь может загрузить себе результат - переведённый документ.

Основные сущности это Document - собственно документ, Segment - сегмент документа, по факту всегда используется вместе с вторым Segment - один для исходного текста (Source), второй для переведённого (Target). Вместе они образуют (точнее, должны бы были образовывать) TranslationUnit - по факту минимальная единица с которой приходится иметь дело.

С технической точки зрения всё куда сложнее. Сразу хочу сказать что отлично понимаю что проект разрабатывался давно и разными людьми, поэтому весь этот бардак вполне понятен. Так что я никого не виню и вообще меня интересует не "кто виноват?", а "что делать?".

В общем, это изначально было ASP.NET WebForms приложение, которое постепенно мигрировало в сторону MVC-подобного решения с помощью handler-ов и ajax-запросов с клиента. За последние полтора года удалось довольно многое причесать, но некоторые вещи подпиливались преимущественно костылями. Например, это был слой работы с базой - там был зоопарк из DataSet-ов, вызовов хранимок, собранного вручную (для фильтрации и пейджинга) SQL кода дёргаемого через ADO.NET. Я уже писал о том что поддерживать это было очень трудно и всегда вылазило довольно много багов.

Так что, у нас было 3 основных задачи:

1. В CRM с которой мы интегрировались существует довольно много различных стадий обработки документа - перевод, валидация, вычитка, исправление замечаний по валидации и т.д. и т.п. (порядка 17 штук). Наша же система ничего об этом не знала - у неё есть только 3 жестко прописанных стадии с некоторыми различиями в поведении - перевод, валидация и вычитка (в базе - по колонке на каждую из стадий). Так что в итоге все стадии из CRM сводились в одну из трёх стадий у нас. Что привносило заметные неудобства, т.к. прогресс и прочее по факту не отражали нужной картины. Не говоря уже о простынях switch-ей для отображения нужной статистики. Так что основной нашей задачей было ввести т.н. Stage - этап обработки документа, который бы мог соответствовать любому из нужных вариантов в CRM.
2. Текущее состояние слоя персистентности в принципе не позволяло реализовать это в какие-то разумные сроки с разумными затратами. Множество хранимок, запросов, датасетов смешавшись в чудесный винегрет превращало процесс изменений в ад. Так что было принято решение (точнее, я топал ногами и кричал что иначе я за это не возьмусь) заодно написать новый слой персистентности на Entity Framework, изменения в котором было бы легко контроллировать
3. Структура базы была заметно переусложнена и обросла многими артефактами. Например вместо простой связки Document -> TranslationUnit была связка Document (у которого две ссылки на одну таблицу Source + Target)  -> Translation -> Segment, так что банальная задача вычитки TranslationUnit-ов превращалась в запрос с минимум 4мя джойнами (по 2 штуки - Document -> Translation -> Segment), не говоря о довольно частой ситуации с вызовами GetTargetSegment(..) и сразу за ним GetSourceSegmentByTargetId(...). В общем, текущий подход ухудшал и производительность и читабельность. И вообще, служил отличным примером о вреде предварительного обобщения - явно было сделано с заделом на какую-то функциональность которая в итоге так и не понадобилась.

В общем и целом, реально поставленной перед нами задачей была именно первая. Но реализовать её без второй и третьей было нереально (во всяком случае силами полутора землекопов разработчиков). В идеале, вторую и третью задачу было желательно решить раньше, в отдельном спринте с отдельной выливкой и т.д. Но кто бы выделил нам минимум месяц на рефакторинг не приносящий практически никакой прямой пользы для бизнеса? Собственно, даже на первую задачу (внутрь которой мы внесли оставшиеся 2 с соответстующими оценками по времени), которая "urgent-immediate-ASAP" время выделили только через 3 или 4 месяца после её возникновения и формулировки (долго, видите ли).

Процесс миграции кода

Для того чтобы всё переписать был специально разработан план по выполняемым задачам:

1. Создать 2 отдельных ветки - в одной находилась миграция данных, а во второй - собственно код. Раздельные ветки были выбраны потому что миграция данных должна была проходить раньше чем выливка кода, возможно даже с разницей в неделю между выливками.

2. Создать миграцию базы в которой бы создавались новые таблицы. У неё обязательно должен быть метод Down(), который бы убивал эти таблицы и приводил базу к состоянию "до миграции". Это довольно важный нюанс, т.к. на практике выяснилось что в процессе обкатки приходится довольно часто что-то подправлять или менять. И простейший способ был "откатить миграцию -> поправить её -> накатить заново". Так что это в итоге сэкономило нам довольно много времени.

3. Создать слой работы с базой (в отдельном проекте), тут я описывал общие принципы того как реализовал его - наружу был видим только четкий контракт и возвращаемые объекты гарантировали что они будут заполнены (т.е. если какие-то данные мы не вытаскиваем в конкретном случае, то результирующие объекты будут без соответствующих полей)

4. Переписывать текущие методы вызывающие старый код на новый, попутно по возможности правя модели идущие на верхние слои. При этом методы из старого DAL удалялись чтобы удостовериться что больше нигде не используются. И так до момента когда всё начнёт компилироваться. После этого можно было бы хотя бы сделать check in хотя бы компилирующегося кода. Благо это не было большой проблемой, т.к. всю работу в это время делал я один.

5. Первичная стабилизация проекта - прогнать набор тестов + проверить руками что основная функциональность работает.

6. Написать миграцию из старых таблиц в новые, сделать миграцию тестовой базы.

7. Прогнать автотест нашего QA и стабилизировать до более-менее нормального состояния

8. Сделать тестовую миграцию более крупных данных объёмов данных, которые ближе к реальности.

9. Отдать на ручное тестирование, вычистить последние баги.

10. Прогнать миграцию на продакшене в фоновом режиме, пока еще работает старый код со старыми таблицами

11. Вылить код на продакшн, перемигрировать изменившиеся данные (их должно быть немного)

В общем и целом, процесс переписывания кода был относительно тривиальным и заключался приблизительно в следующих шагах:

1. Берём какой-нибудь метод из старого DAL, смотрим где он используется.

2. Идём по его вызовам и меняем их на вызов нового DAL - как минимум первое время это выливается сперва в написание классов-маппингов к базе, потом написание классов-моделей, потом в написание соответствующих методов для специфичных CRUD потребностей.

3. В вызывающем коде конвертируем "доменную" модель в модель слоя персистнентности или обратно (что чаще).

4. Большая часть методов теперь стала требовать не только DocumentId/SegmentId, но еще и StageId. Так что менялась их сигнатура и по некомпилирующемуся коду поднимался до самого верха, не забывая потом поискать текст "handlers/Имя_хэндлера" в яваскриптовой части. Для случаев когда вместо Guid documentId надо было передать Guid stageId, специально добавлялся дополнительный параметр int? stub, чтобы не пропустить передачу чего-либо иного. Т.е. относительно легко пропустить то что теперь должен передаваться совсем другой Guid, но сложно это сделать когда вместо одного параметра теперь надо передать два, пусть второй и банальный null. 

5. Попутно зачастую приходилось добавлять промежуточные модели, т.к. довольно часто модель из DataSet-ов передавали напрямую до View, которая оказывалась пронизана чем-то типа 

x.Id = row["Id"];

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

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

Как ни странно, тесты удалось завести в течении буквально пары-тройки часов. Причем чуть ли не половину этого времени заняла починка буквально 3-4 тестов (из прорядка 2-3 сотен) сделанных с помощью Mock-ов. Так что считаю что моя позиция по поводу нелюбви к Mock-ам, фейкам, stub-ам и прочему вполне обоснована.

После этого я еще день или два стабилизировал относительно мелкие проблемы до состояния "проект кажется заработал". Это, кстати, обрадовало по 2м причинам:

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

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

Вместо собираемого вручную SQL-кода для фильтрации, сортировки и пейджинга был сделан специальный класс состоящий из нужных параметров (преимущественно enum-ы + строка фильтра). Этот класс передавался в специальный метод в новом DAL, который у себя внутри собирал IQueryable в зависимости от параметров и возвращал уже готовый результирующий список. Новый код получился во-первых меньше чем старый, а во-вторых заметно более понятный, а в свете рефакторинга базы еще и генерировал лучший SQL код чем тот что был написан вручную раньше.

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

Когда же миграция была более-менее готова, мы решили прогнать полный автотест (это занимает порядка часа) и в целом за день-два сумели заставить его проходить без ошибок. Это были как раз последние дни моей работы в этой компании. Я из интереса посмотрел что для merge в MAIN ветку будет около 310 файлов, это одно из крупнейших изменений делаемых "одним куском" из тех что я когда-либо делал. Впрочем, это далеко не первый мой опыт по переписыванию DAL приложения, может быть поэтому я и был уверен в успехе.

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

Часть 2