Improve our BaseService service class
Problem
I was trying to model something for the Hashed Storage migration, and our current state of our BaseService
is not helping us write better code.
This are few things we do with a Service class today:
- Extract complex logic from Controllers
- Coordinate work between multiple models
- Invoke finders and do operations with returned data
- Make series of sequential actions (that each one can fail or not)
- Return success/fail status
- Use returned status as part of chain of operations (probably multiple service classes involved)
- Validates data before executing "main" operation
- Log debug/error information
What we currently have implemented in our BaseService
class:
- required fields:
project
- optional fields:
current_user
,params={}
- log_info
- log_error
also a bunch of related services that can be initialized from BaseService:
-
NotificationService
(#notificaiton_service) -
EventCreateService
(#event_service) -
TodoService
(#todo_service)
and a method that is probly used only in a few other classes:
deny_visibility_level
Context
I was reading about ROP (Railway Oriented Programming), which can be summed by the idea that we usually think that a sequence of operations will be executed in a happy path, but then we started to figureout that each step may go wrong and needs to have it's own error handling step. If we instead code that chain of operations in a way that accepts that our "train" can change tracks after each step and log/deal with failures in each one, than we have something more robust as a whole.
There is a nice talk with slides here to illustrate that concept better: https://fsharpforfunandprofit.com/rop/
If we try to look at successful implementations of that model, we will find a few gems that does that:
- Trailblazer Operation (http://trailblazer.to/gems/operation/2.0/)
- dry-transaction (https://dry-rb.org/gems/dry-transaction/)
- opie gem (https://github.com/guzart/opie)
And the simplest implementation I've seen so far:
Our BaseService
class is probably one of the most "abused" part of our codebase. We use a lot that class, because we know in many cases what we need to fix bloated code is to extract it to a "Service Class", but because our default implementation doesn't help us model our current needs, what we endup doing is re-implementing parts of out BaseService class for each different problem we are trying to solve, just enough so we can get things done.
Proposal
We should think about defining a good BaseService class that can be used by the variety of cases we cover today, and to do that it must include at least the following public API (or something equivalent):
#success? (bool)
#failure? (bool)
#logger (Gitlab::AppLogger)
-
#errors
(array) -
#valid?
(bool) #validate!
#execute
Ideally we could borrow the ROP concept all together and have the definition of step :method
and fail :method
to define the two different paths (train tracks) the code execution can take.
Stuff to read
Here is a list of articles/talks about ROP alone or about one particular implementation in ruby: