From 739e2e48c399ed2d95aab868c1d201ea82c92dcd Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 7 Apr 2020 11:09:31 -0400 Subject: [PATCH 1/2] MONGOID-4862 Make any_of behave like or but not disjunct the receiver --- docs/tutorials/mongoid-queries.txt | 57 +- lib/mongoid/criteria/queryable/selectable.rb | 97 ++- .../queryable/selectable_logical_spec.rb | 583 ++++++++++++++---- 3 files changed, 592 insertions(+), 145 deletions(-) diff --git a/docs/tutorials/mongoid-queries.txt b/docs/tutorials/mongoid-queries.txt index 17956fce93..cd2fc9f8ca 100644 --- a/docs/tutorials/mongoid-queries.txt +++ b/docs/tutorials/mongoid-queries.txt @@ -365,9 +365,11 @@ The following calls all produce the same query conditions: Operator Combinations ````````````````````` -As of Mongoid 7.1, logical operators have been changed to have the -the same semantics as `those of ActiveRecord +As of Mongoid 7.1, logical operators (``and``, ``or``, ``nor`` and ``not``) +have been changed to have the the same semantics as `those of ActiveRecord `_. +To obtain the semantics of ``or`` as it behaved in Mongoid 7.0 and earlier, +use ``any_of`` which is described below. When conditions are specified on the same field multiple times, all conditions are added to the criteria: @@ -380,11 +382,12 @@ conditions are added to the criteria: Band.where(name: 1).or(name: 2).selector # => {"$or"=>[{"name"=>"1"}, {"name"=>"2"}]} -``nor`` and ``not`` behave similarly, with ``not`` producing different query -shapes as described later. +``any_of``, ``nor`` and ``not`` behave similarly, with ``not`` producing +different query shapes as described below. -When a logical operator is used, it operates on the criteria built up to -that point and its argument. ``where`` has the same meaning as ``and``: +When ``and``, ``or`` and ``nor`` logical operators are used, they +operates on the criteria built up to that point and its argument. +``where`` has the same meaning as ``and``: .. code-block:: ruby @@ -434,21 +437,49 @@ the same field, depending on which form of ``and`` was used. ``or``/``nor`` Behavior ``````````````````````` -``or`` and ``nor`` always combine conditions with ``$or`` and ``$nor`` -MongoDB operators, respectively. If the only condition on the receiving -criteria is another ``or``/``nor``, new conditions are added to the -existing list, otherwise a new top-level ``$or``/``$nor`` operator is created. +``or`` and ``nor`` produce ``$or`` and ``$nor`` MongoDB operators, respectively, +using the receiver and all of the arguments as operands. For example: .. code-block:: ruby - Band.where(name: /Best/).or(name: 'Astral Projection'). - or(Band.where(label: /Records/)).selector - # => {"$or"=>[{"name"=>/Best/}, {"name"=>"Astral Projection"}, {"label"=>/Records/}]} + Band.where(name: /Best/).or(name: 'Astral Projection') + # => {"$or"=>[{"name"=>/Best/}, {"name"=>"Astral Projection"}]} Band.where(name: /Best/).and(name: 'Astral Projection'). or(Band.where(label: /Records/)).and(label: 'Trust').selector # => {"$or"=>[{"name"=>/Best/, "$and"=>[{"name"=>"Astral Projection"}]}, {"label"=>/Records/}], "label"=>"Trust"} +If the only condition on the receiver is another ``or``/``nor``, the new +conditions are added to the existing list: + +.. code-block:: ruby + + Band.where(name: /Best/).or(name: 'Astral Projection'). + or(Band.where(label: /Records/)).selector + # => {"$or"=>[{"name"=>/Best/}, {"name"=>"Astral Projection"}, {"label"=>/Records/}]} + +Use ``any_of`` to add a disjunction to a Criteria object while maintaining +all of the conditions built up so far as they are. + + +``any_of`` Behavior +``````````````````` + +``any_of`` adds a disjunction built from its arguments to the existing +conditions in the criteria. For example: + +.. code-block:: ruby + + Band.where(label: /Trust/).any_of({name: 'Astral Projection'}, {name: /Best/}) + # => {"label"=>/Trust/, "$or"=>[{"name"=>"Astral Projection"}, {"name"=>/Best/}]} + +The conditions are hoisted to the top level if possible: + +.. code-block:: ruby + + Band.where(label: /Trust/).any_of({name: 'Astral Projection'}) + # => {"label"=>/Trust/, "name"=>"Astral Projection"} + ``not`` Behavior ```````````````` diff --git a/lib/mongoid/criteria/queryable/selectable.rb b/lib/mongoid/criteria/queryable/selectable.rb index 837d932b87..6ad47a3a0e 100644 --- a/lib/mongoid/criteria/queryable/selectable.rb +++ b/lib/mongoid/criteria/queryable/selectable.rb @@ -586,13 +586,34 @@ def not(*criteria) end key :not, :override, "$not" - # Adds $or selection to the selectable. + # Creates a disjunction using $or from the existing criteria in the + # receiver and the provided arguments. # - # @example Add the $or selection. + # This behavior (receiver becoming one of the disjunction operands) + # matches ActiveRecord's +or+ behavior. + # + # Use +any_of+ to add a disjunction of the arguments as an additional + # constraint to the criteria already existing in the receiver. + # + # Each argument can be a Hash, a Criteria object, an array of + # Hash or Criteria objects, or a nested array. Nested arrays will be + # flattened and can be of any depth. Passing arrays is deprecated. + # + # @example Add the $or selection where both fields must have the specified values. # selectable.or(field: 1, field: 2) # - # @param [ Array ] criteria Multiple key/value pair - # matches or Criteria objects. + # @example Add the $or selection where either value match is sufficient. + # selectable.or({field: 1}, {field: 2}) + # + # @example Same as previous example but using the deprecated array wrap. + # selectable.or([{field: 1}, {field: 2}]) + # + # @example Same as previous example, also deprecated. + # selectable.or([{field: 1}], [{field: 2}]) + # + # @param [ Hash | Criteria | Array, ... ] criteria + # Multiple key/value pair matches or Criteria objects, or arrays + # thereof. Passing arrays is deprecated. # # @return [ Selectable ] The new selectable. # @@ -600,7 +621,73 @@ def not(*criteria) def or(*criteria) _mongoid_add_top_level_operation('$or', criteria) end - alias :any_of :or + + # Adds a disjunction of the arguments as an additional constraint + # to the criteria already existing in the receiver. + # + # Use +or+ to make the receiver one of the disjunction operands. + # + # Each argument can be a Hash, a Criteria object, an array of + # Hash or Criteria objects, or a nested array. Nested arrays will be + # flattened and can be of any depth. Passing arrays is deprecated. + # + # @example Add the $or selection where both fields must have the specified values. + # selectable.any_of(field: 1, field: 2) + # + # @example Add the $or selection where either value match is sufficient. + # selectable.any_of({field: 1}, {field: 2}) + # + # @example Same as previous example but using the deprecated array wrap. + # selectable.any_of([{field: 1}, {field: 2}]) + # + # @example Same as previous example, also deprecated. + # selectable.any_of([{field: 1}], [{field: 2}]) + # + # @param [ Hash | Criteria | Array, ... ] criteria + # Multiple key/value pair matches or Criteria objects, or arrays + # thereof. Passing arrays is deprecated. + # + # @return [ Selectable ] The new selectable. + # + # @since 1.0.0 + def any_of(*criteria) + criteria = _mongoid_flatten_arrays(criteria) + case criteria.length + when 0 + clone + when 1 + # When we have a single criteria, any_of behaves like and. + # Note: criteria can be a Query object, which #where method does + # not support. + self.and(*criteria) + else + # When we have multiple criteria, combine them all with $or + # and add the result to self. + exprs = criteria.map do |criterion| + if criterion.is_a?(Selectable) + _mongoid_normalize_expr(criterion.selector) + else + Hash[criterion.map do |k, v| + if k.is_a?(Symbol) + [k.to_s, v] + else + [k, v] + end + end] + end + end + # Should be able to do: + #where('$or' => exprs) + # But since that is broken do instead: + clone.tap do |query| + if query.selector['$or'] + query.selector.store('$or', query.selector['$or'] + exprs) + else + query.selector.store('$or', exprs) + end + end + end + end # Add a $size selection for array fields. # diff --git a/spec/mongoid/criteria/queryable/selectable_logical_spec.rb b/spec/mongoid/criteria/queryable/selectable_logical_spec.rb index f7554dde10..80ce8473d6 100644 --- a/spec/mongoid/criteria/queryable/selectable_logical_spec.rb +++ b/spec/mongoid/criteria/queryable/selectable_logical_spec.rb @@ -16,7 +16,161 @@ end end - shared_examples_for 'a non-combining logical operation' do + # Hoisting means the operator can be elided, for example + # Foo.and(a: 1) produces simply {'a' => 1}. + shared_examples_for 'a hoisting logical operation' do + + let(:query) do + Mongoid::Query.new + end + + context "when provided a single criterion" do + + shared_examples_for 'adds the conditions to top level' do + + it "adds the conditions to top level" do + expect(selection.selector).to eq( + "field" => [ 1, 2 ] + ) + end + + it_behaves_like 'returns a cloned query' + end + + let(:selection) do + query.send(tested_method, field: [ 1, 2 ]) + end + + it_behaves_like 'adds the conditions to top level' + + context 'when the criterion is wrapped in an array' do + let(:selection) do + query.send(tested_method, [{field: [ 1, 2 ] }]) + end + + it_behaves_like 'adds the conditions to top level' + end + + context 'when the criterion is wrapped in a deep array with nil elements' do + let(:selection) do + query.send(tested_method, [[[{field: [ 1, 2 ] }]], [nil]]) + end + + it_behaves_like 'adds the conditions to top level' + end + end + + context 'when argument is a Criteria' do + let(:base) do + query.where(hello: 'world') + end + + let(:other) do + query.where(foo: 'bar') + end + + let(:result) { base.send(tested_method, other) } + + it 'combines' do + expect(result.selector).to eq( + 'hello' => 'world', + 'foo' => 'bar', + ) + end + end + + context "when provided a single criterion that is handled via Key" do + + shared_examples_for 'adds the conditions to top level' do + + it "adds the conditions to top level" do + expect(selection.selector).to eq({ + "field" => {'$gt' => 3 }, + }) + end + + it_behaves_like 'returns a cloned query' + end + + let(:selection) do + query.send(tested_method, :field.gt => 3) + end + + it_behaves_like 'adds the conditions to top level' + + context 'when the criterion is wrapped in an array' do + let(:selection) do + query.send(tested_method, [{ :field.gt => 3 }]) + end + + it_behaves_like 'adds the conditions to top level' + end + + context 'when the criterion is wrapped in a deep array with nil elements' do + let(:selection) do + query.send(tested_method, [[[{ :field.gt => 3 }]], [nil]]) + end + + it_behaves_like 'adds the conditions to top level' + end + end + + context "when provided a nested criterion" do + + let(:selection) do + query.send(tested_method, :test.elem_match => { :field.in => [ 1, 2 ] }) + end + + it "builds the correct selector" do + expect(selection.selector).to eq({ + "test" => { "$elemMatch" => { "field" => { "$in" => [ 1, 2 ] }}} + }) + end + + it_behaves_like 'returns a cloned query' + end + + context "when chaining the criteria" do + + context "when the criteria are for different fields" do + + let(:selection) do + query.and(first: [ 1, 2 ]).send(tested_method, second: [ 3, 4 ]) + end + + it "adds the conditions to top level" do + expect(selection.selector).to eq({ + "first" => [ 1, 2 ], + "second" => [ 3, 4 ], + }) + end + + it_behaves_like 'returns a cloned query' + end + + context "when the criteria are on the same field" do + + let(:selection) do + query.and(first: [ 1, 2 ]).send(tested_method, first: [ 3, 4 ]) + end + + it "combines via $and operator" do + expect(selection.selector).to eq({ + "first" => [ 1, 2 ], + "$and" => [ + { "first" => [ 3, 4 ] } + ] + }) + end + + it_behaves_like 'returns a cloned query' + end + end + end + + # Non-hoisting means the operator is always present, for example + # Foo.or(a: 1) produces {'$or' => [{'a' => 1}]}. + shared_examples_for 'a non-hoisting logical operation' do context 'when there is a single predicate' do let(:query) do @@ -88,6 +242,11 @@ describe "#and" do + let(:tested_method) { :and } + let(:expected_operator) { '$and' } + + it_behaves_like 'a hoisting logical operation' + context "when provided no criterion" do let(:selection) do @@ -122,93 +281,6 @@ it_behaves_like 'returns a cloned query' end - context "when provided a single criterion" do - - shared_examples_for 'adds the conditions to top level' do - - it "adds the conditions to top level" do - expect(selection.selector).to eq({ - "field" => [ 1, 2 ] - }) - end - - it_behaves_like 'returns a cloned query' - end - - let(:selection) do - query.and(field: [ 1, 2 ]) - end - - it_behaves_like 'adds the conditions to top level' - - context 'when the criterion is wrapped in an array' do - let(:selection) do - query.and([{field: [ 1, 2 ] }]) - end - - it_behaves_like 'adds the conditions to top level' - end - - context 'when the criterion is wrapped in a deep array with nil elements' do - let(:selection) do - query.and([[[{field: [ 1, 2 ] }]], [nil]]) - end - - it_behaves_like 'adds the conditions to top level' - end - end - - context "when provided a single criterion that is handled via Key" do - - shared_examples_for 'adds the conditions to top level' do - - it "adds the conditions to top level" do - expect(selection.selector).to eq({ - "field" => {'$gt' => 3 }, - }) - end - - it_behaves_like 'returns a cloned query' - end - - let(:selection) do - query.and(:field.gt => 3) - end - - it_behaves_like 'adds the conditions to top level' - - context 'when the criterion is wrapped in an array' do - let(:selection) do - query.and([{ :field.gt => 3 }]) - end - - it_behaves_like 'adds the conditions to top level' - end - - context 'when the criterion is wrapped in a deep array with nil elements' do - let(:selection) do - query.and([[[{ :field.gt => 3 }]], [nil]]) - end - - it_behaves_like 'adds the conditions to top level' - end - end - - context "when provided a nested criterion" do - - let(:selection) do - query.and(:test.elem_match => { :field.in => [ 1, 2 ] }) - end - - it "builds the correct selector" do - expect(selection.selector).to eq({ - "test" => { "$elemMatch" => { "field" => { "$in" => [ 1, 2 ] }}} - }) - end - - it_behaves_like 'returns a cloned query' - end - context "when provided multiple criteria" do context "when the criteria is already included" do @@ -264,43 +336,6 @@ end end - context "when chaining the criteria" do - - context "when the criteria are for different fields" do - - let(:selection) do - query.and(first: [ 1, 2 ]).and(second: [ 3, 4 ]) - end - - it "adds the conditions to top level" do - expect(selection.selector).to eq({ - "first" => [ 1, 2 ], - "second" => [ 3, 4 ], - }) - end - - it_behaves_like 'returns a cloned query' - end - - context "when the criteria are on the same field" do - - let(:selection) do - query.and(first: [ 1, 2 ]).and(first: [ 3, 4 ]) - end - - it "combines via $and operator" do - expect(selection.selector).to eq({ - "first" => [ 1, 2 ], - "$and" => [ - { "first" => [ 3, 4 ] } - ] - }) - end - - it_behaves_like 'returns a cloned query' - end - end - context 'when argument is a Criteria' do let(:query) do Mongoid::Query.new.where(hello: 'world') @@ -431,7 +466,7 @@ let(:tested_method) { :or } let(:expected_operator) { '$or' } - it_behaves_like 'a non-combining logical operation' + it_behaves_like 'a non-hoisting logical operation' context "when provided no arguments" do @@ -679,7 +714,7 @@ let(:tested_method) { :nor } let(:expected_operator) { '$nor' } - it_behaves_like 'a non-combining logical operation' + it_behaves_like 'a non-hoisting logical operation' context "when provided no criterion" do @@ -809,6 +844,300 @@ end end + describe "#any_of" do + + let(:tested_method) { :any_of } + let(:expected_operator) { '$or' } + + it_behaves_like 'a hoisting logical operation' + + # When multiple arguments are given to any_of, it behaves differently + # from and. + context 'when argument is a mix of Criteria and hashes' do + let(:query) do + Mongoid::Query.new.where(hello: 'world') + end + + let(:other1) do + Mongoid::Query.new.where(foo: 'bar') + end + + let(:other2) do + {bar: 42} + end + + let(:other3) do + Mongoid::Query.new.where(a: 2) + end + + let(:result) { query.send(tested_method, other1, other2, other3) } + + it 'combines' do + expect(result.selector).to eq( + 'hello' => 'world', + expected_operator => [ + {'foo' => 'bar'}, + {'bar' => 42}, + {'a' => 2}, + ], + ) + end + end + + context "when provided no arguments" do + + let(:selection) do + query.any_of + end + + it_behaves_like 'returns a cloned query' + + it "does not add any criteria" do + expect(selection.selector).to eq({}) + end + + it "returns the query" do + expect(selection).to eq(query) + end + end + + context "when provided nil" do + + let(:selection) do + query.any_of(nil) + end + + it_behaves_like 'returns a cloned query' + + it "does not add any criteria" do + expect(selection.selector).to eq({}) + end + + it "returns the query" do + expect(selection).to eq(query) + end + end + + context "when provided a single criterion" do + + let(:selection) do + query.any_of(field: [ 1, 2 ]) + end + + it_behaves_like 'returns a cloned query' + + it "adds the $or selector" do + expect(selection.selector).to eq( + "field" => [ 1, 2 ], + ) + end + + context 'when the criterion is wrapped in array' do + + let(:selection) do + query.any_of([{ field: [ 1, 2 ] }]) + end + + it_behaves_like 'returns a cloned query' + + it "adds the condition" do + expect(selection.selector).to eq( + "field" => [ 1, 2 ], + ) + end + + context 'when the array has nil as one of the elements' do + + let(:selection) do + query.any_of([{ field: [ 1, 2 ] }, nil]) + end + + it_behaves_like 'returns a cloned query' + + it "adds the $or selector ignoring the nil element" do + expect(selection.selector).to eq( + "field" => [ 1, 2 ], + ) + end + end + end + + context 'when query already has a condition on another field' do + + context 'when there is one argument' do + + let(:selection) do + query.where(foo: 'bar').any_of(field: [ 1, 2 ]) + end + + it 'adds the new condition' do + expect(selection.selector).to eq( + 'foo' => 'bar', + 'field' => [1, 2], + ) + end + end + + context 'when there are multiple arguments' do + + let(:selection) do + query.where(foo: 'bar').any_of({field: [ 1, 2 ]}, {hello: 'world'}) + end + + it 'adds the new condition' do + expect(selection.selector).to eq( + 'foo' => 'bar', + '$or' => [ + {'field' => [1, 2]}, + {'hello' => 'world'}, + ], + ) + end + end + end + + context 'when query already has an $or condition and another condition' do + + let(:selection) do + query.or(field: [ 1, 2 ]).where(foo: 'bar').any_of(test: 1) + end + + it 'adds the new condition' do + expect(selection.selector).to eq( + '$or' => [{'field' => [1, 2]}], + 'foo' => 'bar', + 'test' => 1, + ) + end + end + end + + context "when provided multiple criteria" do + + context "when the criteria are for different fields" do + + let(:selection) do + query.any_of({ first: [ 1, 2 ] }, { second: [ 3, 4 ] }) + end + + it_behaves_like 'returns a cloned query' + + it "adds the $or selector" do + expect(selection.selector).to eq({ + "$or" => [ + { "first" => [ 1, 2 ] }, + { "second" => [ 3, 4 ] } + ] + }) + end + end + + context "when the criteria uses a Key instance" do + + let(:selection) do + query.any_of({ first: [ 1, 2 ] }, { :second.gt => 3 }) + end + + it "adds the $or selector" do + expect(selection.selector).to eq({ + "$or" => [ + { "first" => [ 1, 2 ] }, + { "second" => { "$gt" => 3 }} + ] + }) + end + + it_behaves_like 'returns a cloned query' + end + + context "when a criterion has an aliased field" do + + let(:selection) do + query.any_of({ id: 1 }) + end + + it "adds the $or selector and aliases the field" do + expect(selection.selector).to eq( + "_id" => 1, + ) + end + + it_behaves_like 'returns a cloned query' + end + + context "when a criterion is wrapped in an array" do + + let(:selection) do + query.any_of([{ first: [ 1, 2 ] }, { :second.gt => 3 }]) + end + + it_behaves_like 'returns a cloned query' + + it "adds the $or selector" do + expect(selection.selector).to eq({ + "$or" => [ + { "first" => [ 1, 2 ] }, + { "second" => { "$gt" => 3 }} + ] + }) + end + end + + context "when the criteria are on the same field" do + + let(:selection) do + query.any_of({ first: [ 1, 2 ] }, { first: [ 3, 4 ] }) + end + + it_behaves_like 'returns a cloned query' + + it "appends both $or expressions" do + expect(selection.selector).to eq({ + "$or" => [ + { "first" => [ 1, 2 ] }, + { "first" => [ 3, 4 ] } + ] + }) + end + end + end + + context "when chaining the criteria" do + + context "when the criteria are for different fields" do + + let(:selection) do + query.any_of(first: [ 1, 2 ]).any_of(second: [ 3, 4 ]) + end + + it_behaves_like 'returns a cloned query' + + it "adds the conditions separately" do + expect(selection.selector).to eq( + "first" => [ 1, 2 ], + "second" => [ 3, 4 ], + ) + end + end + + context "when the criteria are on the same field" do + + let(:selection) do + query.any_of(first: [ 1, 2 ]).any_of(first: [ 3, 4 ]) + end + + it_behaves_like 'returns a cloned query' + + it "adds the conditions separately" do + expect(selection.selector).to eq( + "first" => [ 1, 2 ], + '$and' => [{"first" => [ 3, 4 ]}], + ) + end + end + end + end + describe "#not" do context "when provided no criterion" do From 427a8cf0706fbf3f15eb108e6a42a490532874a1 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 7 Apr 2020 14:49:01 -0400 Subject: [PATCH 2/2] grammar fix --- docs/tutorials/mongoid-queries.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tutorials/mongoid-queries.txt b/docs/tutorials/mongoid-queries.txt index cd2fc9f8ca..754e942e88 100644 --- a/docs/tutorials/mongoid-queries.txt +++ b/docs/tutorials/mongoid-queries.txt @@ -386,7 +386,7 @@ conditions are added to the criteria: different query shapes as described below. When ``and``, ``or`` and ``nor`` logical operators are used, they -operates on the criteria built up to that point and its argument. +operate on the criteria built up to that point and its argument. ``where`` has the same meaning as ``and``: .. code-block:: ruby