Fixing bug with multiple threads using configurators

Ran into this with the Spark connector. We don't need the fix right away in the Spark connector, as we can synchronize the method that creates a client for now.

Author: rjrudin

marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java

@@ -28,13 +28,7 @@
 import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
 import java.time.Instant;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 import java.util.function.Function;
 
 import javax.naming.InvalidNameException;
@@ -63,7 +57,8 @@
  */
 public class DatabaseClientFactory {
 
-  static private List<ClientConfigurator<?>> clientConfigurators = new ArrayList<>();
+  static private List<ClientConfigurator<?>> clientConfigurators = Collections.synchronizedList(new ArrayList<>());
+
   static private HandleFactoryRegistry handleRegistry =
     HandleFactoryRegistryImpl.newDefault();
 

marklogic-client-api/src/test/java/com/marklogic/client/extra/okhttpclient/OkHttpClientConfiguratorTest.java

@@ -0,0 +1,52 @@
+package com.marklogic.client.extra.okhttpclient;
+
+import com.marklogic.client.DatabaseClientFactory;
+import com.marklogic.client.test.Common;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class OkHttpClientConfiguratorTest {
+
+	/**
+	 * Not a test exactly of OkHttp, but rather that multiple threads can add configurators and create clients without
+	 * encountering a ConcurrentModificationException, which used to occur due to the list of configurators in
+	 * DatabaseClientFactory not being synchronized.
+	 * 
+	 * @throws Exception
+	 */
+	@Test
+	void multipleThreads() throws Exception {
+		final int clientsToCreate = 100;
+
+		ExecutorService service = Executors.newFixedThreadPool(16);
+		List<Future> futures = new ArrayList<>();
+		for (int i = 0; i < clientsToCreate; i++) {
+			futures.add(service.submit(() -> {
+				DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) client -> {
+					client.callTimeout(10, TimeUnit.SECONDS);
+				});
+				Common.newClient().release();
+			}));
+		}
+
+		int clientsCreated = 0;
+		for (Future f : futures) {
+			f.get();
+			clientsCreated++;
+		}
+		service.shutdown();
+
+		assertEquals(clientsToCreate, clientsCreated, "The expectation is that many threads can " +
+			"add a configurator and create a client at the same time because DatabaseClientFactory " +
+			"uses a synchronized list for the configurators. So all of the expected clients should be " +
+			"created successfully with no concurrent modification errors occurring.");
+	}
+}