diff --git a/CHANGELOG.md b/CHANGELOG.md index fb8c75e9c2..4e7c0c7fd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) ## Unreleased ([details][unreleased changes details]) -## Added +## Added +- #3205 - HttpClientFactory: Expose a method to customize the underlying HttpClient - #3209 - WARN org.apache.sling.models.impl.ModelAdapterFactory - Cannot provide default for java.util.List ## 6.3.0 - 2023-10-25 diff --git a/bundle/src/main/java/com/adobe/acs/commons/http/HttpClientFactory.java b/bundle/src/main/java/com/adobe/acs/commons/http/HttpClientFactory.java index 2ed3ed1b7d..80c9020b99 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/http/HttpClientFactory.java +++ b/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 builderCustomizer); + + /** + * Get the singleton {@link Executor} object. * - * @return an Executor object + * @return the executor */ Executor getExecutor(); diff --git a/bundle/src/main/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImpl.java b/bundle/src/main/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImpl.java index e807eacebb..b1383fbec5 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImpl.java +++ b/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,68 +36,75 @@ 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 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."); @@ -105,10 +112,7 @@ protected void activate(Map config) throws Exception { 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) @@ -116,7 +120,6 @@ protected void activate(Map config) throws Exception { .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 @@ protected void activate(Map config) throws Exception { 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 @@ protected void deactivate() { } } + @Override + public void customize(Consumer 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; } diff --git a/bundle/src/main/java/com/adobe/acs/commons/http/package-info.java b/bundle/src/main/java/com/adobe/acs/commons/http/package-info.java index 236b0f6dcc..e1fee75664 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/http/package-info.java +++ b/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; \ No newline at end of file diff --git a/bundle/src/test/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImplTest.java b/bundle/src/test/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImplTest.java index ed0969f2b6..e0a6d17fb9 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/http/impl/HttpClientFactoryImplTest.java +++ b/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 config; - private String username; private String password; @Before public void setup() throws Exception { - config = new HashMap(); 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); } } \ No newline at end of file diff --git a/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java b/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java index 9da815aeab..14668c4549 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java +++ b/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java @@ -112,16 +112,21 @@ public class ScrMetadataIT { PROPERTIES_TO_IGNORE.add(Constants.SERVICE_VENDOR); COMPONENT_PROPERTIES_TO_IGNORE = new HashSet<>(); - - COMPONENT_PROPERTIES_TO_IGNORE_FOR_TYPE_CHANGE = new HashSet<>(); COMPONENT_PROPERTIES_TO_IGNORE.add("com.adobe.acs.commons.redirects.filter.RedirectFilter:mapUrls"); COMPONENT_PROPERTIES_TO_IGNORE.add("com.adobe.acs.commons.replication.dispatcher.impl.DispatcherFlusherImpl:service.ranking"); + // Property port on component com.adobe.acs.commons.http.impl.HttpClientFactoryImpl has different types (was: {String}, is: {Integer}) + // Property password on component com.adobe.acs.commons.http.impl.HttpClientFactoryImpl has different types (was: {String}, is: {Password}) + COMPONENT_PROPERTIES_TO_IGNORE_FOR_TYPE_CHANGE = new HashSet<>(); + COMPONENT_PROPERTIES_TO_IGNORE_FOR_TYPE_CHANGE.add("com.adobe.acs.commons.http.impl.HttpClientFactoryImpl:port"); + COMPONENT_PROPERTIES_TO_IGNORE_FOR_TYPE_CHANGE.add("com.adobe.acs.commons.http.impl.HttpClientFactoryImpl:password"); + ALLOWED_SCR_NS_URIS = new HashSet<>(); ALLOWED_SCR_NS_URIS.add("http://www.osgi.org/xmlns/scr/v1.0.0"); ALLOWED_SCR_NS_URIS.add("http://www.osgi.org/xmlns/scr/v1.1.0"); ALLOWED_SCR_NS_URIS.add("http://www.osgi.org/xmlns/scr/v1.2.0"); ALLOWED_SCR_NS_URIS.add("http://www.osgi.org/xmlns/scr/v1.3.0"); + ALLOWED_SCR_NS_URIS.add("http://www.osgi.org/xmlns/scr/v1.4.0");// AEM 6.5 supports DS up to (including) 1.4 EXTRACT_VARIABLES = Pattern.compile("\\{([^}]+)}"); }