Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Calling a model "Update" leads to unexpected behavior

#5772

When you have these two models:

class Update < ActiveRecord::Base
has_many :comments end

and

class Comment < ActiveRecord::Base
belongs_to :update end

then issuing Comment.first.update_attribute(:body,"This fails to be saved") fails to save.

When you rename the Update model to e.g. Post then update_attribute works correctly.

It fails under Rails 3 and 2.3.8, it works under 2.3.5.

Reported by Erik van Eykelen · October 8th, 2010 @ 07:31 PM

State: stale
Milestone: none
Assigned to: Manfred Stienstra Manfred Stienstra
Importance: Low

Activity

  1. David Trasbo
    David Trasbo
    • State changed from new to incomplete
    • Importance changed from to Low

    Confirmed on Rails edge:

    $ script/rails g model update 
    ...
    $ script/rails g model comment update:references body:string
    ...
    $ rake db:migrate
    ...
    

    Models:

    class Update < ActiveRecord::Base
      has_many :comments
    end
    
    class Comment < ActiveRecord::Base
      belongs_to :update
    end
    
    $ script/rails c
    Loading development environment (Rails 3.1.0.beta)
    ruby-1.9.2-p0 > u = Update.create
     => #<Update id: 1, created_at: "2010-10-10 15:21:19", updated_at: "2010-10-10 15:21:19"> 
    ruby-1.9.2-p0 > u.comments << Comment.new(:body => 'Foo.')
     => [#<Comment id: 1, update_id: 1, body: "Foo.", created_at: "2010-10-10 15:21:38", updated_at: "2010-10-10 15:21:38">] 
    ruby-1.9.2-p0 > Comment.first.update_attribute(:body, 'Bar.')
     => true 
    ruby-1.9.2-p0 > Comment.first
     => #<Comment id: 1, update_id: 1, body: "Foo.", created_at: "2010-10-10 15:21:38", updated_at: "2010-10-10 15:21:38">
    

    Please provide a patch (https://rails.lighthouseapp.com/projects/8994/sending-patches) with a fix and/or a failing test.

    October 10th, 2010 @ 04:22 PM

  2. Andrea Campi
    Andrea Campi
    • Tag set to activerecord, patch

    It's not very surprising that your use case doesn't work: #update is a reserved word in ActiveRecord (it's not in the public API, but still documented).
    In fact, it was already forbidden for regular attributes and methods, but not enforced for associations:

    script/rails g model comment update:string
    
    class Comment < ActiveRecord::Base
    end
    
    $ rails c
    ruby-1.9.2-p0 > Comment.new(:body => 'test', :update => 'test')
    ActiveRecord::DangerousAttributeError: update is defined by ActiveRecord
    

    With the attached patch you will get the same exception in your use case, too.

    Note that both in your original scenario and after my patch, you can work around this issue by naming your association differently:

    class Comment < ActiveRecord::Base
      belongs_to :an_update, :class_name => Update
    end
    

    October 10th, 2010 @ 11:11 PM

  3. Erik van Eykelen
    Erik van Eykelen

    I totally agree with avoiding reserved words, I simply overlooked the fact that it applies to model names too. I knew about Sessions though ;-)

    Your patch is great for folks creating a new model from scratch. However, for existing projects, e.g. ones migrated from 2.3.5 where a model called Update still worked, this issue is a bit of pitfall since update actions silently fail (e.g. update_attribute() returns true even though the actual UPDATE was not carried out).

    I guess update actions could trow an error in, but I realize that differentiating between method names like update() and associations like :update is possibly expensive.

    October 11th, 2010 @ 02:27 PM

  4. Andrea Campi
    Andrea Campi

    In my patch, #instance_method_already_implemented? will raise an exception at load time, although that is not apparent from reading the patch.
    So in theory what you say should not happen.

    Do you have a use case that results in modifications being silently ignored? If so, I will be happy to look at ways to address it.

    October 12th, 2010 @ 06:44 AM

  5. Andrea Campi
    Andrea Campi
    • Tag changed from activerecord, patch to 3.0, activerecord, edge, patch

    October 12th, 2010 @ 06:45 AM

  6. Jeff Kreeftmeijer
    Jeff Kreeftmeijer
    • Tag cleared.

    Automatic cleanup of spam.

    October 15th, 2010 @ 07:15 AM

  7. Andrea Campi
    Andrea Campi
    • Tag set to 3.0, activerecord, patch

    October 16th, 2010 @ 08:47 PM

  8. Jeff Kreeftmeijer
    Jeff Kreeftmeijer
    • Tag cleared.

    Automatic cleanup of spam.

    October 19th, 2010 @ 07:09 AM

  9. Ryan Bigg
    Ryan Bigg

    Automatic cleanup of spam.

    October 21st, 2010 @ 03:40 AM

  10. Santiago Pastorino
    Santiago Pastorino
    • State changed from incomplete to open

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

    February 9th, 2011 @ 12:31 AM

  11. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 9th, 2011 @ 12:31 AM

  12. bingbing