Skip to content

Commit

Permalink
feat: [DestinationService] Allow optional retrieval-strategy (tenant)…
Browse files Browse the repository at this point in the history
… in getAllDestinationProperties (#564)
  • Loading branch information
newtork authored Aug 29, 2024
1 parent 6bb77bf commit 83a3962
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public Try<Iterable<Destination>> tryGetAllDestinations()
}

/**
* Fetches all destination properties from the BTP Destination Service.
* Fetches all destination properties from the BTP Destination Service on behalf of the current tenant.
* <p>
* <strong>Caution: This will not perform any authorization flows for the destinations.</strong> Destinations
* obtained this way should only be used for accessing the properties of the destination configuration. For
Expand All @@ -184,21 +184,43 @@ public Try<Iterable<Destination>> tryGetAllDestinations()
* destination at service instance level takes precedence.
*
* @return A list of destination properties.
* @see DestinationService#getAllDestinationProperties(DestinationServiceRetrievalStrategy)
* @since 5.1.0
*/
@Nonnull
public Collection<DestinationProperties> getAllDestinationProperties()
{
return getAllDestinationProperties(DestinationServiceRetrievalStrategy.CURRENT_TENANT);
}

/**
* Fetches all destination properties from the BTP Destination Service.
* <p>
* <strong>Caution: This will not perform any authorization flows for the destinations.</strong> Destinations
* obtained this way should only be used for accessing the properties of the destination configuration. For
* obtaining full destination objects, use {@link #tryGetDestination(String, DestinationOptions)} instead.
* </p>
* In case there exists a destination with the same name on service instance and on sub-account level, the
* destination at service instance level takes precedence.
*
* @return A list of destination properties.
* @param retrievalStrategy
* Strategy for loading destinations in a multi-tenant application.
* @since 5.12.0
*/
@Nonnull
public Collection<DestinationProperties> getAllDestinationProperties(
@Nonnull final DestinationServiceRetrievalStrategy retrievalStrategy )
{
final var augmenter = DestinationServiceOptionsAugmenter.augmenter().retrievalStrategy(retrievalStrategy);
final var options = DestinationOptions.builder().augmentBuilder(augmenter).build();
return new ArrayList<>(
Cache
.getOrComputeAllDestinations(
DestinationOptions.builder().build(),
this::getAllDestinationsByRetrievalStrategy)
.get());
Cache.getOrComputeAllDestinations(options, this::getAllDestinationsByRetrievalStrategy).get());
}

/**
* Fetches the properties of a specific destination from the BTP Destination Service.
* Fetches the properties of a specific destination from the BTP Destination Service on behalf of the current
* tenant.
* <p>
* <strong>Caution: This will not perform any authorization flows for the destination.</strong> Destinations
* obtained this way should only be used for accessing the properties of the destination configuration. For
Expand Down Expand Up @@ -235,7 +257,8 @@ public DestinationProperties getDestinationProperties( @Nonnull final String des
* @param options
* Destination configuration object.
* @return A Try iterable of CF destinations.
* @deprecated since 5.1.0. Use {@link #getAllDestinationProperties()} instead.
* @deprecated since 5.1.0. Use {@link #getAllDestinationProperties()} and
* {@link #getAllDestinationProperties(DestinationServiceRetrievalStrategy)} instead.
*/
@Nonnull
@Deprecated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,38 @@ void testGettingDestinationProperties()
.getConfigurationAsJson("/subaccountDestinations", withoutToken(TECHNICAL_USER_CURRENT_TENANT));
}

@Test
void testGettingDestinationPropertiesProvider()
{
doReturn(responseServiceInstanceDestination)
.when(destinationServiceAdapter)
.getConfigurationAsJson("/instanceDestinations", withoutToken(TECHNICAL_USER_PROVIDER));
doReturn(responseSubaccountDestination)
.when(destinationServiceAdapter)
.getConfigurationAsJson("/subaccountDestinations", withoutToken(TECHNICAL_USER_PROVIDER));

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");
}

@Test
void testGettingDestinationPropertiesSubscriber()
{
doReturn(responseServiceInstanceDestination)
.when(destinationServiceAdapter)
.getConfigurationAsJson("/instanceDestinations", withoutToken(TECHNICAL_USER_CURRENT_TENANT));
doReturn(responseSubaccountDestination)
.when(destinationServiceAdapter)
.getConfigurationAsJson("/subaccountDestinations", withoutToken(TECHNICAL_USER_CURRENT_TENANT));

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");
}

@Test
// slow test, run manually if needed
void destinationServiceTimeOutWhileGettingDestination()
Expand Down Expand Up @@ -540,8 +572,8 @@ void testDestinationServiceTimeoutDisabled()

@SuppressWarnings( "deprecation" )
@Test
@DisplayName( "Test getting Destination Properties for the provider" )
void testDestinationPropertiesForProvider()
@DisplayName( "Test getting Destinations for the provider" )
void testGetAllDestinationsForProvider()
{
doReturn(responseServiceInstanceDestination)
.when(destinationServiceAdapter)
Expand All @@ -567,7 +599,7 @@ void testDestinationPropertiesForProvider()

@SuppressWarnings( "deprecation" )
@Test
@DisplayName( "Test getting Destination Properties can enforce a subscriber tenant" )
@DisplayName( "Test getting Destinations can enforce a subscriber tenant" )
void testGetAllDestinationsOnlySubscriberStrategyReadsSubscriberDestinations()
{
final DestinationOptions options =
Expand All @@ -584,6 +616,17 @@ void testGetAllDestinationsOnlySubscriberStrategyReadsSubscriberDestinations()
"The current tenant is the provider tenant, which should not be the case with the option OnlySubscriber. Cannot retrieve destination.");
}

@Test
@DisplayName( "Test getting Destination Properties can enforce a subscriber tenant" )
void testGetAllDestinationPropertiesOnlySubscriberStrategyReadsSubscriberDestinations()
{
context.setTenant(providerTenant);
assertThatThrownBy(() -> loader.getAllDestinationProperties(ONLY_SUBSCRIBER))
.isInstanceOf(DestinationAccessException.class)
.hasMessageContaining(
"The current tenant is the provider tenant, which should not be the case with the option OnlySubscriber. Cannot retrieve destination.");
}

@Test
void testDestinationRetrievalProviderOnly()
{
Expand Down
4 changes: 4 additions & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
- Timeouts for OAuth2 token retrievals can now be customized.
As part of `ServiceBindingDestinationOptions` the new option `OAuth2Options.TokenRetrievalTimeout` can now be passed to set a custom timeout.
Refer to [this documentation](https://sap.github.io/cloud-sdk/docs/java/features/connectivity/service-bindings#about-the-options) for more details.
- In `DestinationService` class allow for optional argument `DestinationServiceRetrievalStrategy` in method `getAllDestinationProperties`.
This additional API allows for ensuring tenant-specific destination lookups.
Available values are: `CURRENT_TENANT` (default), `ALWAYS_PROVIDER` and `ONLY_SUBSCRIBER`.


### 📈 Improvements

Expand Down

0 comments on commit 83a3962

Please sign in to comment.