Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
202f37e
apply fix
Jonas-Isr Jan 5, 2026
f04cf0b
adapt tests
Jonas-Isr Jan 5, 2026
d0edbfe
add additional lookup
Jonas-Isr Jan 5, 2026
6d66f5c
tests
Jonas-Isr Jan 5, 2026
2f474ba
release notes
Jonas-Isr Jan 5, 2026
342b049
fix DestinationNotFoundException message
Jonas-Isr Jan 5, 2026
1e85272
propagate DestinationOptions
Jonas-Isr Jan 5, 2026
09b6897
correct since tag
Jonas-Isr Jan 6, 2026
af8b3c2
add another unit test
Jonas-Isr Jan 6, 2026
7982777
remove comment
Jonas-Isr Jan 6, 2026
f22e9be
retrigger
Jonas-Isr Jan 7, 2026
bf81047
add warn log
Jonas-Isr Jan 7, 2026
64e4001
Merge branch 'refs/heads/fix-caching-getOrComputeAllDestinations' int…
Jonas-Isr Jan 7, 2026
e47b755
add skipping logic and test
Jonas-Isr Jan 7, 2026
da49da1
Merge branch 'main' into fix-caching-getOrComputeAllDestinations
Jonas-Isr Jan 8, 2026
87dd83d
improve warn messages
Jonas-Isr Jan 8, 2026
85498e1
Merge branch 'refs/heads/fix-caching-getOrComputeAllDestinations' int…
Jonas-Isr Jan 8, 2026
3697192
improve warn message
Jonas-Isr Jan 8, 2026
5473565
Merge remote-tracking branch 'origin/main' into add-preLookup-Check
newtork Jan 9, 2026
a8b7cf2
Merge remote-tracking branch 'origin/main' into add-preLookup-Check
newtork Jan 9, 2026
b969c29
Minor API adjustments
newtork Jan 9, 2026
73f2960
Apply fix
newtork Jan 9, 2026
e3b85b9
Format
newtork Jan 9, 2026
a5c89d0
Minor format
newtork Jan 9, 2026
11d0c66
Minor format
newtork Jan 9, 2026
b444fec
Add low-level test method
newtork Jan 9, 2026
6cab0ac
Format imports
newtork Jan 9, 2026
c492b6f
Fix PMD
newtork Jan 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -122,9 +126,41 @@ static ResilienceConfiguration createResilienceConfiguration(
Try<Destination>
tryGetDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options )
{
final Option<Exception> validationError = validateDestinationLookup(destinationName, options);
if( validationError.isDefined() ) {
return Try.failure(validationError.get());
}
return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination);
}

Option<Exception> validateDestinationLookup( final String destinationName, final DestinationOptions options )
{
final Option<Exception> 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<DestinationProperties> cachedDestinations = getAllDestinationProperties(strategy);
final Predicate<DestinationProperties> 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
Expand Down Expand Up @@ -193,7 +229,7 @@ public Try<Iterable<Destination>> tryGetAllDestinations()
@Nonnull
public Collection<DestinationProperties> getAllDestinationProperties()
{
return getAllDestinationProperties(DestinationServiceRetrievalStrategy.CURRENT_TENANT);
return getAllDestinationProperties(CURRENT_TENANT);
}

/**
Expand Down Expand Up @@ -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();
Expand All @@ -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<CacheKey, Destination> instanceSingle()
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -896,11 +946,15 @@ private static Try<List<DestinationProperties>> 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<String> 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));
Comment on lines +951 to +957
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Comment)

I replaced my incorrectly changed "equalsIgnoreCase" back to your original "startsWith". Unit tests now cover this difference.

Also referenced the options-augmenter directly, in case we'll refine it later.

}

private Cache()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading