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 15dab98c7..64c4f8917 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 @@ -1,5 +1,8 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.getRetrievalStrategy; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.CURRENT_TENANT; + import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; @@ -11,6 +14,7 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiFunction; import java.util.function.Function; +import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -122,9 +126,41 @@ static ResilienceConfiguration createResilienceConfiguration( Try tryGetDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options ) { + final Option validationError = validateDestinationLookup(destinationName, options); + if( validationError.isDefined() ) { + return Try.failure(validationError.get()); + } return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination); } + Option validateDestinationLookup( final String destinationName, final DestinationOptions options ) + { + final Option validLookup = Option.none(); + if( !Cache.isEnabled() ) { + return validLookup; + } + if( !Cache.preLookupCheckEnabled ) { + return validLookup; + } + if( Cache.isUsingExperimentalFeatures(options) ) { + final String msg = + "Using pre-lookup check together with either fragments, cross-level options, or custom headers might lead to unexpected behaviour. Pre-lookup check is skipped."; + log.warn(msg); + return validLookup; + } + + final DestinationServiceRetrievalStrategy strategy = getRetrievalStrategy(options).getOrElse(CURRENT_TENANT); + final Collection cachedDestinations = getAllDestinationProperties(strategy); + final Predicate pred = p -> p.get(DestinationProperty.NAME).contains(destinationName); + if( cachedDestinations.stream().anyMatch(pred) ) { + return validLookup; + } + + final String msgFormat = "Destination %s was not found among the destinations for %s."; + final String msg = String.format(msgFormat, destinationName, strategy); + return Option.some(new DestinationNotFoundException(destinationName, msg)); + } + Destination loadAndParseDestination( final String destName, final DestinationOptions options ) throws DestinationAccessException, DestinationNotFoundException @@ -193,7 +229,7 @@ public Try> tryGetAllDestinations() @Nonnull public Collection getAllDestinationProperties() { - return getAllDestinationProperties(DestinationServiceRetrievalStrategy.CURRENT_TENANT); + return getAllDestinationProperties(CURRENT_TENANT); } /** @@ -428,6 +464,7 @@ public static final class Cache private static boolean cacheEnabled = true; private static boolean changeDetectionEnabled = true; + private static boolean preLookupCheckEnabled = true; static { recreateSingleCache(); @@ -445,6 +482,18 @@ static boolean isChangeDetectionEnabled() return changeDetectionEnabled; } + /** + * 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}. + * + * @since 5.26.0 + */ + public static void disablePreLookupCheck() + { + preLookupCheckEnabled = false; + } + @Nonnull static com.github.benmanes.caffeine.cache.Cache instanceSingle() { @@ -498,6 +547,7 @@ static void reset() cacheEnabled = true; } changeDetectionEnabled = true; + preLookupCheckEnabled = true; sizeLimit = Option.some(DEFAULT_SIZE_LIMIT); expirationDuration = Option.some(DEFAULT_EXPIRATION_DURATION); @@ -896,11 +946,15 @@ private static Try> getOrComputeAllDestinations( .execute(); } - private static boolean isUsingExperimentalFeatures( @Nonnull final DestinationOptions options ) + static boolean isUsingExperimentalFeatures( @Nonnull final DestinationOptions options ) { - final String[] featureNames = { "X-fragment-name", "crossLevelSetting", "customHeader" }; + final String[] featureNames = + { + DestinationServiceOptionsAugmenter.X_FRAGMENT_KEY, + DestinationServiceOptionsAugmenter.CROSS_LEVEL_SETTING_KEY, + DestinationServiceOptionsAugmenter.CUSTOM_HEADER_KEY }; final Set keys = options.getOptionKeys(); - return keys.stream().anyMatch(s -> Arrays.stream(featureNames).anyMatch(s::equalsIgnoreCase)); + return keys.stream().anyMatch(s -> Arrays.stream(featureNames).anyMatch(s::startsWith)); } private Cache() 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..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 @@ -290,7 +292,7 @@ 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)); verifyNoMoreInteractions(mockAdapter); } @@ -382,7 +384,7 @@ 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)); verifyNoMoreInteractions(mockAdapter); } diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceCacheTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceCacheTest.java index e4b031ec1..92cf2d4ed 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceCacheTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServiceCacheTest.java @@ -1,11 +1,20 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.augmenter; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.ALWAYS_PROVIDER; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.CURRENT_TENANT; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.ONLY_SUBSCRIBER; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceTokenExchangeStrategy.EXCHANGE_ONLY; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceTokenExchangeStrategy.FORWARD_USER_TOKEN; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceTokenExchangeStrategy.LOOKUP_ONLY; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import java.time.Duration; +import java.util.Arrays; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.parallel.Isolated; @@ -14,6 +23,7 @@ import com.sap.cloud.sdk.cloudplatform.resilience.CacheExpirationStrategy; import io.vavr.control.Option; +import lombok.val; @Isolated( "Test interacts with global destination cache" ) class DestinationServiceCacheTest @@ -102,4 +112,38 @@ void testSize() assertThat(cacheSingle).isNotSameAs(DestinationService.Cache.instanceSingle()); assertThat(cacheAll).isSameAs(DestinationService.Cache.instanceAll()); } + + @SuppressWarnings( "deprecation" ) + @Test + @DisplayName( "Identification of experimental DestinationOptionsAugmenter settings in options" ) + void testExperimentalSettingsIdentificationInOptions() + { + val experimentalAugmentersSet = + new DestinationServiceOptionsAugmenter[][] { + { augmenter().customHeaders(new Header("experimental-header", "value")) }, + { augmenter().crossLevelConsumption(DestinationServiceOptionsAugmenter.CrossLevelScope.SUBACCOUNT) }, + { augmenter().crossLevelConsumption(DestinationServiceOptionsAugmenter.CrossLevelScope.INSTANCE) }, + { augmenter().fragmentName("a-fragment") } }; + + val legacyAugmentersSet = + new DestinationServiceOptionsAugmenter[][] { + {}, + { augmenter().refreshToken("a-token") }, + { augmenter().retrievalStrategy(ALWAYS_PROVIDER) }, + { augmenter().tokenExchangeStrategy(FORWARD_USER_TOKEN) }, + { augmenter().tokenExchangeStrategy(EXCHANGE_ONLY), augmenter().retrievalStrategy(CURRENT_TENANT) }, + { augmenter().tokenExchangeStrategy(LOOKUP_ONLY), augmenter().retrievalStrategy(ONLY_SUBSCRIBER) } }; + + for( final DestinationServiceOptionsAugmenter[] legacyAugmenters : legacyAugmentersSet ) { + val b = DestinationOptions.builder(); + Arrays.stream(legacyAugmenters).forEach(b::augmentBuilder); + assertThat(DestinationService.Cache.isUsingExperimentalFeatures(b.build())).isFalse(); + } + + for( final DestinationServiceOptionsAugmenter[] experimentalAugmenters : experimentalAugmentersSet ) { + val b = DestinationOptions.builder(); + Arrays.stream(experimentalAugmenters).forEach(b::augmentBuilder); + assertThat(DestinationService.Cache.isUsingExperimentalFeatures(b.build())).isTrue(); + } + } } 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..64fa1d847 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,9 @@ 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 55a418c9a..660149345 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 @@ -5,6 +5,7 @@ import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.DESTINATION_RETRIEVAL_STRATEGY_KEY; import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.DESTINATION_TOKEN_EXCHANGE_STRATEGY_KEY; import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.augmenter; +import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.CrossLevelScope.PROVIDER_SUBACCOUNT; import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.ALWAYS_PROVIDER; import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.CURRENT_TENANT; import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.ONLY_SUBSCRIBER; @@ -49,6 +50,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; +import java.util.stream.Stream; import org.apache.http.HttpVersion; import org.apache.http.client.HttpClient; @@ -62,6 +64,9 @@ import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.parallel.Isolated; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.stubbing.Answer; import com.auth0.jwt.JWT; @@ -84,6 +89,7 @@ import io.github.resilience4j.circuitbreaker.CallNotPermittedException; import io.vavr.control.Try; import lombok.SneakyThrows; +import lombok.val; @Isolated( "Test interacts with global destination cache" ) class DestinationServiceTest @@ -113,6 +119,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" }] """; @@ -336,6 +351,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); @@ -442,7 +460,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 +496,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 +512,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 +607,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" ) @@ -1785,10 +1803,7 @@ void testFragmentDestinationsAreCacheIsolated() .getConfigurationAsJson(any(), argThat(it -> it.fragment() == null)); final Function optsBuilder = - frag -> DestinationOptions - .builder() - .augmentBuilder(DestinationServiceOptionsAugmenter.augmenter().fragmentName(frag)) - .build(); + frag -> DestinationOptions.builder().augmentBuilder(augmenter().fragmentName(frag)).build(); final Destination dA = loader.tryGetDestination("destination", optsBuilder.apply("a-fragment")).get(); final Destination dB = loader.tryGetDestination("destination", optsBuilder.apply("b-fragment")).get(); @@ -1918,4 +1933,188 @@ private String createHttpDestinationServiceResponse( final String name, final St } """, name, url); } + + @Test + void testPrependGetAllDestinationsCall() + { + // Reset Cache to re-enable the PreLookupCheck + DestinationService.Cache.reset(); + + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(responseSubaccountDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + + loader.tryGetDestination(destinationName).get(); + + 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); + } + + @Test + void testPrependGetAllDestinationsCallWithMissingDestination() + { + // Reset Cache to re-enable the PreLookupCheck + DestinationService.Cache.reset(); + + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(responseSubaccountDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + + assertThatThrownBy(() -> loader.tryGetDestination("thisDestinationDoesNotExist").get()) + .isInstanceOf(DestinationNotFoundException.class) + .hasMessageContaining("was not found among the destinations for CurrentTenant."); + + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verifyNoMoreInteractions(destinationServiceAdapter); + } + + @Test + void testPrependGetAllDestinationsCallUsesCorrectRetrievalStrategy() + { + // Reset Cache to re-enable the PreLookupCheck + DestinationService.Cache.reset(); + + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + doReturn(responseServiceInstanceDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + // destination with name destinationName is provider-only + doReturn(responseSubaccountDestination) + .when(destinationServiceAdapter) + .getConfigurationAsJson( + eq("/v1/subaccountDestinations"), + argThat(s -> s.behalf() == TECHNICAL_USER_PROVIDER)); + + // set current tenant to be the subscriber tenant + context.setTenant(providerTenant); + + final DestinationOptions options = + DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); + + final Destination result = loader.tryGetDestination(destinationName, options).get(); + assertThat(result.asHttp().getUri()).isEqualTo(URI.create(providerUrl)); + } + + @ParameterizedTest + @MethodSource( "testCasesUncachedDestinationLookup" ) + void testPrependGetAllDestinationsCallSkipped( final DestinationOptions options, final String expectedPath ) + { + // Reset Cache to re-enable the PreLookupCheck + DestinationService.Cache.reset(); + + // additional mock for cross level test case + final String responseWithCrossLevel = """ + { + "destinationConfiguration": { + "Name": "%s", + "Type": "HTTP", + "URL": "https://foo.com", + "Description": "%s level destination" + } + } + """; + doReturn(responseWithCrossLevel.formatted(destinationName, "subaccount")) + .when(destinationServiceAdapter) + .getConfigurationAsJson(eq(expectedPath), any()); + + // call single destination with options + loader.tryGetDestination(destinationName, options).get(); + + // verify that there was no call to all-destination endpoints + verify(destinationServiceAdapter, times(0)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + verify(destinationServiceAdapter, times(0)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq(expectedPath), any()); + verifyNoMoreInteractions(destinationServiceAdapter); + } + + private static Stream testCasesUncachedDestinationLookup() + { + // custom header + final Header h1 = new Header("X-Custom-Header-1", "value-1"); + final DestinationOptions optionsWithHeader = + DestinationOptions.builder().augmentBuilder(augmenter().customHeaders(h1)).build(); + + // cross-level consumption + final DestinationServiceOptionsAugmenter.CrossLevelScope crossLevelScope = + DestinationServiceOptionsAugmenter.CrossLevelScope.SUBACCOUNT; + final DestinationOptions optionsWithSubaccount = + DestinationOptions.builder().augmentBuilder(augmenter().crossLevelConsumption(crossLevelScope)).build(); + + // fragments + final DestinationOptions optionsWithFragment = + DestinationOptions.builder().augmentBuilder(augmenter().fragmentName("a-fragment")).build(); + + return Stream + .of( + Arguments.of(optionsWithHeader, "/v1/destinations/" + destinationName), + Arguments.of(optionsWithSubaccount, "/v2/destinations/" + destinationName + "@subaccount"), + Arguments.of(optionsWithFragment, "/v1/destinations/" + destinationName)); + } + + @Test + void testValidateDestinationLookup() + { + val OPTIONS_EMPTY = DestinationOptions.builder().build(); + val OPTIONS_EXPER = + DestinationOptions.builder().augmentBuilder(augmenter().crossLevelConsumption(PROVIDER_SUBACCOUNT)).build(); + val OPTIONS_CURRT = + DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(CURRENT_TENANT)).build(); + val OPTIONS_PROVT = + DestinationOptions.builder().augmentBuilder(augmenter().retrievalStrategy(ALWAYS_PROVIDER)).build(); + + val adapter = mock(DestinationServiceAdapter.class); + val sut = spy(new DestinationService(adapter)); + + val curr = List.of(DefaultHttpDestination.builder("http://current-tenant-1").name("current-dest-1").build()); + doReturn(curr).when(sut).getAllDestinationProperties(CURRENT_TENANT); + val prov = List.of(DefaultHttpDestination.builder("http://provider-tenant-1").name("provider-dest-1").build()); + doReturn(prov).when(sut).getAllDestinationProperties(ALWAYS_PROVIDER); + + // valid case: disabled cache + DestinationService.Cache.disable(); + assertThat(sut.validateDestinationLookup("unknown-dest-1", OPTIONS_EMPTY)).isEmpty(); + + // valid case: disabled pre-lookup check + DestinationService.Cache.reset(); + DestinationService.Cache.disablePreLookupCheck(); + assertThat(sut.validateDestinationLookup("unknown-dest-1", OPTIONS_EMPTY)).isEmpty(); + + // valid case: experimental properties + DestinationService.Cache.reset(); + assertThat(sut.validateDestinationLookup("unknown-dest-1", OPTIONS_EXPER)).isEmpty(); + + // valid case: current tenant + DestinationService.Cache.reset(); + assertThat(sut.validateDestinationLookup("current-dest-1", OPTIONS_CURRT)).isEmpty(); + + // valid case: provider tenant + DestinationService.Cache.reset(); + assertThat(sut.validateDestinationLookup("provider-dest-1", OPTIONS_PROVT)).isEmpty(); + + // invalid case: current tenant + DestinationService.Cache.reset(); + assertThat(sut.validateDestinationLookup("unknown-dest-1", OPTIONS_PROVT)) + .allSatisfy( + e -> assertThat(e) + .hasMessage("Destination unknown-dest-1 was not found among the destinations for AlwaysProvider.")); + + // invalid case: provider tenant + DestinationService.Cache.reset(); + assertThat(sut.validateDestinationLookup("unknown-dest-1", OPTIONS_CURRT)) + .allSatisfy( + e -> assertThat(e) + .hasMessage("Destination unknown-dest-1 was not found among the destinations for CurrentTenant.")); + } } diff --git a/release_notes.md b/release_notes.md index c3ce28128..f865d3f10 100644 --- a/release_notes.md +++ b/release_notes.md @@ -14,6 +14,8 @@ ### ✨ New Functionality +- `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`. - Temporary: Use `email` as fallback principal id when `user_uuid` is missing. Will switch to using `sub` once IAS exposes `idtype` (tracked in [SCICAI-1323](https://jira.tools.sap/browse/SCICAI-1323)). ### 📈 Improvements