From b43e3885637f731631ea2cdedf8bc3763e79be82 Mon Sep 17 00:00:00 2001 From: I538344 Date: Wed, 26 Nov 2025 14:58:36 +0100 Subject: [PATCH 01/21] feat: Circuit Breaker thing --- .../GetOrComputeSingleDestinationCommand.java | 19 +++++++++++++++++++ .../DestinationServiceAuthenticationTest.java | 15 ++++++++++++++- ...nationServicePrincipalPropagationTest.java | 18 ++++++++++++++++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java index 9c5943ac9..60ba9fbfa 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java @@ -26,6 +26,7 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import lombok.val; @Slf4j @RequiredArgsConstructor( access = AccessLevel.PRIVATE ) @@ -149,6 +150,10 @@ Try execute() if( result != null ) { return Try.success(result); } + if( !destinationExists() ) { + String msg = "Destination %s was not found among the destinations of the current tenant."; + return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); + } final Try maybeResult = Try.ofSupplier(destinationSupplier); if( maybeResult.isFailure() ) { @@ -263,6 +268,20 @@ private Destination getCachedDestination() return maybeDestination; } + private boolean destinationExists() + { + if( getAllCommand != null ) { + val allDestinations = getAllCommand.execute(); + if( allDestinations != null && allDestinations.isSuccess() ) { + return allDestinations + .get() + .stream() + .anyMatch(properties -> properties.get(DestinationProperty.NAME).contains(destinationName)); + } + } + return false; + } + /** * currently this effectively limits the cache duration to the change detection interval, because change detection * on certificates isn't possible in all cases See the note on {@link DestinationServiceFactory} where this property diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java index 914e6fce1..cfac22b0c 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java @@ -7,6 +7,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.matches; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -42,6 +43,12 @@ class DestinationServiceAuthenticationTest private static final String SERVICE_PATH_DESTINATION = "/v1/destinations/" + DESTINATION_NAME; + private static final String ALL_DESTINATIONS = """ + [{ + "Name": "CXT-HTTP-OAUTH" + }] + """; + @SuppressWarnings( "deprecation" ) private static final DestinationOptions DESTINATION_RETRIEVAL_LOOKUP_EXCHANGE = DestinationOptions @@ -62,6 +69,12 @@ class DestinationServiceAuthenticationTest void setMockAdapter() { mockAdapter = mock(DestinationServiceAdapter.class); + doReturn(ALL_DESTINATIONS) + .when(mockAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + doReturn(ALL_DESTINATIONS) + .when(mockAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); doThrow(new AssertionError("Unexpected invocation to mocked adapter")) .when(mockAdapter) .getConfigurationAsJson(anyString(), any()); @@ -443,6 +456,6 @@ private void mockAdapterResponse( "authTokens", tokens != null ? List.of(tokens) : Collections.emptyList()); - doReturn(new Gson().toJson(destination)).when(mockAdapter).getConfigurationAsJson(any(), eq(expectedStrategy)); + doReturn(new Gson().toJson(destination)).when(mockAdapter).getConfigurationAsJson(matches("/v1/destinations/.*"), eq(expectedStrategy)); } } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java index 1437bca9f..5cc58af5a 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java @@ -16,9 +16,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.matches; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -73,6 +73,11 @@ class DestinationServicePrincipalPropagationTest } } """; + private static final String ALL_DESTINATIONS = """ + [{ + "Name": "test" + }] + """; @RegisterExtension static TestContext context = TestContext.withThreadContext().resetCaches(); @@ -101,7 +106,16 @@ void setupConnectivity( @Nonnull final WireMockRuntimeInfo wm ) DefaultServiceBindingAccessor.setInstance(() -> List.of(connectivityService)); - doReturn(DESTINATION).when(destinationServiceAdapter).getConfigurationAsJson(anyString(), any()); + // WIP + doReturn(ALL_DESTINATIONS) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + doReturn(ALL_DESTINATIONS) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(DESTINATION) + .when(destinationServiceAdapter) + .getConfigurationAsJson(matches("/v1/destinations/.*"), any()); stubFor( post("/xsuaa/oauth/token") From 87aab242277aca22658ccfd7bd20b1c1b418f625 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Wed, 26 Nov 2025 17:15:48 +0100 Subject: [PATCH 02/21] tests green --- .../GetOrComputeSingleDestinationCommand.java | 4 +-- .../DestinationServiceAuthenticationTest.java | 34 ++++++++++++++----- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java index 60ba9fbfa..d8ca1add3 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java @@ -151,8 +151,8 @@ Try execute() return Try.success(result); } if( !destinationExists() ) { - String msg = "Destination %s was not found among the destinations of the current tenant."; - return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); + String msg = "Destination %s was not found among the destinations of the current tenant."; + return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); } final Try maybeResult = Try.ofSupplier(destinationSupplier); diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java index cfac22b0c..bce82cfa5 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java @@ -69,15 +69,13 @@ class DestinationServiceAuthenticationTest void setMockAdapter() { mockAdapter = mock(DestinationServiceAdapter.class); - doReturn(ALL_DESTINATIONS) - .when(mockAdapter) - .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - doReturn(ALL_DESTINATIONS) - .when(mockAdapter) - .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); doThrow(new AssertionError("Unexpected invocation to mocked adapter")) .when(mockAdapter) .getConfigurationAsJson(anyString(), any()); + doReturn(ALL_DESTINATIONS) + .when(mockAdapter) + .getConfigurationAsJson(matches("/v1/subaccountDestinations"), any()); + doReturn(ALL_DESTINATIONS).when(mockAdapter).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); sut = new DestinationService(mockAdapter); } @@ -99,6 +97,8 @@ void testBasicAuth() .containsExactlyInAnyOrder(new Header("Authorization", "Basic " + BASIC_AUTH)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -159,6 +159,8 @@ void testOAuthWithUserTokenExchange() verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedFirstStrategy)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedSecondStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -204,6 +206,8 @@ void testOAuthWithProvidedSystemUser() .containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + OAUTH_TOKEN)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -258,6 +262,8 @@ void testOAuth2JwtBearerWithUserTokenForwarding() .containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + oAuthToken)); verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -303,7 +309,9 @@ void testOAuth2PasswordWithoutUserToken() assertThat(dest.asHttp().getHeaders()) .containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + oAuthToken)); - verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -355,6 +363,8 @@ void testSAMLAssertion() new Header("x-sap-security-session", "create")); verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -395,7 +405,9 @@ void testSAPAssertionSSO() assertThat(dest.asHttp().getAuthenticationType()).isEqualTo(AuthenticationType.SAP_ASSERTION_SSO); assertThat(dest.asHttp().getHeaders()).containsExactlyInAnyOrder(new Header("Cookie", assertionCookie)); - verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -424,6 +436,8 @@ void testOAuth2RefreshToken() assertThat(dest.asHttp().getHeaders()).containsExactlyInAnyOrder(new Header("Authorization", "Bearer ey1234")); verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -456,6 +470,8 @@ private void mockAdapterResponse( "authTokens", tokens != null ? List.of(tokens) : Collections.emptyList()); - doReturn(new Gson().toJson(destination)).when(mockAdapter).getConfigurationAsJson(matches("/v1/destinations/.*"), eq(expectedStrategy)); + doReturn(new Gson().toJson(destination)) + .when(mockAdapter) + .getConfigurationAsJson(matches("/v1/destinations/.*"), eq(expectedStrategy)); } } From 0ec0870444f67b48024b066867bf1de4410eb451 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 27 Nov 2025 15:38:06 +0100 Subject: [PATCH 03/21] add feature switch --- .../connectivity/DestinationService.java | 13 +++++++-- .../GetOrComputeSingleDestinationCommand.java | 10 +++++-- .../GetOrComputeDestinationCommandTest.java | 28 +++++++++++-------- ...tionCommandWithoutAllDestinationsTest.java | 3 +- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 840869d5e..22b270f6a 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -16,6 +16,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import lombok.Setter; import org.apache.commons.lang3.exception.ExceptionUtils; import com.github.benmanes.caffeine.cache.Caffeine; @@ -79,6 +80,10 @@ public class DestinationService implements DestinationLoader @Getter( AccessLevel.PACKAGE ) private final ResilienceConfiguration allDestResilience; + @Nonnull + @Setter + private Boolean prependGetAllDestinationCall = false; + /** * Create instance with all default settings */ @@ -120,7 +125,7 @@ static ResilienceConfiguration createResilienceConfiguration( Try tryGetDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options ) { - return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination); + return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination, prependGetAllDestinationCall); } Destination loadAndParseDestination( final String destName, final DestinationOptions options ) @@ -839,7 +844,8 @@ private static Try getOrComputeDestination( @Nonnull final DestinationService loader, @Nonnull final String destinationName, @Nonnull final DestinationOptions options, - @Nonnull final BiFunction destinationDownloader ) + @Nonnull final BiFunction destinationDownloader, + @Nonnull final Boolean prependGetAllCall) { if( !cacheEnabled ) { return Try.ofSupplier(() -> destinationDownloader.apply(destinationName, options)); @@ -866,7 +872,8 @@ private static Try getOrComputeDestination( instanceSingle(), isolationLocks(), destinationDownloader, - getAllCommand); + getAllCommand, + prependGetAllCall); return command.flatMap(GetOrComputeSingleDestinationCommand::execute); } diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java index d8ca1add3..2710b5790 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java @@ -53,6 +53,8 @@ class GetOrComputeSingleDestinationCommand private final DestinationServiceTokenExchangeStrategy exchangeStrategy; @Nullable private final GetOrComputeAllDestinationsCommand getAllCommand; + @Nonnull + private final Boolean prependGetAllDestinationCall; @SuppressWarnings( "deprecation" ) static Try prepareCommand( @@ -61,7 +63,8 @@ static Try prepareCommand( @Nonnull final Cache destinationCache, @Nonnull final Cache isolationLocks, @Nonnull final BiFunction destinationRetriever, - @Nullable final GetOrComputeAllDestinationsCommand getAllCommand ) + @Nullable final GetOrComputeAllDestinationsCommand getAllCommand, + @Nonnull final Boolean prependGetAllDestinationCall) { final Supplier destinationSupplier = () -> destinationRetriever.apply(destinationName, destinationOptions); @@ -110,7 +113,8 @@ static Try prepareCommand( destinationCache, destinationSupplier, exchangeStrategy, - getAllCommand)); + getAllCommand, + prependGetAllDestinationCall)); } /** @@ -150,7 +154,7 @@ Try execute() if( result != null ) { return Try.success(result); } - if( !destinationExists() ) { + if( prependGetAllDestinationCall && !destinationExists() ) { String msg = "Destination %s was not found among the destinations of the current tenant."; return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java index 766f4e035..b3798415e 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java @@ -72,7 +72,7 @@ void testDefaultTokenExchangeStrategy() final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand("destination", options, destinationCache, isolationLocks, ( foo, bar ) -> null, null) + .prepareCommand("destination", options, destinationCache, isolationLocks, ( foo, bar ) -> null, null, false) .get(); assertThat(sut.getExchangeStrategy()).isEqualTo(DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE); @@ -158,7 +158,7 @@ private void assertCacheKeysMatchExchangeStrategy( final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) .get(); assertThat(sut.getCacheKey()).isEqualTo(expectedCacheKey); @@ -198,7 +198,7 @@ void testLookupOnlyOnUserTokenDestination() final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) .get(); final Try fetchedDestination = sut.execute(); @@ -236,7 +236,7 @@ void testLookupOnlyExchangeStrategy() final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) .get(); final Try fetchedDestination = sut.execute(); @@ -288,7 +288,8 @@ void testLookupThenExchangeStrategy() destinationCache, isolationLocks, function, - null) + null, + false) .get())); final Try fetchedDestination = sut.execute(); @@ -342,7 +343,8 @@ void testForwardUserTokenStrategyForUserPropagationDestination() destinationCache, isolationLocks, function, - null) + null, + false) .get())); final Try fetchedDestination = @@ -393,7 +395,8 @@ void testForwardUserTokenStrategyForUserPropagationDestinationWithoutJwt() destinationCache, isolationLocks, function, - null) + null, + false) .get())); final Try fetchedDestination = sut.execute(); @@ -437,7 +440,7 @@ void testForwardUserTokenStrategyForUserPropagationDestinationWithoutPrincipalUn .executeWithTenant( t1, () -> GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) .get()); final Try shouldBeFailure = TenantAccessor.executeWithTenant(t1, sut::execute); @@ -487,7 +490,8 @@ void testForwardUserTokenStrategyForClientCredentialsDestination() destinationCache, isolationLocks, function, - null) + null, + false) .get())); final Try fetchedDestination = PrincipalAccessor.executeWithPrincipal(p1, () -> sut.execute()); @@ -542,7 +546,8 @@ void testChangeDetectionOnValidAuthToken() destinationCache, isolationLocks, destinationRetriever, - getAllCommand) + getAllCommand, + false) .flatMap(GetOrComputeSingleDestinationCommand::execute); assertThat(result.isSuccess()).isTrue(); @@ -627,7 +632,8 @@ void testFirstDestinationAccessDoesNotTriggerGetAllRequest() destinationCache, isolationLocks, destinationRetriever, - getAllMock) + getAllMock, + false) .get(); final Try result = sut.execute(); 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 0ea3bfa14..f5eebcfdc 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 @@ -233,7 +233,8 @@ private void runTest( TestCase testCase ) destinationCache, isolationLocks, destinationService::loadAndParseDestination, - null); + null, + false); if( maybeCommand.isFailure() ) { if( testCase.isCommandCreationExpectedToSucceed() ) { From 7b0e572c5efca79038e9e21d60ac0bea7949913b Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 27 Nov 2025 15:38:19 +0100 Subject: [PATCH 04/21] add tests for feature --- .../connectivity/DestinationServiceTest.java | 73 +++++++++++++++++-- 1 file changed, 68 insertions(+), 5 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java index a23533da1..e05ad6c62 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java @@ -113,6 +113,15 @@ class DestinationServiceTest "ProxyType": "Internet", "KeyStorePassword": "password", "KeyStoreLocation": "aaa" + }, + { + "Name": "SomeDestinationName", + "Type": "HTTP", + "URL": "https://a.s4hana.ondemand.com", + "Authentication": "ClientCertificateAuthentication", + "ProxyType": "Internet", + "KeyStorePassword": "password", + "KeyStoreLocation": "aaa" }] """; @@ -442,7 +451,7 @@ void testGettingDestinationProperties() assertThat(destinationList) .extracting(d -> d.get(DestinationProperty.NAME).get()) - .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT"); + .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT", destinationName); final DestinationProperties destination = destinationList @@ -478,7 +487,7 @@ void testGettingDestinationPropertiesProvider() final Collection destinationList = loader.getAllDestinationProperties(ALWAYS_PROVIDER); assertThat(destinationList) .extracting(d -> d.get(DestinationProperty.NAME).get()) - .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT"); + .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT", destinationName); } @Test @@ -494,7 +503,7 @@ void testGettingDestinationPropertiesSubscriber() final Collection destinationList = loader.getAllDestinationProperties(ONLY_SUBSCRIBER); assertThat(destinationList) .extracting(d -> d.get(DestinationProperty.NAME).get()) - .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT"); + .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT", destinationName); } @Test @@ -589,10 +598,10 @@ void testGetAllDestinationsForProvider() final List destinationList = new ArrayList<>(); destinations.get().forEach(destinationList::add); - assertThat(destinationList.size()).isEqualTo(3); + assertThat(destinationList.size()).isEqualTo(4); assertThat(destinationList) .extracting(d -> d.get(DestinationProperty.NAME).get()) - .containsOnly("CC8-HTTP-BASIC", "CC8-HTTP-CERT", "CC8-HTTP-CERT1"); + .containsOnly("CC8-HTTP-BASIC", "CC8-HTTP-CERT", "CC8-HTTP-CERT1", destinationName); } @SuppressWarnings( "deprecation" ) @@ -1910,4 +1919,58 @@ private String createHttpDestinationServiceResponse( final String name, final St } """, name, url); } + + @Test + void testPrependGetAllDestinationsCall() + { + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(responseSubaccountDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + + loader.setPrependGetAllDestinationCall(true); + + final DestinationOptions options = + DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); + Destination result = loader.tryGetDestination(destinationName).get(); + + // verify all results are cached + verify(destinationServiceAdapter, times(1)) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + verify(destinationServiceAdapter, times(1)) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(destinationServiceAdapter, times(1)) + .getConfigurationAsJson(eq("/v1/destinations/" + destinationName), any()); + verifyNoMoreInteractions(destinationServiceAdapter); + + loader.setPrependGetAllDestinationCall(false); + } + + @Test + void testPrependGetAllDestinationsCallWithMissingDestination() + { + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(responseSubaccountDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + + loader.setPrependGetAllDestinationCall(true); + + final DestinationOptions options = + DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); + assertThatThrownBy(() -> loader.tryGetDestination("thisDestinationDoesNotExist").get()).isInstanceOf(DestinationAccessException.class).hasMessageContaining("was not found among the destinations of the current tenant."); + + // verify all results are cached + verify(destinationServiceAdapter, times(1)) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + verify(destinationServiceAdapter, times(1)) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verifyNoMoreInteractions(destinationServiceAdapter); + + loader.setPrependGetAllDestinationCall(false); + } } From 077b7849d0bea937691a04c27219e079af53f247 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 27 Nov 2025 15:46:19 +0100 Subject: [PATCH 05/21] remove other changes from tests --- .../DestinationServiceAuthenticationTest.java | 29 +------------------ ...nationServicePrincipalPropagationTest.java | 18 ++---------- 2 files changed, 3 insertions(+), 44 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java index bce82cfa5..990f098b3 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java @@ -7,7 +7,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.matches; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -43,12 +42,6 @@ class DestinationServiceAuthenticationTest private static final String SERVICE_PATH_DESTINATION = "/v1/destinations/" + DESTINATION_NAME; - private static final String ALL_DESTINATIONS = """ - [{ - "Name": "CXT-HTTP-OAUTH" - }] - """; - @SuppressWarnings( "deprecation" ) private static final DestinationOptions DESTINATION_RETRIEVAL_LOOKUP_EXCHANGE = DestinationOptions @@ -72,10 +65,6 @@ void setMockAdapter() doThrow(new AssertionError("Unexpected invocation to mocked adapter")) .when(mockAdapter) .getConfigurationAsJson(anyString(), any()); - doReturn(ALL_DESTINATIONS) - .when(mockAdapter) - .getConfigurationAsJson(matches("/v1/subaccountDestinations"), any()); - doReturn(ALL_DESTINATIONS).when(mockAdapter).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); sut = new DestinationService(mockAdapter); } @@ -97,8 +86,6 @@ void testBasicAuth() .containsExactlyInAnyOrder(new Header("Authorization", "Basic " + BASIC_AUTH)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -159,8 +146,6 @@ void testOAuthWithUserTokenExchange() verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedFirstStrategy)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedSecondStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -206,8 +191,6 @@ void testOAuthWithProvidedSystemUser() .containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + OAUTH_TOKEN)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -262,8 +245,6 @@ void testOAuth2JwtBearerWithUserTokenForwarding() .containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + oAuthToken)); verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -310,8 +291,6 @@ void testOAuth2PasswordWithoutUserToken() .containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + oAuthToken)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -363,8 +342,6 @@ void testSAMLAssertion() new Header("x-sap-security-session", "create")); verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -406,8 +383,6 @@ void testSAPAssertionSSO() assertThat(dest.asHttp().getHeaders()).containsExactlyInAnyOrder(new Header("Cookie", assertionCookie)); verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -436,8 +411,6 @@ void testOAuth2RefreshToken() assertThat(dest.asHttp().getHeaders()).containsExactlyInAnyOrder(new Header("Authorization", "Bearer ey1234")); verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy)); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(mockAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verifyNoMoreInteractions(mockAdapter); } @@ -472,6 +445,6 @@ private void mockAdapterResponse( doReturn(new Gson().toJson(destination)) .when(mockAdapter) - .getConfigurationAsJson(matches("/v1/destinations/.*"), eq(expectedStrategy)); + .getConfigurationAsJson(any(), eq(expectedStrategy)); } } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java index 5cc58af5a..1437bca9f 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java @@ -16,9 +16,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.matches; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -73,11 +73,6 @@ class DestinationServicePrincipalPropagationTest } } """; - private static final String ALL_DESTINATIONS = """ - [{ - "Name": "test" - }] - """; @RegisterExtension static TestContext context = TestContext.withThreadContext().resetCaches(); @@ -106,16 +101,7 @@ void setupConnectivity( @Nonnull final WireMockRuntimeInfo wm ) DefaultServiceBindingAccessor.setInstance(() -> List.of(connectivityService)); - // WIP - doReturn(ALL_DESTINATIONS) - .when(destinationServiceAdapter) - .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - doReturn(ALL_DESTINATIONS) - .when(destinationServiceAdapter) - .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); - doReturn(DESTINATION) - .when(destinationServiceAdapter) - .getConfigurationAsJson(matches("/v1/destinations/.*"), any()); + doReturn(DESTINATION).when(destinationServiceAdapter).getConfigurationAsJson(anyString(), any()); stubFor( post("/xsuaa/oauth/token") From 75e300b22795dce0a1a761cf0301b45c4d57fb4e Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 27 Nov 2025 15:53:06 +0100 Subject: [PATCH 06/21] code style --- .../connectivity/DestinationService.java | 12 +- .../GetOrComputeSingleDestinationCommand.java | 2 +- .../DestinationServiceAuthenticationTest.java | 4 +- .../connectivity/DestinationServiceTest.java | 104 +++++++++--------- .../GetOrComputeDestinationCommandTest.java | 18 ++- 5 files changed, 78 insertions(+), 62 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 22b270f6a..061c27d18 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -16,7 +16,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import lombok.Setter; import org.apache.commons.lang3.exception.ExceptionUtils; import com.github.benmanes.caffeine.cache.Caffeine; @@ -42,6 +41,7 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; +import lombok.Setter; import lombok.extern.slf4j.Slf4j; /** @@ -125,7 +125,13 @@ static ResilienceConfiguration createResilienceConfiguration( Try tryGetDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options ) { - return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination, prependGetAllDestinationCall); + return Cache + .getOrComputeDestination( + this, + destinationName, + options, + this::loadAndParseDestination, + prependGetAllDestinationCall); } Destination loadAndParseDestination( final String destName, final DestinationOptions options ) @@ -845,7 +851,7 @@ private static Try getOrComputeDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options, @Nonnull final BiFunction destinationDownloader, - @Nonnull final Boolean prependGetAllCall) + @Nonnull final Boolean prependGetAllCall ) { if( !cacheEnabled ) { return Try.ofSupplier(() -> destinationDownloader.apply(destinationName, options)); diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java index 2710b5790..a47c8ec20 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java @@ -64,7 +64,7 @@ static Try prepareCommand( @Nonnull final Cache isolationLocks, @Nonnull final BiFunction destinationRetriever, @Nullable final GetOrComputeAllDestinationsCommand getAllCommand, - @Nonnull final Boolean prependGetAllDestinationCall) + @Nonnull final Boolean prependGetAllDestinationCall ) { final Supplier destinationSupplier = () -> destinationRetriever.apply(destinationName, destinationOptions); diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java index 990f098b3..7f899e0e5 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java @@ -443,8 +443,6 @@ private void mockAdapterResponse( "authTokens", tokens != null ? List.of(tokens) : Collections.emptyList()); - doReturn(new Gson().toJson(destination)) - .when(mockAdapter) - .getConfigurationAsJson(any(), eq(expectedStrategy)); + doReturn(new Gson().toJson(destination)).when(mockAdapter).getConfigurationAsJson(any(), eq(expectedStrategy)); } } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java index e05ad6c62..601d1aaf2 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java @@ -1920,57 +1920,55 @@ private String createHttpDestinationServiceResponse( final String name, final St """, name, url); } - @Test - void testPrependGetAllDestinationsCall() - { - doReturn(responseServiceInstanceDestination) - .when(destinationServiceAdapter) - .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); - doReturn(responseSubaccountDestination) - .when(destinationServiceAdapter) - .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - - loader.setPrependGetAllDestinationCall(true); - - final DestinationOptions options = - DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); - Destination result = loader.tryGetDestination(destinationName).get(); - - // verify all results are cached - verify(destinationServiceAdapter, times(1)) - .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); - verify(destinationServiceAdapter, times(1)) - .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verify(destinationServiceAdapter, times(1)) - .getConfigurationAsJson(eq("/v1/destinations/" + destinationName), any()); - verifyNoMoreInteractions(destinationServiceAdapter); - - loader.setPrependGetAllDestinationCall(false); - } - - @Test - void testPrependGetAllDestinationsCallWithMissingDestination() - { - doReturn(responseServiceInstanceDestination) - .when(destinationServiceAdapter) - .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); - doReturn(responseSubaccountDestination) - .when(destinationServiceAdapter) - .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - - loader.setPrependGetAllDestinationCall(true); - - final DestinationOptions options = - DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); - assertThatThrownBy(() -> loader.tryGetDestination("thisDestinationDoesNotExist").get()).isInstanceOf(DestinationAccessException.class).hasMessageContaining("was not found among the destinations of the current tenant."); - - // verify all results are cached - verify(destinationServiceAdapter, times(1)) - .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); - verify(destinationServiceAdapter, times(1)) - .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - verifyNoMoreInteractions(destinationServiceAdapter); - - loader.setPrependGetAllDestinationCall(false); - } + @Test + void testPrependGetAllDestinationsCall() + { + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(responseSubaccountDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + + loader.setPrependGetAllDestinationCall(true); + + final DestinationOptions options = + DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); + Destination result = loader.tryGetDestination(destinationName).get(); + + // verify all results are cached + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(destinationServiceAdapter, times(1)) + .getConfigurationAsJson(eq("/v1/destinations/" + destinationName), any()); + verifyNoMoreInteractions(destinationServiceAdapter); + + loader.setPrependGetAllDestinationCall(false); + } + + @Test + void testPrependGetAllDestinationsCallWithMissingDestination() + { + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(responseSubaccountDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + + loader.setPrependGetAllDestinationCall(true); + + final DestinationOptions options = + DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); + assertThatThrownBy(() -> loader.tryGetDestination("thisDestinationDoesNotExist").get()) + .isInstanceOf(DestinationAccessException.class) + .hasMessageContaining("was not found among the destinations of the current tenant."); + + // verify all results are cached + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verifyNoMoreInteractions(destinationServiceAdapter); + + loader.setPrependGetAllDestinationCall(false); + } } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java index b3798415e..d8dee84cc 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java @@ -72,7 +72,14 @@ void testDefaultTokenExchangeStrategy() final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand("destination", options, destinationCache, isolationLocks, ( foo, bar ) -> null, null, false) + .prepareCommand( + "destination", + options, + destinationCache, + isolationLocks, + ( foo, bar ) -> null, + null, + false) .get(); assertThat(sut.getExchangeStrategy()).isEqualTo(DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE); @@ -440,7 +447,14 @@ void testForwardUserTokenStrategyForUserPropagationDestinationWithoutPrincipalUn .executeWithTenant( t1, () -> GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) + .prepareCommand( + DESTINATION_NAME, + options, + destinationCache, + isolationLocks, + function, + null, + false) .get()); final Try shouldBeFailure = TenantAccessor.executeWithTenant(t1, sut::execute); From 54757771519d46aaf9a398b2b2406c7a064bf8bd Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 27 Nov 2025 16:39:50 +0100 Subject: [PATCH 07/21] PMD --- .../connectivity/GetOrComputeSingleDestinationCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java index a47c8ec20..04997e1fc 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java @@ -155,7 +155,7 @@ Try execute() return Try.success(result); } if( prependGetAllDestinationCall && !destinationExists() ) { - String msg = "Destination %s was not found among the destinations of the current tenant."; + final String msg = "Destination %s was not found among the destinations of the current tenant."; return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); } From 2501904b8349f932a373ec9ef845fc817089a4f7 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Fri, 28 Nov 2025 12:48:52 +0100 Subject: [PATCH 08/21] move feature switch --- .../connectivity/DestinationService.java | 23 +++++++++++-------- .../connectivity/DestinationServiceTest.java | 9 ++++---- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 061c27d18..a5a228694 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -80,10 +80,6 @@ public class DestinationService implements DestinationLoader @Getter( AccessLevel.PACKAGE ) private final ResilienceConfiguration allDestResilience; - @Nonnull - @Setter - private Boolean prependGetAllDestinationCall = false; - /** * Create instance with all default settings */ @@ -130,8 +126,7 @@ static ResilienceConfiguration createResilienceConfiguration( this, destinationName, options, - this::loadAndParseDestination, - prependGetAllDestinationCall); + this::loadAndParseDestination); } Destination loadAndParseDestination( final String destName, final DestinationOptions options ) @@ -437,6 +432,8 @@ public static final class Cache private static boolean cacheEnabled = true; private static boolean changeDetectionEnabled = true; + private static boolean getAllDocumentsPrepended = false; + // JONAS: put feature switch here static { recreateSingleCache(); @@ -454,6 +451,14 @@ static boolean isChangeDetectionEnabled() return changeDetectionEnabled; } + static boolean isGetAllDocumentsPrepended() { + return getAllDocumentsPrepended; + } + + public static void setGetAllDocumentsPrepended(boolean bool) { + getAllDocumentsPrepended = bool; + } + @Nonnull static com.github.benmanes.caffeine.cache.Cache instanceSingle() { @@ -507,6 +512,7 @@ static void reset() cacheEnabled = true; } changeDetectionEnabled = true; + getAllDocumentsPrepended = false; sizeLimit = Option.some(DEFAULT_SIZE_LIMIT); expirationDuration = Option.some(DEFAULT_EXPIRATION_DURATION); @@ -850,8 +856,7 @@ private static Try getOrComputeDestination( @Nonnull final DestinationService loader, @Nonnull final String destinationName, @Nonnull final DestinationOptions options, - @Nonnull final BiFunction destinationDownloader, - @Nonnull final Boolean prependGetAllCall ) + @Nonnull final BiFunction destinationDownloader) { if( !cacheEnabled ) { return Try.ofSupplier(() -> destinationDownloader.apply(destinationName, options)); @@ -879,7 +884,7 @@ private static Try getOrComputeDestination( isolationLocks(), destinationDownloader, getAllCommand, - prependGetAllCall); + getAllDocumentsPrepended); return command.flatMap(GetOrComputeSingleDestinationCommand::execute); } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java index 601d1aaf2..ffb6933f0 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java @@ -1930,7 +1930,8 @@ void testPrependGetAllDestinationsCall() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - loader.setPrependGetAllDestinationCall(true); + DestinationService.Cache.setGetAllDocumentsPrepended(true); + final DestinationOptions options = DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); @@ -1943,7 +1944,7 @@ void testPrependGetAllDestinationsCall() .getConfigurationAsJson(eq("/v1/destinations/" + destinationName), any()); verifyNoMoreInteractions(destinationServiceAdapter); - loader.setPrependGetAllDestinationCall(false); + DestinationService.Cache.setGetAllDocumentsPrepended(false); } @Test @@ -1956,7 +1957,7 @@ void testPrependGetAllDestinationsCallWithMissingDestination() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - loader.setPrependGetAllDestinationCall(true); + DestinationService.Cache.setGetAllDocumentsPrepended(true); final DestinationOptions options = DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); @@ -1969,6 +1970,6 @@ void testPrependGetAllDestinationsCallWithMissingDestination() verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); verifyNoMoreInteractions(destinationServiceAdapter); - loader.setPrependGetAllDestinationCall(false); + DestinationService.Cache.setGetAllDocumentsPrepended(false); } } From 31924b3dca4e3f4543b37c8328cf60792f9b068a Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Fri, 28 Nov 2025 12:56:22 +0100 Subject: [PATCH 09/21] codestyle --- .../connectivity/DestinationService.java | 20 ++++++++----------- .../connectivity/DestinationServiceTest.java | 9 ++++----- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index a5a228694..7ee95b847 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -41,7 +41,6 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; -import lombok.Setter; import lombok.extern.slf4j.Slf4j; /** @@ -121,12 +120,7 @@ static ResilienceConfiguration createResilienceConfiguration( Try tryGetDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options ) { - return Cache - .getOrComputeDestination( - this, - destinationName, - options, - this::loadAndParseDestination); + return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination); } Destination loadAndParseDestination( final String destName, final DestinationOptions options ) @@ -451,12 +445,14 @@ static boolean isChangeDetectionEnabled() return changeDetectionEnabled; } - static boolean isGetAllDocumentsPrepended() { - return getAllDocumentsPrepended; + static boolean isGetAllDocumentsPrepended() + { + return getAllDocumentsPrepended; } - public static void setGetAllDocumentsPrepended(boolean bool) { - getAllDocumentsPrepended = bool; + public static void setGetAllDocumentsPrepended( boolean bool ) + { + getAllDocumentsPrepended = bool; } @Nonnull @@ -856,7 +852,7 @@ private static Try getOrComputeDestination( @Nonnull final DestinationService loader, @Nonnull final String destinationName, @Nonnull final DestinationOptions options, - @Nonnull final BiFunction destinationDownloader) + @Nonnull final BiFunction destinationDownloader ) { if( !cacheEnabled ) { return Try.ofSupplier(() -> destinationDownloader.apply(destinationName, options)); diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java index ffb6933f0..0bee80cab 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java @@ -1930,8 +1930,7 @@ void testPrependGetAllDestinationsCall() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - DestinationService.Cache.setGetAllDocumentsPrepended(true); - + DestinationService.Cache.setGetAllDocumentsPrepended(true); final DestinationOptions options = DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); @@ -1944,7 +1943,7 @@ void testPrependGetAllDestinationsCall() .getConfigurationAsJson(eq("/v1/destinations/" + destinationName), any()); verifyNoMoreInteractions(destinationServiceAdapter); - DestinationService.Cache.setGetAllDocumentsPrepended(false); + DestinationService.Cache.setGetAllDocumentsPrepended(false); } @Test @@ -1957,7 +1956,7 @@ void testPrependGetAllDestinationsCallWithMissingDestination() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - DestinationService.Cache.setGetAllDocumentsPrepended(true); + DestinationService.Cache.setGetAllDocumentsPrepended(true); final DestinationOptions options = DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); @@ -1970,6 +1969,6 @@ void testPrependGetAllDestinationsCallWithMissingDestination() verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); verifyNoMoreInteractions(destinationServiceAdapter); - DestinationService.Cache.setGetAllDocumentsPrepended(false); + DestinationService.Cache.setGetAllDocumentsPrepended(false); } } From 4ab435e2120754c6cbd0079f170ce79ea5971dfe Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Mon, 1 Dec 2025 10:57:47 +0100 Subject: [PATCH 10/21] codestyle --- .../cloudplatform/connectivity/DestinationService.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 7ee95b847..98ca51a81 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -16,6 +16,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import lombok.Setter; import org.apache.commons.lang3.exception.ExceptionUtils; import com.github.benmanes.caffeine.cache.Caffeine; @@ -426,8 +427,8 @@ public static final class Cache private static boolean cacheEnabled = true; private static boolean changeDetectionEnabled = true; + @Setter private static boolean getAllDocumentsPrepended = false; - // JONAS: put feature switch here static { recreateSingleCache(); @@ -450,12 +451,7 @@ static boolean isGetAllDocumentsPrepended() return getAllDocumentsPrepended; } - public static void setGetAllDocumentsPrepended( boolean bool ) - { - getAllDocumentsPrepended = bool; - } - - @Nonnull + @Nonnull static com.github.benmanes.caffeine.cache.Cache instanceSingle() { throwIfDisabled(); From a3b144fbf805266a3cdfae1f20d9f73df6a4913d Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Tue, 2 Dec 2025 10:19:32 +0100 Subject: [PATCH 11/21] formatting --- .../sdk/cloudplatform/connectivity/DestinationService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 98ca51a81..9579e0e46 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -16,7 +16,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import lombok.Setter; import org.apache.commons.lang3.exception.ExceptionUtils; import com.github.benmanes.caffeine.cache.Caffeine; @@ -42,6 +41,7 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; +import lombok.Setter; import lombok.extern.slf4j.Slf4j; /** @@ -451,7 +451,7 @@ static boolean isGetAllDocumentsPrepended() return getAllDocumentsPrepended; } - @Nonnull + @Nonnull static com.github.benmanes.caffeine.cache.Cache instanceSingle() { throwIfDisabled(); From 689acb8572e505bf4ca39f3f21b0d5cb0e8290c1 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Tue, 2 Dec 2025 15:34:00 +0100 Subject: [PATCH 12/21] reviews --- .../connectivity/DestinationService.java | 35 ++++++++++++---- .../GetOrComputeSingleDestinationCommand.java | 27 +----------- .../connectivity/DestinationServiceTest.java | 8 ++-- .../GetOrComputeDestinationCommandTest.java | 42 +++++-------------- ...tionCommandWithoutAllDestinationsTest.java | 3 +- 5 files changed, 45 insertions(+), 70 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 9579e0e46..2c81428c1 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -41,8 +41,8 @@ import lombok.Getter; import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; -import lombok.Setter; import lombok.extern.slf4j.Slf4j; +import lombok.val; /** * Retrieves destination information from the SCP destination service on Cloud Foundry. @@ -121,6 +121,10 @@ static ResilienceConfiguration createResilienceConfiguration( Try tryGetDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options ) { + if( Cache.preLookupCheckEnabled && !preLookupCheckSuccessful(destinationName) ) { + final String msg = "Destination %s was not found among the destinations of the current tenant."; + return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); + } return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination); } @@ -385,6 +389,17 @@ private static boolean hasCauseAssignableFrom( @Nonnull final Throwable t, @Nonn return ExceptionUtils.getThrowableList(t).stream().map(Throwable::getClass).anyMatch(cls::isAssignableFrom); } + private boolean preLookupCheckSuccessful( String destinationName ) + { + val allDestinations = getAllDestinationProperties(); + if( !allDestinations.isEmpty() ) { + return allDestinations + .stream() + .anyMatch(properties -> properties.get(DestinationProperty.NAME).contains(destinationName)); + } + return false; + } + /** * Helper class that encapsulates all caching related configuration options. * @@ -427,8 +442,8 @@ public static final class Cache private static boolean cacheEnabled = true; private static boolean changeDetectionEnabled = true; - @Setter - private static boolean getAllDocumentsPrepended = false; + @Getter( AccessLevel.PACKAGE ) + private static boolean preLookupCheckEnabled = false; static { recreateSingleCache(); @@ -446,9 +461,14 @@ static boolean isChangeDetectionEnabled() return changeDetectionEnabled; } - static boolean isGetAllDocumentsPrepended() + public static void enablePreLookupCheck() + { + preLookupCheckEnabled = true; + } + + public static void disablePreLookupCheck() { - return getAllDocumentsPrepended; + preLookupCheckEnabled = false; } @Nonnull @@ -504,7 +524,7 @@ static void reset() cacheEnabled = true; } changeDetectionEnabled = true; - getAllDocumentsPrepended = false; + preLookupCheckEnabled = false; sizeLimit = Option.some(DEFAULT_SIZE_LIMIT); expirationDuration = Option.some(DEFAULT_EXPIRATION_DURATION); @@ -875,8 +895,7 @@ private static Try getOrComputeDestination( instanceSingle(), isolationLocks(), destinationDownloader, - getAllCommand, - getAllDocumentsPrepended); + getAllCommand); return command.flatMap(GetOrComputeSingleDestinationCommand::execute); } diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java index 04997e1fc..9c5943ac9 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeSingleDestinationCommand.java @@ -26,7 +26,6 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import lombok.val; @Slf4j @RequiredArgsConstructor( access = AccessLevel.PRIVATE ) @@ -53,8 +52,6 @@ class GetOrComputeSingleDestinationCommand private final DestinationServiceTokenExchangeStrategy exchangeStrategy; @Nullable private final GetOrComputeAllDestinationsCommand getAllCommand; - @Nonnull - private final Boolean prependGetAllDestinationCall; @SuppressWarnings( "deprecation" ) static Try prepareCommand( @@ -63,8 +60,7 @@ static Try prepareCommand( @Nonnull final Cache destinationCache, @Nonnull final Cache isolationLocks, @Nonnull final BiFunction destinationRetriever, - @Nullable final GetOrComputeAllDestinationsCommand getAllCommand, - @Nonnull final Boolean prependGetAllDestinationCall ) + @Nullable final GetOrComputeAllDestinationsCommand getAllCommand ) { final Supplier destinationSupplier = () -> destinationRetriever.apply(destinationName, destinationOptions); @@ -113,8 +109,7 @@ static Try prepareCommand( destinationCache, destinationSupplier, exchangeStrategy, - getAllCommand, - prependGetAllDestinationCall)); + getAllCommand)); } /** @@ -154,10 +149,6 @@ Try execute() if( result != null ) { return Try.success(result); } - if( prependGetAllDestinationCall && !destinationExists() ) { - final String msg = "Destination %s was not found among the destinations of the current tenant."; - return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); - } final Try maybeResult = Try.ofSupplier(destinationSupplier); if( maybeResult.isFailure() ) { @@ -272,20 +263,6 @@ private Destination getCachedDestination() return maybeDestination; } - private boolean destinationExists() - { - if( getAllCommand != null ) { - val allDestinations = getAllCommand.execute(); - if( allDestinations != null && allDestinations.isSuccess() ) { - return allDestinations - .get() - .stream() - .anyMatch(properties -> properties.get(DestinationProperty.NAME).contains(destinationName)); - } - } - return false; - } - /** * currently this effectively limits the cache duration to the change detection interval, because change detection * on certificates isn't possible in all cases See the note on {@link DestinationServiceFactory} where this property diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java index 0bee80cab..7ef0cada7 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java @@ -1930,7 +1930,7 @@ void testPrependGetAllDestinationsCall() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - DestinationService.Cache.setGetAllDocumentsPrepended(true); + DestinationService.Cache.enablePreLookupCheck(); final DestinationOptions options = DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); @@ -1943,7 +1943,7 @@ void testPrependGetAllDestinationsCall() .getConfigurationAsJson(eq("/v1/destinations/" + destinationName), any()); verifyNoMoreInteractions(destinationServiceAdapter); - DestinationService.Cache.setGetAllDocumentsPrepended(false); + DestinationService.Cache.disablePreLookupCheck(); } @Test @@ -1956,7 +1956,7 @@ void testPrependGetAllDestinationsCallWithMissingDestination() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - DestinationService.Cache.setGetAllDocumentsPrepended(true); + DestinationService.Cache.enablePreLookupCheck(); final DestinationOptions options = DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); @@ -1969,6 +1969,6 @@ void testPrependGetAllDestinationsCallWithMissingDestination() verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); verifyNoMoreInteractions(destinationServiceAdapter); - DestinationService.Cache.setGetAllDocumentsPrepended(false); + DestinationService.Cache.disablePreLookupCheck(); } } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java index d8dee84cc..766f4e035 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeDestinationCommandTest.java @@ -72,14 +72,7 @@ void testDefaultTokenExchangeStrategy() final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand( - "destination", - options, - destinationCache, - isolationLocks, - ( foo, bar ) -> null, - null, - false) + .prepareCommand("destination", options, destinationCache, isolationLocks, ( foo, bar ) -> null, null) .get(); assertThat(sut.getExchangeStrategy()).isEqualTo(DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE); @@ -165,7 +158,7 @@ private void assertCacheKeysMatchExchangeStrategy( final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) .get(); assertThat(sut.getCacheKey()).isEqualTo(expectedCacheKey); @@ -205,7 +198,7 @@ void testLookupOnlyOnUserTokenDestination() final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) .get(); final Try fetchedDestination = sut.execute(); @@ -243,7 +236,7 @@ void testLookupOnlyExchangeStrategy() final GetOrComputeSingleDestinationCommand sut = GetOrComputeSingleDestinationCommand - .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null, false) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) .get(); final Try fetchedDestination = sut.execute(); @@ -295,8 +288,7 @@ void testLookupThenExchangeStrategy() destinationCache, isolationLocks, function, - null, - false) + null) .get())); final Try fetchedDestination = sut.execute(); @@ -350,8 +342,7 @@ void testForwardUserTokenStrategyForUserPropagationDestination() destinationCache, isolationLocks, function, - null, - false) + null) .get())); final Try fetchedDestination = @@ -402,8 +393,7 @@ void testForwardUserTokenStrategyForUserPropagationDestinationWithoutJwt() destinationCache, isolationLocks, function, - null, - false) + null) .get())); final Try fetchedDestination = sut.execute(); @@ -447,14 +437,7 @@ void testForwardUserTokenStrategyForUserPropagationDestinationWithoutPrincipalUn .executeWithTenant( t1, () -> GetOrComputeSingleDestinationCommand - .prepareCommand( - DESTINATION_NAME, - options, - destinationCache, - isolationLocks, - function, - null, - false) + .prepareCommand(DESTINATION_NAME, options, destinationCache, isolationLocks, function, null) .get()); final Try shouldBeFailure = TenantAccessor.executeWithTenant(t1, sut::execute); @@ -504,8 +487,7 @@ void testForwardUserTokenStrategyForClientCredentialsDestination() destinationCache, isolationLocks, function, - null, - false) + null) .get())); final Try fetchedDestination = PrincipalAccessor.executeWithPrincipal(p1, () -> sut.execute()); @@ -560,8 +542,7 @@ void testChangeDetectionOnValidAuthToken() destinationCache, isolationLocks, destinationRetriever, - getAllCommand, - false) + getAllCommand) .flatMap(GetOrComputeSingleDestinationCommand::execute); assertThat(result.isSuccess()).isTrue(); @@ -646,8 +627,7 @@ void testFirstDestinationAccessDoesNotTriggerGetAllRequest() destinationCache, isolationLocks, destinationRetriever, - getAllMock, - false) + getAllMock) .get(); final Try result = sut.execute(); 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 f5eebcfdc..0ea3bfa14 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 @@ -233,8 +233,7 @@ private void runTest( TestCase testCase ) destinationCache, isolationLocks, destinationService::loadAndParseDestination, - null, - false); + null); if( maybeCommand.isFailure() ) { if( testCase.isCommandCreationExpectedToSucceed() ) { From 624c10a895c94e7c491dd1eb8355355ad4c4a09f Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Tue, 2 Dec 2025 15:47:29 +0100 Subject: [PATCH 13/21] javadoc --- .../cloudplatform/connectivity/DestinationService.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 2c81428c1..f0e99f805 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -442,7 +442,6 @@ public static final class Cache private static boolean cacheEnabled = true; private static boolean changeDetectionEnabled = true; - @Getter( AccessLevel.PACKAGE ) private static boolean preLookupCheckEnabled = false; static { @@ -461,11 +460,16 @@ static boolean isChangeDetectionEnabled() return changeDetectionEnabled; } - public static void enablePreLookupCheck() + /** + * Enables checking if a destination exists before trying to call it directly when invoking {@link #tryGetDestination}. + */ + public static void enablePreLookupCheck() { preLookupCheckEnabled = true; } - + /** + * Disables checking if a destination exists before trying to call it directly when invoking {@link #tryGetDestination}. + */ public static void disablePreLookupCheck() { preLookupCheckEnabled = false; From d573e5176dd4f9a5bb92f53816457ffd1d9781f9 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Wed, 3 Dec 2025 12:19:02 +0100 Subject: [PATCH 14/21] codestyle --- .../connectivity/DestinationService.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index f0e99f805..ae0eecc7b 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -460,15 +460,18 @@ static boolean isChangeDetectionEnabled() return changeDetectionEnabled; } - /** - * Enables checking if a destination exists before trying to call it directly when invoking {@link #tryGetDestination}. - */ - public static void enablePreLookupCheck() + /** + * Enables checking if a destination exists before trying to call it directly when invoking + * {@link #tryGetDestination}. + */ + public static void enablePreLookupCheck() { preLookupCheckEnabled = true; } + /** - * Disables checking if a destination exists before trying to call it directly when invoking {@link #tryGetDestination}. + * Disables checking if a destination exists before trying to call it directly when invoking + * {@link #tryGetDestination}. */ public static void disablePreLookupCheck() { From 6cf6d0f7344f805abd8075ae6437ad9bb3077222 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Wed, 3 Dec 2025 12:20:44 +0100 Subject: [PATCH 15/21] release notes --- release_notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_notes.md b/release_notes.md index 3112514a9..6a83a3fd3 100644 --- a/release_notes.md +++ b/release_notes.md @@ -12,7 +12,7 @@ ### ✨ New Functionality -- +- Added new functionality to check if a destination exists before trying to call it directly when invoking `DestinationService.tryGetDestination`. ### 📈 Improvements From d8f9139f4aa5eae6e6ef760c721dcaa7b56d86a3 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Wed, 3 Dec 2025 12:25:41 +0100 Subject: [PATCH 16/21] PMD --- .../sdk/cloudplatform/connectivity/DestinationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index ae0eecc7b..eed77bdf5 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -389,7 +389,7 @@ private static boolean hasCauseAssignableFrom( @Nonnull final Throwable t, @Nonn return ExceptionUtils.getThrowableList(t).stream().map(Throwable::getClass).anyMatch(cls::isAssignableFrom); } - private boolean preLookupCheckSuccessful( String destinationName ) + private boolean preLookupCheckSuccessful( final String destinationName ) { val allDestinations = getAllDestinationProperties(); if( !allDestinations.isEmpty() ) { From d62b9704601b37fd479eb0735ee399101bfc8cdd Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Mon, 8 Dec 2025 16:26:10 +0100 Subject: [PATCH 17/21] reviews + enable by default --- .../connectivity/DestinationService.java | 19 +++++++++---------- .../DestinationServiceAuthenticationTest.java | 2 ++ ...nationServicePrincipalPropagationTest.java | 2 ++ .../connectivity/DestinationServiceTest.java | 12 +++--------- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index eed77bdf5..004a0d3ee 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -42,7 +42,6 @@ import lombok.NoArgsConstructor; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; -import lombok.val; /** * Retrieves destination information from the SCP destination service on Cloud Foundry. @@ -391,13 +390,9 @@ private static boolean hasCauseAssignableFrom( @Nonnull final Throwable t, @Nonn private boolean preLookupCheckSuccessful( final String destinationName ) { - val allDestinations = getAllDestinationProperties(); - if( !allDestinations.isEmpty() ) { - return allDestinations - .stream() - .anyMatch(properties -> properties.get(DestinationProperty.NAME).contains(destinationName)); - } - return false; + return getAllDestinationProperties() + .stream() + .anyMatch(properties -> properties.get(DestinationProperty.NAME).contains(destinationName)); } /** @@ -442,7 +437,7 @@ public static final class Cache private static boolean cacheEnabled = true; private static boolean changeDetectionEnabled = true; - private static boolean preLookupCheckEnabled = false; + private static boolean preLookupCheckEnabled = true; static { recreateSingleCache(); @@ -463,6 +458,8 @@ static boolean isChangeDetectionEnabled() /** * Enables checking if a destination exists before trying to call it directly when invoking * {@link #tryGetDestination}. + * + * @since 5.25.0 */ public static void enablePreLookupCheck() { @@ -472,6 +469,8 @@ public static void enablePreLookupCheck() /** * Disables checking if a destination exists before trying to call it directly when invoking * {@link #tryGetDestination}. + * + * @since 5.25.0 */ public static void disablePreLookupCheck() { @@ -531,7 +530,7 @@ static void reset() cacheEnabled = true; } changeDetectionEnabled = true; - preLookupCheckEnabled = false; + preLookupCheckEnabled = true; sizeLimit = Option.some(DEFAULT_SIZE_LIMIT); expirationDuration = Option.some(DEFAULT_EXPIRATION_DURATION); diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java index 7f899e0e5..62a9e7127 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceAuthenticationTest.java @@ -66,6 +66,8 @@ void setMockAdapter() .when(mockAdapter) .getConfigurationAsJson(anyString(), any()); sut = new DestinationService(mockAdapter); + // Disable PreLookupCheck to simplify test setup + DestinationService.Cache.disablePreLookupCheck(); } @Test diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java index 1437bca9f..a771cd901 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java @@ -108,6 +108,8 @@ void setupConnectivity( @Nonnull final WireMockRuntimeInfo wm ) .withRequestBody( equalTo("grant_type=client_credentials&client_secret=CLIENT_SECRET&client_id=CLIENT_ID")) .willReturn(okJson("{\"access_token\":\"provider-client-credentials\",\"expires_in\":3600}"))); + // Disable PreLookupCheck to simplify test setup + DestinationService.Cache.disablePreLookupCheck(); } @AfterEach diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java index 7ef0cada7..7cf385aac 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java @@ -345,6 +345,9 @@ class DestinationServiceTest @BeforeEach void setup() { + // Disable PreLookupCheck to simplify test setup + DestinationService.Cache.disablePreLookupCheck(); + providerTenant = new DefaultTenant("provider-tenant"); subscriberTenant = new DefaultTenant("subscriber-tenant"); context.setTenant(subscriberTenant); @@ -1932,18 +1935,13 @@ void testPrependGetAllDestinationsCall() DestinationService.Cache.enablePreLookupCheck(); - final DestinationOptions options = - DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); Destination result = loader.tryGetDestination(destinationName).get(); - // verify all results are cached verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); verify(destinationServiceAdapter, times(1)) .getConfigurationAsJson(eq("/v1/destinations/" + destinationName), any()); verifyNoMoreInteractions(destinationServiceAdapter); - - DestinationService.Cache.disablePreLookupCheck(); } @Test @@ -1958,17 +1956,13 @@ void testPrependGetAllDestinationsCallWithMissingDestination() DestinationService.Cache.enablePreLookupCheck(); - final DestinationOptions options = - DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); assertThatThrownBy(() -> loader.tryGetDestination("thisDestinationDoesNotExist").get()) .isInstanceOf(DestinationAccessException.class) .hasMessageContaining("was not found among the destinations of the current tenant."); - // verify all results are cached verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); verifyNoMoreInteractions(destinationServiceAdapter); - DestinationService.Cache.disablePreLookupCheck(); } } From 5ba5226cfe4ef4176bf3f64ac0384656e14da727 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Mon, 8 Dec 2025 16:37:28 +0100 Subject: [PATCH 18/21] reviews --- release_notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_notes.md b/release_notes.md index 6a83a3fd3..6139cc979 100644 --- a/release_notes.md +++ b/release_notes.md @@ -12,7 +12,7 @@ ### ✨ New Functionality -- Added new functionality to check if a destination exists before trying to call it directly when invoking `DestinationService.tryGetDestination`. +- `DestinationService.tryGetDestination` now checks if the given destination exists before trying to call it directly. This behaviour is enabled by default and can be disabled via `DestinationService.Cache.disablePreLookupCheck`. ### 📈 Improvements From 0bf62f6bb9539ddcbb13085bcd935a46c0ea6c19 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Tue, 9 Dec 2025 14:41:59 +0100 Subject: [PATCH 19/21] reviews --- .../connectivity/DestinationService.java | 13 +------------ .../connectivity/DestinationServiceTest.java | 12 +++++++----- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 004a0d3ee..45d8e83ff 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -122,7 +122,7 @@ static ResilienceConfiguration createResilienceConfiguration( { if( Cache.preLookupCheckEnabled && !preLookupCheckSuccessful(destinationName) ) { final String msg = "Destination %s was not found among the destinations of the current tenant."; - return Try.failure(new DestinationAccessException(String.format(msg, destinationName))); + return Try.failure(new DestinationNotFoundException(String.format(msg, destinationName))); } return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination); } @@ -455,17 +455,6 @@ static boolean isChangeDetectionEnabled() return changeDetectionEnabled; } - /** - * Enables checking if a destination exists before trying to call it directly when invoking - * {@link #tryGetDestination}. - * - * @since 5.25.0 - */ - public static void enablePreLookupCheck() - { - preLookupCheckEnabled = true; - } - /** * Disables checking if a destination exists before trying to call it directly when invoking * {@link #tryGetDestination}. diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java index 7cf385aac..7bb4bb669 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceTest.java @@ -1926,6 +1926,9 @@ private String createHttpDestinationServiceResponse( final String name, final St @Test void testPrependGetAllDestinationsCall() { + // Reset Cache to re-enable the PreLookupCheck + DestinationService.Cache.reset(); + doReturn(responseServiceInstanceDestination) .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); @@ -1933,8 +1936,6 @@ void testPrependGetAllDestinationsCall() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - DestinationService.Cache.enablePreLookupCheck(); - Destination result = loader.tryGetDestination(destinationName).get(); verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); @@ -1947,6 +1948,9 @@ void testPrependGetAllDestinationsCall() @Test void testPrependGetAllDestinationsCallWithMissingDestination() { + // Reset Cache to re-enable the PreLookupCheck + DestinationService.Cache.reset(); + doReturn(responseServiceInstanceDestination) .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); @@ -1954,10 +1958,8 @@ void testPrependGetAllDestinationsCallWithMissingDestination() .when(destinationServiceAdapter) .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); - DestinationService.Cache.enablePreLookupCheck(); - assertThatThrownBy(() -> loader.tryGetDestination("thisDestinationDoesNotExist").get()) - .isInstanceOf(DestinationAccessException.class) + .isInstanceOf(DestinationNotFoundException.class) .hasMessageContaining("was not found among the destinations of the current tenant."); verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); From 7b6e46ec8cfdcb175ebac0ea3662aa95e37cda80 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Wed, 10 Dec 2025 14:53:06 +0100 Subject: [PATCH 20/21] reviews --- .../sdk/cloudplatform/connectivity/DestinationService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index 45d8e83ff..a21e0fb88 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -457,7 +457,8 @@ static boolean isChangeDetectionEnabled() /** * Disables checking if a destination exists before trying to call it directly when invoking - * {@link #tryGetDestination}. + * {@link #tryGetDestination}. All available destinations can be found with {@link + * #getAllDestinationProperties}. * * @since 5.25.0 */ From 2aef26bf0df10ea81ef78198a1e960b740490619 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Wed, 10 Dec 2025 15:25:40 +0100 Subject: [PATCH 21/21] codestyle --- .../sdk/cloudplatform/connectivity/DestinationService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java index a21e0fb88..2d91b987c 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java @@ -457,8 +457,8 @@ static boolean isChangeDetectionEnabled() /** * Disables checking if a destination exists before trying to call it directly when invoking - * {@link #tryGetDestination}. All available destinations can be found with {@link - * #getAllDestinationProperties}. + * {@link #tryGetDestination}. All available destinations can be found with + * {@link #getAllDestinationProperties}. * * @since 5.25.0 */