HttpClientFactory: Expose a method to customize the underlying

CHANGELOG.md

@@ -10,6 +10,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
 
 ## Unreleased ([details][unreleased changes details])
 
+## Added
+
+- #3205 - HttpClientFactory: Expose a method to customize the underlying HttpClient
 
 ## 6.3.0 - 2023-10-25
 

bundle/src/main/java/com/adobe/acs/commons/http/HttpClientFactory.java

@@ -17,21 +17,35 @@
  */
 package com.adobe.acs.commons.http;
 
+import java.util.function.Consumer;
+
+import org.apache.http.client.HttpClient;
 import org.apache.http.client.fluent.Executor;
 import org.apache.http.client.fluent.Request;
+import org.apache.http.impl.client.HttpClientBuilder;
 
 /**
- * Factory for building pre-configured HttpClient Fluent Executor and Request objects
- * based a configure host, port and (optionally) username/password.
+ * Factory for building pre-configured HttpClient Fluent {@link Executor} and {@link Request} objects
+ * based on a configured host, port and (optionally) username/password.
  *
- * Factories will generally be accessed by service lookup using the factory.name property.
+ * Each OSGi configuration with factory PID {@code com.adobe.acs.commons.http.impl.HttpClientFactoryImpl} exposes one service of this kind.
+ * The individual factories will generally be accessed by service lookup using the {@code factory.name} property.
+ * Despite the name each instance of this interface only holds a single {@link Executor} instance (bound to a single {@code HttpClient}).
  */
 public interface HttpClientFactory {
 
     /**
-     * Get the configured Executor object from this factory.
+     * Customizes the underlying {@link HttpClientBuilder} which is used to create the singleton http client and executor
+     * @param builderCustomizer a {@link Consumer} taking the {@link HttpClientBuilder} initialized with the configured basic options.
+     * @throws IllegalStateException in case {@link #getExecutor()} has been called already
+     * @since 2.1.0 (Bundle version 6.4.0)
+     */
+    void customize(Consumer<HttpClientBuilder> builderCustomizer);
+
+    /**
+     * Get the singleton {@link Executor} object.
      *
-     * @return an Executor object
+     * @return the executor
      */
     Executor getExecutor();
 

bundle/src/main/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImpl.java

@@ -17,15 +17,15 @@
  */
 package com.adobe.acs.commons.http.impl;
 
-import com.adobe.acs.commons.http.HttpClientFactory;
-import org.apache.felix.scr.annotations.Activate;
-import org.apache.felix.scr.annotations.Component;
-import org.apache.felix.scr.annotations.ConfigurationPolicy;
-import org.apache.felix.scr.annotations.Deactivate;
-import org.apache.felix.scr.annotations.Property;
-import org.apache.felix.scr.annotations.Reference;
-import org.apache.felix.scr.annotations.Service;
+import java.io.IOException;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.util.function.Consumer;
+
 import org.apache.http.HttpHost;
+import org.apache.http.auth.Credentials;
+import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.fluent.Executor;
 import org.apache.http.client.fluent.Request;
@@ -36,87 +36,90 @@
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.osgi.services.HttpClientBuilderFactory;
 import org.apache.http.ssl.SSLContextBuilder;
-import org.apache.sling.commons.osgi.PropertiesUtil;
-
-import java.io.IOException;
-import java.util.Map;
-
-@Component(
-        label = "ACS AEM Commons - Http Components Fluent Executor Factory",
-        description = "ACS AEM Commons - Http Components Fluent Executor Factory",
-        configurationFactory = true, policy = ConfigurationPolicy.REQUIRE,
-        metatype = true)
-@Service
-@Property(label = "Factory Name", description = "Name of this factory", name = "factory.name")
-public class HttpClientFactoryImpl implements HttpClientFactory {
-
-    public static final boolean DEFAULT_USE_SSL = false;
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
+import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.metatype.annotations.AttributeDefinition;
+import org.osgi.service.metatype.annotations.AttributeType;
+import org.osgi.service.metatype.annotations.ObjectClassDefinition;
 
-    public static final boolean DEFAULT_DISABLE_CERT_CHECK = false;
-
-    public static final int DEFAULT_CONNECT_TIMEOUT = 30000;
+import com.adobe.acs.commons.http.HttpClientFactory;
 
-    public static final int DEFAULT_SOCKET_TIMEOUT = 30000;
+@Component()
+public class HttpClientFactoryImpl implements HttpClientFactory {
 
-    @Property(label = "host name", description = "host name")
-    private static final String PROP_HOST_DOMAIN = "hostname";
+    @ObjectClassDefinition(factoryPid = "com.adobe.acs.commons.http.impl.HttpClientFactoryImpl",
+            name="ACS AEM Commons - Http Components Fluent Executor Factory",
+            description = "ACS AEM Commons - Http Components Fluent Executor Factory"
+            )
+    @interface Config {
+        @AttributeDefinition(name = "Factory Name", description = "Name of this factory")
+        String factory_name();
 
-    @Property(label = "port", description = "port")
-    private static final String PROP_GATEWAY_PORT = "port";
+        @AttributeDefinition(name = "host name", description="Mandatory")
+        String hostname();
 
-    @Property(label = "Use SSL", description = "Select it if only using https connection for calls.", boolValue = DEFAULT_USE_SSL)
-    private static final String PROP_USE_SSL = "use.ssl";
+        @AttributeDefinition(name = "port", description="Mandatory")
+        int port();
 
-    @Property(label = "Disable certificate check", description = "If selected it will disable certificate check for the SSL connection.",
-            boolValue = DEFAULT_DISABLE_CERT_CHECK)
-    private static final String PROP_DISABLE_CERT_CHECK = "disable.certificate.check";
+        @AttributeDefinition(name = "Use SSL", description = "Select it if only using https connection for calls.")
+        boolean use_ssl() default false;
 
-    @Property(label = "Username", description = "Username for requests (using basic authentication)")
-    private static final String PROP_USERNAME = "username";
+        @AttributeDefinition(name = "Disable certificate ceck", description = "If selected it will disable certificate check for the SSL connection.")
+        boolean disable_certificate_check() default false;
 
-    @Property(label = "Password", description = "Password for requests (using basic authentication)")
-    @SuppressWarnings("squid:S2068")
-    private static final String PROP_PASSWORD = "password";
+        @AttributeDefinition(name = "Username", description = "Username for requests (using basic authentication)")
+        String username();
 
-    @Property(label = "Socket Timeout", description = "Socket timeout in milliseconds", intValue = DEFAULT_SOCKET_TIMEOUT)
-    private static final String PROP_SO_TIMEOUT = "so.timeout";
+        @AttributeDefinition(name = "Password", description = "Password for requests (using basic authentication)", type = AttributeType.PASSWORD)
+        @SuppressWarnings("squid:S2068")
+        String password();
 
-    @Property(label = "Connect Timeout", description = "Connect timeout in milliseconds", intValue = DEFAULT_CONNECT_TIMEOUT)
-    private static final String PROP_CONNECT_TIMEOUT = "conn.timeout";
+        @AttributeDefinition(name = "Socket Timeout", description = "Socket timeout in milliseconds")
+        int so_timeout() default 30000;
 
-    @Reference
-    private HttpClientBuilderFactory httpClientBuilderFactory;
+        @AttributeDefinition(name = "Connect Timeout", description = "Connect timeout in milliseconds")
+        int conn_timeout() default 30000;
+
+        // Internal Name hint for web console.
+        String webconsole_configurationFactory_nameHint() default "Factory Name: {factory.name}";
+    }
+
+    private final HttpClientBuilder builder;
+    private final HttpHost httpHost;
+    private final Credentials credentials;
+    private final String baseUrl;
 
     private Executor executor;
-    private String baseUrl;
     private CloseableHttpClient httpClient;
 
     @Activate
-    protected void activate(Map<String, Object> config) throws Exception {
-        boolean useSSL = PropertiesUtil.toBoolean(config.get(PROP_USE_SSL), DEFAULT_USE_SSL);
+    public HttpClientFactoryImpl(HttpClientFactoryImpl.Config config, @Reference HttpClientBuilderFactory httpClientBuilderFactory) throws KeyManagementException, NoSuchAlgorithmException, KeyStoreException {
+        this(config.use_ssl(), config.hostname(), config.port(), config.so_timeout(),
+                config.conn_timeout(), config.disable_certificate_check(), config.username(), config.password(),
+                httpClientBuilderFactory);
+    }
 
+    HttpClientFactoryImpl(boolean useSSL, String hostname, int port, int soTimeout, 
+            int connectTimeout, boolean disableCertCheck, String username, String password,
+            HttpClientBuilderFactory httpClientBuilderFactory) throws NoSuchAlgorithmException, KeyStoreException, KeyManagementException {
         String scheme = useSSL ? "https" : "http";
-        String hostname = PropertiesUtil.toString(config.get(PROP_HOST_DOMAIN), null);
-        int port = PropertiesUtil.toInteger(config.get(PROP_GATEWAY_PORT), 0);
 
         if (hostname == null || port == 0) {
             throw new IllegalArgumentException("Configuration not valid. Both host and port must be provided.");
         }
 
         baseUrl = String.format("%s://%s:%s", scheme, hostname, port);
 
-        int connectTimeout = PropertiesUtil.toInteger(config.get(PROP_CONNECT_TIMEOUT), DEFAULT_CONNECT_TIMEOUT);
-        int soTimeout = PropertiesUtil.toInteger(config.get(PROP_SO_TIMEOUT), DEFAULT_SOCKET_TIMEOUT);
-
-        HttpClientBuilder builder = httpClientBuilderFactory.newBuilder();
+        builder = httpClientBuilderFactory.newBuilder();
 
         RequestConfig requestConfig = RequestConfig.custom()
                 .setConnectTimeout(connectTimeout)
                 .setSocketTimeout(soTimeout)
                 .build();
         builder.setDefaultRequestConfig(requestConfig);
 
-        boolean disableCertCheck = PropertiesUtil.toBoolean(config.get(PROP_DISABLE_CERT_CHECK), DEFAULT_DISABLE_CERT_CHECK);
 
         if (useSSL && disableCertCheck) {
             // Disable hostname verification and allow self-signed certificates
@@ -126,15 +129,22 @@
                     sslbuilder.build(), NoopHostnameVerifier.INSTANCE);
             builder.setSSLSocketFactory(sslsf);
         }
+        httpHost = new HttpHost(hostname, port, useSSL ? "https" : "http");
+        if (username != null && password != null) {
+            credentials = new UsernamePasswordCredentials(username, password);
+        } else {
+            credentials = null;
+        }
+    }
+
+    synchronized Executor createExecutor() {
         httpClient = builder.build();
         executor = Executor.newInstance(httpClient);
 
-        String username = PropertiesUtil.toString(config.get(PROP_USERNAME), null);
-        String password = PropertiesUtil.toString(config.get(PROP_PASSWORD), null);
-        if (username != null && password != null) {
-            HttpHost httpHost = new HttpHost(hostname, port, useSSL ? "https" : "http");
-            executor.auth(httpHost, username, password).authPreemptive(httpHost);
+        if (credentials != null) {
+            executor.auth(httpHost, credentials).authPreemptive(httpHost);
         }
+        return executor;
     }
 
     @Deactivate
@@ -148,8 +158,19 @@
         }
     }
 
+    @Override
+    public void customize(Consumer<HttpClientBuilder> builderCustomizer) {
+        if (httpClient != null) {
+            throw new IllegalStateException("The underlying http client has already been created through a call of getExecutor() and can no longer be customized");
+        }
+        builderCustomizer.accept(builder);
+    }
+
     @Override
     public Executor getExecutor() {
+        if (executor == null) {
+            executor = createExecutor();
+        }
         return executor;
     }
 

bundle/src/main/java/com/adobe/acs/commons/http/package-info.java

@@ -18,5 +18,5 @@
 /**
  * Http Injectors.
  */
-@org.osgi.annotation.versioning.Version("2.0.0")
+@org.osgi.annotation.versioning.Version("2.1.0")
 package com.adobe.acs.commons.http;

bundle/src/test/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImplTest.java

@@ -24,7 +24,6 @@
 
 import com.adobe.acs.commons.http.JsonObjectResponseHandler;
 import com.google.gson.JsonObject;
-import junitx.util.PrivateAccessor;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang.RandomStringUtils;
 import org.apache.http.client.fluent.Executor;
@@ -39,8 +38,9 @@
 import org.mockserver.client.MockServerClient;
 import org.mockserver.junit.MockServerRule;
 
-import java.util.HashMap;
-import java.util.Map;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
 
 public class HttpClientFactoryImplTest {
 
@@ -49,24 +49,16 @@ public class HttpClientFactoryImplTest {
 
     private MockServerClient mockServerClient;
 
-    private HttpClientFactoryImpl impl;
-
-    private Map<String, Object> config;
-
     private String username;
 
     private String password;
 
     @Before
     public void setup() throws Exception {
-        config = new HashMap<String, Object>();
         username = RandomStringUtils.randomAlphabetic(5);
         password = RandomStringUtils.randomAlphabetic(6);
         final String authHeaderValue = Base64.encodeBase64String((username + ":" + password).getBytes());
 
-        config.put("hostname", "localhost");
-        config.put("port", mockServerRule.getPort().intValue());
-
         mockServerClient.when(
                 request().withMethod("GET").withPath("/anon")
         ).respond(
@@ -83,8 +75,16 @@ public void setup() throws Exception {
         ).respond(
                 response().withStatusCode(200).withBody("OK")
         );
-        impl = new HttpClientFactoryImpl();
-        PrivateAccessor.setField(impl, "httpClientBuilderFactory", new HttpClientBuilderFactory() {
+    }
+
+    protected HttpClientFactoryImpl createFactory() throws KeyManagementException, NoSuchAlgorithmException, KeyStoreException {
+        return createFactory(false, null, null);
+    }
+
+    protected HttpClientFactoryImpl createFactory(boolean isDisabledCertCheck, String username, String password) throws KeyManagementException, NoSuchAlgorithmException, KeyStoreException {
+        return new HttpClientFactoryImpl(false, "localhost", mockServerRule.getPort().intValue(),
+                30000, 30000, isDisabledCertCheck, username, password,
+                new HttpClientBuilderFactory() {
             @Override
             public HttpClientBuilder newBuilder() {
                 return HttpClients.custom();
@@ -94,8 +94,7 @@ public HttpClientBuilder newBuilder() {
 
     @Test
     public void testAnonymousGet() throws Exception {
-        impl.activate(config);
-
+        HttpClientFactoryImpl impl = createFactory();
         Request get = impl.get("/anon");
         Executor exec = impl.getExecutor();
         String str = exec.execute(get).handleResponse(new BasicResponseHandler());
@@ -104,8 +103,7 @@ public void testAnonymousGet() throws Exception {
 
     @Test
     public void testJsonGet() throws Exception {
-        impl.activate(config);
-
+        HttpClientFactoryImpl impl = createFactory();
         Request get = impl.get("/anonJson");
         Executor exec = impl.getExecutor();
         JsonObject jsonObject = exec.execute(get).handleResponse(new JsonObjectResponseHandler()).getAsJsonObject();
@@ -115,9 +113,7 @@ public void testJsonGet() throws Exception {
 
     @Test
     public void testAuthenticatedGet() throws Exception {
-        config.put("username", username);
-        config.put("password", password);
-        impl.activate(config);
+        HttpClientFactoryImpl impl = createFactory(false, username, password);
 
         Request get = impl.get("/auth");
         Executor exec = impl.getExecutor();
@@ -129,9 +125,7 @@ public void testAuthenticatedGet() throws Exception {
     public void testDisableSSLCertCheck() throws Exception {
         // this test doesn't actually test anything, but at least ensures that the SSL
         // initialization code doesn't throw exceptions
-        config.put("use.ssl", true);
-        config.put("disable.certificate.check", true);
-        impl.activate(config);
+        HttpClientFactoryImpl impl = createFactory(false, null, null);
     }
 
 }