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..15dab98c7 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 @@ -2,10 +2,12 @@ import java.time.Duration; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiFunction; import java.util.function.Function; @@ -847,6 +849,11 @@ private static Try getOrComputeDestination( @Nullable final GetOrComputeAllDestinationsCommand getAllCommand; if( changeDetectionEnabled ) { + if( isUsingExperimentalFeatures(options) ) { + final String msg = + "Using change detection together with either fragments, cross-level options, or custom headers is discouraged and might lead to unexpected caching behaviour."; + log.warn(msg); + } getAllCommand = GetOrComputeAllDestinationsCommand .prepareCommand( @@ -878,11 +885,24 @@ private static Try> getOrComputeAllDestinations( return destinationDownloader.apply(options); } + if( isUsingExperimentalFeatures(options) ) { + final String msg = + "Using caching together with either fragments, cross-level options, or custom headers is discouraged and might lead to unexpected caching behaviour."; + log.warn(msg); + } + return GetOrComputeAllDestinationsCommand .prepareCommand(options, instanceAll(), isolationLocks(), destinationDownloader) .execute(); } + private static boolean isUsingExperimentalFeatures( @Nonnull final DestinationOptions options ) + { + final String[] featureNames = { "X-fragment-name", "crossLevelSetting", "customHeader" }; + final Set keys = options.getOptionKeys(); + return keys.stream().anyMatch(s -> Arrays.stream(featureNames).anyMatch(s::equalsIgnoreCase)); + } + private Cache() { throw new IllegalStateException("This static class must never be instantiated."); diff --git a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommand.java b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommand.java index ac9c217d2..bef1257ad 100644 --- a/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommand.java +++ b/cloudplatform/connectivity-destination-service/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommand.java @@ -41,7 +41,7 @@ static GetOrComputeAllDestinationsCommand prepareCommand( final CacheKey cacheKey = CacheKey.ofTenantOptionalIsolation(); - cacheKey.append(destinationOptions); + cacheKey.append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(destinationOptions)); final ReentrantLock isolationLock = Objects.requireNonNull(isolationLocks.get(cacheKey, any -> new ReentrantLock())); 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..55a418c9a 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 @@ -1255,9 +1255,13 @@ void testConcurrentFetchSameDestinationButDifferentTenant() assertThat(DestinationService.Cache.isolationLocks().asMap()) .containsOnlyKeys( CacheKey.fromIds("TenantA", null).append(destinationName, options), - CacheKey.fromIds("TenantA", null).append(options), + CacheKey + .fromIds("TenantA", null) + .append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(options)), CacheKey.fromIds("TenantB", null).append(destinationName, options), - CacheKey.fromIds("TenantB", null).append(options)); + CacheKey + .fromIds("TenantB", null) + .append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(options))); // assert cache entries for one get-single command for each tenant assertThat(DestinationService.Cache.instanceSingle().asMap()) @@ -1327,7 +1331,9 @@ void testPrincipalIsolationForDestinationWithUserPropagationWithExchangeOnlyStra .containsOnlyKeys( CacheKey.of(subscriberTenant, principal1).append(destinationName, options), CacheKey.of(subscriberTenant, principal2).append(destinationName, options), - CacheKey.of(subscriberTenant, null).append(options)); + CacheKey + .of(subscriberTenant, null) + .append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(options))); assertThat(DestinationService.Cache.instanceSingle().asMap()) .containsOnlyKeys( @@ -1371,7 +1377,9 @@ void testPrincipalIsolationForDestinationWithUserPropagationWithDefaultExchangeS assertThat(DestinationService.Cache.isolationLocks().asMap()) .containsOnlyKeys( CacheKey.of(subscriberTenant, null).append(destinationName, options), - CacheKey.of(subscriberTenant, null).append(options)); + CacheKey + .of(subscriberTenant, null) + .append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(options))); assertThat(DestinationService.Cache.instanceSingle().asMap()) .containsOnlyKeys( diff --git a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommandTest.java b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommandTest.java index afbd3cc98..a66528fb0 100644 --- a/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommandTest.java +++ b/cloudplatform/connectivity-destination-service/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/GetOrComputeAllDestinationsCommandTest.java @@ -90,7 +90,12 @@ void testCommandIsIdempotent() assertThat(result.get()).containsExactly(destination); assertThat(allDestinationsCache.estimatedSize()).isEqualTo(1); - assertThat(allDestinationsCache.getIfPresent(CacheKey.ofNoIsolation().append(EMPTY_OPTIONS))) + assertThat( + allDestinationsCache + .getIfPresent( + CacheKey + .ofNoIsolation() + .append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(EMPTY_OPTIONS)))) .containsExactly(destination); verify(tryGetAllDestinations, times(1)).get(); } @@ -104,8 +109,10 @@ void testIsolationAndAtomicityPerTenant() final CountDownLatch mainThreadLatch = new CountDownLatch(1); final CountDownLatch getAllLatch = new CountDownLatch(1); final AtomicInteger lockInvocations = new AtomicInteger(); - final CacheKey t1Key = CacheKey.of(t1, null).append(EMPTY_OPTIONS); - final CacheKey t2Key = CacheKey.of(t2, null).append(EMPTY_OPTIONS); + final CacheKey t1Key = + CacheKey.of(t1, null).append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(EMPTY_OPTIONS)); + final CacheKey t2Key = + CacheKey.of(t2, null).append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(EMPTY_OPTIONS)); final ReentrantLock tenantIsolationLock = spy(ReentrantLock.class); doAnswer(invocation -> {