Let #executorService() takes precedence over #register() (#3886)

jansupol · web-flow · commit e8cb0d8caf86 · 2018-07-26T16:29:10.000+02:00
* Let ClientBuilder#executorService takes precedence over ClientBuilder#register(&lt;@ClientAsyncExecutor&gt;)

Signed-off-by: Jan Supol &lt;jan.supol@oracle.com&gt;
diff --git a/core-client/src/main/java/org/glassfish/jersey/client/JerseyClient.java b/core-client/src/main/java/org/glassfish/jersey/client/JerseyClient.java
@@ -71,8 +71,6 @@ public SSLContext getDefaultSslContext() {
     private final LinkedBlockingDeque<WeakReference<JerseyClient.ShutdownHook>> shutdownHooks =
                                         new LinkedBlockingDeque<WeakReference<JerseyClient.ShutdownHook>>();
     private final ReferenceQueue<JerseyClient.ShutdownHook> shReferenceQueue = new ReferenceQueue<JerseyClient.ShutdownHook>();
-    private final ExecutorService executorService;
-    private final ScheduledExecutorService scheduledExecutorService;
 
     /**
      * Client instance shutdown hook.
@@ -134,39 +132,6 @@ protected JerseyClient(final Configuration config,
         this(config, sslContextProvider, verifier, null);
     }
 
-    /**
-     * Create a new Jersey client instance.
-     *
-     * @param config                    jersey client configuration.
-     * @param sslContext                jersey client SSL context.
-     * @param verifier                  jersey client host name verifier.
-     * @param defaultSslContextProvider default SSL context provider.
-     */
-    protected JerseyClient(final Configuration config,
-                           final SSLContext sslContext,
-                           final HostnameVerifier verifier,
-                           final DefaultSslContextProvider defaultSslContextProvider,
-                           ExecutorService executorService,
-                           ScheduledExecutorService scheduledExecutorService) {
-        this(config, sslContext == null ? null : Values.unsafe(sslContext), verifier,
-             defaultSslContextProvider, executorService, scheduledExecutorService);
-    }
-
-    /**
-     * Create a new Jersey client instance.
-     *
-     * @param config             jersey client configuration.
-     * @param sslContextProvider jersey client SSL context provider.
-     * @param verifier           jersey client host name verifier.
-     */
-    protected JerseyClient(final Configuration config,
-                           final UnsafeValue<SSLContext, IllegalStateException> sslContextProvider,
-                           final HostnameVerifier verifier,
-                           ExecutorService executorService,
-                           ScheduledExecutorService scheduledExecutorService) {
-        this(config, sslContextProvider, verifier, null, executorService, scheduledExecutorService);
-    }
-
     /**
      * Create a new Jersey client instance.
      *
@@ -180,15 +145,6 @@ protected JerseyClient(final Configuration config,
                            final UnsafeValue<SSLContext, IllegalStateException> sslContextProvider,
                            final HostnameVerifier verifier,
                            final DefaultSslContextProvider defaultSslContextProvider) {
-        this(config, sslContextProvider, verifier, defaultSslContextProvider, null, null);
-    }
-
-    protected JerseyClient(final Configuration config,
-                           final UnsafeValue<SSLContext, IllegalStateException> sslContextProvider,
-                           final HostnameVerifier verifier,
-                           final DefaultSslContextProvider defaultSslContextProvider,
-                           ExecutorService executorService,
-                           ScheduledExecutorService scheduledExecutorService) {
         this.config = config == null ? new ClientConfig(this) : new ClientConfig(this, config);
 
         if (sslContextProvider == null) {
@@ -216,8 +172,6 @@ protected JerseyClient(final Configuration config,
         }
 
         this.hostnameVerifier = verifier;
-        this.executorService = executorService;
-        this.scheduledExecutorService = scheduledExecutorService;
     }
 
     @Override
@@ -430,11 +384,11 @@ public HostnameVerifier getHostnameVerifier() {
     }
 
     public ExecutorService getExecutorService() {
-        return executorService;
+        return config.getExecutorService();
     }
 
     public ScheduledExecutorService getScheduledExecutorService() {
-        return scheduledExecutorService;
+        return config.getScheduledExecutorService();
     }
 
     @Override
diff --git a/core-client/src/main/java/org/glassfish/jersey/client/JerseyClientBuilder.java b/core-client/src/main/java/org/glassfish/jersey/client/JerseyClientBuilder.java
@@ -44,8 +44,6 @@ public class JerseyClientBuilder extends ClientBuilder {
     private HostnameVerifier hostnameVerifier;
     private SslConfigurator sslConfigurator;
     private SSLContext sslContext;
-    private ExecutorService executorService;
-    private ScheduledExecutorService scheduledExecutorService;
 
     /**
      * Create a new custom-configured {@link JerseyClient} instance.
@@ -124,13 +122,13 @@ public JerseyClientBuilder hostnameVerifier(HostnameVerifier hostnameVerifier) {
 
     @Override
     public ClientBuilder executorService(ExecutorService executorService) {
-        this.executorService = executorService;
+        config.executorService(executorService);
         return this;
     }
 
     @Override
     public ClientBuilder scheduledExecutorService(ScheduledExecutorService scheduledExecutorService) {
-        this.scheduledExecutorService = scheduledExecutorService;
+        config.scheduledExecutorService(scheduledExecutorService);
         return this;
     }
 
@@ -157,8 +155,7 @@ public ClientBuilder readTimeout(long timeout, TimeUnit unit) {
     @Override
     public JerseyClient build() {
         if (sslContext != null) {
-            return new JerseyClient(config, sslContext, hostnameVerifier, null, executorService,
-                                    scheduledExecutorService);
+            return new JerseyClient(config, sslContext, hostnameVerifier, null);
         } else if (sslConfigurator != null) {
             final SslConfigurator sslConfiguratorCopy = sslConfigurator.copy();
             return new JerseyClient(
@@ -169,10 +166,9 @@ public SSLContext get() {
                             return sslConfiguratorCopy.createSSLContext();
                         }
                     }),
-                    hostnameVerifier, executorService, scheduledExecutorService);
+                    hostnameVerifier);
         } else {
-            return new JerseyClient(config, (UnsafeValue<SSLContext, IllegalStateException>) null, hostnameVerifier,
-                                    executorService, scheduledExecutorService);
+            return new JerseyClient(config, (UnsafeValue<SSLContext, IllegalStateException>) null, hostnameVerifier);
         }
     }
 
diff --git a/tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/ExecutorServiceProviderTest.java b/tests/e2e/src/test/java/org/glassfish/jersey/tests/e2e/ExecutorServiceProviderTest.java
@@ -16,12 +16,15 @@
 
 package org.glassfish.jersey.tests.e2e;
 
+import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.IdentityHashMap;
 import java.util.List;
+import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -37,6 +40,8 @@
 import javax.ws.rs.Produces;
 import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.client.ClientRequestContext;
+import javax.ws.rs.client.ClientRequestFilter;
 import javax.ws.rs.container.AsyncResponse;
 import javax.ws.rs.container.Suspended;
 import javax.ws.rs.core.Application;
@@ -52,6 +57,7 @@
 import org.glassfish.jersey.test.JerseyTest;
 import org.junit.Test;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * {@link org.glassfish.jersey.spi.ExecutorServiceProvider} E2E tests.
@@ -137,17 +143,21 @@ public void dispose(final ExecutorService executorService) {
             executorService.shutdownNow();
         }
 
-        private class CustomExecutorService implements ExecutorService {
+        /* package private */ class CustomExecutorService implements ExecutorService {
 
             private final ExecutorService delegate;
             private final AtomicBoolean isCleanedUp;
 
             public CustomExecutorService() {
-                this.isCleanedUp = new AtomicBoolean(false);
-                this.delegate = Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
+                this(Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
                         .setNameFormat("async-request-%d")
                         .setUncaughtExceptionHandler(new JerseyProcessingUncaughtExceptionHandler())
-                        .build());
+                        .build()));
+            }
+
+            public CustomExecutorService(ExecutorService delegate) {
+                this.isCleanedUp = new AtomicBoolean(false);
+                this.delegate = delegate;
 
                 executorCreationCount++;
                 executors.add(this);
@@ -231,6 +241,17 @@ public void execute(Runnable command) {
         }
     }
 
+    public static class SecondCustomExecutorProvider extends CustomExecutorProvider {
+        public static final String NAME_FORMAT = "second-async-request";
+
+        public ExecutorService getExecutorService() {
+            return new CustomExecutorService(Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
+                    .setNameFormat(NAME_FORMAT + "-%d")
+                    .setUncaughtExceptionHandler(new JerseyProcessingUncaughtExceptionHandler())
+                    .build()));
+        }
+    }
+
     private static final CustomExecutorProvider serverExecutorProvider = new CustomExecutorProvider();
 
     @Override
@@ -355,4 +376,56 @@ public void testCustomNamedServerExecutorsInjectionAndReleasing() throws Excepti
 
         setUp(); // re-starting test container to ensure proper post-test tearDown.
     }
+
+    @Test
+    public void testClientBuilderExecutorServiceTakesPrecedenceOverRegistered() throws Exception {
+        serverExecutorProvider.reset();
+        CountDownLatch nameLatch = new CountDownLatch(1);
+        Set<String> threadName = new HashSet<>(1);
+
+        final CustomExecutorProvider executorProvider = new CustomExecutorProvider();
+        Client client = ClientBuilder.newBuilder().register(new ClientRequestFilter() {
+            @Override
+            public void filter(ClientRequestContext requestContext) throws IOException {
+                threadName.add(Thread.currentThread().getName());
+                nameLatch.countDown();
+            }
+        }).executorService(new SecondCustomExecutorProvider().getExecutorService()).register(executorProvider).build();
+
+        client.target(getBaseUri()).path("resource").request().async().get(String.class).get();
+        assertTrue(nameLatch.await(10, TimeUnit.SECONDS));
+        assertEquals(threadName.size(), 1);
+        assertTrue(threadName.iterator().next().startsWith(SecondCustomExecutorProvider.NAME_FORMAT));
+
+        tearDown(); // stopping test container
+        client.close();
+
+        setUp(); // re-starting test container to ensure proper post-test tearDown.
+    }
+
+    @Test
+    public void testRegisteredExecutorServiceDoesNotTakesPrecedenceOverClientBuilder() throws Exception {
+        serverExecutorProvider.reset();
+        CountDownLatch nameLatch = new CountDownLatch(1);
+        Set<String> threadName = new HashSet<>(1);
+
+        final CustomExecutorProvider executorProvider = new CustomExecutorProvider();
+        Client client = ClientBuilder.newBuilder().register(executorProvider).register(new ClientRequestFilter() {
+            @Override
+            public void filter(ClientRequestContext requestContext) throws IOException {
+                threadName.add(Thread.currentThread().getName());
+                nameLatch.countDown();
+            }
+        }).executorService(new SecondCustomExecutorProvider().getExecutorService()).build();
+
+        client.target(getBaseUri()).path("resource").request().async().get(String.class).get();
+        assertTrue(nameLatch.await(10, TimeUnit.SECONDS));
+        assertEquals(threadName.size(), 1);
+        assertTrue(threadName.iterator().next().startsWith(SecondCustomExecutorProvider.NAME_FORMAT));
+
+        tearDown(); // stopping test container
+        client.close();
+
+        setUp(); // re-starting test container to ensure proper post-test tearDown.
+    }
 }
