MONGOID-5330 Operator methods overwrite previous conditions (#5263)

* MONGOID-5330 add failing test

* MONGOID-5330 pend test

* MONGOID-5330 make gt use and

* MONGOID-5330 change hard coded value

* MONGOID-5330 fix tests

* MONGOID-5330 add feature flag

* MONGOID-5330 use the feature flag

* MONGOID-5330 add testing and fix eq

* MONGOID-5330 change op map

* MONGOID-5330 fix elemMatch

* Update lib/mongoid/criteria/queryable/mergeable.rb

* Update lib/mongoid/criteria/queryable/mergeable.rb

* Update lib/mongoid/criteria/queryable/mergeable.rb

* MONGOID-5330 make this feature flag in 7.5

* MONGOID-5330 remove extra spacing

* MONGOID-5330 update docs to update default

* MONGOID-5330 update tests after switching defaults

* MONGOID-5330 update docs with note about strings vs symbols

* MONGOID-5330 update test

* MONGOID-5330 change feature flag to overwrite

Co-authored-by: Oleg Pudeyev <[email protected]>

Author: neilshweky

docs/reference/configuration.txt

@@ -395,6 +395,12 @@ for details on driver options.
       # (default: false)
       #object_id_as_json_oid: true
 
+      # When chaining the same operators that use the same field, setting this
+      # feature flag to false will cause those operators to be combined using an
+      # and. Setting this feature flag to true will cause the later chained
+      # operators to overwrite the earlier ones. (default: false)
+      #overwrite_chained_operators: false
+
       # Preload all models in development, needed when models use
       # inheritance. (default: false)
       preload_models: false

docs/release-notes/mongoid-7.5.txt

@@ -0,0 +1,82 @@
+***********
+Mongoid 7.5
+***********
+
+.. default-domain:: mongodb
+
+.. contents:: On this page
+   :local:
+   :backlinks: none
+   :depth: 2
+   :class: singlecol
+
+This page describes significant changes and improvements in Mongoid 7.5.
+The complete list of releases is available `on GitHub
+<https://github.com/mongodb/mongoid/releases>`_ and `in JIRA
+<https://jira.mongodb.org/projects/MONGOID?selectedItem=com.atlassian.jira.jira-projects-plugin:release-page>`_;
+please consult GitHub releases for detailed release notes and JIRA for
+the complete list of issues fixed in each release, including bug fixes.
+
+
+``Document#to_a`` deprecated
+````````````````````````````
+
+The ``Document#to_a`` method is deprecated in Mongoid 7.5.
+
+
+Combine Chained Operators Using ``and`` Instead of ``override``
+```````````````````````````````````````````````````````````````
+
+Mongoid 7.5 with the ``Mongoid.overwrite_chained_operators`` option set to ``false``
+will combine conditions that use the same operator and field using an ``and``.
+For example, in the following query:
+
+.. code-block:: ruby
+
+  Band.ne(name: "The Rolling Stones").ne(name: "The Beatles")
+
+Mongoid 7.5 with the ``Mongoid.overwrite_chained_operators`` option set to ``false``
+will generate the following criteria:
+
+.. code-block:: ruby
+
+  #<Mongoid::Criteria
+    selector: {"name"=>{"$ne"=>"The Rolling Stones"}, "$and"=>[{"name"=>{"$ne"=>"The Beatles"}}]}
+    options:  {}
+    class:    Band
+    embedded: false>
+
+In Mongoid 7.4 and earlier, and in 7.5 with the ``Mongoid.overwrite_chained_operators``
+option set to ``true``, the following criteria would be generated instead which
+overwrites the first condition:
+
+.. code-block:: ruby
+
+  #<Mongoid::Criteria
+    selector: {"name"=>{"$ne"=>"The Beatles"}}
+    options:  {}
+    class:    Band
+    embedded: false>
+
+The following functions are affected by this change:
+
+- ``eq``
+- ``elem_match``
+- ``gt``
+- ``gte``
+- ``lt``
+- ``lte``
+- ``mod``
+- ``ne``
+- ``near``
+- ``near_sphere``
+
+.. note::
+
+   In Mongoid 7.5 with the ``Mongoid.overwrite_chained_operators`` option set to
+   ``false``, nested keys in the generated selector will always be strings,
+   whereas in Mongoid 7.4 and earlier, and in 7.5 with the
+   ``Mongoid.overwrite_chained_operators`` option set to ``true``, nested keys in
+   the selector can be strings or symbols, depending on what was passed to the
+   operator.
+

docs/release-notes/mongoid-8.0.txt

@@ -31,6 +31,7 @@ Default Option Values Changed
 **Breaking change:** The following options have had their default values
 changed in Mongoid 8.0:
 
+- ``:overwrite_chained_operators`` => ``false``
 - ``:broken_aggregables`` => ``false``
 - ``:broken_alias_handling`` => ``false``
 - ``:broken_and`` => ``false``
@@ -161,20 +162,20 @@ Mongoid 8.0 behavior:
 
 .. code-block:: ruby
 
-  Band.any_of({name: 'Rolling Stone'}, {founded: 1990}).
+  Band.any_of({name: 'The Rolling Stones'}, {founded: 1990}).
     any_of({members: 2}, {last_tour: 1995})
   # =>
   # #<Mongoid::Criteria
-  #   selector: {"$or"=>[{"name"=>"Rolling Stone"}, {"founded"=>1990}],
+  #   selector: {"$or"=>[{"name"=>"The Rolling Stones"}, {"founded"=>1990}],
   #     "$and"=>[{"$or"=>[{"members"=>2}, {"last_tour"=>1995}]}]}
   #   options:  {}
   #   class:    Band
   #   embedded: false>
 
-  Band.any_of({name: 'Rolling Stone'}, {founded: 1990}).any_of({members: 2})
+  Band.any_of({name: 'The Rolling Stones'}, {founded: 1990}).any_of({members: 2})
   # =>
   # #<Mongoid::Criteria
-  #   selector: {"$or"=>[{"name"=>"Rolling Stone"}, {"founded"=>1990}], "members"=>2}
+  #   selector: {"$or"=>[{"name"=>"The Rolling Stones"}, {"founded"=>1990}], "members"=>2}
   #   options:  {}
   #   class:    Band
   #   embedded: false>
@@ -183,20 +184,20 @@ Mongoid 7 behavior:
 
 .. code-block:: ruby
 
-  Band.any_of({name: 'Rolling Stone'}, {founded: 1990}).
+  Band.any_of({name: 'The Rolling Stones'}, {founded: 1990}).
     any_of({members: 2}, {last_tour: 1995})
   # =>
   # #<Mongoid::Criteria
-  #   selector: {"$or"=>[{"name"=>"Rolling Stone"}, {"founded"=>1990},
+  #   selector: {"$or"=>[{"name"=>"The Rolling Stones"}, {"founded"=>1990},
   #     {"members"=>2}, {"last_tour"=>1995}]}
   #   options:  {}
   #   class:    Band
   #   embedded: false>
 
-  Band.any_of({name: 'Rolling Stone'}, {founded: 1990}).any_of({members: 2})
+  Band.any_of({name: 'The Rolling Stones'}, {founded: 1990}).any_of({members: 2})
   # =>
   # #<Mongoid::Criteria
-  #   selector: {"$or"=>[{"name"=>"Rolling Stone"}, {"founded"=>1990}], "members"=>2}
+  #   selector: {"$or"=>[{"name"=>"The Rolling Stones"}, {"founded"=>1990}], "members"=>2}
   #   options:  {}
   #   class:    Band
   #   embedded: false>

lib/mongoid/config.rb

@@ -115,6 +115,10 @@ module Config
     # demongoize the values on returning them.
     option :legacy_pluck_distinct, default: false
 
+    # Combine chained operators, which use the same field and operator,
+    # using and's instead of overwriting them.
+    option :overwrite_chained_operators, default: false
+
     # Has Mongoid been configured? This is checking that at least a valid
     # client config exists.
     #

lib/mongoid/criteria/queryable/mergeable.rb

@@ -52,6 +52,27 @@ def reset_strategies!
           self
         end
 
+        # Merge criteria with operators using the and operator.
+        #
+        # @param [ Hash ] criterion The criterion to add to the criteria.
+        # @param [ String ] operator The MongoDB operator.
+        #
+        # @return [ Criteria ] The resulting criteria.
+        def and_with_operator(criterion, operator)
+          crit = self
+          if criterion
+            criterion.each_pair do |field, value|
+              val = prepare(field, operator, value)
+              # The prepare method already takes the negation into account. We
+              # set negating to false here so that ``and`` doesn't also apply
+              # negation and we have a double negative.
+              crit.negating = false
+              crit = crit.and(field => val)
+            end
+          end
+          crit
+        end
+
         private
 
         # Adds the criterion to the existing selection.

lib/mongoid/criteria/queryable/selectable.rb

@@ -154,7 +154,7 @@ def elem_match(criterion)
             raise Errors::CriteriaArgumentRequired, :elem_match
           end
 
-          __override__(criterion, "$elemMatch")
+          and_or_override(criterion, "$elemMatch")
         end
         key :elem_match, :override, "$elemMatch"
 
@@ -257,7 +257,7 @@ def eq(criterion)
             raise Errors::CriteriaArgumentRequired, :eq
           end
 
-          __override__(criterion, "$eq")
+          and_or_override(criterion, "$eq")
         end
         key :eq, :override, "$eq"
 
@@ -277,7 +277,7 @@ def gt(criterion)
             raise Errors::CriteriaArgumentRequired, :gt
           end
 
-          __override__(criterion, "$gt")
+          and_or_override(criterion, "$gt")
         end
         key :gt, :override, "$gt"
 
@@ -297,7 +297,7 @@ def gte(criterion)
             raise Errors::CriteriaArgumentRequired, :gte
           end
 
-          __override__(criterion, "$gte")
+          and_or_override(criterion, "$gte")
         end
         key :gte, :override, "$gte"
 
@@ -353,7 +353,7 @@ def lt(criterion)
             raise Errors::CriteriaArgumentRequired, :lt
           end
 
-          __override__(criterion, "$lt")
+          and_or_override(criterion, "$lt")
         end
         key :lt, :override, "$lt"
 
@@ -373,7 +373,7 @@ def lte(criterion)
             raise Errors::CriteriaArgumentRequired, :lte
           end
 
-          __override__(criterion, "$lte")
+          and_or_override(criterion, "$lte")
         end
         key :lte, :override, "$lte"
 
@@ -410,7 +410,7 @@ def mod(criterion)
             raise Errors::CriteriaArgumentRequired, :mod
           end
 
-          __override__(criterion, "$mod")
+          and_or_override(criterion, "$mod")
         end
         key :mod, :override, "$mod"
 
@@ -430,7 +430,7 @@ def ne(criterion)
             raise Errors::CriteriaArgumentRequired, :ne
           end
 
-          __override__(criterion, "$ne")
+          and_or_override(criterion, "$ne")
         end
         alias :excludes :ne
         key :ne, :override, "$ne"
@@ -451,7 +451,7 @@ def near(criterion)
             raise Errors::CriteriaArgumentRequired, :near
           end
 
-          __override__(criterion, "$near")
+          and_or_override(criterion, "$near")
         end
         key :near, :override, "$near"
 
@@ -471,7 +471,7 @@ def near_sphere(criterion)
             raise Errors::CriteriaArgumentRequired, :near_sphere
           end
 
-          __override__(criterion, "$nearSphere")
+          and_or_override(criterion, "$nearSphere")
         end
         key :near_sphere, :override, "$nearSphere"
 
@@ -902,6 +902,22 @@ def selection(criterion = nil)
           end
         end
 
+        # Combine operator expessions onto a Criteria using either
+        # an override or ands depending on the status of the
+        # Mongoid.overwrite_chained_operators feature flag.
+        #
+        # @param [ Hash ] The criterion to add to the criteria.
+        # @param [ String ] operator The MongoDB operator.
+        #
+        # @return [ Criteria ] The resulting criteria.
+        def and_or_override(criterion, operator)
+          if Mongoid.overwrite_chained_operators
+            __override__(criterion, operator)
+          else
+            and_with_operator(criterion, operator)
+          end
+        end
+
         class << self
 
           # Get the methods on the selectable that can be forwarded to from a model.

spec/mongoid/config_spec.rb

@@ -335,6 +335,12 @@
     it_behaves_like "a config option"
   end
 
+  context 'when setting the overwrite_chained_operators option in the config' do
+    let(:option) { :overwrite_chained_operators }
+    let(:default) { false }
+
+    it_behaves_like "a config option"
+  end
 
   describe "#load!" do