check command response by default; better DB#command api

lib/mongo/collection.rb

@@ -452,7 +452,7 @@ module Mongo
       cmd.merge!(opts)
       cmd[:sort] = Mongo::Support.format_order_clause(opts[:sort]) if opts[:sort]
 
-      @db.command(cmd, false, true)['value']
+      @db.command(cmd)['value']
     end
 
     # Perform a map/reduce operation on the current collection.
@@ -536,7 +536,7 @@ module Mongo
         group_command['group']['finalize'] = finalize
       end
 
-      result = @db.command group_command
+      result = @db.command(group_command)
 
       if result["ok"] == 1
         result["retval"]

lib/mongo/connection.rb

@@ -219,7 +219,7 @@ module Mongo
     #
     # @return [Hash]
     def database_info
-      doc = self['admin'].command({:listDatabases => 1}, false, true)
+      doc = self['admin'].command({:listDatabases => 1})
       returning({}) do |info|
         doc['databases'].each { |db| info[db['name']] = db['sizeOnDisk'].to_i }
       end
@@ -283,12 +283,12 @@ module Mongo
         nonce_cmd = BSON::OrderedHash.new
         nonce_cmd[:copydbgetnonce] = 1
         nonce_cmd[:fromhost] = from_host
-        result = self["admin"].command(nonce_cmd, true, true)
+        result = self["admin"].command(nonce_cmd)
         oh[:nonce] = result["nonce"]
         oh[:username] = username
         oh[:key] = Mongo::Support.auth_key(username, password, oh[:nonce])
       end
-      self["admin"].command(oh, true, true)
+      self["admin"].command(oh)
     end
 
     # Increment and return the next available request id.
@@ -306,7 +306,7 @@ module Mongo
     #
     # @return [Hash]
     def server_info
-      self["admin"].command({:buildinfo => 1}, false, true)
+      self["admin"].command({:buildinfo => 1})
     end
 
     # Get the build version of the current server.
@@ -422,7 +422,7 @@ module Mongo
           socket.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
 
           # If we're connected to master, set the @host and @port
-          result = self['admin'].command({:ismaster => 1}, false, false, socket)
+          result = self['admin'].command({:ismaster => 1}, :check_response => false, :sock => socket)
           if result['ok'] == 1 && ((is_master = result['ismaster'] == 1) || @slave_ok)
             @host, @port = host, port
             apply_saved_authentication

lib/mongo/cursor.rb

@@ -40,6 +40,9 @@ module Mongo
 
       @selector   = convert_selector_for_query(options[:selector])
       @fields     = convert_fields_for_query(options[:fields])
+      if options[:admin]
+        warn "The admin option to Cursor#new has been deprecated. The cursor should now be passed the admin collection explicitly."
+      end
       @admin      = options[:admin]    || false
       @skip       = options[:skip]     || 0
       @limit      = options[:limit]    || 0
@@ -264,7 +267,6 @@ module Mongo
     def query_options_hash
       { :selector => @selector,
         :fields   => @fields,
-        :admin    => @admin,
         :skip     => @skip_num,
         :limit    => @limit_num,
         :order    => @order,

lib/mongo/db.rb

@@ -87,7 +87,7 @@ module Mongo
     #
     # @core authenticate authenticate-instance_method
     def authenticate(username, password, save_auth=true)
-      doc = command(:getnonce => 1)
+      doc = command(:getnonce => 1, :check_response => false)
       raise "error retrieving nonce: #{doc}" unless ok?(doc)
       nonce = doc['nonce']
 
@@ -96,7 +96,7 @@ module Mongo
       auth['user'] = username
       auth['nonce'] = nonce
       auth['key'] = Mongo::Support.auth_key(username, password, nonce)
-      if ok?(command(auth))
+      if ok?(self.command(auth, :check_response => false))
         if save_auth
           @connection.add_auth(@name, username, password)
         end
@@ -260,7 +260,7 @@ module Mongo
       cmd = BSON::OrderedHash.new
       cmd[:getlasterror] = 1
       cmd.merge!(opts) unless opts.empty?
-      doc = command(cmd)
+      doc = command(cmd, :check_response => false)
       raise MongoDBError, "error retrieving last error: #{doc.inspect}" unless ok?(doc)
       doc['err']
     end
@@ -359,7 +359,7 @@ module Mongo
       oh = BSON::OrderedHash.new
       oh[:renameCollection] = "#{@name}.#{from}"
       oh[:to] = "#{@name}.#{to}"
-      doc = command(oh, true)
+      doc = DB.new('admin', @connection).command(oh)
       ok?(doc) || raise(MongoDBError, "Error renaming collection: #{doc.inspect}")
     end
 
@@ -441,22 +441,31 @@ module Mongo
     # to see how it works.
     #
     # @param [OrderedHash, Hash] selector an OrderedHash, or a standard Hash with just one
-    # key, specifying the command to be performed.
+    # key, specifying the command to be performed. In Ruby 1.9, OrderedHash isn't necessary since
+    # hashes are ordered by default.
     #
     # @param [Boolean] admin If +true+, the command will be executed on the admin
-    # collection.
+    # collection. DEPRECATED.
     #
-    # @param [Boolean] check_response If +true+, will raise an exception if the
+    # @option opts [Boolean] :check_response (true) If +true+, raises an exception if the
     # command fails.
-    #
-    # @param [Socket] sock a socket to use. This is mainly for internal use.
+    # @option opts [Socket] :sock a socket to use for sending the command. This is mainly for internal use.
     #
     # @return [Hash]
     #
     # @core commands command_instance-method
-    def command(selector, admin=false, check_response=false, sock=nil)
+    def command(selector, opts={}, old_check_response=false, old_sock=nil)
+      if opts.is_a?(Hash)
+        check_response = opts[:check_response].nil? ? true : opts[:check_response]
+        sock           = opts[:sock]
+      else
+        warn "The options passed to DB#command should now be passed as hash keys; the admin option has been deprecated."
+        admin          = opts
+        check_response = old_check_response
+        sock           = old_sock
+      end
       raise MongoArgumentError, "command must be given a selector" unless selector.is_a?(Hash) && !selector.empty?
-      if selector.class.eql?(Hash) && selector.keys.length > 1
+      if selector.class.eql?(Hash) && selector.keys.length > 1 && RUBY_VERSION < '1.9'
         raise MongoArgumentError, "DB#command requires an OrderedHash when hash contains multiple keys"
       end
 

lib/mongo/gridfs/grid_io.rb

@@ -340,7 +340,7 @@ module Mongo
       md5_command            = BSON::OrderedHash.new
       md5_command['filemd5'] = @files_id
       md5_command['root']    = @fs_name
-      @server_md5 = @files.db.command(md5_command, false, true)['md5']
+      @server_md5 = @files.db.command(md5_command)['md5']
       if @safe
         @client_md5 = @local_md5.hexdigest
         if @local_md5 != @server_md5

test/db_test.rb

@@ -152,18 +152,27 @@ class DBTest < Test::Unit::TestCase
     assert @@db.logout
   end
 
+  def test_command
+    assert_raise OperationFailure do
+      @@db.command({:non_command => 1}, :check_response => true)
+    end
+
+    result = @@db.command({:non_command => 1}, :check_response => false)
+    assert_equal 0, result['ok'].to_i
+  end
+
   def test_error
     @@db.reset_error_history
     assert_nil @@db.error
     assert !@@db.error?
     assert_nil @@db.previous_error
 
-    @@db.send(:command, :forceerror => 1)
+    @@db.command({:forceerror => 1}, :check_response => false)
     assert @@db.error?
     assert_not_nil @@db.error
     assert_not_nil @@db.previous_error
 
-    @@db.send(:command, :forceerror => 1)
+    @@db.command({:forceerror => 1}, :check_response => false)
     assert @@db.error?
     assert @@db.error
     prev_error = @@db.previous_error
@@ -203,7 +212,7 @@ class DBTest < Test::Unit::TestCase
   def test_check_command_response
     command = {:forceerror => 1}
     assert_raise OperationFailure do 
-      @@db.command(command, false, true)
+      @@db.command(command)
     end
   end
 

test/unit/db_test.rb

@@ -22,8 +22,10 @@ class DBTest < Test::Unit::TestCase
       end
 
       should "raise an error if given a hash with more than one key" do
-        assert_raise MongoArgumentError do
-          @db.command(:buildinfo => 1, :somekey => 1)
+        if RUBY_VERSION < '1.9'
+          assert_raise MongoArgumentError do
+            @db.command(:buildinfo => 1, :somekey => 1)
+          end
         end
       end
 
@@ -73,13 +75,6 @@ class DBTest < Test::Unit::TestCase
         end
       end
 
-      should "raise an error if rename fails" do
-        @db.expects(:command).returns({})
-        assert_raise Mongo::MongoDBError do
-          @db.rename_collection("foo", "bar")
-        end
-      end
-
       should "raise an error if drop_index fails" do
         @db.expects(:command).returns({})
         assert_raise Mongo::MongoDBError do