MONGOID-5665 [Monkey Patch Removal] Remove Object#__find_args__ (#5706)

* Remove Object#__find_args__. This primarily used in Criteria::Findable where it is now a private method.

2 points to note:
- __find_args__ was mistakenly called in Atomic#unset method. Although technically __find_args__ does a similar thing as what #unset requires for its argument preparation, this is pure coincidence and its a kludge to use find_args here. So I've inlined the code and made a new private method.
- __find_args__ for Range previously called to_a. This is actually a bug / DOS risk, because ranges of strings tend to explode when you use to_a. As an example, ('64e0'..'64ff').to_a.size => 9320, and it gets much worse for 24-char BSON ids! Interestingly however, MongoDB itself can handle ranges of ids gracefully, so I am now just passing them into the DB as-is and it magically works--I've added tests.

* Fix specs

* Add test to cover #find for nested arrays, which was pre-existing behavior.

* deprecate __find_args__ instead of removing it

* Set#resizable? has to be true for this to work

---------

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

Author: johnnyshields

lib/mongoid/contextual/atomic.rb

@@ -169,17 +169,14 @@ def set(sets)
       # @example Unset the field on the matches.
       #   context.unset(:name)
       #
-      # @param [ [ String | Symbol | Array<String | Symbol> | Hash ]... ] *args
-      #   The name(s) of the field(s) to unset.
-      #   If a Hash is specified, its keys will be used irrespective of what
-      #   each key's value is, even if the value is nil or false.
+      # @param [ [ String | Symbol | Array<String | Symbol> | Hash ]... ] *unsets
+      #   The name(s) of the field(s) to unset. If a Hash is specified,
+      #   its keys will be used irrespective of value, even if the value
+      #   is nil or false.
       #
       # @return [ nil ] Nil.
-      def unset(*args)
-        fields = args.map { |a| a.is_a?(Hash) ? a.keys : a }
-                     .__find_args__
-                     .map { |f| [database_field_name(f), true] }
-        view.update_many("$unset" => Hash[fields])
+      def unset(*unsets)
+        view.update_many('$unset' => collect_unset_operations(unsets))
       end
 
       # Performs an atomic $min update operation on the given field or fields.
@@ -247,6 +244,25 @@ def collect_each_operations(ops)
           operations[database_field_name(field)] = { "$each" => Array.wrap(value).mongoize }
         end
       end
+
+      # Builds the selector an atomic $unset operation from arguments.
+      #
+      # @example Prepare selector from array.
+      #   context.collect_unset_operations([:name, :age])
+      #   #=> { "name" => true, "age" => true }
+      #
+      # @example Prepare selector from hash.
+      #   context.collect_unset_operations({ name: 1 }, { age: 1 })
+      #   #=> { "name" => true, "age" => true }
+      #
+      # @param [ String | Symbol | Array<String | Symbol> | Hash ] ops
+      #   The name(s) of the field(s) to unset.
+      #
+      # @return [ Hash ] The selector for the atomic $unset operation.
+      def collect_unset_operations(ops)
+        ops.map { |op| op.is_a?(Hash) ? op.keys : op }.flatten
+           .map { |field| [database_field_name(field), true] }.to_h
+      end
     end
   end
 end

lib/mongoid/criteria/findable.rb

@@ -41,7 +41,7 @@ def execute_or_raise(ids, multi)
       #
       # @return [ Document | Array<Document> ] The matching document(s).
       def find(*args)
-        ids = args.__find_args__
+        ids = prepare_ids_for_find(args)
         raise_invalid if ids.any?(&:nil?)
         for_ids(ids).execute_or_raise(ids, multi_args?(args))
       end
@@ -134,6 +134,27 @@ def mongoize_ids(ids)
         end
       end
 
+      # Convert args to the +#find+ method into a flat array of ids.
+      #
+      # @example Get the ids.
+      #   prepare_ids_for_find([ 1, [ 2, 3 ] ])
+      #
+      # @param [ Array<Object> ] args The arguments.
+      #
+      # @return [ Array ] The array of ids.
+      def prepare_ids_for_find(args)
+        args.flat_map do |arg|
+          case arg
+          when Array, Set
+            prepare_ids_for_find(arg)
+          when Range
+            arg.begin&.numeric? && arg.end&.numeric? ? arg.to_a : arg
+          else
+            arg
+          end
+        end.uniq(&:to_s)
+      end
+
       # Indicates whether the given arguments array is a list of values.
       # Used by the +find+ method to determine whether to return an array
       # or single value.

lib/mongoid/extensions/array.rb

@@ -23,9 +23,11 @@ def __evolve_object_id__
       #   [ 1, 2, 3 ].__find_args__
       #
       # @return [ Array ] The array of args.
+      # @deprecated
       def __find_args__
         flat_map{ |a| a.__find_args__ }.uniq{ |a| a.to_s }
       end
+      Mongoid.deprecate(self, :__find_args__)
 
       # Mongoize the array into an array of object ids.
       #

lib/mongoid/extensions/object.rb

@@ -24,9 +24,11 @@ def __evolve_object_id__
       #   object.__find_args__
       #
       # @return [ Object ] self.
+      # @deprecated
       def __find_args__
         self
       end
+      Mongoid.deprecate(self, :__find_args__)
 
       # Mongoize a plain object into a time.
       #

lib/mongoid/extensions/range.rb

@@ -3,19 +3,23 @@
 
 module Mongoid
   module Extensions
-
     # Adds type-casting behavior to Range class.
     module Range
+      def self.included(base)
+        base.extend(ClassMethods)
+      end
 
       # Get the range as arguments for a find.
       #
       # @example Get the range as find args.
       #   range.__find_args__
       #
       # @return [ Array ] The range as an array.
+      # @deprecated
       def __find_args__
         to_a
       end
+      Mongoid.deprecate(self, :__find_args__)
 
       # Turn the object from the ruby type we deal with to a Mongo friendly
       # type.
@@ -39,7 +43,6 @@ def resizable?
       end
 
       module ClassMethods
-
         # Convert the object from its mongo friendly ruby type to this type.
         #
         # @example Demongoize the object.
@@ -107,5 +110,4 @@ def __mongoize_range__(object)
   end
 end
 
-::Range.__send__(:include, Mongoid::Extensions::Range)
-::Range.extend(Mongoid::Extensions::Range::ClassMethods)
+Range.include(Mongoid::Extensions::Range)

lib/mongoid/extensions/set.rb

@@ -6,7 +6,6 @@ module Extensions
 
     # Adds type-casting behavior to Set class.
     module Set
-
       # Turn the object from the ruby type we deal with to a Mongo friendly
       # type.
       #
@@ -18,8 +17,17 @@ def mongoize
         ::Set.mongoize(self)
       end
 
-      module ClassMethods
+      # Returns whether the object's size can be changed.
+      #
+      # @example Is the object resizable?
+      #   object.resizable?
+      #
+      # @return [ true ] true.
+      def resizable?
+        true
+      end
 
+      module ClassMethods
         # Convert the object from its mongo friendly ruby type to this type.
         #
         # @example Demongoize the object.

spec/mongoid/criteria/findable_spec.rb

@@ -260,6 +260,196 @@
         end
       end
 
+      context "when providing nested arrays of ids" do
+
+        let!(:band_two) do
+          Band.create!(name: "Tool")
+        end
+
+        context "when all ids match" do
+
+          let(:found) do
+            Band.find([ [ band.id ], [ [ band_two.id ] ] ])
+          end
+
+          it "contains the first match" do
+            expect(found).to include(band)
+          end
+
+          it "contains the second match" do
+            expect(found).to include(band_two)
+          end
+
+          context "when ids are duplicates" do
+
+            let(:found) do
+              Band.find([ [ band.id ], [ [ band.id ] ] ])
+            end
+
+            it "contains only the first match" do
+              expect(found).to eq([band])
+            end
+          end
+        end
+
+        context "when any id does not match" do
+
+          context "when raising a not found error" do
+            config_override :raise_not_found_error, true
+
+            let(:found) do
+              Band.find([ [ band.id ], [ [ BSON::ObjectId.new ] ] ])
+            end
+
+            it "raises an error" do
+              expect {
+                found
+              }.to raise_error(Mongoid::Errors::DocumentNotFound, /Document\(s\) not found for class Band with id\(s\)/)
+            end
+          end
+
+          context "when raising no error" do
+            config_override :raise_not_found_error, false
+
+            let(:found) do
+              Band.find([ [ band.id ], [ [ BSON::ObjectId.new ] ] ])
+            end
+
+            it "returns only the matching documents" do
+              expect(found).to eq([ band ])
+            end
+          end
+        end
+      end
+
+      context "when providing a Set of ids" do
+
+        let!(:band_two) do
+          Band.create!(name: "Tool")
+        end
+
+        context "when all ids match" do
+          let(:found) do
+            Band.find(Set[ band.id, band_two.id ])
+          end
+
+          it "contains the first match" do
+            expect(found).to include(band)
+          end
+
+          it "contains the second match" do
+            expect(found).to include(band_two)
+          end
+        end
+
+        context "when any id does not match" do
+
+          context "when raising a not found error" do
+            config_override :raise_not_found_error, true
+
+            let(:found) do
+              Band.find(Set[ band.id, BSON::ObjectId.new ])
+            end
+
+            it "raises an error" do
+              expect {
+                found
+              }.to raise_error(Mongoid::Errors::DocumentNotFound, /Document\(s\) not found for class Band with id\(s\)/)
+            end
+          end
+
+          context "when raising no error" do
+            config_override :raise_not_found_error, false
+
+            let(:found) do
+              Band.find(Set[ band.id, BSON::ObjectId.new ])
+            end
+
+            it "returns only the matching documents" do
+              expect(found).to eq([ band ])
+            end
+          end
+        end
+      end
+
+      context "when providing a Range of ids" do
+
+        let!(:band_two) do
+          Band.create!(name: "Tool")
+        end
+
+        context "when all ids match" do
+          let(:found) do
+            Band.find(band.id.to_s..band_two.id.to_s)
+          end
+
+          it "contains the first match" do
+            expect(found).to include(band)
+          end
+
+          it "contains the second match" do
+            expect(found).to include(band_two)
+          end
+
+
+          context "when any id does not match" do
+
+            context "when raising a not found error" do
+              config_override :raise_not_found_error, true
+
+              let(:found) do
+                Band.find(band_two.id.to_s..BSON::ObjectId.new)
+              end
+
+              it "does not raise error and returns only the matching documents" do
+                expect(found).to eq([ band_two ])
+              end
+            end
+
+            context "when raising no error" do
+              config_override :raise_not_found_error, false
+
+              let(:found) do
+                Band.find(band_two.id.to_s..BSON::ObjectId.new)
+              end
+
+              it "returns only the matching documents" do
+                expect(found).to eq([ band_two ])
+              end
+            end
+          end
+        end
+
+        context "when all ids do not match" do
+
+          context "when raising a not found error" do
+            config_override :raise_not_found_error, true
+
+            let(:found) do
+              Band.find(BSON::ObjectId.new..BSON::ObjectId.new)
+            end
+
+            it "raises an error" do
+              expect {
+                found
+              }.to raise_error(Mongoid::Errors::DocumentNotFound, /Document\(s\) not found for class Band with id\(s\)/)
+            end
+          end
+
+          context "when raising no error" do
+            config_override :raise_not_found_error, false
+
+            let(:found) do
+              Band.find(BSON::ObjectId.new..BSON::ObjectId.new)
+            end
+
+            it "returns only the matching documents" do
+              expect(found).to eq([])
+            end
+          end
+        end
+      end
+
       context "when providing a single id as extended json" do
 
         context "when the id matches" do

spec/mongoid/extensions/object_spec.rb

@@ -16,13 +16,6 @@
     end
   end
 
-  describe "#__find_args__" do
-
-    it "returns self" do
-      expect(object.__find_args__).to eq(object)
-    end
-  end
-
   describe "#__mongoize_object_id__" do
 
     it "returns self" do