MONGOID-5247 change feature flag names to be reflective of user-visible behavior (#5163)

* doc tweak

* MONGOID-5195 MONGOID-5237 reword compare_time_by_ms description

* MONGOID-5151 MONGOID-5233 broken_alias_handling

* MONGOID-5183 MONGOID-5236 broken_and

* MONGOID-5186 MONGOID-5229 broken_scoping

* MONGOID-4755 MONGOID-5239 broken_aggregables

* MONGOID-5126 MONGOID-5232 broken_triple_equals

* more triple equals tests

* MONGOID-5206 MONGOID-5235 broken_updates

* MONGOID-5162 MONGOID-5234 object_id_as_json_oid

* change to legacy_triple_equals

* order options alphabetically

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

Author: p-mongo

docs/reference/configuration.txt

@@ -252,12 +252,12 @@ for details on driver options.
           # Compressors to use. (default is to not use compression)
           compressors: [zlib]
 
-    # Configure Mongoid specific options. (optional)
+    # Configure Mongoid-specific options. (optional)
     options:
-      # Application name that is printed to the mongodb logs upon establishing
-      # a connection in server versions >= 3.4. Note that the name cannot
-      # exceed 128 bytes. It is also used as the database name if the
-      # database name is not explicitly defined. (default: nil)
+      # Application name that is printed to the MongoDB logs upon establishing
+      # a connection in server versions 3.4 or greater. Note that the name
+      # cannot exceed 128 bytes in length. It is also used as the database name
+      # if the database name is not explicitly defined. (default: nil)
       app_name: MyApplicationName
 
       # Create indexes in background by default. (default: false)
@@ -268,30 +268,34 @@ for details on driver options.
       # error. (default: true)
       belongs_to_required_by_default: true
 
-      # In Mongoid 7.3 and earlier, embedded matching performed time comparisons
-      # with full (microsecond) precision. However, MongoDB server is only
-      # capable of storing times with millisecond precision. Enabling this
-      # option makes Mongoid truncate times to millisecond precision for
-      # comparison purposes, to match the server behavior. (default: false)
-      compare_time_by_ms: false
-
-      # Set the global discriminator key. (default: "_type")
-      discriminator_key: "_type"
-
-      # Raise an exception when a field is redefined. (default: false)
-      duplicate_fields_exception: false
-
-      # In Mongoid 7.3.3 and earlier, the pluck and distinct operations did
-      # not respect aliased fields for embedded documents. Turning on this
-      # flag changes those functions to recognize those aliases.
-      # (default: false)
-      fix_embedded_alias_pluck_distinct: false
-
-      # In Mongoid 7.3.3 and earlier, there was a bug when adding expressions
-      # using the #and function. In certain situations, an $and clause would be
-      # lost and replaced by one of the previous $and clauses. This would
-      # happen when using the same operator on the same field multiple times.
-      # For example:
+      # Maintain broken behavior of sum over empty result sets for backwards
+      # compatibility. When calculating a sum on a field with a null context,
+      # for example:
+      #
+      # Product.none.sum(:price)
+      #
+      # ... return field name (`:price') instead of 0.
+      #
+      # When calculating a sum via a database query with an empty result set,
+      # for example:
+      #
+      # Product.where(impossible_condition: true).sum(:price)
+      #
+      # ... return nil instead of 0.
+      # (default: true in Mongoid 7, will change to false in Mongoid 8)
+      broken_aggregables: true
+
+      # Ignore aliased fields in embedded documents when performing pluck and
+      # distinct operations, for backwards compatibility.
+      # (default: true in Mongoid 7, will change to false in Mongoid 8)
+      broken_alias_handling: true
+
+      # Maintain broken `and' method behavior that existed in Mongoid 7.3
+      # and earlier for backwards compatibility: in some situations, conditions
+      # already present in a Criteria object would be replaced by newer
+      # conditions when `and' method is used instead of the new conditions
+      # being added to the existing conditions. This would happen when using
+      # the same operator on the same field multiple times. For example:
       #
       # Band.where(id: 1).and({year: {'$in' => [2020]}}, {year: {'$in' => [2021]}}).where(id: 2)
       #
@@ -305,9 +309,53 @@ for details on driver options.
       #
       # This is obviously incorrect as the {"$in"=>[2021]} clause is lost.
       # Notice that the clause is only lost when both clauses are added using
-      # the #and function. Turning on this feature flag fixes this bug.
-      # (default: false)
-      fix_multiple_ands: false
+      # the #and method.
+      # (default: true in Mongoid 7, will change to false in Mongoid 8)
+      broken_and: true
+
+      # When exiting a nested `with_scope' block, set the current scope to
+      # nil instead of the parent scope for backwards compatibility.
+      # (default: true in Mongoid 7, will change to false in Mongoid 8)
+      broken_scoping: true
+
+      # Maintain broken update behavior in some cases for backwards
+      # compatibility.
+      #
+      # In Mongoid 7.3 and earlier, when assigning a value to an embedded
+      # document, then setting it to nil, then assigning the original value
+      # to it again, the second update would not work and the value for the
+      # embedded document would remain nil. Take this case:
+      #
+      # canvas.palette = palette
+      # canvas.palette = nil
+      # canvas.palette = palette
+      #
+      # ... where canvas embeds_one palette.
+      #
+      # In Mongoid 7.3 and earlier, canvas.palette would be nil when we would
+      # expect it to be palette. Set this option to true to keep this behavior,
+      # set the option to false to perform the second update correctly.
+      # (default: true in Mongoid 7, will change to false in Mongoid 8)
+      broken_updates: true
+
+      # Time objects in Ruby have nanosecond precision, whereas MongoDB server
+      # can only store times with millisecond precision. Set this option to
+      # true to truncate times to millisecond precision when performing
+      # queries on already loaded embedded associations (this is also called
+      # "embedded matching" and is done completely in Ruby), to obtain the
+      # same query results when performing time comparisons regardless of
+      # which documents are being queried. Setting this option to false will
+      # produce different results for queries on embedded associations that
+      # are already loaded into memory vs queries on unloaded associations and
+      # top-level models. (default: false in Mongoid 7, will change to true
+      # in Mongoid 8).
+      compare_time_by_ms: false
+
+      # Set the global discriminator key. (default: "_type")
+      discriminator_key: "_type"
+
+      # Raise an exception when a field is redefined. (default: false)
+      duplicate_fields_exception: false
 
       # Include the root model name in json serialization. (default: false)
       include_root_in_json: false
@@ -319,6 +367,15 @@ for details on driver options.
       # to parent contexts by default. (default: false)
       join_contexts: false
 
+      # Maintain legacy behavior of === on Mongoid document classes, which
+      # returns true in a number of cases where Ruby's === implementation would
+      # return false. Note that the behavior of === on Mongoid document
+      # instances differs from both the behavior of === on document classes
+      # and from Ruby's behavior of === on simple object instances regardless
+      # of the value of this option.
+      # (default: true in Mongoid 7, will change to false in Mongoid 8)
+      legacy_triple_equals: true
+
       # Set the Mongoid and Ruby driver log levels when Mongoid is not using
       # Ruby on Rails logger instance. (default: :info)
       log_level: :info
@@ -327,6 +384,12 @@ for details on driver options.
       # as a BSON::Decimal128 instead of a string. (default: false)
       map_big_decimal_to_decimal128: false
 
+      # Force ``BSON::ObjectId#as_json`` method to return the hash
+      # { "$oid" => id.to_s }. When this option is false, and bson-ruby 5
+      # is used, the return value will be the hexadecimal ObjectId string only.
+      # (default: true in Mongoid 7, will change to false in Mongoid 8)
+      object_id_as_json_oid: true
+
       # Preload all models in development, needed when models use
       # inheritance. (default: false)
       preload_models: false
@@ -335,59 +398,15 @@ for details on driver options.
       # (default: true)
       raise_not_found_error: true
 
-      # In Mongoid 7.3.3 and earlier, there was a bug when using with_scope
-      # that after the function returned, any previous scopes were lost.
-      # Turning this flag on fixes that bug. (default: false)
-      restore_previous_scope: false
-
-      # In Mongoid 7.3.3 and earlier, when doing a sum on a field with a null
-      # context, for example:
-      #
-      # Product.none.sum(:price)
-      #
-      # the sum incorrectly returned the field name as a Symbol. Turning on
-      # this feature flag fixes this functionality to automatically return 0.
-      # (default: false)
-      return_zero_on_sum_none: false
-
       # Raise an error when defining a scope with the same name as an
       # existing method. (default: false)
       scope_overwrite_exception: false
 
-      # Corrects === behavior to be consistent with Ruby by only calling is_a?
-      # (default: false)
-      triple_equals_uses_is_a: false
-
-      # In Mongoid 7.3.3 and earlier, when setting an embedded document, setting
-      # it to nil, and then setting it again to that same original value, the
-      # second update would not work and the value for the embedded document
-      # would remain nil. Take this case:
-      #
-      # canvas.palette = palette
-      # canvas.palette = nil
-      # canvas.palette = palette
-      # Where canvas embeds_one palette.
-      #
-      # In 7.3.3 and earlier, canvas.palette would be nil when we would expect
-      # it to be palette. Since fixing this would be a breaking change, we have
-      # put this fix behind this feature flag. Turning this flag on fixes this
-      # bug, and canvas.palette would contain the correct value: the assigned
-      # palette. (default: false)
-      update_embedded_after_nil: false
-
       # Use ActiveSupport's time zone in time operations instead of
       # the Ruby default time zone. See the time zone section below for
       # further information. (default: true)
       use_activesupport_time_zone: true
 
-      # In Mongoid 7.3.3 and earlier, the BSON::ObjectId#as_json method would
-      # default to returning { "$oid" => id.to_s }. Turning on this flag will
-      # defer the as_json call to bson-ruby's implementation of
-      # BSON::ObjectId#as_json. In bson-ruby 5.x.x as_json will return
-      # the id as a string. In bson-ruby 4.x.x as_json will return
-      # { "$oid" => id.to_s }. (default: false)
-      use_bson_ruby_as_json: false
-
       # Return stored times as UTC. See the time zone section below for
       # further information. Most applications should not use this option.
       # (default: false)

docs/reference/queries.txt

@@ -1783,10 +1783,11 @@ at runtime:
 
 .. note::
 
-  In Mongoid versions 7.3.3 and earlier, there is a bug in ``with_scope``
-  where the previous scope is lost after the function returns. Set the
-  ``Mongoid.restore_previous_scope`` global flag to true in order to get the
-  correct functionality.
+  If with_scope calls are nested, when the nested with_scope block completes
+  Mongoid 7 sets the current scope to nil instead of the parent scope.
+  Mongoid 8 will set the current scope to the correct parent scope.
+  To get Mongoid 8 behavior in Mongoid 7.4 and higher, set the
+  ``Mongoid.broken_scoping`` global option to false.
 
 
 Class Methods

docs/release-notes/mongoid-7.4.txt

@@ -44,8 +44,8 @@ Previously, ``===`` returned ``true`` for some cases when the equivalent Ruby
 
 .. note::
 
-  In order to get this functionality, the ``Mongoid.triple_equals_uses_is_a``
-  global flag must be set to true. If it is set to false, the ``===`` operator
+  In order to get this functionality, the ``Mongoid.legacy_triple_equals``
+  global flag must be set to false. If it is set to true, the ``===`` operator
   will function as it did in Mongoid 7.3.
 
 Mongoid 7.4 behavior:
@@ -103,25 +103,42 @@ and matches the core Ruby behavior:
   # => true
 
 
-``BSON::ObjectId#as_json`` Implementation Removed
--------------------------------------------------
+``BSON::ObjectId#as_json`` Override Is Made Optional
+----------------------------------------------------
 
-Mongoid in versions up to 7.3 used to provide a ``BSON::ObjectId#as_json``
+Mongoid 7.3 and earlier provided a ``BSON::ObjectId#as_json``
 implementation that was identical to the one provided by ``bson-ruby``.
-Mongoid 7.4 removes its copy of this implementation.
+Mongoid 7.4 adds an option to bypass its copy of this implementation,
+delegating to ``BSON::ObjectId#as_json``: ``Mongoid.object_id_as_json_oid``.
 
-This change has no immediate impact on applications - ``BSON::ObjectId#as_json``
-behaves the same in Mongoid 7.4 as it did in Mongoid 7.3. However, if
-this implementation is changed in a future version of ``bson-ruby``
-which is `under consideration <https://jira.mongodb.org/browse/MONGOID-5158>`_,
-Mongoid 7.4 and later will inherit the new implementation provided by
-``bson-ruby`` while Mongoid 7.3 and earlier will continue with the
-implementation returning a hash of ``{"$oid" => "..."}``.
+When ``Mongoid.object_id_as_json_oid`` is set to ``true``, Mongoid will
+install an implementation of ``BSON::ObjectId#as_json`` which returns
+the hash ``{"$oid" => "..."}`` as it did in Mongoid 7.3 and earlier.
 
-..  note::
+When ``Mongoid.object_id_as_json_oid`` is set to ``false``, Mongoid will
+delegate to ``bson-ruby`` implementation of ``BSON::ObjectId#as_json``, which
+in ``bson-ruby`` 4 continues to return the hash ``{"$oid" => "..."}`` but
+in ``bson-ruby`` 5 will return only the hexadecimal ObjectId string.
+
+The behavior of ``as_json`` is summarized in the following table:
+
+.. list-table::
+   :header-rows: 1
+   :stub-columns: 1
+   :class: compatibility-large no-padding
+
+   * - ``Mongoid.object_id_as_json_oid`` value
+     - true
+     - false
+
+   * - ``bson-ruby`` 4
+     - ``{"$oid"=>"621ed7fda15d5d231594627c"}``
+     - ``{"$oid"=>"621ed7fda15d5d231594627c"}``
+
+   * - ``bson-ruby`` 5
+     - ``{"$oid"=>"621ed7fda15d5d231594627c"}``
+     - ``"621ed7fda15d5d231594627c"``
 
-  In order to get this functionality, the ``Mongoid.use_bson_ruby_as_json``
-  feature flag must be turned on.
 
 Scoped Associations
 -------------------

lib/mongoid/association/embedded/batchable.rb

@@ -38,11 +38,13 @@ def batch_clear(docs)
               positionally(selector, "$unset" => { path => true }),
               session: _session
             )
-            # This solves the case in which a user sets, clears and resets an
-            # embedded document. Previously, since the embedded document was
-            # already marked not a "new_record", it wouldn't be persisted to
-            # the second time. This change fixes that and allows it to be persisted.
-            docs.each { |doc| doc.new_record = true } if Mongoid.update_embedded_after_nil
+            unless Mongoid.broken_updates
+              # This solves the case in which a user sets, clears and resets an
+              # embedded document. Previously, since the embedded document was
+              # already marked not a "new_record", it wouldn't be persisted to
+              # the second time. This change fixes that and allows it to be persisted.
+              docs.each { |doc| doc.new_record = true }
+            end
             post_process_batch_remove(docs, :delete)
           end
           _unscoped.clear

lib/mongoid/association/embedded/embeds_one/proxy.rb

@@ -54,28 +54,30 @@ def substitute(replacement)
                 # run the callbacks and state-changing code by passing persist: false in that case.
                 _target.destroy(persist: !replacement) if persistable?
 
-                # A little explanation on why this is needed... Say we have three assignments:
-                #
-                # canvas.palette = palette
-                # canvas.palette = nil
-                # canvas.palette = palette
-                # Where canvas embeds_one palette.
-                #
-                # Previously, what was happening was, on the first assignment,
-                # palette was considered a "new record" (new_record?=true) and
-                # thus palette was being inserted into the database. However,
-                # on the third assignment, we're trying to reassign the palette,
-                # palette is no longer considered a new record, because it had
-                # been inserted previously. This is not exactly accurate,
-                # because the second assignment ultimately removed the palette
-                # from the database, so it needs to be reinserted. Since the
-                # palette's new_record is false, Mongoid ends up "updating" the
-                # document, which doesn't reinsert it into the database.
-                #
-                # The change I introduce here, respecifies palette as a "new
-                # record" when it gets removed from the database, so if it is
-                # reassigned, it will be reinserted into the database.
-                _target.new_record = true if Mongoid.update_embedded_after_nil
+                unless Mongoid.broken_updates
+                  # A little explanation on why this is needed... Say we have three assignments:
+                  #
+                  # canvas.palette = palette
+                  # canvas.palette = nil
+                  # canvas.palette = palette
+                  # Where canvas embeds_one palette.
+                  #
+                  # Previously, what was happening was, on the first assignment,
+                  # palette was considered a "new record" (new_record?=true) and
+                  # thus palette was being inserted into the database. However,
+                  # on the third assignment, we're trying to reassign the palette,
+                  # palette is no longer considered a new record, because it had
+                  # been inserted previously. This is not exactly accurate,
+                  # because the second assignment ultimately removed the palette
+                  # from the database, so it needs to be reinserted. Since the
+                  # palette's new_record is false, Mongoid ends up "updating" the
+                  # document, which doesn't reinsert it into the database.
+                  #
+                  # The change I introduce here, respecifies palette as a "new
+                  # record" when it gets removed from the database, so if it is
+                  # reassigned, it will be reinserted into the database.
+                  _target.new_record = true
+                end
               end
               unbind_one
               return nil unless replacement