From e3886a32d9040956f6f8e36c8a5174ffd536da0e Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 30 Nov 2023 02:05:36 +0200 Subject: [PATCH 1/5] Migrate specs in criteria_spec.rb --- spec/dynamoid/criteria/chain_spec.rb | 75 +++++++++++++ spec/dynamoid/criteria_new_spec.rb | 2 + spec/dynamoid/criteria_spec.rb | 136 ----------------------- spec/support/helpers/new_class_helper.rb | 5 + 4 files changed, 82 insertions(+), 136 deletions(-) delete mode 100644 spec/dynamoid/criteria_spec.rb diff --git a/spec/dynamoid/criteria/chain_spec.rb b/spec/dynamoid/criteria/chain_spec.rb index 73fd154d..8af6f3f8 100644 --- a/spec/dynamoid/criteria/chain_spec.rb +++ b/spec/dynamoid/criteria/chain_spec.rb @@ -4,6 +4,7 @@ describe Dynamoid::Criteria::Chain do let(:time) { DateTime.now } + # TODO: get rid of predefined models let!(:user) { User.create(name: 'Josh', email: 'josh@joshsymonds.com', password: 'Test123') } let(:chain) { described_class.new(User) } @@ -1261,6 +1262,33 @@ def request_params end.to output('run after_initialize' + 'run after_find').to_stdout end end + + context 'when uses Scan but there are conditions on non-key fields and warn_on_scan config option is true' do + before do + @warn_on_scan = Dynamoid::Config.warn_on_scan + Dynamoid::Config.warn_on_scan = true + end + + after do + Dynamoid::Config.warn_on_scan = @warn_on_scan + end + + it 'logs warnings' do + expect(Dynamoid.logger).to receive(:warn).with('Queries without an index are forced to use scan and are generally much slower than indexed queries!') + expect(Dynamoid.logger).to receive(:warn).with('You can index this query by adding index declaration to user.rb:') + expect(Dynamoid.logger).to receive(:warn).with("* global_secondary_index hash_key: 'some-name', range_key: 'some-another-name'") + expect(Dynamoid.logger).to receive(:warn).with("* local_secondary_index range_key: 'some-name'") + expect(Dynamoid.logger).to receive(:warn).with('Not indexed attributes: :name, :password') + + klass = new_class class_name: :User do + field :name + field :password + end + + klass.create_table + klass.where(name: 'a', password: 'b').all.to_a + end + end end describe '#find_by_pages' do @@ -2278,4 +2306,51 @@ def request_params end end end + + # TODO: check generated request JSON instead of checking parameters of some private method call + describe '#consistent' do + context 'when Query' do + it 'passes consistent_read = true option to adapter when #consistent called' do + klass = new_class + klass.create(id: 'x') + + expect_any_instance_of(Dynamoid::Adapter).to \ + receive(:query) { |_, _, options| options[:consistent_read] == true } + .and_call_original + klass.where(id: 'x').consistent.all.to_a + end + + it 'passes consistent_read = false option to adapter when #consistent is not called' do + klass = new_class + klass.create(id: 'x') + + expect_any_instance_of(Dynamoid::Adapter).to \ + receive(:query) { |_, _, options| options[:consistent_read] == false } + .and_call_original + klass.where(id: 'x').all.to_a + end + end + + context 'when Scan' do + it 'passes consistent_read = true option to adapter when #consistent called' do + klass = new_class + klass.create + + expect_any_instance_of(Dynamoid::Adapter).to \ + receive(:scan) { |_, _, options| options[:consistent_read] == true } + .and_call_original + klass.consistent.all.to_a + end + + it 'passes consistent_read = false option to adapter when #consistent is not called' do + klass = new_class + klass.create + + expect_any_instance_of(Dynamoid::Adapter).to \ + receive(:scan) { |_, _, options| options[:consistent_read] == false } + .and_call_original + klass.all.to_a + end + end + end end diff --git a/spec/dynamoid/criteria_new_spec.rb b/spec/dynamoid/criteria_new_spec.rb index 7a68dbb6..06137e90 100644 --- a/spec/dynamoid/criteria_new_spec.rb +++ b/spec/dynamoid/criteria_new_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' +# Test here that methods defined in Criteria::Chain could be called on a document. +# The methods are tested separately in spec/dynamoid/criteria/chain_spec.rb describe Dynamoid::Criteria do it 'supports querying with .where method' do klass = new_class do diff --git a/spec/dynamoid/criteria_spec.rb b/spec/dynamoid/criteria_spec.rb deleted file mode 100644 index 23c90e59..00000000 --- a/spec/dynamoid/criteria_spec.rb +++ /dev/null @@ -1,136 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -# This file contains legacy specs. They should be revised and moved to the -# criteria_new_spec.rb file. - -describe Dynamoid::Criteria do - let!(:user1) { User.create(name: 'Josh', email: 'josh@joshsymonds.com', admin: true) } - let!(:user2) { User.create(name: 'Justin', email: 'justin@joshsymonds.com', admin: false) } - - it 'finds first using where' do - expect(User.where(name: 'Josh').first).to eq user1 - end - - it 'finds last using where' do - expect(User.where(admin: false).last).to eq user2 - end - - it 'finds all using where' do - expect(User.where(name: 'Josh').all.to_a).to eq [user1] - end - - it 'returns all records' do - expect(Set.new(User.all)).to eq Set.new([user1, user2]) - expect(User.all.first.new_record).to be_falsey - end - - context 'Magazine table' do - before do - Magazine.create_table - end - - it 'returns empty attributes for where' do - expect(Magazine.where(title: 'Josh').all.to_a).to eq [] - end - - it 'returns empty attributes for all' do - expect(Magazine.all.to_a).to eq [] - end - end - - it 'passes each to all members' do - expect { |b| User.each(&b) }.to yield_successive_args( - be_kind_of(User).and(have_attributes('new_record' => false)), - be_kind_of(User).and(have_attributes('new_record' => false)) - ) - end - - it 'passes find_by_pages to all members' do - expect { |b| User.find_by_pages(&b) }.to yield_successive_args( - [all(be_kind_of(User)), { last_evaluated_key: nil }] - ) - end - - it 'returns a last_evaluated_key which may be used to restart iteration' do - # Creates exactly 2 full pages - 58.times { User.create(name: SecureRandom.uuid * 1024) } - - first_page, first_page_meta = User.find_by_pages.first - second_page, = User.start(first_page_meta[:last_evaluated_key]).find_by_pages.first - - expect(first_page & second_page).to be_empty - end - - it 'returns N records' do - 5.times { |i| User.create(name: 'Josh', email: "josh_#{i}@joshsymonds.com") } - expect(User.record_limit(2).all.count).to eq(2) - end - - # TODO: This test is broken using the AWS SDK adapter. - # it 'start with a record' do - # 5.times { |i| User.create(:name => 'Josh', :email => 'josh_#{i}@joshsymonds.com') } - # all = User.all - # User.start(all[3]).all.should eq(all[4..-1]) - # - # all = User.where(:name => 'Josh').all - # User.where(:name => 'Josh').start(all[3]).all.should eq(all[4..-1]) - # end - - it 'send consistent option to adapter' do - pending 'This test is broken as we are overriding the consistent_read option to true inside the adapter' - expect(Dynamoid::Adapter).to receive(:get_item) { |_table_name, _key, options| options[:consistent_read] == true } - User.where(name: 'x').consistent.first - - expect(Dynamoid::Adapter).to receive(:query) { |_table_name, options| options[:consistent_read] == true }.returns([]) - Tweet.where(tweet_id: 'xx', group: 'two').consistent.all - - expect(Dynamoid::Adapter).to receive(:query) { |_table_name, options| options[:consistent_read] == false }.returns([]) - Tweet.where(tweet_id: 'xx', group: 'two').all - end - - it 'does not raise exception when consistent_read is used with scan' do - expect do - User.where(password: 'password').consistent.first - end.not_to raise_error(Dynamoid::Errors::InvalidQuery) - end - - context 'when scans using non-indexed fields and warn_on_scan config option is true' do - before do - @warn_on_scan = Dynamoid::Config.warn_on_scan - Dynamoid::Config.warn_on_scan = true - end - - after do - Dynamoid::Config.warn_on_scan = @warn_on_scan - end - - it 'logs warnings' do - expect(Dynamoid.logger).to receive(:warn).with('Queries without an index are forced to use scan and are generally much slower than indexed queries!') - expect(Dynamoid.logger).to receive(:warn).with('You can index this query by adding index declaration to user.rb:') - expect(Dynamoid.logger).to receive(:warn).with("* global_secondary_index hash_key: 'some-name', range_key: 'some-another-name'") - expect(Dynamoid.logger).to receive(:warn).with("* local_secondary_index range_key: 'some-name'") - expect(Dynamoid.logger).to receive(:warn).with('Not indexed attributes: :name, :password') - - User.where(name: 'x', password: 'password').all - end - end - - context 'when doing intentional, full-table scan (query is empty) and warn_on_scan config option is true' do - before do - @warn_on_scan = Dynamoid::Config.warn_on_scan - Dynamoid::Config.warn_on_scan = true - end - - after do - Dynamoid::Config.warn_on_scan = @warn_on_scan - end - - it 'does not log any warnings' do - expect(Dynamoid.logger).not_to receive(:warn) - - User.all - end - end -end diff --git a/spec/support/helpers/new_class_helper.rb b/spec/support/helpers/new_class_helper.rb index 1fc463f9..4ce04c02 100644 --- a/spec/support/helpers/new_class_helper.rb +++ b/spec/support/helpers/new_class_helper.rb @@ -33,7 +33,12 @@ def new_class(options = {}, &blk) def self.name @class_name end + + def self.to_s + @class_name + end end + klass.class_exec(options, &blk) if block_given? klass end From abf7ceda4be5df6bbb4db77ee2569f3e7bd191b6 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 30 Nov 2023 02:06:20 +0200 Subject: [PATCH 2/5] Rename criteria_new_spec.rb to criteria_spec.rb --- spec/dynamoid/{criteria_new_spec.rb => criteria_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/dynamoid/{criteria_new_spec.rb => criteria_spec.rb} (100%) diff --git a/spec/dynamoid/criteria_new_spec.rb b/spec/dynamoid/criteria_spec.rb similarity index 100% rename from spec/dynamoid/criteria_new_spec.rb rename to spec/dynamoid/criteria_spec.rb From e04911aa687366ec7f918afa21a50d2a23f19183 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 30 Nov 2023 23:14:05 +0200 Subject: [PATCH 3/5] Support Map attribute referencing in where's condition --- lib/dynamoid/criteria/chain.rb | 41 ++++++++++++++---- spec/dynamoid/criteria/chain_spec.rb | 64 ++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 9 deletions(-) diff --git a/lib/dynamoid/criteria/chain.rb b/lib/dynamoid/criteria/chain.rb index 2902803b..38de2b18 100644 --- a/lib/dynamoid/criteria/chain.rb +++ b/lib/dynamoid/criteria/chain.rb @@ -585,14 +585,36 @@ def count_via_scan end def field_condition(key, value_before_type_casting) - name, operator = key.to_s.split('.') - value = type_cast_condition_parameter(name, value_before_type_casting) - operator ||= 'eq' + sections = key.to_s.split('.') + + if sections.size == 1 + name = sections[0] + selector = name + operator = nil + elsif sections.last.in? ALLOWED_FIELD_OPERATORS + name = sections[0] + selector = sections[0...-1].join('.') + operator = sections[-1] + else + name = sections[0] + selector = sections.join('.') + operator = nil + end - unless operator.in? ALLOWED_FIELD_OPERATORS - raise Dynamoid::Errors::Error, "Unsupported operator #{operator} in #{key}" + type = source.attributes[name.to_sym][:type] + if type != :map && name != selector + raise Dynamoid::Errors::Error, + "Map element referencing (#{key}) in condition is not allowed for not :map field '#{name}'" end + value = if name == selector + type_cast_condition_parameter(name, value_before_type_casting) + else + value_before_type_casting + end + + operator ||= 'eq' + condition = \ case operator # NULL/NOT_NULL operators don't have parameters @@ -606,7 +628,7 @@ def field_condition(key, value_before_type_casting) [operator.to_sym, value] end - [name.to_sym, condition] + [selector, condition] end def query_key_conditions @@ -726,9 +748,10 @@ def scan_conditions {}.tap do |opts| @where_conditions.keys.map(&:to_sym).each do |key| - name, condition = field_condition(key, @where_conditions[key]) - opts[name] ||= [] - opts[name] << condition + selector, condition = field_condition(key, @where_conditions[key]) + + opts[selector] ||= [] + opts[selector] << condition end end end diff --git a/spec/dynamoid/criteria/chain_spec.rb b/spec/dynamoid/criteria/chain_spec.rb index 8af6f3f8..c7cda367 100644 --- a/spec/dynamoid/criteria/chain_spec.rb +++ b/spec/dynamoid/criteria/chain_spec.rb @@ -687,6 +687,70 @@ def request_params end end + describe 'condition on a Map key-value pair' do + context 'when Scan' 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('settings.threshold' => 10) + expect(chain).to receive(:raw_pages_via_scan).and_call_original + expect(chain.to_a).to eq [object] + + chain = klass_with_map.where('settings.threshold' => 12) + expect(chain).to receive(:raw_pages_via_scan).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('settings.threshold.gt' => 5) + expect(chain).to receive(:raw_pages_via_scan).and_call_original + expect(chain.to_a).to eq [object] + + chain = klass_with_map.where('settings.threshold.lt' => 5) + expect(chain).to receive(:raw_pages_via_scan).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('settings.threshold.foobar' => 5) + expect(chain).to receive(:raw_pages_via_scan).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('age.threshold' => 5) + 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'/) + 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_a }.to raise_error(ArgumentError, /unsupported type, expected Hash, Array,/) + end + end + end + describe 'local secondary indexes used for `where` clauses' do let(:model) do new_class(partition_key: :name) do From 320780d9cfb736bbb6cd78f6ebecddd881ebc9c6 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Fri, 1 Dec 2023 00:09:32 +0200 Subject: [PATCH 4/5] @wip --- .../aws_sdk_v3/filter_expression_convertor.rb | 23 ++-- lib/dynamoid/criteria/chain.rb | 15 +-- spec/dynamoid/criteria/chain_spec.rb | 100 ++++++++++++++++-- 3 files changed, 117 insertions(+), 21 deletions(-) diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb index 7698dcb7..d7a303df 100644 --- a/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb +++ b/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) diff --git a/lib/dynamoid/criteria/chain.rb b/lib/dynamoid/criteria/chain.rb index 38de2b18..e4ef9f30 100644 --- a/lib/dynamoid/criteria/chain.rb +++ b/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 diff --git a/spec/dynamoid/criteria/chain_spec.rb b/spec/dynamoid/criteria/chain_spec.rb index c7cda367..590525d5 100644 --- a/spec/dynamoid/criteria/chain_spec.rb +++ b/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 From ec824483cc3f002a426f66ff674a5e8969223a4e Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Sat, 30 Dec 2023 19:50:55 +0200 Subject: [PATCH 5/5] @wip --- Gemfile.lock | 2 +- lib/dynamoid/adapter_plugin/aws_sdk_v3/create_table.rb | 1 + .../aws_sdk_v3/filter_expression_convertor.rb | 2 +- lib/dynamoid/criteria/chain.rb | 2 +- lib/dynamoid/criteria/where_conditions.rb | 5 ++++- spec/dynamoid/criteria/chain_spec.rb | 10 ++++++---- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index c0ce2748..450d7515 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - dynamoid (3.8.0) + dynamoid (3.9.0) activemodel (>= 4) aws-sdk-dynamodb (~> 1.0) concurrent-ruby (>= 1.0) diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3/create_table.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3/create_table.rb index 6c1c6f35..60ead1c2 100644 --- a/lib/dynamoid/adapter_plugin/aws_sdk_v3/create_table.rb +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3/create_table.rb @@ -168,6 +168,7 @@ def attribute_definition_element(name, dynamoid_type) # Converts from symbol to the API string for the given data type # E.g. :number -> 'N' def api_type(type) + # TODO: seems we need more types to support case type when :string then STRING_TYPE when :number then NUM_TYPE diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb index d7a303df..f0244f96 100644 --- a/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb @@ -60,7 +60,7 @@ def build end # Replace reserved words with placeholders - def name_or_placeholder_for(path) + def name_or_placeholder_for(path) # TODO: support List elements sections = path.to_s.split('.') sanitized = sections.map do |name| diff --git a/lib/dynamoid/criteria/chain.rb b/lib/dynamoid/criteria/chain.rb index e4ef9f30..173a578b 100644 --- a/lib/dynamoid/criteria/chain.rb +++ b/lib/dynamoid/criteria/chain.rb @@ -748,7 +748,7 @@ def scan_conditions end {}.tap do |opts| - @where_conditions.keys.map(&:to_sym).each do |key| + @where_conditions.keys.map(&:to_sym).each do |key| # TODO: remove map to_sym selector, condition = field_condition(key, @where_conditions[key]) opts[selector] ||= [] diff --git a/lib/dynamoid/criteria/where_conditions.rb b/lib/dynamoid/criteria/where_conditions.rb index 52baadd7..acfcba95 100644 --- a/lib/dynamoid/criteria/where_conditions.rb +++ b/lib/dynamoid/criteria/where_conditions.rb @@ -22,7 +22,10 @@ def empty? def [](key) hash = @conditions.find { |h| h.key?(key) } - hash[key] if hash + + return nil unless hash + + hash[key] end end end diff --git a/spec/dynamoid/criteria/chain_spec.rb b/spec/dynamoid/criteria/chain_spec.rb index 590525d5..a033e30e 100644 --- a/spec/dynamoid/criteria/chain_spec.rb +++ b/spec/dynamoid/criteria/chain_spec.rb @@ -456,7 +456,7 @@ def request_params end # http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/LegacyConditionalParameters.ScanFilter.html - describe 'Scan conditions ' do + describe 'Scan conditions' do let(:model) do new_class do field :age, :integer @@ -693,6 +693,8 @@ def request_params end end + describe 'condition on a List element' + describe 'condition on a Map key-value pair' do context 'when Query' do let(:klass_with_map) do @@ -1273,7 +1275,7 @@ def request_params end describe '#where' do - context 'passed condition for nonexistent attribute' do + context 'passed condition for non-existing attribute' do let(:model) do new_class do field :city @@ -1291,14 +1293,14 @@ def request_params model.where(town: 'New York') end - it 'writes warning message for condition with operator' do + it 'writes warning message even if there is an operator' do expect(Dynamoid.logger).to receive(:warn) .with('where conditions contain nonexistent field name `town`') model.where('town.contain': 'New York') end - it 'writes warning message with a list of attributes' do + it 'writes warning message if there are several non-existing attributes' do expect(Dynamoid.logger).to receive(:warn) .with('where conditions contain nonexistent field names `town`, `street1`')