fix: Improve Circuit breaker behaviour

cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationService.java

@@ -120,6 +120,10 @@ static ResilienceConfiguration createResilienceConfiguration(
         Try<Destination>
         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<CacheKey, Destination> 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);

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);
     }
 

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

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<DestinationProperties> 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<DestinationProperties> 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<Destination> 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);
+
+    }
 }

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