From 924a275ea3261c5cfb1151dbd0fd64f7bc019294 Mon Sep 17 00:00:00 2001 From: Kyle Banker Date: Tue, 18 May 2010 16:17:17 -0400 Subject: [PATCH] check command response by default; better DB#command api --- lib/mongo/collection.rb | 4 ++-- lib/mongo/connection.rb | 10 +++++----- lib/mongo/cursor.rb | 4 +++- lib/mongo/db.rb | 31 ++++++++++++++++++++----------- lib/mongo/gridfs/grid_io.rb | 2 +- test/db_test.rb | 15 ++++++++++++--- test/unit/db_test.rb | 13 ++++--------- 7 files changed, 47 insertions(+), 32 deletions(-) diff --git a/lib/mongo/collection.rb b/lib/mongo/collection.rb index 8792dc4..f99087c 100644 --- a/lib/mongo/collection.rb +++ b/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"] diff --git a/lib/mongo/connection.rb b/lib/mongo/connection.rb index b99eaf2..6236392 100644 --- a/lib/mongo/connection.rb +++ b/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 diff --git a/lib/mongo/cursor.rb b/lib/mongo/cursor.rb index fd583dd..a0284c6 100644 --- a/lib/mongo/cursor.rb +++ b/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, diff --git a/lib/mongo/db.rb b/lib/mongo/db.rb index 3969256..9573d0c 100644 --- a/lib/mongo/db.rb +++ b/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 diff --git a/lib/mongo/gridfs/grid_io.rb b/lib/mongo/gridfs/grid_io.rb index 0eb22ca..6f6b8ee 100644 --- a/lib/mongo/gridfs/grid_io.rb +++ b/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 diff --git a/test/db_test.rb b/test/db_test.rb index 51ba360..7667d1b 100644 --- a/test/db_test.rb +++ b/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 diff --git a/test/unit/db_test.rb b/test/unit/db_test.rb index cc29be9..b262589 100644 --- a/test/unit/db_test.rb +++ b/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