From 9a026997dd70d468327fd784e340ea0a676e3d14 Mon Sep 17 00:00:00 2001 From: Peter Gumeson Date: Thu, 28 May 2009 12:18:23 -0700 Subject: [PATCH] valid? now recursively checks casted models. Added better validation spec coverage. --- lib/couchrest/mixins/validation.rb | 33 ++++----------- spec/couchrest/more/casted_model_spec.rb | 52 +++++++++++++++++++++++- spec/couchrest/more/extended_doc_spec.rb | 46 +++++++++++++++++++++ spec/fixtures/more/cat.rb | 1 + 4 files changed, 106 insertions(+), 26 deletions(-) diff --git a/lib/couchrest/mixins/validation.rb b/lib/couchrest/mixins/validation.rb index 82c8ab5..dda708e 100644 --- a/lib/couchrest/mixins/validation.rb +++ b/lib/couchrest/mixins/validation.rb @@ -115,8 +115,7 @@ module CouchRest # Check if a resource is valid in a given context # def valid?(context = :default) - result = self.class.validators.execute(context, self) - result && validate_casted_arrays + recursive_valid?(self, context, true) end # checking on casted objects @@ -133,29 +132,22 @@ module CouchRest result end - # Begin a recursive walk of the model checking validity - # - def all_valid?(context = :default) - recursive_valid?(self, context, true) - end - # Do recursive validity checking # def recursive_valid?(target, context, state) valid = state - target.instance_variables.each do |ivar| - ivar_value = target.instance_variable_get(ivar) - if ivar_value.validatable? - valid = valid && recursive_valid?(ivar_value, context, valid) - elsif ivar_value.respond_to?(:each) - ivar_value.each do |item| + target.each do |key, prop| + if prop.is_a?(Array) + prop.each do |item| if item.validatable? - valid = valid && recursive_valid?(item, context, valid) + valid = recursive_valid?(item, context, valid) && valid end end + elsif prop.validatable? + valid = recursive_valid?(prop, context, valid) && valid end end - return valid && target.valid? + target.class.validators.execute(context, target) && valid end @@ -218,15 +210,6 @@ module CouchRest end # end EOS end - - all = "all_valid_for_#{context.to_s}?" # all_valid_for_signup? - if !self.instance_methods.include?(all) - class_eval <<-EOS, __FILE__, __LINE__ - def #{all} # def all_valid_for_signup? - all_valid?('#{context.to_s}'.to_sym) # all_valid?('signup'.to_sym) - end # end - EOS - end end # Create a new validator of the given klazz and push it onto the diff --git a/spec/couchrest/more/casted_model_spec.rb b/spec/couchrest/more/casted_model_spec.rb index 0f90360..43524a8 100644 --- a/spec/couchrest/more/casted_model_spec.rb +++ b/spec/couchrest/more/casted_model_spec.rb @@ -205,7 +205,57 @@ describe CouchRest::CastedModel do cat.masters.push Person.new cat.should be_valid end - end + describe "calling valid?" do + before :each do + @cat = Cat.new + @toy1 = CatToy.new + @toy2 = CatToy.new + @toy3 = CatToy.new + @cat.favorite_toy = @toy1 + @cat.toys << @toy2 + @cat.toys << @toy3 + end + + describe "on the top document" do + it "should put errors on all invalid casted models" do + @cat.should_not be_valid + @cat.errors.should_not be_empty + @toy1.errors.should_not be_empty + @toy2.errors.should_not be_empty + @toy3.errors.should_not be_empty + end + + it "should not put errors on valid casted models" do + @toy1.name = "Feather" + @toy2.name = "Twine" + @cat.should_not be_valid + @cat.errors.should_not be_empty + @toy1.errors.should be_empty + @toy2.errors.should be_empty + @toy3.errors.should_not be_empty + end + end + + describe "on a casted model property" do + it "should only validate itself" do + @toy1.should_not be_valid + @toy1.errors.should_not be_empty + @cat.errors.should be_empty + @toy2.errors.should be_empty + @toy3.errors.should be_empty + end + end + + describe "on a casted model inside a casted collection" do + it "should only validate itself" do + @toy2.should_not be_valid + @toy2.errors.should_not be_empty + @cat.errors.should be_empty + @toy1.errors.should be_empty + @toy3.errors.should be_empty + end + end + end end diff --git a/spec/couchrest/more/extended_doc_spec.rb b/spec/couchrest/more/extended_doc_spec.rb index 66000a0..dcec051 100644 --- a/spec/couchrest/more/extended_doc_spec.rb +++ b/spec/couchrest/more/extended_doc_spec.rb @@ -1,6 +1,7 @@ require File.dirname(__FILE__) + '/../../spec_helper' require File.join(FIXTURE_PATH, 'more', 'article') require File.join(FIXTURE_PATH, 'more', 'course') +require File.join(FIXTURE_PATH, 'more', 'cat') describe "ExtendedDocument" do @@ -561,4 +562,49 @@ describe "ExtendedDocument" do @doc.other_arg.should == "foo-foo" end end + + describe "recursive validation on an extended document" do + before :each do + reset_test_db! + @cat = Cat.new(:name => 'Sockington') + end + + it "should not save if a nested casted model is invalid" do + @cat.favorite_toy = CatToy.new + @cat.should_not be_valid + @cat.save.should be_false + lambda{@cat.save!}.should raise_error + end + + it "should save when nested casted model is valid" do + @cat.favorite_toy = CatToy.new(:name => 'Squeaky') + @cat.should be_valid + @cat.save.should be_true + lambda{@cat.save!}.should_not raise_error + end + + it "should not save when nested collection contains an invalid casted model" do + @cat.toys = [CatToy.new(:name => 'Feather'), CatToy.new] + @cat.should_not be_valid + @cat.save.should be_false + lambda{@cat.save!}.should raise_error + end + + it "should save when nested collection contains valid casted models" do + @cat.toys = [CatToy.new(:name => 'feather'), CatToy.new(:name => 'ball-o-twine')] + @cat.should be_valid + @cat.save.should be_true + lambda{@cat.save!}.should_not raise_error + end + + it "should not fail if the nested casted model doesn't have validation" do + Cat.property :trainer, :cast_as => 'Person' + Cat.validates_present :name + cat = Cat.new(:name => 'Mr Bigglesworth') + cat.trainer = Person.new + cat.trainer.validatable?.should be_false + cat.should be_valid + cat.save.should be_true + end + end end diff --git a/spec/fixtures/more/cat.rb b/spec/fixtures/more/cat.rb index 54abad5..a3cb054 100644 --- a/spec/fixtures/more/cat.rb +++ b/spec/fixtures/more/cat.rb @@ -6,6 +6,7 @@ class Cat < CouchRest::ExtendedDocument property :name property :toys, :cast_as => ['CatToy'], :default => [] + property :favorite_toy, :cast_as => 'CatToy' end class CatToy < Hash