This project is archived and is in readonly mode.

#3560 ✓resolved
tankwanghow

accepts_nested_attributes_for with :touch => ture cause ActiveRecord::StaleObjectError

Reported by tankwanghow | December 11th, 2009 @ 02:35 AM

I have the following models, I have optimistic locking ON using( adding lock_version to Invoice model) :-

class Invoice < ActiveRecord::Base
  has_many :line_items
  accepts_nested_attributes_for :line_items
end

class LineItem < ActiveRecord::Base
  belongs_to :invoice, :touch => true
end

And I am using a multiple-models form, with javascript to add or remove LineItem (base on http://github.com/ryanb/complex-form-examples)

Without :touch => true, the code work fine.
With :touch => true, I can add one LineItem with more than one LineItem, ActiveRecord::StaleObjectError will be raise

After looking at the log file, I found out the the lock_version was being updated multiple times.

Is there a way to touch the parent model once ??

Comments and changes to this ticket

  • tankwanghow

    tankwanghow December 11th, 2009 @ 02:54 AM

    Sorry about the formatting, this is better.

    class Invoice < ActiveRecord::Base
      has_many :line_items 
      accepts_nested_attributes_for :line_items 
    end
    
    class LineItem < ActiveRecord::Base
      belongs_to :invoice, :touch => true 
    end
    

    I have found a solution from UVSoft plugins :-

    http://github.com/UVSoft/improved_touch http://github.com/UVSoft/touching_nested_attributes

    I found this plugin is very useful, can these plugins be made it to the rails core?

  • Ivan Ukhov

    Ivan Ukhov December 11th, 2009 @ 08:17 AM

    Here is my patch. This is also useful for fragment caching with cach-keys based on timestamps (<% cache @record do %> <% end %>). When fragments include information not only about the record itself, but also about its association, and when you update nested attributes without updating record, the fragments do not expire, but they should. This patch touch the record without saving when nested attributes are being changed.

  • Eloy Duran

    Eloy Duran December 11th, 2009 @ 10:21 AM

    Seems valid, however, you patch is lacking test coverage. Could you please add tests to the patch?

    Also, it seems by glancing over the patch that this would always touch the parent, whereas I'd expect that to only happen if :touch => true' was set on the parent. Or did I misinterpret the code?

  • Ivan Ukhov

    Ivan Ukhov December 11th, 2009 @ 08:53 PM

    Yes, the patch really touches always. In my case I don't use :touch => true at all, that's why I haven't implement this option. And about testing, to my shame, I dont know how to do it.

  • tankwanghow

    tankwanghow December 12th, 2009 @ 06:25 PM

    Using UVSoft timestamp.rb patch, I have added a feature in accepts_nested_attributes_for method in nested_attribute.rb, which will add an attr_accessor :silent to the association model. Then added check condition in associations.rb in add_touch_callbacks method to check for silent attribute is true, then the touch callback will execute the update_timestamp method in timestamp.rb, Yes touch will be call multiple time, with :_silent => true, save! will not be call, thus solved the ActiveRecord::StaleObjectError

    In the view files, just add a hidden field named :_silent

    @@@ruby object.hidden_field :_silent, :value => true

    
    
  • tankwanghow

    tankwanghow December 12th, 2009 @ 06:28 PM

    the belongs_to :invoice, :touch => true can still be use.

  • tankwanghow

    tankwanghow December 13th, 2009 @ 07:11 AM

    Sorry, I have submitted the wrong patch.

    This is the correct one.

    Please Review, I think this feature is very useful for optimistic locking in nested_attributes and javascript style of multiple-models form.

    If you are already using accepts_nested_attributes_for like the following :-

    class Invoice < ActiveRecord::Base
      has_many :line_items 
      accepts_nested_attributes_for :line_items 
    end
    
    class LineItem < ActiveRecord::Base
      belongs_to :invoice, :touch => true 
    end
    

    No code need to be change in the model or the controller.

    In your view just add a

    hidden_field :_silent_touch, :value => true
    
    to your nested_attribute (ie. LineItem view)

    Then optimistic locking will work on nested attributes form.

    I am not good in writing test-unit can anyone help!!!

    Thank You,

  • tankwanghow

    tankwanghow December 13th, 2009 @ 07:16 AM

    • Tag changed from accepts_nested_attributes_for, active_record, touch to accepts_nested_attributes_for, active_record, locking, optimistic, touch
  • Eloy Duran

    Eloy Duran December 28th, 2009 @ 05:31 PM

    • State changed from “new” to “incomplete”

    It would be great if you could extract a small applications showing the problem and your solution. Then I'll be able to write the appropriate test coverage and judge the patch.

  • tankwanghow

    tankwanghow January 2nd, 2010 @ 02:29 AM

    Thank for your concern on my problem.

    Here is the two rails application opt_lock_nest_problem.tar.gz and opt_lock_nest_solution.tar.gz

    In order for the solution to work, you have to add the nested_attributes_silent_touch patch.

    But I am still having problem with lock_version, in my last two spec.

    Please review and advise.

    Thank you

  • tankwanghow

    tankwanghow January 2nd, 2010 @ 06:50 AM

    I have found a problem with my patch, let me fix the bug and I will send a new patch.

  • Eloy Duran

    Eloy Duran January 2nd, 2010 @ 01:26 PM

    Thanks for the update. I'll await your new patch.

  • tankwanghow

    tankwanghow January 3rd, 2010 @ 06:09 AM

    • Tag changed from accepts_nested_attributes_for, active_record, locking, optimistic, touch to accepts_nested_attributes_for, active_record, locking, optimistic, patch, touch

    Sorry for the delay, hopefully this time, I get it right...

    Sample application opt_lock_nest.zip and the patch fix_nested_opt_lock.diff

    Run the project_spec.rb before the patch -> all specs fail
    After the patch, run the spec again -> all specs pass

    Thank you

  • Eloy Duran

    Eloy Duran January 4th, 2010 @ 11:16 AM

    Thanks for the application. I;ve ran it against 2.3.5 and see the StaleObjectError's. However, if I run it against the current 2-3-stable branch, I don't get these errors. The specs still fail though, but it's because the lock_version doesn't match the expected versions.

    The output I get:

    % spec spec/models/project_spec.rb
    FFFF
    
    1)
    'Project when save with nested attributes should not have STALE object error and :lock_version should be 0' FAILED
    expected: 0,
         got: 3 (using ==)
    ./spec/models/project_spec.rb:15:
    
    2)
    'Project when save with nested attributes should not have STALE object error and :lock_version should be 7' FAILED
    expected: 7,
         got: 8 (using ==)
    ./spec/models/project_spec.rb:38:
    
    3)
    'Project when save with nested attributes should not have STALE object error and :lock_version should be 0' FAILED
    expected: 0,
         got: 2 (using ==)
    ./spec/models/project_spec.rb:48:
    
    4)
    'Project when save with nested attributes should not have STALE object error and :lock_version should be 4' FAILED
    expected: 4,
         got: 5 (using ==)
    ./spec/models/project_spec.rb:66:
    
    Finished in 0.332062 seconds
    
    4 examples, 4 failures
    

    Could you check that, if it now works as you'd want, and if the versions not matching exactly is irrelevant or not?

    In order to check that out do the following in the application source root:

    cd vendor
    git clone git://github.com/rails/rails.git
    cd rails
    git checkout 2-3-stable
    

    Thanks!

  • tankwanghow

    tankwanghow January 7th, 2010 @ 01:06 AM

    As long as, optimistic locking is working with :touch => true for nested_attributes I don't worry about the lock_version number.

    So the new rails 2-3-stable WORKS !!!

    Thank You

  • Eloy Duran

    Eloy Duran January 7th, 2010 @ 03:18 PM

    • State changed from “incomplete” to “resolved”

    Great, no problem :)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Pages