Merge pull request #118 from tablecheck/test-sessions-fix

Test sessions fix

lib/mongoid/validatable/associated.rb

@@ -15,32 +15,108 @@ module Validatable
     #
     #     validates_associated :name, :addresses
     #   end
-    class AssociatedValidator < ActiveModel::EachValidator
+    class AssociatedValidator < ActiveModel::Validator
+      # Required by `validates_with` so that the validator
+      # gets added to the correct attributes.
+      def attributes
+        options[:attributes]
+      end
 
-      # Validates that the associations provided are either all nil or all
-      # valid. If neither is true then the appropriate errors will be added to
-      # the parent document.
+      # Checks that the named associations of the given record
+      # (`attributes`) are valid. This does NOT load the associations
+      # from the database, and will only validate records that are dirty
+      # or unpersisted.
       #
-      # @example Validate the association.
-      #   validator.validate_each(document, :name, name)
+      # If anything is not valid, appropriate errors will be added to
+      # the `document` parameter.
+      #
+      # @param [ Mongoid::Document ] document the document with the
+      #   associations to validate.
+      def validate(document)
+        options[:attributes].each do |attr_name|
+          validate_association(document, attr_name)
+        end
+      end
+
+      private
+
+      # Validates that the given association provided is either nil,
+      # persisted and unchanged, or invalid. Otherwise, the appropriate errors
+      # will be added to the parent document.
       #
       # @param [ Mongoid::Document ] document The document to validate.
       # @param [ Symbol ] attribute The association to validate.
-      # @param [ Object ] value The value of the association.
-      def validate_each(document, attribute, value)
-        begin
-          document.begin_validate
-          valid = Array.wrap(value).collect do |doc|
-            if doc.nil? || doc.flagged_for_destroy?
-              true
+      def validate_association(document, attribute)
+        # grab the proxy from the instance variable directly; we don't want
+        # any loading logic to run; we just want to see if it's already
+        # been loaded.
+        proxy = document.ivar(attribute)
+        return unless proxy
+
+        # if the variable exists, now we see if it is a proxy, or an actual
+        # document. It might be a literal document instead of a proxy if this
+        # document was created with a Document instance as a provided attribute,
+        # e.g. "Post.new(message: Message.new)".
+        target = proxy.respond_to?(:_target) ? proxy._target : proxy
+
+        # Now, fetch the list of documents from the target. Target may be a
+        # single value, or a list of values, and in the case of HasMany,
+        # might be a rather complex collection. We need to do this without
+        # triggering a load, so it's a bit of a delicate dance.
+        list = get_target_documents(target)
+
+        valid = document.validating do
+          # Now, treating the target as an array, look at each element
+          # and see if it is valid, but only if it has already been
+          # persisted, or changed, and hasn't been flagged for destroy.
+          list.all? do |value|
+            if value && !value.flagged_for_destroy? && (!value.persisted? || value.changed?)
+              value.validated? ? true : value.valid?
             else
-              doc.validated? ? true : doc.valid?
+              true
             end
-          end.all?
-        ensure
-          document.exit_validate
+          end
         end
-        document.errors.add(attribute, :invalid, **options) unless valid
+
+        document.errors.add(attribute, :invalid) unless valid
+      end
+
+      # Examine the given target object and return an array of
+      # documents (possibly empty) that the target represents.
+      #
+      # @param [ Array | Mongoid::Document | Mongoid::Association::Proxy | HasMany::Enumerable ] target
+      #   the target object to examine.
+      #
+      # @return [ Array<Mongoid::Document> ] the list of documents
+      def get_target_documents(target)
+        if target.respond_to?(:_loaded?)
+          get_target_documents_for_has_many(target)
+        else
+          get_target_documents_for_other(target)
+        end
+      end
+
+      # Returns the list of all currently in-memory values held by
+      # the target. The target will not be loaded.
+      #
+      # @param [ HasMany::Enumerable ] target the target that will
+      #   be examined for in-memory documents.
+      #
+      # @return [ Array<Mongoid::Document> ] the in-memory documents
+      #   held by the target.
+      def get_target_documents_for_has_many(target)
+        [*target._loaded.values, *target._added.values]
+      end
+
+      # Returns the target as an array. If the target represents a single
+      # value, it is wrapped in an array.
+      #
+      # @param [ Array | Mongoid::Document | Mongoid::Association::Proxy ] target
+      #   the target to return.
+      #
+      # @return [ Array<Mongoid::Document> ] the target, as an array.
+      def get_target_documents_for_other(target)
+        Array.wrap(target)
       end
     end
   end

spec/mongoid/association/auto_save_spec.rb

@@ -9,16 +9,16 @@
   describe '.auto_save' do
 
     before(:all) do
-      Person.has_many :drugs, validate: false, autosave: true
-      Person.has_one :account, validate: false, autosave: true
+      PersonAuto.has_many :drugs, class_name: 'DrugAuto', validate: false, autosave: true
+      PersonAuto.has_one :account, class_name: 'AccountAuto', validate: false, autosave: true
     end
 
     after(:all) do
-      Person.reset_callbacks(:save)
+      PersonAuto.reset_callbacks(:save)
     end
 
     let(:person) do
-      Person.new
+      PersonAuto.new
     end
 
     context 'when the option is not provided' do
@@ -85,11 +85,11 @@
       context 'when the relation has already had the autosave callback added' do
 
         before do
-          Person.has_many :drugs, validate: false, autosave: true
+          PersonAuto.has_many :drugs, class_name: 'DrugAuto', validate: false, autosave: true
         end
 
         let(:drug) do
-          Drug.new(name: 'Percocet')
+          DrugAuto.new(name: 'Percocet')
         end
 
         it 'does not add the autosave callback twice' do
@@ -102,15 +102,15 @@
       context 'when the relation is a references many' do
 
         let(:drug) do
-          Drug.new(name: 'Percocet')
+          DrugAuto.new(name: 'Percocet')
         end
 
         context 'when saving a new parent document' do
 
           context 'when persistence options are not set on the parent' do
 
             before do
-              Person.has_many :drugs, validate: false, autosave: true
+              PersonAuto.has_many :drugs, class_name: 'DrugAuto', validate: false, autosave: true
             end
 
             before do
@@ -130,8 +130,8 @@
             end
 
             after do
-              Person.with(database: other_database, &:delete_all)
-              Drug.with(database: other_database, &:delete_all)
+              PersonAuto.with(database: other_database, &:delete_all)
+              DrugAuto.with(database: other_database, &:delete_all)
             end
 
             before do
@@ -142,7 +142,7 @@
             end
 
             it 'saves the relation with the persistence options' do
-              Drug.with(database: other_database) do |drug_class|
+              DrugAuto.with(database: other_database) do |drug_class|
                 expect(drug_class.count).to eq(1)
               end
             end
@@ -165,7 +165,7 @@
         context 'when not updating the document' do
 
           let(:from_db) do
-            Person.find person.id
+            PersonAuto.find person.id
           end
 
           before do
@@ -183,7 +183,7 @@
       context 'when the relation is a references one' do
 
         let(:account) do
-          Account.new(name: 'Testing')
+          AccountAuto.new(name: 'Testing')
         end
 
         context 'when saving a new parent document' do
@@ -237,7 +237,7 @@
         context 'when not updating the document' do
 
           let(:from_db) do
-            Person.find person.id
+            PersonAuto.find person.id
           end
 
           before do
@@ -291,25 +291,25 @@
       context 'when it has two relations with autosaves' do
 
         let!(:person) do
-          Person.create!(drugs: [percocet], account: account)
+          PersonAuto.create!(drugs: [percocet], account: account)
         end
 
         let(:from_db) do
-          Person.find person.id
+          PersonAuto.find person.id
         end
 
         let(:percocet) do
-          Drug.new(name: 'Percocet')
+          DrugAuto.new(name: 'Percocet')
         end
 
         let(:account) do
-          Account.new(name: 'Testing')
+          AccountAuto.new(name: 'Testing')
         end
 
         context 'when updating one document' do
 
           let(:placebo) do
-            Drug.new(name: 'Placebo')
+            DrugAuto.new(name: 'Placebo')
           end
 
           before do

spec/mongoid/validatable/associated_spec.rb

@@ -75,31 +75,28 @@
         end
 
         it 'does not run validation on them' do
-          expect(description).to_not receive(:valid?)
           expect(user).to be_valid
         end
-
       end
-
     end
   end
 
-  describe '#validate_each' do
+  describe '#validate' do
 
     let(:person) do
       Person.new
     end
 
     let(:validator) do
-      described_class.new(attributes: person.attributes)
+      described_class.new(attributes: person.relations.keys)
     end
 
     context 'when the association is a one to one' do
 
       context 'when the association is nil' do
 
         before do
-          validator.validate_each(person, :name, nil)
+          validator.validate(person)
         end
 
         it 'adds no errors' do
@@ -108,14 +105,9 @@
       end
 
       context 'when the association is valid' do
-
-        let(:associated) do
-          double(valid?: true, flagged_for_destroy?: false)
-        end
-
         before do
-          expect(associated).to receive(:validated?).and_return(false)
-          validator.validate_each(person, :name, associated)
+          person.name = Name.new(first_name: 'A', last_name: 'B')
+          validator.validate(person)
         end
 
         it 'adds no errors' do
@@ -125,13 +117,9 @@
 
       context 'when the association is invalid' do
 
-        let(:associated) do
-          double(valid?: false, flagged_for_destroy?: false)
-        end
-
         before do
-          expect(associated).to receive(:validated?).and_return(false)
-          validator.validate_each(person, :name, associated)
+          person.name = Name.new(first_name: 'Jamis', last_name: 'Buck')
+          validator.validate(person)
         end
 
         it 'adds errors to the parent document' do
@@ -149,7 +137,7 @@
       context 'when the association is empty' do
 
         before do
-          validator.validate_each(person, :addresses, [])
+          validator.validate(person)
         end
 
         it 'adds no errors' do
@@ -159,13 +147,9 @@
 
       context 'when the association has invalid documents' do
 
-        let(:associated) do
-          double(valid?: false, flagged_for_destroy?: false)
-        end
-
         before do
-          expect(associated).to receive(:validated?).and_return(false)
-          validator.validate_each(person, :addresses, [associated])
+          person.addresses << Address.new(street: '123')
+          validator.validate(person)
         end
 
         it 'adds errors to the parent document' do
@@ -175,13 +159,10 @@
 
       context 'when the association has all valid documents' do
 
-        let(:associated) do
-          double(valid?: true, flagged_for_destroy?: false)
-        end
-
         before do
-          expect(associated).to receive(:validated?).and_return(false)
-          validator.validate_each(person, :addresses, [associated])
+          person.addresses << Address.new(street: '123 First St')
+          person.addresses << Address.new(street: '456 Second St')
+          validator.validate(person)
         end
 
         it 'adds no errors' do