Remove unecessary assertions from DataMapper::Query

* These assertions were originally meant to prevent common errors, but I now
  believe they do more harm than good because they prevent other kinds of
  objects from being injected into the system and then used in the adapters.

Author: dkubb

lib/dm-core/query.rb

@@ -783,10 +783,6 @@ def assert_valid_options(options)
     #
     # @api private
     def assert_valid_fields(fields, unique)
-      fields = fields.to_ary
-
-      model = self.model
-
       valid_properties = model.properties
 
       model.descendants.each do |descendant|
@@ -799,14 +795,6 @@ def assert_valid_fields(fields, unique)
             unless valid_properties.named?(field)
               raise ArgumentError, "+options[:fields]+ entry #{field.inspect} does not map to a property in #{model}"
             end
-
-          when Property
-            unless valid_properties.include?(field)
-              raise ArgumentError, "+options[:field]+ entry #{field.name.inspect} does not map to a property in #{model}"
-            end
-
-          else
-            raise ArgumentError, "+options[:fields]+ entry #{field.inspect} of an unsupported object #{field.class}"
         end
       end
     end
@@ -816,8 +804,6 @@ def assert_valid_fields(fields, unique)
     #
     # @api private
     def assert_valid_links(links)
-      links = links.to_ary
-
       if links.empty?
         raise ArgumentError, '+options[:links]+ should not be empty'
       end
@@ -828,15 +814,6 @@ def assert_valid_links(links)
             unless @relationships.named?(link.to_sym)
               raise ArgumentError, "+options[:links]+ entry #{link.inspect} does not map to a relationship in #{model}"
             end
-
-          when Associations::Relationship
-            # TODO: figure out how to validate links from other models
-            #unless @relationships.value?(link)
-            #  raise ArgumentError, "+options[:links]+ entry #{link.name.inspect} does not map to a relationship in #{model}"
-            #end
-
-          else
-            raise ArgumentError, "+options[:links]+ entry #{link.inspect} of an unsupported object #{link.class}"
         end
       end
     end
@@ -852,40 +829,14 @@ def assert_valid_conditions(conditions)
         when Hash
           conditions.each do |subject, bind_value|
             case subject
-              when Symbol, ::String
+              when Symbol, String
                 original = subject
                 subject  = subject.to_s
                 name     = subject[0, subject.index('.') || subject.length]
 
                 unless @properties.named?(name) || @relationships.named?(name)
                   raise ArgumentError, "condition #{original.inspect} does not map to a property or relationship in #{model}"
                 end
-
-              when Property
-                unless @properties.include?(subject)
-                  raise ArgumentError, "condition #{subject.name.inspect} does not map to a property in #{model}, but belongs to #{subject.model}"
-                end
-
-              when Operator
-                operator = subject.operator
-
-                unless Conditions::Comparison.slugs.include?(operator) || operator == :not
-                  raise ArgumentError, "condition #{subject.inspect} used an invalid operator #{operator}"
-                end
-
-                assert_valid_conditions(subject.target => bind_value)
-
-              when Path
-                assert_valid_links(subject.relationships)
-
-              when Associations::Relationship
-                # TODO: validate that it belongs to the current model
-                #unless subject.source_model.equal?(model)
-                #  raise ArgumentError, "condition #{subject.name.inspect} is not a valid relationship for #{model}, it's source model was #{subject.source_model}"
-                #end
-
-              else
-                raise ArgumentError, "condition #{subject.inspect} of an unsupported object #{subject.class}"
             end
           end
 
@@ -942,30 +893,12 @@ def assert_valid_order(order, fields)
         raise ArgumentError, '+options[:order]+ should not be empty if +options[:fields] contains a non-operator'
       end
 
-      model = self.model
-
       order.each do |order_entry|
         case order_entry
           when Symbol, String
             unless @properties.named?(order_entry)
               raise ArgumentError, "+options[:order]+ entry #{order_entry.inspect} does not map to a property in #{model}"
             end
-
-          when Property, Path
-            # Allow any arbitrary property, since it may map to a model
-            # that has been included via the :links option
-
-          when Operator, Direction
-            operator = order_entry.operator
-
-            unless operator == :asc || operator == :desc
-              raise ArgumentError, "+options[:order]+ entry #{order_entry.inspect} used an invalid operator #{operator}"
-            end
-
-            assert_valid_order([ order_entry.target ], fields)
-
-          else
-            raise ArgumentError, "+options[:order]+ entry #{order_entry.inspect} of an unsupported object #{order_entry.class}"
         end
       end
     end

spec/semipublic/query_spec.rb

@@ -189,22 +189,6 @@ class ::Contact < User; end
           }.should raise_error(ArgumentError, "+options[:fields]+ entry \"unknown\" does not map to a property in #{@model}")
         end
       end
-
-      describe 'that is an Array containing an invalid object' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:fields => [ 1 ]))
-          }.should raise_error(ArgumentError, '+options[:fields]+ entry 1 of an unsupported object Fixnum')
-        end
-      end
-
-      describe 'that is an Array containing an unknown Property' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:fields => [ DataMapper::Property::String.new(@model, :unknown) ]))
-          }.should raise_error(ArgumentError, "+options[:field]+ entry :unknown does not map to a property in #{@model}")
-        end
-      end
     end
 
     describe 'with a links option' do
@@ -291,14 +275,6 @@ class ::Contact < User; end
           }.should raise_error(ArgumentError, "+options[:links]+ entry \"unknown\" does not map to a relationship in #{@model}")
         end
       end
-
-      describe 'that is an Array containing an invalid object' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:links => [ 1 ]))
-          }.should raise_error(ArgumentError, '+options[:links]+ entry 1 of an unsupported object Fixnum')
-        end
-      end
     end
 
     describe 'with a conditions option' do
@@ -794,37 +770,6 @@ class ::Contact < User; end
           }.should raise_error(ArgumentError, "condition \"unknown.id\" does not map to a property or relationship in #{@model}")
         end
       end
-
-      describe 'that is a Hash with a Property that does not belong to the model' do
-        before do
-          Object.send(:remove_const, :Alternate) if Object.const_defined?(:Alternate)
-          @alternate_model = DataMapper::Model.new('Alternate') do
-            property :id, DataMapper::Property::Serial
-          end
-        end
-
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:conditions => { @alternate_model.properties[:id] => 1 }))
-          }.should raise_error(ArgumentError, "condition :id does not map to a property in #{@model}, but belongs to #{@alternate_model}")
-        end
-      end
-
-      describe 'that is a Hash with a Query::Operator key that is not for a Property in the model' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:conditions => { :unknown.asc => 1 }))
-          }.should raise_error(ArgumentError, 'condition #<DataMapper::Query::Operator @target=:unknown @operator=:asc> used an invalid operator asc')
-        end
-      end
-
-      describe 'that is a Hash with a key of a type that is not permitted' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:conditions => { 1 => 1 }))
-          }.should raise_error(ArgumentError, 'condition 1 of an unsupported object Fixnum')
-        end
-      end
     end
 
     describe 'with an offset option' do
@@ -1129,30 +1074,6 @@ class ::Contact < User; end
         end
       end
 
-      describe 'that is an Array containing an invalid object' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:order => [ 1 ]))
-          }.should raise_error(ArgumentError, '+options[:order]+ entry 1 of an unsupported object Fixnum')
-        end
-      end
-
-      describe 'that contains a Query::Operator with a target that is not part of the model' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:order => [ :unknown.desc ]))
-          }.should raise_error(ArgumentError, "+options[:order]+ entry :unknown does not map to a property in #{@model}")
-        end
-      end
-
-      describe 'that contains a Query::Operator with an unknown operator' do
-        it 'should raise an exception' do
-          lambda {
-            DataMapper::Query.new(@repository, @model, @options.update(:order => [ :name.gt ]))
-          }.should raise_error(ArgumentError, '+options[:order]+ entry #<DataMapper::Query::Operator @target=:name @operator=:gt> used an invalid operator gt')
-        end
-      end
-
       describe 'that contains a Symbol that is not for a Property in the model' do
         it 'should raise an exception' do
           lambda {