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 ||= falseend
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 🙂
Damn, yeah that sucks.
Thanks for the warning
Yeah, this happens with before_filters in controllers as well.
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…
I agree, it’s a great feature. It would just be nice if it was better documented 😉
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
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 😉
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.
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.
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.
This is actually documented here:
http://api.rubyonrails.org/classes/ActiveRecord/Callbacks.html
Under the section Canceling callbacks.
You just got me out of big of trouble. Thank you very, very much.