Skip to content

Commit

Permalink
feat: [Destinations] Support Fragments (#491)
Browse files Browse the repository at this point in the history
Co-authored-by: KavithaSiva <[email protected]>
Co-authored-by: Alexander Dümont <[email protected]>
  • Loading branch information
3 people authored Jul 29, 2024
1 parent 4b575ff commit e370185
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ final class DestinationRetrievalStrategy
@Nullable
@ToString.Exclude
private final String token;
@Nullable
private String fragment;

static DestinationRetrievalStrategy withoutToken( @Nonnull final OnBehalfOf behalf )
{
Expand All @@ -53,6 +55,19 @@ static DestinationRetrievalStrategy withUserToken( @Nonnull final OnBehalfOf beh
return new DestinationRetrievalStrategy(behalf, REFRESH_TOKEN, token);
}

DestinationRetrievalStrategy withFragmentName( @Nonnull final String fragmentName )
{
if( fragmentName.isBlank() ) {
throw new IllegalArgumentException("Fragment name must not be empty");
}
// sanity check to enforce this is only ever set once
if( fragment != null ) {
throw new IllegalStateException("Attempted to change an already set fragment name");
}
fragment = fragmentName;
return this;
}

enum TokenForwarding
{
USER_TOKEN,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,18 @@ DestinationRetrieval prepareSupplier( @Nonnull final DestinationOptions options
.getRefreshToken(options)
.peek(any -> log.debug("Refresh token given, applying refresh token flow."))
.getOrNull();
final String fragmentName =
DestinationServiceOptionsAugmenter
.getFragmentName(options)
.peek(it -> log.debug("Found fragment name '{}'.", it))
.getOrNull();

log
.debug(
"Loading destination from reuse-destination-service with retrieval strategy {} and token exchange strategy {}.",
retrievalStrategy,
tokenExchangeStrategy);
return prepareSupplier(retrievalStrategy, tokenExchangeStrategy, refreshToken);
return prepareSupplier(retrievalStrategy, tokenExchangeStrategy, refreshToken, fragmentName);
}

/**
Expand Down Expand Up @@ -159,7 +164,8 @@ private DestinationServiceTokenExchangeStrategy getDefaultTokenExchangeStrategy(
DestinationRetrieval prepareSupplier(
@Nonnull final DestinationServiceRetrievalStrategy retrievalStrategy,
@Nonnull final DestinationServiceTokenExchangeStrategy tokenExchangeStrategy,
@Nullable final String refreshToken )
@Nullable final String refreshToken,
@Nullable final String fragmentName )
throws DestinationAccessException
{
log
Expand All @@ -175,6 +181,9 @@ DestinationRetrieval prepareSupplier(
retrievalStrategy,
DestinationServiceTokenExchangeStrategy.LOOKUP_ONLY,
refreshToken);
if( fragmentName != null ) {
strategy.withFragmentName(fragmentName);
}
return new DestinationRetrieval(() -> {
final DestinationServiceV1Response result = destinationRetriever.apply(strategy);
if( !doesDestinationConfigurationRequireUserTokenExchange(result) ) {
Expand All @@ -190,6 +199,9 @@ DestinationRetrieval prepareSupplier(

final DestinationRetrievalStrategy strategy =
resolveSingleRequestStrategy(retrievalStrategy, tokenExchangeStrategy, refreshToken);
if( fragmentName != null ) {
strategy.withFragmentName(fragmentName);
}
return new DestinationRetrieval(() -> destinationRetriever.apply(strategy), strategy.behalf());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ private HttpUriRequest prepareRequest( final String servicePath, final Destinati
if( headerName != null ) {
request.addHeader(headerName, strategy.token());
}
if( strategy.fragment() != null ) {
request.addHeader("x-fragment-name", strategy.fragment());
}
return request;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class DestinationServiceOptionsAugmenter implements DestinationOptionsAug
static final String DESTINATION_RETRIEVAL_STRATEGY_KEY = "scp.cf.destinationRetrievalStrategy";
static final String DESTINATION_TOKEN_EXCHANGE_STRATEGY_KEY = "scp.cf.destinationTokenExchangeStrategy";
static final String X_REFRESH_TOKEN_KEY = "x-refresh-token";
static final String X_FRAGMENT_KEY = "X-fragment-name";

private final Map<String, Object> parameters = new HashMap<>();

Expand Down Expand Up @@ -86,6 +87,31 @@ public DestinationServiceOptionsAugmenter refreshToken( @Nonnull final String re
return this;
}

/**
* Fragment that should enhance the destination to be fetched.
*
* @param fragmentName
* The fragment name.
* @return The same augmenter that called this method.
* @since 5.11.0
*/
@Beta
@Nonnull
public DestinationServiceOptionsAugmenter fragmentName( @Nonnull final String fragmentName )
{
parameters.put(X_FRAGMENT_KEY, fragmentName);
if( DestinationService.Cache.isEnabled() && DestinationService.Cache.isChangeDetectionEnabled() ) {
log
.warn(
"""
A fragment was requested while change detection caching is enabled.\
This is not recommended, as fragment-based destinations will effectively not be cached with this strategy.\
Consider disabling change detection, if you frequently use destination fragments.
""");
}
return this;
}

@Override
public void augmentBuilder( @Nonnull final DestinationOptions.Builder builder )
{
Expand Down Expand Up @@ -145,4 +171,10 @@ static Option<String> getRefreshToken( @Nonnull final DestinationOptions options
{
return options.get(X_REFRESH_TOKEN_KEY).filter(String.class::isInstance).map(String.class::cast);
}

@Nonnull
static Option<String> getFragmentName( @Nonnull final DestinationOptions options )
{
return options.get(X_FRAGMENT_KEY).filter(String.class::isInstance).map(String.class::cast);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ void testExceptionsAreThrownOnIllegalCombinations()
.executeWithTenant(
c._3(),
() -> softly
.assertThatThrownBy(() -> sut.prepareSupplier(c._1(), c._2(), null))
.assertThatThrownBy(() -> sut.prepareSupplier(c._1(), c._2(), null, null))
.as("Expecting '%s' with '%s' and '%s' to throw.", c._1(), c._2(), c._3())
.isInstanceOf(DestinationAccessException.class)));

Expand All @@ -234,7 +234,12 @@ void testExceptionsAreThrownForImpossibleTokenExchanges()
{
doAnswer(( any ) -> true).when(sut).doesDestinationConfigurationRequireUserTokenExchange(any());
final DestinationRetrieval supplier =
sut.prepareSupplier(ALWAYS_PROVIDER, DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE, null);
sut
.prepareSupplier(
ALWAYS_PROVIDER,
DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE,
null,
null);

TenantAccessor
.executeWithTenant(
Expand All @@ -259,7 +264,7 @@ void testDefaultStrategies()
sut.prepareSupplier(DestinationOptions.builder().build());
sut.prepareSupplierAllDestinations(DestinationOptions.builder().build());

verify(sut).prepareSupplier(CURRENT_TENANT, FORWARD_USER_TOKEN, null);
verify(sut).prepareSupplier(CURRENT_TENANT, FORWARD_USER_TOKEN, null, null);
verify(sut).prepareSupplierAllDestinations(CURRENT_TENANT);
}

Expand All @@ -272,7 +277,8 @@ void testDefaultNonXsuaaTokenStrategy()
sut.prepareSupplier(DestinationOptions.builder().build());
sut.prepareSupplierAllDestinations(DestinationOptions.builder().build());

verify(sut).prepareSupplier(CURRENT_TENANT, DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE, null);
verify(sut)
.prepareSupplier(CURRENT_TENANT, DestinationServiceTokenExchangeStrategy.LOOKUP_THEN_EXCHANGE, null, null);
verify(sut).prepareSupplierAllDestinations(CURRENT_TENANT);
}

Expand All @@ -290,4 +296,19 @@ void testRefreshToken()

verify(sut).resolveSingleRequestStrategy(eq(CURRENT_TENANT), eq(LOOKUP_ONLY), eq(refreshToken));
}

@Test
void testFragmentName()
{
final String fragmentName = "my-fragment";
final DestinationOptions opts =
DestinationOptions
.builder()
.augmentBuilder(DestinationServiceOptionsAugmenter.augmenter().fragmentName(fragmentName))
.build();

sut.prepareSupplier(opts);

verify(sut).prepareSupplier(any(), any(), eq(null), eq(fragmentName));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,31 @@ void testRefreshTokenFlow()
.withoutHeader("x-user-token"));
}

@Test
void testFragmentName()
{
final DestinationServiceAdapter adapterToTest = createSut(DEFAULT_SERVICE_BINDING);

final String fragment = "my-fragment";

final String destinationResponse =
adapterToTest
.getConfigurationAsJson(
"/",
DestinationRetrievalStrategy
.withoutToken(TECHNICAL_USER_CURRENT_TENANT)
.withFragmentName(fragment));

assertThat(destinationResponse).isEqualTo(DESTINATION_RESPONSE);

verify(
1,
getRequestedFor(urlEqualTo(DESTINATION_SERVICE_URL))
.withHeader("Authorization", equalTo("Bearer " + xsuaaToken))
.withHeader("x-fragment-name", equalTo(fragment))
.withoutHeader("x-user-token"));
}

@Test
void getDestinationServiceProviderTenantShouldReturnProviderTenantFromServiceBinding()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Function;

import org.apache.http.HttpVersion;
import org.apache.http.client.HttpClient;
Expand Down Expand Up @@ -1598,6 +1599,60 @@ void testAuthTokenFailureIsNotCached()
withUserToken(TECHNICAL_USER_CURRENT_TENANT, userToken));
}

@Test
void testFragmentDestinationsAreCacheIsolated()
{
DestinationService.Cache.disableChangeDetection();
final String destinationTemplate = """
{
"owner": {
"SubaccountId": "00000000-0000-0000-0000-000000000000",
"InstanceId": null
},
"destinationConfiguration": {
"Name": "destination",
%s
"Type": "HTTP",
"URL": "https://%s.com/",
"Authentication": "NoAuthentication",
"ProxyType": "Internet"
}
}
""";

doReturn(destinationTemplate.formatted("\"FragmentName\": \"a-fragment\",", "a.fragment"))
.when(destinationServiceAdapter)
.getConfigurationAsJson(any(), argThat(it -> "a-fragment".equals(it.fragment())));
doReturn(destinationTemplate.formatted("\"FragmentName\": \"b-fragment\",", "b.fragment"))
.when(destinationServiceAdapter)
.getConfigurationAsJson(any(), argThat(it -> "b-fragment".equals(it.fragment())));
doReturn(destinationTemplate.formatted("", "destination"))
.when(destinationServiceAdapter)
.getConfigurationAsJson(any(), argThat(it -> it.fragment() == null));

final Function<String, DestinationOptions> optsBuilder =
frag -> DestinationOptions
.builder()
.augmentBuilder(DestinationServiceOptionsAugmenter.augmenter().fragmentName(frag))
.build();

final Destination dA = loader.tryGetDestination("destination", optsBuilder.apply("a-fragment")).get();
final Destination dB = loader.tryGetDestination("destination", optsBuilder.apply("b-fragment")).get();
final Destination d = loader.tryGetDestination("destination").get();

assertThat(dA).isNotEqualTo(dB).isNotEqualTo(d);
assertThat(dA.get("FragmentName")).contains("a-fragment");
assertThat(dB).isNotEqualTo(d);
assertThat(dB.get("FragmentName")).contains("b-fragment");

assertThat(d.get("FragmentName")).isEmpty();

assertThat(dA)
.describedAs("Destinations with fragments should be cached")
.isSameAs(loader.tryGetDestination("destination", optsBuilder.apply("a-fragment")).get());
verify(destinationServiceAdapter, times(3)).getConfigurationAsJson(any(), any());
}

// @Test
// Performance test is unreliable on Jenkins
void runLoadTest()
Expand Down
4 changes: 3 additions & 1 deletion release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@

### ✨ New Functionality

-
- Add experimental support for [_Destination Fragments_](https://help.sap.com/docs/connectivity/sap-btp-connectivity-cf/extending-destinations-with-fragments).
Fragment names can be passed upon requesting destinations via `DestinationServiceOptionsAugmenter.fragmentName("my-fragment-name")`.
For further details refer to [the documentation]().

### 📈 Improvements

Expand Down

0 comments on commit e370185

Please sign in to comment.