Refactored most of the places that touch the database to be more cons…

…istent. Everything now takes options and collection.find_one is used in places where only one document is being found instead of find with a limit. Fixes mongomapper#91.

lib/mongo_mapper.rb

@@ -58,7 +58,7 @@ def dynamic_find(finder, args)
         attributes[attr] = args[index]
       end
 
-      options = args.extract_options!.deep_merge(attributes)
+      options = args.extract_options!.merge(attributes)
       result  = find(finder.finder, options)
 
       if result.nil?

lib/mongo_mapper/associations/many_documents_proxy.rb

@@ -27,8 +27,8 @@ def last(options={})
         find(:last, scoped_options(options))
       end
 
-      def count(conditions={})
-        klass.count(conditions.deep_merge(scoped_conditions))
+      def count(options={})
+        klass.count(options.merge(scoped_conditions))
       end
 
       def replace(docs)
@@ -57,13 +57,13 @@ def create(attrs={})
         doc
       end
 
-      def destroy_all(conditions={})
-        all(conditions).map(&:destroy)
+      def destroy_all(options={})
+        all(options).map(&:destroy)
         reset
       end
 
-      def delete_all(conditions={})
-        klass.delete_all(conditions.deep_merge(scoped_conditions))
+      def delete_all(options={})
+        klass.delete_all(options.merge(scoped_conditions))
         reset
       end
 
@@ -91,7 +91,7 @@ def scoped_conditions
         end
 
         def scoped_options(options)
-          options.deep_merge(scoped_conditions)
+          options.merge(scoped_conditions)
         end
 
         def find_target

lib/mongo_mapper/document.rb

@@ -55,7 +55,7 @@ def find(*args)
               when 0
                 raise DocumentNotFound, "Couldn't find without an ID"
               when 1
-                find_one(args[0], options)
+                find_one!(options.merge({:_id => args[0]}))
               else
                 find_some(args, options)
             end
@@ -66,47 +66,36 @@ def paginate(options)
         per_page      = options.delete(:per_page) || self.per_page
         page          = options.delete(:page)
         total_entries = count(options)
-        collection    = Pagination::PaginationProxy.new(total_entries, page, per_page)
+        pagination    = Pagination::PaginationProxy.new(total_entries, page, per_page)
 
-        options[:limit] = collection.limit
-        options[:skip]  = collection.skip
-
-        collection.subject = find_every(options)
-        collection
+        options.merge!(:limit => pagination.limit, :skip => pagination.skip)
+        pagination.subject = find_every(options)
+        pagination
       end
 
       def first(options={})
-        options.merge!(:limit => 1)
-        find_every(options)[0]
+        find_one(options)
       end
 
       def last(options={})
-        if options[:order].blank?
-          raise ':order option must be provided when using last'
-        end
-
-        options.merge!(:limit => 1)
-        options[:order] = invert_order_clause(options[:order])
-        find_every(options)[0]
+        raise ':order option must be provided when using last' if options[:order].blank?
+        find_one(options.merge(:order => invert_order_clause(options[:order])))
       end
 
       def all(options={})
         find_every(options)
       end
 
       def find_by_id(id)
-        criteria = FinderOptions.new(self, :_id => id).criteria
-        if doc = collection.find_one(criteria)
-          new(doc)
-        end
+        find_one(:_id => id)
       end
 
-      def count(conditions={})
-        collection.find(FinderOptions.new(self, conditions).criteria).count
+      def count(options={})
+        collection.find(to_criteria(options)).count
       end
 
-      def exists?(conditions={})
-        !count(conditions).zero?
+      def exists?(options={})
+        !count(options).zero?
       end
 
       def create(*docs)
@@ -123,8 +112,7 @@ def create!(*docs)
       # For updating multiple documents at once:
       #   Person.update({'1' => {:foo => 'bar'}, '2' => {:baz => 'wick'}})
       def update(*args)
-        updating_multiple = args.length == 1
-        if updating_multiple
+        if args.length == 1
           update_multiple(args[0])
         else
           id, attributes = args
@@ -133,21 +121,19 @@ def update(*args)
       end
 
       def delete(*ids)
-        criteria = FinderOptions.new(self, :_id => ids.flatten).criteria
-        collection.remove(criteria)
+        collection.remove(to_criteria(:_id => ids.flatten))
       end
 
-      def delete_all(conditions={})
-        criteria = FinderOptions.new(self, conditions).criteria
-        collection.remove(criteria)
+      def delete_all(options={})
+        collection.remove(to_criteria(options))
       end
 
       def destroy(*ids)
         find_some(ids.flatten).each(&:destroy)
       end
 
-      def destroy_all(conditions={})
-        all(conditions).each(&:destroy)
+      def destroy_all(options={})
+        all(options).each(&:destroy)
       end
 
       def connection(mongo_connection=nil)
@@ -187,7 +173,6 @@ def collection
       def timestamps!
         key :created_at, Time
         key :updated_at, Time
-
         class_eval { before_save :update_timestamps }
       end
 
@@ -212,52 +197,58 @@ def method_missing(method, *args)
         end
 
       private
-        # Initializes each document and yields each initialized document
+        def create_indexes_for(key)
+          ensure_index key.name if key.options[:index]
+        end
+
         def initialize_each(*docs)
           instances = []
           docs = [{}] if docs.blank?
           docs.flatten.each do |attrs|
-            doc = new(attrs)
+            doc = initialize_doc(attrs)
             yield(doc)
             instances << doc
           end
           instances.size == 1 ? instances[0] : instances
         end
-
-        def create_indexes_for(key)
-          ensure_index key.name if key.options[:index]
+
+        def initialize_doc(doc)
+          begin
+            klass = doc['_type'].present? ? doc['_type'].constantize : self
+            klass.new(doc)
+          rescue NameError
+            new(doc)
+          end
         end
 
         def find_every(options)
-          criteria, options = FinderOptions.new(self, options).to_a
+          criteria, options = to_finder_options(options)
           collection.find(criteria, options).to_a.map do |doc|
-            begin
-              klass = doc['_type'].present? ? doc['_type'].constantize : self
-              klass.new(doc)
-            rescue NameError
-              new(doc)
-            end
+            initialize_doc(doc)
           end
         end
 
         def find_some(ids, options={})
-          ids = ids.flatten.compact.uniq
-          documents = find_every(options.deep_merge(:_id => ids))
+          ids       = ids.flatten.compact.uniq
+          documents = find_every(options.merge(:_id => ids))
 
           if ids.size == documents.size
             documents
           else
             raise DocumentNotFound, "Couldn't find all of the ids (#{ids.to_sentence}). Found #{documents.size}, but was expecting #{ids.size}"
           end
         end
-
-        def find_one(id, options={})
-          if doc = find_every(options.deep_merge(:_id => id)).first
-            doc
-          else
-            raise DocumentNotFound, "Document with id of #{id} does not exist in collection named #{collection.name}"
+
+        def find_one(options={})
+          criteria, options = to_finder_options(options)
+          if doc = collection.find_one(criteria, options)
+            initialize_doc(doc)
           end
         end
+
+        def find_one!(options={})
+          find_one(options) || raise(DocumentNotFound, "Document match #{options.inspect} does not exist in #{collection.name} collection")
+        end
 
         def invert_order_clause(order)
           order.split(',').map do |order_segment| 
@@ -290,6 +281,14 @@ def update_multiple(docs)
           docs.each_pair { |id, attrs| instances << update(id, attrs) }
           instances
         end
+
+        def to_criteria(options={})
+          FinderOptions.new(self, options).criteria
+        end
+
+        def to_finder_options(options={})
+          FinderOptions.new(self, options).to_a
+        end
     end
 
     module InstanceMethods      
@@ -311,8 +310,7 @@ def save!
 
       def destroy
         return false if frozen?
-        criteria = FinderOptions.new(self.class, :_id => id).criteria
-        collection.remove(criteria) unless new?
+        self.class.delete(id) unless new?
         freeze
       end
 

test/functional/test_document.rb

@@ -298,7 +298,9 @@ def setup
         end
 
         should "raise error if document not found" do
-          lambda { @document.find(123) }.should raise_error(MongoMapper::DocumentNotFound)
+          lambda {
+            @document.find(123)
+          }.should raise_error(MongoMapper::DocumentNotFound)
         end
       end