MONGOID-5677 [Monkey Patch Removal] Remove Hash#to_criteria and Criteria#to_criteria. Add Criteria.from_hash (#5717)

* Remove Hash#to_criteria and Criteria#to_criteria. Add Criteria.from_hash

* deprecate rathr than remove

also, some code-style standardization

---------

Co-authored-by: Jamis Buck <[email protected]>

Author: johnnyshields

lib/mongoid/criteria.rb

@@ -41,7 +41,25 @@ class Criteria
     include Clients::Sessions
     include Options
 
-    Mongoid.deprecate(self, :for_js)
+    class << self
+      # Convert the given hash to a criteria. Will iterate over each keys in the
+      # hash which must correspond to method on a criteria object. The hash
+      # must also include a "klass" key.
+      #
+      # @example Convert the hash to a criteria.
+      #   Criteria.from_hash({ klass: Band, where: { name: "Depeche Mode" })
+      #
+      # @param [ Hash ] hash The hash to convert.
+      #
+      # @return [ Criteria ] The criteria.
+      def from_hash(hash)
+        criteria = Criteria.new(hash.delete(:klass) || hash.delete('klass'))
+        hash.each_pair do |method, args|
+          criteria = criteria.__send__(method, args)
+        end
+        criteria
+      end
+    end
 
     # Static array used to check with method missing - we only need to ever
     # instantiate once.
@@ -250,16 +268,16 @@ def merge(other)
     # @example Merge another criteria into this criteria.
     #   criteria.merge(Person.where(name: "bob"))
     #
-    # @param [ Criteria ] other The criteria to merge in.
+    # @param [ Criteria | Hash ] other The criteria to merge in.
     #
     # @return [ Criteria ] The merged criteria.
     def merge!(other)
-      criteria = other.to_criteria
-      selector.merge!(criteria.selector)
-      options.merge!(criteria.options)
-      self.documents = criteria.documents.dup unless criteria.documents.empty?
-      self.scoping_options = criteria.scoping_options
-      self.inclusions = (inclusions + criteria.inclusions).uniq
+      other = self.class.from_hash(other) if other.is_a?(Hash)
+      selector.merge!(other.selector)
+      options.merge!(other.options)
+      self.documents = other.documents.dup unless other.documents.empty?
+      self.scoping_options = other.scoping_options
+      self.inclusions = (inclusions + other.inclusions).uniq
       self
     end
 
@@ -352,9 +370,11 @@ def respond_to?(name, include_private = false)
     #   criteria.to_criteria
     #
     # @return [ Criteria ] self.
+    # @deprecated
     def to_criteria
       self
     end
+    Mongoid.deprecate(self, :to_criteria)
 
     # Convert the criteria to a proc.
     #
@@ -452,6 +472,7 @@ def for_js(javascript, scope = {})
       end
       js_query(code)
     end
+    Mongoid.deprecate(self, :for_js)
 
     private
 

lib/mongoid/extensions/hash.rb

@@ -101,13 +101,11 @@ def resizable?
       #   { klass: Band, where: { name: "Depeche Mode" }.to_criteria
       #
       # @return [ Criteria ] The criteria.
+      # @deprecated
       def to_criteria
-        criteria = Criteria.new(delete(:klass) || delete("klass"))
-        each_pair do |method, args|
-          criteria = criteria.__send__(method, args)
-        end
-        criteria
+        Criteria.from_hash(self)
       end
+      Mongoid.deprecate(self, :to_criteria)
 
       private
 

spec/mongoid/criteria_spec.rb

@@ -1461,52 +1461,68 @@
     end
   end
 
-  describe "#merge!" do
+  describe '#merge!' do
+    let(:band) { Band.new }
+    let(:criteria) { Band.scoped.where(name: 'Depeche Mode').asc(:name) }
+    let(:association) { Band.relations['records'] }
+    subject(:merged) { criteria.merge!(other) }
 
-    let(:band) do
-      Band.new
-    end
+    context 'when merging a Criteria' do
+      let(:other) do
+        { klass: Band, includes: [:records] }
+      end
 
-    let(:criteria) do
-      Band.scoped.where(name: "Depeche Mode").asc(:name)
-    end
+      it 'merges the selector' do
+        expect(merged.selector).to eq({ 'name' => 'Depeche Mode' })
+      end
 
-    let(:mergeable) do
-      Band.includes(:records).tap do |crit|
-        crit.documents = [ band ]
+      it 'merges the options' do
+        expect(merged.options).to eq({ sort: { 'name' => 1 }})
       end
-    end
 
-    let(:association) do
-      Band.relations["records"]
-    end
+      it 'merges the scoping options' do
+        expect(merged.scoping_options).to eq([ nil, nil ])
+      end
 
-    let(:merged) do
-      criteria.merge!(mergeable)
-    end
+      it 'merges the inclusions' do
+        expect(merged.inclusions).to eq([ association ])
+      end
 
-    it "merges the selector" do
-      expect(merged.selector).to eq({ "name" => "Depeche Mode" })
+      it 'returns the same criteria' do
+        expect(merged).to equal(criteria)
+      end
     end
 
-    it "merges the options" do
-      expect(merged.options).to eq({ sort: { "name" => 1 }})
-    end
+    context 'when merging a Hash' do
+      let(:other) do
+        Band.includes(:records).tap do |crit|
+          crit.documents = [ band ]
+        end
+      end
 
-    it "merges the documents" do
-      expect(merged.documents).to eq([ band ])
-    end
+      it 'merges the selector' do
+        expect(merged.selector).to eq({ 'name' => 'Depeche Mode' })
+      end
 
-    it "merges the scoping options" do
-      expect(merged.scoping_options).to eq([ nil, nil ])
-    end
+      it 'merges the options' do
+        expect(merged.options).to eq({ sort: { 'name' => 1 }})
+      end
 
-    it "merges the inclusions" do
-      expect(merged.inclusions).to eq([ association ])
-    end
+      it 'merges the documents' do
+        expect(merged.documents).to eq([ band ])
+      end
+
+      it 'merges the scoping options' do
+        expect(merged.scoping_options).to eq([ nil, nil ])
+      end
 
-    it "returns the same criteria" do
-      expect(merged).to equal(criteria)
+      it 'merges the inclusions' do
+        expect(merged.inclusions).to eq([ association ])
+      end
+
+      it 'returns the same criteria' do
+        expect(merged).to equal(criteria)
+      end
     end
   end
 
@@ -2308,17 +2324,6 @@ def self.ages; self; end
     end
   end
 
-  describe "#to_criteria" do
-
-    let(:criteria) do
-      Band.all
-    end
-
-    it "returns self" do
-      expect(criteria.to_criteria).to eq(criteria)
-    end
-  end
-
   describe "#to_proc" do
 
     let(:criteria) do
@@ -3031,11 +3036,11 @@ def self.ages; self; end
     context "when the method exists on the criteria" do
 
       before do
-        expect(criteria).to receive(:to_criteria).and_call_original
+        expect(criteria).to receive(:only).and_call_original
       end
 
       it "calls the method on the criteria" do
-        expect(criteria.to_criteria).to eq(criteria)
+        expect(criteria.only).to eq(criteria)
       end
     end
 
@@ -3242,4 +3247,44 @@ def self.ages; self; end
       end
     end
   end
+
+  describe '.from_hash' do
+    subject(:criteria) { described_class.from_hash(hash) }
+
+    context 'when klass is specified' do
+      let(:hash) do
+        { klass: Band, where: { name: 'Songs Ohia' } }
+      end
+
+      it 'returns a criteria' do
+        expect(criteria).to be_a(Mongoid::Criteria)
+      end
+
+      it 'sets the klass' do
+        expect(criteria.klass).to eq(Band)
+      end
+
+      it 'sets the selector' do
+        expect(criteria.selector).to eq({ 'name' => 'Songs Ohia' })
+      end
+    end
+
+    context 'when klass is missing' do
+      let(:hash) do
+        { where: { name: 'Songs Ohia' } }
+      end
+
+      it 'returns a criteria' do
+        expect(criteria).to be_a(Mongoid::Criteria)
+      end
+
+      it 'has klass nil' do
+        expect(criteria.klass).to be_nil
+      end
+
+      it 'sets the selector' do
+        expect(criteria.selector).to eq({ 'name' => 'Songs Ohia' })
+      end
+    end
+  end
 end