before_create considered harmful

I had a fsck of a time today working with rails. Normally I love Ruby on Rails, but today it gave me the screaming shifts!

I had a model class to which I wanted to add some sensible default values to satisfy some constraints in the database.
The logical place for that was before_create, so I did something like the following

class MyModel < ActiveRecord::Base

def before_create

self.foo ||= true
self.bar ||= Time.now
self.baz ||= false

end

end

Now, I would have thought that would work, but whenever I tried to save an instance of my class to the database the save method would return false πŸ™
Hmmmm. Ok, let’s add some logging code and see what the problem is… Are there any errors? Nope. Does the instance think it’s valid? Yes. OK, that’s weird. Maybe there is something going on and the exception is getting eaten by some other code in the application (there are three of us working on this code and we’re not all in the same state, so who knows what one of the others may have done πŸ˜‰ ). I try creating an instance using the rails console. Same thing (which is to be expected, but I was starting to get a little desperate by this stage).

What was the next thing to do? Go through the code and remove things line by line until I start getting some exceptions. Finally in desperation I took out my before_create method, and lo and behold my error messages come back. It was then that I was struck by the thought that Ruby methods take their return value from the last expression evaluated in their body, so the before_create method was returning false. It seems that there is a nice undocumented “feature” in ActiveRecord that allows application code to stop an object being saved to the database by returning false from before_create, and I imagine before_save.

A little note to that affect in the documentation would have saved me quite a bit of time today.

Ah well. Live and learn I guess πŸ™‚

11 Replies to “before_create considered harmful”

  1. Thanks for the posting, though i do consider this a feature as it is nice that i can do a before_create that checks to see if something exists before creating another one.. such as the case for tagging. so i can just return false if one exists…

  2. Oh, and I do think the title of this article is misleading. Dijkstra didn’t say that goto was harmful if you didn’t fully understand its behavior. πŸ™‚

    (Are these comments really moderated??)

    ///ark

  3. The comments are moderated when I get time to look through them, which isn’t very often these days. As for a misleading title, I was a little annoyed when I wrote the post (to say the least). It wasn’t a rant against ruby or rails, rather against the state of the documentation at the time. Things have improved quite a lot since then πŸ˜‰

  4. The best place for default values are your migrations.

    As for your Gotcha! the callbacks documentation mentions that any before_* callbacks that return false will cancel the entire operation. I’m not sure if it was worded that way when you wrote this post.

  5. I’m in agreement with you Daniel that this is horrible – it seems like a bug to me. Anything that creates this sitution must be wrong surely:

    @user.valid?
    => true
    @user.save
    => false
    @user.errors.full_messages
    => []

    I pulled half my beard out trying to get to the bottom of this today. There’s nothing in the rails documentation to say that if a method called by before_create returns false then saving will fail. In my case everything was fine – i was setting one of my fields according to the state of an associated object, and the set value happened to be ‘false’. This was fine and totally expected. But, it was breaking my save.

    E – you say that the callbacks documentation describes this – and, indeed it does, but it’s a bit buried halfway down the page on the callbacks module: to quote

    Canceling callbacks

    If a before_* callback returns false, all the later callbacks and the associated action are cancelled. If an after_* callback returns false, all the later callbacks are cancelled. Callbacks are generally run in the order they are defined, with the exception of callbacks defined as methods on the model, which are called last.

  6. Come on, this is pretty obvious when you think of it πŸ™‚

    Besides, http://api.rubyonrails.org/ says pretty clearly in the “Canceling callbacks” section:

    If a before_* callback returns false, all the later callbacks and the associated action are cancelled.

    But still, I admit, it’s a pitfall that it’s good to warn about.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.