This design issue was addressed in Rails 5.0. Read more about the change here
The Code
The following class combines multiple attributes into one boolean attribute. Most of the time it's an optimization, but sometimes it's mandatory because the boolean calculation can't be expressed in SQL.
Can you spot the bug?
class Movie < ApplicationRecord
validates_presence_of :title
scope :streaming, -> { where(is_streaming: true) }
before_save :autoset_is_streaming
def on_netflix_instant?
netflix_instant_url != nil
end
def on_hulu?
hulu_url != nil
end
def on_youtube?
youtube_url != nil
end
private
def autoset_is_streaming
self.is_streaming = on_netflix_instant? || on_hulu? || on_youtube?
end
end
The Problem
Your movie class is working great as you read in a bunch of streaming movies, but eventually the movies won't save, so you jump into the console:
> Movie.create!(title: 'Grand Budapest Hotel')
(0.2ms) BEGIN
(0.2ms) ROLLBACK
ActiveRecord::RecordNotSaved: ActiveRecord::RecordNotSaved
> movie = Movie.new(title: 'Grand Budapest Hotel')
> movie.valid?
=> true
> movie.save
(0.2ms) BEGIN
(0.2ms) ROLLBACK
=> false
> movie.save!
(0.2ms) BEGIN
(0.2ms) ROLLBACK
ActiveRecord::RecordNotSaved: ActiveRecord::RecordNotSaved
> movie.errors.full_messages
=> []
The movie is valid, but it doesn't save? What's is going on?
The Feature
The bug is caused by an ActiveRecord feature called "Canceling Callbacks":
If a before_* callback returns false, all the later callbacks and the associated action are cancelled.
The example code is setting the is_streaming
attribute to false
, and thanks to ruby's non-explicit returns, the function returns false
, canceling the save. No validation errors are stored because the validation passed.
The Fix
Once you uncover the issue, the fix is simple:
def autoset_is_streaming
self.is_streaming = on_netflix_instant? || on_hulu? || on_youtube?
true
end
nil would work too, but I prefer true indicating the value was successfully set. After running into this problem occasionally in my first couple years of Ruby on Rails programming, I engrained this thought process into my ActiveRecord programming:
- Writing a
before_*
callback? - Could it
return false
? - Don't!
return true
!
I see how canceling an ActiveRecord
action is useful, but I always cancel it with validation errors or exceptions. Anyway, I hope you learned something and maybe avoided some future head-scratching.