Rails core - Sep 2009 - accepts_nested_models_for (& autosave) versus :touch

Hi... I''m trying to use nested models in a situation where
I''ve been using
:touch => true to update a parent model, resulting in an infinite loop that
quickly blows the stack. Here''s a simple relationship that exhibits the
problem, an invoice and its lineitems: the invoice has a "total"
attribute
that sums its lineitems'' "amount" attributes.

class LineItem < ActiveRecord::Base
  # t.integer :amount
  # t.integer :invoice_id
  belongs_to :invoice, :touch => true
end

class Invoice < ActiveRecord::Base
  # t.integer :total
  # t.timestamps
  has_many :line_items
  accepts_nested_attributes_for :line_items
  before_save :summarize

  def summarize
    self.total = line_items.map(&:amount).sum
  end
end

With these models, script/console:
>> Invoice.create
=> #<Invoice id: 1, total: 0, created_at: "2009-09-30 05:04:41",
updated_at:
"2009-09-30 05:04:41">
>> LineItem.create(:invoice_id => 1, :amount => 10)
This fails:
- The line-item saves itself, then the generated
#belongs_to_touch_after_save_or_destroy_for_invoice causes the
invoice''s
updated_at to be changed, then the invoice to be saved.
- Saving the invoice fires its before_save callback, which sums up its
total, which is then saved.
- Because accepts_nested_models_for turns on autosave for the association,
the invoice then saves its lineitems.
- Saving the lineitems triggers
#belongs_to_touch_after_save_or_destroy_for_invoice again, and the cycle
repeats until the stack overflows.

I was hoping there''s a way to break this cycle. I don''t want
to get rid of
my use of :touch; it not only simplifies doing this sort of summarization in
the database, it also makes it easy to come up with cache keys for UI
fragments (I can just use the parent model''s updated_at time).

So, I focused on why the lineitems were being saved after the parent model
was touched; this let me to
AutosaveAssociation#save_collection_association<http://github.com/rails/rails/blob/v2.3.4/activerecord/lib/active_record/autosave_association.rb#L272>,
which iterates over the loaded records and saves them, apparently even if
they''re not new or dirty (the #save call on line 295) - this surprised
me,
so I tried hacking my version to change line 294 from
"elsif autosave" to "elsif autosave &&
record.changed?" -- and my problem
went away.

I''m starting down the path of writing tests and creating a patch;
before I
do, anyone see why that save (on line 295) needs to happen even if the
record isn''t dirty?

Thanks,
...Bryan

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Hi Bryan,

Pls create a ticket at http://rails.lighthouseapp.com, so we can move 
the discussion there.

What you suggest makes sense. However, instead of modifying that line 
you''re referring to I suggest to modify 
+associated_records_to_validate_or_save+ instead. I think it should be:

    def associated_records_to_validate_or_save(association, new_record, 
autosave)
      if new_record
        association
      elsif autosave
        association.select { |record| record.new_record? || 
record.changed? }
      else
        association.select { |record| record.new_record? }
      end
    end


Greets,
Lawrence
> Hi... I''m trying to use nested models in a situation where
I''ve been
> using :touch => true to update a parent model, resulting in an 
> infinite loop that quickly blows the stack. Here''s a simple 
> relationship that exhibits the problem, an invoice and its lineitems: 
> the invoice has a "total" attribute that sums its
lineitems'' "amount"
> attributes.
>
> class LineItem < ActiveRecord::Base
>   # t.integer :amount
>   # t.integer :invoice_id
>   belongs_to :invoice, :touch => true
> end
>
> class Invoice < ActiveRecord::Base
>   # t.integer :total
>   # t.timestamps
>   has_many :line_items
>   accepts_nested_attributes_for :line_items
>   before_save :summarize
>
>   def summarize
>     self.total = line_items.map(&:amount).sum
>   end
> end
>
> With these models, script/console:
> >> Invoice.create
> => #<Invoice id: 1, total: 0, created_at: "2009-09-30
05:04:41",
> updated_at: "2009-09-30 05:04:41">
> >> LineItem.create(:invoice_id => 1, :amount => 10)
>
> This fails:
> - The line-item saves itself, then the generated 
> #belongs_to_touch_after_save_or_destroy_for_invoice causes the 
> invoice''s updated_at to be changed, then the invoice to be saved.
> - Saving the invoice fires its before_save callback, which sums up its 
> total, which is then saved.
> - Because accepts_nested_models_for turns on autosave for the 
> association, the invoice then saves its lineitems.
> - Saving the lineitems triggers 
> #belongs_to_touch_after_save_or_destroy_for_invoice again, and the 
> cycle repeats until the stack overflows.
>
> I was hoping there''s a way to break this cycle. I don''t
want to get
> rid of my use of :touch; it not only simplifies doing this sort of 
> summarization in the database, it also makes it easy to come up with 
> cache keys for UI fragments (I can just use the parent model''s 
> updated_at time).
>
> So, I focused on why the lineitems were being saved after the parent 
> model was touched; this let me to 
> AutosaveAssociation#save_collection_association 
>
<http://github.com/rails/rails/blob/v2.3.4/activerecord/lib/active_record/autosave_association.rb#L272>,
> which iterates over the loaded records and saves them, apparently even 
> if they''re not new or dirty (the #save call on line 295) - this 
> surprised me, so I tried hacking my version to change line 294 from
> "elsif autosave" to "elsif autosave &&
record.changed?" -- and my
> problem went away.
>
> I''m starting down the path of writing tests and creating a patch; 
> before I do, anyone see why that save (on line 295) needs to happen 
> even if the record isn''t dirty?
>
> Thanks,
> ...Bryan
>
>
> >

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Thanks, Lawrence - I found the existing ticket 2578 which seems to be
another case of this problem, and added my explanation and both our patches
to it. (In the meantime, I found that both our patches cause lots of other
tests to fail... I''m still trying to figure out why.)

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/2578

Best,
...Bryan

On Wed, Sep 30, 2009 at 2:47 PM, Lawrence Pit
<lawrence.pit@gmail.com>wrote:
>  Hi Bryan,
>
> Pls create a ticket at http://rails.lighthouseapp.com, so we can move the
> discussion there.
>
> What you suggest makes sense. However, instead of modifying that line
> you''re referring to I suggest to modify
> +associated_records_to_validate_or_save+ instead. I think it should be:
>
>     def associated_records_to_validate_or_save(association, new_record,
> autosave)
>       if new_record
>         association
>       elsif autosave
>         association.select { |record| record.new_record? || record.changed?
> }
>       else
>         association.select { |record| record.new_record? }
>       end
>     end
>
>
> Greets,
> Lawrence
>
> Hi... I''m trying to use nested models in a situation where
I''ve been using
> :touch => true to update a parent model, resulting in an infinite loop
that
> quickly blows the stack. Here''s a simple relationship that
exhibits the
> problem, an invoice and its lineitems: the invoice has a "total"
attribute
> that sums its lineitems'' "amount" attributes.
>
> class LineItem < ActiveRecord::Base
>   # t.integer :amount
>   # t.integer :invoice_id
>   belongs_to :invoice, :touch => true
> end
>
> class Invoice < ActiveRecord::Base
>   # t.integer :total
>   # t.timestamps
>   has_many :line_items
>   accepts_nested_attributes_for :line_items
>   before_save :summarize
>
>   def summarize
>     self.total = line_items.map(&:amount).sum
>   end
> end
>
> With these models, script/console:
> >> Invoice.create
> => #<Invoice id: 1, total: 0, created_at: "2009-09-30
05:04:41",
> updated_at: "2009-09-30 05:04:41">
> >> LineItem.create(:invoice_id => 1, :amount => 10)
>
> This fails:
> - The line-item saves itself, then the generated
> #belongs_to_touch_after_save_or_destroy_for_invoice causes the
invoice''s
> updated_at to be changed, then the invoice to be saved.
> - Saving the invoice fires its before_save callback, which sums up its
> total, which is then saved.
> - Because accepts_nested_models_for turns on autosave for the association,
> the invoice then saves its lineitems.
> - Saving the lineitems triggers
> #belongs_to_touch_after_save_or_destroy_for_invoice again, and the cycle
> repeats until the stack overflows.
>
> I was hoping there''s a way to break this cycle. I don''t
want to get rid of
> my use of :touch; it not only simplifies doing this sort of summarization
in
> the database, it also makes it easy to come up with cache keys for UI
> fragments (I can just use the parent model''s updated_at time).
>
> So, I focused on why the lineitems were being saved after the parent model
> was touched; this let me to
>
AutosaveAssociation#save_collection_association<http://github.com/rails/rails/blob/v2.3.4/activerecord/lib/active_record/autosave_association.rb#L272>,
> which iterates over the loaded records and saves them, apparently even if
> they''re not new or dirty (the #save call on line 295) - this
surprised me,
> so I tried hacking my version to change line 294 from
> "elsif autosave" to "elsif autosave &&
record.changed?" -- and my problem
> went away.
>
> I''m starting down the path of writing tests and creating a patch;
before I
> do, anyone see why that save (on line 295) needs to happen even if the
> record isn''t dirty?
>
> Thanks,
> ...Bryan
>
>
>
>
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---