Merge pull request #812 from Dynamoid/ak/use-placeholders-for-every-a…

…ttribute-in-where-condition

Fix field names with special characters in where conditions

lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb

@@ -22,46 +22,50 @@ def initialize(conditions, name_placeholders, value_placeholders, name_placehold
         def build
           clauses = @conditions.map do |name, attribute_conditions|
             attribute_conditions.map do |operator, value|
-              name_or_placeholder = name_or_placeholder_for(name)
+              # replace attribute names with placeholders unconditionally to support
+              # - special characters (e.g. '.', ':', and '#') and
+              # - leading '_'
+              # See
+              # - https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.NamingRules
+              # - https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ExpressionAttributeNames.html#Expressions.ExpressionAttributeNames.AttributeNamesContainingSpecialCharacters
+              name_placeholder = name_placeholder_for(name)
 
               case operator
               when :eq
-                "#{name_or_placeholder} = #{value_placeholder_for(value)}"
+                "#{name_placeholder} = #{value_placeholder_for(value)}"
               when :ne
-                "#{name_or_placeholder} <> #{value_placeholder_for(value)}"
+                "#{name_placeholder} <> #{value_placeholder_for(value)}"
               when :gt
-                "#{name_or_placeholder} > #{value_placeholder_for(value)}"
+                "#{name_placeholder} > #{value_placeholder_for(value)}"
               when :lt
-                "#{name_or_placeholder} < #{value_placeholder_for(value)}"
+                "#{name_placeholder} < #{value_placeholder_for(value)}"
               when :gte
-                "#{name_or_placeholder} >= #{value_placeholder_for(value)}"
+                "#{name_placeholder} >= #{value_placeholder_for(value)}"
               when :lte
-                "#{name_or_placeholder} <= #{value_placeholder_for(value)}"
+                "#{name_placeholder} <= #{value_placeholder_for(value)}"
               when :between
-                "#{name_or_placeholder} BETWEEN #{value_placeholder_for(value[0])} AND #{value_placeholder_for(value[1])}"
+                "#{name_placeholder} BETWEEN #{value_placeholder_for(value[0])} AND #{value_placeholder_for(value[1])}"
               when :begins_with
-                "begins_with (#{name_or_placeholder}, #{value_placeholder_for(value)})"
+                "begins_with (#{name_placeholder}, #{value_placeholder_for(value)})"
               when :in
                 list = value.map(&method(:value_placeholder_for)).join(' , ')
-                "#{name_or_placeholder} IN (#{list})"
+                "#{name_placeholder} IN (#{list})"
               when :contains
-                "contains (#{name_or_placeholder}, #{value_placeholder_for(value)})"
+                "contains (#{name_placeholder}, #{value_placeholder_for(value)})"
               when :not_contains
-                "NOT contains (#{name_or_placeholder}, #{value_placeholder_for(value)})"
+                "NOT contains (#{name_placeholder}, #{value_placeholder_for(value)})"
               when :null
-                "attribute_not_exists (#{name_or_placeholder})"
+                "attribute_not_exists (#{name_placeholder})"
               when :not_null
-                "attribute_exists (#{name_or_placeholder})"
+                "attribute_exists (#{name_placeholder})"
               end
             end
           end.flatten
 
           @expression = clauses.join(' AND ')
         end
 
-        def name_or_placeholder_for(name)
-          return name unless name.upcase.in?(Dynamoid::AdapterPlugin::AwsSdkV3::RESERVED_WORDS)
-
+        def name_placeholder_for(name)
           placeholder = @name_placeholder_sequence.call
           @name_placeholders[placeholder] = name
           placeholder

lib/dynamoid/adapter_plugin/aws_sdk_v3/projection_expression_convertor.rb

@@ -21,13 +21,15 @@ def build
           return if @names.nil? || @names.empty?
 
           clauses = @names.map do |name|
-            if name.upcase.in?(Dynamoid::AdapterPlugin::AwsSdkV3::RESERVED_WORDS)
-              placeholder = @name_placeholder_sequence.call
-              @name_placeholders[placeholder] = name
-              placeholder
-            else
-              name.to_s
-            end
+            # replace attribute names with placeholders unconditionally to support
+            # - special characters (e.g. '.', ':', and '#') and
+            # - leading '_'
+            # See
+            # - https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.NamingRules
+            # - https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ExpressionAttributeNames.html#Expressions.ExpressionAttributeNames.AttributeNamesContainingSpecialCharacters
+            placeholder = @name_placeholder_sequence.call
+            @name_placeholders[placeholder] = name
+            placeholder
           end
 
           @expression = clauses.join(' , ')

spec/dynamoid/criteria/chain_spec.rb

@@ -227,6 +227,30 @@ def request_params
       expect(documents.map(&:id)).to eql ['1']
     end
 
+    it 'allows conditions with attribute names containing special characters' do
+      model = new_class do
+        range :'sort:key'
+      end
+
+      model.create_table
+      put_attributes(model.table_name, id: '1', 'sort:key': 'c')
+
+      documents = model.where(id: '1', 'sort:key': 'c').to_a
+      expect(documents.map(&:id)).to eql ['1']
+    end
+
+    it 'allows conditions with attribute names starting with _' do
+      model = new_class do
+        range :_sortKey
+      end
+
+      model.create_table
+      put_attributes(model.table_name, id: '1', _sortKey: 'c')
+
+      documents = model.where(id: '1', _sortKey: 'c').to_a
+      expect(documents.map(&:id)).to eql ['1']
+    end
+
     it 'raises error when operator is not supported' do
       expect do
         model.where(name: 'Bob', 'age.foo': 10).to_a
@@ -443,6 +467,30 @@ def request_params
       expect(documents.map(&:id)).to eql ['1']
     end
 
+    it 'allows conditions with attribute names containing special characters' do
+      model = new_class do
+        field :'last:name'
+      end
+
+      model.create_table
+      put_attributes(model.table_name, id: '1', 'last:name': 'c')
+
+      documents = model.where(id: '1', 'last:name': 'c').to_a
+      expect(documents.map(&:id)).to eql ['1']
+    end
+
+    it 'allows conditions with attribute names starting with _' do
+      model = new_class do
+        field :_lastName
+      end
+
+      model.create_table
+      put_attributes(model.table_name, id: '1', _lastName: 'c')
+
+      documents = model.where(id: '1', _lastName: 'c').to_a
+      expect(documents.map(&:id)).to eql ['1']
+    end
+
     it 'raises error when operator is not supported' do
       expect do
         model.where(name: 'a', 'age.foo': 9).to_a
@@ -659,6 +707,30 @@ def request_params
       expect(documents.map(&:id)).to eql ['1']
     end
 
+    it 'allows conditions with attribute names containing special characters' do
+      model = new_class do
+        field :'last:name'
+      end
+
+      model.create_table
+      put_attributes(model.table_name, id: '1', 'last:name': 'c')
+
+      documents = model.where('last:name': 'c').to_a
+      expect(documents.map(&:id)).to eql ['1']
+    end
+
+    it 'allows conditions with attribute names starting with _' do
+      model = new_class do
+        field :_lastName
+      end
+
+      model.create_table
+      put_attributes(model.table_name, id: '1', _lastName: 'c')
+
+      documents = model.where(_lastName: 'c').to_a
+      expect(documents.map(&:id)).to eql ['1']
+    end
+
     it 'raises error when operator is not supported' do
       expect do
         model.where('age.foo': 9).to_a
@@ -1903,6 +1975,62 @@ def request_params
         expect(obj.attributes).to eq(bucket: 2)
       end
     end
+
+    context 'when attribute name contains special characters' do
+      let(:model) do
+        new_class do
+          field :'first:name'
+        end
+      end
+
+      it 'works with Scan' do
+        model.create('first:name': 'Alex')
+
+        chain = described_class.new(model)
+        expect(chain).to receive(:raw_pages_via_scan).and_call_original
+
+        obj, = chain.project(:'first:name').to_a
+        expect(obj.attributes).to eq('first:name': 'Alex')
+      end
+
+      it 'works with Query' do
+        object = model.create('first:name': 'Alex')
+
+        chain = described_class.new(model)
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+
+        obj, = chain.where(id: object.id).project(:'first:name').to_a
+        expect(obj.attributes).to eq('first:name': 'Alex')
+      end
+    end
+
+    context 'when attribute name starts with _' do
+      let(:model) do
+        new_class do
+          field :_name
+        end
+      end
+
+      it 'works with Scan' do
+        model.create(_name: 'Alex')
+
+        chain = described_class.new(model)
+        expect(chain).to receive(:raw_pages_via_scan).and_call_original
+
+        obj, = chain.project(:_name).to_a
+        expect(obj.attributes).to eq(_name: 'Alex')
+      end
+
+      it 'works with Query' do
+        object = model.create(_name: 'Alex')
+
+        chain = described_class.new(model)
+        expect(chain).to receive(:raw_pages_via_query).and_call_original
+
+        obj, = chain.where(id: object.id).project(:_name).to_a
+        expect(obj.attributes).to eq(_name: 'Alex')
+      end
+    end
   end
 
   describe '#pluck' do
@@ -2007,6 +2135,48 @@ def request_params
         expect(model.where(id: object.id).pluck(:bucket)).to eq([1001])
       end
     end
+
+    context 'when attribute name contains special characters' do
+      let(:model) do
+        new_class do
+          field :'first:name'
+        end
+      end
+
+      it 'works with Scan' do
+        model.create('first:name': 'Alice')
+        model.create('first:name': 'Bob')
+
+        expect(model.pluck(:'first:name')).to contain_exactly('Alice', 'Bob')
+      end
+
+      it 'works with Query' do
+        object = model.create('first:name': 'Alice')
+
+        expect(model.where(id: object.id).pluck(:'first:name')).to eq(['Alice'])
+      end
+    end
+
+    context 'when attribute name starts with _' do
+      let(:model) do
+        new_class do
+          field :_name
+        end
+      end
+
+      it 'works with Scan' do
+        model.create(_name: 'Alice')
+        model.create(_name: 'Bob')
+
+        expect(model.pluck(:_name)).to contain_exactly('Alice', 'Bob')
+      end
+
+      it 'works with Query' do
+        object = model.create(_name: 'Alice')
+
+        expect(model.where(id: object.id).pluck(:_name)).to eq(['Alice'])
+      end
+    end
   end
 
   describe 'User' do