Diverting all BSON::ObjectId conversion calls through

BSON::ObjectId.convert:

- This now handles all the conversions, for every case.
- Allows proper String -> ObjectId conversions in the criteria methods
  that pass through #update_selector: all_in, any_in, any_of, and,
  where, excludes, not_in.
- Fixes #370.
- Fixes #395.
- Fixes #372.

lib/mongoid/criteria.rb

@@ -320,7 +320,8 @@ def initialize_copy(other)
     # <tt>criteria.update_selector({ :field => "value" }, "$in")</tt>
     def update_selector(attributes, operator)
       clone.tap do |crit|
-        attributes.each do |key, value|
+        converted = BSON::ObjectId.convert(klass, attributes || {})
+        converted.each do |key, value|
           unless crit.selector[key]
             crit.selector[key] = { operator => value }
           else

lib/mongoid/criterion/inclusion.rb

@@ -50,7 +50,8 @@ def and(selector = nil)
       def any_of(*args)
         clone.tap do |crit|
           criterion = @selector["$or"] || []
-          expanded = args.flatten.collect(&:expand_complex_criteria)
+          converted = BSON::ObjectId.convert(klass, args.flatten)
+          expanded = converted.collect(&:expand_complex_criteria)
           crit.selector["$or"] = criterion.concat(expanded)
         end
       end
@@ -140,7 +141,8 @@ def where(selector = nil)
         clone.tap do |crit|
           selector = case selector
             when String then {"$where" => selector}
-            else selector ? selector.expand_complex_criteria : {}
+            else
+              BSON::ObjectId.convert(klass, selector || {}).expand_complex_criteria
           end
 
           selector.each_pair do |key, value|

lib/mongoid/criterion/optional.rb

@@ -106,12 +106,12 @@ def id(*ids)
         ids.flatten!
         if ids.size > 1
           any_in(
-            :_id => ::BSON::ObjectId.cast!(klass, ids, klass.primary_key.nil?)
+            :_id => ::BSON::ObjectId.convert(klass, ids)
           )
         else
           clone.tap do |crit|
             crit.selector[:_id] =
-              ::BSON::ObjectId.cast!(klass, ids.first, klass.primary_key.nil?)
+              ::BSON::ObjectId.convert(klass, ids.first)
           end
         end
       end

lib/mongoid/extensions/object_id/conversions.rb

@@ -2,8 +2,21 @@
 module Mongoid #:nodoc:
   module Extensions #:nodoc:
     module ObjectId #:nodoc:
-      module Conversions #:nodoc:
 
+      # Provides conversions to and from BSON::ObjectIds and Strings, Arrays,
+      # and Hashes.
+      module Conversions
+
+        # Set the BSON::ObjectId value.
+        #
+        # @example Set the value.
+        #   BSON::ObjectId.set("4c52c439931a90ab29000003")
+        #
+        # @param [ String, BSON::ObjectId ] value The value to set.
+        #
+        # @return [ BSON::ObjectId ] The set value.
+        #
+        # @since 1.0
         def set(value)
           if value.is_a?(::String)
             BSON::ObjectId.from_string(value) unless value.blank?
@@ -12,41 +25,58 @@ def set(value)
           end
         end
 
+        # Get the BSON::ObjectId value.
+        #
+        # @example Get the value.
+        #   BSON::ObjectId.set(BSON::ObjectId.new)
+        #
+        # @param [ BSON::ObjectId ] value The value to get.
+        #
+        # @return [ BSON::ObjectId ] The value.
+        #
+        # @since 1.0
         def get(value)
           value
         end
 
-        # If the document is using BSON::ObjectIds the convert the argument to
-        # either an object id or an array of them if the supplied argument is an
-        # Array. Otherwise just return.
-        #
-        # Options:
-        #  args: A +String+ or an +Array+ convert to +BSON::ObjectId+
-        #  cast: A +Boolean+ define if we can or not cast to BSON::ObjectId.
-        #        If false, we use the default type of args
-        #
-        # Example:
-        #
-        # <tt>Mongoid.cast_ids!("4ab2bc4b8ad548971900005c", true)</tt>
-        # <tt>Mongoid.cast_ids!(["4ab2bc4b8ad548971900005c"])</tt>
-        #
-        # Returns:
-        #
-        # If using object ids:
-        #   An +Array+ of +BSON::ObjectId+ of each element if params is an +Array+
-        #   A +BSON::ObjectId+ from params if params is +String+
-        # Otherwise:
-        #   <tt>args</tt>
-        def cast!(klass, args, cast = true)
-          if !klass.using_object_ids? || args.is_a?(::BSON::ObjectId) || !cast
-            return args
-          end
-          if args.is_a?(::String)
-            ::BSON::ObjectId.from_string(args)
-          elsif args.is_a?(::Array)
-            args.map{ |a|
-              a.is_a?(::BSON::ObjectId) ? a : ::BSON::ObjectId(a)
-            }
+        # Convert the supplied arguments to object ids based on the class
+        # settings.
+        #
+        # @example Convert a string to an object id
+        #   BSON::ObjectId.convert(Person, "4c52c439931a90ab29000003")
+        #
+        # @example Convert an array of strings to object ids.
+        #   BSON::ObjectId.convert(Person, [ "4c52c439931a90ab29000003" ])
+        #
+        # @example Convert a hash of id strings to object ids.
+        #   BSON::ObjectId.convert(Person, { :_id => "4c52c439931a90ab29000003" })
+        #
+        # @param [ Class ] klass The class to convert the ids for.
+        # @param [ Object, Array, Hash ] args The object to convert.
+        #
+        # @raise BSON::InvalidObjectId If using object ids and passed bad
+        #   strings.
+        #
+        # @return [ BSON::ObjectId, Array, Hash ] The converted object ids.
+        #
+        # @since 2.0.0.rc.7
+        def convert(klass, args)
+          return args if args.is_a?(BSON::ObjectId) || !klass.using_object_ids?
+          case args
+          when ::String
+            BSON::ObjectId.from_string(args)
+          when ::Array
+            args.map do |arg|
+              convert(klass, arg)
+            end
+          when ::Hash
+            args.tap do |hash|
+              args.each_pair do |key, value|
+                begin
+                  args[key] = convert(klass, value)
+                rescue BSON::InvalidObjectId; end
+              end
+            end
           else
             args
           end

lib/mongoid/relations/constraint.rb

@@ -35,10 +35,7 @@ def initialize(metadata)
       # @since 2.0.0.rc.7
       def convert(object)
         return object if metadata.polymorphic?
-        if metadata.klass.using_object_ids?
-          return BSON::ObjectId.cast!(metadata.klass, object)
-        end
-        object
+        BSON::ObjectId.convert(metadata.klass, object)
       end
     end
   end

spec/functional/mongoid/criterion/inclusion_spec.rb

@@ -6,6 +6,24 @@
     Person.delete_all
   end
 
+  describe "#all_in" do
+
+    context "when providing string ids" do
+
+      let!(:person) do
+        Person.create(:ssn => "444-44-4444")
+      end
+
+      let(:from_db) do
+        Person.all_in(:_id => [ person.id.to_s ])
+      end
+
+      it "returns the matching documents" do
+        from_db.should == [ person ]
+      end
+    end
+  end
+
   describe "#any_in" do
 
     context "when the field value is nil" do
@@ -35,14 +53,29 @@
       context "when searching for any value" do
 
         let(:from_db) do
-          Person.criteria.in(:terms => [ true, false, nil ])
+          Person.any_in(:terms => [ true, false, nil ])
         end
 
         it "returns the matching documents" do
           from_db.should == [ person ]
         end
       end
     end
+
+    context "when providing string ids" do
+
+      let!(:person) do
+        Person.create(:ssn => "444-44-4444")
+      end
+
+      let(:from_db) do
+        Person.any_in(:_id => [ person.id.to_s ])
+      end
+
+      it "returns the matching documents" do
+        from_db.should == [ person ]
+      end
+    end
   end
 
   describe "#any_of" do
@@ -80,6 +113,23 @@
         from_db.should == [ person_two, person_three ]
       end
     end
+
+    context "when using object ids" do
+
+      context "when provided strings as params" do
+
+        let(:from_db) do
+          Person.any_of(
+            { :_id => person_one.id.to_s },
+            { :_id => person_two.id.to_s }
+          )
+        end
+
+        it "returns the matching documents" do
+          from_db.should == [ person_one, person_two ]
+        end
+      end
+    end
   end
 
   describe "#find" do
@@ -200,6 +250,20 @@
       )
     end
 
+    context "when providing string object ids" do
+
+      context "when providing a single id" do
+
+        let(:from_db) do
+          Person.where(:_id => person.id.to_s).first
+        end
+
+        it "returns the matching documents" do
+          from_db.should == person
+        end
+      end
+    end
+
     context "chaining multiple wheres" do
 
       context "when chaining on the same key" do

spec/unit/mongoid/extensions/hash/criteria_helpers_spec.rb

@@ -13,7 +13,5 @@
       hash = {:age.gt => 40, :age.lt => 45}
       hash.expand_complex_criteria.should == {:age => {"$gt" => 40, "$lt" => 45}}
     end
-
   end
-
 end