Callbacks and setting false to attributes. #2888

Closed
fivetwentysix opened this Issue Sep 6, 2011 · 2 comments

Comments

Projects
None yet
2 participants
@fivetwentysix

Hello there!

While I was developing my Rails 3 application, with the release of Lion and 3.1, my application got on board and migrated to:

  • Rails 3.1 from Rails 3.0.10
  • MySQL 5.x something to Postgres 9.04

And then something strange happened. My tests started breaking and I could not for the life of me figure it out because this application was big, and the error messages I got from this are, to put it politely, not too helpful.

So I found the ugly fix. And now I want the Ruby fix.

I would actually like to write the patch my self, even though I have not much experience committing to the Rails source code. But if someone could, perhaps mentor me a little bit for this case. I'd be more than happy to contribute as much as I can to Rails.

Anyways, here is a demo application. Clone it, run rails s, goto http://localhost:3000/people/new. Create a person, and bang, validation error with no error message!

The source of the problem, and solution is in app/models/person.rb.

I think I should also add, I'm on Ruby 1.9.2.

@asanghi

This comment has been minimized.

Show comment Hide comment
@asanghi

asanghi Sep 6, 2011

Contributor

The problem is simply with the return value of the lambda. A slightly less ugly fix might be

before_save lambda{self.awesome = false; true}

I'm sure there is no Ruby bug to be fixed here and I'm fairly certain there is not even a Rails core issue here. before_save callback chain fails if any callback returns false. If you happen to write your callback in a way that it returns false then the callback chain will break. The workaround is to not return false.

The recommended/accepted design for active record callbacks is to provide a symbol of the method you wish to invoke

before_save :make_awesome

def make_awesome
  # do something awesome here 
  self.awesome = false
  # return true to continue callback chain and false to break it
  return true
  # OR
  self.tap do |me|
    me.awesome = false
  end
end

Re-reading your question --

Are you saying this was working in Rails 3.0.10 and is not working in Rails 3.1? If so, i'm sure it's Rails 3.0.10 that needs to be fixed to plug this whole that you were able to exploit there. :)
Read #2881 , #956
http://factore.ca/on-the-floor/77-rails-gotcha-with-before-validation

Contributor

asanghi commented Sep 6, 2011

The problem is simply with the return value of the lambda. A slightly less ugly fix might be

before_save lambda{self.awesome = false; true}

I'm sure there is no Ruby bug to be fixed here and I'm fairly certain there is not even a Rails core issue here. before_save callback chain fails if any callback returns false. If you happen to write your callback in a way that it returns false then the callback chain will break. The workaround is to not return false.

The recommended/accepted design for active record callbacks is to provide a symbol of the method you wish to invoke

before_save :make_awesome

def make_awesome
  # do something awesome here 
  self.awesome = false
  # return true to continue callback chain and false to break it
  return true
  # OR
  self.tap do |me|
    me.awesome = false
  end
end

Re-reading your question --

Are you saying this was working in Rails 3.0.10 and is not working in Rails 3.1? If so, i'm sure it's Rails 3.0.10 that needs to be fixed to plug this whole that you were able to exploit there. :)
Read #2881 , #956
http://factore.ca/on-the-floor/77-rails-gotcha-with-before-validation

@fivetwentysix

This comment has been minimized.

Show comment Hide comment
@fivetwentysix

fivetwentysix Sep 7, 2011

Thanks for the great response!

Thanks for the great response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment