Revert "fix: Improve Circuit breaker behaviour (#1017)" (#1040)

Author: newtork

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

@@ -120,10 +120,6 @@ 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);
     }
 
@@ -388,13 +384,6 @@ 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.
      *
@@ -437,7 +426,6 @@ public static final class Cache
 
         private static boolean cacheEnabled = true;
         private static boolean changeDetectionEnabled = true;
-        private static boolean preLookupCheckEnabled = true;
 
         static {
             recreateSingleCache();
@@ -455,18 +443,6 @@ 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()
         {
@@ -520,7 +496,6 @@ 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,8 +66,6 @@ void setMockAdapter()
             .when(mockAdapter)
             .getConfigurationAsJson(anyString(), any());
         sut = new DestinationService(mockAdapter);
-        // Disable PreLookupCheck to simplify test setup
-        DestinationService.Cache.disablePreLookupCheck();
     }
 
     @Test
@@ -292,7 +290,7 @@ void testOAuth2PasswordWithoutUserToken()
         assertThat(dest.asHttp().getHeaders())
             .containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + oAuthToken));
 
-        verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy));
+        verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy));
         verifyNoMoreInteractions(mockAdapter);
     }
 
@@ -384,7 +382,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(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy));
+        verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy));
         verifyNoMoreInteractions(mockAdapter);
     }
 

cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationServicePrincipalPropagationTest.java

@@ -108,8 +108,6 @@ 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,15 +113,6 @@ 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"
           }]
         """;
 
@@ -345,9 +336,6 @@ 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);
@@ -454,7 +442,7 @@ void testGettingDestinationProperties()
 
         assertThat(destinationList)
             .extracting(d -> d.get(DestinationProperty.NAME).get())
-            .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT", destinationName);
+            .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT");
 
         final DestinationProperties destination =
             destinationList
@@ -490,7 +478,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", destinationName);
+            .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT");
     }
 
     @Test
@@ -506,7 +494,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", destinationName);
+            .containsExactly("CC8-HTTP-BASIC", "CC8-HTTP-CERT1", "CC8-HTTP-CERT");
     }
 
     @Test
@@ -601,10 +589,10 @@ void testGetAllDestinationsForProvider()
         final List<Destination> destinationList = new ArrayList<>();
         destinations.get().forEach(destinationList::add);
 
-        assertThat(destinationList.size()).isEqualTo(4);
+        assertThat(destinationList.size()).isEqualTo(3);
         assertThat(destinationList)
             .extracting(d -> d.get(DestinationProperty.NAME).get())
-            .containsOnly("CC8-HTTP-BASIC", "CC8-HTTP-CERT", "CC8-HTTP-CERT1", destinationName);
+            .containsOnly("CC8-HTTP-BASIC", "CC8-HTTP-CERT", "CC8-HTTP-CERT1");
     }
 
     @SuppressWarnings( "deprecation" )
@@ -1922,49 +1910,4 @@ 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,8 +12,6 @@
 
 ### ✨ 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`.
 - Added native DestinationLoader implementation through the `TransparentProxy` class. This enables seamless destination
   retrieval via `tryGetDestination` and provides full Cloud SDK integration for applications running in Kubernetes
   environments where Transparent Proxy is available.