If you've done development on a Ruby on Rails application before you're probably familiar with Active Record callbacks. For those unfamiliar, they are ORM object life-cycle hooks. They allow the developer to run custom code when an object is created, saved, updated, deleted and validated. They can be defined on an Active Record object at the class level like so:
class Foo < ActiveRecord::Base
before_save :run_callback, if: -> { some_attr_changed? && some_other_attr_changed? }
def run_callback
# do something
end
end
I'm mostly concerned with these ones that run during save
.
before_validation
after_validation
before_create
after_create
before_update
after_update
before_save
after_save
# I include custom active model validations because they operate in much the
# same way and cause similar headaches.
validate
Unfortunately, for various reasons, callbacks tend to be overused. Their power can be alluring. They allow you to add code that will run whenever an object is saved to the database without needing to update the save call sites with any new code. There also exists the influence from arguments about Fat Controllers & Thin Models vs. Thin Controllers & Fat Models. While these arguments have died down, echoes remain in the way many developers write Rails code.
What also remains are the concrete dependencies that were created when the Thin Controllers & Fat Models methodology was taken to the extreme. Developers were attempting to make controllers completely disappear and codify them into abstractions. Libraries like inherited_resources popped up that completely removed the need to write traditional CRUD controller code. Without any other place for the code to go it would end up being shoehorned into Active Record callbacks. inherited_resources was ultimately abandoned by its original author and deemed a misadventure. However, ActiveAdmin remains a popular library to construct admin panels with and continues to use inherited_resources with a "no controllers" coding style.
At our company these effects lead to a codebase with too many callbacks which has caused us many headaches. As well, a tendency for 'God Objects' to accrue in long lived projects compounded our situation. The combination of these problems left us with a few classes that had over seventy callbacks and custom validations defined on them. This makes reasoning about saving, creating and updating these objects incredibly difficult for a multitude of reasons: Callback code tends to be side effect heavy. Unscrupulously making just in time changes to the model attributes before save or validation. Schlepping data between systems to provide eventual consistency. Triggering updates on another model via the ever-present recalculate!
method and of course, last but not least, firing off background jobs to do god knows what.
Another aspect of side effects that's important to understand is the order they occur in. A good trick to pull on a co-worker is ask them the order of execution on these callbacks when you create a Foo record without looking at the documentation:
class Foo < ActiveRecord::Base
before_create :one
before_save :two
after_commit :three
after_commit :four
end
The order will, oddly enough, be: two, one, four, three; see the documentation for further details.
Side effects and state changes in the system are the most difficult thing to understand because of their far reaching consequences. Now thread the application of them through a gamut of conditionals that are nigh impossible to understand. Having fun yet? You may as well be staring into the Ark of the Covenant instead of trying to understand the code. "It's not that bad! Just fire up the debugger." you might say. Oh wait, right, since callbacks are declared at the class level you can't actually step through them with a debugger as you would imperative code and the conditionals defined on them often can't be inspected in an easy way at runtime.
Unlike Indy, in our case, we could not just close our eyes. We wanted to regain the debuggability and readability of simple imperative Ruby code while maintaining interoperability with the existing code that uses our objects. This meant that if we had a model Foo
we didn't want to go to hundreds of places in our code to modify calls to #save
, #update
, .create
, etc. While that might be an admirable goal at some point it wasn't within the scope of this project. We tried a couple different proof of concepts and ultimately landed on something structured like this:
class Foo < ActionRecord::Base
alias_method :active_model_validate, :valid?
def valid?(...)
end
alias_method :validate, :valid?
alias_method :active_record_save!, :save!
def save!(...)
end
def save(...)
end
end
All entry-point methods that invoke callback style code ultimately execute these methods. By aliasing the methods before overriding them we preserve access to the original implementations. This provides a starting point for our refactor. The theory being: all we need to do is delete the callback declarations from the model and write our callback code inline in these methods. Let's look at an example of what that might look like. First the before code with some nasty callbacks.
class Foo < ActionRecord::Base
before_validation :before_validation_callback, if: -> { some_attr_changed? }
after_validation :after_validation_callback, if: -> { some_other_attr_changed? }
before_create :before_create_callback, if: -> { some_attr_changed? && some_other_attr_changed? }
after_create :after_create_callback, if: :some_condition?
before_update :before_update_callback, if: :some_other_condition?
after_update :after_update_callback
before_save :before_save_callback, unless: -> { some_attr_changed? }
before_save :before_save_callback_on_create, on: :create, if: :some_condition?
before_save :before_save_callback_on_update, on: :update
after_save :after_save_callback, if: -> { some_other_attr_changed? }
after_save :after_save_callback_on_create, on: :create
after_save :after_save_callback_on_update, on: :update, unless: :some_other_condition?
validate :custom_validation
validate :maybe_custom_validation, if: :some_condition?
validate :custom_validation_on_create, unless: :some_other_condition?, on: :create
validate :custom_validation_on_update, if: -> { some_attr_changed? }, on: :update
# referenced methods would be defined below...
end
That's only 16 callbacks and custom validations on a single model. Imagine seventy. Now let's refactor this into our structure above. With some careful reading of the documentation and some experimentation we can discern the expected execution order of these callbacks. Leading us to code that looks like this.
class Foo < ActionRecord::Base
alias_method :active_model_validate, :valid?
def valid?(...)
# Before Validation callbacks
before_validation_callback if some_attr_changed?
# Call the original validations.
active_model_validate(...)
# Custom validations
custom_validation
maybe_custom_validation if some_condition?
custom_validation_on_create if !some_other_condition? && creating_record
custom_validation_on_update if some_attr_changed? && !creating_record
# After Validation callbacks
after_validation_callback if some_other_attr_changed?
errors.blank?
end
alias_method :validate, :valid?
alias_method :active_record_save!, :save!
def save(...)
save!(...)
rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved
false
end
def save!(**opts)
creating_record = new_record?
saved = false
# All callback code runs inside a single transaction
Foo.transaction do
# Throw :abort can be used inside callbacks to skip saving
catch(:abort) do
# Don't validate if validate: false was passed in
if opts[:validate] != false
raise ActiveRecord::RecordInvalid, self unless valid?
end
# Before save callbacks
before_save_callback unless some_attr_changed?
before_save_callback_on_create if creating_record && some_condition?
before_save_callback_on_update if creating_record
# Before create/update callbacks
if creating_record
before_create_callback if some_attr_changed? && some_other_attr_changed?
else
before_update_callback if some_other_condition?
end
# Save
saved = active_record_save!(**opts.merge(validate: false))
end
# Raise error if record wans't saved
raise ActiveRecord::RecordNotSaved unless saved
# After create/update callbacks
if creating_record
after_create_callback if some_condition?
else
after_update_callback
end
# After save callbacks
after_save_callback if some_other_attr_changed?
after_save_callback_on_create if creating_record
after_save_callback_on_update, if !creating_record && !some_other_condition?
end
rescue StandardError => e
# Signal "rollback"
rolledback!
raise e
end
# referenced methods would be defined below...
end
After the refactor you end up with some pretty ugly code. Make no mistake though, just because there were less lines before doesn't mean it was better or clearer. Our refactor has brought us numerous advantages. It's now possible to stick a debug statement at the top of save!
and step through all custom validations and callbacks. You can even be debugging another piece of code and step into the save
or save!
call and step through it. All the weird control flow that was buried in the Active Record documentation is now obvious. Also, now that we are dealing with plain old Ruby code we've opened up more avenues to factor this code out of our model.
Which is exactly what we wanted to do considering how large our model was. At Wonolo we have chosen the Command pattern to fill out our service layer so we explored factoring out the callback code into a command. We created two new commands, one for validating and one for saving. These commands can be filled with the code from the valid?
and save!
methods respectively.
def valid?(*args)
Commands::Foo::Validate.new(self, *args).call
end
def save!(**opts)
Commands::Foo::Save.new(self, **opts).call
end
With these two new command classes we now have a place to put code. Not only can we move the contents of these methods but with some minor tweaks we can start factoring out the callback method definitions themselves.
I think there are clear advantages to ditching callbacks for more traditional style code. As shown here it's possible without needing to change call sites all over the codebase. I think there is still more work to be done to get the code to the place we want it. However, this strategy provides a great starting point to get the ball rolling. From here I can easily see this code being broken up further into create and update commands. Even that may not be the correct stopping point. Depending on the complexity of the model, create and update could be subdivided further into finer grained commands.
So far I've also only written about doing this on a single model by hand. To do this to all models across a large code base is going to be a lot of effort. Fortunately, the code transformations to do this are quite rote and a good candidate for automation. So that's exactly what we did to perform this refactor across our entire codebase. However, that's a topic for another day, so stay tuned.