rails/rails — pull request #18198 by al2o3cr (2014-12-25 19:00:06+00:00)
Title: Only redefine association autosave callback if it isn't defined by the current class

Addresses #17105 and #16494. Reverts the majority of the changes from #15358 but retains the test coverage.

TL;DR on the discussion in #17105: the fix in #15358 ensured that subclasses that redeclare associations get newly-defined autosave callbacks. Unfortunately, enabling `accepts_nested_attributes_for` can cause this redefinition within a single class, breaking the order of callbacks and causing records to be saved in the wrong order.

This PR reinstates the method-definition check removed in #15358 but restricts it to only match definitions on the current class rather than inherited ones.

Several other changes are included, mostly for symmetry and to prevent additional similar bugs:

* the definition in the `reflection.has_one?` branch now uses `define_non_cyclic_method` to match the other branches of the conditional. It could be argued that two models who both `has_one` each other are probably incorrect DB design, but that still shouldn't produce infinite recursion.

* similarly, the `method_defined?` check in the validation conditional (line 208) has been changed to match the check for `save_method` on line 182. I don't have a test case for this, but it would likely manifest in the same "redefinition in subclasses" sorts of situations that #15358 addressed.

I've included the test written by @pranas and mentioned in https://github.com/rails/rails/issues/17015#issuecomment-67736744 as a separate commit as it accurately reflects the code's authorship. Let me know if I should squash, etc.

4.2.0 is also affected by this bug. Happy to backport this when it gets merged.

/cc @tenderlove @sgrif