ClusterSettings does not compute ClusterConnectionMode consistently
#1163
Conversation
There was a problem hiding this comment.
Change looks good, but I recommend that we defer it to 5.0 since it could cause upgrade issues. Though it's a bug, it's similar to the issue regarding changing the default value of directionConnection, which I'm recommending that we not even do in a major release.
| @@ -629,9 +630,14 @@ private ClusterSettings(final Builder builder) { | |||
| if (builder.mode == ClusterConnectionMode.SINGLE && builder.hosts.size() > 1) { | |||
| throw new IllegalArgumentException("Can not directly connect to more than one server"); | |||
| } | |||
| mode = builder.mode != null ? builder.mode : hosts.size() == 1 ? ClusterConnectionMode.SINGLE : ClusterConnectionMode.MULTIPLE; | |||
| if (builder.mode != null) { | |||
There was a problem hiding this comment.
If this conditional is removed, the only test that fails is when connection string is applied to builder, all properties should be set. It seems like there should be a more direct test which covers the scenario where there is no connection string applied at all.
| @@ -288,15 +288,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'() { | |||
There was a problem hiding this comment.
Consider adding more tests which combine connection string application with explicit use of the builder properties (both before and after connection string application), e.g.
ClusterSettings.builder()
.applyConnectionString(new ConnectionString("mongodb://127.0.0.1:27017/"))
.requiredReplicaSetName("replset")
.build()
.getMode()
and
ClusterSettings.builder()
.requiredReplicaSetName("replset")
.applyConnectionString(new ConnectionString("mongodb://127.0.0.1:27017/"))
.build()
.getMode()
|
Closed the PR, because the ticket was retargeted to 5.0. |
JAVA-5088