Skip to content

Commit a911780

Browse files
vbabaninvbabanin
committed
Address PR review feedback
- Remove redundant @VisibleForTesting annotation and use static import for SYSTEM_OVERLOADED_ERROR_LABEL constant - Fix ServerDeprioritizationTest to use parameterized cluster in assertions instead of SHARDED_CLUSTER constant - Move ServerSelectionSelectionTest to internal.connection package and update references - Update TODO comments to TODO-BACKPRESSURE format with JAVA-6167 reference JAVA-6021
1 parent 40aecc7 commit a911780

6 files changed

Lines changed: 25 additions & 21 deletions

File tree

‎driver-core/src/main/com/mongodb/internal/connection/BaseCluster.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767
import static com.mongodb.connection.ServerDescription.MIN_DRIVER_SERVER_VERSION;
6868
import static com.mongodb.connection.ServerDescription.MIN_DRIVER_WIRE_VERSION;
6969
import static com.mongodb.internal.Locks.withInterruptibleLock;
70-
import static com.mongodb.internal.VisibleForTesting.AccessModifier.PACKAGE;
7170
import static com.mongodb.internal.VisibleForTesting.AccessModifier.PRIVATE;
7271
import static com.mongodb.internal.VisibleForTesting.AccessModifier.PROTECTED;
7372
import static com.mongodb.internal.connection.EventHelper.wouldDescriptionsGenerateEquivalentEvents;
@@ -115,8 +114,7 @@ public abstract class BaseCluster implements Cluster {
115114
private volatile boolean isClosed;
116115
private volatile ClusterDescription description;
117116

118-
@VisibleForTesting(otherwise = PACKAGE)
119-
protected BaseCluster(final ClusterId clusterId,
117+
BaseCluster(final ClusterId clusterId,
120118
final ClusterSettings settings,
121119
final ClusterableServerFactory serverFactory,
122120
final ClientMetadata clientMetadata) {

‎driver-core/src/main/com/mongodb/internal/connection/OperationContext.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.util.concurrent.TimeUnit;
4242
import java.util.concurrent.atomic.AtomicLong;
4343

44+
import static com.mongodb.MongoException.SYSTEM_OVERLOADED_ERROR_LABEL;
4445
import static com.mongodb.internal.VisibleForTesting.AccessModifier.PACKAGE;
4546
import static java.util.stream.Collectors.toList;
4647

@@ -253,7 +254,7 @@ public void onAttemptFailure(final Throwable failure) {
253254
}
254255

255256
boolean isSystemOverloadedError = failure instanceof MongoException
256-
&& ((MongoException) failure).hasErrorLabel(MongoException.SYSTEM_OVERLOADED_ERROR_LABEL);
257+
&& ((MongoException) failure).hasErrorLabel(SYSTEM_OVERLOADED_ERROR_LABEL);
257258
if (clusterType == ClusterType.SHARDED || isSystemOverloadedError) {
258259
deprioritized.add(candidate);
259260
}

‎driver-core/src/test/unit/com/mongodb/internal/connection/ServerDeprioritizationTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,18 +210,19 @@ void onAttemptFailureDoesNotThrowIfNoCandidate() {
210210
@ParameterizedTest
211211
@EnumSource(value = ClusterType.class, names = "SHARDED", mode = EnumSource.Mode.EXCLUDE)
212212
void onAttemptFailureIgnoresIfNonShardedWithoutOverloadError(final ClusterType clusterType) {
213+
ClusterDescription cluster = multipleModeClusterDescription(clusterType);
213214
ServerSelector selector = createAssertingSelector(ALL_SERVERS, singletonList(SERVER_A));
214215

215216
assertAll(() -> {
216217
serverDeprioritization.updateCandidate(SERVER_B.getAddress(), clusterType);
217218
serverDeprioritization.onAttemptFailure(new RuntimeException());
218-
assertEquals(singletonList(SERVER_A), serverDeprioritization.apply(selector).select(SHARDED_CLUSTER),
219+
assertEquals(singletonList(SERVER_A), serverDeprioritization.apply(selector).select(cluster),
219220
"Expected no deprioritization for " + clusterType + " with RuntimeException");
220221
}, () -> {
221222
serverDeprioritization = createOperationContext(TIMEOUT_SETTINGS).getServerDeprioritization();
222223
serverDeprioritization.updateCandidate(SERVER_B.getAddress(), clusterType);
223224
serverDeprioritization.onAttemptFailure(new MongoException(1, "error"));
224-
assertEquals(singletonList(SERVER_A), serverDeprioritization.apply(selector).select(SHARDED_CLUSTER),
225+
assertEquals(singletonList(SERVER_A), serverDeprioritization.apply(selector).select(cluster),
225226
"Expected no deprioritization for " + clusterType + " with no SystemOverloadedError MongoException");
226227
}
227228
);

driver-core/src/test/unit/com/mongodb/connection/ServerSelectionSelectionTest.java renamed to driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionSelectionTest.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
package com.mongodb.connection;
17+
package com.mongodb.internal.connection;
1818

1919
import com.mongodb.ClusterFixture;
2020
import com.mongodb.MongoConfigurationException;
@@ -25,14 +25,17 @@
2525
import com.mongodb.Tag;
2626
import com.mongodb.TagSet;
2727
import com.mongodb.assertions.Assertions;
28+
import com.mongodb.connection.ClusterConnectionMode;
29+
import com.mongodb.connection.ClusterDescription;
30+
import com.mongodb.connection.ClusterId;
31+
import com.mongodb.connection.ClusterSettings;
32+
import com.mongodb.connection.ClusterType;
33+
import com.mongodb.connection.ServerConnectionState;
34+
import com.mongodb.connection.ServerDescription;
35+
import com.mongodb.connection.ServerSettings;
36+
import com.mongodb.connection.ServerType;
2837
import com.mongodb.event.ServerDescriptionChangedEvent;
2938
import com.mongodb.internal.TimeoutContext;
30-
import com.mongodb.internal.connection.BaseCluster;
31-
import com.mongodb.internal.connection.Cluster;
32-
import com.mongodb.internal.connection.OperationContext;
33-
import com.mongodb.internal.connection.Server;
34-
import com.mongodb.internal.connection.ServerTuple;
35-
import com.mongodb.internal.connection.TestClusterableServerFactory;
3639
import com.mongodb.internal.mockito.MongoMockito;
3740
import com.mongodb.internal.selector.ReadPreferenceServerSelector;
3841
import com.mongodb.internal.selector.WritableServerSelector;

‎driver-core/src/test/unit/com/mongodb/internal/connection/ServerSelectionWithinLatencyWindowTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343

4444
import static com.mongodb.ClusterFixture.TIMEOUT_SETTINGS;
4545
import static com.mongodb.ClusterFixture.createOperationContext;
46-
import static com.mongodb.connection.ServerSelectionSelectionTest.buildClusterDescription;
46+
import static com.mongodb.internal.connection.ServerSelectionSelectionTest.buildClusterDescription;
4747
import static java.util.stream.Collectors.groupingBy;
4848
import static java.util.stream.Collectors.toMap;
4949
import static org.junit.Assert.assertEquals;

‎driver-sync/src/test/functional/com/mongodb/client/AbstractRetryableReadsProseTest.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
import static java.util.Arrays.asList;
4343
import static java.util.Collections.emptyList;
4444
import static org.junit.jupiter.api.Assertions.assertEquals;
45-
import static org.junit.jupiter.api.Assertions.assertFalse;
45+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
4646
import static org.junit.jupiter.api.Assumptions.assumeTrue;
4747

4848
/**
@@ -53,7 +53,7 @@ public abstract class AbstractRetryableReadsProseTest {
5353

5454
private static final String COLLECTION_NAME = "test";
5555

56-
protected abstract MongoClient createClient(MongoClientSettings settings);
56+
protected abstract MongoClient createClient(final MongoClientSettings settings);
5757

5858
@AfterEach
5959
void afterEach() {
@@ -102,7 +102,7 @@ void retriesOnSameMongosWhenAnotherNotAvailable() {
102102
* <a href="https://github.com/mongodb/specifications/blob/master/source/retryable-reads/tests/README.md#31-retryable-reads-caused-by-overload-errors-are-retried-on-a-different-replicaset-server-when-one-is-available">
103103
* 3.1 Retryable Reads Caused by Overload Errors Are Retried on a Different Replicaset Server When One is Available</a>.
104104
*/
105-
//TODO-JAVA-6141 add overloadRetargeting into tests.
105+
//TODO-BACKPRESSURE Slav Babanin JAVA-6167 add overloadRetargeting into tests.
106106
@Test
107107
void overloadErrorRetriedOnDifferentReplicaSetServer() throws InterruptedException {
108108
//given
@@ -115,12 +115,13 @@ void overloadErrorRetriedOnDifferentReplicaSetServer() throws InterruptedExcepti
115115
+ " configureFailPoint: \"failCommand\",\n"
116116
+ " mode: { times: 1 },\n"
117117
+ " data: {\n"
118-
+ " failCommands: [\"find\"],\n"
118+
+ " failCommands: [\"insert\"],\n"
119119
+ " errorLabels: ['" + RETRYABLE_ERROR_LABEL + "', '" + SYSTEM_OVERLOADED_ERROR_LABEL + "'],\n"
120120
+ " errorCode: 6\n"
121121
+ " }\n"
122122
+ "}\n");
123123

124+
124125
try (FailPoint ignored = FailPoint.enable(configureFailPoint, primaryServerAddress);
125126
MongoClient client = createClient(getMongoClientSettingsBuilder()
126127
.retryReads(true)
@@ -133,7 +134,7 @@ void overloadErrorRetriedOnDifferentReplicaSetServer() throws InterruptedExcepti
133134
commandListener.reset();
134135

135136
//when
136-
collection.find().first();
137+
collection.insertOne(new Document());
137138

138139
//then
139140
List<CommandFailedEvent> commandFailedEvents = commandListener.getCommandFailedEvents();
@@ -144,7 +145,7 @@ void overloadErrorRetriedOnDifferentReplicaSetServer() throws InterruptedExcepti
144145
ServerAddress failedServer = commandFailedEvents.get(0).getConnectionDescription().getServerAddress();
145146
ServerAddress succeededServer = commandSucceededEvents.get(0).getConnectionDescription().getServerAddress();
146147

147-
assertFalse(failedServer.equals(succeededServer),
148+
assertNotEquals(failedServer, succeededServer,
148149
format("Expected retry on different server but both were %s", failedServer));
149150
}
150151
}
@@ -153,7 +154,7 @@ void overloadErrorRetriedOnDifferentReplicaSetServer() throws InterruptedExcepti
153154
* <a href="https://github.com/mongodb/specifications/blob/master/source/retryable-reads/tests/README.md#32-retryable-reads-caused-by-non-overload-errors-are-retried-on-the-same-replicaset-server">
154155
* 3.2 Retryable Reads Caused by Non-Overload Errors Are Retried on the Same Replicaset Server</a>.
155156
*/
156-
//TODO-JAVA-6141 add overloadRetargeting into tests.
157+
//TODO-BACKPRESSURE Slav Babanin JAVA-6167 add overloadRetargeting into tests.
157158
@Test
158159
void nonOverloadErrorRetriedOnSameReplicaSetServer() throws InterruptedException {
159160
//given

0 commit comments

Comments
 (0)