Don't allow static slave_ok opt on Server creation

Author: durran

lib/mongo/client.rb

@@ -21,6 +21,12 @@ module Mongo
   class Client
     extend Forwardable
 
+    # The options that do not affect the behaviour of a cluster and its
+    # subcomponents.
+    #
+    # @since 2.1.0
+    CRUD_OPTIONS = [ :database, :read, :write ].freeze
+
     # @return [ Mongo::Cluster ] cluster The cluster of servers for the client.
     attr_reader :cluster
 
@@ -203,8 +209,9 @@ def with(new_options = {})
       clone.tap do |client|
         client.options.update(new_options)
         Database.create(client)
-        # We can't use the same cluster if authentication details have changed.
-        if new_options[:user] || new_options[:password]
+        # We can't use the same cluster if some options that would affect it
+        # have changed.
+        if cluster_modifying?(new_options)
           Cluster.create(client)
         end
       end
@@ -244,9 +251,7 @@ def database_names
     #
     # @since 2.0.5
     def list_databases
-      use(Database::ADMIN).command(
-        listDatabases: 1
-      ).first['databases']
+      use(Database::ADMIN).command(listDatabases: 1).first['databases']
     end
 
     private
@@ -270,5 +275,14 @@ def initialize_copy(original)
       @read_preference = nil
       @write_concern = nil
     end
+
+    def cluster_modifying?(new_options)
+      cluster_options = new_options.reject do |name|
+        CRUD_OPTIONS.include?(name)
+      end
+      cluster_options.any? do |name, value|
+        options[name] != value
+      end
+    end
   end
 end

lib/mongo/cluster.rb

@@ -70,8 +70,7 @@ def add(host)
         if addition_allowed?(address)
           log_debug([ "Adding #{address.to_s} to the cluster." ])
           addresses.push(address)
-          server = Server.new(address, event_listeners,
-                              options.merge(slave_ok: @slave_ok))
+          server = Server.new(address, self, event_listeners, options)
           @servers.push(server)
           server
         end
@@ -93,7 +92,6 @@ def initialize(seeds, options = {})
       @event_listeners = Event::Listeners.new
       @options = options.freeze
       @topology = Topology.initial(seeds, options)
-      @slave_ok = @topology.single? unless options[:read]
 
       subscribe_to(Event::SERVER_ADDED, Event::ServerAdded.new(self))
       subscribe_to(Event::SERVER_REMOVED, Event::ServerRemoved.new(self))

lib/mongo/operation/read_preferrable.rb

@@ -37,7 +37,7 @@ def update_selector(context)
       end
 
       def update_options(context)
-        if context.slave_ok? || (!context.mongos? && read.slave_ok?)
+        if context.cluster.single? || (!context.mongos? && read.slave_ok?)
           options.dup.tap do |opts|
             (opts[:flags] ||= []) << SLAVE_OK
           end

lib/mongo/server.rb

@@ -31,6 +31,9 @@ class Server
     # @return [ String ] The configured address for the server.
     attr_reader :address
 
+    # @return [ Cluster ] cluster The server cluster.
+    attr_reader :cluster
+
     # @return [ Monitor ] monitor The server monitor.
     attr_reader :monitor
 
@@ -105,15 +108,17 @@ def disconnect!
     # subscribe to the appropriate events.
     #
     # @example Initialize the server.
-    #   Mongo::Server.new('127.0.0.1:27017', listeners)
+    #   Mongo::Server.new('127.0.0.1:27017', cluster, listeners)
     #
     # @param [ Address ] address The host:port address to connect to.
+    # @param [ Cluster ] cluster  The cluster the server belongs to.
     # @param [ Event::Listeners ] event_listeners The event listeners.
     # @param [ Hash ] options The server options.
     #
     # @since 2.0.0
-    def initialize(address, event_listeners, options = {})
+    def initialize(address, cluster, event_listeners, options = {})
       @address = address
+      @cluster = cluster
       @options = options.freeze
       @monitor = Monitor.new(address, event_listeners, options)
       monitor.scan!
@@ -159,17 +164,5 @@ def matches_tag_set?(tag_set)
         tags[k] && tags[k] == tag_set[k]
       end
     end
-
-    # Whether the slaveOk bit must be set for all queries sent to the server.
-    #
-    # @example Does the slaveOk bit need to be set for all queries sent to the server.
-    #   server.slave_ok?
-    #
-    # @return [ true, false ] If the slaveOk bit needs to be set for all queries.
-    #
-    # @since 2.0.0
-    def slave_ok?
-      options[:slave_ok]
-    end
   end
 end

lib/mongo/server/context.rb

@@ -27,14 +27,14 @@ class Context
 
       # Delegate state checks to the server.
       def_delegators :@server,
+                     :cluster,
                      :features,
                      :max_wire_version,
                      :max_write_batch_size,
                      :mongos?,
                      :primary?,
                      :secondary?,
-                     :standalone?,
-                     :slave_ok?
+                     :standalone?
 
       # Instantiate a server context.
       #

spec/mongo/client_spec.rb

@@ -412,6 +412,10 @@
       it 'returns the new client' do
         expect(client.send(:database).name).to eq('ruby-driver')
       end
+
+      it 'keeps the same cluster' do
+        expect(database.cluster).to equal(client.cluster)
+      end
     end
 
     context 'when provided a string' do

spec/mongo/operation/read_preferrable_spec.rb

@@ -28,9 +28,14 @@
     end
   end
 
+  let(:cluster_double) do
+    double('cluster')
+  end
+
   let(:context) do
     double('context').tap do |c|
-      allow(c).to receive(:slave_ok?).and_return(slave_ok)
+      allow(c).to receive(:cluster).and_return(cluster_double)
+      allow(cluster_double).to receive(:single?).and_return(slave_ok)
       allow(c).to receive(:mongos?).and_return(mongos)
     end
   end

spec/mongo/server_discovery_and_monitoring_spec.rb

@@ -34,8 +34,9 @@ class Mongo::Server
 
           # The contructor keeps the same API, but does not instantiate a
           # monitor and run it.
-          def initialize(address, event_listeners, options = {})
+          def initialize(address, cluster, event_listeners, options = {})
             @address = address
+            @cluster = cluster
             @options = options.freeze
             @monitor = Monitor.new(address, event_listeners, options)
           end
@@ -58,8 +59,9 @@ def disconnect!; true; end
         class Mongo::Server
 
           # Returns the constructor to its original implementation.
-          def initialize(address, event_listeners, options = {})
+          def initialize(address, cluster, event_listeners, options = {})
             @address = address
+            @cluster = cluster
             @options = options.freeze
             @monitor = Monitor.new(address, event_listeners, options)
             @monitor.scan!

spec/mongo/server_spec.rb

@@ -2,6 +2,10 @@
 
 describe Mongo::Server do
 
+  let(:cluster) do
+    double('cluster')
+  end
+
   let(:listeners) do
     Mongo::Event::Listeners.new
   end
@@ -13,7 +17,7 @@
   describe '#==' do
 
     let(:server) do
-      described_class.new(address, listeners, TEST_OPTIONS)
+      described_class.new(address, cluster, listeners, TEST_OPTIONS)
     end
 
     context 'when the other is not a server' do
@@ -32,7 +36,7 @@
       context 'when the addresses match' do
 
         let(:other) do
-          described_class.new(address, listeners, TEST_OPTIONS)
+          described_class.new(address, cluster, listeners, TEST_OPTIONS)
         end
 
         it 'returns true' do
@@ -47,7 +51,7 @@
         end
 
         let(:other) do
-          described_class.new(other_address, listeners, TEST_OPTIONS)
+          described_class.new(other_address, cluster, listeners, TEST_OPTIONS)
         end
 
         it 'returns false' do
@@ -60,7 +64,7 @@
   describe '#context' do
 
     let(:server) do
-      described_class.new(address, listeners, TEST_OPTIONS)
+      described_class.new(address, cluster, listeners, TEST_OPTIONS)
     end
 
     let(:context) do
@@ -75,7 +79,7 @@
   describe '#disconnect!' do
 
     let(:server) do
-      described_class.new(address, listeners, TEST_OPTIONS)
+      described_class.new(address, cluster, listeners, TEST_OPTIONS)
     end
 
     it 'stops the monitor instance' do
@@ -87,7 +91,7 @@
   describe '#initialize' do
 
     let(:server) do
-      described_class.new(address, listeners, TEST_OPTIONS.merge(:heartbeat_frequency => 5))
+      described_class.new(address, cluster, listeners, TEST_OPTIONS.merge(:heartbeat_frequency => 5))
     end
 
     it 'sets the address host' do
@@ -106,7 +110,7 @@
   describe '#pool' do
 
     let(:server) do
-      described_class.new(address, listeners, TEST_OPTIONS)
+      described_class.new(address, cluster, listeners, TEST_OPTIONS)
     end
 
     let(:pool) do
@@ -121,7 +125,7 @@
   describe '#scan!' do
 
     let(:server) do
-      described_class.new(address, listeners, TEST_OPTIONS)
+      described_class.new(address, cluster, listeners, TEST_OPTIONS)
     end
 
     it 'forces a scan on the monitor' do

spec/support/shared/operation.rb

@@ -5,6 +5,10 @@
   let(:write_concern) { Mongo::WriteConcern.get(:w => 1) }
   let(:options) { {} }
 
+  let(:cluster_double) do
+    double('cluster')
+  end
+
   # Server doubles
   let(:secondary_server) do
     double('secondary_server').tap do |s|
@@ -43,7 +47,8 @@
       allow(cxt).to receive(:primary?) { true }
       allow(cxt).to receive(:secondary?) { false }
       allow(cxt).to receive(:standalone?) { false }
-      allow(cxt).to receive(:slave_ok?) { false }
+      allow(cxt).to receive(:cluster) { cluster_double }
+      allow(cluster_double).to receive(:single?) { false }
     end
   end
   let(:secondary_context) do
@@ -55,7 +60,8 @@
       allow(cxt).to receive(:secondary?) { true }
       allow(cxt).to receive(:primary?) { false }
       allow(cxt).to receive(:standalone?) { false }
-      allow(cxt).to receive(:slave_ok?) { false }
+      allow(cxt).to receive(:cluster) { cluster_double }
+      allow(cluster_double).to receive(:single?) { false }
     end
   end
   let(:secondary_context_slave) do
@@ -67,7 +73,8 @@
       allow(cxt).to receive(:secondary?) { true }
       allow(cxt).to receive(:primary?) { false }
       allow(cxt).to receive(:standalone?) { false }
-      allow(cxt).to receive(:slave_ok?) { true }
+      allow(cxt).to receive(:cluster) { cluster_double }
+      allow(cluster_double).to receive(:single?) { true }
     end
   end
   let(:primary_context_2_4_version) do
@@ -78,7 +85,8 @@
       allow(cxt).to receive(:primary?) { true }
       allow(cxt).to receive(:secondary?) { false }
       allow(cxt).to receive(:standalone?) { false }
-      allow(cxt).to receive(:slave_ok?) { false }
+      allow(cxt).to receive(:cluster) { cluster_double }
+      allow(cluster_double).to receive(:single?) { false }
       allow(cxt).to receive(:features) { features_2_4 }
     end
   end