MONGOID-5228 disallow _id to be updated on persisted documents

lib/config/locales/en.yml

@@ -119,6 +119,13 @@ en:
           summary: "Your mongoid.yml configuration file appears to be empty."
           resolution: "Ensure your configuration file contains the correct contents.
             Refer to: https://www.mongodb.com/docs/mongoid/current/reference/configuration/"
+        immutable_attribute:
+          message: "Attempted to change the immutable attribute '%{name}' with
+            the value: %{value}."
+          summary: "Immutable attributes can only have values set when the
+            document is a new record."
+          resolution: "Do not attempt to update the value of '%{name}' after
+            the document is persisted."
         invalid_collection:
           message: "Access to the collection for %{klass} is not allowed."
           summary: "%{klass}.collection was called, and %{klass} is an embedded

lib/mongoid/association/nested/one.rb

@@ -32,6 +32,8 @@ def build(parent)
             parent.send(association.setter, Factory.build(@class_name, attributes))
           elsif delete?
             parent.send(association.setter, nil)
+          else
+            check_for_id_violation!
           end
         end
 
@@ -54,6 +56,17 @@ def initialize(association, attributes, options)
 
         private
 
+        # Extracts and converts the id to the expected type.
+        #
+        # @return [ BSON::ObjectId | String | Object | nil ] The converted id,
+        #   or nil if no id is present in the attributes hash.
+        def extracted_id
+          @extracted_id ||= begin
+            id = association.klass.extract_id_field(attributes)
+            convert_id(existing.class, id)
+          end
+        end
+
         # Is the id in the attributes acceptable for allowing an update to
         # the existing association?
         #
@@ -64,8 +77,7 @@ def initialize(association, attributes, options)
         #
         # @return [ true | false ] If the id part of the logic will allow an update.
         def acceptable_id?
-          id = association.klass.extract_id_field(attributes)
-          id = convert_id(existing.class, id)
+          id = extracted_id
           existing._id == id || id.nil? || (existing._id != id && update_only?)
         end
 
@@ -110,6 +122,32 @@ def replace?
         def update?
           existing && !destroyable? && acceptable_id?
         end
+
+        # Checks to see if the _id attribute (which is supposed to be
+        # immutable) is being asked to change. If so, raise an exception.
+        #
+        # If Mongoid::Config.immutable_ids is false, this will do nothing,
+        # and the update operation will fail silently.
+        #
+        # @raise [ Errors::ImmutableAttribute ] if _id has changed, and
+        #   the document has been persisted.
+        def check_for_id_violation!
+          # look for the basic criteria of an update (see #update?)
+          return unless existing&.persisted? && !destroyable?
+
+          # if the id is either absent, or if it equals the existing record's
+          # id, there is no immutability violation.
+          id = extracted_id
+          return if existing._id == id || id.nil?
+
+          # otherwise, an attempt has been made to set the _id of an existing,
+          # persisted document.
+          if Mongoid::Config.immutable_ids
+            raise Errors::ImmutableAttribute.new(:_id, id)
+          else
+            Mongoid::Warnings.warn_mutable_ids
+          end
+        end
       end
     end
   end

lib/mongoid/config.rb

@@ -151,6 +151,13 @@ module Config
     # reload, but when it is turned off, it won't be.
     option :legacy_readonly, default: false
 
+    # When this flag is true, any attempt to change the _id of a persisted
+    # document will raise an exception (`Errors::ImmutableAttribute`).
+    # This is the default in 9.0. Setting this flag to false restores the
+    # pre-9.0 behavior, where changing the _id of a persisted
+    # document might be ignored, or it might work, depending on the situation.
+    option :immutable_ids, default: true
+
     # Returns the Config singleton, for use in the configure DSL.
     #
     # @return [ self ] The Config singleton.

lib/mongoid/config/defaults.rb

@@ -46,11 +46,16 @@ def load_defaults(version)
         when "7.5"
           # flags introduced in 8.0 - old functionality
           self.map_big_decimal_to_decimal128 = false
+
+          load_defaults "8.0"
         when "8.0"
-          # All flag defaults currently reflect 8.0 behavior.
           self.legacy_readonly = true
+
+          load_defaults "8.1"
         when "8.1"
-          # All flag defaults currently reflect 8.1 behavior.
+          self.immutable_ids = true
+
+          load_defaults "9.0"
         when "9.0"
           # All flag defaults currently reflect 9.0 behavior.
         else

lib/mongoid/errors.rb

@@ -9,6 +9,7 @@
 require "mongoid/errors/document_not_destroyed"
 require "mongoid/errors/document_not_found"
 require "mongoid/errors/empty_config_file"
+require "mongoid/errors/immutable_attribute"
 require "mongoid/errors/in_memory_collation_not_supported"
 require "mongoid/errors/invalid_async_query_executor"
 require "mongoid/errors/invalid_collection"

lib/mongoid/errors/immutable_attribute.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module Mongoid
+  module Errors
+
+    # This error is raised when attempting the change the value of an
+    # immutable attribute. For example, the _id attribute is immutable,
+    # and attempting to change it on a document that has already been
+    # persisted will result in this error.
+    class ImmutableAttribute < MongoidError
+
+      # Create the new error.
+      #
+      # @example Create the new error.
+      #   ImmutableAttribute.new(:_id, "1234")
+      #
+      # @param [ Symbol | String ] name The name of the attribute.
+      # @param [ Object ] value The attempted set value.
+      def initialize(name, value)
+        super(
+          compose_message("immutable_attribute", { name: name, value: value })
+        )
+      end
+    end
+  end
+end

lib/mongoid/persistable/updatable.rb

@@ -97,6 +97,7 @@ def init_atomic_updates
       # @return [ true | false ] The result of the update.
       def prepare_update(options = {})
         raise Errors::ReadonlyDocument.new(self.class) if readonly? && !Mongoid.legacy_readonly
+        enforce_immutability_of_id_field!
         return false if performing_validations?(options) &&
           invalid?(options[:context] || :update)
         process_flagged_destroys
@@ -186,6 +187,25 @@ def process_touch_option(options, children)
           children.each(&:timeless)
         end
       end
+
+      # Checks to see if the _id field has been modified. If it has, and if
+      # the document has already been persisted, this is an error. Otherwise,
+      # returns without side-effects.
+      #
+      # Note that if `Mongoid::Config.immutable_ids` is false, this will do
+      # nothing.
+      #
+      # @raise [ Errors::ImmutableAttribute ] if _id has changed, and document
+      #   has been persisted.
+      def enforce_immutability_of_id_field!
+        if _id_changed? && persisted?
+          if Mongoid::Config.immutable_ids
+            raise Errors::ImmutableAttribute.new(:_id, _id)
+          else
+            Mongoid::Warnings.warn_mutable_ids
+          end
+        end
+      end
     end
   end
 end

lib/mongoid/warnings.rb

@@ -25,5 +25,6 @@ def warning(id, message)
     warning :as_json_compact_deprecated, '#as_json :compact option is deprecated. Please call #compact on the returned Hash object instead.'
     warning :symbol_type_deprecated, 'The BSON Symbol type is deprecated by MongoDB. Please use String or StringifiedSymbol field types instead of the Symbol field type.'
     warning :legacy_readonly, 'The readonly! method will only mark the document readonly when the legacy_readonly feature flag is switched off.'
+    warning :mutable_ids, 'Ignoring updates to immutable attribute `_id`. Please set Mongoid::Config.immutable_ids to true and update your code so that `_id` is never updated.'
   end
 end

spec/mongoid/config/defaults_spec.rb

@@ -10,8 +10,8 @@
 
   describe ".load_defaults" do
 
-    shared_examples "turns off 7.4 flags" do
-      it "turns off the 7.4 flags" do
+    shared_examples "uses settings for 7.3" do
+      it "uses settings for 7.3" do
         expect(Mongoid.broken_aggregables).to be true
         expect(Mongoid.broken_alias_handling).to be true
         expect(Mongoid.broken_and).to be true
@@ -24,8 +24,8 @@
       end
     end
 
-    shared_examples "turns on 7.4 flags" do
-      it "turns on the 7.4 flags" do
+    shared_examples "does not use settings for 7.3" do
+      it "does not use settings for 7.3" do
         expect(Mongoid.broken_aggregables).to be false
         expect(Mongoid.broken_alias_handling).to be false
         expect(Mongoid.broken_and).to be false
@@ -34,36 +34,59 @@
         expect(Mongoid.compare_time_by_ms).to be true
         expect(Mongoid.legacy_pluck_distinct).to be false
         expect(Mongoid.legacy_triple_equals).to be false
-        expect(Mongoid.object_id_as_json_oid).to be false
       end
     end
 
-    shared_examples "turns off 7.5 flags" do
-      it "turns off the 7.5 flags" do
+    shared_examples "uses settings for 7.4" do
+      it "uses settings for 7.4" do
         expect(Mongoid.legacy_attributes).to be true
         expect(Mongoid.overwrite_chained_operators).to be true
       end
     end
 
-    shared_examples "turns on 7.5 flags" do
-      it "turns on the 7.5 flags" do
+    shared_examples "does not use settings for 7.4" do
+      it "does not use settings for 7.4" do
         expect(Mongoid.legacy_attributes).to be false
         expect(Mongoid.overwrite_chained_operators).to be false
       end
     end
 
-    shared_examples "turns off 8.0 flags" do
-      it "turns off the 8.0 flags" do
+    shared_examples "uses settings for 7.5" do
+      it "uses settings for 7.5" do
         expect(Mongoid.map_big_decimal_to_decimal128).to be false
       end
     end
 
-    shared_examples "turns on 8.0 flags" do
-      it "turns on the 8.0 flags" do
+    shared_examples "does not use settings for 7.5" do
+      it "does not use settings for 7.5" do
         expect(Mongoid.map_big_decimal_to_decimal128).to be true
       end
     end
 
+    shared_examples "uses settings for 8.0" do
+      it "uses settings for 8.0" do
+        expect(Mongoid.legacy_readonly).to be true
+      end
+    end
+
+    shared_examples "does not use settings for 8.0" do
+      it "does not use settings for 8.0" do
+        expect(Mongoid.legacy_readonly).to be false
+      end
+    end
+
+    shared_examples "uses settings for 8.1" do
+      it "uses settings for 8.1" do
+        expect(Mongoid.immutable_ids).to be false
+      end
+    end
+
+    shared_examples "does not use settings for 8.1" do
+      it "does not use settings for 8.1" do
+        expect(Mongoid.immutable_ids).to be true
+      end
+    end
+
     context "when giving a valid version" do
 
       before do
@@ -78,45 +101,66 @@
 
         let(:version) { 7.3 }
 
-        it_behaves_like "turns off 7.4 flags"
-        it_behaves_like "turns off 7.5 flags"
-        it_behaves_like "turns off 8.0 flags"
+        it_behaves_like "uses settings for 7.3"
+        it_behaves_like "uses settings for 7.4"
+        it_behaves_like "uses settings for 7.5"
+        it_behaves_like "uses settings for 8.0"
+        it_behaves_like "uses settings for 8.1"
       end
 
       context "when the given version is 7.4" do
 
         let(:version) { 7.4 }
 
-        it_behaves_like "turns on 7.4 flags"
-        it_behaves_like "turns off 7.5 flags"
-        it_behaves_like "turns off 8.0 flags"
+        it_behaves_like "does not use settings for 7.3"
+        it_behaves_like "uses settings for 7.4"
+        it_behaves_like "uses settings for 7.5"
+        it_behaves_like "uses settings for 8.0"
+        it_behaves_like "uses settings for 8.1"
       end
 
       context "when the given version is 7.5" do
 
         let(:version) { 7.5 }
 
-        it_behaves_like "turns on 7.4 flags"
-        it_behaves_like "turns on 7.5 flags"
-        it_behaves_like "turns off 8.0 flags"
+        it_behaves_like "does not use settings for 7.3"
+        it_behaves_like "does not use settings for 7.4"
+        it_behaves_like "uses settings for 7.5"
+        it_behaves_like "uses settings for 8.0"
+        it_behaves_like "uses settings for 8.1"
       end
 
       context "when the given version is 8.0" do
 
         let(:version) { 8.0 }
 
-        it_behaves_like "turns on 7.4 flags"
-        it_behaves_like "turns on 7.5 flags"
-        it_behaves_like "turns on 8.0 flags"
+        it_behaves_like "does not use settings for 7.3"
+        it_behaves_like "does not use settings for 7.4"
+        it_behaves_like "does not use settings for 7.5"
+        it_behaves_like "uses settings for 8.0"
+        it_behaves_like "uses settings for 8.1"
       end
 
       context "when the given version is 8.1" do
 
-        let(:version) { 8.0 }
+        let(:version) { 8.1 }
+
+        it_behaves_like "does not use settings for 7.3"
+        it_behaves_like "does not use settings for 7.4"
+        it_behaves_like "does not use settings for 7.5"
+        it_behaves_like "does not use settings for 8.0"
+        it_behaves_like "uses settings for 8.1"
+      end
+
+      context "when the given version is 9.0" do
+
+        let(:version) { 9.0 }
 
-        it_behaves_like "turns on 7.4 flags"
-        it_behaves_like "turns on 7.5 flags"
-        it_behaves_like "turns on 8.0 flags"
+        it_behaves_like "does not use settings for 7.3"
+        it_behaves_like "does not use settings for 7.4"
+        it_behaves_like "does not use settings for 7.5"
+        it_behaves_like "does not use settings for 8.0"
+        it_behaves_like "does not use settings for 8.1"
       end
     end
 

spec/mongoid/persistable/savable_spec.rb

@@ -1,8 +1,11 @@
 # frozen_string_literal: true
 
 require "spec_helper"
+require "support/immutable_ids"
 
 describe Mongoid::Persistable::Savable do
+  extend Mongoid::ImmutableIds
+  immutable_id_examples_as "persisted _ids are immutable"
 
   describe "#save" do
 
@@ -604,6 +607,15 @@
         end
       end
     end
+
+    context "when the _id has been modified" do
+      def invoke_operation!
+        object._id = new_id_value
+        object.save
+      end
+
+      it_behaves_like "persisted _ids are immutable"
+    end
   end
 
   describe "save!" do