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..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 @@ -120,6 +120,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 DestinationNotFoundException(String.format(msg, destinationName))); + } return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination); } @@ -384,6 +388,13 @@ private static boolean hasCauseAssignableFrom( @Nonnull final Throwable t, @Nonn return ExceptionUtils.getThrowableList(t).stream().map(Throwable::getClass).anyMatch(cls::isAssignableFrom); } + private boolean preLookupCheckSuccessful( final String destinationName ) + { + return getAllDestinationProperties() + .stream() + .anyMatch(properties -> properties.get(DestinationProperty.NAME).contains(destinationName)); + } + /** * Helper class that encapsulates all caching related configuration options. * @@ -426,6 +437,7 @@ public static final class Cache private static boolean cacheEnabled = true; private static boolean changeDetectionEnabled = true; + private static boolean preLookupCheckEnabled = true; static { recreateSingleCache(); @@ -443,6 +455,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.25.0 + */ + public static void disablePreLookupCheck() + { + preLookupCheckEnabled = false; + } + @Nonnull static com.github.benmanes.caffeine.cache.Cache instanceSingle() { @@ -496,6 +520,7 @@ static void reset() cacheEnabled = true; } changeDetectionEnabled = true; + 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 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/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 a23533da1..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 @@ -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" }] """; @@ -336,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); @@ -442,7 +454,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 +490,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 +506,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 +601,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 +1922,49 @@ 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()); + + Destination result = 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 of the current tenant."); + + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/instanceDestinations"), any()); + verify(destinationServiceAdapter, times(1)).getConfigurationAsJson(eq("/v1/subaccountDestinations"), any()); + verifyNoMoreInteractions(destinationServiceAdapter); + + } } diff --git a/release_notes.md b/release_notes.md index b03a236a7..b0c19875b 100644 --- a/release_notes.md +++ b/release_notes.md @@ -12,7 +12,7 @@ ### ✨ 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`. ### 📈 Improvements