Allow :attr.not => [] to match all records

* Each operation and comparison now has a reference to it's parent
  so that we can determine if a specific comparison is negated. This
  allows a negated InclusionComparison with an empty Enumerable to
  match every record.  It also allows it to be seen as valid, so that
  the query actually reaches the datastore.
* Allow :attr.not => nil to work for attributes that do not allow
  nil values.  Previously this would've been denied because the
  not-nullable Property would say that the nil is an invalid value.

[#1050 state:resolved]

lib/dm-core/adapters/data_objects_adapter.rb

@@ -613,6 +613,8 @@ def comparison_statement(comparison, qualify)
             else
               return conditions_statement(comparison.foreign_key_mapping, qualify)
             end
+          elsif @negated && comparison.slug == :in && !value.any?
+            return [ '1 = 1' ]  # match everything
           end
 
           operator = comparison_operator(comparison)

lib/dm-core/associations/relationship.rb

@@ -317,9 +317,9 @@ def loaded?(resource)
       #   true if the resource is valid
       #
       # @api semipulic
-      def valid?(value)
+      def valid?(value, negated = false)
         case value
-          when Enumerable then valid_target_collection?(value)
+          when Enumerable then valid_target_collection?(value, negated)
           when Resource   then valid_target?(value)
           when nil        then true
           else
@@ -527,7 +527,7 @@ def eager_load_targets(source, targets, query)
       end
 
       # @api private
-      def valid_target_collection?(collection)
+      def valid_target_collection?(collection, negated)
         if collection.kind_of?(Collection)
           # TODO: move the check for model_key into Collection#reloadable?
           # since what we're really checking is a Collection's ability
@@ -537,7 +537,7 @@ def valid_target_collection?(collection)
 
           collection.model <= target_model                   &&
           (collection.query.fields & model_key) == model_key &&
-          (collection.loaded? ? collection.any? : true)
+          (collection.loaded? ? (collection.any? || negated) : true)
         else
           collection.all? { |resource| valid_target?(resource) }
         end

lib/dm-core/property.rb

@@ -711,9 +711,9 @@ def value(value)
     #   true if the value is valid
     #
     # @api semipulic
-    def valid?(value)
+    def valid?(value, negated = false)
       value = self.value(value)
-      primitive?(value) || (value.nil? && nullable?)
+      primitive?(value) || (value.nil? && (nullable? || negated))
     end
 
     # Returns a concise string representation of the property instance.

lib/dm-core/query/conditions/comparison.rb

@@ -114,6 +114,9 @@ class AbstractComparison
 
         equalize :slug, :subject, :value
 
+        # @api semipublic
+        attr_accessor :parent
+
         # The property or relationship which is being matched against
         #
         # @return [Property, Associations::Relationship]
@@ -215,11 +218,7 @@ def slug
         #
         # @api semipublic
         def valid?
-          # This needs to be deferred until the last moment because the value
-          # could be a reference to a Resource, that when the comparison was
-          # created was invalid, but has since been saved and has it's key
-          # set.
-          subject.valid?(loaded_value)
+          valid_for_subject?(loaded_value)
         end
 
         # Returns whether the subject is a Relationship
@@ -263,6 +262,12 @@ def to_s
           "#{@subject.name} #{comparator_string} #{@value}"
         end
 
+        # @api private
+        def negated?
+          return @negated if defined?(@negated)
+          @negated = parent ? parent.negated? : false
+        end
+
         private
 
         # Creates a new AbstractComparison instance with +subject+ and +value+
@@ -278,8 +283,6 @@ def initialize(subject, value)
           @subject      = subject
           @loaded_value = typecast_value(value)
           @value        = dumped_value(@loaded_value)
-
-          freeze
         end
 
         # Typecasts the given +val+ using subject#typecast
@@ -408,6 +411,15 @@ def expected_value(val = @loaded_value)
             expected_value
           end
         end
+
+        # Test the value to see if it is valid
+        #
+        # @return [Boolean] true if the value is valid
+        #
+        # @api semipublic
+        def valid_for_subject?(value)
+          subject.valid?(value, negated?)
+        end
       end # class AbstractComparison
 
       # Included into comparisons which are capable of supporting
@@ -497,9 +509,9 @@ def valid?
             when Collection
               super
             when Range
-              loaded_value.any? && subject.valid?(loaded_value.first) && subject.valid?(loaded_value.last)
+              loaded_value.any? && valid_for_subject?(loaded_value.first) && valid_for_subject?(loaded_value.last)
             when Enumerable
-              loaded_value.any? && loaded_value.all? { |val| subject.valid?(val) }
+              (loaded_value.any? || negated?) && loaded_value.all? { |val| valid_for_subject?(val) }
             else
               false
           end

lib/dm-core/query/conditions/operation.rb

@@ -40,9 +40,15 @@ class AbstractOperation
 
         equalize :slug, :sorted_operands
 
+        # @api semipublic
+        attr_accessor :parent
+
         # @api semipublic
         attr_reader :operands
 
+        # @api semipublic
+        alias children operands
+
         # @api private
         def self.descendants
           @descendants ||= Set.new
@@ -88,14 +94,17 @@ def valid?
         # @api semipublic
         def <<(operand)
           assert_valid_operand(operand)
-          @operands << operand unless operand.nil?
+          unless operand.nil?
+            operand.parent = self if operand.respond_to?(:parent=)
+            @operands << operand
+          end
           self
         end
 
         # @api semipublic
         def merge(operands)
           operands.each { |operand| assert_valid_operand(operand) }
-          @operands.merge(operands)
+          operands.each { |operand| self << operand }
           self
         end
 
@@ -115,6 +124,12 @@ def to_s
           "(#{@operands.to_a.join(" #{slug.to_s.upcase} ")})"
         end
 
+        # @api private
+        def negated?
+          return @negated if defined?(@negated)
+          @negated = parent ? parent.negated? : false
+        end
+
         # Return a list of operands in predictable order
         #
         # @return [Array<AbstractOperation>]
@@ -202,6 +217,12 @@ def operand
           @operands.to_a.first
         end
 
+        # @api private
+        def negated?
+          return @negated if defined?(@negated)
+          @negated = parent ? !parent.negated? : true
+        end
+
         private
 
         # @api semipublic

lib/dm-core/spec/adapter_shared_spec.rb

@@ -173,6 +173,14 @@ class ::Heffalump
             Heffalump.all(:color.not => nil).should_not be_include(@two)
           end
 
+          it 'should be able to search for object with a nil value using not-nullable properties' do
+            Heffalump.all(:id.not => nil).should == [ @red, @two, @five ]
+          end
+
+          it 'should be able to search for objects not in an empty list (match all)' do
+            Heffalump.all(:color.not => []).should == [ @red, @two, @five ]
+          end
+
           it 'should be able to search for objects not included in an array of values' do
             Heffalump.all(:num_spots.not => [ 1, 3, 5, 7 ]).should be_include(@two)
           end