ClusterSettings does not compute ClusterConnectionMode consistently (mongodb#1273)

`ClusterSettings` does not compute `ClusterConnectionMode` consistently

JAVA-5088

Author: stIncMale

driver-core/src/main/com/mongodb/connection/ClusterSettings.java

@@ -337,16 +337,16 @@ public Builder applyConnectionString(final ConnectionString connectionString) {
                 if (srvServiceName != null) {
                     srvServiceName(srvServiceName);
                 }
-            } else if ((directConnection != null && directConnection)
-                    || (directConnection == null && connectionString.getHosts().size() == 1
-                        && connectionString.getRequiredReplicaSetName() == null)) {
-                mode(ClusterConnectionMode.SINGLE)
-                        .hosts(singletonList(createServerAddress(connectionString.getHosts().get(0))));
+            } else if (directConnection != null) {
+                mode(directConnection ? ClusterConnectionMode.SINGLE : ClusterConnectionMode.MULTIPLE);
+                List<String> hosts = directConnection ? singletonList(connectionString.getHosts().get(0)) : connectionString.getHosts();
+                hosts(hosts.stream().map(ServerAddressHelper::createServerAddress).collect(Collectors.toList()));
             } else {
+                mode = null;
                 List<ServerAddress> seedList = connectionString.getHosts().stream()
                         .map(ServerAddressHelper::createServerAddress)
                         .collect(Collectors.toList());
-                mode(ClusterConnectionMode.MULTIPLE).hosts(seedList);
+                hosts(seedList);
             }
             requiredReplicaSetName(connectionString.getRequiredReplicaSetName());
 
@@ -612,26 +612,39 @@ private ClusterSettings(final Builder builder) {
             }
         }
 
-        if (builder.mode == ClusterConnectionMode.LOAD_BALANCED && builder.srvHost == null && builder.hosts.size() != 1) {
-            throw new IllegalArgumentException("Multiple hosts cannot be specified when in load balancing mode");
-        }
-
         srvHost = builder.srvHost;
         srvMaxHosts = builder.srvMaxHosts;
         srvServiceName = builder.srvServiceName;
         hosts = builder.hosts;
-        if (srvHost != null) {
-            if (builder.mode == ClusterConnectionMode.SINGLE) {
-                throw new IllegalArgumentException("An SRV host name was provided but the connection mode is not MULTIPLE");
+        requiredReplicaSetName = builder.requiredReplicaSetName;
+        if (builder.mode != null) {
+            switch (builder.mode) {
+                case SINGLE: {
+                    if (srvHost != null) {
+                        throw new IllegalArgumentException("An SRV host name was provided but the connection mode is not MULTIPLE");
+                    } else if (builder.hosts.size() > 1) {
+                        throw new IllegalArgumentException("Can not directly connect to more than one server");
+                    }
+                    break;
+                }
+                case LOAD_BALANCED: {
+                    if (builder.srvHost == null && builder.hosts.size() != 1) {
+                        throw new IllegalArgumentException("Multiple hosts cannot be specified when in load balancing mode");
+                    }
+                    break;
+                }
+                default:
             }
-            mode = builder.mode != null ? builder.mode : ClusterConnectionMode.MULTIPLE;
+            mode = builder.mode;
         } else {
-            if (builder.mode == ClusterConnectionMode.SINGLE && builder.hosts.size() > 1) {
-                throw new IllegalArgumentException("Can not directly connect to more than one server");
+            if (srvHost != null) {
+                mode = ClusterConnectionMode.MULTIPLE;
+            } else {
+                mode = hosts.size() == 1 && requiredReplicaSetName == null
+                        ? ClusterConnectionMode.SINGLE
+                        : ClusterConnectionMode.MULTIPLE;
             }
-            mode = builder.mode != null ? builder.mode : hosts.size() == 1 ? ClusterConnectionMode.SINGLE : ClusterConnectionMode.MULTIPLE;
         }
-        requiredReplicaSetName = builder.requiredReplicaSetName;
         requiredClusterType = builder.requiredClusterType;
         localThresholdMS = builder.localThresholdMS;
         serverSelector = builder.serverSelector;

driver-core/src/test/unit/com/mongodb/connection/ClusterSettingsSpecification.groovy

@@ -136,6 +136,17 @@ class ClusterSettingsSpecification extends Specification {
         thrown(IllegalArgumentException)
     }
 
+    def 'when srvHost is specified and mode is SINGLE, should throw'() {
+        when:
+        ClusterSettings.builder()
+                .srvHost('foo.bar.com')
+                .mode(ClusterConnectionMode.SINGLE)
+                .build()
+
+        then:
+        thrown(IllegalArgumentException)
+    }
+
     def 'when srvHost is specified, should set mode to MULTIPLE if mode is not configured'() {
         when:
         def builder = ClusterSettings.builder()
@@ -181,8 +192,10 @@ class ClusterSettingsSpecification extends Specification {
 
     def 'when connection string is applied to builder, all properties should be set'() {
         when:
-        def settings = ClusterSettings.builder().applyConnectionString(new ConnectionString('mongodb://example.com:27018'))
-                                      .build()
+        def settings = ClusterSettings.builder()
+                .requiredReplicaSetName("test")
+                .applyConnectionString(new ConnectionString('mongodb://example.com:27018'))
+                .build()
 
         then:
         settings.mode == ClusterConnectionMode.SINGLE
@@ -192,6 +205,20 @@ class ClusterSettingsSpecification extends Specification {
         settings.srvMaxHosts == null
         settings.srvServiceName == 'mongodb'
 
+        when:
+        settings = ClusterSettings.builder()
+                .applyConnectionString(new ConnectionString('mongodb://example.com:27018'))
+                .requiredReplicaSetName("test")
+                .build()
+
+        then:
+        settings.mode == ClusterConnectionMode.MULTIPLE
+        settings.hosts == [new ServerAddress('example.com:27018')]
+        settings.requiredClusterType == ClusterType.REPLICA_SET
+        settings.requiredReplicaSetName == 'test'
+        settings.srvMaxHosts == null
+        settings.srvServiceName == 'mongodb'
+
         when:
         settings = ClusterSettings.builder().applyConnectionString(new ConnectionString('mongodb+srv://test5.test.build.10gen.cc/')).build()
 
@@ -216,8 +243,10 @@ class ClusterSettingsSpecification extends Specification {
         settings.srvServiceName == 'customname'
 
         when:
-        settings = ClusterSettings.builder().applyConnectionString(new ConnectionString('mongodb://example.com:27018/?replicaSet=test'))
-                                  .build()
+        settings = ClusterSettings.builder()
+                .mode(ClusterConnectionMode.SINGLE)
+                .applyConnectionString(new ConnectionString('mongodb://example.com:27018/?replicaSet=test'))
+                .build()
 
         then:
         settings.mode == ClusterConnectionMode.MULTIPLE
@@ -240,6 +269,19 @@ class ClusterSettingsSpecification extends Specification {
         settings.srvMaxHosts == null
         settings.srvServiceName == 'mongodb'
 
+        when:
+        settings = ClusterSettings.builder()
+                .applyConnectionString(new ConnectionString('mongodb://example.com:27017,example.com:27018/?directConnection=false'))
+                .build()
+
+        then:
+        settings.mode == ClusterConnectionMode.MULTIPLE
+        settings.hosts == [new ServerAddress('example.com:27017'), new ServerAddress('example.com:27018')]
+        settings.requiredClusterType == ClusterType.UNKNOWN
+        settings.requiredReplicaSetName == null
+        settings.srvMaxHosts == null
+        settings.srvServiceName == 'mongodb'
+
         when:
         settings = ClusterSettings.builder()
                 .applyConnectionString(new ConnectionString('mongodb://example.com:27018/?directConnection=true'))
@@ -288,15 +330,16 @@ class ClusterSettingsSpecification extends Specification {
         settings.hosts == [new ServerAddress('example.com:27018')]
     }
 
-    def 'when cluster type is unknown and replica set name is specified, should set cluster type to ReplicaSet'() {
+    def 'when cluster type is UNKNOWN and replica set name is set, should set cluster type to REPLICA_SET and mode to MULTIPLE'() {
         when:
         def settings = ClusterSettings.builder().hosts([new ServerAddress()]).requiredReplicaSetName('yeah').build()
 
         then:
         ClusterType.REPLICA_SET == settings.requiredClusterType
+        ClusterConnectionMode.MULTIPLE == settings.mode
     }
 
-    def 'connection mode should default to single if one host or multiple if more'() {
+    def 'connection mode should default to SINGLE if replica set name is not set and one host, or MULTIPLE if more'() {
         when:
         def settings = ClusterSettings.builder().hosts([new ServerAddress()]).build()
 
@@ -310,11 +353,28 @@ class ClusterSettingsSpecification extends Specification {
         settings.mode == ClusterConnectionMode.MULTIPLE
     }
 
+    def 'when a valid mode is specified, should use it'() {
+        when:
+        def mode = ClusterConnectionMode.LOAD_BALANCED
+        def settings = ClusterSettings.builder().mode(mode).build()
+
+        then:
+        settings.mode == mode
+    }
+
     def 'when mode is Single and hosts size is greater than one, should throw'() {
         when:
         ClusterSettings.builder().hosts([new ServerAddress(), new ServerAddress('other')]).mode(ClusterConnectionMode.SINGLE).build()
         then:
         thrown(IllegalArgumentException)
+
+        when:
+        ClusterSettings.builder()
+                .applyConnectionString(new ConnectionString("mongodb://host1,host2/"))
+                .mode(ClusterConnectionMode.SINGLE)
+                .build()
+        then:
+        thrown(IllegalArgumentException)
     }
 
     def 'when cluster type is Standalone and multiple hosts are specified, should throw'() {