From 938c0ee0455b8e784a771ce31631d9ec376ee6ab Mon Sep 17 00:00:00 2001 From: Alexey Kovyrin Date: Sat, 12 Sep 2009 14:55:34 +0200 Subject: [PATCH] Define autosave association validation methods only when needed. [#3161 state:resolved] Signed-off-by: Eloy Duran --- .../lib/active_record/autosave_association.rb | 13 +- .../test/cases/autosave_association_test.rb | 115 ++++++++++++++++++ activerecord/test/models/pirate.rb | 9 +- activerecord/test/models/ship.rb | 2 +- 4 files changed, 132 insertions(+), 7 deletions(-) diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index bfae5933ea649..7b5bd33863b1f 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -158,7 +158,7 @@ def #{type}(name, options = {}) def add_autosave_association_callbacks(reflection) save_method = "autosave_associated_records_for_#{reflection.name}" validation_method = "validate_associated_records_for_#{reflection.name}" - validate validation_method + force_validation = (reflection.options[:validate] == true || reflection.options[:autosave] == true) case reflection.macro when :has_many, :has_and_belongs_to_many @@ -169,7 +169,10 @@ def add_autosave_association_callbacks(reflection) after_create save_method after_update save_method - define_method(validation_method) { validate_collection_association(reflection) } + if 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 @@ -179,7 +182,11 @@ def add_autosave_association_callbacks(reflection) define_method(save_method) { save_belongs_to_association(reflection) } before_save save_method end - define_method(validation_method) { validate_single_association(reflection) } + + if force_validation + define_method(validation_method) { validate_single_association(reflection) } + validate validation_method + end end end end diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 29b199cf041a9..9164701601e5b 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -1056,3 +1056,118 @@ def setup include AutosaveAssociationOnACollectionAssociationTests end + +class TestAutosaveAssociationValidationsOnAHasManyAssocication < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + def setup + @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?") + @pirate.birds.create(:name => 'cookoo') + end + + test "should automatically validate associations" do + assert @pirate.valid? + @pirate.birds.each { |bird| bird.name = '' } + + assert !@pirate.valid? + end +end + +class TestAutosaveAssociationValidationsOnAHasOneAssocication < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + def setup + @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?") + @pirate.create_ship(:name => 'titanic') + end + + test "should automatically validate associations with :validate => true" do + assert @pirate.valid? + @pirate.ship.name = '' + assert !@pirate.valid? + end + + test "should not automatically validate associations without :validate => true" do + assert @pirate.valid? + @pirate.non_validated_ship.name = '' + assert @pirate.valid? + end +end + +class TestAutosaveAssociationValidationsOnABelongsToAssocication < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + def setup + @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?") + end + + test "should automatically validate associations with :validate => true" do + assert @pirate.valid? + @pirate.parrot = Parrot.new(:name => '') + assert !@pirate.valid? + end + + test "should not automatically validate associations without :validate => true" do + assert @pirate.valid? + @pirate.non_validated_parrot = Parrot.new(:name => '') + assert @pirate.valid? + end +end + +class TestAutosaveAssociationValidationsOnAHABTMAssocication < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + def setup + @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?") + end + + test "should automatically validate associations with :validate => true" do + assert @pirate.valid? + @pirate.parrots = [ Parrot.new(:name => 'popuga') ] + @pirate.parrots.each { |parrot| parrot.name = '' } + assert !@pirate.valid? + end + + test "should not automatically validate associations without :validate => true" do + assert @pirate.valid? + @pirate.non_validated_parrots = [ Parrot.new(:name => 'popuga') ] + @pirate.non_validated_parrots.each { |parrot| parrot.name = '' } + assert @pirate.valid? + end +end + +class TestAutosaveAssociationValidationMethodsGeneration < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + def setup + @pirate = Pirate.new + end + + test "should generate validation methods for has_many associations" do + assert @pirate.respond_to?(:validate_associated_records_for_birds) + end + + test "should generate validation methods for has_one associations with :validate => true" do + assert @pirate.respond_to?(:validate_associated_records_for_ship) + end + + test "should not generate validation methods for has_one associations without :validate => true" do + assert !@pirate.respond_to?(:validate_associated_records_for_non_validated_ship) + end + + test "should generate validation methods for belongs_to associations with :validate => true" do + assert @pirate.respond_to?(:validate_associated_records_for_parrot) + end + + test "should not generate validation methods for belongs_to associations without :validate => true" do + assert !@pirate.respond_to?(:validate_associated_records_for_non_validated_parrot) + end + + test "should generate validation methods for HABTM associations with :validate => true" do + assert @pirate.respond_to?(:validate_associated_records_for_parrots) + end + + test "should not generate validation methods for HABTM associations without :validate => true" do + assert !@pirate.respond_to?(:validate_associated_records_for_non_validated_parrots) + end +end diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index acf53fce8b55b..3d7c4bc48a6e0 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -1,6 +1,8 @@ class Pirate < ActiveRecord::Base - belongs_to :parrot - has_and_belongs_to_many :parrots + belongs_to :parrot, :validate => true + belongs_to :non_validated_parrot, :class_name => 'Parrot' + has_and_belongs_to_many :parrots, :validate => true + has_and_belongs_to_many :non_validated_parrots, :class_name => 'Parrot' has_and_belongs_to_many :parrots_with_method_callbacks, :class_name => "Parrot", :before_add => :log_before_add, :after_add => :log_after_add, @@ -16,7 +18,8 @@ class Pirate < ActiveRecord::Base has_many :treasure_estimates, :through => :treasures, :source => :price_estimates # These both have :autosave enabled because accepts_nested_attributes_for is used on them. - has_one :ship + has_one :ship, :validate => true + has_one :non_validated_ship, :class_name => 'Ship' has_many :birds has_many :birds_with_method_callbacks, :class_name => "Bird", :before_add => :log_before_add, diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index 06759d64b8d18..d0df9516221e0 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -1,7 +1,7 @@ class Ship < ActiveRecord::Base self.record_timestamps = false - belongs_to :pirate + belongs_to :pirate, :validate => true has_many :parts, :class_name => 'ShipPart', :autosave => true accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }