accepts_nested_models_for (& autosave) versus :touch

[Bryan Stearns]

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

[Lawrence Pit]

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

[Bryan Stearns]

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