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.

:autosave => true + accepts_nested_attributes_for = circular validation & blown stack

#3588

Consider the following:

class Discussion < ActiveRecord::Base
  belongs_to :context, :polymorphic => true
  has_many :messages, :dependent => :destroy, :inverse => :discussion
  accepts_nested_attributes_for :messages
end

class Message < ActiveRecord::Base
  belongs_to :discussion, :inverse => :messages, :autosave => true
  validates_presence_of :discussion

  def before_validation_on_create
    self.discussion ||= Discussion.new
  end
end

Scenario 1:
Create a new message. Save the message. Discussion gets created pre-validation, the discussion gets validated and everything is saved properly. All is well, huzzah.

Scenario 2:
Create a new discussion together with nested message attributes. Save the discussion. It attempts to validate its messages, but they in turn try to validate their discussion, which tries to validate its messages... it recurses and blows stack. Boo.

This scenario can be prevented by turning off autosave => true, at the cost of breaking scenario 1.

This can also be prevented by turning off inverse (running from http://github.com/Fingertips/rails), at the cost of bug #2815 (#1943 may also be relevant), which makes Message's validation of its parent's existence fail. Additionally @discussion.messages.first.discussion != @discussion, which causes other issues. Fingertips' inverse_of patches nicely solve this.

I'm not sure how to fix this. Perhaps one could flag a record as having been validated at the BEGINNING of its validation cycle, and then not try to validate it again if that flag is set? This would prevent recursion, but might cause issues with mutually dependent validations of some sort that I'm failing to imagine.

Reported by Sai Emrys · December 16th, 2009 @ 09:51 PM

State: resolved
Milestone: none
Assigned to: Eloy Duran Eloy Duran
Importance: Low

Activity

  1. Manfred Stienstra
    Manfred Stienstra
    • Title changed from :autosave => true + accepts_nested_attributes_for = circular validation & blown stack to :autosave => true + accepts_nested_attributes_for = circular validation & blown stack
    • Assigned user set to Eloy Duran

    Eloy, would you mind looking at this?

    December 16th, 2009 @ 10:08 PM

  2. Sai Emrys
  3. Eloy Duran
    Eloy Duran

    I’m actually pretty glad you run into this problem, as you could verify the patch on #3533 for me :)

    Please report on that ticket if the patch fixes your problem as expected, thanks.

    December 17th, 2009 @ 10:44 AM

  4. Eloy Duran
    Eloy Duran

    Actually, instead of only that patch, could you try this Rails branch? http://github.com/Fingertips/rails/tree/2-3-stable

    It contains the aforementioned patch, plus a few other related ones that I'd also love to be tested by others.

    December 17th, 2009 @ 02:41 PM

  5. Eloy Duran
    Eloy Duran
    • State changed from new to resolved

    The :inverse_of patches have been pushed to 2-3-stable, closing for now.

    December 28th, 2009 @ 08:53 PM