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, 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

Hi Bryan,

Pls create a ticket at , 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

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