Сегодня попался рельсовый код, в котором используются монады, сервисы и прочее. Решил сделать обзор с объяснением того, что в коде не нравится и что можно исправить.
Данный разбор основан только на личном опыте и избегает попытку написать самый идеальный код на свете. К сожалению пошарить ссылку на код не могу, потому что автор попросил опубликовать анонимно.
Главная операция, которая вызывается из контроллера выглядит следующим образом:
module Excursion
module Services
class JoinUser < Service
def initialize(params, record: ,
input_validator: Users::Validators::NewUser.new,
user_exists_check: Users::Services::CheckExists,
user_creator: Users::Services::Create
)
@params = params
@input_validator = input_validator
@record = record
@user_creator = user_creator
@user_exists_check = user_exists_check
end
def call
validated_params = yield input_validate
yield business_validate(validated_params.to_h)
save(validated_params.to_h)
end
private
attr_reader :params, :record, :input_validator, :user_creator, :user_exists_check
def input_validate
input_validator.call(params[:group]).to_monad
end
def business_validate(validated_params)
return Failure(:already_exists) if record.users.where(validated_params).exists?
Success()
end
def save(validated_params)
user_check_result = user_exists_check.new(validated_params[:email]).call
if user_check_result == Success(:user_not_exists)
user = yield user_creator.new(validated_params).call
record.users << user
return Success(:created)
end
record.users << user_check_result.value!
Success(:created)
end
end
end
endДва сервиса зависимостей, которые используются в этом примере выглядят так:
module Users
module Service
class CheckExists < Service
def initialize(email, model: User)
@email = email
@model = model
end
def call
return Success(model.find_by(email: email)) if model.where(email: email).exists?
Success(:user_not_exists)
end
private
attr_reader :email, :model
end
end
end
module Users
module Services
class Create < Service
def initialize(params,
input_validator: Users::Validators::NewUser.new,
model: User
)
@params = params
@input_validator = input_validator
@model = model
end
def call
yield input_validate
save
end
private
attr_reader :params, :model, :input_validator
def input_validate
input_validator.call(params).to_monad
end
def save
Try() { model.create!(params) }
end
end
end
endУбрал общие слова из именования объектов, сервисов и данных. Как пример, в изначальном варианте есть строчка save(validated_params.to_h) (Excursion::Services::JoinUser#call), как оказалось - это запись пользователя на экскурсию, т.е. с точки зрения кода - это сохранение чего-то в базе, но с точки зрения логики это совершенно другая операция. Если использовать общее именование - придется каждый раз думать зачем этот код, а если использовать описание процесса - появляется контекст, в котором понять реализацию проще, чем по реализации понять контекст. Поэтому переименовал этот шаг в enroll.
В изначальном примере можно увидеть 2 глобальные проблемы:
- Зависимость от кучи зависимостей
- Оба сервиса реализуют логику работы с базой данных (проверка существования пользователя и пользователя сохранение в базе).
Большой соблазн сделать 2 отдельных сервиса и положить эту логику туда, как сделал автор оригинального примера. К сожалению, можно заметить, что такой подход приводит к усложнению бизнес логики, яркий пример - две строчки из Excursion::Services::JoinUser#save:
def save(validated_params)
user_check_result = user_exists_check.new(validated_params[:email]).call
# вот тут приходится делать страшную проверку с использованием монады
#
# так же, сама монада самоисключающий случай и вне контекста логики не понятно
# почему отсутствие пользователя успех
if user_check_result == Success(:user_not_exists)
# в данном случае приходится использовать DO notations во вложенном методе,
# таким образом появляется вложенность, что приводит к усложению data flow
user = yield user_creator.new(validated_params).callПолучается, что автор лишний раз обернул операцию с базой данных и тем самым раздул собственный код. Поэтому, кажется, что подобные методы должны находиться в моделе/репозитории. Так же, судя по неймингу, приходим к проблеме контекста, так как названия сервисов является описанием работы кода, а не описанием бизнес логики.
В примере много общих слов, которые увеличивают конгнетивную нагрузку на человека, который этот код читает. Я говорю о record, user_exists_check, validated_params, business_validate, input_validate, params без контекста не понятно, что это такое, потому что терминология общая. В качестве примера: прочтите название двух сервисов и попробуйте ответить себе, что каждый из них делает:
Excursion::Services::JoinUserExcursion::Services::EnrollUser
В оригинальном примере можно заметить, что конструктор каждого из классов представляет из себя микс из зависимостей и переменных.
def initialize(params, record:, # данные
input_validator: Users::Validators::NewUserContract.new, # зависимость
user_exists_check: Users::Services::CheckExists, # зависимость
user_creator: Users::Services::Create # зависимость
)
def initialize(email, model: User)
@email = email # данные
@model = model # зависимость
end
def initialize(params, # данные
input_validator: Users::Validators::NewUserContract.new, # зависимость
model: User # зависимость
)В этом нет проблем до тех пор, пок не придется тестировать код и использовать тяжелособирающиеся зависимости (например rom). Так же, каждый вызов сервиса приведет к новой аллокации объектов, но это не так критично. Намного критичнее завязка локики на внутренний стейт объекта. Во первых, сервис перестает быть чистым(мутация данных приведет к багам), во вторых, поддержка кода становится сложнее, так как аргументы могут подсказать детали реализации. В качестве примера сравните два определения функций:
def save(validated_params)def save(record, validated_params)
В первом случае первая мысль приходящая в голову - параметры сохраняются в базе и создается новый объект модели. Во втором случае понимаешь, что сохраняются данные для конкретной записи. Проблема в том, что это не выдуманный пример. Если посмотреть на оригинал - save действительно использует record который является внутренним стейтом объекта. Но на самом деле record - это просито инстанс модели excursion.
Распространный подход в рельсе: цепочки методов в моделе, которые сложно тестировать. Я говорю о случаях record.users.where(payload).exists? и подобных ему. Проблема заключается в том, что для такого кода сложно написать юнит тест, поэтому приходиться либо писать сложный мок, либо использовать интеграционное тестирование (на любой тест ходить в базу). В каждом из случаев возникают проблемы: моки сложно поддерживать и понять. А походы в базу увеличивают время прохождения тестов на минуты и даже часы.
В итоге, после рефакторинга, основанного на идеях описанных выше, получился код, в котором видно какие бизнес шаги происходят (валидация данных, проверка пользователя на участие в экскурсии, запись пользователя на экскурсию). Данный код не идеальны, но даже сейчас он позволяет отказаться от избыточности сервисов, добавляет бизнес контекст и может быть покрыт юнит тестами без сложной подготовки данных.
module Excursion
module Services
class EnrollUser < Core::Service
attr_reader :validator
def initialize(validator: Users::Validators::NewUserContract.new)
@validator = validator
end
def call(excursion, params)
paylaod = yield validate_raw_data(params)
yield attender_of_excursion?(excursion, payload)
user = yield find_or_create_by_email(payload[:email])
Success(excursion.enroll(user))
end
def attender_of_excursion?(excursion, payload)
excursion.attender?(payload) ? Failure(:attender_of_excursion) : Success()
end
def find_or_create_by_email(payload)
Try() { User.find_or_create(payload) }.to_result
end
end
end
end
class Excursion
def attender?(payload)
users.where(payload).exists?
end
def enroll(user)
self.users << user
self
end
end
class User
def self.find_or_create(payload)
find_by_email(payload[:email]) || create!(payload)
end
end- спасибо @likeath, в последнем варианте
Userдолжен инжектиться в сервис, т.е. конструктор сервиса должен выглядеть так:
module Excursion
module Services
class EnrollUser < Core::Service
attr_reader :model, :validator
def initialize(model: User, validator: Users::Validators::NewUserContract.new)
@model = model
@validator = validator
end
Хотелось бы посмотреть что там за блок, с которым вызывается этот
call. Блок принимает три последовательных вызова, причём:1й на входе принимает результат validate_raw_data (подозреваю, что Success и что-то в нём), а на выходе выдаёт хеш с атрибутами.
2й на входе имеет boolean (судя по ? в
attender_of_excursion?), хотя по факту Success/Failure. А на выходе - ничего.3й на входе имеет Success/Failure, и на выходе разворачивает Success для получения завёрнутого в него user.
А если Failure? Например, потому что
find_by_email || create!упали с исключением (race condition, например). Где обработка ошибки?Есть подозрение, что сложность этого блока зашкаливает.