@wip

lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb

@@ -20,9 +20,9 @@ def initialize(conditions, name_placeholders, value_placeholders, name_placehold
         private
 
         def build
-          clauses = @conditions.map do |name, attribute_conditions|
+          clauses = @conditions.map do |path, attribute_conditions|
             attribute_conditions.map do |operator, value|
-              name_or_placeholder = name_or_placeholder_for(name)
+              name_or_placeholder = name_or_placeholder_for(path)
 
               case operator
               when :eq
@@ -59,12 +59,21 @@ def build
           @expression = clauses.join(' AND ')
         end
 
-        def name_or_placeholder_for(name)
-          return name unless name.upcase.in?(Dynamoid::AdapterPlugin::AwsSdkV3::RESERVED_WORDS)
+        # Replace reserved words with placeholders
+        def name_or_placeholder_for(path)
+          sections = path.to_s.split('.')
 
-          placeholder = @name_placeholder_sequence.call
-          @name_placeholders[placeholder] = name
-          placeholder
+          sanitized = sections.map do |name|
+            unless name.upcase.to_sym.in?(Dynamoid::AdapterPlugin::AwsSdkV3::RESERVED_WORDS)
+              next name
+            end
+
+            placeholder = @name_placeholder_sequence.call
+            @name_placeholders[placeholder] = name
+            placeholder
+          end
+
+          sanitized.join('.')
         end
 
         def value_placeholder_for(value)

lib/dynamoid/criteria/chain.rb

@@ -589,25 +589,26 @@ def field_condition(key, value_before_type_casting)
 
         if sections.size == 1
           name = sections[0]
-          selector = name
+          path = name
           operator = nil
         elsif sections.last.in? ALLOWED_FIELD_OPERATORS
           name = sections[0]
-          selector = sections[0...-1].join('.')
+          path = sections[0...-1].join('.')
           operator = sections[-1]
         else
           name = sections[0]
-          selector = sections.join('.')
+          path = sections.join('.')
           operator = nil
         end
 
         type = source.attributes[name.to_sym][:type]
-        if type != :map && name != selector
+        if type != :map && name != path
           raise Dynamoid::Errors::Error,
-            "Map element referencing (#{key}) in condition is not allowed for not :map field '#{name}'"
+            "Dereference operator '.' in '#{key}' document path is not allowed for not :map field '#{name}'"
         end
 
-        value = if name == selector
+        # we don't know types of nested attributes
+        value = if name == path
                   type_cast_condition_parameter(name, value_before_type_casting)
                 else
                   value_before_type_casting
@@ -628,7 +629,7 @@ def field_condition(key, value_before_type_casting)
             [operator.to_sym, value]
           end
 
-        [selector, condition]
+        [path, condition]
       end
 
       def query_key_conditions

spec/dynamoid/criteria/chain_spec.rb

@@ -231,7 +231,9 @@ def request_params
     it 'raises error when operator is not supported' do
       expect do
         model.where(name: 'Bob', 'age.foo': 10).to_a
-      end.to raise_error(Dynamoid::Errors::Error, 'Unsupported operator foo in age.foo')
+      end.to raise_error(
+        Dynamoid::Errors::Error,
+        "Dereference operator '.' in 'age.foo' document path is not allowed for not :map field 'age'")
     end
   end
 
@@ -447,7 +449,9 @@ def request_params
     it 'raises error when operator is not supported' do
       expect do
         model.where(name: 'a', 'age.foo': 9).to_a
-      end.to raise_error(Dynamoid::Errors::Error, 'Unsupported operator foo in age.foo')
+      end.to raise_error(
+        Dynamoid::Errors::Error,
+        "Dereference operator '.' in 'age.foo' document path is not allowed for not :map field 'age'")
     end
   end
 
@@ -663,7 +667,9 @@ def request_params
     it 'raises error when operator is not supported' do
       expect do
         model.where('age.foo': 9).to_a
-      end.to raise_error(Dynamoid::Errors::Error, 'Unsupported operator foo in age.foo')
+      end.to raise_error(
+        Dynamoid::Errors::Error,
+        "Dereference operator '.' in 'age.foo' document path is not allowed for not :map field 'age'")
     end
   end
 
@@ -688,6 +694,77 @@ def request_params
   end
 
   describe 'condition on a Map key-value pair' do
+    context 'when Query' do
+      let(:klass_with_map) do
+        new_class do
+          field :settings, :map
+        end
+      end
+
+      it 'returns correct result when called without explicit operator' do
+        object = klass_with_map.create(settings: {threshold: 10})
+
+        chain = klass_with_map.where(id: object.id, 'settings.threshold': 10)
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect(chain.to_a).to eq [object]
+
+        chain = klass_with_map.where(id: object.id, 'settings.threshold': 12)
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect(chain.to_a).to eq []
+      end
+
+      it 'returns correct result when called with explicit operator' do
+        object = klass_with_map.create(settings: {threshold: 10})
+
+        chain = klass_with_map.where(id: object.id, 'settings.threshold.gt': 5)
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect(chain.to_a).to eq [object]
+
+        chain = klass_with_map.where(id: object.id, 'settings.threshold.lt': 5)
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect(chain.to_a).to eq []
+      end
+
+      it 'does not raise any error and just returns empty result when called with not existing map key' do
+        object = klass_with_map.create(settings: {threshold: 10})
+        chain = klass_with_map.where(id: object.id, 'settings.threshold.foobar': 5)
+
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect(chain.to_a).to eq []
+      end
+
+      it 'does not raise any error and just returns empty result when called with non-map field' do
+        klass_without_map = new_class do
+          field :age, :integer
+        end
+
+        klass_without_map.create_table
+        chain = klass_without_map.where(id: 'not-exist', 'age.threshold': 5)
+
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect { chain.to_a }.to raise_error(
+          Dynamoid::Errors::Error,
+          "Dereference operator '.' in 'age.threshold' document path is not allowed for not :map field 'age'")
+      end
+
+      it 'does not type cast value' do
+        klass_with_map.create_table
+        chain = klass_with_map.where(id: 'not-exist', 'settings.threshold.gt': Time.now)
+
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect { chain.to_a }.to raise_error(ArgumentError, /unsupported type, expected Hash, Array,/)
+      end
+
+      it 'allows conditions with nested attribute names conflicting with DynamoDB reserved words' do
+        # SCAN, SET and SIZE are reserved words
+        object = klass_with_map.create(settings: {scan: 1, set: 2, size: 3})
+        chain = klass_with_map.where(id: object.id, 'settings.scan': 1, 'settings.set': 2, 'settings.size': 3)
+
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+        expect(chain.to_a).to eq [object]
+      end
+    end
+
     context 'when Scan' do
       let(:klass_with_map) do
         new_class do
@@ -721,8 +798,8 @@ def request_params
 
       it 'does not raise any error and just returns empty result when called with not existing map key' do
         object = klass_with_map.create(settings: {threshold: 10})
-
         chain = klass_with_map.where('settings.threshold.foobar' => 5)
+
         expect(chain).to receive(:raw_pages_via_scan).and_call_original
         expect(chain.to_a).to eq []
       end
@@ -734,20 +811,29 @@ def request_params
 
         klass_without_map.create_table
         chain = klass_without_map.where('age.threshold' => 5)
-        expect(chain).to receive(:raw_pages_via_scan).and_call_original
 
+        expect(chain).to receive(:raw_pages_via_scan).and_call_original
         expect { chain.to_a }.to raise_error(
           Dynamoid::Errors::Error,
-          /Map element referencing \(age\.threshold\) in condition is not allowed for not :map field 'age'/)
+          "Dereference operator '.' in 'age.threshold' document path is not allowed for not :map field 'age'")
       end
 
       it 'does not type cast value' do
         klass_with_map.create_table
         chain = klass_with_map.where('settings.threshold.gt' => Time.now)
-        expect(chain).to receive(:raw_pages_via_scan).and_call_original
 
+        expect(chain).to receive(:raw_pages_via_scan).and_call_original
         expect { chain.to_a }.to raise_error(ArgumentError, /unsupported type, expected Hash, Array,/)
       end
+
+      it 'allows conditions with nested attribute names conflicting with DynamoDB reserved words' do
+        # SCAN, SET and SIZE are reserved words
+        object = klass_with_map.create(settings: {scan: 1, set: 2, size: 3})
+        chain = klass_with_map.where('settings.scan': 1, 'settings.set': 2, 'settings.size': 3)
+
+        expect(chain).to receive(:raw_pages_via_scan).and_call_original
+        expect(chain.to_a).to eq [object]
+      end
     end
   end