From 50b6a19d7a7ee0ea2886082048c42595f07406eb Mon Sep 17 00:00:00 2001 From: Xavier Shay Date: Sat, 26 Feb 2011 20:50:48 +1100 Subject: [PATCH 1/3] Basic auto validation of associations --- lib/dm-validations.rb | 28 +++++++++++++++++ spec/fixtures/company.rb | 12 ++++++++ .../association_validation_spec.rb | 30 +++++++++++++++++++ 3 files changed, 70 insertions(+) diff --git a/lib/dm-validations.rb b/lib/dm-validations.rb index 0d20920a..ed147aa7 100644 --- a/lib/dm-validations.rb +++ b/lib/dm-validations.rb @@ -235,6 +235,34 @@ def add_validator_to_context(opts, fields, validator_class) end end end + + # Automatically adds a validation to all associations so that validation + # on parent models will return false when children are invalid. This + # matches the behaviour of #save, which returns false if child models + # cannot be saved. + # + # TODO: Validations are only run on dirty models (which includes new models), + # since clean ones are assumed to be valid. This prevents validations + # from cascading down to nested child models when not required. + def has(cardinality, property, *args) + super + add_validation = lambda do |conditional| + validates_with_block property do + value = send(property) + if conditional[value] + [false, "#{property.to_s.titleize} must be valid"] + else + true + end + end + end + + if cardinality == 1 || (cardinality.is_a?(Range) && cardinality.max == 1) + add_validation[lambda {|val| val && !val.valid? }] + else + add_validation[lambda {|val| !val.map(&:valid?).all? }] + end + end end # module ClassMethods end # module Validations diff --git a/spec/fixtures/company.rb b/spec/fixtures/company.rb index 4c072aad..0e662780 100644 --- a/spec/fixtures/company.rb +++ b/spec/fixtures/company.rb @@ -60,6 +60,18 @@ class ProductCompany < Company validates_presence_of :title, :message => "Product company must have a name" validates_presence_of :flagship_product + + has n, :products, :child_key => [:company_id] + has 1, :profile + has 0..1, :alternate_profile, :model => "Profile" + end + + class Profile + include DataMapper::Resource + + property :id, Serial + belongs_to :product_company + property :description, Text, :required => true end class Product diff --git a/spec/integration/datamapper_models/association_validation_spec.rb b/spec/integration/datamapper_models/association_validation_spec.rb index eab6ddcd..4b51a5f6 100644 --- a/spec/integration/datamapper_models/association_validation_spec.rb +++ b/spec/integration/datamapper_models/association_validation_spec.rb @@ -24,3 +24,33 @@ end end end + +describe 'DataMapper::Validations::Fixtures::ProductCompany' do + before :all do + @model = DataMapper::Validations::Fixtures::ProductCompany.new(:title => "Apple", :flagship_product => "Macintosh") + end + + describe 'with invalid products' do + before :all do + @model.products = [DataMapper::Validations::Fixtures::Product.new] + end + + it_should_behave_like "invalid model" + + it "has a meaningful error message" do + @model.errors.on(:products).should == [ 'Products must be valid' ] + end + end + + describe 'with invalid profile' do + before :all do + @model.profile = DataMapper::Validations::Fixtures::Profile.new + end + + it_should_behave_like "invalid model" + + it "has a meaningful error message" do + @model.errors.on(:profile).should == [ 'Profile must be valid' ] + end + end +end From 72794ebe6cbe389ef9fda7b38a382eea8dd6ad27 Mon Sep 17 00:00:00 2001 From: Xavier Shay Date: Sat, 26 Feb 2011 20:59:50 +1100 Subject: [PATCH 2/3] Allow validations to be disabled for associations --- lib/dm-validations.rb | 29 ++++++++++--------- spec/fixtures/company.rb | 4 +++ .../association_validation_spec.rb | 8 +++++ 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/lib/dm-validations.rb b/lib/dm-validations.rb index ed147aa7..23d4306c 100644 --- a/lib/dm-validations.rb +++ b/lib/dm-validations.rb @@ -245,22 +245,25 @@ def add_validator_to_context(opts, fields, validator_class) # since clean ones are assumed to be valid. This prevents validations # from cascading down to nested child models when not required. def has(cardinality, property, *args) - super - add_validation = lambda do |conditional| - validates_with_block property do - value = send(property) - if conditional[value] - [false, "#{property.to_s.titleize} must be valid"] - else - true + super.tap do + next if @disable_auto_validations + + add_validation = lambda do |conditional| + validates_with_block property do + value = send(property) + if conditional[value] + [false, "#{property.to_s.titleize} must be valid"] + else + true + end end end - end - if cardinality == 1 || (cardinality.is_a?(Range) && cardinality.max == 1) - add_validation[lambda {|val| val && !val.valid? }] - else - add_validation[lambda {|val| !val.map(&:valid?).all? }] + if cardinality == 1 || (cardinality.is_a?(Range) && cardinality.max == 1) + add_validation[lambda {|val| val && !val.valid? }] + else + add_validation[lambda {|val| !val.map(&:valid?).all? }] + end end end end # module ClassMethods diff --git a/spec/fixtures/company.rb b/spec/fixtures/company.rb index 0e662780..f5626839 100644 --- a/spec/fixtures/company.rb +++ b/spec/fixtures/company.rb @@ -64,6 +64,10 @@ class ProductCompany < Company has n, :products, :child_key => [:company_id] has 1, :profile has 0..1, :alternate_profile, :model => "Profile" + + without_auto_validations do + has 0..1, :yet_another_profile, :model => "Profile" + end end class Profile diff --git a/spec/integration/datamapper_models/association_validation_spec.rb b/spec/integration/datamapper_models/association_validation_spec.rb index 4b51a5f6..f66cd780 100644 --- a/spec/integration/datamapper_models/association_validation_spec.rb +++ b/spec/integration/datamapper_models/association_validation_spec.rb @@ -53,4 +53,12 @@ @model.errors.on(:profile).should == [ 'Profile must be valid' ] end end + + describe 'with invalid yet_another_profile' do + before :all do + @model.yet_another_profile = DataMapper::Validations::Fixtures::Profile.new + end + + it_should_behave_like "valid model" + end end From 9c9859f4d36c0566ae5b07de04edf773d4dd8d2e Mon Sep 17 00:00:00 2001 From: Xavier Shay Date: Sat, 26 Feb 2011 21:13:24 +1100 Subject: [PATCH 3/3] Don't trigger cascading validation chains on child models --- lib/dm-validations.rb | 4 +-- spec/fixtures/company.rb | 3 +- .../association_validation_spec.rb | 31 ++++++++++++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/dm-validations.rb b/lib/dm-validations.rb index 23d4306c..863d6b7b 100644 --- a/lib/dm-validations.rb +++ b/lib/dm-validations.rb @@ -260,9 +260,9 @@ def has(cardinality, property, *args) end if cardinality == 1 || (cardinality.is_a?(Range) && cardinality.max == 1) - add_validation[lambda {|val| val && !val.valid? }] + add_validation[lambda {|val| val && val.dirty? && !val.valid? }] else - add_validation[lambda {|val| !val.map(&:valid?).all? }] + add_validation[lambda {|val| val.loaded? && !val.map(&:valid?).all? }] end end end diff --git a/spec/fixtures/company.rb b/spec/fixtures/company.rb index f5626839..ace0fc43 100644 --- a/spec/fixtures/company.rb +++ b/spec/fixtures/company.rb @@ -75,7 +75,8 @@ class Profile property :id, Serial belongs_to :product_company - property :description, Text, :required => true + property :description, Text, :required => false # Allow NULL values, enforce validation in the app + validates_presence_of :description end class Product diff --git a/spec/integration/datamapper_models/association_validation_spec.rb b/spec/integration/datamapper_models/association_validation_spec.rb index f66cd780..cb478b6c 100644 --- a/spec/integration/datamapper_models/association_validation_spec.rb +++ b/spec/integration/datamapper_models/association_validation_spec.rb @@ -27,7 +27,36 @@ describe 'DataMapper::Validations::Fixtures::ProductCompany' do before :all do - @model = DataMapper::Validations::Fixtures::ProductCompany.new(:title => "Apple", :flagship_product => "Macintosh") + @model = DataMapper::Validations::Fixtures::ProductCompany.create(:title => "Apple", :flagship_product => "Macintosh") + end + + describe 'with no products loaded' do + it 'does not load or validate it' do + @model.reload + @model.valid? + @model.products.should_not be_loaded + end + end + + describe 'with no profile loaded' do + it 'does not load or validate it' do + pending "Unsure how to test this" + @model.reload + @model.valid? + @model.profile.should_not be_loaded + end + end + + describe 'with not dirty profile' do + before :all do + # Force an invalid, yet clean model. This should not happen in real + # code, but gives us an easy way to check whether the validations + # are getting run on the profile. + profile = DataMapper::Validations::Fixtures::Profile.create!(:product_company => @model) + @model.reload + end + + it_should_behave_like "valid model" end describe 'with invalid products' do