Merge pull request #1638 from marklogic/feature/configurator-fix

Fixing bug with multiple threads using configurators

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.");
+	}
+}