fix(secondaries): fixes connection with secondary readPreference (#258)

Ensures that when readPreference is `secondary`, the `connect`
event is not triggered until connection has been established
with a secondary.

Fixes NODE-1089

Author: daprahamian

lib/topologies/replset.js

@@ -773,6 +773,22 @@ function applyAuthenticationContexts(self, server, callback) {
   applyAuth(authContexts, server, callback);
 }
 
+function shouldTriggerConnect(self) {
+  const isConnecting = self.state === CONNECTING;
+  const hasPrimary = self.s.replicaSetState.hasPrimary();
+  const hasSecondary = self.s.replicaSetState.hasSecondary();
+  const secondaryOnlyConnectionAllowed = self.s.options.secondaryOnlyConnectionAllowed;
+  const readPreferenceSecondary =
+    self.s.connectOptions.readPreference &&
+    self.s.connectOptions.readPreference.equals(ReadPreference.secondary);
+
+  return (
+    (isConnecting &&
+      ((readPreferenceSecondary && hasSecondary) || (!readPreferenceSecondary && hasPrimary))) ||
+    (hasSecondary && secondaryOnlyConnectionAllowed)
+  );
+}
+
 function handleInitialConnectEvent(self, event) {
   return function() {
     var _this = this;
@@ -835,10 +851,7 @@ function handleInitialConnectEvent(self, event) {
           _this.on('parseError', handleEvent(self, 'parseError'));
 
           // Do we have a primary or primaryAndSecondary
-          if (
-            (self.state === CONNECTING && self.s.replicaSetState.hasPrimary()) ||
-            (self.s.replicaSetState.hasSecondary() && self.s.options.secondaryOnlyConnectionAllowed)
-          ) {
+          if (shouldTriggerConnect(self)) {
             // We are connected
             self.state = CONNECTED;
 

test/tests/unit/replset/read_preference_tests.js

@@ -0,0 +1,51 @@
+'use strict';
+
+const expect = require('chai').expect;
+const ReplSet = require('../../../../lib/topologies/replset');
+const ReadPreference = require('../../../../lib/topologies/read_preference');
+const mock = require('../../../mock');
+const ReplSetFixture = require('../common').ReplSetFixture;
+
+describe('Secondaries (ReplSet)', function() {
+  let test;
+  before(() => (test = new ReplSetFixture()));
+  afterEach(() => mock.cleanup());
+  beforeEach(() => test.setup());
+
+  it('Should not be "connected" with ReadPreference secondary unless secondary is connected', {
+    metadata: {
+      requires: {
+        topology: 'single'
+      }
+    },
+
+    test: function(done) {
+      const replSet = new ReplSet(
+        [test.primaryServer.address(), test.firstSecondaryServer.address()],
+        {
+          setName: 'rs',
+          connectionTimeout: 3000,
+          socketTimeout: 0,
+          haInterval: 100,
+          size: 1
+        }
+      );
+
+      replSet.on('error', done);
+
+      replSet.on('connect', server => {
+        let err;
+        try {
+          expect(server.s.replicaSetState.hasSecondary()).to.equal(true);
+        } catch (e) {
+          err = e;
+        }
+
+        replSet.destroy();
+        done(err);
+      });
+
+      replSet.connect({ readPreference: new ReadPreference('secondary') });
+    }
+  });
+});