RUBY-239 warn if bad options are passed to connection constructors

lib/mongo/connection.rb

@@ -33,6 +33,8 @@ class Connection
     Thread.abort_on_exception = true
 
     DEFAULT_PORT = 27017
+    GENERIC_OPTS = [:ssl, :auths, :pool_size, :pool_timeout, :timeout, :op_timeout, :connect_timeout, :safe, :logger, :connect]
+    CONNECTION_OPTS = [:slave_ok]
 
     mongo_thread_local_accessor :connections
 
@@ -97,10 +99,18 @@ def initialize(host=nil, port=nil, opts={})
 
       # Host and port of current master.
       @host = @port = nil
+
+      # Default maximum BSON object size
+      @max_bson_size = Mongo::DEFAULT_MAX_BSON_SIZE
+
+      # Lock for request ids.
+      @id_lock = Mutex.new
+
+      # Connection pool for primay node
+      @primary      = nil
+      @primary_pool = nil
 
-      # slave_ok can be true only if one node is specified
-      @slave_ok = opts[:slave_ok]
-
+      check_opts(opts)
       setup(opts)
     end
 
@@ -504,12 +514,24 @@ def checkin(socket)
     end
 
     protected
+
+    def valid_opts
+      GENERIC_OPTS + CONNECTION_OPTS
+    end
+
+    def check_opts(opts)
+      bad_opts = opts.keys.reject { |opt| valid_opts.include?(opt) }
 
-    # Generic initialization code.
-    def setup(opts)
-      # Default maximum BSON object size
-      @max_bson_size = Mongo::DEFAULT_MAX_BSON_SIZE
+      unless bad_opts.empty?
+        bad_opts.each {|opt| warn "#{opt} is not a valid option for #{self.class}"}
+      end
+    end
 
+    # Parse option hash
+    def setup(opts)
+      # slave_ok can be true only if one node is specified
+      @slave_ok = opts[:slave_ok]
+
       # Determine whether to use SSL.
       @ssl = opts.fetch(:ssl, false)
       if @ssl
@@ -521,9 +543,6 @@ def setup(opts)
       # Authentication objects
       @auths = opts.fetch(:auths, [])
 
-      # Lock for request ids.
-      @id_lock = Mutex.new
-
       # Pool size and timeout.
       @pool_size = opts[:pool_size] || 1
       if opts[:timeout]
@@ -540,10 +559,6 @@ def setup(opts)
 
       # Global safe option. This is false by default.
       @safe = opts[:safe] || false
-
-      # Connection pool for primay node
-      @primary      = nil
-      @primary_pool = nil
 
       @logger = opts.fetch(:logger, nil)
 

lib/mongo/repl_set_connection.rb

@@ -20,6 +20,8 @@ module Mongo
 
   # Instantiates and manages connections to a MongoDB replica set.
   class ReplSetConnection < Connection
+
+    REPL_SET_OPTS = [:read, :refresh_mode, :refresh_interval, :require_primary, :read_secondary, :rs_name]
 
     attr_reader :replica_set_name, :seeds, :refresh_interval, :refresh_mode,
       :refresh_version
@@ -101,55 +103,33 @@ def initialize(*args)
           seeds << seed
         end
       end
-
+      
       # TODO: add a method for replacing this list of node.
       @seeds.freeze
-
+      
       # Refresh
-      @refresh_mode = opts.fetch(:refresh_mode, false)
-      @refresh_interval = opts[:refresh_interval] || 90
       @last_refresh = Time.now
       @refresh_version = 0
 
       # No connection manager by default.
       @manager = nil
+
+      # Lock for request ids.
+      @id_lock = Mutex.new
+
       @pool_mutex = Mutex.new
-
-      if @refresh_mode == :async
-        warn ":async refresh mode has been deprecated. Refresh
-        mode will be disabled."
-      elsif ![:sync, false].include?(@refresh_mode)
-        raise MongoArgumentError,
-          "Refresh mode must be either :sync or false."
-      end
-
-      # Are we allowing reads from secondaries?
-      if opts[:read_secondary]
-        warn ":read_secondary options has now been deprecated and will " +
-          "be removed in driver v2.0. Use the :read option instead."
-        @read_secondary = opts.fetch(:read_secondary, false)
-        @read = :secondary
-      else
-        @read = opts.fetch(:read, :primary)
-        Mongo::Support.validate_read_preference(@read)
-      end
-
       @connected = false
-
-      # Replica set name
-      if opts[:rs_name]
-        warn ":rs_name option has been deprecated and will be removed in v2.0. " +
-          "Please use :name instead."
-        @replica_set_name = opts[:rs_name]
-      else
-        @replica_set_name = opts[:name]
-      end
-
-      # Require a primary node to connect?
-      @require_primary = opts.fetch(:require_primary, true)
-
+
+      @safe_mutex_lock = Mutex.new
+      @safe_mutexes = Hash.new {|hash, key| hash[key] = Mutex.new}
+
+      check_opts(opts)
       setup(opts)
     end
+
+    def valid_opts
+      GENERIC_OPTS + REPL_SET_OPTS
+    end
 
     def inspect
       "<Mongo::ReplSetConnection:0x#{self.object_id.to_s(16)} @seeds=#{@seeds.inspect} " +
@@ -466,10 +446,42 @@ def max_bson_size
 
     private
 
-    # Generic initialization code.
+    # Parse option hash
     def setup(opts)
-      @safe_mutex_lock = Mutex.new
-      @safe_mutexes = Hash.new {|hash, key| hash[key] = Mutex.new}
+      # Require a primary node to connect?
+      @require_primary = opts.fetch(:require_primary, true)
+
+      # Refresh
+      @refresh_mode = opts.fetch(:refresh_mode, false)
+      @refresh_interval = opts[:refresh_interval] || 90
+
+      if @refresh_mode == :async
+        warn ":async refresh mode has been deprecated. Refresh
+        mode will be disabled."
+      elsif ![:sync, false].include?(@refresh_mode)
+        raise MongoArgumentError,
+          "Refresh mode must be either :sync or false."
+      end
+
+      # Are we allowing reads from secondaries?
+      if opts[:read_secondary]
+        warn ":read_secondary options has now been deprecated and will " +
+          "be removed in driver v2.0. Use the :read option instead."
+        @read_secondary = opts.fetch(:read_secondary, false)
+        @read = :secondary
+      else
+        @read = opts.fetch(:read, :primary)
+        Mongo::Support.validate_read_preference(@read)
+      end
+
+      # Replica set name
+      if opts[:rs_name]
+        warn ":rs_name option has been deprecated and will be removed in v2.0. " +
+          "Please use :name instead."
+        @replica_set_name = opts[:rs_name]
+      else
+        @replica_set_name = opts[:name]
+      end
 
       opts[:connect_timeout] = opts[:connect_timeout] || 30
 

test/unit/connection_test.rb

@@ -26,6 +26,34 @@ class ConnectionTest < Test::Unit::TestCase
       should "default slave_ok to false" do
         assert !@conn.slave_ok?
       end
+
+      should "warn if invalid options are specified" do
+        conn = Connection.allocate
+        opts = {:connect => false}
+
+        ReplSetConnection::REPL_SET_OPTS.each do |opt|
+          conn.expects(:warn).with("#{opt} is not a valid option for #{conn.class}")
+          opts[opt] = true
+        end
+
+        args = ['localhost', 27017, opts]
+        conn.send(:initialize, *args)
+      end
+
+      context "given a replica set" do
+        should "warn if invalid options are specified" do
+          conn = ReplSetConnection.allocate
+          opts = {:connect => false}
+
+          Connection::CONNECTION_OPTS.each do |opt|
+            conn.expects(:warn).with("#{opt} is not a valid option for #{conn.class}")
+            opts[opt] = true
+          end
+
+          args = [['localhost:27017'], opts]
+          conn.send(:initialize, *args)
+        end
+      end
     end
 
     context "initializing with a mongodb uri" do