From c2d6a35a6bab1c41f9c46db857ad69b97978c39b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 17:35:25 +0100 Subject: [PATCH 1/8] Initial --- .../ApacheHttpClient5FactoryBuilder.java | 103 +++++-- .../ConnectionPoolManagerProvider.java | 66 +++++ .../ConnectionPoolManagerProviders.java | 269 ++++++++++++++++++ .../connectivity/ConnectionPoolSettings.java | 79 +++++ .../DefaultApacheHttpClient5Factory.java | 101 +------ .../DefaultConnectionPoolSettings.java | 101 +++++++ .../ConnectionPoolManagerProvidersTest.java | 183 ++++++++++++ .../DefaultApacheHttpClient5CacheTest.java | 5 +- .../DefaultApacheHttpClient5FactoryTest.java | 58 +++- 9 files changed, 828 insertions(+), 137 deletions(-) create mode 100644 cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java create mode 100644 cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java create mode 100644 cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java create mode 100644 cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java create mode 100644 cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java index 527249bb7..616c356f0 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java @@ -3,7 +3,10 @@ import java.time.Duration; import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import lombok.Setter; +import lombok.experimental.Accessors; import org.apache.hc.client5.http.classic.HttpClient; import com.google.common.annotations.Beta; @@ -13,13 +16,77 @@ * * @since 4.20.0 */ +@Accessors( fluent = true ) public class ApacheHttpClient5FactoryBuilder { + /** + * The {@code Upgrade} header. Only {@link ProxyType#INTERNET} has the {@code Upgrade} header by default. + *

+ * {@link TlsUpgrade#DISABLED} only works for {@link ProxyType#INTERNET} + *

+ * {@link TlsUpgrade#ENABLED} only works for {@link ProxyType#ON_PREMISE} + * + * @since 5.14.0 + */ + @Setter @Nonnull - private Duration timeout = DefaultApacheHttpClient5Factory.DEFAULT_TIMEOUT; private TlsUpgrade tlsUpgrade = TlsUpgrade.AUTOMATIC; - private int maxConnectionsTotal = DefaultApacheHttpClient5Factory.DEFAULT_MAX_CONNECTIONS_TOTAL; - private int maxConnectionsPerRoute = DefaultApacheHttpClient5Factory.DEFAULT_MAX_CONNECTIONS_PER_ROUTE; + + /** + * The {@link ConnectionPoolSettings} to use for configuring connection pool managers and request timeouts. + *

+ * This replaces any previously configured settings from {@link #timeout(Duration)}, + * {@link #maxConnectionsTotal(int)}, or {@link #maxConnectionsPerRoute(int)}. + *

+ *

+ * This is an optional parameter. By default, settings use the default values. + *

+ * + * @see DefaultConnectionPoolSettings#ofDefaults() + * @see DefaultConnectionPoolSettings#builder() + * @since 5.XX.0 + */ + @Setter( onMethod_ = @Beta ) + @Nonnull + private DefaultConnectionPoolSettings settings = DefaultConnectionPoolSettings.ofDefaults(); + + /** + * A custom {@link ConnectionPoolManagerProvider} for creating and managing HTTP connection pool managers. + *

+ * This allows customization of how connection managers are created and cached. Use + * {@link ConnectionPoolManagerProviders} to obtain pre-built implementations with common caching strategies: + *

+ * + *

+ * This is an optional parameter. By default, a new connection manager is created for each HTTP client + * without caching. + *

+ * + *

Example Usage

+ * + * 
+     * {@code
+     * // Cache connection managers by tenant to reduce memory consumption
+     * ApacheHttpClient5Factory factory =
+     *     new ApacheHttpClient5FactoryBuilder()
+     *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.byTenant())
+     *         .build();
+     * }
+     *
+ * + * @see ConnectionPoolManagerProvider + * @see ConnectionPoolManagerProviders + * @since 5.XX.0 + */ + @Setter( onMethod_ = @Beta ) + @Nonnull + private ConnectionPoolManagerProvider connectionPoolManagerProvider = ConnectionPoolManagerProviders.noCache(); /** * Enum to control the automatic TLS upgrade feature for insecure connections. @@ -88,7 +155,8 @@ public ApacheHttpClient5FactoryBuilder timeoutInMilliseconds( final int timeoutI @Nonnull public ApacheHttpClient5FactoryBuilder timeout( @Nonnull final Duration timeout ) { - this.timeout = timeout; + settings = + settings.withConnectTimeout(timeout).withSocketTimeout(timeout).withConnectionRequestTimeout(timeout); return this; } @@ -106,23 +174,7 @@ public ApacheHttpClient5FactoryBuilder timeout( @Nonnull final Duration timeout @Nonnull public ApacheHttpClient5FactoryBuilder maxConnectionsTotal( final int maxConnectionsTotal ) { - this.maxConnectionsTotal = maxConnectionsTotal; - return this; - } - - /** - * Sets the {@code Upgrade} header. Only {@link ProxyType#INTERNET} has the {@code Upgrade} header by default. - *

- * {@link TlsUpgrade#DISABLED} only works for {@link ProxyType#INTERNET} - *

- * {@link TlsUpgrade#ENABLED} only works for {@link ProxyType#ON_PREMISE} - * - * @since 5.14.0 - */ - @Nonnull - public ApacheHttpClient5FactoryBuilder tlsUpgrade( @Nonnull final TlsUpgrade tlsUpgrade ) - { - this.tlsUpgrade = tlsUpgrade; + settings = settings.withMaxConnectionsTotal(maxConnectionsTotal); return this; } @@ -141,7 +193,7 @@ public ApacheHttpClient5FactoryBuilder tlsUpgrade( @Nonnull final TlsUpgrade tls @Nonnull public ApacheHttpClient5FactoryBuilder maxConnectionsPerRoute( final int maxConnectionsPerRoute ) { - this.maxConnectionsPerRoute = maxConnectionsPerRoute; + settings = settings.withMaxConnectionsPerRoute(maxConnectionsPerRoute); return this; } @@ -153,11 +205,6 @@ public ApacheHttpClient5FactoryBuilder maxConnectionsPerRoute( final int maxConn @Nonnull public ApacheHttpClient5Factory build() { - return new DefaultApacheHttpClient5Factory( - timeout, - maxConnectionsTotal, - maxConnectionsPerRoute, - null, - tlsUpgrade); + return new DefaultApacheHttpClient5Factory(settings, connectionPoolManagerProvider, null, tlsUpgrade); } } diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java new file mode 100644 index 000000000..4f39189e5 --- /dev/null +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java @@ -0,0 +1,66 @@ +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import org.apache.hc.client5.http.io.HttpClientConnectionManager; + +import com.google.common.annotations.Beta; +import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; + +/** + * Functional interface for creating or retrieving {@link HttpClientConnectionManager} instances. + *

+ * Implementations can choose to cache connection managers based on various strategies (e.g., by tenant, by destination + * name, globally) to reduce memory consumption. Each connection manager typically consumes around 100KB of memory. + *

+ *

+ * Use {@link ConnectionPoolManagerProviders} to obtain pre-built implementations with common caching strategies. + *

+ * + *

Example Usage

+ * + * 
+ * {@code
+ * // Simple lambda implementation (no caching)
+ * ConnectionPoolManagerProvider provider =
+ *     ( settings, dest ) -> PoolingHttpClientConnectionManagerBuilder
+ *         .create()
+ *         .setMaxConnTotal(settings.maxConnectionsTotal())
+ *         .build();
+ *
+ * // Using pre-built providers
+ * ConnectionPoolManagerProvider cachingProvider = ConnectionPoolManagerProviders.byTenant();
+ * }
+ *
+ * + * @see ConnectionPoolManagerProviders + * @see ApacheHttpClient5FactoryBuilder#connectionPoolManagerProvider(ConnectionPoolManagerProvider) + * @since 5.XX.0 + */ +@Beta +@FunctionalInterface +public interface ConnectionPoolManagerProvider +{ + /** + * Gets or creates an {@link HttpClientConnectionManager} for the given destination. + *

+ * Implementations may cache connection managers based on destination properties, tenant context, or other criteria. + * The settings parameter provides the configuration for creating new connection managers. + *

+ * + * @param settings + * The connection pool settings to use when creating a new connection manager. + * @param destination + * The destination properties to create the connection manager for, or {@code null} for a generic + * connection manager. + * @return A connection manager suitable for the given destination. + * @throws HttpClientInstantiationException + * If the connection manager cannot be created. + */ + @Nonnull + HttpClientConnectionManager getConnectionManager( + @Nonnull ConnectionPoolSettings settings, + @Nullable HttpDestinationProperties destination ) + throws HttpClientInstantiationException; +} diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java new file mode 100644 index 000000000..c7c940acc --- /dev/null +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java @@ -0,0 +1,269 @@ +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLContext; + +import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; +import org.apache.hc.client5.http.config.ConnectionConfig; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; +import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier; +import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; +import org.apache.hc.core5.http.io.SocketConfig; +import org.apache.hc.core5.util.Timeout; + +import com.google.common.annotations.Beta; +import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; +import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; +import com.sap.cloud.sdk.cloudplatform.util.StringUtils; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +/** + * Factory class providing pre-built {@link ConnectionPoolManagerProvider} implementations with various caching + * strategies. + *

+ * Connection pool managers can consume significant memory (~100KB each). By caching and reusing connection managers + * based on appropriate keys, applications can reduce memory consumption while maintaining proper isolation where + * needed. + *

+ * + *

Usage Examples

+ * + * 
+ * {@code
+ * // Cache connection managers by tenant
+ * ApacheHttpClient5Factory factory =
+ *     new ApacheHttpClient5FactoryBuilder()
+ *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.byTenant())
+ *         .build();
+ *
+ * // Use a single global connection manager
+ * ApacheHttpClient5Factory factory =
+ *     new ApacheHttpClient5FactoryBuilder()
+ *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.global())
+ *         .build();
+ *
+ * // Custom caching strategy
+ * ApacheHttpClient5Factory factory =
+ *     new ApacheHttpClient5FactoryBuilder()
+ *         .connectionPoolManagerProvider(
+ *             ConnectionPoolManagerProviders
+ *                 .withCacheKey(dest -> dest.get(DestinationProperty.NAME).getOrElse("default")))
+ *         .build();
+ * }
+ *
+ * + * @see ConnectionPoolManagerProvider + * @see ConnectionPoolSettings + * @see ApacheHttpClient5FactoryBuilder#connectionPoolManagerProvider(ConnectionPoolManagerProvider) + * @since 5.XX.0 + */ +@Beta +@Slf4j +@NoArgsConstructor( access = AccessLevel.PRIVATE ) +public final class ConnectionPoolManagerProviders +{ + // Constant keys for cache entries + private static final String GLOBAL_KEY = "__GLOBAL__"; + private static final String NULL_KEY = "__NULL__"; + + /** + * Creates a provider that does not cache connection managers. + *

+ * A new {@link HttpClientConnectionManager} is created for each call. This is the default behavior and provides + * maximum isolation but highest memory consumption. + *

+ * + * @return A provider that creates a new connection manager for each request. + */ + @Nonnull + public static ConnectionPoolManagerProvider noCache() + { + return ConnectionPoolManagerProviders::createConnectionManager; + } + + /** + * Creates a provider that caches connection managers by the current tenant. + *

+ * Connection managers are shared among all destinations accessed within the same tenant context. This is useful + * when tenant isolation is required but destination-level isolation is not necessary. + *

+ *

+ * If no tenant is available in the current context, a shared "no-tenant" connection manager is used. + *

+ * + * @return A provider that caches connection managers by tenant. + * @see TenantAccessor#tryGetCurrentTenant() + */ + @Nonnull + public static ConnectionPoolManagerProvider byTenant() + { + return withCacheKey(dest -> TenantAccessor.tryGetCurrentTenant().map(Tenant::getTenantId).getOrNull()); + } + + /** + * Creates a provider that caches connection managers by destination name. + *

+ * Connection managers are shared among all requests to destinations with the same name. This is useful when + * different destinations may have different TLS or proxy configurations. + *

+ *

+ * If the destination has no name or is {@code null}, a shared "unnamed" connection manager is used. + *

+ * + * @return A provider that caches connection managers by destination name. + */ + @Nonnull + public static ConnectionPoolManagerProvider byDestinationName() + { + return withCacheKey(dest -> dest != null ? dest.get(DestinationProperty.NAME).getOrNull() : null); + } + + /** + * Creates a provider that uses a single global connection manager for all destinations. + *

+ * This provides the lowest memory consumption but no isolation between tenants or destinations. Use this only when + * all destinations have compatible TLS configurations and isolation is not required. + *

+ *

+ * Warning: This strategy does not support destination-specific TLS configurations. All + * destinations will use the default TLS settings. + *

+ * + * @return A provider that uses a single global connection manager. + */ + @Nonnull + public static ConnectionPoolManagerProvider global() + { + return withCacheKey(dest -> GLOBAL_KEY); + } + + /** + * Creates a provider that caches connection managers using a custom cache key extractor. + *

+ * The cache key extractor function is called for each request to determine which cached connection manager to use. + * Requests that produce equal cache keys (via {@link Object#equals(Object)}) will share the same connection + * manager. + *

+ *

+ * Note: The cache key extractor should return consistent keys for destinations that can safely + * share a connection manager. Consider TLS configuration, proxy settings, and isolation requirements when designing + * the key extraction logic. + *

+ * + * @param cacheKeyExtractor + * A function that extracts a cache key from the destination. The function receives {@code null} when + * creating a generic (non-destination-specific) connection manager. The returned key may be {@code null} + * to indicate a shared "null-key" bucket. + * @return A provider that caches connection managers using the custom key extractor. + */ + @Nonnull + public static ConnectionPoolManagerProvider withCacheKey( + @Nonnull final Function cacheKeyExtractor ) + { + Objects.requireNonNull(cacheKeyExtractor, "Cache key extractor must not be null"); + return new CachingProvider(cacheKeyExtractor); + } + + /** + * Provider that caches connection managers using a custom key extractor. + */ + private static final class CachingProvider implements ConnectionPoolManagerProvider + { + private final Function cacheKeyExtractor; + private final ConcurrentMap cache = new ConcurrentHashMap<>(); + + CachingProvider( final Function cacheKeyExtractor ) + { + this.cacheKeyExtractor = cacheKeyExtractor; + } + + @Nonnull + @Override + public HttpClientConnectionManager getConnectionManager( + @Nonnull final ConnectionPoolSettings settings, + @Nullable final HttpDestinationProperties destination ) + throws HttpClientInstantiationException + { + final Object rawKey = cacheKeyExtractor.apply(destination); + final Object cacheKey = rawKey != null ? rawKey : NULL_KEY; + + return cache.computeIfAbsent(cacheKey, key -> { + log.debug("Creating new connection manager for cache key: {}", rawKey); + return createConnectionManager(settings, destination); + }); + } + } + + /** + * Creates a new connection manager with the given settings and destination-specific TLS configuration. + */ + @Nonnull + private static HttpClientConnectionManager createConnectionManager( + @Nonnull final ConnectionPoolSettings settings, + @Nullable final HttpDestinationProperties destination ) + throws HttpClientInstantiationException + { + try { + final Timeout timeoutSocket = Timeout.of(settings.getSocketTimeout()); + final Timeout timeoutConnect = Timeout.of(settings.getConnectTimeout()); + + return PoolingHttpClientConnectionManagerBuilder + .create() + .setTlsSocketStrategy(getTlsSocketStrategy(destination)) + .setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeoutSocket).build()) + .setDefaultConnectionConfig( + ConnectionConfig.custom().setConnectTimeout(timeoutConnect).setSocketTimeout(timeoutSocket).build()) + .setMaxConnTotal(settings.getMaxConnectionsTotal()) + .setMaxConnPerRoute(settings.getMaxConnectionsPerRoute()) + .build(); + } + catch( final GeneralSecurityException | IOException e ) { + throw new HttpClientInstantiationException("Failed to create HTTP client connection manager.", e); + } + } + + @Nullable + private static TlsSocketStrategy getTlsSocketStrategy( @Nullable final HttpDestinationProperties destination ) + throws GeneralSecurityException, + IOException + { + if( !supportsTls(destination) ) { + return null; + } + + log.debug("The destination uses HTTPS for target \"{}\".", destination.getUri()); + final SSLContext sslContext = new SSLContextFactory().createSSLContext(destination); + final HostnameVerifier hostnameVerifier = getHostnameVerifier(destination); + + return new DefaultClientTlsStrategy(sslContext, hostnameVerifier); + } + + private static HostnameVerifier getHostnameVerifier( @Nonnull final HttpDestinationProperties destination ) + { + return destination.isTrustingAllCertificates() ? new NoopHostnameVerifier() : new DefaultHostnameVerifier(); + } + + private static boolean supportsTls( @Nullable final HttpDestinationProperties destination ) + { + if( destination == null ) { + return false; + } + final String scheme = destination.getUri().getScheme(); + return "https".equalsIgnoreCase(scheme) || StringUtils.isEmpty(scheme); + } +} diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java new file mode 100644 index 000000000..57031eda7 --- /dev/null +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java @@ -0,0 +1,79 @@ +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import java.time.Duration; + +import javax.annotation.Nonnull; + +import com.google.common.annotations.Beta; + +/** + * Configuration settings for HTTP connection pool managers and HTTP client request configuration. + *

+ * These settings control the behavior of {@link org.apache.hc.client5.http.io.HttpClientConnectionManager} instances + * created by {@link ConnectionPoolManagerProvider} implementations, as well as the request-level timeout configuration. + *

+ *

+ * Use {@link DefaultConnectionPoolSettings#ofDefaults()} or {@link DefaultConnectionPoolSettings#builder()} to create + * instances. Users can also implement this interface to provide custom settings implementations. + *

+ * + * @see ConnectionPoolManagerProviders + * @see DefaultConnectionPoolSettings + * @since 5.XX.0 + */ +@Beta +public interface ConnectionPoolSettings +{ + /** + * Default timeout of 2 minutes (applies to connect, socket, and connection request timeouts). + */ + Duration DEFAULT_TIMEOUT = Duration.ofMinutes(2L); + + /** + * Default maximum total connections of 200. + */ + int DEFAULT_MAX_CONNECTIONS_TOTAL = 200; + + /** + * Default maximum connections per route of 100. + */ + int DEFAULT_MAX_CONNECTIONS_PER_ROUTE = 100; + + /** + * Returns the timeout until a new connection is fully established. + * + * @return The connect timeout. + */ + @Nonnull + Duration getConnectTimeout(); + + /** + * Returns the default socket timeout value for I/O operations on connections. + * + * @return The socket timeout. + */ + @Nonnull + Duration getSocketTimeout(); + + /** + * Returns the timeout when requesting a connection lease from the connection pool. + * + * @return The connection request timeout. + */ + @Nonnull + Duration getConnectionRequestTimeout(); + + /** + * Returns the maximum number of total connections in the pool. + * + * @return The maximum total connections. + */ + int getMaxConnectionsTotal(); + + /** + * Returns the maximum number of connections per route (e.g., per remote host). + * + * @return The maximum connections per route. + */ + int getMaxConnectionsPerRoute(); +} \ No newline at end of file diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java index f7660519b..bd7d56789 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java @@ -1,51 +1,37 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; -import java.io.IOException; import java.net.URI; -import java.security.GeneralSecurityException; -import java.time.Duration; import java.util.Objects; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; import org.apache.hc.client5.http.classic.HttpClient; -import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.impl.classic.HttpClients; -import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.HttpClientConnectionManager; -import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; -import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier; -import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; -import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequestInterceptor; -import org.apache.hc.core5.http.io.SocketConfig; import org.apache.hc.core5.util.Timeout; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; -import com.sap.cloud.sdk.cloudplatform.util.StringUtils; import io.vavr.control.Option; +import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @Slf4j +@RequiredArgsConstructor class DefaultApacheHttpClient5Factory implements ApacheHttpClient5Factory { - static final Duration DEFAULT_TIMEOUT = Duration.ofMinutes(2L); - static final int DEFAULT_MAX_CONNECTIONS_TOTAL = 200; - static final int DEFAULT_MAX_CONNECTIONS_PER_ROUTE = 100; + @Nonnull + private final ConnectionPoolSettings settings; @Nonnull - private final Timeout timeout; - private final int maxConnectionsTotal; - private final int maxConnectionsPerRoute; + private final ConnectionPoolManagerProvider connectionPoolManagerProvider; @Nullable private final HttpRequestInterceptor requestInterceptor; @@ -53,20 +39,6 @@ class DefaultApacheHttpClient5Factory implements ApacheHttpClient5Factory @Nonnull private final ApacheHttpClient5FactoryBuilder.TlsUpgrade tlsUpgrade; - DefaultApacheHttpClient5Factory( - @Nonnull final Duration timeout, - final int maxConnectionsTotal, - final int maxConnectionsPerRoute, - @Nullable final HttpRequestInterceptor requestInterceptor, - @Nonnull final ApacheHttpClient5FactoryBuilder.TlsUpgrade tlsUpgrade ) - { - this.timeout = toTimeout(timeout); - this.maxConnectionsTotal = maxConnectionsTotal; - this.maxConnectionsPerRoute = maxConnectionsPerRoute; - this.requestInterceptor = requestInterceptor; - this.tlsUpgrade = tlsUpgrade; - } - @Nonnull @Override public HttpClient createHttpClient( @Nullable final HttpDestinationProperties destination ) @@ -87,10 +59,13 @@ private CloseableHttpClient buildHttpClient( @Nullable final HttpDestinationProperties destination, @Nonnull final RequestConfig requestConfig ) { + final HttpClientConnectionManager connManager = + connectionPoolManagerProvider.getConnectionManager(settings, destination); + final HttpClientBuilder builder = HttpClients .custom() - .setConnectionManager(getConnectionManager(destination)) + .setConnectionManager(connManager) .setDefaultRequestConfig(requestConfig) .setProxy(getProxy(destination)); @@ -101,69 +76,13 @@ private CloseableHttpClient buildHttpClient( return builder.build(); } - @Nonnull - private HttpClientConnectionManager getConnectionManager( @Nullable final HttpDestinationProperties destination ) - { - try { - return PoolingHttpClientConnectionManagerBuilder - .create() - .setTlsSocketStrategy(getTlsSocketStrategy(destination)) - .setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout).build()) - .setDefaultConnectionConfig( - ConnectionConfig.custom().setConnectTimeout(timeout).setSocketTimeout(timeout).build()) - .setMaxConnTotal(maxConnectionsTotal) - .setMaxConnPerRoute(maxConnectionsPerRoute) - .build(); - } - catch( final GeneralSecurityException | IOException e ) { - throw new HttpClientInstantiationException("Failed to create HTTP client connection manager.", e); - } - } - - @Nonnull - private static Timeout toTimeout( @Nonnull final Duration duration ) - { - return Timeout.ofMilliseconds(duration.toMillis()); - } - - @Nullable - private TlsSocketStrategy getTlsSocketStrategy( @Nullable final HttpDestinationProperties destination ) - throws GeneralSecurityException, - IOException - { - if( !supportsTls(destination) ) { - return null; - } - - log.debug("The destination uses HTTPS for target \"{}\".", destination.getUri()); - final SSLContext sslContext = new SSLContextFactory().createSSLContext(destination); - - final HostnameVerifier hostnameVerifier = getHostnameVerifier(destination); - - return new DefaultClientTlsStrategy(sslContext, hostnameVerifier); - } - - private boolean supportsTls( @Nullable final HttpDestinationProperties destination ) - { - if( destination == null ) { - return false; - } - final String scheme = destination.getUri().getScheme(); - return "https".equalsIgnoreCase(scheme) || StringUtils.isEmpty(scheme); - } - - private HostnameVerifier getHostnameVerifier( final HttpDestinationProperties destination ) - { - return destination.isTrustingAllCertificates() ? new NoopHostnameVerifier() : new DefaultHostnameVerifier(); - } - @Nonnull private RequestConfig getRequestConfig( @Nullable final HttpDestinationProperties destination ) { return RequestConfig .custom() .setProtocolUpgradeEnabled(isProtocolUpgradeEnabled(destination)) - .setConnectionRequestTimeout(timeout) + .setConnectionRequestTimeout(Timeout.ofMilliseconds(settings.getConnectionRequestTimeout().toMillis())) .build(); } diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java new file mode 100644 index 000000000..a4711cb50 --- /dev/null +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java @@ -0,0 +1,101 @@ +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import java.time.Duration; + +import javax.annotation.Nonnull; + +import com.google.common.annotations.Beta; + +import lombok.Builder; +import lombok.Value; +import lombok.With; + +/** + * Default implementation of {@link ConnectionPoolSettings} using Lombok's {@code @Value} for immutability. + *

+ * Use {@link #ofDefaults()} to create an instance with default values, or {@link #builder()} to create instances with + * custom values. + *

+ * + *

Example Usage

+ * + * 
+ * {@code
+ * // Using defaults
+ * ConnectionPoolSettings settings = DefaultConnectionPoolSettings.ofDefaults();
+ *
+ * // Using builder
+ * ConnectionPoolSettings settings = DefaultConnectionPoolSettings.builder()
+ *     .connectTimeout(Duration.ofSeconds(30))
+ *     .socketTimeout(Duration.ofMinutes(1))
+ *     .maxConnectionsTotal(500)
+ *     .build();
+ *
+ * // Using with() for copy-with-modification
+ * DefaultConnectionPoolSettings modified = settings.withMaxConnectionsTotal(1000);
+ * }
+ *
+ * + * @see ConnectionPoolSettings + * @since 5.XX.0 + */ +@Beta +@Value +@Builder +@With +public class DefaultConnectionPoolSettings implements ConnectionPoolSettings +{ + /** + * The timeout until a new connection is fully established. + */ + @Nonnull + @Builder.Default + Duration connectTimeout = ConnectionPoolSettings.DEFAULT_TIMEOUT; + + /** + * The default socket timeout value for I/O operations on connections. + */ + @Nonnull + @Builder.Default + Duration socketTimeout = ConnectionPoolSettings.DEFAULT_TIMEOUT; + + /** + * The timeout when requesting a connection lease from the connection pool. + */ + @Nonnull + @Builder.Default + Duration connectionRequestTimeout = ConnectionPoolSettings.DEFAULT_TIMEOUT; + + /** + * The maximum number of total connections in the pool. + */ + @Builder.Default + int maxConnectionsTotal = ConnectionPoolSettings.DEFAULT_MAX_CONNECTIONS_TOTAL; + + /** + * The maximum number of connections per route (e.g., per remote host). + */ + @Builder.Default + int maxConnectionsPerRoute = ConnectionPoolSettings.DEFAULT_MAX_CONNECTIONS_PER_ROUTE; + + /** + * Creates a new {@link DefaultConnectionPoolSettings} with default values. + *

+ * Default values: + *

    + *
  • Connect timeout: 2 minutes
    • + *
  • Socket timeout: 2 minutes
    • + *
  • Connection request timeout: 2 minutes
    • + *
  • Max connections total: 200
    • + *
  • Max connections per route: 100
    • + *
+ *

+ * + * @return A new instance with default settings. + */ + @Nonnull + public static DefaultConnectionPoolSettings ofDefaults() + { + return builder().build(); + } +} \ No newline at end of file diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java new file mode 100644 index 000000000..f2b1c2582 --- /dev/null +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java @@ -0,0 +1,183 @@ +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNullPointerException; + +import java.net.URI; + +import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.junit.jupiter.api.Test; + +import com.sap.cloud.sdk.cloudplatform.tenant.DefaultTenant; +import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; + +class ConnectionPoolManagerProvidersTest +{ + private static final ConnectionPoolSettings DEFAULT_SETTINGS = DefaultConnectionPoolSettings.ofDefaults(); + + @Test + void testNoCacheCreatesNewManagerEachTime() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.noCache(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + + assertThat(manager1).isNotNull(); + assertThat(manager2).isNotNull(); + assertThat(manager1).isNotSameAs(manager2); + } + + @Test + void testGlobalReturnsSameManagerForAllCalls() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.global(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + + final HttpDestinationProperties destination = + DefaultHttpDestination.builder(URI.create("http://example.com")).build(); + final HttpClientConnectionManager manager3 = provider.getConnectionManager(DEFAULT_SETTINGS, destination); + + assertThat(manager1).isNotNull(); + assertThat(manager1).isSameAs(manager2); + assertThat(manager1).isSameAs(manager3); + } + + @Test + void testByDestinationNameCachesByName() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byDestinationName(); + + final HttpDestinationProperties dest1 = + DefaultHttpDestination.builder(URI.create("http://example1.com")).name("dest-a").build(); + final HttpDestinationProperties dest2 = + DefaultHttpDestination.builder(URI.create("http://example2.com")).name("dest-a").build(); + final HttpDestinationProperties dest3 = + DefaultHttpDestination.builder(URI.create("http://example3.com")).name("dest-b").build(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, dest1); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, dest2); + final HttpClientConnectionManager manager3 = provider.getConnectionManager(DEFAULT_SETTINGS, dest3); + + assertThat(manager1).isNotNull(); + assertThat(manager1).isSameAs(manager2); // Same name "dest-a" + assertThat(manager1).isNotSameAs(manager3); // Different name "dest-b" + } + + @Test + void testByDestinationNameHandlesNullDestination() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byDestinationName(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + + assertThat(manager1).isNotNull(); + assertThat(manager1).isSameAs(manager2); + } + + @Test + void testByDestinationNameHandlesUnnamedDestination() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byDestinationName(); + + final HttpDestinationProperties unnamedDest = + DefaultHttpDestination.builder(URI.create("http://example.com")).build(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, unnamedDest); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + + // Both should use the same "null key" bucket + assertThat(manager1).isNotNull(); + assertThat(manager1).isSameAs(manager2); + } + + @Test + void testByTenantCachesByTenant() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byTenant(); + + final HttpClientConnectionManager managerTenant1 = + TenantAccessor + .executeWithTenant( + new DefaultTenant("tenant-1"), + () -> provider.getConnectionManager(DEFAULT_SETTINGS, null)); + + final HttpClientConnectionManager managerTenant1Again = + TenantAccessor + .executeWithTenant( + new DefaultTenant("tenant-1"), + () -> provider.getConnectionManager(DEFAULT_SETTINGS, null)); + + final HttpClientConnectionManager managerTenant2 = + TenantAccessor + .executeWithTenant( + new DefaultTenant("tenant-2"), + () -> provider.getConnectionManager(DEFAULT_SETTINGS, null)); + + assertThat(managerTenant1).isNotNull(); + assertThat(managerTenant1).isSameAs(managerTenant1Again); // Same tenant + assertThat(managerTenant1).isNotSameAs(managerTenant2); // Different tenant + } + + @Test + void testByTenantHandlesNoTenant() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byTenant(); + + // Without tenant context + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + + assertThat(manager1).isNotNull(); + assertThat(manager1).isSameAs(manager2); + } + + @Test + void testWithCacheKeyCustomExtractor() + { + // Custom extractor that uses the URI host as cache key + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.withCacheKey(dest -> { + if( dest == null ) { + return "no-destination"; + } + return dest.getUri().getHost(); + }); + + final HttpDestinationProperties dest1 = + DefaultHttpDestination.builder(URI.create("http://host-a.com/path1")).build(); + final HttpDestinationProperties dest2 = + DefaultHttpDestination.builder(URI.create("http://host-a.com/path2")).build(); + final HttpDestinationProperties dest3 = + DefaultHttpDestination.builder(URI.create("http://host-b.com/path1")).build(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, dest1); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, dest2); + final HttpClientConnectionManager manager3 = provider.getConnectionManager(DEFAULT_SETTINGS, dest3); + + assertThat(manager1).isNotNull(); + assertThat(manager1).isSameAs(manager2); // Same host "host-a.com" + assertThat(manager1).isNotSameAs(manager3); // Different host "host-b.com" + } + + @Test + void testNullCacheKeyExtractorThrowsException() + { + assertThatNullPointerException() + .isThrownBy(() -> ConnectionPoolManagerProviders.withCacheKey(null)) + .withMessageContaining("Cache key extractor must not be null"); + } + + @Test + void testFunctionalInterfaceCanBeUsedWithLambda() + { + // Verify that ConnectionPoolManagerProvider can be used as a lambda + final ConnectionPoolManagerProvider lambdaProvider = + ( settings, dest ) -> ConnectionPoolManagerProviders.noCache().getConnectionManager(settings, dest); + + final HttpClientConnectionManager manager = lambdaProvider.getConnectionManager(DEFAULT_SETTINGS, null); + assertThat(manager).isNotNull(); + } +} diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5CacheTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5CacheTest.java index 7295ec9d9..b9e9e5303 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5CacheTest.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5CacheTest.java @@ -46,9 +46,8 @@ class DefaultApacheHttpClient5CacheTest private static final ApacheHttpClient5Factory FACTORY = new DefaultApacheHttpClient5Factory( - DefaultApacheHttpClient5Factory.DEFAULT_TIMEOUT, - DefaultApacheHttpClient5Factory.DEFAULT_MAX_CONNECTIONS_TOTAL, - DefaultApacheHttpClient5Factory.DEFAULT_MAX_CONNECTIONS_PER_ROUTE, + DefaultConnectionPoolSettings.ofDefaults(), + ConnectionPoolManagerProviders.noCache(), null, ApacheHttpClient5FactoryBuilder.TlsUpgrade.AUTOMATIC); private static final long NANOSECONDS_IN_MINUTE = 60_000_000_000L; diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java index 80a9e6e01..02e342f53 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5FactoryTest.java @@ -80,11 +80,11 @@ void setup() requestInterceptor = mock(HttpRequestInterceptor.class); doNothing().when(requestInterceptor).process(any(), any(), any()); + final ConnectionPoolSettings settings = DefaultConnectionPoolSettings.ofDefaults(); sut = new DefaultApacheHttpClient5Factory( - CLIENT_TIMEOUT, - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, + settings, + ConnectionPoolManagerProviders.noCache(), requestInterceptor, AUTOMATIC); } @@ -95,19 +95,31 @@ void testHttpClientUsesTimeout() { WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/timeout")).willReturn(ok().withFixedDelay(5_000))); + final Duration tooLittleTimeout = Duration.ofSeconds(3L); + final ConnectionPoolSettings settingsTooLittle = + DefaultConnectionPoolSettings + .ofDefaults() + .withConnectTimeout(tooLittleTimeout) + .withSocketTimeout(tooLittleTimeout) + .withConnectionRequestTimeout(tooLittleTimeout); final ApacheHttpClient5Factory factoryWithTooLittleTimeout = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, + settingsTooLittle, + ConnectionPoolManagerProviders.noCache(), requestInterceptor, AUTOMATIC); + final Duration enoughTimeout = Duration.ofSeconds(7L); + final ConnectionPoolSettings settingsEnough = + DefaultConnectionPoolSettings + .ofDefaults() + .withConnectTimeout(enoughTimeout) + .withSocketTimeout(enoughTimeout) + .withConnectionRequestTimeout(enoughTimeout); final ApacheHttpClient5Factory factoryWithEnoughTimeout = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(7L), - MAX_CONNECTIONS, - MAX_CONNECTIONS_PER_ROUTE, + settingsEnough, + ConnectionPoolManagerProviders.noCache(), requestInterceptor, AUTOMATIC); @@ -132,11 +144,19 @@ void testHttpClientUsesMaxConnections() WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-1")).willReturn(ok())); WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-2")).willReturn(ok())); + final Duration timeout = Duration.ofSeconds(3L); // this timeout is also used for the connection lease + final ConnectionPoolSettings settings = + DefaultConnectionPoolSettings + .builder() + .connectTimeout(timeout) + .socketTimeout(timeout) + .connectionRequestTimeout(timeout) + .maxConnectionsTotal(1) + .build(); final ApacheHttpClient5Factory sut = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), // this timeout is also used for the connection lease - 1, - MAX_CONNECTIONS_PER_ROUTE, + settings, + ConnectionPoolManagerProviders.noCache(), requestInterceptor, AUTOMATIC); @@ -155,11 +175,19 @@ void testHttpClientUsesMaxConnectionsPerRoute() WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); SECOND_WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/max-connections-per-route")).willReturn(ok())); + final Duration timeout = Duration.ofSeconds(3L); // this timeout is also used for the connection lease + final ConnectionPoolSettings settings = + DefaultConnectionPoolSettings + .builder() + .connectTimeout(timeout) + .socketTimeout(timeout) + .connectionRequestTimeout(timeout) + .maxConnectionsPerRoute(1) + .build(); final ApacheHttpClient5Factory sut = new DefaultApacheHttpClient5Factory( - Duration.ofSeconds(3L), // this timeout is also used for the connection lease - MAX_CONNECTIONS, - 1, + settings, + ConnectionPoolManagerProviders.noCache(), requestInterceptor, AUTOMATIC); From 4e7f1ed1dcfa72c8daa598c1d2c96fb6fa27f28c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 18:04:11 +0100 Subject: [PATCH 2/8] Improve APi --- .../ConnectionPoolManagerProviders.java | 259 +++++++++++------- .../ConnectionPoolManagerProvidersTest.java | 115 ++++++-- 2 files changed, 256 insertions(+), 118 deletions(-) diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java index c7c940acc..95f3358d8 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java @@ -2,9 +2,10 @@ import java.io.IOException; import java.security.GeneralSecurityException; +import java.time.Duration; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.function.BiFunction; import java.util.function.Function; import javax.annotation.Nonnull; @@ -12,7 +13,8 @@ import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; -import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.HttpClientConnectionManager; @@ -25,11 +27,13 @@ import com.google.common.annotations.Beta; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; +import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; import com.sap.cloud.sdk.cloudplatform.util.StringUtils; import lombok.AccessLevel; import lombok.NoArgsConstructor; +import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; /** @@ -45,24 +49,33 @@ * * 
  * {@code
- * // Cache connection managers by tenant
+ * // No caching (default behavior)
+ * ApacheHttpClient5Factory factory =
+ *     new ApacheHttpClient5FactoryBuilder()
+ *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.noCache())
+ *         .build();
+ *
+ * // Cache connection managers by tenant using default ConcurrentHashMap
  * ApacheHttpClient5Factory factory =
  *     new ApacheHttpClient5FactoryBuilder()
- *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.byTenant())
+ *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.cached().byTenant())
  *         .build();
  *
- * // Use a single global connection manager
+ * // Cache with custom ConcurrentMap
+ * ConcurrentMap myCache = new ConcurrentHashMap<>();
  * ApacheHttpClient5Factory factory =
  *     new ApacheHttpClient5FactoryBuilder()
- *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.global())
+ *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.cached(myCache::computeIfAbsent).byTenant())
  *         .build();
  *
- * // Custom caching strategy
+ * // Cache with Caffeine cache (supports expiration, size limits, etc.)
+ * Cache caffeineCache = Caffeine.newBuilder()
+ *     .expireAfterAccess(Duration.ofMinutes(30))
+ *     .maximumSize(100)
+ *     .build();
  * ApacheHttpClient5Factory factory =
  *     new ApacheHttpClient5FactoryBuilder()
- *         .connectionPoolManagerProvider(
- *             ConnectionPoolManagerProviders
- *                 .withCacheKey(dest -> dest.get(DestinationProperty.NAME).getOrElse("default")))
+ *         .connectionPoolManagerProvider(ConnectionPoolManagerProviders.cached(caffeineCache::get).byDestinationName())
  *         .build();
  * }
  *
@@ -77,11 +90,9 @@ @NoArgsConstructor( access = AccessLevel.PRIVATE ) public final class ConnectionPoolManagerProviders { - // Constant keys for cache entries - private static final String GLOBAL_KEY = "__GLOBAL__"; - private static final String NULL_KEY = "__NULL__"; + private static final Duration DEFAULT_CACHE_DURATION = DefaultApacheHttpClient5Cache.DEFAULT_DURATION; - /** + /** * Creates a provider that does not cache connection managers. *

* A new {@link HttpClientConnectionManager} is created for each call. This is the default behavior and provides @@ -97,115 +108,175 @@ public static ConnectionPoolManagerProvider noCache() } /** - * Creates a provider that caches connection managers by the current tenant. - *

- * Connection managers are shared among all destinations accessed within the same tenant context. This is useful - * when tenant isolation is required but destination-level isolation is not necessary. - *

+ * Creates a builder for cached connection pool manager providers using a default {@link ConcurrentHashMap} as the + * cache. *

- * If no tenant is available in the current context, a shared "no-tenant" connection manager is used. + * The returned builder allows selecting a caching strategy (by tenant, by destination name, global, or custom). *

* - * @return A provider that caches connection managers by tenant. - * @see TenantAccessor#tryGetCurrentTenant() - */ - @Nonnull - public static ConnectionPoolManagerProvider byTenant() - { - return withCacheKey(dest -> TenantAccessor.tryGetCurrentTenant().map(Tenant::getTenantId).getOrNull()); - } - - /** - * Creates a provider that caches connection managers by destination name. - *

- * Connection managers are shared among all requests to destinations with the same name. This is useful when - * different destinations may have different TLS or proxy configurations. - *

- *

- * If the destination has no name or is {@code null}, a shared "unnamed" connection manager is used. - *

+ *

Example Usage

+ * + * 
+     * {@code
+     * // Cache by tenant
+     * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byTenant();
+     *
+     * // Cache by destination name
+     * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byDestinationName();
      *
-     * @return A provider that caches connection managers by destination name.
+     * // Single global cache
+     * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().global();
+     *
+     * // Custom cache key
+     * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached()
+     *     .withCacheKey(dest -> dest.getUri().getHost());
+     * }
+     *
+ * + * @return A builder for configuring the caching strategy. */ @Nonnull - public static ConnectionPoolManagerProvider byDestinationName() + public static CachedProviderBuilder cached() { - return withCacheKey(dest -> dest != null ? dest.get(DestinationProperty.NAME).getOrNull() : null); + final Cache cache = Caffeine.newBuilder().expireAfterAccess(DEFAULT_CACHE_DURATION).build(); + return new CachedProviderBuilder(cache::get); } /** - * Creates a provider that uses a single global connection manager for all destinations. + * Creates a builder for cached connection pool manager providers using a custom cache function. *

- * This provides the lowest memory consumption but no isolation between tenants or destinations. Use this only when - * all destinations have compatible TLS configurations and isolation is not required. - *

- *

- * Warning: This strategy does not support destination-specific TLS configurations. All - * destinations will use the default TLS settings. + * The cache function must have the signature {@code (key, loader) -> value}, which is compatible with: *

+ *
    + *
  • {@link java.util.concurrent.ConcurrentMap#computeIfAbsent(Object, Function)} - e.g., + * {@code myMap::computeIfAbsent}
    • + *
  • {@code com.github.benmanes.caffeine.cache.Cache#get(Object, Function)} - e.g., + * {@code caffeineCache::get}
    • + *
+ * + *

Example Usage

* - * @return A provider that uses a single global connection manager. + * 
+     * {@code
+     * // Using a custom ConcurrentMap
+     * ConcurrentMap myCache = new ConcurrentHashMap<>();
+     * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders
+     *     .cached(myCache::computeIfAbsent)
+     *     .byTenant();
+     *
+     * // Using Caffeine cache with expiration
+     * Cache caffeineCache = Caffeine.newBuilder()
+     *     .expireAfterAccess(Duration.ofMinutes(30))
+     *     .maximumSize(100)
+     *     .build();
+     * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders
+     *     .cached(caffeineCache::get)
+     *     .byDestinationName();
+     * }
+     *
+ * + * @param cacheFunction + * A function that takes a cache key and a loader function, and returns the cached or newly computed + * value. The signature matches {@code ConcurrentMap::computeIfAbsent} and {@code Cache::get}. + * @return A builder for configuring the caching strategy. */ @Nonnull - public static ConnectionPoolManagerProvider global() + public static CachedProviderBuilder cached( + @Nonnull final BiFunction, HttpClientConnectionManager> cacheFunction ) { - return withCacheKey(dest -> GLOBAL_KEY); + Objects.requireNonNull(cacheFunction, "Cache function must not be null"); + return new CachedProviderBuilder(cacheFunction); } /** - * Creates a provider that caches connection managers using a custom cache key extractor. + * Builder class for creating cached {@link ConnectionPoolManagerProvider} instances with various caching + * strategies. *

- * The cache key extractor function is called for each request to determine which cached connection manager to use. - * Requests that produce equal cache keys (via {@link Object#equals(Object)}) will share the same connection - * manager. - *

- *

- * Note: The cache key extractor should return consistent keys for destinations that can safely - * share a connection manager. Consider TLS configuration, proxy settings, and isolation requirements when designing - * the key extraction logic. + * Use {@link ConnectionPoolManagerProviders#cached()} or + * {@link ConnectionPoolManagerProviders#cached(BiFunction)} to obtain an instance of this builder. *

* - * @param cacheKeyExtractor - * A function that extracts a cache key from the destination. The function receives {@code null} when - * creating a generic (non-destination-specific) connection manager. The returned key may be {@code null} - * to indicate a shared "null-key" bucket. - * @return A provider that caches connection managers using the custom key extractor. - */ - @Nonnull - public static ConnectionPoolManagerProvider withCacheKey( - @Nonnull final Function cacheKeyExtractor ) - { - Objects.requireNonNull(cacheKeyExtractor, "Cache key extractor must not be null"); - return new CachingProvider(cacheKeyExtractor); - } - - /** - * Provider that caches connection managers using a custom key extractor. + * @since 5.XX.0 */ - private static final class CachingProvider implements ConnectionPoolManagerProvider + @Beta + @RequiredArgsConstructor( access = AccessLevel.PRIVATE ) + public static final class CachedProviderBuilder { - private final Function cacheKeyExtractor; - private final ConcurrentMap cache = new ConcurrentHashMap<>(); + @Nonnull + private final BiFunction, HttpClientConnectionManager> cacheFunction; - CachingProvider( final Function cacheKeyExtractor ) + /** + * Creates a provider that caches connection managers by the current tenant. + *

+ * Connection managers are shared among all destinations accessed within the same tenant context. This is useful + * when tenant isolation is required but destination-level isolation is not necessary. + *

+ *

+ * If no tenant is available in the current context, a new connection manager is used. + *

+ * + * @return A provider that caches connection managers by tenant. + * @see TenantAccessor#tryGetCurrentTenant() + */ + @Nonnull + public ConnectionPoolManagerProvider byCurrentTenant() { - this.cacheKeyExtractor = cacheKeyExtractor; + return by(dest -> TenantAccessor.tryGetCurrentTenant().map(Tenant::getTenantId).getOrNull()); } + /** + * Creates a provider that caches connection managers by destination name. + *

+ * Connection managers are shared among all requests to destinations with the same name. This is useful when + * different destinations may have different TLS or proxy configurations. + *

+ *

+ * If the destination has no name or is {@code null}, a new connection manager is used. + *

+ * + * @return A provider that caches connection managers by destination name. + */ @Nonnull - @Override - public HttpClientConnectionManager getConnectionManager( - @Nonnull final ConnectionPoolSettings settings, - @Nullable final HttpDestinationProperties destination ) - throws HttpClientInstantiationException + public ConnectionPoolManagerProvider byDestinationName() { - final Object rawKey = cacheKeyExtractor.apply(destination); - final Object cacheKey = rawKey != null ? rawKey : NULL_KEY; + return by(dest -> dest != null ? dest.get(DestinationProperty.NAME).getOrNull() : null); + } - return cache.computeIfAbsent(cacheKey, key -> { - log.debug("Creating new connection manager for cache key: {}", rawKey); - return createConnectionManager(settings, destination); - }); + /** + * Creates a provider that caches connection managers using a custom cache key extractor. + *

+ * The cache key extractor function is called for each request to determine which cached connection manager to + * use. Requests that produce equal cache keys (via {@link Object#equals(Object)}) will share the same + * connection manager. + *

+ *

+ * Note: The cache key extractor should return consistent keys for destinations that can safely + * share a connection manager. Consider TLS configuration, proxy settings, and isolation requirements when + * designing the key extraction logic. + *

+ * + * @param cacheKeyExtractor + * A function that extracts a cache key from the destination. The function receives {@code null} when + * creating a generic (non-destination-specific) connection manager. The returned key may be + * {@code null} to indicate a new uncached entry. + * @return A provider that caches connection managers using the custom key extractor. + */ + @Nonnull + public ConnectionPoolManagerProvider by( + @Nonnull final Function cacheKeyExtractor ) + { + Objects.requireNonNull(cacheKeyExtractor, "Cache key extractor must not be null"); + return ( settings, destination ) -> { + final Object rawKey = cacheKeyExtractor.apply(destination); + if(rawKey==null) { + log.debug("Creating new connection manager due to missing cache key for destination: {}", destination); + return createConnectionManager(settings, destination); + } + return cacheFunction.apply(rawKey, key -> { + log.debug("Creating new connection manager for cache key: {}", rawKey); + return createConnectionManager(settings, destination); + }); + }; } } @@ -213,7 +284,7 @@ public HttpClientConnectionManager getConnectionManager( * Creates a new connection manager with the given settings and destination-specific TLS configuration. */ @Nonnull - private static HttpClientConnectionManager createConnectionManager( + static HttpClientConnectionManager createConnectionManager( @Nonnull final ConnectionPoolSettings settings, @Nullable final HttpDestinationProperties destination ) throws HttpClientInstantiationException @@ -266,4 +337,4 @@ private static boolean supportsTls( @Nullable final HttpDestinationProperties de final String scheme = destination.getUri().getScheme(); return "https".equalsIgnoreCase(scheme) || StringUtils.isEmpty(scheme); } -} +} \ No newline at end of file diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java index f2b1c2582..56c2db8ce 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java @@ -4,6 +4,11 @@ import static org.assertj.core.api.Assertions.assertThatNullPointerException; import java.net.URI; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BiFunction; +import java.util.function.Function; import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.junit.jupiter.api.Test; @@ -29,9 +34,9 @@ void testNoCacheCreatesNewManagerEachTime() } @Test - void testGlobalReturnsSameManagerForAllCalls() + void testCachedGlobalReturnsSameManagerForAllCalls() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.global(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().by(destination -> true); final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); @@ -46,9 +51,9 @@ void testGlobalReturnsSameManagerForAllCalls() } @Test - void testByDestinationNameCachesByName() + void testCachedByDestinationNameCachesByName() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byDestinationName(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byDestinationName(); final HttpDestinationProperties dest1 = DefaultHttpDestination.builder(URI.create("http://example1.com")).name("dest-a").build(); @@ -67,21 +72,21 @@ void testByDestinationNameCachesByName() } @Test - void testByDestinationNameHandlesNullDestination() + void testCachedByDestinationNameHandlesNullDestination() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byDestinationName(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byDestinationName(); final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); assertThat(manager1).isNotNull(); - assertThat(manager1).isSameAs(manager2); + assertThat(manager1).isNotSameAs(manager2); } @Test - void testByDestinationNameHandlesUnnamedDestination() + void testCachedByDestinationNameHandlesUnnamedDestination() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byDestinationName(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byDestinationName(); final HttpDestinationProperties unnamedDest = DefaultHttpDestination.builder(URI.create("http://example.com")).build(); @@ -91,13 +96,13 @@ void testByDestinationNameHandlesUnnamedDestination() // Both should use the same "null key" bucket assertThat(manager1).isNotNull(); - assertThat(manager1).isSameAs(manager2); + assertThat(manager1).isNotSameAs(manager2); } @Test - void testByTenantCachesByTenant() + void testCachedByTenantCachesByTenant() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byTenant(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byCurrentTenant(); final HttpClientConnectionManager managerTenant1 = TenantAccessor @@ -123,28 +128,29 @@ void testByTenantCachesByTenant() } @Test - void testByTenantHandlesNoTenant() + void testCachedByTenantHandlesNoTenant() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.byTenant(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byCurrentTenant(); // Without tenant context final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); assertThat(manager1).isNotNull(); - assertThat(manager1).isSameAs(manager2); + assertThat(manager1).isNotSameAs(manager2); } @Test - void testWithCacheKeyCustomExtractor() + void testCachedWithCacheKeyCustomExtractor() { // Custom extractor that uses the URI host as cache key - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.withCacheKey(dest -> { - if( dest == null ) { - return "no-destination"; - } - return dest.getUri().getHost(); - }); + final ConnectionPoolManagerProvider provider = + ConnectionPoolManagerProviders.cached().by(dest -> { + if( dest == null ) { + return "no-destination"; + } + return dest.getUri().getHost(); + }); final HttpDestinationProperties dest1 = DefaultHttpDestination.builder(URI.create("http://host-a.com/path1")).build(); @@ -162,14 +168,75 @@ void testWithCacheKeyCustomExtractor() assertThat(manager1).isNotSameAs(manager3); // Different host "host-b.com" } + @Test + void testCachedWithCustomConcurrentMap() + { + // Use a custom ConcurrentMap + final ConcurrentMap customCache = new ConcurrentHashMap<>(); + + final ConnectionPoolManagerProvider provider = + ConnectionPoolManagerProviders.cached(customCache::computeIfAbsent).byDestinationName(); + + final HttpDestinationProperties dest = + DefaultHttpDestination.builder(URI.create("http://example.com")).name("my-dest").build(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, dest); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, dest); + + assertThat(manager1).isNotNull(); + assertThat(manager1).isSameAs(manager2); + + // Verify the cache was used + assertThat(customCache).hasSize(1); + assertThat(customCache).containsKey("my-dest"); + assertThat(customCache.get("my-dest")).isSameAs(manager1); + } + + @Test + void testCachedWithCustomCacheFunction() + { + // Simulate a Caffeine-like cache with a custom BiFunction + final ConcurrentMap backingMap = new ConcurrentHashMap<>(); + final AtomicInteger loadCount = new AtomicInteger(0); + + final BiFunction, HttpClientConnectionManager> cacheFunction = + ( key, loader ) -> { + return backingMap.computeIfAbsent(key, k -> { + loadCount.incrementAndGet(); + return loader.apply(k); + }); + }; + + final ConnectionPoolManagerProvider provider = + ConnectionPoolManagerProviders.cached(cacheFunction).by(destination -> true); + + // First call should load + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + assertThat(loadCount.get()).isEqualTo(1); + + // Second call should use cache + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); + assertThat(loadCount.get()).isEqualTo(1); // Still 1, no new load + + assertThat(manager1).isSameAs(manager2); + } + @Test void testNullCacheKeyExtractorThrowsException() { assertThatNullPointerException() - .isThrownBy(() -> ConnectionPoolManagerProviders.withCacheKey(null)) + .isThrownBy(() -> ConnectionPoolManagerProviders.cached().by(null)) .withMessageContaining("Cache key extractor must not be null"); } + @Test + void testNullCacheFunctionThrowsException() + { + assertThatNullPointerException() + .isThrownBy(() -> ConnectionPoolManagerProviders.cached(null)) + .withMessageContaining("Cache function must not be null"); + } + @Test void testFunctionalInterfaceCanBeUsedWithLambda() { @@ -180,4 +247,4 @@ void testFunctionalInterfaceCanBeUsedWithLambda() final HttpClientConnectionManager manager = lambdaProvider.getConnectionManager(DEFAULT_SETTINGS, null); assertThat(manager).isNotNull(); } -} +} \ No newline at end of file From e5753555417b3c1b00a62ef73d9058eb782ac9da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 19:07:24 +0100 Subject: [PATCH 3/8] Finalize solution for behalf indicated caching --- .../connectivity/IsOnBehalfOf.java | 22 +++ .../connectivity/AbstractHttpClientCache.java | 7 +- .../ApacheHttpClient5FactoryBuilder.java | 4 +- .../ConnectionPoolManagerProvider.java | 2 +- .../ConnectionPoolManagerProviders.java | 80 +++++--- .../connectivity/ConnectionPoolSettings.java | 4 +- .../DefaultConnectionPoolSettings.java | 16 +- .../ConnectionPoolManagerProvidersTest.java | 186 +++++++++++++++--- .../connectivity/OAuth2HeaderProvider.java | 11 +- .../connectivity/OAuth2Service.java | 3 +- 10 files changed, 266 insertions(+), 69 deletions(-) create mode 100644 cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/IsOnBehalfOf.java diff --git a/cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/IsOnBehalfOf.java b/cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/IsOnBehalfOf.java new file mode 100644 index 000000000..0feff2310 --- /dev/null +++ b/cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/IsOnBehalfOf.java @@ -0,0 +1,22 @@ +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import javax.annotation.Nullable; + +/** + * Interface to be implemented by classes that can provide information about the behalf upon which an action is run. + * + * @since 4.27.0 + */ +interface IsOnBehalfOf +{ + /** + * Returns the behalf upon which an action is run. + * + * @return The behalf upon which an action is run, or {@code null} if no information about the behalf is available. + */ + @Nullable + default OnBehalfOf getOnBehalfOf() + { + return null; + } +} diff --git a/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java b/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java index 29c336cf0..ccecf7129 100644 --- a/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java +++ b/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java @@ -79,11 +79,8 @@ private Try tryGetOrCreateHttpClient( final Try maybeKey = destination != null ? getCacheKey(destination) : getCacheKey(); if( maybeKey.isFailure() ) { - return Try - .failure( - new HttpClientInstantiationException( - "Failed to create cache key for HttpClient", - maybeKey.getCause())); + final String msg = "Failed to create cache key for HttpClient"; + return Try.failure(new HttpClientInstantiationException(msg, maybeKey.getCause())); } final Cache cache = maybeCache.get(); diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java index 616c356f0..583dbcc17 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java @@ -44,7 +44,7 @@ public class ApacheHttpClient5FactoryBuilder * * @see DefaultConnectionPoolSettings#ofDefaults() * @see DefaultConnectionPoolSettings#builder() - * @since 5.XX.0 + * @since 5.27.0 */ @Setter( onMethod_ = @Beta ) @Nonnull @@ -82,7 +82,7 @@ public class ApacheHttpClient5FactoryBuilder * * @see ConnectionPoolManagerProvider * @see ConnectionPoolManagerProviders - * @since 5.XX.0 + * @since 5.27.0 */ @Setter( onMethod_ = @Beta ) @Nonnull diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java index 4f39189e5..451b9c1ee 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvider.java @@ -36,7 +36,7 @@ * * @see ConnectionPoolManagerProviders * @see ApacheHttpClient5FactoryBuilder#connectionPoolManagerProvider(ConnectionPoolManagerProvider) - * @since 5.XX.0 + * @since 5.27.0 */ @Beta @FunctionalInterface diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java index 95f3358d8..09a93bd92 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.security.GeneralSecurityException; import java.time.Duration; +import java.util.List; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiFunction; @@ -36,6 +37,9 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.NAMED_USER_CURRENT_TENANT; +import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT; + /** * Factory class providing pre-built {@link ConnectionPoolManagerProvider} implementations with various caching * strategies. @@ -69,10 +73,8 @@ * .build(); * * // Cache with Caffeine cache (supports expiration, size limits, etc.) - * Cache caffeineCache = Caffeine.newBuilder() - * .expireAfterAccess(Duration.ofMinutes(30)) - * .maximumSize(100) - * .build(); + * Cache caffeineCache = + * Caffeine.newBuilder().expireAfterAccess(Duration.ofMinutes(30)).maximumSize(100).build(); * ApacheHttpClient5Factory factory = * new ApacheHttpClient5FactoryBuilder() * .connectionPoolManagerProvider(ConnectionPoolManagerProviders.cached(caffeineCache::get).byDestinationName()) @@ -83,7 +85,7 @@ * @see ConnectionPoolManagerProvider * @see ConnectionPoolSettings * @see ApacheHttpClient5FactoryBuilder#connectionPoolManagerProvider(ConnectionPoolManagerProvider) - * @since 5.XX.0 + * @since 5.27.0 */ @Beta @Slf4j @@ -92,7 +94,7 @@ public final class ConnectionPoolManagerProviders { private static final Duration DEFAULT_CACHE_DURATION = DefaultApacheHttpClient5Cache.DEFAULT_DURATION; - /** + /** * Creates a provider that does not cache connection managers. *

* A new {@link HttpClientConnectionManager} is created for each call. This is the default behavior and provides @@ -128,8 +130,8 @@ public static ConnectionPoolManagerProvider noCache() * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().global(); * * // Custom cache key - * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached() - * .withCacheKey(dest -> dest.getUri().getHost()); + * ConnectionPoolManagerProvider provider = + * ConnectionPoolManagerProviders.cached().withCacheKey(dest -> dest.getUri().getHost()); * } * * @@ -138,8 +140,9 @@ public static ConnectionPoolManagerProvider noCache() @Nonnull public static CachedProviderBuilder cached() { - final Cache cache = Caffeine.newBuilder().expireAfterAccess(DEFAULT_CACHE_DURATION).build(); - return new CachedProviderBuilder(cache::get); + final Cache cache = + Caffeine.newBuilder().expireAfterAccess(DEFAULT_CACHE_DURATION).build(); + return new CachedProviderBuilder(cache::get); } /** @@ -160,18 +163,13 @@ public static CachedProviderBuilder cached() * {@code * // Using a custom ConcurrentMap * ConcurrentMap myCache = new ConcurrentHashMap<>(); - * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders - * .cached(myCache::computeIfAbsent) - * .byTenant(); + * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached(myCache::computeIfAbsent).byTenant(); * * // Using Caffeine cache with expiration - * Cache caffeineCache = Caffeine.newBuilder() - * .expireAfterAccess(Duration.ofMinutes(30)) - * .maximumSize(100) - * .build(); - * ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders - * .cached(caffeineCache::get) - * .byDestinationName(); + * Cache caffeineCache = + * Caffeine.newBuilder().expireAfterAccess(Duration.ofMinutes(30)).maximumSize(100).build(); + * ConnectionPoolManagerProvider provider = + * ConnectionPoolManagerProviders.cached(caffeineCache::get).byDestinationName(); * } * * @@ -192,11 +190,11 @@ public static CachedProviderBuilder cached( * Builder class for creating cached {@link ConnectionPoolManagerProvider} instances with various caching * strategies. *

- * Use {@link ConnectionPoolManagerProviders#cached()} or - * {@link ConnectionPoolManagerProviders#cached(BiFunction)} to obtain an instance of this builder. + * Use {@link ConnectionPoolManagerProviders#cached()} or {@link ConnectionPoolManagerProviders#cached(BiFunction)} + * to obtain an instance of this builder. *

* - * @since 5.XX.0 + * @since 5.27.0 */ @Beta @RequiredArgsConstructor( access = AccessLevel.PRIVATE ) @@ -242,6 +240,31 @@ public ConnectionPoolManagerProvider byDestinationName() return by(dest -> dest != null ? dest.get(DestinationProperty.NAME).getOrNull() : null); } + @Nonnull + public ConnectionPoolManagerProvider byIndicatedBehalfOf() + { + return by(destination -> { + if( !(destination instanceof final DefaultHttpDestination dest) ) { + return null; + } + final boolean indicatesCurrentTenant = + dest + .getCustomHeaderProviders() + .stream() + .filter(IsOnBehalfOf.class::isInstance) + .map(d -> ((IsOnBehalfOf) d).getOnBehalfOf()) + .anyMatch(b -> b == NAMED_USER_CURRENT_TENANT || b == TECHNICAL_USER_CURRENT_TENANT); + + // If the destination indicates that it is on behalf of the current tenant, include the tenant ID in the cache key to ensure proper isolation. + if( indicatesCurrentTenant ) { + return List.of(TenantAccessor.tryGetCurrentTenant().map(Tenant::getTenantId).getOrNull(), dest); + } + + // Otherwise, return a cache key that does not include tenant information, allowing sharing across tenants if other properties match. + return dest; + }); + } + /** * Creates a provider that caches connection managers using a custom cache key extractor. *

@@ -268,9 +291,12 @@ public ConnectionPoolManagerProvider by( Objects.requireNonNull(cacheKeyExtractor, "Cache key extractor must not be null"); return ( settings, destination ) -> { final Object rawKey = cacheKeyExtractor.apply(destination); - if(rawKey==null) { - log.debug("Creating new connection manager due to missing cache key for destination: {}", destination); - return createConnectionManager(settings, destination); + if( rawKey == null ) { + log + .debug( + "Creating new connection manager due to missing cache key for destination: {}", + destination); + return createConnectionManager(settings, destination); } return cacheFunction.apply(rawKey, key -> { log.debug("Creating new connection manager for cache key: {}", rawKey); @@ -337,4 +363,4 @@ private static boolean supportsTls( @Nullable final HttpDestinationProperties de final String scheme = destination.getUri().getScheme(); return "https".equalsIgnoreCase(scheme) || StringUtils.isEmpty(scheme); } -} \ No newline at end of file +} diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java index 57031eda7..473a5e66b 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolSettings.java @@ -19,7 +19,7 @@ * * @see ConnectionPoolManagerProviders * @see DefaultConnectionPoolSettings - * @since 5.XX.0 + * @since 5.27.0 */ @Beta public interface ConnectionPoolSettings @@ -76,4 +76,4 @@ public interface ConnectionPoolSettings * @return The maximum connections per route. */ int getMaxConnectionsPerRoute(); -} \ No newline at end of file +} diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java index a4711cb50..22879eca3 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java @@ -25,11 +25,13 @@ * ConnectionPoolSettings settings = DefaultConnectionPoolSettings.ofDefaults(); * * // Using builder - * ConnectionPoolSettings settings = DefaultConnectionPoolSettings.builder() - * .connectTimeout(Duration.ofSeconds(30)) - * .socketTimeout(Duration.ofMinutes(1)) - * .maxConnectionsTotal(500) - * .build(); + * ConnectionPoolSettings settings = + * DefaultConnectionPoolSettings + * .builder() + * .connectTimeout(Duration.ofSeconds(30)) + * .socketTimeout(Duration.ofMinutes(1)) + * .maxConnectionsTotal(500) + * .build(); * * // Using with() for copy-with-modification * DefaultConnectionPoolSettings modified = settings.withMaxConnectionsTotal(1000); @@ -37,7 +39,7 @@ * * * @see ConnectionPoolSettings - * @since 5.XX.0 + * @since 5.27.0 */ @Beta @Value @@ -98,4 +100,4 @@ public static DefaultConnectionPoolSettings ofDefaults() { return builder().build(); } -} \ No newline at end of file +} diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java index 56c2db8ce..71f07a91d 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java @@ -3,19 +3,24 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatNullPointerException; -import java.net.URI; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; import java.util.function.Function; +import lombok.Getter; +import lombok.RequiredArgsConstructor; import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.junit.jupiter.api.Test; import com.sap.cloud.sdk.cloudplatform.tenant.DefaultTenant; import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; +import javax.annotation.Nonnull; +import java.util.Collections; +import java.util.List; + class ConnectionPoolManagerProvidersTest { private static final ConnectionPoolSettings DEFAULT_SETTINGS = DefaultConnectionPoolSettings.ofDefaults(); @@ -41,8 +46,7 @@ void testCachedGlobalReturnsSameManagerForAllCalls() final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, null); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); - final HttpDestinationProperties destination = - DefaultHttpDestination.builder(URI.create("http://example.com")).build(); + final HttpDestinationProperties destination = DefaultHttpDestination.builder("http://example.com").build(); final HttpClientConnectionManager manager3 = provider.getConnectionManager(DEFAULT_SETTINGS, destination); assertThat(manager1).isNotNull(); @@ -56,11 +60,11 @@ void testCachedByDestinationNameCachesByName() final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byDestinationName(); final HttpDestinationProperties dest1 = - DefaultHttpDestination.builder(URI.create("http://example1.com")).name("dest-a").build(); + DefaultHttpDestination.builder("http://example1.com").name("dest-a").build(); final HttpDestinationProperties dest2 = - DefaultHttpDestination.builder(URI.create("http://example2.com")).name("dest-a").build(); + DefaultHttpDestination.builder("http://example2.com").name("dest-a").build(); final HttpDestinationProperties dest3 = - DefaultHttpDestination.builder(URI.create("http://example3.com")).name("dest-b").build(); + DefaultHttpDestination.builder("http://example3.com").name("dest-b").build(); final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, dest1); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, dest2); @@ -88,8 +92,7 @@ void testCachedByDestinationNameHandlesUnnamedDestination() { final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byDestinationName(); - final HttpDestinationProperties unnamedDest = - DefaultHttpDestination.builder(URI.create("http://example.com")).build(); + final HttpDestinationProperties unnamedDest = DefaultHttpDestination.builder("http://example.com").build(); final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, unnamedDest); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, null); @@ -144,20 +147,16 @@ void testCachedByTenantHandlesNoTenant() void testCachedWithCacheKeyCustomExtractor() { // Custom extractor that uses the URI host as cache key - final ConnectionPoolManagerProvider provider = - ConnectionPoolManagerProviders.cached().by(dest -> { - if( dest == null ) { - return "no-destination"; - } - return dest.getUri().getHost(); - }); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().by(dest -> { + if( dest == null ) { + return "no-destination"; + } + return dest.getUri().getHost(); + }); - final HttpDestinationProperties dest1 = - DefaultHttpDestination.builder(URI.create("http://host-a.com/path1")).build(); - final HttpDestinationProperties dest2 = - DefaultHttpDestination.builder(URI.create("http://host-a.com/path2")).build(); - final HttpDestinationProperties dest3 = - DefaultHttpDestination.builder(URI.create("http://host-b.com/path1")).build(); + final HttpDestinationProperties dest1 = DefaultHttpDestination.builder("http://host-a.com/path1").build(); + final HttpDestinationProperties dest2 = DefaultHttpDestination.builder("http://host-a.com/path2").build(); + final HttpDestinationProperties dest3 = DefaultHttpDestination.builder("http://host-b.com/path1").build(); final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, dest1); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, dest2); @@ -178,7 +177,7 @@ void testCachedWithCustomConcurrentMap() ConnectionPoolManagerProviders.cached(customCache::computeIfAbsent).byDestinationName(); final HttpDestinationProperties dest = - DefaultHttpDestination.builder(URI.create("http://example.com")).name("my-dest").build(); + DefaultHttpDestination.builder("http://example.com").name("my-dest").build(); final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, dest); final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, dest); @@ -247,4 +246,145 @@ void testFunctionalInterfaceCanBeUsedWithLambda() final HttpClientConnectionManager manager = lambdaProvider.getConnectionManager(DEFAULT_SETTINGS, null); assertThat(manager).isNotNull(); } -} \ No newline at end of file + + @Test + void testCachedByIndicatedBehalfOfWithCurrentTenantHeaderProvider() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + + // Create a header provider that indicates NAMED_USER_CURRENT_TENANT + final DestinationHeaderProvider namedUserProvider = + new TestHeaderProvider(OnBehalfOf.NAMED_USER_CURRENT_TENANT); + + // Create destinations with the header provider + final DefaultHttpDestination destTenant1 = + DefaultHttpDestination.builder("http://example.com").headerProviders(namedUserProvider).build(); + + final DefaultHttpDestination destTenant2 = + DefaultHttpDestination.builder("http://example.com").headerProviders(namedUserProvider).build(); + + // Same destination with same tenant should return same manager + final DefaultTenant tenant1 = new DefaultTenant("tenant-1"); + final HttpClientConnectionManager managerTenant1 = + TenantAccessor + .executeWithTenant(tenant1, () -> provider.getConnectionManager(DEFAULT_SETTINGS, destTenant1)); + + final HttpClientConnectionManager managerTenant1Again = + TenantAccessor + .executeWithTenant(tenant1, () -> provider.getConnectionManager(DEFAULT_SETTINGS, destTenant2)); + + // Different tenant should return different manager + final DefaultTenant tenant2 = new DefaultTenant("tenant-2"); + final HttpClientConnectionManager managerTenant2 = + TenantAccessor + .executeWithTenant(tenant2, () -> provider.getConnectionManager(DEFAULT_SETTINGS, destTenant1)); + + assertThat(managerTenant1).isNotNull(); + assertThat(managerTenant1).isSameAs(managerTenant1Again); // Same tenant, same destination + assertThat(managerTenant1).isNotSameAs(managerTenant2); // Different tenant + } + + @Test + void testCachedByIndicatedBehalfOfWithTechnicalUserCurrentTenant() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + + final DefaultHttpDestination dest = + DefaultHttpDestination + .builder("http://example.com") + .headerProviders(new TestHeaderProvider(OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT)) + .build(); + + // Different tenants should get different managers + final DefaultTenant tenant1 = new DefaultTenant("tenant-1"); + final HttpClientConnectionManager managerTenant1 = + TenantAccessor.executeWithTenant(tenant1, () -> provider.getConnectionManager(DEFAULT_SETTINGS, dest)); + + final DefaultTenant tenant2 = new DefaultTenant("tenant-2"); + final HttpClientConnectionManager managerTenant2 = + TenantAccessor.executeWithTenant(tenant2, () -> provider.getConnectionManager(DEFAULT_SETTINGS, dest)); + + assertThat(managerTenant1).isNotNull(); + assertThat(managerTenant2).isNotNull(); + assertThat(managerTenant1).isNotSameAs(managerTenant2); // Different tenant + } + + @Test + void testCachedByIndicatedBehalfOfWithProviderUserSharesAcrossTenants() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + + // Create a header provider that indicates TECHNICAL_USER_PROVIDER (not current tenant) + final DefaultHttpDestination dest = + DefaultHttpDestination + .builder("http://example.com") + .headerProviders(new TestHeaderProvider(OnBehalfOf.TECHNICAL_USER_PROVIDER)) + .build(); + + // Different tenants should share the same manager since it's not on behalf of current tenant + final DefaultTenant tenant1 = new DefaultTenant("tenant-1"); + final HttpClientConnectionManager managerTenant1 = + TenantAccessor.executeWithTenant(tenant1, () -> provider.getConnectionManager(DEFAULT_SETTINGS, dest)); + + final DefaultTenant tenant2 = new DefaultTenant("tenant-2"); + final HttpClientConnectionManager managerTenant2 = + TenantAccessor.executeWithTenant(tenant2, () -> provider.getConnectionManager(DEFAULT_SETTINGS, dest)); + + assertThat(managerTenant1).isNotNull(); + assertThat(managerTenant1).isSameAs(managerTenant2); // Same manager shared across tenants + } + + @Test + void testCachedByIndicatedBehalfOfWithNoHeaderProvider() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + + // Destination without any header provider + final DefaultHttpDestination dest = DefaultHttpDestination.builder("http://example.com").build(); + + // Different tenants should share the same manager since there's no on-behalf-of indication + final DefaultTenant tenant1 = new DefaultTenant("tenant-1"); + final HttpClientConnectionManager managerTenant1 = + TenantAccessor.executeWithTenant(tenant1, () -> provider.getConnectionManager(DEFAULT_SETTINGS, dest)); + + final DefaultTenant tenant2 = new DefaultTenant("tenant-2"); + final HttpClientConnectionManager managerTenant2 = + TenantAccessor.executeWithTenant(tenant2, () -> provider.getConnectionManager(DEFAULT_SETTINGS, dest)); + + assertThat(managerTenant1).isNotNull(); + assertThat(managerTenant1).isSameAs(managerTenant2); // Same manager shared across tenants + } + + @Test + void testCachedByIndicatedBehalfOfWithNonDefaultHttpDestination() + { + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + + // Non-DefaultHttpDestination should return null key and create new manager each time + final HttpDestinationProperties nonDefaultDest = DefaultHttpDestination.builder("http://example.com").build(); + + final HttpClientConnectionManager manager1 = provider.getConnectionManager(DEFAULT_SETTINGS, nonDefaultDest); + final HttpClientConnectionManager manager2 = provider.getConnectionManager(DEFAULT_SETTINGS, nonDefaultDest); + + assertThat(manager1).isNotNull(); + assertThat(manager2).isNotNull(); + assertThat(manager1).isSameAs(manager2); // New manager each time for non-DefaultHttpDestination + } + + /** + * Test implementation of DestinationHeaderProvider that also implements IsOnBehalfOf. + */ + @RequiredArgsConstructor + private static class TestHeaderProvider implements DestinationHeaderProvider, IsOnBehalfOf + { + @Getter + private final OnBehalfOf onBehalfOf; + + @Nonnull + @Override + public List

getHeaders( @Nonnull final DestinationRequestContext requestContext ) + { + return Collections.emptyList(); + } + } +} diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java index 3aa246367..9f973064e 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java @@ -11,15 +11,24 @@ import io.vavr.control.Option; import lombok.RequiredArgsConstructor; +import org.jetbrains.annotations.Nullable; @RequiredArgsConstructor -class OAuth2HeaderProvider implements DestinationHeaderProvider +class OAuth2HeaderProvider implements DestinationHeaderProvider, IsOnBehalfOf { @Nonnull private final OAuth2Service oauth2service; + @Nonnull private final String authHeaderName; + @Nullable + @Override + public OnBehalfOf getOnBehalfOf() + { + return oauth2service.getOnBehalfOf(); + } + @Nonnull @Override public List
getHeaders( @Nonnull final DestinationRequestContext requestContext ) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java index ea3b29d8a..00155666a 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java @@ -54,7 +54,7 @@ */ @RequiredArgsConstructor( access = AccessLevel.PACKAGE ) @Slf4j -class OAuth2Service +class OAuth2Service implements IsOnBehalfOf { /** * Cache to reuse OAuth2TokenService and with that reuse the underlying response cache. @@ -83,6 +83,7 @@ class OAuth2Service @Nonnull private final ClientIdentity identity; @Nonnull + @Getter private final OnBehalfOf onBehalfOf; @Nonnull private final TenantPropagationStrategy tenantPropagationStrategy; From 50ef2bfe13b385f3e7a056021bf08a20e1876ce3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 19:50:53 +0100 Subject: [PATCH 4/8] Add release note; Minor comment fix --- .../ConnectionPoolManagerProviders.java | 47 ++++++++++--------- release_notes.md | 10 ++++ 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java index 09a93bd92..4adaf1d2c 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java @@ -37,9 +37,6 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.NAMED_USER_CURRENT_TENANT; -import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT; - /** * Factory class providing pre-built {@link ConnectionPoolManagerProvider} implementations with various caching * strategies. @@ -101,7 +98,7 @@ public final class ConnectionPoolManagerProviders * maximum isolation but highest memory consumption. *

* - * @return A provider that creates a new connection manager for each request. + * @return A provider that creates a new connection manager for each http-client. */ @Nonnull public static ConnectionPoolManagerProvider noCache() @@ -225,8 +222,8 @@ public ConnectionPoolManagerProvider byCurrentTenant() /** * Creates a provider that caches connection managers by destination name. *

- * Connection managers are shared among all requests to destinations with the same name. This is useful when - * different destinations may have different TLS or proxy configurations. + * Connection managers are shared among all HttpClients and their requests to destinations with the same name. + * This is useful when different destinations may have different TLS or proxy configurations. *

*

* If the destination has no name or is {@code null}, a new connection manager is used. @@ -244,32 +241,41 @@ public ConnectionPoolManagerProvider byDestinationName() public ConnectionPoolManagerProvider byIndicatedBehalfOf() { return by(destination -> { + // Check if the destination has any OnBehalfOf indicators in its custom header providers if( !(destination instanceof final DefaultHttpDestination dest) ) { return null; } - final boolean indicatesCurrentTenant = - dest - .getCustomHeaderProviders() + final List headerProviders = dest.getCustomHeaderProviders(); + final List behalfOfIndicators = + headerProviders .stream() .filter(IsOnBehalfOf.class::isInstance) .map(d -> ((IsOnBehalfOf) d).getOnBehalfOf()) - .anyMatch(b -> b == NAMED_USER_CURRENT_TENANT || b == TECHNICAL_USER_CURRENT_TENANT); + .filter(Objects::nonNull) + .toList(); - // If the destination indicates that it is on behalf of the current tenant, include the tenant ID in the cache key to ensure proper isolation. - if( indicatesCurrentTenant ) { - return List.of(TenantAccessor.tryGetCurrentTenant().map(Tenant::getTenantId).getOrNull(), dest); + // If no OnBehalfOf indicators are present, return null to avoid caching + if( behalfOfIndicators.isEmpty() && !headerProviders.isEmpty() ) { + return null; } - // Otherwise, return a cache key that does not include tenant information, allowing sharing across tenants if other properties match. - return dest; + final boolean indicatesProviderTenant = + behalfOfIndicators.stream().allMatch(b -> b == OnBehalfOf.TECHNICAL_USER_PROVIDER); + + // If the destination indicates that it is on behalf of a provider tenant, return a cache key that does not include tenant information, allowing sharing across tenants if other properties match. + if( indicatesProviderTenant ) { + return dest; + } + // For other cases, include tenant information in the cache key to ensure proper isolation + return List.of(TenantAccessor.tryGetCurrentTenant().map(Tenant::getTenantId).getOrNull(), dest); }); } /** * Creates a provider that caches connection managers using a custom cache key extractor. *

- * The cache key extractor function is called for each request to determine which cached connection manager to - * use. Requests that produce equal cache keys (via {@link Object#equals(Object)}) will share the same + * The cache key extractor function is called for each HttpClient to determine which cached connection manager + * to use. Calls that produce equal cache keys (via {@link Object#equals(Object)}) will share the same * connection manager. *

*

@@ -292,14 +298,11 @@ public ConnectionPoolManagerProvider by( return ( settings, destination ) -> { final Object rawKey = cacheKeyExtractor.apply(destination); if( rawKey == null ) { - log - .debug( - "Creating new connection manager due to missing cache key for destination: {}", - destination); + log.debug("Creating new uncached connection manager for destination: {}", destination); return createConnectionManager(settings, destination); } return cacheFunction.apply(rawKey, key -> { - log.debug("Creating new connection manager for cache key: {}", rawKey); + log.debug("Creating new cached connection manager for key: {}", rawKey); return createConnectionManager(settings, destination); }); }; diff --git a/release_notes.md b/release_notes.md index bc140eba2..f048abbc9 100644 --- a/release_notes.md +++ b/release_notes.md @@ -14,6 +14,16 @@ ### ✨ New Functionality - [OpenAPI] Cloud SDK OpenAPI Generator now supports `apache-httpclient` library besides Spring RestTemplate through the newly introduced module `openapi-core-apache`. +- [Connectivity HttpClient5] _(Experimental)_ Added opt-in API for caching HTTP connection pool managers to reduce memory consumption. + Connection pool managers can consume ~100KB each, and this feature allows sharing them based on configurable caching strategies: + ```java + ApacheHttpClient5Factory factory = new ApacheHttpClient5FactoryBuilder() + .connectionPoolManagerProvider(ConnectionPoolManagerProviders.noCache()) // new API (default behavior) + .connectionPoolManagerProvider(ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf()) // new API + .build(); + ``` + Available caching strategies include `byCurrentTenant()`, `byDestinationName()`, `byIndicatedBehalfOf()`, and custom key extractors via `by(Function)`. + The `byIndicatedBehalfOf()` strategy intelligently determines tenant isolation requirements based on the destination's `OnBehalfOf` indication. ### 📈 Improvements From c1157c965b47edaa1e59515ce152697b777cc910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 19:55:45 +0100 Subject: [PATCH 5/8] Fix imports --- .../connectivity/ApacheHttpClient5FactoryBuilder.java | 6 +++--- .../connectivity/ConnectionPoolManagerProviders.java | 4 ++-- .../ConnectionPoolManagerProvidersTest.java | 11 ++++++----- .../connectivity/OAuth2HeaderProvider.java | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java index 583dbcc17..f8ee7277e 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5FactoryBuilder.java @@ -3,14 +3,14 @@ import java.time.Duration; import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import lombok.Setter; -import lombok.experimental.Accessors; import org.apache.hc.client5.http.classic.HttpClient; import com.google.common.annotations.Beta; +import lombok.Setter; +import lombok.experimental.Accessors; + /** * Builder class for a default implementation of the {@link ApacheHttpClient5Factory} interface. * diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java index 4adaf1d2c..3741ba346 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java @@ -14,8 +14,6 @@ import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.HttpClientConnectionManager; @@ -26,6 +24,8 @@ import org.apache.hc.core5.http.io.SocketConfig; import org.apache.hc.core5.util.Timeout; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.annotations.Beta; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java index 71f07a91d..b7bb506d4 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java @@ -3,23 +3,24 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatNullPointerException; +import java.util.Collections; +import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; import java.util.function.Function; -import lombok.Getter; -import lombok.RequiredArgsConstructor; +import javax.annotation.Nonnull; + import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.junit.jupiter.api.Test; import com.sap.cloud.sdk.cloudplatform.tenant.DefaultTenant; import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; -import javax.annotation.Nonnull; -import java.util.Collections; -import java.util.List; +import lombok.Getter; +import lombok.RequiredArgsConstructor; class ConnectionPoolManagerProvidersTest { diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java index 9f973064e..48d5c650f 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2HeaderProvider.java @@ -5,13 +5,13 @@ import java.util.List; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; import io.vavr.control.Option; import lombok.RequiredArgsConstructor; -import org.jetbrains.annotations.Nullable; @RequiredArgsConstructor class OAuth2HeaderProvider implements DestinationHeaderProvider, IsOnBehalfOf From 710eefe9fb27cab32365b9eb4d3fa57400411594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 19:59:02 +0100 Subject: [PATCH 6/8] Improve Naming --- .../ConnectionPoolManagerProviders.java | 2 +- .../ConnectionPoolManagerProvidersTest.java | 20 +++++++++---------- release_notes.md | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java index 3741ba346..debe43aa0 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java @@ -238,7 +238,7 @@ public ConnectionPoolManagerProvider byDestinationName() } @Nonnull - public ConnectionPoolManagerProvider byIndicatedBehalfOf() + public ConnectionPoolManagerProvider byOnBehalfOf() { return by(destination -> { // Check if the destination has any OnBehalfOf indicators in its custom header providers diff --git a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java index b7bb506d4..2e60f6414 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProvidersTest.java @@ -249,9 +249,9 @@ void testFunctionalInterfaceCanBeUsedWithLambda() } @Test - void testCachedByIndicatedBehalfOfWithCurrentTenantHeaderProvider() + void testCachedByOnBehalfOfWithCurrentTenantHeaderProvider() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byOnBehalfOf(); // Create a header provider that indicates NAMED_USER_CURRENT_TENANT final DestinationHeaderProvider namedUserProvider = @@ -286,9 +286,9 @@ void testCachedByIndicatedBehalfOfWithCurrentTenantHeaderProvider() } @Test - void testCachedByIndicatedBehalfOfWithTechnicalUserCurrentTenant() + void testCachedByOnBehalfOfWithTechnicalUserCurrentTenant() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byOnBehalfOf(); final DefaultHttpDestination dest = DefaultHttpDestination @@ -311,9 +311,9 @@ void testCachedByIndicatedBehalfOfWithTechnicalUserCurrentTenant() } @Test - void testCachedByIndicatedBehalfOfWithProviderUserSharesAcrossTenants() + void testCachedByOnBehalfOfWithProviderUserSharesAcrossTenants() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byOnBehalfOf(); // Create a header provider that indicates TECHNICAL_USER_PROVIDER (not current tenant) final DefaultHttpDestination dest = @@ -336,9 +336,9 @@ void testCachedByIndicatedBehalfOfWithProviderUserSharesAcrossTenants() } @Test - void testCachedByIndicatedBehalfOfWithNoHeaderProvider() + void testCachedByOnBehalfOfWithNoHeaderProvider() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byOnBehalfOf(); // Destination without any header provider final DefaultHttpDestination dest = DefaultHttpDestination.builder("http://example.com").build(); @@ -357,9 +357,9 @@ void testCachedByIndicatedBehalfOfWithNoHeaderProvider() } @Test - void testCachedByIndicatedBehalfOfWithNonDefaultHttpDestination() + void testCachedByOnBehalfOfWithNonDefaultHttpDestination() { - final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf(); + final ConnectionPoolManagerProvider provider = ConnectionPoolManagerProviders.cached().byOnBehalfOf(); // Non-DefaultHttpDestination should return null key and create new manager each time final HttpDestinationProperties nonDefaultDest = DefaultHttpDestination.builder("http://example.com").build(); diff --git a/release_notes.md b/release_notes.md index f048abbc9..3e7be64eb 100644 --- a/release_notes.md +++ b/release_notes.md @@ -19,11 +19,11 @@ ```java ApacheHttpClient5Factory factory = new ApacheHttpClient5FactoryBuilder() .connectionPoolManagerProvider(ConnectionPoolManagerProviders.noCache()) // new API (default behavior) - .connectionPoolManagerProvider(ConnectionPoolManagerProviders.cached().byIndicatedBehalfOf()) // new API + .connectionPoolManagerProvider(ConnectionPoolManagerProviders.cached().byOnBehalfOf()) // new API .build(); ``` - Available caching strategies include `byCurrentTenant()`, `byDestinationName()`, `byIndicatedBehalfOf()`, and custom key extractors via `by(Function)`. - The `byIndicatedBehalfOf()` strategy intelligently determines tenant isolation requirements based on the destination's `OnBehalfOf` indication. + Available caching strategies include `byCurrentTenant()`, `byDestinationName()`, `byOnBehalfOf()`, and custom key extractors via `by(Function)`. + The `byOnBehalfOf()` strategy intelligently determines tenant isolation requirements based on the destination's `OnBehalfOf` indication. ### 📈 Improvements From 5c126f117439e240c173f378757a6d1dcc1521f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 20:03:00 +0100 Subject: [PATCH 7/8] Add missing javadoc --- .../ConnectionPoolManagerProviders.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java index debe43aa0..7961f378f 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ConnectionPoolManagerProviders.java @@ -237,6 +237,19 @@ public ConnectionPoolManagerProvider byDestinationName() return by(dest -> dest != null ? dest.get(DestinationProperty.NAME).getOrNull() : null); } + /** + * Creates a provider that reuses the connection managers for different tenants if the destination is + * tenant-independent. + *

+ * The provider checks for the presence of {@link OnBehalfOf} indicators in the destination's custom header + * providers. If all indicators show that the destination is on behalf of a provider tenant, the same connection + * manager is reused across tenants. Otherwise, tenant information is included in the cache key to ensure + * isolation. + *

+ * + * @return A provider that reuses the connection managers for different tenants if the destination is + * tenant-independent. + */ @Nonnull public ConnectionPoolManagerProvider byOnBehalfOf() { From 09f5822d00a9eef8c3e3c684cd6ef4f6b84c040a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 10 Feb 2026 20:11:54 +0100 Subject: [PATCH 8/8] Fix Checkstyle --- .../connectivity/DefaultApacheHttpClient5Factory.java | 1 + .../connectivity/DefaultConnectionPoolSettings.java | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java index bd7d56789..cbf658215 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java @@ -55,6 +55,7 @@ public HttpClient createHttpClient( @Nullable final HttpDestinationProperties de } @Nonnull + @SuppressWarnings( "PMD.CloseResource" ) // The HttpClient instance and the connection manager instance are not being closed here. private CloseableHttpClient buildHttpClient( @Nullable final HttpDestinationProperties destination, @Nonnull final RequestConfig requestConfig ) diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java index 22879eca3..353adc54d 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultConnectionPoolSettings.java @@ -52,33 +52,33 @@ public class DefaultConnectionPoolSettings implements ConnectionPoolSettings */ @Nonnull @Builder.Default - Duration connectTimeout = ConnectionPoolSettings.DEFAULT_TIMEOUT; + Duration connectTimeout = DEFAULT_TIMEOUT; /** * The default socket timeout value for I/O operations on connections. */ @Nonnull @Builder.Default - Duration socketTimeout = ConnectionPoolSettings.DEFAULT_TIMEOUT; + Duration socketTimeout = DEFAULT_TIMEOUT; /** * The timeout when requesting a connection lease from the connection pool. */ @Nonnull @Builder.Default - Duration connectionRequestTimeout = ConnectionPoolSettings.DEFAULT_TIMEOUT; + Duration connectionRequestTimeout = DEFAULT_TIMEOUT; /** * The maximum number of total connections in the pool. */ @Builder.Default - int maxConnectionsTotal = ConnectionPoolSettings.DEFAULT_MAX_CONNECTIONS_TOTAL; + int maxConnectionsTotal = DEFAULT_MAX_CONNECTIONS_TOTAL; /** * The maximum number of connections per route (e.g., per remote host). */ @Builder.Default - int maxConnectionsPerRoute = ConnectionPoolSettings.DEFAULT_MAX_CONNECTIONS_PER_ROUTE; + int maxConnectionsPerRoute = DEFAULT_MAX_CONNECTIONS_PER_ROUTE; /** * Creates a new {@link DefaultConnectionPoolSettings} with default values.