MONGOID-5827 Allow duplicate indexes to be declared (#5967) (#5971)

* MONGOID-5827 allow duplicate indexes to be declared

This adds a new feature flag (`allow_duplicate_index_declarations`)
which defaults to false, which is the legacy behavior. When false,
superficially-duplicate indexes are silently ignored. When true,
all index declarations are passed through to the server, where
duplicates will trigger a server-side error.

* account for 4.4 behavior

Author: jamis

lib/mongoid/config.rb

@@ -198,6 +198,19 @@ module Config
     # See https://jira.mongodb.org/browse/MONGOID-5658 for more details.
     option :around_callbacks_for_embeds, default: true
 
+    # When this flag is false, indexes are (roughly) validated on the client
+    # to prevent duplicate indexes being declared. This validation is
+    # incomplete, however, and can result in some indexes being silently
+    # ignored.
+    #
+    # Setting this to true will allow duplicate indexes to be declared and sent
+    # to the server. The server will then validate the indexes and raise an
+    # exception if duplicates are detected.
+    #
+    # See https://jira.mongodb.org/browse/MONGOID-5827 for an example of the
+    # consequences of duplicate index checking.
+    option :allow_duplicate_index_declarations, default: false
+
     # Returns the Config singleton, for use in the configure DSL.
     #
     # @return [ self ] The Config singleton.

lib/mongoid/indexable.rb

@@ -94,7 +94,13 @@ def add_indexes
       # @return [ Hash ] The index options.
       def index(spec, options = nil)
         specification = Specification.new(self, spec, options)
-        if !index_specifications.include?(specification)
+
+        # the equality test for Indexable::Specification instances does not
+        # consider any options, which means names are not compared. This means
+        # that an index with different options from another, and a different
+        # name, will be silently ignored unless duplicate index declarations
+        # are allowed.
+        if Mongoid.allow_duplicate_index_declarations || !index_specifications.include?(specification)
           index_specifications.push(specification)
         end
       end
@@ -109,9 +115,8 @@ def index(spec, options = nil)
       #
       # @return [ Specification ] The found specification.
       def index_specification(index_hash, index_name = nil)
-        index = OpenStruct.new(fields: index_hash.keys, key: index_hash)
         index_specifications.detect do |spec|
-          spec == index || (index_name && index_name == spec.name)
+          spec.superficial_match?(key: index_hash, name: index_name)
         end
       end
 

lib/mongoid/indexable/specification.rb

@@ -29,7 +29,25 @@ class Specification
       #
       # @return [ true | false ] If the specs are equal.
       def ==(other)
-        fields == other.fields && key == other.key
+        superficial_match?(key: other.key)
+      end
+
+      # Performs a superficial comparison with the given criteria, checking
+      # only the key and (optionally) the name. Options are not compared.
+      #
+      # Note that the ordering of the fields in the key is significant. Two
+      # keys with different orderings will not match, here.
+      #
+      # @param [ Hash ] key the key that defines the index.
+      # @param [ String | nil ] name the name given to the index, or nil to
+      #    ignore the name.
+      #
+      # @return [ true | false ] the result of the comparison, true if this
+      #   specification matches the criteria, and false otherwise.
+      def superficial_match?(key: {}, name: nil)
+        (name && name == self.name) ||
+          self.fields == key.keys &&
+          self.key == key
       end
 
       # Instantiate a new index specification.

spec/mongoid/indexable_spec.rb

@@ -645,5 +645,61 @@ def self.hereditary?
         end
       end
     end
+
+    context 'when declaring a duplicate index with different options' do
+      def declare_duplicate_indexes!
+        klass.index({ name: 1 }, { partial_filter_expression: { name: 'a' } })
+        klass.index({ name: 1 }, { partial_filter_expression: { name: 'b' } })
+        klass.create_indexes
+      end
+
+      context 'when allow_duplicate_index_declarations is false' do
+        config_override :allow_duplicate_index_declarations, false
+
+        it 'silently ignores the duplicate definition' do
+          expect { declare_duplicate_indexes! }.not_to raise_exception
+        end
+      end
+
+      context 'when allow_duplicate_index_declarations is true' do
+        config_override :allow_duplicate_index_declarations, true
+
+        it 'raises a server error' do
+          expect { declare_duplicate_indexes! }.to raise_exception
+        end
+      end
+    end
+
+    context 'when declaring a duplicate index with different names' do
+      def declare_duplicate_indexes!
+        klass.index({ name: 1 }, { partial_filter_expression: { name: 'a' } })
+        klass.index({ name: 1 }, { name: 'alt_name', partial_filter_expression: { name: 'b' } })
+        klass.create_indexes
+      end
+
+      let(:index_count) { klass.collection.indexes.count }
+
+
+      context 'when allow_duplicate_index_declarations is false' do
+        config_override :allow_duplicate_index_declarations, false
+
+        it 'silently ignores the duplicate definition' do
+          expect { declare_duplicate_indexes! }.not_to raise_exception
+          expect(index_count).to be == 2 # _id and name
+        end
+      end
+
+      context 'when allow_duplicate_index_declarations is true' do
+        # 4.4 apparently doesn't recognize :name option for indexes?
+        min_server_version '5.0'
+
+        config_override :allow_duplicate_index_declarations, true
+
+        it 'creates both indexes' do
+          expect { declare_duplicate_indexes! }.not_to raise_exception
+          expect(index_count).to be == 3 # _id, name, alt_name
+        end
+      end
+    end
   end
 end