diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c147a48..4fbf1f71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `#touch` * `#increment!` * `#decrement!` -* [#642](https://github.com/Dynamoid/dynamoid/pull/642) Run specs on CI agains Ruby 3.2 +* [#642](https://github.com/Dynamoid/dynamoid/pull/642) Run specs on CI against Ruby 3.2 * [#645](https://github.com/Dynamoid/dynamoid/pull/645) Added `after_find` callback ### Changed * [#610](https://github.com/Dynamoid/dynamoid/pull/610) Switch to [`rubocop-lts`](https://rubocop-lts.gitlab.io/) (@pboling) diff --git a/README.md b/README.md index 0dfac838..1652f61d 100644 --- a/README.md +++ b/README.md @@ -723,18 +723,6 @@ join, but instead finds all the user's addresses and naively filters them in Ruby. For large associations this is a performance hit compared to relational database engines. -**WARNING:** There is a limitation of conditions passed to `where` -method. Only one condition for some particular field could be specified. -The last one only will be applied and others will be ignored. E.g. in -examples: - -```ruby -User.where('age.gt': 10, 'age.lt': 20) -User.where(name: 'Mike').where('name.begins_with': 'Ed') -``` - -the first one will be ignored and the last one will be used. - **Warning:** There is a caveat with filtering documents by `nil` value attribute. By default Dynamoid ignores attributes with `nil` value and doesn't store them in a DynamoDB document. This behavior could be diff --git a/lib/dynamoid/adapter.rb b/lib/dynamoid/adapter.rb index a0d08327..664a67c2 100644 --- a/lib/dynamoid/adapter.rb +++ b/lib/dynamoid/adapter.rb @@ -171,19 +171,25 @@ def method_missing(method, *args, &block) # only really useful for range queries, since it can only find by one hash key at once. Only provide # one range key to the hash. # + # Dynamoid.adapter.query('users', { id: [[:eq, '1']], age: [[:between, [10, 30]]] }, { batch_size: 1000 }) + # # @param [String] table_name the name of the table - # @param [Hash] opts the options to query the table with - # @option opts [String] :hash_value the value of the hash key to find - # @option opts [Range] :range_value find the range key within this range - # @option opts [Number] :range_greater_than find range keys greater than this - # @option opts [Number] :range_less_than find range keys less than this - # @option opts [Number] :range_gte find range keys greater than or equal to this - # @option opts [Number] :range_lte find range keys less than or equal to this - # - # @return [Array] an array of all matching items - # - def query(table_name, opts = {}) - adapter.query(table_name, opts) + # @param [Array[Array]] key_conditions conditions for the primary key attributes + # @param [Array[Array]] non_key_conditions (optional) conditions for non-primary key attributes + # @param [Hash] options (optional) the options to query the table with + # @option options [Boolean] :consistent_read You can set the ConsistentRead parameter to true and obtain a strongly consistent result + # @option options [Boolean] :scan_index_forward Specifies the order for index traversal: If true (default), the traversal is performed in ascending order; if false, the traversal is performed in descending order. + # @option options [Symbop] :select The attributes to be returned in the result (one of ALL_ATTRIBUTES, ALL_PROJECTED_ATTRIBUTES, ...) + # @option options [Symbol] :index_name The name of an index to query. This index can be any local secondary index or global secondary index on the table. + # @option options [Hash] :exclusive_start_key The primary key of the first item that this operation will evaluate. + # @option options [Integer] :batch_size The number of items to lazily load one by one + # @option options [Integer] :record_limit The maximum number of items to return (not necessarily the number of evaluated items) + # @option options [Integer] :scan_limit The maximum number of items to evaluate (not necessarily the number of matching items) + # @option options [Array[Symbol]] :project The attributes to retrieve from the table + # + # @return [Enumerable] matching items + def query(table_name, key_conditions, non_key_conditions = {}, options = {}) + adapter.query(table_name, key_conditions, non_key_conditions, options) end def self.adapter_plugin_class diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3.rb index c43cbbae..d4d3f641 100644 --- a/lib/dynamoid/adapter_plugin/aws_sdk_v3.rb +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3.rb @@ -24,31 +24,6 @@ module AdapterPlugin class AwsSdkV3 EQ = 'EQ' - RANGE_MAP = { - range_greater_than: 'GT', - range_less_than: 'LT', - range_gte: 'GE', - range_lte: 'LE', - range_begins_with: 'BEGINS_WITH', - range_between: 'BETWEEN', - range_eq: 'EQ' - }.freeze - - FIELD_MAP = { - eq: 'EQ', - ne: 'NE', - gt: 'GT', - lt: 'LT', - gte: 'GE', - lte: 'LE', - begins_with: 'BEGINS_WITH', - between: 'BETWEEN', - in: 'IN', - contains: 'CONTAINS', - not_contains: 'NOT_CONTAINS', - null: 'NULL', - not_null: 'NOT_NULL', - }.freeze HASH_KEY = 'HASH' RANGE_KEY = 'RANGE' STRING_TYPE = 'S' @@ -70,26 +45,70 @@ class AwsSdkV3 CONNECTION_CONFIG_OPTIONS = %i[endpoint region http_continue_timeout http_idle_timeout http_open_timeout http_read_timeout].freeze - attr_reader :table_cache + # See https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/ReservedWords.html + RESERVED_WORDS = Set.new( + %i[ + ABORT ABSOLUTE ACTION ADD AFTER AGENT AGGREGATE ALL ALLOCATE ALTER ANALYZE + AND ANY ARCHIVE ARE ARRAY AS ASC ASCII ASENSITIVE ASSERTION ASYMMETRIC AT + ATOMIC ATTACH ATTRIBUTE AUTH AUTHORIZATION AUTHORIZE AUTO AVG BACK BACKUP + BASE BATCH BEFORE BEGIN BETWEEN BIGINT BINARY BIT BLOB BLOCK BOOLEAN BOTH + BREADTH BUCKET BULK BY BYTE CALL CALLED CALLING CAPACITY CASCADE CASCADED + CASE CAST CATALOG CHAR CHARACTER CHECK CLASS CLOB CLOSE CLUSTER CLUSTERED + CLUSTERING CLUSTERS COALESCE COLLATE COLLATION COLLECTION COLUMN COLUMNS + COMBINE COMMENT COMMIT COMPACT COMPILE COMPRESS CONDITION CONFLICT CONNECT + CONNECTION CONSISTENCY CONSISTENT CONSTRAINT CONSTRAINTS CONSTRUCTOR + CONSUMED CONTINUE CONVERT COPY CORRESPONDING COUNT COUNTER CREATE CROSS + CUBE CURRENT CURSOR CYCLE DATA DATABASE DATE DATETIME DAY DEALLOCATE DEC + DECIMAL DECLARE DEFAULT DEFERRABLE DEFERRED DEFINE DEFINED DEFINITION + DELETE DELIMITED DEPTH DEREF DESC DESCRIBE DESCRIPTOR DETACH DETERMINISTIC + DIAGNOSTICS DIRECTORIES DISABLE DISCONNECT DISTINCT DISTRIBUTE DO DOMAIN + DOUBLE DROP DUMP DURATION DYNAMIC EACH ELEMENT ELSE ELSEIF EMPTY ENABLE + END EQUAL EQUALS ERROR ESCAPE ESCAPED EVAL EVALUATE EXCEEDED EXCEPT + EXCEPTION EXCEPTIONS EXCLUSIVE EXEC EXECUTE EXISTS EXIT EXPLAIN EXPLODE + EXPORT EXPRESSION EXTENDED EXTERNAL EXTRACT FAIL FALSE FAMILY FETCH FIELDS + FILE FILTER FILTERING FINAL FINISH FIRST FIXED FLATTERN FLOAT FOR FORCE + FOREIGN FORMAT FORWARD FOUND FREE FROM FULL FUNCTION FUNCTIONS GENERAL + GENERATE GET GLOB GLOBAL GO GOTO GRANT GREATER GROUP GROUPING HANDLER HASH + HAVE HAVING HEAP HIDDEN HOLD HOUR IDENTIFIED IDENTITY IF IGNORE IMMEDIATE + IMPORT IN INCLUDING INCLUSIVE INCREMENT INCREMENTAL INDEX INDEXED INDEXES + INDICATOR INFINITE INITIALLY INLINE INNER INNTER INOUT INPUT INSENSITIVE + INSERT INSTEAD INT INTEGER INTERSECT INTERVAL INTO INVALIDATE IS ISOLATION + ITEM ITEMS ITERATE JOIN KEY KEYS LAG LANGUAGE LARGE LAST LATERAL LEAD + LEADING LEAVE LEFT LENGTH LESS LEVEL LIKE LIMIT LIMITED LINES LIST LOAD + LOCAL LOCALTIME LOCALTIMESTAMP LOCATION LOCATOR LOCK LOCKS LOG LOGED LONG + LOOP LOWER MAP MATCH MATERIALIZED MAX MAXLEN MEMBER MERGE METHOD METRICS + MIN MINUS MINUTE MISSING MOD MODE MODIFIES MODIFY MODULE MONTH MULTI + MULTISET NAME NAMES NATIONAL NATURAL NCHAR NCLOB NEW NEXT NO NONE NOT NULL + NULLIF NUMBER NUMERIC OBJECT OF OFFLINE OFFSET OLD ON ONLINE ONLY OPAQUE + OPEN OPERATOR OPTION OR ORDER ORDINALITY OTHER OTHERS OUT OUTER OUTPUT + OVER OVERLAPS OVERRIDE OWNER PAD PARALLEL PARAMETER PARAMETERS PARTIAL + PARTITION PARTITIONED PARTITIONS PATH PERCENT PERCENTILE PERMISSION + PERMISSIONS PIPE PIPELINED PLAN POOL POSITION PRECISION PREPARE PRESERVE + PRIMARY PRIOR PRIVATE PRIVILEGES PROCEDURE PROCESSED PROJECT PROJECTION + PROPERTY PROVISIONING PUBLIC PUT QUERY QUIT QUORUM RAISE RANDOM RANGE RANK + RAW READ READS REAL REBUILD RECORD RECURSIVE REDUCE REF REFERENCE + REFERENCES REFERENCING REGEXP REGION REINDEX RELATIVE RELEASE REMAINDER + RENAME REPEAT REPLACE REQUEST RESET RESIGNAL RESOURCE RESPONSE RESTORE + RESTRICT RESULT RETURN RETURNING RETURNS REVERSE REVOKE RIGHT ROLE ROLES + ROLLBACK ROLLUP ROUTINE ROW ROWS RULE RULES SAMPLE SATISFIES SAVE SAVEPOINT + SCAN SCHEMA SCOPE SCROLL SEARCH SECOND SECTION SEGMENT SEGMENTS SELECT SELF + SEMI SENSITIVE SEPARATE SEQUENCE SERIALIZABLE SESSION SET SETS SHARD SHARE + SHARED SHORT SHOW SIGNAL SIMILAR SIZE SKEWED SMALLINT SNAPSHOT SOME SOURCE + SPACE SPACES SPARSE SPECIFIC SPECIFICTYPE SPLIT SQL SQLCODE SQLERROR + SQLEXCEPTION SQLSTATE SQLWARNING START STATE STATIC STATUS STORAGE STORE + STORED STREAM STRING STRUCT STYLE SUB SUBMULTISET SUBPARTITION SUBSTRING + SUBTYPE SUM SUPER SYMMETRIC SYNONYM SYSTEM TABLE TABLESAMPLE TEMP TEMPORARY + TERMINATED TEXT THAN THEN THROUGHPUT TIME TIMESTAMP TIMEZONE TINYINT TO + TOKEN TOTAL TOUCH TRAILING TRANSACTION TRANSFORM TRANSLATE TRANSLATION + TREAT TRIGGER TRIM TRUE TRUNCATE TTL TUPLE TYPE UNDER UNDO UNION UNIQUE UNIT + UNKNOWN UNLOGGED UNNEST UNPROCESSED UNSIGNED UNTIL UPDATE UPPER URL USAGE + USE USER USERS USING UUID VACUUM VALUE VALUED VALUES VARCHAR VARIABLE + VARIANCE VARINT VARYING VIEW VIEWS VIRTUAL VOID WAIT WHEN WHENEVER WHERE + WHILE WINDOW WITH WITHIN WITHOUT WORK WRAPPED WRITE YEAR ZONE + ] + ).freeze - # Build an array of values for Condition - # Is used in ScanFilter and QueryFilter - # https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_Condition.html - # @param [String] operator value of RANGE_MAP or FIELD_MAP hash, e.g. "EQ", "LT" etc - # @param [Object] value scalar value or array/set - def self.attribute_value_list(operator, value) - # For BETWEEN and IN operators we should keep value as is (it should be already an array) - # NULL and NOT_NULL require absence of attribute list - # For all the other operators we wrap the value with array - # https://docs.aws.amazon.com/en_us/amazondynamodb/latest/developerguide/LegacyConditionalParameters.Conditions.html - if %w[BETWEEN IN].include?(operator) - [value].flatten - elsif %w[NULL NOT_NULL].include?(operator) - nil - else - [value] - end - end + attr_reader :table_cache # Establish the connection to DynamoDB. # @@ -470,25 +489,32 @@ def put_item(table_name, object, options = {}) # only really useful for range queries, since it can only find by one hash key at once. Only provide # one range key to the hash. # + # Dynamoid.adapter.query('users', { id: [[:eq, '1']], age: [[:between, [10, 30]]] }, { batch_size: 1000 }) + # # @param [String] table_name the name of the table - # @param [Hash] options the options to query the table with - # @option options [String] :hash_value the value of the hash key to find - # @option options [Number, Number] :range_between find the range key within this range - # @option options [Number] :range_greater_than find range keys greater than this - # @option options [Number] :range_less_than find range keys less than this - # @option options [Number] :range_gte find range keys greater than or equal to this - # @option options [Number] :range_lte find range keys less than or equal to this + # @param [Array[Array]] key_conditions conditions for the primary key attributes + # @param [Array[Array]] non_key_conditions (optional) conditions for non-primary key attributes + # @param [Hash] options (optional) the options to query the table with + # @option options [Boolean] :consistent_read You can set the ConsistentRead parameter to true and obtain a strongly consistent result + # @option options [Boolean] :scan_index_forward Specifies the order for index traversal: If true (default), the traversal is performed in ascending order; if false, the traversal is performed in descending order. + # @option options [Symbop] :select The attributes to be returned in the result (one of ALL_ATTRIBUTES, ALL_PROJECTED_ATTRIBUTES, ...) + # @option options [Symbol] :index_name The name of an index to query. This index can be any local secondary index or global secondary index on the table. + # @option options [Hash] :exclusive_start_key The primary key of the first item that this operation will evaluate. + # @option options [Integer] :batch_size The number of items to lazily load one by one + # @option options [Integer] :record_limit The maximum number of items to return (not necessarily the number of evaluated items) + # @option options [Integer] :scan_limit The maximum number of items to evaluate (not necessarily the number of matching items) + # @option options [Array[Symbol]] :project The attributes to retrieve from the table # # @return [Enumerable] matching items # # @since 1.0.0 # # @todo Provide support for various other options http://docs.aws.amazon.com/sdkforruby/api/Aws/DynamoDB/Client.html#query-instance_method - def query(table_name, options = {}) + def query(table_name, key_conditions, non_key_conditions = {}, options = {}) Enumerator.new do |yielder| table = describe_table(table_name) - Query.new(client, table, options).call.each do |page| + Query.new(client, table, key_conditions, non_key_conditions, options).call.each do |page| yielder.yield( page.items.map { |item| item_to_hash(item) }, last_evaluated_key: page.last_evaluated_key @@ -497,11 +523,11 @@ def query(table_name, options = {}) end end - def query_count(table_name, options = {}) + def query_count(table_name, key_conditions, non_key_conditions, options) table = describe_table(table_name) options[:select] = 'COUNT' - Query.new(client, table, options).call + Query.new(client, table, key_conditions, non_key_conditions, options).call .map(&:count) .reduce(:+) end 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 new file mode 100644 index 00000000..7698dcb7 --- /dev/null +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3/filter_expression_convertor.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module Dynamoid + # @private + module AdapterPlugin + class AwsSdkV3 + class FilterExpressionConvertor + attr_reader :expression, :name_placeholders, :value_placeholders + + def initialize(conditions, name_placeholders, value_placeholders, name_placeholder_sequence, value_placeholder_sequence) + @conditions = conditions + @name_placeholders = name_placeholders.dup + @value_placeholders = value_placeholders.dup + @name_placeholder_sequence = name_placeholder_sequence + @value_placeholder_sequence = value_placeholder_sequence + + build + end + + private + + def build + clauses = @conditions.map do |name, attribute_conditions| + attribute_conditions.map do |operator, value| + name_or_placeholder = name_or_placeholder_for(name) + + case operator + when :eq + "#{name_or_placeholder} = #{value_placeholder_for(value)}" + when :ne + "#{name_or_placeholder} <> #{value_placeholder_for(value)}" + when :gt + "#{name_or_placeholder} > #{value_placeholder_for(value)}" + when :lt + "#{name_or_placeholder} < #{value_placeholder_for(value)}" + when :gte + "#{name_or_placeholder} >= #{value_placeholder_for(value)}" + when :lte + "#{name_or_placeholder} <= #{value_placeholder_for(value)}" + when :between + "#{name_or_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)})" + when :in + list = value.map(&method(:value_placeholder_for)).join(' , ') + "#{name_or_placeholder} IN (#{list})" + when :contains + "contains (#{name_or_placeholder}, #{value_placeholder_for(value)})" + when :not_contains + "NOT contains (#{name_or_placeholder}, #{value_placeholder_for(value)})" + when :null + "attribute_not_exists (#{name_or_placeholder})" + when :not_null + "attribute_exists (#{name_or_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) + + placeholder = @name_placeholder_sequence.call + @name_placeholders[placeholder] = name + placeholder + end + + def value_placeholder_for(value) + placeholder = @value_placeholder_sequence.call + @value_placeholders[placeholder] = value + placeholder + end + end + end + end +end diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3/projection_expression_convertor.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3/projection_expression_convertor.rb new file mode 100644 index 00000000..1b71f1c4 --- /dev/null +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3/projection_expression_convertor.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Dynamoid + # @private + module AdapterPlugin + class AwsSdkV3 + class ProjectionExpressionConvertor + attr_reader :expression, :name_placeholders + + def initialize(names, name_placeholders, name_placeholder_sequence) + @names = names + @name_placeholders = name_placeholders.dup + @name_placeholder_sequence = name_placeholder_sequence + + build + end + + private + + 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 + end + + @expression = clauses.join(' , ') + end + end + end + end +end diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3/query.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3/query.rb index 48598cce..25cf8835 100644 --- a/lib/dynamoid/adapter_plugin/aws_sdk_v3/query.rb +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3/query.rb @@ -3,6 +3,8 @@ require_relative 'middleware/backoff' require_relative 'middleware/limit' require_relative 'middleware/start_key' +require_relative 'filter_expression_convertor' +require_relative 'projection_expression_convertor' module Dynamoid # @private @@ -10,20 +12,19 @@ module AdapterPlugin class AwsSdkV3 class Query OPTIONS_KEYS = %i[ - limit hash_key hash_value range_key consistent_read scan_index_forward - select index_name batch_size exclusive_start_key record_limit scan_limit - project + consistent_read scan_index_forward select index_name batch_size + exclusive_start_key record_limit scan_limit project ].freeze attr_reader :client, :table, :options, :conditions - def initialize(client, table, opts = {}) + def initialize(client, table, key_conditions, non_key_conditions, options) @client = client @table = table - opts = opts.symbolize_keys - @options = opts.slice(*OPTIONS_KEYS) - @conditions = opts.except(*OPTIONS_KEYS) + @key_conditions = key_conditions + @non_key_conditions = non_key_conditions + @options = options.slice(*OPTIONS_KEYS) end def call @@ -53,6 +54,37 @@ def call private def build_request + # expressions + name_placeholder = +'#_a0' + value_placeholder = +':_a0' + + name_placeholder_sequence = -> { name_placeholder.next!.dup } + value_placeholder_sequence = -> { value_placeholder.next!.dup } + + name_placeholders = {} + value_placeholders = {} + + # Deal with various limits and batching + batch_size = options[:batch_size] + limit = [record_limit, scan_limit, batch_size].compact.min + + # key condition expression + convertor = FilterExpressionConvertor.new(@key_conditions, name_placeholders, value_placeholders, name_placeholder_sequence, value_placeholder_sequence) + key_condition_expression = convertor.expression + value_placeholders = convertor.value_placeholders + name_placeholders = convertor.name_placeholders + + # filter expression + convertor = FilterExpressionConvertor.new(@non_key_conditions, name_placeholders, value_placeholders, name_placeholder_sequence, value_placeholder_sequence) + filter_expression = convertor.expression + value_placeholders = convertor.value_placeholders + name_placeholders = convertor.name_placeholders + + # projection expression + convertor = ProjectionExpressionConvertor.new(options[:project], name_placeholders, name_placeholder_sequence) + projection_expression = convertor.expression + name_placeholders = convertor.name_placeholders + request = options.slice( :consistent_read, :scan_index_forward, @@ -61,15 +93,13 @@ def build_request :exclusive_start_key ).compact - # Deal with various limits and batching - batch_size = options[:batch_size] - limit = [record_limit, scan_limit, batch_size].compact.min - - request[:limit] = limit if limit - request[:table_name] = table.name - request[:key_conditions] = key_conditions - request[:query_filter] = query_filter - request[:attributes_to_get] = attributes_to_get + request[:table_name] = table.name + request[:limit] = limit if limit + request[:key_condition_expression] = key_condition_expression if key_condition_expression.present? + request[:filter_expression] = filter_expression if filter_expression.present? + request[:expression_attribute_values] = value_placeholders if value_placeholders.present? + request[:expression_attribute_names] = name_placeholders if name_placeholders.present? + request[:projection_expression] = projection_expression if projection_expression.present? request end @@ -81,51 +111,6 @@ def record_limit def scan_limit options[:scan_limit] end - - def hash_key_name - (options[:hash_key] || table.hash_key) - end - - def range_key_name - (options[:range_key] || table.range_key) - end - - def key_conditions - result = { - hash_key_name => { - comparison_operator: AwsSdkV3::EQ, - attribute_value_list: AwsSdkV3.attribute_value_list(AwsSdkV3::EQ, options[:hash_value].freeze) - } - } - - conditions.slice(*AwsSdkV3::RANGE_MAP.keys).each do |k, _v| - op = AwsSdkV3::RANGE_MAP[k] - - result[range_key_name] = { - comparison_operator: op, - attribute_value_list: AwsSdkV3.attribute_value_list(op, conditions[k].freeze) - } - end - - result - end - - def query_filter - conditions.except(*AwsSdkV3::RANGE_MAP.keys).reduce({}) do |result, (attr, cond)| - condition = { - comparison_operator: AwsSdkV3::FIELD_MAP[cond.keys[0]], - attribute_value_list: AwsSdkV3.attribute_value_list(AwsSdkV3::FIELD_MAP[cond.keys[0]], cond.values[0].freeze) - } - result[attr] = condition - result - end - end - - def attributes_to_get - return if options[:project].nil? - - options[:project].map(&:to_s) - end end end end diff --git a/lib/dynamoid/adapter_plugin/aws_sdk_v3/scan.rb b/lib/dynamoid/adapter_plugin/aws_sdk_v3/scan.rb index 52c106c7..d4b1d7ad 100644 --- a/lib/dynamoid/adapter_plugin/aws_sdk_v3/scan.rb +++ b/lib/dynamoid/adapter_plugin/aws_sdk_v3/scan.rb @@ -3,6 +3,8 @@ require_relative 'middleware/backoff' require_relative 'middleware/limit' require_relative 'middleware/start_key' +require_relative 'filter_expression_convertor' +require_relative 'projection_expression_convertor' module Dynamoid # @private @@ -45,6 +47,31 @@ def call private def build_request + # expressions + name_placeholder = +'#_a0' + value_placeholder = +':_a0' + + name_placeholder_sequence = -> { name_placeholder.next!.dup } + value_placeholder_sequence = -> { value_placeholder.next!.dup } + + name_placeholders = {} + value_placeholders = {} + + # Deal with various limits and batching + batch_size = options[:batch_size] + limit = [record_limit, scan_limit, batch_size].compact.min + + # filter expression + convertor = FilterExpressionConvertor.new(conditions, name_placeholders, value_placeholders, name_placeholder_sequence, value_placeholder_sequence) + filter_expression = convertor.expression + value_placeholders = convertor.value_placeholders + name_placeholders = convertor.name_placeholders + + # projection expression + convertor = ProjectionExpressionConvertor.new(options[:project], name_placeholders, name_placeholder_sequence) + projection_expression = convertor.expression + name_placeholders = convertor.name_placeholders + request = options.slice( :consistent_read, :exclusive_start_key, @@ -52,14 +79,12 @@ def build_request :index_name ).compact - # Deal with various limits and batching - batch_size = options[:batch_size] - limit = [record_limit, scan_limit, batch_size].compact.min - - request[:limit] = limit if limit - request[:table_name] = table.name - request[:scan_filter] = scan_filter - request[:attributes_to_get] = attributes_to_get + request[:table_name] = table.name + request[:limit] = limit if limit + request[:filter_expression] = filter_expression if filter_expression.present? + request[:expression_attribute_values] = value_placeholders if value_placeholders.present? + request[:expression_attribute_names] = name_placeholders if name_placeholders.present? + request[:projection_expression] = projection_expression if projection_expression.present? request end @@ -71,25 +96,6 @@ def record_limit def scan_limit options[:scan_limit] end - - def scan_filter - conditions.reduce({}) do |result, (attr, cond)| - condition = { - comparison_operator: AwsSdkV3::FIELD_MAP[cond.keys[0]], - attribute_value_list: AwsSdkV3.attribute_value_list(AwsSdkV3::FIELD_MAP[cond.keys[0]], cond.values[0].freeze) - } - # nil means operator doesn't require attribute value list - conditions.delete(:attribute_value_list) if conditions[:attribute_value_list].nil? - result[attr] = condition - result - end - end - - def attributes_to_get - return if options[:project].nil? - - options[:project].map(&:to_s) - end end end end diff --git a/lib/dynamoid/criteria/chain.rb b/lib/dynamoid/criteria/chain.rb index c8d72ec9..2902803b 100644 --- a/lib/dynamoid/criteria/chain.rb +++ b/lib/dynamoid/criteria/chain.rb @@ -1,29 +1,35 @@ # frozen_string_literal: true require_relative 'key_fields_detector' -require_relative 'ignored_conditions_detector' -require_relative 'overwritten_conditions_detector' require_relative 'nonexistent_fields_detector' +require_relative 'where_conditions' module Dynamoid module Criteria # The criteria chain is equivalent to an ActiveRecord relation (and realistically I should change the name from # chain to relation). It is a chainable object that builds up a query and eventually executes it by a Query or Scan. class Chain - attr_reader :query, :source, :consistent_read, :key_fields_detector + attr_reader :source, :consistent_read, :key_fields_detector include Enumerable + + ALLOWED_FIELD_OPERATORS = Set.new( + %w[ + eq ne gt lt gte lte between begins_with in contains not_contains null not_null + ] + ).freeze + # Create a new criteria chain. # # @param [Class] source the class upon which the ultimate query will be performed. def initialize(source) - @query = {} + @where_conditions = WhereConditions.new @source = source @consistent_read = false @scan_index_forward = true - # we should re-initialize keys detector every time we change query - @key_fields_detector = KeyFieldsDetector.new(@query, @source) + # we should re-initialize keys detector every time we change @where_conditions + @key_fields_detector = KeyFieldsDetector.new(@where_conditions, @source) end # Returns a chain which is a result of filtering current chain with the specified conditions. @@ -92,25 +98,15 @@ def initialize(source) # @return [Dynamoid::Criteria::Chain] # @since 0.2.0 def where(args) - detector = IgnoredConditionsDetector.new(args) - if detector.found? - Dynamoid.logger.warn(detector.warning_message) - end - - detector = OverwrittenConditionsDetector.new(@query, args) - if detector.found? - Dynamoid.logger.warn(detector.warning_message) - end - detector = NonexistentFieldsDetector.new(args, @source) if detector.found? Dynamoid.logger.warn(detector.warning_message) end - query.update(args.symbolize_keys) + @where_conditions.update(args.symbolize_keys) - # we should re-initialize keys detector every time we change query - @key_fields_detector = KeyFieldsDetector.new(@query, @source, forced_index_name: @forced_index_name) + # we should re-initialize keys detector every time we change @where_conditions + @key_fields_detector = KeyFieldsDetector.new(@where_conditions, @source, forced_index_name: @forced_index_name) self end @@ -187,7 +183,7 @@ def count def first(*args) n = args.first || 1 - return dup.scan_limit(n).to_a.first(*args) if @query.blank? + return dup.scan_limit(n).to_a.first(*args) if @where_conditions.empty? return super if @key_fields_detector.non_key_present? dup.record_limit(n).to_a.first(*args) @@ -230,12 +226,12 @@ def delete_all ranges = [] if @key_fields_detector.key_present? - Dynamoid.adapter.query(source.table_name, range_query).flat_map { |i| i }.collect do |hash| + Dynamoid.adapter.query(source.table_name, query_key_conditions, query_non_key_conditions, query_options).flat_map { |i| i }.collect do |hash| ids << hash[source.hash_key.to_sym] ranges << hash[source.range_key.to_sym] if source.range_key end else - Dynamoid.adapter.scan(source.table_name, scan_query, scan_opts).flat_map { |i| i }.collect do |hash| + Dynamoid.adapter.scan(source.table_name, scan_conditions, scan_options).flat_map { |i| i }.collect do |hash| ids << hash[source.hash_key.to_sym] ranges << hash[source.range_key.to_sym] if source.range_key end @@ -384,7 +380,7 @@ def with_index(index_name) raise Dynamoid::Errors::InvalidIndex, "Unknown index #{index_name}" unless @source.find_index_by_name(index_name) @forced_index_name = index_name - @key_fields_detector = KeyFieldsDetector.new(@query, @source, forced_index_name: index_name) + @key_fields_detector = KeyFieldsDetector.new(@where_conditions, @source, forced_index_name: index_name) self end @@ -487,6 +483,8 @@ def project(*fields) def pluck(*args) fields = args.map(&:to_sym) + # `project` has a side effect - it sets `@project` instance variable. + # So use a duplicate to not pollute original chain. scope = dup scope.project(*fields) @@ -535,7 +533,7 @@ def raw_pages if @key_fields_detector.key_present? raw_pages_via_query else - issue_scan_warning if Dynamoid::Config.warn_on_scan && query.present? + issue_scan_warning if Dynamoid::Config.warn_on_scan && !@where_conditions.empty? raw_pages_via_scan end end @@ -547,7 +545,7 @@ def raw_pages # @since 3.1.0 def raw_pages_via_query Enumerator.new do |y| - Dynamoid.adapter.query(source.table_name, range_query).each do |items, metadata| + Dynamoid.adapter.query(source.table_name, query_key_conditions, query_non_key_conditions, query_options).each do |items, metadata| options = metadata.slice(:last_evaluated_key) y.yield items, options @@ -562,7 +560,7 @@ def raw_pages_via_query # @since 3.1.0 def raw_pages_via_scan Enumerator.new do |y| - Dynamoid.adapter.scan(source.table_name, scan_query, scan_opts).each do |items, metadata| + Dynamoid.adapter.scan(source.table_name, scan_conditions, scan_options).each do |items, metadata| options = metadata.slice(:last_evaluated_key) y.yield items, options @@ -575,121 +573,88 @@ def issue_scan_warning Dynamoid.logger.warn "You can index this query by adding index declaration to #{source.to_s.underscore}.rb:" Dynamoid.logger.warn "* global_secondary_index hash_key: 'some-name', range_key: 'some-another-name'" Dynamoid.logger.warn "* local_secondary_index range_key: 'some-name'" - Dynamoid.logger.warn "Not indexed attributes: #{query.keys.sort.collect { |name| ":#{name}" }.join(', ')}" + Dynamoid.logger.warn "Not indexed attributes: #{@where_conditions.keys.sort.collect { |name| ":#{name}" }.join(', ')}" end def count_via_query - Dynamoid.adapter.query_count(source.table_name, range_query) + Dynamoid.adapter.query_count(source.table_name, query_key_conditions, query_non_key_conditions, query_options) end def count_via_scan - Dynamoid.adapter.scan_count(source.table_name, scan_query, scan_opts) - end - - def range_hash(key) - name, operation = key.to_s.split('.') - val = type_cast_condition_parameter(name, query[key]) - - case operation - when 'gt' - { range_greater_than: val } - when 'lt' - { range_less_than: val } - when 'gte' - { range_gte: val } - when 'lte' - { range_lte: val } - when 'between' - { range_between: val } - when 'begins_with' - { range_begins_with: val } - end + Dynamoid.adapter.scan_count(source.table_name, scan_conditions, scan_options) end - def field_hash(key) - name, operation = key.to_s.split('.') - val = type_cast_condition_parameter(name, query[key]) - - hash = case operation - when 'ne' - { ne: val } - when 'gt' - { gt: val } - when 'lt' - { lt: val } - when 'gte' - { gte: val } - when 'lte' - { lte: val } - when 'between' - { between: val } - when 'begins_with' - { begins_with: val } - when 'in' - { in: val } - when 'contains' - { contains: val } - when 'not_contains' - { not_contains: val } - # NULL/NOT_NULL operators don't have parameters - # So { null: true } means NULL check and { null: false } means NOT_NULL one - # The same logic is used for { not_null: BOOL } - when 'null' - val ? { null: nil } : { not_null: nil } - when 'not_null' - val ? { not_null: nil } : { null: nil } - end - - { name.to_sym => hash } - end - - def consistent_opts - { consistent_read: consistent_read } - end - - def range_query - opts = {} - query = self.query + 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' - # Honor STI and :type field if it presents - if @source.attributes.key?(@source.inheritance_field) && - @key_fields_detector.hash_key.to_sym != @source.inheritance_field.to_sym - query.update(sti_condition) + unless operator.in? ALLOWED_FIELD_OPERATORS + raise Dynamoid::Errors::Error, "Unsupported operator #{operator} in #{key}" end + condition = \ + case operator + # NULL/NOT_NULL operators don't have parameters + # So { null: true } means NULL check and { null: false } means NOT_NULL one + # The same logic is used for { not_null: BOOL } + when 'null' + value ? [:null, nil] : [:not_null, nil] + when 'not_null' + value ? [:not_null, nil] : [:null, nil] + else + [operator.to_sym, value] + end + + [name.to_sym, condition] + end + + def query_key_conditions + opts = {} + # Add hash key - opts[:hash_key] = @key_fields_detector.hash_key - opts[:hash_value] = type_cast_condition_parameter(@key_fields_detector.hash_key, query[@key_fields_detector.hash_key]) + # TODO: always have hash key in @where_conditions? + _, condition = field_condition(@key_fields_detector.hash_key, @where_conditions[@key_fields_detector.hash_key]) + opts[@key_fields_detector.hash_key] = [condition] # Add range key if @key_fields_detector.range_key - add_range_key_to_range_query(query, opts) - end + if @where_conditions[@key_fields_detector.range_key].present? + _, condition = field_condition(@key_fields_detector.range_key, @where_conditions[@key_fields_detector.range_key]) + opts[@key_fields_detector.range_key] = [condition] + end - (query.keys.map(&:to_sym) - [@key_fields_detector.hash_key.to_sym, @key_fields_detector.range_key.try(:to_sym)]) - .reject { |k, _| k.to_s =~ /^#{@key_fields_detector.range_key}\./ } - .each do |key| - if key.to_s.include?('.') - opts.update(field_hash(key)) - else - value = type_cast_condition_parameter(key, query[key]) - opts[key] = { eq: value } + @where_conditions.keys.select { |k| k.to_s =~ /^#{@key_fields_detector.range_key}\./ }.each do |key| + name, condition = field_condition(key, @where_conditions[key]) + opts[name] ||= [] + opts[name] << condition end end - opts.merge(query_opts).merge(consistent_opts) + opts end - def add_range_key_to_range_query(query, opts) - opts[:range_key] = @key_fields_detector.range_key - if query[@key_fields_detector.range_key].present? - value = type_cast_condition_parameter(@key_fields_detector.range_key, query[@key_fields_detector.range_key]) - opts.update(range_eq: value) + def query_non_key_conditions + opts = {} + + # Honor STI and :type field if it presents + if @source.attributes.key?(@source.inheritance_field) && + @key_fields_detector.hash_key.to_sym != @source.inheritance_field.to_sym + @where_conditions.update(sti_condition) end - query.keys.select { |k| k.to_s =~ /^#{@key_fields_detector.range_key}\./ }.each do |key| - opts.merge!(range_hash(key)) + # TODO: Separate key conditions and non-key conditions properly: + # only =, >, >=, <, <=, between and begins_with + # could be used for sort key in KeyConditionExpression + keys = (@where_conditions.keys.map(&:to_sym) - [@key_fields_detector.hash_key.to_sym, @key_fields_detector.range_key.try(:to_sym)]) + .reject { |k, _| k.to_s =~ /^#{@key_fields_detector.range_key}\./ } + keys.each do |key| + name, condition = field_condition(key, @where_conditions[key]) + opts[name] ||= [] + opts[name] << condition end + + opts end # TODO: casting should be operator aware @@ -737,7 +702,7 @@ def start_key key end - def query_opts + def query_options opts = {} # Don't specify select = ALL_ATTRIBUTES option explicitly because it's # already a default value of Select statement. Explicite Select value @@ -749,30 +714,26 @@ def query_opts opts[:exclusive_start_key] = start_key if @start opts[:scan_index_forward] = @scan_index_forward opts[:project] = @project + opts[:consistent_read] = true if @consistent_read opts end - def scan_query - query = self.query - + def scan_conditions # Honor STI and :type field if it presents if sti_condition - query.update(sti_condition) + @where_conditions.update(sti_condition) end {}.tap do |opts| - query.keys.map(&:to_sym).each do |key| - if key.to_s.include?('.') - opts.update(field_hash(key)) - else - value = type_cast_condition_parameter(key, query[key]) - opts[key] = { eq: value } - end + @where_conditions.keys.map(&:to_sym).each do |key| + name, condition = field_condition(key, @where_conditions[key]) + opts[name] ||= [] + opts[name] << condition end end end - def scan_opts + def scan_options opts = {} opts[:index_name] = @key_fields_detector.index_name if @key_fields_detector.index_name opts[:record_limit] = @record_limit if @record_limit @@ -784,6 +745,7 @@ def scan_opts opts end + # TODO: return Array, not String def sti_condition condition = {} type = @source.inheritance_field diff --git a/lib/dynamoid/criteria/ignored_conditions_detector.rb b/lib/dynamoid/criteria/ignored_conditions_detector.rb deleted file mode 100644 index 8810b2f2..00000000 --- a/lib/dynamoid/criteria/ignored_conditions_detector.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -module Dynamoid - module Criteria - # @private - class IgnoredConditionsDetector - def initialize(conditions) - @conditions = conditions - @ignored_keys = ignored_keys - end - - def found? - @ignored_keys.present? - end - - def warning_message - return unless found? - - 'Where conditions may contain only one condition for an attribute. ' \ - "Following conditions are ignored: #{ignored_conditions}" - end - - private - - def ignored_keys - @conditions.keys - .group_by(&method(:key_to_field)) - .select { |_, ary| ary.size > 1 } - .flat_map { |_, ary| ary[0..-2] } - end - - def key_to_field(key) - key.to_s.split('.')[0] - end - - def ignored_conditions - @conditions.slice(*@ignored_keys) - end - end - end -end diff --git a/lib/dynamoid/criteria/key_fields_detector.rb b/lib/dynamoid/criteria/key_fields_detector.rb index 89f44230..e86e71a1 100644 --- a/lib/dynamoid/criteria/key_fields_detector.rb +++ b/lib/dynamoid/criteria/key_fields_detector.rb @@ -5,10 +5,10 @@ module Criteria # @private class KeyFieldsDetector class Query - def initialize(query_hash) - @query_hash = query_hash - @fields_with_operator = query_hash.keys.map(&:to_s) - @fields = query_hash.keys.map(&:to_s).map { |s| s.split('.').first } + def initialize(where_conditions) + @where_conditions = where_conditions + @fields_with_operator = where_conditions.keys.map(&:to_s) + @fields = where_conditions.keys.map(&:to_s).map { |s| s.split('.').first } end def contain_only?(field_names) @@ -24,10 +24,9 @@ def contain_with_eq_operator?(field_name) end end - def initialize(query, source, forced_index_name: nil) - @query = query + def initialize(where_conditions, source, forced_index_name: nil) @source = source - @query = Query.new(query) + @query = Query.new(where_conditions) @forced_index_name = forced_index_name @result = find_keys_in_query end diff --git a/lib/dynamoid/criteria/overwritten_conditions_detector.rb b/lib/dynamoid/criteria/overwritten_conditions_detector.rb deleted file mode 100644 index 5b1acdd9..00000000 --- a/lib/dynamoid/criteria/overwritten_conditions_detector.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -module Dynamoid - module Criteria - # @private - class OverwrittenConditionsDetector - def initialize(conditions, conditions_new) - @conditions = conditions - @new_conditions = conditions_new - @overwritten_keys = overwritten_keys - end - - def found? - @overwritten_keys.present? - end - - def warning_message - return unless found? - - 'Where conditions may contain only one condition for an attribute. ' \ - "Following conditions are ignored: #{ignored_conditions}" - end - - private - - def overwritten_keys - new_fields = @new_conditions.keys.map(&method(:key_to_field)) - @conditions.keys.select { |key| key_to_field(key).in?(new_fields) } - end - - def key_to_field(key) - key.to_s.split('.')[0] - end - - def ignored_conditions - @conditions.slice(*@overwritten_keys.map(&:to_sym)) - end - end - end -end diff --git a/lib/dynamoid/criteria/where_conditions.rb b/lib/dynamoid/criteria/where_conditions.rb new file mode 100644 index 00000000..52baadd7 --- /dev/null +++ b/lib/dynamoid/criteria/where_conditions.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Dynamoid + module Criteria + # @private + class WhereConditions + def initialize + @conditions = [] + end + + def update(hash) + @conditions << hash.symbolize_keys + end + + def keys + @conditions.flat_map(&:keys) + end + + def empty? + @conditions.empty? + end + + def [](key) + hash = @conditions.find { |h| h.key?(key) } + hash[key] if hash + end + end + end +end diff --git a/lib/dynamoid/finders.rb b/lib/dynamoid/finders.rb index 8e8a563f..f047357a 100644 --- a/lib/dynamoid/finders.rb +++ b/lib/dynamoid/finders.rb @@ -6,17 +6,6 @@ module Dynamoid module Finders extend ActiveSupport::Concern - # @private - RANGE_MAP = { - 'gt' => :range_greater_than, - 'lt' => :range_less_than, - 'gte' => :range_gte, - 'lte' => :range_lte, - 'begins_with' => :range_begins_with, - 'between' => :range_between, - 'eq' => :range_eq - }.freeze - module ClassMethods # Find one or many objects, specified by one id or an array of ids. # @@ -253,7 +242,6 @@ def find_all_by_secondary_index(hash, options = {}) range = options[:range] || {} hash_key_field, hash_key_value = hash.first range_key_field, range_key_value = range.first - range_op_mapped = nil if range_key_field range_key_field = range_key_field.to_s @@ -261,27 +249,30 @@ def find_all_by_secondary_index(hash, options = {}) if range_key_field.include?('.') range_key_field, range_key_op = range_key_field.split('.', 2) end - range_op_mapped = RANGE_MAP.fetch(range_key_op) end # Find the index index = find_index(hash_key_field, range_key_field) raise Dynamoid::Errors::MissingIndex, "attempted to find #{[hash_key_field, range_key_field]}" if index.nil? - # query - opts = { - hash_key: hash_key_field.to_s, - hash_value: hash_key_value, - index_name: index.name - } + # Query + query_key_conditions = {} + query_key_conditions[hash_key_field.to_sym] = [[:eq, hash_key_value]] if range_key_field - opts[:range_key] = range_key_field - opts[range_op_mapped] = range_key_value - end - dynamo_options = opts.merge(options.reject { |key, _| key == :range }) - Dynamoid.adapter.query(table_name, dynamo_options).flat_map { |i| i }.map do |item| - from_database(item) + query_key_conditions[range_key_field.to_sym] = [[range_key_op.to_sym, range_key_value]] end + + query_non_key_conditions = options + .except(*Dynamoid::AdapterPlugin::AwsSdkV3::Query::OPTIONS_KEYS) + .except(:range) + .symbolize_keys + + query_options = options.slice(*Dynamoid::AdapterPlugin::AwsSdkV3::Query::OPTIONS_KEYS) + query_options[:index_name] = index.name + + Dynamoid.adapter.query(table_name, query_key_conditions, query_non_key_conditions, query_options) + .flat_map { |i| i } + .map { |item| from_database(item) } end # Find using exciting method_missing finders attributes. Uses criteria diff --git a/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb b/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb index 8592bf1d..2da796b8 100644 --- a/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb +++ b/spec/dynamoid/adapter_plugin/aws_sdk_v3_spec.rb @@ -37,16 +37,16 @@ @request_type = request_type end - def request_params - return { hash_value: '1' } if @request_type == :query - - {} + def query_key_conditions + { id: [[:eq, '1']] } end - def dynamo_request(table_name, scan_hash = {}, select_opts = {}) - return Dynamoid.adapter.query(table_name, scan_hash.merge(select_opts)).flat_map { |i| i } if @request_type == :query - - Dynamoid.adapter.scan(table_name, scan_hash, select_opts).flat_map { |i| i } + def dynamo_request(table_name, conditions = {}, options = {}) + if @request_type == :query + Dynamoid.adapter.query(table_name, query_key_conditions, conditions, options).flat_map { |i| i } + else + Dynamoid.adapter.scan(table_name, conditions, options).flat_map { |i| i } + end end context 'multiple name entities' do @@ -58,39 +58,39 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) end it 'returns correct records' do - expect(dynamo_request(test_table3, request_params, {}).count).to eq(8) + expect(dynamo_request(test_table3).count).to eq(8) end it 'returns correct record limit' do - expect(dynamo_request(test_table3, request_params, record_limit: 1).count).to eq(1) - expect(dynamo_request(test_table3, request_params, record_limit: 3).count).to eq(3) + expect(dynamo_request(test_table3, {}, { record_limit: 1 }).count).to eq(1) + expect(dynamo_request(test_table3, {}, { record_limit: 3 }).count).to eq(3) end it 'returns correct batch' do # Receives 8 times for each item and 1 more for empty page expect(Dynamoid.adapter.client).to receive(request_type).exactly(9).times.and_call_original - expect(dynamo_request(test_table3, request_params, batch_size: 1).count).to eq(8) + expect(dynamo_request(test_table3, {}, { batch_size: 1 }).count).to eq(8) end it 'returns correct batch and paginates in batches' do expect(Dynamoid.adapter.client).to receive(request_type).exactly(3).times.and_call_original - expect(dynamo_request(test_table3, request_params, batch_size: 3).count).to eq(8) + expect(dynamo_request(test_table3, {}, { batch_size: 3 }).count).to eq(8) end it 'returns correct record limit and batch' do - expect(dynamo_request(test_table3, request_params, record_limit: 1, batch_size: 1).count).to eq(1) + expect(dynamo_request(test_table3, {}, { record_limit: 1, batch_size: 1 }).count).to eq(1) end it 'returns correct record limit with filter' do expect( - dynamo_request(test_table3, request_params.merge(name: { eq: 'Josh' }), record_limit: 1).count + dynamo_request(test_table3, { name: [[:eq, 'Josh']] }, { record_limit: 1 }).count ).to eq(1) end it 'obeys correct scan limit with filter' do expect(Dynamoid.adapter.client).to receive(request_type).once.and_call_original expect( - dynamo_request(test_table3, request_params.merge(name: { eq: 'Josh' }), scan_limit: 2).count + dynamo_request(test_table3, { name: [[:eq, 'Josh']] }, { scan_limit: 2 }).count ).to eq(2) end @@ -99,9 +99,11 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) expect( dynamo_request( test_table3, - request_params.merge(name: { eq: 'Josh' }), - scan_limit: 2, - record_limit: 10 # Won't be able to return more than 2 due to scan limit + { name: [[:eq, 'Josh']] }, + { + scan_limit: 2, + record_limit: 10 # Won't be able to return more than 2 due to scan limit + } ).count ).to eq(2) end @@ -109,7 +111,7 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) it 'obeys correct scan limit with filter with some return' do expect(Dynamoid.adapter.client).to receive(request_type).once.and_call_original expect( - dynamo_request(test_table3, request_params.merge(name: { eq: 'Pascal' }), scan_limit: 5).count + dynamo_request(test_table3, { name: [[:eq, 'Pascal']] }, { scan_limit: 5 }).count ).to eq(1) end @@ -118,9 +120,11 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) expect( dynamo_request( test_table3, - request_params.merge(name: { eq: 'Josh' }), - scan_limit: 3, - batch_size: 2 # This would force batching of size 2 for potential of 4 results! + { name: [[:eq, 'Josh']] }, + { + scan_limit: 3, + batch_size: 2 # This would force batching of size 2 for potential of 4 results! + } ).count ).to eq(3) end @@ -133,10 +137,12 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) expect( dynamo_request( test_table3, - request_params.merge(name: { eq: 'Pascal' }), - batch_size: 1, - scan_limit: 5, - record_limit: 3 + { name: [[:eq, 'Pascal']] }, + { + batch_size: 1, + scan_limit: 5, + record_limit: 3 + } ).count ).to eq(1) end @@ -149,10 +155,12 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) expect( dynamo_request( test_table3, - request_params.merge(name: { eq: 'Pascal' }), - batch_size: 1, - scan_limit: 10, - record_limit: 2 + { name: [[:eq, 'Pascal']] }, + { + batch_size: 1, + scan_limit: 10, + record_limit: 2 + } ).count ).to eq(2) end @@ -178,7 +186,7 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) end it 'returns correct for limits and scan limit' do - expect(dynamo_request(test_table3, request_params, scan_limit: 100).count).to eq(100) + expect(dynamo_request(test_table3, {}, { scan_limit: 100 }).count).to eq(100) end it 'returns correct for scan limit with filtering' do @@ -188,20 +196,20 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) pages = request_type == :query ? 1 : 2 expect(Dynamoid.adapter.client).to receive(request_type).exactly(pages).times.and_call_original expect( - dynamo_request(test_table3, request_params.merge(age: { gte: 90.0 }), scan_limit: 100).count + dynamo_request(test_table3, { age: [[:gte, 90.0]] }, { scan_limit: 100 }).count ).to eq(10) end it 'returns correct for record limit' do expect(Dynamoid.adapter.client).to receive(request_type).twice.and_call_original expect( - dynamo_request(test_table3, request_params.merge(age: { gte: 5.0 }), record_limit: 100).count + dynamo_request(test_table3, { age: [[:gte, 5.0]] }, { record_limit: 100 }).count ).to eq(100) end it 'returns correct record limit with filtering' do expect( - dynamo_request(test_table3, request_params.merge(age: { gte: 133.0 }), record_limit: 100).count + dynamo_request(test_table3, { age: [[:gte, 133.0]] }, { record_limit: 100 }).count ).to eq(67) end @@ -210,7 +218,7 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) # which is limitation of DynamoDB and therefore batch limit is # restricted by this limitation as well! expect(Dynamoid.adapter.client).to receive(request_type).exactly(4).times.and_call_original - expect(dynamo_request(test_table3, request_params, batch_size: 100).count).to eq(200) + expect(dynamo_request(test_table3, {}, { batch_size: 100 }).count).to eq(200) end it 'returns correct with batching and record limit beyond data size limit' do @@ -218,7 +226,7 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) # requests for as many as we have left for our record limit. expect(Dynamoid.adapter.client).to receive(request_type).twice.and_call_original expect( - dynamo_request(test_table3, request_params, record_limit: 83, batch_size: 100).count + dynamo_request(test_table3, {}, { record_limit: 83, batch_size: 100 }).count ).to eq(83) end @@ -228,9 +236,11 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) expect( dynamo_request( test_table3, - request_params.merge(age: { gte: 5.0 }), - record_limit: 100, - batch_size: 10 + { age: [[:gte, 5.0]] }, + { + record_limit: 100, + batch_size: 10 + } ).count ).to eq(100) end @@ -253,10 +263,12 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) expect( dynamo_request( test_table3, - request_params.merge(name: { eq: 'Josh' }), - batch_size: 4, - scan_limit: 5, # Scan limit would adjust requested limit to 1 - record_limit: 6 # Record limit would adjust requested limit to 2 + { name: [[:eq, 'Josh']] }, + { + batch_size: 4, + scan_limit: 5, # Scan limit would adjust requested limit to 1 + record_limit: 6 # Record limit would adjust requested limit to 2 + } ).count ).to eq(4) end @@ -272,37 +284,37 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) end it 'performs query on a table with a range and selects items in a range' do - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_between: [0.0, 3.0]).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:between, [0.0, 3.0]]] }).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] end it 'performs query on a table with a range and selects items in a range with :select option' do - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_between: [0.0, 3.0], select: 'ALL_ATTRIBUTES').to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:between, [0.0, 3.0]]] }, {}, { select: 'ALL_ATTRIBUTES' }).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] end it 'performs query on a table with a range and selects items greater than' do - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_greater_than: 1.0).to_a).to eq [[[{ id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:gt, 1.0]] }).to_a).to eq [[[{ id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] end it 'performs query on a table with a range and selects items less than' do - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_less_than: 2.0).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:lt, 2.0]] }).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }], { last_evaluated_key: nil }]] end it 'performs query on a table with a range and selects items gte' do - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_gte: 1.0).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:gte, 1.0]] }).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] end it 'performs query on a table with a range and selects items lte' do - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_lte: 3.0).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:lte, 3.0]] }).to_a).to eq [[[{ id: '1', range: BigDecimal(1) }, { id: '1', range: BigDecimal(3) }], { last_evaluated_key: nil }]] end it 'performs query on a table and returns items based on returns correct limit' do - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_greater_than: 0.0, record_limit: 1).flat_map { |i| i }.count).to eq(1) + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:gt, 0.0]] }, {}, { record_limit: 1 }).flat_map { |i| i }.count).to eq(1) end it 'performs query on a table with a range and selects all items' do 200.times { |i| Dynamoid.adapter.put_item(test_table3, id: '1', range: i.to_f, data: 'A' * 1024 * 16) } # 64 of these items will exceed the 1MB result limit thus query won't return all results on first loop - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', range_gte: 0.0).flat_map { |i| i }.count).to eq(200) + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']], range: [[:gte, 0.0]] }).flat_map { |i| i }.count).to eq(200) end end @@ -320,7 +332,7 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) end it 'performs query on a table with a range and selects items less than that is in the correct order, scan_index_forward true' do - query = Dynamoid.adapter.query(test_table4, hash_value: '1', range_greater_than: 0, scan_index_forward: true).flat_map { |i| i }.to_a + query = Dynamoid.adapter.query(test_table4, { id: [[:eq, '1']], range: [[:gt, 0]] }, {}, { scan_index_forward: true }).flat_map { |i| i }.to_a expect(query[0]).to eq(id: '1', order: 1, range: BigDecimal(1)) expect(query[1]).to eq(id: '1', order: 2, range: BigDecimal(2)) expect(query[2]).to eq(id: '1', order: 3, range: BigDecimal(3)) @@ -330,7 +342,7 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) end it 'performs query on a table with a range and selects items less than that is in the correct order, scan_index_forward false' do - query = Dynamoid.adapter.query(test_table4, hash_value: '1', range_greater_than: 0, scan_index_forward: false).flat_map { |i| i }.to_a + query = Dynamoid.adapter.query(test_table4, { id: [[:eq, '1']], range: [[:gt, 0]] }, {}, { scan_index_forward: false }).flat_map { |i| i }.to_a expect(query[5]).to eq(id: '1', order: 1, range: BigDecimal(1)) expect(query[4]).to eq(id: '1', order: 2, range: BigDecimal(2)) expect(query[3]).to eq(id: '1', order: 3, range: BigDecimal(3)) @@ -974,14 +986,14 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) it 'performs query on a table and returns items' do Dynamoid.adapter.put_item(test_table1, id: '1', name: 'Josh') - expect(Dynamoid.adapter.query(test_table1, hash_value: '1').first).to eq([[id: '1', name: 'Josh'], { last_evaluated_key: nil }]) + expect(Dynamoid.adapter.query(test_table1, { id: [[:eq, '1']] }).first).to eq([[id: '1', name: 'Josh'], { last_evaluated_key: nil }]) end it 'performs query on a table and returns items if there are multiple items' do Dynamoid.adapter.put_item(test_table1, id: '1', name: 'Josh') Dynamoid.adapter.put_item(test_table1, id: '2', name: 'Justin') - expect(Dynamoid.adapter.query(test_table1, hash_value: '1').first).to eq([[id: '1', name: 'Josh'], { last_evaluated_key: nil }]) + expect(Dynamoid.adapter.query(test_table1, { id: [[:eq, '1']] }).first).to eq([[id: '1', name: 'Josh'], { last_evaluated_key: nil }]) end context 'backoff is specified' do @@ -1003,7 +1015,7 @@ def dynamo_request(table_name, scan_hash = {}, select_opts = {}) Dynamoid.adapter.put_item(test_table3, id: '1', range: 1) Dynamoid.adapter.put_item(test_table3, id: '1', range: 2) - expect(Dynamoid.adapter.query(test_table3, hash_value: '1', batch_size: 1).flat_map { |i| i }.count).to eq 2 + expect(Dynamoid.adapter.query(test_table3, { id: [[:eq, '1']] }, {}, { batch_size: 1 }).flat_map { |i| i }.count).to eq 2 expect(@counter).to eq 2 end end diff --git a/spec/dynamoid/criteria/chain_spec.rb b/spec/dynamoid/criteria/chain_spec.rb index ac1c1ac1..73fd154d 100644 --- a/spec/dynamoid/criteria/chain_spec.rb +++ b/spec/dynamoid/criteria/chain_spec.rb @@ -192,10 +192,50 @@ def request_params expect(model.where(name: 'Bob', 'age.between': [19, 31]).all).to contain_exactly(customer2, customer3) end + + it 'supports multiple conditions for the same attribute' do + skip 'Aws::DynamoDB::Errors::ValidationException: KeyConditionExpressions must only contain one condition per key' + + customer1 = model.create(name: 'Bob', age: 10) + customer2 = model.create(name: 'Bob', age: 20) + customer3 = model.create(name: 'Bob', age: 30) + customer4 = model.create(name: 'Bob', age: 40) + + expect(model.where(name: 'Bob', 'age.gt': 19).where('age.lt': 31).all).to contain_exactly(customer2, customer3) + end + + it 'supports multiple conditions for the same attribute with the same operator' do + skip 'Aws::DynamoDB::Errors::ValidationException: KeyConditionExpressions must only contain one condition per key' + + customer1 = model.create(name: 'Bob', age: 10) + customer2 = model.create(name: 'Bob', age: 20) + customer3 = model.create(name: 'Bob', age: 30) + customer4 = model.create(name: 'Bob', age: 40) + + expect(model.where(name: 'Bob', 'age.gt': 31).where('age.gt': 19).all).to contain_exactly(customer4) + end + + it 'allows conditions with attribute names conflicting with DynamoDB reserved words' do + model = new_class do + range :size # SIZE is reserved word + end + + model.create_table + put_attributes(model.table_name, id: '1', size: 'c') + + documents = model.where(id: '1', size: '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 + end.to raise_error(Dynamoid::Errors::Error, 'Unsupported operator foo in age.foo') + end end - # http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/LegacyConditionalParameters.QueryFilter.html?shortFooter=true - describe 'Query with not-keys conditions' do + # http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/LegacyConditionalParameters.QueryFilter.html + describe 'Query with non-keys conditions' do let(:model) do new_class do table name: :customer, key: :name @@ -369,9 +409,48 @@ def request_params documents = model.where('age.not_null': false).to_a expect(documents.map(&:last_name)).to contain_exactly('cc') end + + it 'supports multiple conditions for the same attribute' do + customer1 = model.create(name: 'a', last_name: 'a', age: 10) + customer2 = model.create(name: 'a', last_name: 'b', age: 20) + customer3 = model.create(name: 'a', last_name: 'c', age: 30) + customer4 = model.create(name: 'a', last_name: 'd', age: 40) + + expect(model.where(name: 'a', 'age.gt': 19, 'age.lt': 31).all).to contain_exactly(customer2, customer3) + end + + it 'supports multiple conditions for the same attribute with the same operator' do + customer1 = model.create(name: 'a', last_name: 'a', age: 10) + customer2 = model.create(name: 'a', last_name: 'b', age: 20) + customer3 = model.create(name: 'a', last_name: 'c', age: 30) + customer4 = model.create(name: 'a', last_name: 'd', age: 40) + + expect(model.where(name: 'a', 'age.gt': 31).where('age.gt': 19).all).to contain_exactly(customer4) + end + + it 'allows conditions with attribute names conflicting with DynamoDB reserved words' do + model = new_class do + # SCAN, SET and SIZE are reserved words + field :scan + field :set + field :size + end + + model.create_table + put_attributes(model.table_name, id: '1', scan: 'a', set: 'b', size: 'c') + + documents = model.where(id: '1', scan: 'a', set: 'b', size: '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 + end.to raise_error(Dynamoid::Errors::Error, 'Unsupported operator foo in age.foo') + end end - # http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/LegacyConditionalParameters.ScanFilter.html?shortFooter=true + # http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/LegacyConditionalParameters.ScanFilter.html describe 'Scan conditions ' do let(:model) do new_class do @@ -546,6 +625,45 @@ def request_params documents = model.where('age.not_null': false).to_a expect(documents.map(&:id)).to contain_exactly('3') end + + it 'supports multiple conditions for the same attribute' do + customer1 = model.create(age: 10) + customer2 = model.create(age: 20) + customer3 = model.create(age: 30) + customer4 = model.create(age: 40) + + expect(model.where('age.gt': 19, 'age.lt': 31).all).to contain_exactly(customer2, customer3) + end + + it 'supports multiple conditions for the same attribute with the same operator' do + customer1 = model.create(age: 10) + customer2 = model.create(age: 20) + customer3 = model.create(age: 30) + customer4 = model.create(age: 40) + + expect(model.where('age.gt': 31).where('age.gt': 19).all.to_a).to eq([customer4]) + end + + it 'allows conditions with attribute names conflicting with DynamoDB reserved words' do + model = new_class do + # SCAN, SET and SIZE are reserved words + field :scan + field :set + field :size + end + + model.create_table + put_attributes(model.table_name, id: '1', scan: 'a', set: 'b', size: 'c') + + documents = model.where(scan: 'a', set: 'b', size: '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 + end.to raise_error(Dynamoid::Errors::Error, 'Unsupported operator foo in age.foo') + end end describe 'Lazy loading' do @@ -1037,61 +1155,6 @@ def request_params end end - context 'passed several conditions for the same attribute' do - let(:model) do - new_class do - field :age, :integer - field :name - end - end - - before do - model.create_table - end - - it 'ignores conditions except the last one' do - (1..5).each { |i| model.create(age: i) } - - models = model.where('age.gt': 2, 'age.lt': 4).to_a - expect(models.map(&:age)).to contain_exactly(1, 2, 3) - - models = model.where('age.lt': 4, 'age.gt': 2).to_a - expect(models.map(&:age)).to contain_exactly(3, 4, 5) - end - - describe 'warning' do - it 'writes warning message' do - expect(Dynamoid.logger).to receive(:warn) - .with( - 'Where conditions may contain only one condition for an attribute. ' \ - 'Following conditions are ignored: {:"age.gt"=>2}' - ) - - model.where('age.gt': 2, 'age.lt': 4) - end - - it 'writes warning message when conditions are build with several calls of `where`' do - expect(Dynamoid.logger).to receive(:warn) - .with( - 'Where conditions may contain only one condition for an attribute. ' \ - 'Following conditions are ignored: {:"age.gt"=>2}' - ) - - model.where('age.gt': 2).where('age.lt': 4) - end - - it 'writes warning message when there are ignored conditions for several attributes' do - expect(Dynamoid.logger).to receive(:warn) - .with( - 'Where conditions may contain only one condition for an attribute. ' \ - 'Following conditions are ignored: {:age=>2, :name=>"Alex"}' - ) - - model.where(age: 2, name: 'Alex').where('age.gt': 3, name: 'David') - end - end - end - context 'nil check' do let(:model) do new_class do @@ -1110,7 +1173,6 @@ def request_params end it 'supports "in [nil]" check' do - pending 'because of temporary bug with nil type casting' expect(model.where('name.in': [nil]).to_a).to eq [@johndoe] end @@ -1135,7 +1197,6 @@ def request_params end it 'does not supports "in [nil]" check' do - pending 'because of temporary bug with nil type casting' expect(model.where('name.in': [nil]).to_a).to eq [] end @@ -1810,6 +1871,35 @@ def request_params obj_loaded, = chain.where(id: obj.id).project(:age).to_a expect(obj_loaded.attributes).to eq(age: 21) end + + context 'when attribute name is a DynamoDB reserved word' do + let(:model) do + new_class do + field :name + field :bucket, :integer # BUCKET is a reserved word + end + end + + it 'works with Scan' do + model.create(name: 'Alex', bucket: 2) + + chain = described_class.new(model) + expect(chain).to receive(:raw_pages_via_scan).and_call_original + + obj, = chain.project(:bucket).to_a + expect(obj.attributes).to eq(bucket: 2) + end + + it 'works with Query' do + object = model.create(name: 'Alex', bucket: 2) + + chain = described_class.new(model) + expect(chain).to receive(:raw_pages_via_query).and_call_original + + obj, = chain.where(id: object.id).project(:bucket).to_a + expect(obj.attributes).to eq(bucket: 2) + end + end end describe '#pluck' do @@ -1892,6 +1982,28 @@ def request_params expect(object.tag_id).to eq '719' end end + + context 'when attribute name is a DynamoDB reserved word' do + let(:model) do + new_class do + field :name + field :bucket, :integer # BUCKET is a reserved word + end + end + + it 'works with Scan' do + model.create(name: 'Alice', bucket: 1001) + model.create(name: 'Bob', bucket: 1002) + + expect(model.pluck(:bucket)).to contain_exactly(1001, 1002) + end + + it 'works with Query' do + object = model.create(name: 'Alice', bucket: 1001) + + expect(model.where(id: object.id).pluck(:bucket)).to eq([1001]) + end + end end describe 'User' do