From 0a60295d4256ff4b6b1968a4bcb069a6f87763c5 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Mon, 3 Apr 2023 08:10:47 -0600 Subject: [PATCH 1/9] Refactor tests, credential cache Refactoring for: JAVA-4757 --- .../org/bson/codecs/pojo/PojoTestCase.java | 40 ++------- .../src/test/unit/util/ThreadTestHelpers.java | 66 +++++++++++++++ .../main/com/mongodb/ConnectionString.java | 6 +- .../src/main/com/mongodb/MongoCredential.java | 37 +++++---- .../internal/connection/AwsAuthenticator.java | 19 ++--- .../connection/MongoCredentialWithCache.java | 27 +++--- .../connection/SaslAuthenticator.java | 3 +- .../mongodb/AbstractConnectionStringTest.java | 2 +- .../com/mongodb/AuthConnectionStringTest.java | 82 +++++++++++-------- 9 files changed, 169 insertions(+), 113 deletions(-) create mode 100644 bson/src/test/unit/util/ThreadTestHelpers.java diff --git a/bson/src/test/unit/org/bson/codecs/pojo/PojoTestCase.java b/bson/src/test/unit/org/bson/codecs/pojo/PojoTestCase.java index 0957833a5d8..6383e6b1de8 100644 --- a/bson/src/test/unit/org/bson/codecs/pojo/PojoTestCase.java +++ b/bson/src/test/unit/org/bson/codecs/pojo/PojoTestCase.java @@ -70,17 +70,13 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import static java.lang.String.format; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.bson.codecs.configuration.CodecRegistries.fromProviders; import static org.bson.codecs.pojo.Conventions.DEFAULT_CONVENTIONS; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static util.ThreadTestHelpers.executeAll; abstract class PojoTestCase { @@ -97,34 +93,12 @@ void roundTrip(final PojoCodecProvider.Builder builder, final T value, final void threadedRoundTrip(final PojoCodecProvider.Builder builder, final T value, final String json) { int numberOfThreads = 5; - ExecutorService service = null; - try { - service = Executors.newFixedThreadPool(10); - CountDownLatch latch = new CountDownLatch(numberOfThreads); - List errors = new ArrayList<>(); - CodecRegistry codecRegistry = getCodecRegistry(builder); - for (int i = 0; i < numberOfThreads; i++) { - service.submit(() -> { - try { - encodesTo(codecRegistry, value, json); - decodesTo(codecRegistry, json, value); - } catch (Exception e) { - errors.add(e instanceof NullPointerException ? "NPE: " + e.getStackTrace()[0] : e.getMessage()); - } - latch.countDown(); - }); - } - try { - latch.await(); - } catch (InterruptedException e) { - // Ignore - } - assertTrue(format("Errors encountered: [%s]", String.join(",", errors)), errors.isEmpty()); - } finally { - if (service != null) { - service.shutdown(); - } - } + CodecRegistry codecRegistry = getCodecRegistry(builder); + + executeAll(numberOfThreads, () -> { + encodesTo(codecRegistry, value, json); + decodesTo(codecRegistry, json, value); + }); } void roundTrip(final CodecRegistry registry, final T value, final String json) { diff --git a/bson/src/test/unit/util/ThreadTestHelpers.java b/bson/src/test/unit/util/ThreadTestHelpers.java new file mode 100644 index 00000000000..6267f574ecd --- /dev/null +++ b/bson/src/test/unit/util/ThreadTestHelpers.java @@ -0,0 +1,66 @@ +/* + * Copyright 2008-present MongoDB, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package util; + +import org.opentest4j.MultipleFailuresError; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +public final class ThreadTestHelpers { + + private ThreadTestHelpers() { + } + + public static void executeAll(final int nThreads, final Runnable c) { + ExecutorService service = null; + try { + service = Executors.newFixedThreadPool(10); + CountDownLatch latch = new CountDownLatch(nThreads); + List failures = new ArrayList<>(); + for (int i = 0; i < nThreads; i++) { + service.submit(() -> { + try { + c.run(); + } catch (Throwable e) { + failures.add(e); + } finally { + latch.countDown(); + } + }); + } + try { + latch.await(); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + if (!failures.isEmpty()) { + MultipleFailuresError multipleFailuresError = new MultipleFailuresError("Failed to execute all", failures); + failures.forEach(multipleFailuresError::addSuppressed); + throw multipleFailuresError; + } + } finally { + if (service != null) { + service.shutdown(); + } + } + } + +} diff --git a/driver-core/src/main/com/mongodb/ConnectionString.java b/driver-core/src/main/com/mongodb/ConnectionString.java index cd8f359f35c..b84125fb436 100644 --- a/driver-core/src/main/com/mongodb/ConnectionString.java +++ b/driver-core/src/main/com/mongodb/ConnectionString.java @@ -800,8 +800,10 @@ private UuidRepresentation createUuidRepresentation(final String value) { } @Nullable - private MongoCredential createCredentials(final Map> optionsMap, @Nullable final String userName, - @Nullable final char[] password) { + private MongoCredential createCredentials( + final Map> optionsMap, + @Nullable final String userName, + @Nullable final char[] password) { AuthenticationMechanism mechanism = null; String authSource = null; String gssapiServiceName = null; diff --git a/driver-core/src/main/com/mongodb/MongoCredential.java b/driver-core/src/main/com/mongodb/MongoCredential.java index 59c7811e043..c52e1d03f09 100644 --- a/driver-core/src/main/com/mongodb/MongoCredential.java +++ b/driver-core/src/main/com/mongodb/MongoCredential.java @@ -362,14 +362,22 @@ public MongoCredential withMechanism(final AuthenticationMechanism mechanism) { * @param source the source of the user name, typically a database name * @param password the password */ - MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, final String source, - @Nullable final char[] password) { + MongoCredential( + @Nullable final AuthenticationMechanism mechanism, + @Nullable final String userName, + final String source, + @Nullable final char[] password) { this(mechanism, userName, source, password, Collections.emptyMap()); } - MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, final String source, - @Nullable final char[] password, final Map mechanismProperties) { - if (mechanism != MONGODB_X509 && mechanism != MONGODB_AWS && userName == null) { + MongoCredential( + @Nullable final AuthenticationMechanism mechanism, + @Nullable final String userName, + final String source, + @Nullable final char[] password, + final Map mechanismProperties) { + + if (userName == null && !Arrays.asList(MONGODB_X509, MONGODB_AWS).contains(mechanism)) { throw new IllegalArgumentException("username can not be null"); } @@ -399,7 +407,6 @@ public MongoCredential withMechanism(final AuthenticationMechanism mechanism) { private boolean mechanismRequiresPassword(@Nullable final AuthenticationMechanism mechanism) { return mechanism == PLAIN || mechanism == SCRAM_SHA_1 || mechanism == SCRAM_SHA_256; - } /** @@ -411,14 +418,16 @@ private boolean mechanismRequiresPassword(@Nullable final AuthenticationMechanis * @param the mechanism property type */ MongoCredential(final MongoCredential from, final String mechanismPropertyKey, final T mechanismPropertyValue) { - notNull("mechanismPropertyKey", mechanismPropertyKey); - - this.mechanism = from.mechanism; - this.userName = from.userName; - this.source = from.source; - this.password = from.password; - this.mechanismProperties = new HashMap<>(from.mechanismProperties); - this.mechanismProperties.put(mechanismPropertyKey.toLowerCase(), mechanismPropertyValue); + this(from.mechanism, from.userName, from.source, from.password, mapWith( + from.mechanismProperties, + notNull("mechanismPropertyKey", mechanismPropertyKey).toLowerCase(), + mechanismPropertyValue)); + } + + private static Map mapWith(final Map map, final String key, final T value) { + HashMap result = new HashMap<>(map); + result.put(key, value); + return result; } /** diff --git a/driver-core/src/main/com/mongodb/internal/connection/AwsAuthenticator.java b/driver-core/src/main/com/mongodb/internal/connection/AwsAuthenticator.java index ec2e5b1e50b..ec0fc3f9c8f 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/AwsAuthenticator.java +++ b/driver-core/src/main/com/mongodb/internal/connection/AwsAuthenticator.java @@ -105,8 +105,7 @@ public byte[] evaluateChallenge(final byte[] challenge) throws SaslException { step++; if (step == 0) { return computeClientFirstMessage(); - } - if (step == 1) { + } else if (step == 1) { return computeClientFinalMessage(challenge); } else { throw new SaslException(format("Too many steps involved in the %s negotiation.", getMechanismName())); @@ -207,14 +206,14 @@ private AwsCredential createAwsCredential() { } return awsCredential; } + } - private byte[] toBson(final BsonDocument document) { - byte[] bytes; - BasicOutputBuffer buffer = new BasicOutputBuffer(); - new BsonDocumentCodec().encode(new BsonBinaryWriter(buffer), document, EncoderContext.builder().build()); - bytes = new byte[buffer.size()]; - System.arraycopy(buffer.getInternalBuffer(), 0, bytes, 0, buffer.getSize()); - return bytes; - } + private static byte[] toBson(final BsonDocument document) { + byte[] bytes; + BasicOutputBuffer buffer = new BasicOutputBuffer(); + new BsonDocumentCodec().encode(new BsonBinaryWriter(buffer), document, EncoderContext.builder().build()); + bytes = new byte[buffer.size()]; + System.arraycopy(buffer.getInternalBuffer(), 0, bytes, 0, buffer.getSize()); + return bytes; } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java b/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java index 8889960a5e7..05d4dfa4ca8 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java +++ b/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java @@ -18,12 +18,12 @@ import com.mongodb.AuthenticationMechanism; import com.mongodb.MongoCredential; +import com.mongodb.internal.Locks; import com.mongodb.lang.Nullable; -import java.util.concurrent.locks.Lock; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantLock; - -import static com.mongodb.internal.Locks.withLock; +import java.util.function.Supplier; /** *

This class is not part of the public API and may be removed or changed at any time

@@ -36,7 +36,7 @@ public MongoCredentialWithCache(final MongoCredential credential) { this(credential, null); } - public MongoCredentialWithCache(final MongoCredential credential, @Nullable final Cache cache) { + private MongoCredentialWithCache(final MongoCredential credential, @Nullable final Cache cache) { this.credential = credential; this.cache = cache != null ? cache : new Cache(); } @@ -63,28 +63,23 @@ public void putInCache(final Object key, final Object value) { cache.set(key, value); } - public Lock getLock() { - return cache.lock; + public V withLock(final Supplier k) { + return Locks.withLock(cache.lock, k); } static class Cache { private final ReentrantLock lock = new ReentrantLock(); - private Object cacheKey; - private Object cacheValue; + private final ConcurrentHashMap cache = new ConcurrentHashMap<>(); Object get(final Object key) { - return withLock(lock, () -> { - if (cacheKey != null && cacheKey.equals(key)) { - return cacheValue; - } - return null; + return Locks.withLock(lock, () -> { + return cache.get(key); }); } void set(final Object key, final Object value) { - withLock(lock, () -> { - cacheKey = key; - cacheValue = value; + Locks.withLock(lock, () -> { + cache.put(key, value); }); } } diff --git a/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java b/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java index ebcf81c8532..6ce36cd6f5e 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java +++ b/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java @@ -42,7 +42,6 @@ import static com.mongodb.MongoCredential.JAVA_SUBJECT_KEY; import static com.mongodb.MongoCredential.JAVA_SUBJECT_PROVIDER_KEY; import static com.mongodb.assertions.Assertions.assertNotNull; -import static com.mongodb.internal.Locks.withLock; import static com.mongodb.internal.async.ErrorHandlingResultCallback.errorHandlingCallback; import static com.mongodb.internal.connection.CommandHelper.executeCommand; import static com.mongodb.internal.connection.CommandHelper.executeCommandAsync; @@ -199,7 +198,7 @@ protected Subject getSubject() { @NonNull private SubjectProvider getSubjectProvider() { - return withLock(getMongoCredentialWithCache().getLock(), () -> { + return getMongoCredentialWithCache().withLock(() -> { SubjectProvider subjectProvider = getMongoCredentialWithCache().getFromCache(SUBJECT_PROVIDER_CACHE_KEY, SubjectProvider.class); if (subjectProvider == null) { diff --git a/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java index c5b90aa95fa..0bed81d706b 100644 --- a/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java @@ -241,7 +241,7 @@ protected void testValidAuth() { // We don't allow null passwords without setting the authentication mechanism. return; } else { - fail(String.format("Connection string '%s' should not have throw an exception: %s", input, t)); + fail(String.format("Connection string '%s' should not have thrown an exception: %s", input, t)); } } diff --git a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java index d59ce19e8ba..71c26376412 100644 --- a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java @@ -16,8 +16,10 @@ package com.mongodb; +import com.mongodb.lang.Nullable; import junit.framework.TestCase; import org.bson.BsonDocument; +import org.bson.BsonNull; import org.bson.BsonValue; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,15 +36,14 @@ // See https://github.com/mongodb/specifications/tree/master/source/auth/tests @RunWith(Parameterized.class) public class AuthConnectionStringTest extends TestCase { - private final String filename; - private final String description; private final String input; private final BsonDocument definition; - public AuthConnectionStringTest(final String filename, final String description, final String input, - final BsonDocument definition) { - this.filename = filename; - this.description = description; + public AuthConnectionStringTest( + final String filename, + final String description, + final String input, + final BsonDocument definition) { this.input = input; this.definition = definition; } @@ -62,8 +63,11 @@ public static Collection data() throws URISyntaxException, IOException for (File file : JsonPoweredTestHelper.getTestFiles("/auth")) { BsonDocument testDocument = JsonPoweredTestHelper.getTestDocument(file); for (BsonValue test : testDocument.getArray("tests")) { - data.add(new Object[]{file.getName(), test.asDocument().getString("description").getValue(), - test.asDocument().getString("uri").getValue(), test.asDocument()}); + data.add(new Object[]{ + file.getName(), + test.asDocument().getString("description").getValue(), + test.asDocument().getString("uri").getValue(), + test.asDocument()}); } } return data; @@ -71,27 +75,18 @@ public static Collection data() throws URISyntaxException, IOException private void testInvalidUris() { Throwable expectedError = null; - try { - new ConnectionString(input); + getMongoCredential(); } catch (Throwable t) { expectedError = t; } - - assertTrue(String.format("Connection string '%s' should have throw an exception", input), + assertTrue(String.format("Connection string '%s' should have thrown an exception. Instead, %s", input, expectedError), expectedError instanceof IllegalArgumentException); } private void testValidUris() { - ConnectionString connectionString = null; - - try { - connectionString = new ConnectionString(input); - } catch (Throwable t) { - fail(String.format("Connection string '%s' should not have throw an exception: %s", input, t)); - } + MongoCredential credential = getMongoCredential(); - MongoCredential credential = connectionString.getCredential(); if (credential != null) { assertString("credential.source", credential.getSource()); assertString("credential.username", credential.getUserName()); @@ -104,9 +99,21 @@ private void testValidUris() { } assertMechanism("credential.mechanism", credential.getMechanism()); + } else { + if (!getExpectedValue("credential").equals(BsonNull.VALUE)) { + fail(String.format("Connection string '%s' should not produce credentials", input)); + } } } + @Nullable + private MongoCredential getMongoCredential() { + ConnectionString connectionString; + connectionString = new ConnectionString(input); + MongoCredential credential = connectionString.getCredential(); + return credential; + } + private void assertString(final String key, final String actual) { BsonValue expected = getExpectedValue(key); @@ -133,27 +140,32 @@ private void assertMechanism(final String key, final String actual) { private void assertMechanismProperties(final MongoCredential credential) { BsonValue expected = getExpectedValue("credential.mechanism_properties"); - - if (!expected.isNull()) { - BsonDocument document = expected.asDocument(); - for (String key : document.keySet()) { - if (document.get(key).isString()) { - String expectedValue = document.getString(key).getValue(); - - // If the mechanism is "GSSAPI", the default SERVICE_NAME, which is stated as "mongodb" in the specification, - // is set to null in the driver. - if (credential.getMechanism().equals("GSSAPI") && key.equals("SERVICE_NAME") && expectedValue.equals("mongodb")) { - assertNull(credential.getMechanismProperty(key, null)); - } else { - assertEquals(expectedValue, credential.getMechanismProperty(key, null)); - } + if (expected.isNull()) { + return; + } + BsonDocument document = expected.asDocument(); + for (String key : document.keySet()) { + Object actualMechanismProperty = credential.getMechanismProperty(key, null); + if (document.get(key).isString()) { + String expectedValue = document.getString(key).getValue(); + // If the mechanism is "GSSAPI", the default SERVICE_NAME, which is stated as "mongodb" in the specification, + // is set to null in the driver. + if (credential.getMechanism().equals("GSSAPI") && key.equals("SERVICE_NAME") && expectedValue.equals("mongodb")) { + assertNull(actualMechanismProperty); } else { - assertEquals(document.getBoolean(key).getValue(), credential.getMechanismProperty(key, (Boolean) null).booleanValue()); + assertEquals(expectedValue, actualMechanismProperty); } + } else if ((document.get(key).isBoolean())) { + boolean expectedValue = document.getBoolean(key).getValue(); + assertNotNull(actualMechanismProperty); + assertEquals(expectedValue, actualMechanismProperty); + } else { + fail("unsupported property type"); } } } + @Nullable private BsonValue getExpectedValue(final String key) { BsonValue expected = definition; if (key.contains(".")) { From 856b3eaaf1d56864450690bc504d2a215c53dc58 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Mon, 3 Apr 2023 13:30:07 -0600 Subject: [PATCH 2/9] Remove extra not, revert typo fix, revert cache change --- .../connection/MongoCredentialWithCache.java | 23 ++++++++++++------- .../mongodb/AbstractConnectionStringTest.java | 2 +- .../com/mongodb/AuthConnectionStringTest.java | 2 +- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java b/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java index 05d4dfa4ca8..4bf8cd0e1c8 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java +++ b/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java @@ -21,7 +21,6 @@ import com.mongodb.internal.Locks; import com.mongodb.lang.Nullable; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Supplier; @@ -30,15 +29,15 @@ */ public class MongoCredentialWithCache { private final MongoCredential credential; - private final Cache cache; + private final SingleValueCache cache; public MongoCredentialWithCache(final MongoCredential credential) { this(credential, null); } - private MongoCredentialWithCache(final MongoCredential credential, @Nullable final Cache cache) { + private MongoCredentialWithCache(final MongoCredential credential, @Nullable final SingleValueCache cache) { this.credential = credential; - this.cache = cache != null ? cache : new Cache(); + this.cache = cache != null ? cache : new SingleValueCache(); } public MongoCredentialWithCache withMechanism(final AuthenticationMechanism mechanism) { @@ -59,6 +58,9 @@ public T getFromCache(final Object key, final Class clazz) { return clazz.cast(cache.get(key)); } + /** + * Putting a key and value will overwrite any prior key and value. + */ public void putInCache(final Object key, final Object value) { cache.set(key, value); } @@ -67,19 +69,24 @@ public V withLock(final Supplier k) { return Locks.withLock(cache.lock, k); } - static class Cache { + static class SingleValueCache { private final ReentrantLock lock = new ReentrantLock(); - private final ConcurrentHashMap cache = new ConcurrentHashMap<>(); + private Object cacheKey; + private Object cacheValue; Object get(final Object key) { return Locks.withLock(lock, () -> { - return cache.get(key); + if (cacheKey != null && cacheKey.equals(key)) { + return cacheValue; + } + return null; }); } void set(final Object key, final Object value) { Locks.withLock(lock, () -> { - cache.put(key, value); + cacheKey = key; + cacheValue = value; }); } } diff --git a/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java index 0bed81d706b..c5b90aa95fa 100644 --- a/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AbstractConnectionStringTest.java @@ -241,7 +241,7 @@ protected void testValidAuth() { // We don't allow null passwords without setting the authentication mechanism. return; } else { - fail(String.format("Connection string '%s' should not have thrown an exception: %s", input, t)); + fail(String.format("Connection string '%s' should not have throw an exception: %s", input, t)); } } diff --git a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java index 71c26376412..8d39d502ccd 100644 --- a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java @@ -101,7 +101,7 @@ private void testValidUris() { assertMechanism("credential.mechanism", credential.getMechanism()); } else { if (!getExpectedValue("credential").equals(BsonNull.VALUE)) { - fail(String.format("Connection string '%s' should not produce credentials", input)); + fail(String.format("Connection string '%s' should produce credentials", input)); } } } From 822d0624b54c2d5a15e922a1adee0815826357d0 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Thu, 18 May 2023 09:09:24 -0600 Subject: [PATCH 3/9] Remove getMongoCredential. --- .../unit/com/mongodb/AuthConnectionStringTest.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java index 8d39d502ccd..7ebddb5fb9d 100644 --- a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java @@ -76,7 +76,7 @@ public static Collection data() throws URISyntaxException, IOException private void testInvalidUris() { Throwable expectedError = null; try { - getMongoCredential(); + new ConnectionString(input).getCredential(); } catch (Throwable t) { expectedError = t; } @@ -85,7 +85,7 @@ private void testInvalidUris() { } private void testValidUris() { - MongoCredential credential = getMongoCredential(); + MongoCredential credential = new ConnectionString(input).getCredential(); if (credential != null) { assertString("credential.source", credential.getSource()); @@ -106,14 +106,6 @@ private void testValidUris() { } } - @Nullable - private MongoCredential getMongoCredential() { - ConnectionString connectionString; - connectionString = new ConnectionString(input); - MongoCredential credential = connectionString.getCredential(); - return credential; - } - private void assertString(final String key, final String actual) { BsonValue expected = getExpectedValue(key); From ad9b6c2351764c35713b7bf1f5786fccb0889570 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Thu, 18 May 2023 10:13:56 -0600 Subject: [PATCH 4/9] Formatting --- .../main/com/mongodb/ConnectionString.java | 6 ++---- .../src/main/com/mongodb/MongoCredential.java | 21 ++++++------------- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/driver-core/src/main/com/mongodb/ConnectionString.java b/driver-core/src/main/com/mongodb/ConnectionString.java index b84125fb436..b597740a9ce 100644 --- a/driver-core/src/main/com/mongodb/ConnectionString.java +++ b/driver-core/src/main/com/mongodb/ConnectionString.java @@ -800,10 +800,8 @@ private UuidRepresentation createUuidRepresentation(final String value) { } @Nullable - private MongoCredential createCredentials( - final Map> optionsMap, - @Nullable final String userName, - @Nullable final char[] password) { + private MongoCredential createCredentials(final Map> optionsMap, + @Nullable final String userName, @Nullable final char[] password) { AuthenticationMechanism mechanism = null; String authSource = null; String gssapiServiceName = null; diff --git a/driver-core/src/main/com/mongodb/MongoCredential.java b/driver-core/src/main/com/mongodb/MongoCredential.java index c52e1d03f09..d1d0cd26fa0 100644 --- a/driver-core/src/main/com/mongodb/MongoCredential.java +++ b/driver-core/src/main/com/mongodb/MongoCredential.java @@ -362,20 +362,13 @@ public MongoCredential withMechanism(final AuthenticationMechanism mechanism) { * @param source the source of the user name, typically a database name * @param password the password */ - MongoCredential( - @Nullable final AuthenticationMechanism mechanism, - @Nullable final String userName, - final String source, - @Nullable final char[] password) { + MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, + final String source, @Nullable final char[] password) { this(mechanism, userName, source, password, Collections.emptyMap()); } - MongoCredential( - @Nullable final AuthenticationMechanism mechanism, - @Nullable final String userName, - final String source, - @Nullable final char[] password, - final Map mechanismProperties) { + MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, + final String source, @Nullable final char[] password, final Map mechanismProperties) { if (userName == null && !Arrays.asList(MONGODB_X509, MONGODB_AWS).contains(mechanism)) { throw new IllegalArgumentException("username can not be null"); @@ -418,10 +411,8 @@ private boolean mechanismRequiresPassword(@Nullable final AuthenticationMechanis * @param the mechanism property type */ MongoCredential(final MongoCredential from, final String mechanismPropertyKey, final T mechanismPropertyValue) { - this(from.mechanism, from.userName, from.source, from.password, mapWith( - from.mechanismProperties, - notNull("mechanismPropertyKey", mechanismPropertyKey).toLowerCase(), - mechanismPropertyValue)); + this(from.mechanism, from.userName, from.source, from.password, mapWith(from.mechanismProperties, notNull( + "mechanismPropertyKey", mechanismPropertyKey).toLowerCase(), mechanismPropertyValue)); } private static Map mapWith(final Map map, final String key, final T value) { From d46f1220cd9ef10e1c56b37635a3d966c1c7ce19 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Thu, 18 May 2023 10:37:56 -0600 Subject: [PATCH 5/9] Formatting --- .../src/main/com/mongodb/ConnectionString.java | 4 ++-- .../src/main/com/mongodb/MongoCredential.java | 8 ++++---- .../unit/com/mongodb/AuthConnectionStringTest.java | 14 ++++---------- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/driver-core/src/main/com/mongodb/ConnectionString.java b/driver-core/src/main/com/mongodb/ConnectionString.java index b597740a9ce..cd8f359f35c 100644 --- a/driver-core/src/main/com/mongodb/ConnectionString.java +++ b/driver-core/src/main/com/mongodb/ConnectionString.java @@ -800,8 +800,8 @@ private UuidRepresentation createUuidRepresentation(final String value) { } @Nullable - private MongoCredential createCredentials(final Map> optionsMap, - @Nullable final String userName, @Nullable final char[] password) { + private MongoCredential createCredentials(final Map> optionsMap, @Nullable final String userName, + @Nullable final char[] password) { AuthenticationMechanism mechanism = null; String authSource = null; String gssapiServiceName = null; diff --git a/driver-core/src/main/com/mongodb/MongoCredential.java b/driver-core/src/main/com/mongodb/MongoCredential.java index d1d0cd26fa0..ffa2a3c4e02 100644 --- a/driver-core/src/main/com/mongodb/MongoCredential.java +++ b/driver-core/src/main/com/mongodb/MongoCredential.java @@ -362,13 +362,13 @@ public MongoCredential withMechanism(final AuthenticationMechanism mechanism) { * @param source the source of the user name, typically a database name * @param password the password */ - MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, - final String source, @Nullable final char[] password) { + MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, final String source, + @Nullable final char[] password) { this(mechanism, userName, source, password, Collections.emptyMap()); } - MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, - final String source, @Nullable final char[] password, final Map mechanismProperties) { + MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, final String source, + @Nullable final char[] password, final Map mechanismProperties) { if (userName == null && !Arrays.asList(MONGODB_X509, MONGODB_AWS).contains(mechanism)) { throw new IllegalArgumentException("username can not be null"); diff --git a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java index 7ebddb5fb9d..b2c8e07adc3 100644 --- a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java @@ -39,11 +39,8 @@ public class AuthConnectionStringTest extends TestCase { private final String input; private final BsonDocument definition; - public AuthConnectionStringTest( - final String filename, - final String description, - final String input, - final BsonDocument definition) { + public AuthConnectionStringTest(final String filename, final String description, final String input, + final BsonDocument definition) { this.input = input; this.definition = definition; } @@ -63,11 +60,8 @@ public static Collection data() throws URISyntaxException, IOException for (File file : JsonPoweredTestHelper.getTestFiles("/auth")) { BsonDocument testDocument = JsonPoweredTestHelper.getTestDocument(file); for (BsonValue test : testDocument.getArray("tests")) { - data.add(new Object[]{ - file.getName(), - test.asDocument().getString("description").getValue(), - test.asDocument().getString("uri").getValue(), - test.asDocument()}); + data.add(new Object[]{file.getName(), test.asDocument().getString("description").getValue(), + test.asDocument().getString("uri").getValue(), test.asDocument()}); } } return data; From a57248367ca5caf23a5871a64efa7c013406655d Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Thu, 18 May 2023 17:32:39 -0600 Subject: [PATCH 6/9] Revert cache changes --- .../connection/MongoCredentialWithCache.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java b/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java index 4bf8cd0e1c8..6b0f6e4ec3c 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java +++ b/driver-core/src/main/com/mongodb/internal/connection/MongoCredentialWithCache.java @@ -18,26 +18,27 @@ import com.mongodb.AuthenticationMechanism; import com.mongodb.MongoCredential; -import com.mongodb.internal.Locks; import com.mongodb.lang.Nullable; +import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Supplier; + +import static com.mongodb.internal.Locks.withLock; /** *

This class is not part of the public API and may be removed or changed at any time

*/ public class MongoCredentialWithCache { private final MongoCredential credential; - private final SingleValueCache cache; + private final Cache cache; public MongoCredentialWithCache(final MongoCredential credential) { this(credential, null); } - private MongoCredentialWithCache(final MongoCredential credential, @Nullable final SingleValueCache cache) { + private MongoCredentialWithCache(final MongoCredential credential, @Nullable final Cache cache) { this.credential = credential; - this.cache = cache != null ? cache : new SingleValueCache(); + this.cache = cache != null ? cache : new Cache(); } public MongoCredentialWithCache withMechanism(final AuthenticationMechanism mechanism) { @@ -58,24 +59,21 @@ public T getFromCache(final Object key, final Class clazz) { return clazz.cast(cache.get(key)); } - /** - * Putting a key and value will overwrite any prior key and value. - */ public void putInCache(final Object key, final Object value) { cache.set(key, value); } - public V withLock(final Supplier k) { - return Locks.withLock(cache.lock, k); + public Lock getLock() { + return cache.lock; } - static class SingleValueCache { + static class Cache { private final ReentrantLock lock = new ReentrantLock(); private Object cacheKey; private Object cacheValue; Object get(final Object key) { - return Locks.withLock(lock, () -> { + return withLock(lock, () -> { if (cacheKey != null && cacheKey.equals(key)) { return cacheValue; } @@ -84,7 +82,7 @@ Object get(final Object key) { } void set(final Object key, final Object value) { - Locks.withLock(lock, () -> { + withLock(lock, () -> { cacheKey = key; cacheValue = value; }); From 422622628879fc026680b0f115c4a2af67d6d54d Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Fri, 19 May 2023 08:56:34 -0600 Subject: [PATCH 7/9] Fix race, more reverts --- bson/src/test/unit/util/ThreadTestHelpers.java | 4 ++-- .../com/mongodb/internal/connection/SaslAuthenticator.java | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bson/src/test/unit/util/ThreadTestHelpers.java b/bson/src/test/unit/util/ThreadTestHelpers.java index 6267f574ecd..d87c1e4d23b 100644 --- a/bson/src/test/unit/util/ThreadTestHelpers.java +++ b/bson/src/test/unit/util/ThreadTestHelpers.java @@ -19,6 +19,7 @@ import org.opentest4j.MultipleFailuresError; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; @@ -34,7 +35,7 @@ public static void executeAll(final int nThreads, final Runnable c) { try { service = Executors.newFixedThreadPool(10); CountDownLatch latch = new CountDownLatch(nThreads); - List failures = new ArrayList<>(); + List failures = Collections.synchronizedList(new ArrayList<>()); for (int i = 0; i < nThreads; i++) { service.submit(() -> { try { @@ -62,5 +63,4 @@ public static void executeAll(final int nThreads, final Runnable c) { } } } - } diff --git a/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java b/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java index 6ce36cd6f5e..ebcf81c8532 100644 --- a/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java +++ b/driver-core/src/main/com/mongodb/internal/connection/SaslAuthenticator.java @@ -42,6 +42,7 @@ import static com.mongodb.MongoCredential.JAVA_SUBJECT_KEY; import static com.mongodb.MongoCredential.JAVA_SUBJECT_PROVIDER_KEY; import static com.mongodb.assertions.Assertions.assertNotNull; +import static com.mongodb.internal.Locks.withLock; import static com.mongodb.internal.async.ErrorHandlingResultCallback.errorHandlingCallback; import static com.mongodb.internal.connection.CommandHelper.executeCommand; import static com.mongodb.internal.connection.CommandHelper.executeCommandAsync; @@ -198,7 +199,7 @@ protected Subject getSubject() { @NonNull private SubjectProvider getSubjectProvider() { - return getMongoCredentialWithCache().withLock(() -> { + return withLock(getMongoCredentialWithCache().getLock(), () -> { SubjectProvider subjectProvider = getMongoCredentialWithCache().getFromCache(SUBJECT_PROVIDER_CACHE_KEY, SubjectProvider.class); if (subjectProvider == null) { From 4e59f976161a4baabd5a7be40e22e7958efee0c7 Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Fri, 19 May 2023 14:59:24 -0600 Subject: [PATCH 8/9] PR fixes --- bson/src/test/unit/util/ThreadTestHelpers.java | 3 ++- .../src/test/unit/com/mongodb/AuthConnectionStringTest.java | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bson/src/test/unit/util/ThreadTestHelpers.java b/bson/src/test/unit/util/ThreadTestHelpers.java index d87c1e4d23b..a4767c503f9 100644 --- a/bson/src/test/unit/util/ThreadTestHelpers.java +++ b/bson/src/test/unit/util/ThreadTestHelpers.java @@ -33,7 +33,7 @@ private ThreadTestHelpers() { public static void executeAll(final int nThreads, final Runnable c) { ExecutorService service = null; try { - service = Executors.newFixedThreadPool(10); + service = Executors.newFixedThreadPool(nThreads); CountDownLatch latch = new CountDownLatch(nThreads); List failures = Collections.synchronizedList(new ArrayList<>()); for (int i = 0; i < nThreads; i++) { @@ -50,6 +50,7 @@ public static void executeAll(final int nThreads, final Runnable c) { try { latch.await(); } catch (InterruptedException e) { + Thread.currentThread().interrupt(); throw new RuntimeException(e); } if (!failures.isEmpty()) { diff --git a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java index b2c8e07adc3..22186228454 100644 --- a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java @@ -151,7 +151,6 @@ private void assertMechanismProperties(final MongoCredential credential) { } } - @Nullable private BsonValue getExpectedValue(final String key) { BsonValue expected = definition; if (key.contains(".")) { From 12def7953efc515597999519d426fb41040b1d5d Mon Sep 17 00:00:00 2001 From: Maxim Katcharov Date: Thu, 25 May 2023 07:35:52 -0600 Subject: [PATCH 9/9] Checkstyle --- .../src/test/unit/com/mongodb/AuthConnectionStringTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java index 22186228454..dfb81ba8de4 100644 --- a/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java +++ b/driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java @@ -16,7 +16,6 @@ package com.mongodb; -import com.mongodb.lang.Nullable; import junit.framework.TestCase; import org.bson.BsonDocument; import org.bson.BsonNull;