MONGOID-5226 Allow setting "store_in collection" on document subclass (#5449)

* MONGOID-5226 Allow setting "store_in collection" on document subclass

* remove comment

* MONGOID-5226 add docs and testing

* Apply suggestions from code review

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

* MONGOID-5226 answer comments

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

Author: neilshweky

docs/reference/inheritance.txt

@@ -253,3 +253,56 @@ either normal setting or through the build and create methods on the association
 
    rect = Rectangle.new(width: 100, height: 200)
    firefox.shapes
+
+
+.. _inheritance-persistence-context:
+
+Persistence Contexts
+====================
+
+Mongoid allows the persistence context of a subclass to be changed from the
+persistence context of its parent. This means that, using the ``store_in``
+method, we can store the documents of the subclasses in different collections
+(as well as different databases, clients) than their parents:
+
+.. code:: ruby
+
+  class Shape
+    include Mongoid::Document
+    store_in collection: :shapes
+  end
+
+  class Circle < Shape
+    store_in collection: :circles
+  end
+
+  class Square < Shape
+    store_in collection: :squares
+  end
+
+  Shape.create!
+  Circle.create!
+  Square.create!
+
+Setting the collection on the children causes the documents for those children
+to be stored in the set collection, instead of in the parent's collection:
+
+.. code:: javascript
+
+  > db.shapes.find()
+  { "_id" : ObjectId("62fe9a493282a43d6b725e10"), "_type" : "Shape" }
+  > db.circles.find()
+  { "_id" : ObjectId("62fe9a493282a43d6b725e11"), "_type" : "Circle" }
+  > db.squares.find()
+  { "_id" : ObjectId("62fe9a493282a43d6b725e12"), "_type" : "Square" }
+
+If the collection is set on some of the subclasses and not others, the subclasses
+with set collections will store documents in those collections, and the
+subclasses without set collections will be store documents in the parent's
+collection.
+
+.. note::
+
+  Note that changing the collection that a subclass is stored in will cause
+  documents of that subclass to no longer be found in the results of querying
+  its parent class.

docs/release-notes/mongoid-8.1.txt

@@ -213,3 +213,29 @@ to or from a certain value:
   # => true
   person.name_changed?(from: "Trout", to: "Fletcher")
   # => false
+
+
+Allowed ``store_in`` to be called on subclasses
+-----------------------------------------------
+
+Starting in Mongoid 8.1, subclasses can now specify which collection its
+documents should be stored in using the ``store_in`` macro:
+
+.. code:: ruby
+
+  class Shape
+    include Mongoid::Document
+    store_in collection: :shapes
+  end
+
+  class Circle < Shape
+    store_in collection: :circles
+  end
+
+  class Square < Shape
+    store_in collection: :squares
+  end
+
+Previously, an error was raised if this was done. See the section on
+:ref:`Inheritance Persistence Context <inheritance-persistence-context>`
+for more details.

lib/config/locales/en.yml

@@ -324,12 +324,6 @@ en:
             \_\_\_\_include Mongoid::Document\n
             \_\_\_\_store_in collection: 'artists', database: 'music'\n
             \_\_end\n\n"
-        invalid_storage_parent:
-          message: "Invalid store_in call on class %{klass}."
-          summary: "The :store_in macro can only be called on a base Mongoid Document"
-          resolution: "Remove the store_in call on class %{klass}, as it will use its
-            parent store configuration. Or remove the hierarchy extension for this
-            class."
         invalid_time:
           message: "'%{value}' is not a valid Time."
           summary: "Mongoid tries to serialize the values for Date, DateTime, and

lib/mongoid/clients/storage_options.rb

@@ -6,10 +6,7 @@ module StorageOptions
       extend ActiveSupport::Concern
 
       included do
-
-        cattr_accessor :storage_options, instance_writer: false do
-          storage_options_defaults
-        end
+        class_attribute :storage_options, instance_writer: false, default: storage_options_defaults
       end
 
       module ClassMethods
@@ -49,7 +46,7 @@ module ClassMethods
         # @return [ Class ] The model class.
         def store_in(options)
           Validators::Storage.validate(self, options)
-          storage_options.merge!(options)
+          self.storage_options = self.storage_options.merge(options)
         end
 
         # Reset the store_in options

lib/mongoid/clients/validators/storage.rb

@@ -20,21 +20,9 @@ module Storage
         # @param [ Hash | String | Symbol ] options The provided options.
         def validate(klass, options)
           valid_keys?(options) or raise Errors::InvalidStorageOptions.new(klass, options)
-          valid_parent?(klass) or raise Errors::InvalidStorageParent.new(klass)
         end
 
         private
-        # Determine if the current klass is valid to change store_in
-        # options
-        #
-        # @api private
-        #
-        # @param [ Class ] klass
-        #
-        # @return [ true | false ] If the class is valid.
-        def valid_parent?(klass)
-          !klass.superclass.include?(Mongoid::Document)
-        end
 
         # Determine if all keys in the options hash are valid.
         #

lib/mongoid/errors.rb

@@ -35,7 +35,6 @@
 require "mongoid/errors/invalid_session_use"
 require "mongoid/errors/invalid_set_polymorphic_relation"
 require "mongoid/errors/invalid_storage_options"
-require "mongoid/errors/invalid_storage_parent"
 require "mongoid/errors/invalid_time"
 require "mongoid/errors/inverse_not_found"
 require "mongoid/errors/mixed_relations"

lib/mongoid/errors/invalid_storage_parent.rb

@@ -4,6 +4,8 @@ module Mongoid
   module Errors
 
     # Raised when calling store_in in a sub-class of Mongoid::Document
+    #
+    # @deprecated
     class InvalidStorageParent < MongoidError
 
       # Create the new error.

spec/mongoid/clients_spec.rb

@@ -738,19 +738,6 @@
       end
     end
 
-    context "when provided a class that extends another document" do
-
-      let(:klass) do
-        Class.new(Band)
-      end
-
-      it "raises an error" do
-        expect {
-          klass.store_in(database: :artists)
-        }.to raise_error(Mongoid::Errors::InvalidStorageParent)
-      end
-    end
-
     context "when provided a hash" do
 
       context "when the hash is not valid" do
@@ -762,6 +749,128 @@
         end
       end
     end
+
+    context "when it is called on a subclass" do
+
+      let(:client) { StoreParent.collection.client }
+      let(:parent) { StoreParent.create! }
+      let(:child1) { StoreChild1.create! }
+      let(:child2) { StoreChild2.create! }
+
+      before do
+        class StoreParent
+          include Mongoid::Document
+        end
+
+        class StoreChild1 < StoreParent
+        end
+
+        class StoreChild2 < StoreParent
+        end
+      end
+
+      after do
+        Object.send(:remove_const, :StoreParent)
+        Object.send(:remove_const, :StoreChild1)
+        Object.send(:remove_const, :StoreChild2)
+      end
+
+      context "when it is not called on the parent" do
+
+        context "when it is called on all subclasses" do
+
+          before do
+            StoreChild1.store_in collection: :store_ones
+            StoreChild2.store_in collection: :store_twos
+            [ parent, child1, child2 ]
+          end
+
+          let(:db_parent) { client['store_parents'].find.first }
+          let(:db_child1) { client['store_ones'].find.first }
+          let(:db_child2) { client['store_twos'].find.first }
+
+          it "stores the documents in the correct collections" do
+            expect(db_parent).to eq({ "_id" => parent.id, "_type" => "StoreParent" })
+            expect(db_child1).to eq({ "_id" => child1.id, "_type" => "StoreChild1" })
+            expect(db_child2).to eq({ "_id" => child2.id, "_type" => "StoreChild2" })
+          end
+
+          it "only queries from its own collections" do
+            expect(StoreParent.count).to eq(1)
+            expect(StoreChild1.count).to eq(1)
+            expect(StoreChild2.count).to eq(1)
+          end
+        end
+
+        context "when it is called on one of the subclasses" do
+
+          before do
+            StoreChild1.store_in collection: :store_ones
+            [ parent, child1, child2 ]
+          end
+
+          let(:db_parent) { client['store_parents'].find.first }
+          let(:db_child1) { client['store_ones'].find.first }
+          let(:db_child2) { client['store_parents'].find.to_a.last }
+
+          it "stores the documents in the correct collections" do
+            expect(db_parent).to eq({ "_id" => parent.id, "_type" => "StoreParent" })
+            expect(db_child1).to eq({ "_id" => child1.id, "_type" => "StoreChild1" })
+            expect(db_child2).to eq({ "_id" => child2.id, "_type" => "StoreChild2" })
+          end
+
+          it "queries from its own collections" do
+            expect(StoreParent.count).to eq(2)
+            expect(StoreChild1.count).to eq(1)
+            expect(StoreChild2.count).to eq(1)
+          end
+        end
+      end
+
+      context "when it is called on the parent" do
+
+        before do
+          StoreParent.store_in collection: :st_parents
+        end
+
+        context "when it is called on all subclasses" do
+
+          before do
+            StoreChild1.store_in collection: :store_ones
+            StoreChild2.store_in collection: :store_twos
+            [ parent, child1, child2 ]
+          end
+
+          let(:db_parent) { client['st_parents'].find.first }
+          let(:db_child1) { client['store_ones'].find.first }
+          let(:db_child2) { client['store_twos'].find.first }
+
+          it "stores the documents in the correct collections" do
+            expect(db_parent).to eq({ "_id" => parent.id, "_type" => "StoreParent" })
+            expect(db_child1).to eq({ "_id" => child1.id, "_type" => "StoreChild1" })
+            expect(db_child2).to eq({ "_id" => child2.id, "_type" => "StoreChild2" })
+          end
+        end
+
+        context "when it is called on one of the subclasses" do
+
+          before do
+            StoreChild1.store_in collection: :store_ones
+            [ parent, child1, child2 ]
+          end
+
+          let(:db_parent) { client['st_parents'].find.first }
+          let(:db_child1) { client['store_ones'].find.first }
+          let(:db_child2) { client['st_parents'].find.to_a.last }
+
+          it "stores the documents in the correct collections" do
+            expect(db_parent).to eq({ "_id" => parent.id, "_type" => "StoreParent" })
+            expect(db_child1).to eq({ "_id" => child1.id, "_type" => "StoreChild1" })
+            expect(db_child2).to eq({ "_id" => child2.id, "_type" => "StoreChild2" })
+          end
+        end
+      end
+    end
   end
 
   describe ".with" do