Refactor tests, credential code (#1102)

Refactoring for:
JAVA-4757

Author: katcharov

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 @@ <T> void roundTrip(final PojoCodecProvider.Builder builder, final T value, final
 
     <T> 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<String> 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);
+        });
     }
 
     <T> void roundTrip(final CodecRegistry registry, final T value, final String json) {

bson/src/test/unit/util/ThreadTestHelpers.java

@@ -0,0 +1,67 @@
+/*
+ * 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.Collections;
+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(nThreads);
+            CountDownLatch latch = new CountDownLatch(nThreads);
+            List<Throwable> failures = Collections.synchronizedList(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) {
+                Thread.currentThread().interrupt();
+                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();
+            }
+        }
+    }
+}

driver-core/src/main/com/mongodb/MongoCredential.java

@@ -369,7 +369,8 @@ public MongoCredential withMechanism(final AuthenticationMechanism mechanism) {
 
     MongoCredential(@Nullable final AuthenticationMechanism mechanism, @Nullable final String userName, final String source,
                     @Nullable final char[] password, final Map<String, Object> mechanismProperties) {
-        if (mechanism != MONGODB_X509 && mechanism != MONGODB_AWS && userName == null) {
+
+        if (userName == null && !Arrays.asList(MONGODB_X509, MONGODB_AWS).contains(mechanism)) {
             throw new IllegalArgumentException("username can not be null");
         }
 
@@ -399,7 +400,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 +411,14 @@ private boolean mechanismRequiresPassword(@Nullable final AuthenticationMechanis
      * @param <T>                    the mechanism property type
      */
     <T> 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 <T> Map<String, Object> mapWith(final Map<String, Object> map, final String key, final T value) {
+        HashMap<String, Object> result = new HashMap<>(map);
+        result.put(key, value);
+        return result;
     }
 
     /**

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;
     }
 }

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

@@ -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();
     }

driver-core/src/test/unit/com/mongodb/AuthConnectionStringTest.java

@@ -18,6 +18,7 @@
 
 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 +35,11 @@
 // 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;
         this.input = input;
         this.definition = definition;
     }
@@ -71,27 +68,18 @@ public static Collection<Object[]> data() throws URISyntaxException, IOException
 
     private void testInvalidUris() {
         Throwable expectedError = null;
-
         try {
-            new ConnectionString(input);
+            new ConnectionString(input).getCredential();
         } 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 = new ConnectionString(input).getCredential();
 
-        MongoCredential credential = connectionString.getCredential();
         if (credential != null) {
             assertString("credential.source", credential.getSource());
             assertString("credential.username", credential.getUserName());
@@ -104,6 +92,10 @@ private void testValidUris() {
             }
 
             assertMechanism("credential.mechanism", credential.getMechanism());
+        } else {
+            if (!getExpectedValue("credential").equals(BsonNull.VALUE)) {
+                fail(String.format("Connection string '%s' should produce credentials", input));
+            }
         }
     }
 
@@ -133,23 +125,27 @@ 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");
             }
         }
     }