diff --git a/cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpDestination.java b/cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpDestination.java index abf2160107..3d68e80ae6 100644 --- a/cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpDestination.java +++ b/cloudplatform/cloudplatform-connectivity/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpDestination.java @@ -43,6 +43,7 @@ import lombok.experimental.Accessors; import lombok.experimental.Delegate; import lombok.extern.slf4j.Slf4j; +import lombok.val; /** * Immutable default implementation of the {@link HttpDestination} interface. @@ -66,6 +67,13 @@ public final class DefaultHttpDestination implements HttpDestination @Getter( AccessLevel.PACKAGE ) private final ImmutableList customHeaderProviders; + /** + * Lazily initialized and cached header providers loaded via FacadeLocator. This ensures the same instances are + * shared across all DefaultHttpDestination instances. Uses volatile to ensure visibility of changes across threads. + */ + @Nullable + private static volatile ImmutableList cachedHeaderProvidersFromClassLoading; + @Nonnull private final ImmutableList headerProvidersFromClassLoading; @@ -114,10 +122,7 @@ private DefaultHttpDestination( this.customHeaders = customHeaders != null ? ImmutableList.
builder().addAll(customHeaders).build() : ImmutableList.of(); - final Collection headerProvidersFromClassLoading = - FacadeLocator.getFacades(DestinationHeaderProvider.class); - this.headerProvidersFromClassLoading = - ImmutableList. builder().addAll(headerProvidersFromClassLoading).build(); + this.headerProvidersFromClassLoading = getCachedHeaderProvidersFromClassLoading(); this.customHeaderProviders = customHeaderProviders != null @@ -144,6 +149,32 @@ private DefaultHttpDestination( .build(); } + /** + * Lazily initializes and returns the cached header providers from class loading. Uses double-checked locking to + * ensure thread-safe lazy initialization while minimizing synchronization overhead. + * + * @return The immutable list of header providers loaded via FacadeLocator. + */ + @Nonnull + private static ImmutableList getCachedHeaderProvidersFromClassLoading() + { + ImmutableList cached = cachedHeaderProvidersFromClassLoading; + if( cached == null ) { + synchronized( DefaultHttpDestination.class ) { + cached = cachedHeaderProvidersFromClassLoading; + if( cached == null ) { + cached = + ImmutableList + . builder() + .addAll(FacadeLocator.getFacades(DestinationHeaderProvider.class)) + .build(); + cachedHeaderProvidersFromClassLoading = cached; + } + } + } + return cached; + } + /** * Verifies that the given "generic" destination might be decorated into a {@code DefaultHttpDestination}. * @@ -511,8 +542,7 @@ public static Builder fromDestination( @Nonnull final Destination destination ) .getPropertyNames() .forEach(propertyName -> builder.property(propertyName, destination.get(propertyName).get())); - if( destination instanceof DefaultHttpDestination ) { - final DefaultHttpDestination httpDestination = (DefaultHttpDestination) destination; + if( destination instanceof DefaultHttpDestination httpDestination ) { builder.headers(httpDestination.customHeaders); builder .headerProviders(httpDestination.getCustomHeaderProviders().toArray(new DestinationHeaderProvider[0])); @@ -536,25 +566,47 @@ public boolean equals( @Nullable final Object o ) } final DefaultHttpDestination that = (DefaultHttpDestination) o; - return new EqualsBuilder() - .append(baseProperties, that.baseProperties) - .append(customHeaders, that.customHeaders) - .append( - resolveCertificatesOnly(keyStoreSupplier.get().getOrNull()), - resolveCertificatesOnly(that.keyStoreSupplier.get().getOrNull())) - .append(resolveCertificatesOnly(trustStore), resolveCertificatesOnly(that.trustStore)) - .isEquals(); + + if( headerProvidersFromClassLoading.size() != that.headerProvidersFromClassLoading.size() + || customHeaderProviders.size() != that.customHeaderProviders.size() ) { + return false; + } + + val builder = + new EqualsBuilder() + .append(baseProperties, that.baseProperties) + .append(customHeaders, that.customHeaders) + .append( + resolveCertificatesOnly(keyStoreSupplier.get().getOrNull()), + resolveCertificatesOnly(that.keyStoreSupplier.get().getOrNull())) + .append(resolveCertificatesOnly(trustStore), resolveCertificatesOnly(that.trustStore)); + + customHeaderProviders + .forEach( + provider -> builder + .append(provider, that.customHeaderProviders.get(customHeaderProviders.indexOf(provider)))); + headerProvidersFromClassLoading + .forEach( + provider -> builder + .append( + provider, + that.headerProvidersFromClassLoading.get(headerProvidersFromClassLoading.indexOf(provider)))); + return builder.isEquals(); } @Override public int hashCode() { - return new HashCodeBuilder(17, 37) - .append(baseProperties) - .append(customHeaders) - .append(resolveKeyStoreHashCode(keyStoreSupplier.get().getOrNull())) - .append(resolveKeyStoreHashCode(trustStore)) - .toHashCode(); + val builder = + new HashCodeBuilder(17, 37) + .append(baseProperties) + .append(customHeaders) + .append(resolveKeyStoreHashCode(keyStoreSupplier.get().getOrNull())) + .append(resolveKeyStoreHashCode(trustStore)); + + customHeaderProviders.forEach(builder::append); + headerProvidersFromClassLoading.forEach(builder::append); + return builder.toHashCode(); } /** diff --git a/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClientWrapper.java b/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClientWrapper.java index e1e3739f87..e4b074087c 100644 --- a/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClientWrapper.java +++ b/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClientWrapper.java @@ -93,10 +93,7 @@ public String toString() HttpClientWrapper withDestination( final HttpDestinationProperties destination ) { - // explicitly check the reference equality, since equals doesn't check header providers - // this is a slight improvement, avoiding unnecessary wrapper instantiation - // in cases where destination objects are reused / served from cache - if( !destination.equals(this.destination) ) { + if( !destination.getUri().equals(this.destination.getUri()) ) { throw new ShouldNotHappenException( "This method must not be used outside of updating an instance of HttpClientWrapper for http clients served from the HttpClientCache."); } diff --git a/cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpClientCacheTest.java b/cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpClientCacheTest.java index 9d06adb885..ecfd747d03 100644 --- a/cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpClientCacheTest.java +++ b/cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultHttpClientCacheTest.java @@ -1,5 +1,12 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static com.github.tomakehurst.wiremock.stubbing.Scenario.STARTED; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -20,6 +27,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.parallel.Isolated; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import com.sap.cloud.sdk.cloudplatform.cache.CacheManager; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; import com.sap.cloud.sdk.cloudplatform.security.principal.DefaultPrincipal; @@ -28,9 +36,15 @@ import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; import com.sap.cloud.sdk.testutil.TestContext; +import lombok.SneakyThrows; + @Isolated class DefaultHttpClientCacheTest { + @RegisterExtension + static final WireMockExtension WIRE_MOCK_SERVER = + WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); + private static final HttpDestination DESTINATION = DefaultHttpDestination.builder("https://url1").build(); private static final DefaultHttpDestination USER_TOKEN_EXCHANGE_DESTINATION = DefaultHttpDestination @@ -357,6 +371,87 @@ void testInvalidatePrincipalCacheEntriesWithUserTokenExchangeDestination() assertThat(unclearedClientWithoutDestination).isSameAs(sut.tryGetHttpClient(FACTORY).get()); } + @Test + @SneakyThrows + void testCachedEqualHttpClientsClosingBehavior() + { + WIRE_MOCK_SERVER.stubFor(get(anyUrl()).willReturn(ok())); + + final DefaultHttpDestination destination1 = + DefaultHttpDestination + .builder(WIRE_MOCK_SERVER.baseUrl()) + .headerProviders(c -> List.of(new Header("Authorization", "Bearer old"))) + .build(); + final DefaultHttpDestination destination2 = + DefaultHttpDestination + .builder(WIRE_MOCK_SERVER.baseUrl()) + .headerProviders(c -> List.of(new Header("Authorization", "Bearer new"))) + .build(); + + final HttpClient client1 = sut.tryGetHttpClient(destination1, FACTORY).get(); + assertThat(((HttpClientWrapper) client1).getDestination()).isSameAs(destination1); + final HttpClient client2 = sut.tryGetHttpClient(destination2, FACTORY).get(); + assertThat(((HttpClientWrapper) client2).getDestination()).isSameAs(destination2); + + assertThat(client1).isNotSameAs(client2); + + // When using the exact same destination object, the same http-client wrapper should be returned + final HttpClient client1Again = sut.tryGetHttpClient(destination1, FACTORY).get(); + assertThat(((HttpClientWrapper) client1Again).getDestination()).isSameAs(destination1); + assertThat(client1Again).isSameAs(client1); + + // simulate garbage collection on client1 + ((HttpClientWrapper) client1).close(); + + // since client1 did not inherit client2 connection manager, client2 is not shut down + client2.execute(new HttpGet()); + WIRE_MOCK_SERVER.verify(1, getRequestedFor(anyUrl())); + } + + @Test + @SneakyThrows + void testCachedDestinationIsReused() + { + WIRE_MOCK_SERVER + .stubFor( + get(anyUrl()) + .withHeader("Authorization", equalTo("Bearer token1")) + .inScenario("Refreshing token") + .whenScenarioStateIs(STARTED) + .willReturn(ok()) + .willSetStateTo("First token sent")); + WIRE_MOCK_SERVER + .stubFor( + get(anyUrl()) + .withHeader("Authorization", equalTo("Bearer token2")) + .inScenario("Refreshing token") + .whenScenarioStateIs("First token sent") + .willReturn(ok())); + + final DefaultHttpDestination destination = + DefaultHttpDestination.builder(WIRE_MOCK_SERVER.baseUrl()).headerProviders(c -> getHeaders()).build(); + + // token1 is sent + final HttpClient client1 = sut.tryGetHttpClient(destination, FACTORY).get(); + client1.execute(new HttpGet()); + WIRE_MOCK_SERVER.verify(1, getRequestedFor(anyUrl()).withHeader("Authorization", equalTo("Bearer token1"))); + + // token2 is sent + final HttpClient client2 = sut.tryGetHttpClient(destination, FACTORY).get(); + client2.execute(new HttpGet()); + WIRE_MOCK_SERVER.verify(1, getRequestedFor(anyUrl()).withHeader("Authorization", equalTo("Bearer token2"))); + + // Because the destination is cached, the same client is reused + assertThat(client1).isSameAs(client2); + } + + private int count = 1; + + private List
getHeaders() + { + return List.of(new Header("Authorization", "Bearer token" + count++)); + } + @Test void testPrincipalPropagationIsPrincipalIsolated() { diff --git a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5Wrapper.java b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5Wrapper.java index e7e48783ee..5a16c1ac39 100644 --- a/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5Wrapper.java +++ b/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ApacheHttpClient5Wrapper.java @@ -78,10 +78,7 @@ public void close( final CloseMode closeMode ) ApacheHttpClient5Wrapper withDestination( final HttpDestinationProperties destination ) { - // explicitly check the reference equality, since equals doesn't check header providers - // this is a slight improvement, avoiding unnecessary wrapper instantiation - // in cases where destination objects are reused / served from cache - if( !destination.equals(this.destination) ) { + if( !destination.getUri().equals(this.destination.getUri()) ) { throw new ShouldNotHappenException( "This method must not be used outside of updating an instance of ApacheHttpClient5Wrapper for http clients served from the ApacheHttpClient5Cache."); } 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 7295ec9d96..7c4d09f369 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 @@ -1,12 +1,18 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static com.github.tomakehurst.wiremock.stubbing.Scenario.STARTED; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -14,12 +20,12 @@ import org.apache.hc.client5.http.classic.HttpClient; import org.apache.hc.client5.http.classic.methods.HttpGet; -import org.apache.hc.core5.http.ClassicHttpRequest; -import org.apache.hc.core5.http.message.BasicHeader; +import org.apache.hc.client5.http.impl.classic.BasicHttpClientResponseHandler; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import com.sap.cloud.sdk.cloudplatform.cache.CacheManager; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.HttpClientInstantiationException; import com.sap.cloud.sdk.cloudplatform.security.principal.DefaultPrincipal; @@ -28,8 +34,14 @@ import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; import com.sap.cloud.sdk.testutil.TestContext; +import lombok.SneakyThrows; + class DefaultApacheHttpClient5CacheTest { + @RegisterExtension + static final WireMockExtension WIRE_MOCK_SERVER = + WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build(); + private static final List TENANTS = Arrays.asList(new DefaultTenant("tenant#1"), new DefaultTenant("tenant#2"), null); private static final List PRINCIPALS = @@ -348,49 +360,84 @@ void testInvalidatePrincipalCacheEntriesWithUserTokenExchangeDestination() } @Test - //This is a known limitation of excluding header providers in the equality check of destinations - void testGetClientReturnsSameClientForDestinationsWithOnlyDifferentHeaderProviders() + @SneakyThrows + void testCachedEqualHttpClientsClosingBehavior() { - final Header header1 = new Header("foo", "bar"); - final Header header2 = new Header("foo1", "bar1"); + WIRE_MOCK_SERVER.stubFor(get(anyUrl()).willReturn(ok())); - final DefaultHttpDestination firstDestination = + final DefaultHttpDestination destination1 = DefaultHttpDestination - .builder("http://some-uri") - .headerProviders(( any ) -> Collections.singletonList(header1)) + .builder(WIRE_MOCK_SERVER.baseUrl()) + .headerProviders(c -> List.of(new Header("Authorization", "Bearer old"))) .build(); - - final DefaultHttpDestination secondDestination = + final DefaultHttpDestination destination2 = DefaultHttpDestination - .fromDestination(firstDestination) - .headerProviders(( any ) -> Collections.singletonList(header2)) + .builder(WIRE_MOCK_SERVER.baseUrl()) + .headerProviders(c -> List.of(new Header("Authorization", "Bearer new"))) .build(); - final ApacheHttpClient5Wrapper client1 = - (ApacheHttpClient5Wrapper) sut.tryGetHttpClient(firstDestination, FACTORY).get(); - final ApacheHttpClient5Wrapper client2 = - (ApacheHttpClient5Wrapper) sut.tryGetHttpClient(secondDestination, FACTORY).get(); - - assertThat(client1.getDestination()).isSameAs(firstDestination); - assertThat(client2.getDestination()).isSameAs(secondDestination); - - final ClassicHttpRequest request1 = client1.wrapRequest(new HttpGet("/")); - final ClassicHttpRequest request2 = client2.wrapRequest(new HttpGet("/")); - - final List headersRequest1 = new ArrayList<>(); - final List headersRequest2 = new ArrayList<>(); - request1.headerIterator().forEachRemaining(headersRequest1::add); - request2.headerIterator().forEachRemaining(headersRequest2::add); - - // recursive comparison because BasicHeader doesn't implement equals/hashCode - assertThat(headersRequest1) - .usingRecursiveFieldByFieldElementComparator() - .containsExactly(new BasicHeader(header1.getName(), header1.getValue())); - assertThat(headersRequest2) - .usingRecursiveFieldByFieldElementComparator() - .containsExactly( - new BasicHeader(header1.getName(), header1.getValue()), - new BasicHeader(header2.getName(), header2.getValue())); + final HttpClient client1 = sut.tryGetHttpClient(destination1, FACTORY).get(); + assertThat(((ApacheHttpClient5Wrapper) client1).getDestination()).isSameAs(destination1); + final HttpClient client2 = sut.tryGetHttpClient(destination2, FACTORY).get(); + assertThat(((ApacheHttpClient5Wrapper) client2).getDestination()).isSameAs(destination2); + + assertThat(client1).isNotSameAs(client2); + + // When using the exact same destination object, the same http-client wrapper should be returned + final HttpClient client1Again = sut.tryGetHttpClient(destination1, FACTORY).get(); + assertThat(((ApacheHttpClient5Wrapper) client1Again).getDestination()).isSameAs(destination1); + assertThat(client1Again).isSameAs(client1); + + // simulate garbage collection on client1 + ((ApacheHttpClient5Wrapper) client1).close(); + + // since client1 did not inherit client2 connection manager, client2 is not shut down + client2.execute(new HttpGet("/"), new BasicHttpClientResponseHandler()); + WIRE_MOCK_SERVER.verify(1, getRequestedFor(anyUrl())); + } + + @Test + @SneakyThrows + void testCachedDestinationIsReused() + { + WIRE_MOCK_SERVER + .stubFor( + get(anyUrl()) + .withHeader("Authorization", equalTo("Bearer token1")) + .inScenario("Refreshing token") + .whenScenarioStateIs(STARTED) + .willReturn(ok()) + .willSetStateTo("First token sent")); + WIRE_MOCK_SERVER + .stubFor( + get(anyUrl()) + .withHeader("Authorization", equalTo("Bearer token2")) + .inScenario("Refreshing token") + .whenScenarioStateIs("First token sent") + .willReturn(ok())); + + final DefaultHttpDestination destination = + DefaultHttpDestination.builder(WIRE_MOCK_SERVER.baseUrl()).headerProviders(c -> getHeaders()).build(); + + // token1 is sent + final HttpClient client1 = sut.tryGetHttpClient(destination, FACTORY).get(); + client1.execute(new HttpGet("/"), new BasicHttpClientResponseHandler()); + WIRE_MOCK_SERVER.verify(1, getRequestedFor(anyUrl()).withHeader("Authorization", equalTo("Bearer token1"))); + + // token2 is sent + final HttpClient client2 = sut.tryGetHttpClient(destination, FACTORY).get(); + client2.execute(new HttpGet("/"), new BasicHttpClientResponseHandler()); + WIRE_MOCK_SERVER.verify(1, getRequestedFor(anyUrl()).withHeader("Authorization", equalTo("Bearer token2"))); + + // Because the destination is cached, the same client is reused + assertThat(client1).isSameAs(client2); + } + + private int count = 1; + + private List
getHeaders() + { + return List.of(new Header("Authorization", "Bearer token" + count++)); } @Test diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommandWithoutAllDestinationsTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommandWithoutAllDestinationsTest.java index 0ea3bfa147..31e4342e64 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommandWithoutAllDestinationsTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommandWithoutAllDestinationsTest.java @@ -261,7 +261,6 @@ private void runTest( TestCase testCase ) softly.fail("Expected command execution to fail, but it succeeded."); } - softly.assertThat(maybeDestination.get()).isEqualTo(testCase.getExpectedDestination()); // sanity checks no cache was hit if( testCase.getTokenExchangeStrategy() == DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE && DestinationUtility.requiresUserTokenExchange(testCase.getExpectedDestination()) ) { diff --git a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java index 2b05514fbd..117291b703 100644 --- a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java +++ b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java @@ -16,7 +16,6 @@ import java.net.URI; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -118,7 +117,7 @@ void testOptionsMatcher() .builder() .copy(Collections.emptyMap()) .withServiceIdentifier(TEST_SERVICE) - .withTags(Arrays.asList("test")) + .withTags(List.of("test")) .build(); final ServiceBindingDestinationOptions options = ServiceBindingDestinationOptions.forService(binding).build(); @@ -209,11 +208,8 @@ void testClientSecretBasedBinding() assertThat(sut.tryGetDestination(OPTIONS_WITH_EMPTY_BINDING).get()) .as("The destination should not be cached.") .isNotSameAs(result.get()); - assertThat(sut.tryGetDestination(OPTIONS_WITH_EMPTY_BINDING).get()) - .as("The destination objects should be equal so that they use the same HTTP client.") - .isEqualTo(result.get()); - verify(sut, times(3)) + verify(sut, times(2)) .toDestination( eq(baseUrl), eq(tokenUrl), @@ -396,9 +392,6 @@ void testProxiedDestination() assertThat(secondInvocationResult) .as("There should not be a cache in place for proxied destinations.") .isNotSameAs(result); - assertThat(secondInvocationResult) - .as("The destination objects should be equal so that they use the same HTTP client.") - .isEqualTo(result); verify(sut, times(2)) .createHeaderProvider( diff --git a/release_notes.md b/release_notes.md index 8fb3414093..e5b00e5e55 100644 --- a/release_notes.md +++ b/release_notes.md @@ -8,7 +8,7 @@ ### 🔧 Compatibility Notes -- +- [Connectivity] Destination objects with different header providers are now considered not equal. ### ✨ New Functionality @@ -16,7 +16,7 @@ ### 📈 Improvements -- +- [Connectivity] Fixed `Connection pool shut down` edge-case when closing a client with a destination that has a custom header provider. ### 🐛 Fixed Issues