Skip to content

Commit 84b4e9e

Browse files
rpanackalrpanackal
authored
Merge branch 'main' into feat/openapi/optional-spring-base
2 parents 1c0bef8 + 9fdb944 commit 84b4e9e

11 files changed

Lines changed: 410 additions & 24 deletions

File tree

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

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
package com.sap.cloud.sdk.cloudplatform.connectivity;
22

3+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.getRetrievalStrategy;
4+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.CURRENT_TENANT;
5+
36
import java.time.Duration;
47
import java.util.ArrayList;
8+
import java.util.Arrays;
59
import java.util.Collection;
610
import java.util.LinkedHashMap;
711
import java.util.List;
812
import java.util.Map;
13+
import java.util.Set;
914
import java.util.concurrent.locks.ReentrantLock;
1015
import java.util.function.BiFunction;
1116
import java.util.function.Function;
17+
import java.util.function.Predicate;
1218
import java.util.function.Supplier;
1319
import java.util.stream.Collectors;
1420

@@ -120,9 +126,41 @@ static ResilienceConfiguration createResilienceConfiguration(
120126
Try<Destination>
121127
tryGetDestination( @Nonnull final String destinationName, @Nonnull final DestinationOptions options )
122128
{
129+
final Option<Exception> validationError = validateDestinationLookup(destinationName, options);
130+
if( validationError.isDefined() ) {
131+
return Try.failure(validationError.get());
132+
}
123133
return Cache.getOrComputeDestination(this, destinationName, options, this::loadAndParseDestination);
124134
}
125135

136+
Option<Exception> validateDestinationLookup( final String destinationName, final DestinationOptions options )
137+
{
138+
final Option<Exception> validLookup = Option.none();
139+
if( !Cache.isEnabled() ) {
140+
return validLookup;
141+
}
142+
if( !Cache.preLookupCheckEnabled ) {
143+
return validLookup;
144+
}
145+
if( Cache.isUsingExperimentalFeatures(options) ) {
146+
final String msg =
147+
"Using pre-lookup check together with either fragments, cross-level options, or custom headers might lead to unexpected behaviour. Pre-lookup check is skipped.";
148+
log.warn(msg);
149+
return validLookup;
150+
}
151+
152+
final DestinationServiceRetrievalStrategy strategy = getRetrievalStrategy(options).getOrElse(CURRENT_TENANT);
153+
final Collection<DestinationProperties> cachedDestinations = getAllDestinationProperties(strategy);
154+
final Predicate<DestinationProperties> pred = p -> p.get(DestinationProperty.NAME).contains(destinationName);
155+
if( cachedDestinations.stream().anyMatch(pred) ) {
156+
return validLookup;
157+
}
158+
159+
final String msgFormat = "Destination %s was not found among the destinations for %s.";
160+
final String msg = String.format(msgFormat, destinationName, strategy);
161+
return Option.some(new DestinationNotFoundException(destinationName, msg));
162+
}
163+
126164
Destination loadAndParseDestination( final String destName, final DestinationOptions options )
127165
throws DestinationAccessException,
128166
DestinationNotFoundException
@@ -191,7 +229,7 @@ public Try<Iterable<Destination>> tryGetAllDestinations()
191229
@Nonnull
192230
public Collection<DestinationProperties> getAllDestinationProperties()
193231
{
194-
return getAllDestinationProperties(DestinationServiceRetrievalStrategy.CURRENT_TENANT);
232+
return getAllDestinationProperties(CURRENT_TENANT);
195233
}
196234

197235
/**
@@ -426,6 +464,7 @@ public static final class Cache
426464

427465
private static boolean cacheEnabled = true;
428466
private static boolean changeDetectionEnabled = true;
467+
private static boolean preLookupCheckEnabled = true;
429468

430469
static {
431470
recreateSingleCache();
@@ -443,6 +482,18 @@ static boolean isChangeDetectionEnabled()
443482
return changeDetectionEnabled;
444483
}
445484

485+
/**
486+
* Disables checking if a destination exists before trying to call it directly when invoking
487+
* {@link #tryGetDestination}. All available destinations can be found with
488+
* {@link #getAllDestinationProperties}.
489+
*
490+
* @since 5.26.0
491+
*/
492+
public static void disablePreLookupCheck()
493+
{
494+
preLookupCheckEnabled = false;
495+
}
496+
446497
@Nonnull
447498
static com.github.benmanes.caffeine.cache.Cache<CacheKey, Destination> instanceSingle()
448499
{
@@ -496,6 +547,7 @@ static void reset()
496547
cacheEnabled = true;
497548
}
498549
changeDetectionEnabled = true;
550+
preLookupCheckEnabled = true;
499551

500552
sizeLimit = Option.some(DEFAULT_SIZE_LIMIT);
501553
expirationDuration = Option.some(DEFAULT_EXPIRATION_DURATION);
@@ -847,6 +899,11 @@ private static Try<Destination> getOrComputeDestination(
847899
@Nullable
848900
final GetOrComputeAllDestinationsCommand getAllCommand;
849901
if( changeDetectionEnabled ) {
902+
if( isUsingExperimentalFeatures(options) ) {
903+
final String msg =
904+
"Using change detection together with either fragments, cross-level options, or custom headers is discouraged and might lead to unexpected caching behaviour.";
905+
log.warn(msg);
906+
}
850907
getAllCommand =
851908
GetOrComputeAllDestinationsCommand
852909
.prepareCommand(
@@ -878,11 +935,28 @@ private static Try<List<DestinationProperties>> getOrComputeAllDestinations(
878935
return destinationDownloader.apply(options);
879936
}
880937

938+
if( isUsingExperimentalFeatures(options) ) {
939+
final String msg =
940+
"Using caching together with either fragments, cross-level options, or custom headers is discouraged and might lead to unexpected caching behaviour.";
941+
log.warn(msg);
942+
}
943+
881944
return GetOrComputeAllDestinationsCommand
882945
.prepareCommand(options, instanceAll(), isolationLocks(), destinationDownloader)
883946
.execute();
884947
}
885948

949+
static boolean isUsingExperimentalFeatures( @Nonnull final DestinationOptions options )
950+
{
951+
final String[] featureNames =
952+
{
953+
DestinationServiceOptionsAugmenter.X_FRAGMENT_KEY,
954+
DestinationServiceOptionsAugmenter.CROSS_LEVEL_SETTING_KEY,
955+
DestinationServiceOptionsAugmenter.CUSTOM_HEADER_KEY };
956+
final Set<String> keys = options.getOptionKeys();
957+
return keys.stream().anyMatch(s -> Arrays.stream(featureNames).anyMatch(s::startsWith));
958+
}
959+
886960
private Cache()
887961
{
888962
throw new IllegalStateException("This static class must never be instantiated.");

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ static GetOrComputeAllDestinationsCommand prepareCommand(
4141

4242
final CacheKey cacheKey = CacheKey.ofTenantOptionalIsolation();
4343

44-
cacheKey.append(destinationOptions);
44+
cacheKey.append(DestinationServiceOptionsAugmenter.getRetrievalStrategy(destinationOptions));
4545

4646
final ReentrantLock isolationLock =
4747
Objects.requireNonNull(isolationLocks.get(cacheKey, any -> new ReentrantLock()));

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ void setMockAdapter()
6666
.when(mockAdapter)
6767
.getConfigurationAsJson(anyString(), any());
6868
sut = new DestinationService(mockAdapter);
69+
// Disable PreLookupCheck to simplify test setup
70+
DestinationService.Cache.disablePreLookupCheck();
6971
}
7072

7173
@Test
@@ -290,7 +292,7 @@ void testOAuth2PasswordWithoutUserToken()
290292
assertThat(dest.asHttp().getHeaders())
291293
.containsExactlyInAnyOrder(new Header("Authorization", "Bearer " + oAuthToken));
292294

293-
verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy));
295+
verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy));
294296
verifyNoMoreInteractions(mockAdapter);
295297
}
296298

@@ -382,7 +384,7 @@ void testSAPAssertionSSO()
382384
assertThat(dest.asHttp().getAuthenticationType()).isEqualTo(AuthenticationType.SAP_ASSERTION_SSO);
383385
assertThat(dest.asHttp().getHeaders()).containsExactlyInAnyOrder(new Header("Cookie", assertionCookie));
384386

385-
verify(mockAdapter, times(1)).getConfigurationAsJson(anyString(), eq(expectedStrategy));
387+
verify(mockAdapter, times(1)).getConfigurationAsJson(eq(SERVICE_PATH_DESTINATION), eq(expectedStrategy));
386388
verifyNoMoreInteractions(mockAdapter);
387389
}
388390

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,20 @@
11
package com.sap.cloud.sdk.cloudplatform.connectivity;
22

3+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceOptionsAugmenter.augmenter;
4+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.ALWAYS_PROVIDER;
5+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.CURRENT_TENANT;
6+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceRetrievalStrategy.ONLY_SUBSCRIBER;
7+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceTokenExchangeStrategy.EXCHANGE_ONLY;
8+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceTokenExchangeStrategy.FORWARD_USER_TOKEN;
9+
import static com.sap.cloud.sdk.cloudplatform.connectivity.DestinationServiceTokenExchangeStrategy.LOOKUP_ONLY;
310
import static org.assertj.core.api.Assertions.assertThat;
411
import static org.assertj.core.api.Assertions.assertThatCode;
512

613
import java.time.Duration;
14+
import java.util.Arrays;
715

816
import org.junit.jupiter.api.AfterEach;
17+
import org.junit.jupiter.api.DisplayName;
918
import org.junit.jupiter.api.Test;
1019
import org.junit.jupiter.api.parallel.Isolated;
1120

@@ -14,6 +23,7 @@
1423
import com.sap.cloud.sdk.cloudplatform.resilience.CacheExpirationStrategy;
1524

1625
import io.vavr.control.Option;
26+
import lombok.val;
1727

1828
@Isolated( "Test interacts with global destination cache" )
1929
class DestinationServiceCacheTest
@@ -102,4 +112,38 @@ void testSize()
102112
assertThat(cacheSingle).isNotSameAs(DestinationService.Cache.instanceSingle());
103113
assertThat(cacheAll).isSameAs(DestinationService.Cache.instanceAll());
104114
}
115+
116+
@SuppressWarnings( "deprecation" )
117+
@Test
118+
@DisplayName( "Identification of experimental DestinationOptionsAugmenter settings in options" )
119+
void testExperimentalSettingsIdentificationInOptions()
120+
{
121+
val experimentalAugmentersSet =
122+
new DestinationServiceOptionsAugmenter[][] {
123+
{ augmenter().customHeaders(new Header("experimental-header", "value")) },
124+
{ augmenter().crossLevelConsumption(DestinationServiceOptionsAugmenter.CrossLevelScope.SUBACCOUNT) },
125+
{ augmenter().crossLevelConsumption(DestinationServiceOptionsAugmenter.CrossLevelScope.INSTANCE) },
126+
{ augmenter().fragmentName("a-fragment") } };
127+
128+
val legacyAugmentersSet =
129+
new DestinationServiceOptionsAugmenter[][] {
130+
{},
131+
{ augmenter().refreshToken("a-token") },
132+
{ augmenter().retrievalStrategy(ALWAYS_PROVIDER) },
133+
{ augmenter().tokenExchangeStrategy(FORWARD_USER_TOKEN) },
134+
{ augmenter().tokenExchangeStrategy(EXCHANGE_ONLY), augmenter().retrievalStrategy(CURRENT_TENANT) },
135+
{ augmenter().tokenExchangeStrategy(LOOKUP_ONLY), augmenter().retrievalStrategy(ONLY_SUBSCRIBER) } };
136+
137+
for( final DestinationServiceOptionsAugmenter[] legacyAugmenters : legacyAugmentersSet ) {
138+
val b = DestinationOptions.builder();
139+
Arrays.stream(legacyAugmenters).forEach(b::augmentBuilder);
140+
assertThat(DestinationService.Cache.isUsingExperimentalFeatures(b.build())).isFalse();
141+
}
142+
143+
for( final DestinationServiceOptionsAugmenter[] experimentalAugmenters : experimentalAugmentersSet ) {
144+
val b = DestinationOptions.builder();
145+
Arrays.stream(experimentalAugmenters).forEach(b::augmentBuilder);
146+
assertThat(DestinationService.Cache.isUsingExperimentalFeatures(b.build())).isTrue();
147+
}
148+
}
105149
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ void setupConnectivity( @Nonnull final WireMockRuntimeInfo wm )
108108
.withRequestBody(
109109
equalTo("grant_type=client_credentials&client_secret=CLIENT_SECRET&client_id=CLIENT_ID"))
110110
.willReturn(okJson("{\"access_token\":\"provider-client-credentials\",\"expires_in\":3600}")));
111+
112+
// Disable PreLookupCheck to simplify test setup
113+
DestinationService.Cache.disablePreLookupCheck();
111114
}
112115

113116
@AfterEach

0 commit comments

Comments
 (0)