Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Make sure to not add autosave callbacks multiple times. [#3575 state:…

…resolved]

This makes sure that, in a HABTM association, only one join record is craeted.
  • Loading branch information...
commit 146a7505680cbb646c0b9d55dca7cc8494b36e47 1 parent 3470b30
@alloy alloy authored
View
40 activerecord/lib/active_record/autosave_association.rb
@@ -156,6 +156,13 @@ def #{type}(name, options = {})
# Adds a validate and save callback for the association as specified by
# the +reflection+.
+ #
+ # For performance reasons, we don't check whether to validate at runtime,
+ # but instead only define the method and callback when needed. However,
+ # this can change, for instance, when using nested attributes. Since we
+ # don't want the callbacks to get defined multiple times, there are
+ # guards that check if the save or validation methods have already been
+ # defined before actually defining them.
def add_autosave_association_callbacks(reflection)
save_method = "autosave_associated_records_for_#{reflection.name}"
validation_method = "validate_associated_records_for_#{reflection.name}"
@@ -163,28 +170,33 @@ def add_autosave_association_callbacks(reflection)
case reflection.macro
when :has_many, :has_and_belongs_to_many
- before_save :before_save_collection_association
+ unless method_defined?(save_method)
+ before_save :before_save_collection_association
- define_method(save_method) { save_collection_association(reflection) }
- # Doesn't use after_save as that would save associations added in after_create/after_update twice
- after_create save_method
- after_update save_method
+ define_method(save_method) { save_collection_association(reflection) }
+ # Doesn't use after_save as that would save associations added in after_create/after_update twice
+ after_create save_method
+ after_update save_method
+ end
- if force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false)
+ if !method_defined?(validation_method) &&
+ (force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false))
define_method(validation_method) { validate_collection_association(reflection) }
validate validation_method
end
else
- case reflection.macro
- when :has_one
- define_method(save_method) { save_has_one_association(reflection) }
- after_save save_method
- when :belongs_to
- define_method(save_method) { save_belongs_to_association(reflection) }
- before_save save_method
+ unless method_defined?(save_method)
+ case reflection.macro
+ when :has_one
+ define_method(save_method) { save_has_one_association(reflection) }
+ after_save save_method
+ when :belongs_to
+ define_method(save_method) { save_belongs_to_association(reflection) }
+ before_save save_method
+ end
end
- if force_validation
+ if !method_defined?(validation_method) && force_validation
define_method(validation_method) { validate_single_association(reflection) }
validate validation_method
end
View
3  activerecord/lib/active_record/nested_attributes.rb
@@ -229,6 +229,7 @@ def accepts_nested_attributes_for(*attr_names)
end
reflection.options[:autosave] = true
+ add_autosave_association_callbacks(reflection)
self.nested_attributes_options[association_name.to_sym] = options
# def pirate_attributes=(attributes)
@@ -239,8 +240,6 @@ def #{association_name}_attributes=(attributes)
assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes)
end
}, __FILE__, __LINE__
-
- add_autosave_association_callbacks(reflection)
else
raise ArgumentError, "No association found for name `#{association_name}'. Has it been defined yet?"
end
View
29 activerecord/test/cases/autosave_association_test.rb
@@ -30,11 +30,40 @@ def test_autosave_should_be_a_valid_option_for_has_and_belongs_to_many
assert base.valid_keys_for_has_and_belongs_to_many_association.include?(:autosave)
end
+ def test_should_not_add_the_same_callbacks_multiple_times_for_has_one
+ assert_no_difference_when_adding_callbacks_twice_for Pirate, :ship
+ end
+
+ def test_should_not_add_the_same_callbacks_multiple_times_for_belongs_to
+ assert_no_difference_when_adding_callbacks_twice_for Ship, :pirate
+ end
+
+ def test_should_not_add_the_same_callbacks_multiple_times_for_has_many
+ assert_no_difference_when_adding_callbacks_twice_for Pirate, :birds
+ end
+
+ def test_should_not_add_the_same_callbacks_multiple_times_for_has_and_belongs_to_many
+ assert_no_difference_when_adding_callbacks_twice_for Pirate, :parrots
+ end
+
private
def base
ActiveRecord::Base
end
+
+ def assert_no_difference_when_adding_callbacks_twice_for(model, association_name)
+ reflection = model.reflect_on_association(association_name)
+ assert_no_difference "callbacks_for_model(#{model.name}).length" do
+ model.send(:add_autosave_association_callbacks, reflection)
+ end
+ end
+
+ def callbacks_for_model(model)
+ model.instance_variables.grep(/_callbacks$/).map do |ivar|
+ model.instance_variable_get(ivar)
+ end.flatten
+ end
end
class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
View
9 activerecord/test/cases/nested_attributes_test.rb
@@ -354,6 +354,15 @@ def test_should_define_an_attribute_writer_method_for_the_association
assert_respond_to @pirate, association_setter
end
+ def test_should_save_only_one_association_on_create
+ pirate = Pirate.create!({
+ :catchphrase => 'Arr',
+ association_getter => { 'foo' => { :name => 'Grace OMalley' } }
+ })
+
+ assert_equal 1, pirate.reload.send(@association_name).count
+ end
+
def test_should_take_a_hash_with_string_keys_and_assign_the_attributes_to_the_associated_models
@alternate_params[association_getter].stringify_keys!
@pirate.update_attributes @alternate_params
Please sign in to comment.
Something went wrong with that request. Please try again.