Skip to content

ClusterSettings does not compute ClusterConnectionMode consistently #1273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 6, 2023
Merged

ClusterSettings does not compute ClusterConnectionMode consistently #1273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1ebfcc
`ClusterSettings` does not compute `ClusterConnectionMode` consistently
stIncMale Jul 28, 2023
1ea39dc
Fix a bug discovered when reviewing
stIncMale Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 31 additions & 18 deletions driver-core/src/main/com/mongodb/connection/ClusterSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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;
Expand Down
72 changes: 66 additions & 6 deletions driver-core/src/test/unit/com/mongodb/connection/ClusterSettingsSpecification.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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()

Expand All @@ -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
Expand All @@ -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'))
Expand Down Expand Up @@ -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()

Expand All @@ -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'() {
Expand Down