From d6c66649fb2798931596c1f9e4149cfebb3f3516 Mon Sep 17 00:00:00 2001 From: rnewbigging <103130942+rnewbigging@users.noreply.github.com> Date: Fri, 6 Oct 2023 15:59:26 +0200 Subject: [PATCH] 322 - remove CURRENT_TENANT_THEN_PROVIDER (#86) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Dümont <22489773+newtork@users.noreply.github.com> --- .../DestinationRetrievalStrategyResolver.java | 158 ++---------------- .../ScpCfDestinationRetrievalStrategy.java | 13 -- ...tinationRetrievalStrategyResolverTest.java | 94 ----------- .../ScpCfDestinationLoaderTest.java | 138 --------------- release_notes_next_major.md | 2 + 5 files changed, 20 insertions(+), 385 deletions(-) diff --git a/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolver.java b/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolver.java index 07d2ee4ee..cdfa483fe 100644 --- a/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolver.java +++ b/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolver.java @@ -10,7 +10,6 @@ import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.TECHNICAL_USER_PROVIDER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ALWAYS_PROVIDER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT; -import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ONLY_SUBSCRIBER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.EXCHANGE_ONLY; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.FORWARD_USER_TOKEN; @@ -29,7 +28,6 @@ import javax.annotation.Nullable; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException; -import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationNotFoundException; import com.sap.cloud.sdk.cloudplatform.security.AuthTokenAccessor; import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; @@ -41,13 +39,13 @@ @Slf4j @RequiredArgsConstructor -@SuppressWarnings( { "PMD.TooManyStaticImports", "deprecation" } ) // without these static imports the code becomes unreadable +@SuppressWarnings( "PMD.TooManyStaticImports" ) // without these static imports the code becomes unreadable class DestinationRetrievalStrategyResolver { private static final Strategy tokenExchangeOnlyStrategy = new Strategy(NAMED_USER_CURRENT_TENANT, false); private final Supplier providerTenantIdSupplier; private final Function destinationRetriever; - private final Function> allDstinationRetriever; + private final Function> allDestinationRetriever; static final String JWT_ATTR_EXT = "ext_attr"; static final String JWT_ATTR_ENHANCER = "enhancer"; @@ -72,12 +70,12 @@ static DestinationRetrievalStrategyResolver forSingleDestination( static DestinationRetrievalStrategyResolver forAllDestinations( final Supplier providerTenantIdSupplier, - final Function> allDstinationRetriever ) + final Function> allDestinationRetriever ) { return new DestinationRetrievalStrategyResolver( providerTenantIdSupplier, ( any ) -> null, - allDstinationRetriever); + allDestinationRetriever); } Strategy resolveSingleRequestStrategy( @@ -94,7 +92,7 @@ Strategy resolveSingleRequestStrategy( case ONLY_SUBSCRIBER: behalfTechnicalUser = TECHNICAL_USER_CURRENT_TENANT; break; - // sanity check that this method is never called for CURRENT_TENANT_THEN_PROVIDER + // sanity check default: throw new IllegalStateException( "Unexpected retrieval strategy " @@ -164,34 +162,20 @@ private ScpCfDestinationTokenExchangeStrategy getDefaultTokenExchangeStrategy() } Supplier prepareSupplier( - @Nonnull final ScpCfDestinationRetrievalStrategy originalRetrievalStrategy, + @Nonnull final ScpCfDestinationRetrievalStrategy retrievalStrategy, @Nonnull final ScpCfDestinationTokenExchangeStrategy tokenExchangeStrategy ) throws DestinationAccessException { log .debug( "Preparing request(s) towards the destination service based on the strategies {} and {}", - originalRetrievalStrategy, + retrievalStrategy, tokenExchangeStrategy); - warnOrThrowOnDeprecatedOrUnsupportedCombinations(originalRetrievalStrategy, tokenExchangeStrategy); - - final ScpCfDestinationRetrievalStrategy retrievalStrategy; - if( originalRetrievalStrategy == CURRENT_TENANT_THEN_PROVIDER && currentTenantIsProvider() ) { - retrievalStrategy = CURRENT_TENANT; - } else { - retrievalStrategy = originalRetrievalStrategy; - } + warnOrThrowOnDeprecatedOrUnsupportedCombinations(retrievalStrategy, tokenExchangeStrategy); final Strategy strategy; - // handle the simple cases - if( tokenExchangeStrategy != LOOKUP_THEN_EXCHANGE && retrievalStrategy != CURRENT_TENANT_THEN_PROVIDER ) { - strategy = resolveSingleRequestStrategy(retrievalStrategy, tokenExchangeStrategy); - return () -> destinationRetriever.apply(strategy); - } - - // deal with LOOKUP_THEN_EXCHANGE but exclude CURRENT_TENANT_THEN_PROVIDER for now - if( retrievalStrategy != CURRENT_TENANT_THEN_PROVIDER ) { + if( tokenExchangeStrategy == LOOKUP_THEN_EXCHANGE ) { strategy = resolveSingleRequestStrategy(retrievalStrategy, LOOKUP_ONLY); return () -> { final ScpCfDestinationServiceV1Response result = destinationRetriever.apply(strategy); @@ -206,62 +190,8 @@ Supplier prepareSupplier( }; } - // handle CURRENT_TENANT_THEN_PROVIDER where current tenant != provider - return prepareSupplierForSubscriberThenProviderCase(tokenExchangeStrategy); - } - - Supplier prepareSupplierForSubscriberThenProviderCase( - @Nonnull final ScpCfDestinationTokenExchangeStrategy tokenExchangeStrategy ) - throws DestinationAccessException - { - // sanity check that this is never called with the provider tenant - if( currentTenantIsProvider() ) { - throw new IllegalStateException( - "Unexpected state: Preparing request to destination service for subscriber tenant, but current tenant is provider."); - } - final Strategy strategy; - final Strategy providerLookupOnlyStrategy = resolveSingleRequestStrategy(ALWAYS_PROVIDER, LOOKUP_ONLY); - - if( tokenExchangeStrategy == LOOKUP_ONLY || tokenExchangeStrategy == FORWARD_USER_TOKEN ) { - strategy = resolveSingleRequestStrategy(ONLY_SUBSCRIBER, tokenExchangeStrategy); - return () -> { - try { - return destinationRetriever.apply(strategy); - } - catch( final DestinationNotFoundException e ) { - log - .debug( - "Did not find the destination in the subscriber account, falling back to the provider account.", - e); - return destinationRetriever.apply(providerLookupOnlyStrategy); - } - }; - } - - if( tokenExchangeStrategy == EXCHANGE_ONLY ) { - strategy = resolveSingleRequestStrategy(ONLY_SUBSCRIBER, EXCHANGE_ONLY); - return () -> destinationRetriever.apply(strategy); - } - - // handle LOOKUP_THEN_EXCHANGE - strategy = resolveSingleRequestStrategy(ONLY_SUBSCRIBER, LOOKUP_ONLY); - - return () -> { - try { - final ScpCfDestinationServiceV1Response result = destinationRetriever.apply(strategy); - if( !doesDestinationConfigurationRequireUserTokenExchange(result) ) { - return result; - } - return destinationRetriever.apply(tokenExchangeOnlyStrategy); - } - catch( final DestinationNotFoundException e ) { - log - .debug( - "Did not find the destination in the subscriber account, falling back to the provider account.", - e); - return destinationRetriever.apply(providerLookupOnlyStrategy); - } - }; + strategy = resolveSingleRequestStrategy(retrievalStrategy, tokenExchangeStrategy); + return () -> destinationRetriever.apply(strategy); } private void warnOrThrowOnDeprecatedOrUnsupportedCombinations( @@ -290,44 +220,6 @@ private void warnOrThrowOnDeprecatedOrUnsupportedCombinations( LOOKUP_ONLY); } } - if( retrievalStrategy != CURRENT_TENANT_THEN_PROVIDER ) { - return; - } - log - .warn( - "The retrieval strategy {} is deprecated and should no longer be used." - + " Please query subscriber and provider accounts individually using {} and {}", - CURRENT_TENANT_THEN_PROVIDER, - ONLY_SUBSCRIBER, - ALWAYS_PROVIDER); - if( currentTenantIsProvider() ) { - log - .warn( - "The retrieval strategy {} is used unnecessarily, the current tenant is the provider tenant." - + " Only a single request will be made.", - CURRENT_TENANT_THEN_PROVIDER); - return; - } - if( tokenExchangeStrategy == null || tokenExchangeStrategy == LOOKUP_ONLY ) { - return; - } - log.warn("Option {} is not supported in conjunction with {}.", retrievalStrategy, tokenExchangeStrategy); - switch( tokenExchangeStrategy ) { - case EXCHANGE_ONLY: //fallthrough - log.warn("Falling back to {} with {}.", CURRENT_TENANT, EXCHANGE_ONLY); - break; - case FORWARD_USER_TOKEN: - case LOOKUP_THEN_EXCHANGE: - log - .warn( - "Attempting to apply {} for {}, hoping that destinations requiring a user token will only be present in the subscriber account." - + " Potential requests to the provider account will not contain any user information.", - tokenExchangeStrategy, - CURRENT_TENANT_THEN_PROVIDER); - break; - default: - throw new IllegalStateException("Unexpected token strategy " + tokenExchangeStrategy); - } } Supplier> prepareSupplierAllDestinations( @Nonnull final DestinationOptions options ) @@ -349,34 +241,20 @@ Supplier> prepareSupplierAllDestinations( @Nonnull final Desti Supplier> prepareSupplierAllDestinations( @Nonnull final ScpCfDestinationRetrievalStrategy strategy ) + throws IllegalArgumentException { warnOrThrowOnDeprecatedOrUnsupportedCombinations(strategy, null); switch( strategy ) { case ALWAYS_PROVIDER: { - return () -> allDstinationRetriever.apply(TECHNICAL_USER_PROVIDER); - } - case CURRENT_TENANT_THEN_PROVIDER: { - return () -> { - try { - final List destinations = - allDstinationRetriever.apply(TECHNICAL_USER_CURRENT_TENANT); - if( currentTenantIsProvider() || !destinations.isEmpty() ) { - return destinations; - } - } - catch( final Exception e ) { - log - .warn( - "Falling back to the provider tenant after failing to retrieve destinations for the subscriber tenant."); - log.debug("Lookup of all destinations for the subscriber tenant failed.", e); - } - return allDstinationRetriever.apply(TECHNICAL_USER_PROVIDER); - }; + return () -> allDestinationRetriever.apply(TECHNICAL_USER_PROVIDER); } case ONLY_SUBSCRIBER: - case CURRENT_TENANT: + case CURRENT_TENANT: { + return () -> allDestinationRetriever.apply(TECHNICAL_USER_CURRENT_TENANT); + } default: { - return () -> allDstinationRetriever.apply(TECHNICAL_USER_CURRENT_TENANT); + throw new IllegalArgumentException( + "The provided destination retrieval strategy " + strategy + " is not valid."); } } } diff --git a/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationRetrievalStrategy.java b/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationRetrievalStrategy.java index c54f5805f..dcdf28ecf 100644 --- a/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationRetrievalStrategy.java +++ b/cloudplatform/cloudplatform-connectivity-scp-cf/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationRetrievalStrategy.java @@ -20,19 +20,6 @@ */ public enum ScpCfDestinationRetrievalStrategy { - /** - * Try to retrieve the current tenant's destination first. If the current tenant is a subscriber and no destination - * was found, fallback to the provider's destination. When loading all destinations both subaccount and instance - * level will be considered individually: If the current tenant is a subscriber and there are no destinations on the - * subaccount level, the subaccount level of the provider will be considered. Independently of that, if the current - * tenant is a subscriber and there are no destinations on the instance level, the provider instance level will be - * queried. - * - * @deprecated Please query subscriber and provider tenants individually instead using {@link #ONLY_SUBSCRIBER} and - * {@link #ALWAYS_PROVIDER}. - */ - @Deprecated - CURRENT_TENANT_THEN_PROVIDER("CurrentTenantThenProvider"), /** * Only load destination from the provider's sub-account, regardless if subscribers have a destination of the same diff --git a/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolverTest.java b/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolverTest.java index 717a624b8..1c761af67 100644 --- a/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolverTest.java +++ b/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/DestinationRetrievalStrategyResolverTest.java @@ -10,7 +10,6 @@ import static com.sap.cloud.sdk.cloudplatform.connectivity.OnBehalfOf.TECHNICAL_USER_PROVIDER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ALWAYS_PROVIDER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT; -import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy.ONLY_SUBSCRIBER; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.EXCHANGE_ONLY; import static com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationTokenExchangeStrategy.FORWARD_USER_TOKEN; @@ -20,7 +19,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -41,7 +39,6 @@ import com.auth0.jwt.JWT; import com.auth0.jwt.algorithms.Algorithm; import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException; -import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationNotFoundException; import com.sap.cloud.sdk.cloudplatform.security.AuthToken; import com.sap.cloud.sdk.cloudplatform.security.AuthTokenAccessor; import com.sap.cloud.sdk.cloudplatform.tenant.DefaultTenant; @@ -52,7 +49,6 @@ import io.vavr.Tuple3; import io.vavr.control.Try; -@SuppressWarnings( "deprecation" ) class DestinationRetrievalStrategyResolverTest { private static final Tenant providerT = new DefaultTenant("provider"); @@ -166,96 +162,6 @@ void testExceptionsAreThrownForImpossibleTokenExchanges() verifyNoMoreInteractions(destinationRetriever); } - @Test - @DisplayName( "When current tenant == provider then CURRENT_TENANT_THEN_PROVIDER should be equal to CURRENT_TENANT" ) - void testCurrentThenProviderSimpleCase() - { - TenantAccessor - .executeWithTenant( - providerT, - () -> sut.prepareSupplier(CURRENT_TENANT_THEN_PROVIDER, LOOKUP_THEN_EXCHANGE)); - - verify(sut, times(1)).resolveSingleRequestStrategy(CURRENT_TENANT, LOOKUP_ONLY); - - TenantAccessor - .executeWithTenant( - providerT, - () -> assertThatThrownBy(() -> sut.prepareSupplierForSubscriberThenProviderCase(LOOKUP_ONLY)) - .isInstanceOf(IllegalStateException.class)); - } - - @Test - @DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with LOOKUP_ONLY" ) - void testSubThenProvLookupOnly() - { - doThrow(DestinationNotFoundException.class).when(destinationRetriever).apply(any()); - - // subscriber tenant is implied - assertThatThrownBy(sut.prepareSupplierForSubscriberThenProviderCase(LOOKUP_ONLY)::get) - .isInstanceOf(DestinationNotFoundException.class); - - verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_CURRENT_TENANT, false))); - verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_PROVIDER, false))); - verifyNoMoreInteractions(destinationRetriever); - } - - @Test - @DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with FORWARD_USER_TOKEN" ) - void testSubThenProvFwdUserToken() - { - doThrow(DestinationNotFoundException.class).when(destinationRetriever).apply(any()); - - // subscriber tenant is implied - assertThatThrownBy(sut.prepareSupplierForSubscriberThenProviderCase(FORWARD_USER_TOKEN)::get) - .isInstanceOf(DestinationNotFoundException.class); - - verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_CURRENT_TENANT, true))); - verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_PROVIDER, false))); - verifyNoMoreInteractions(destinationRetriever); - } - - @Test - @DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with EXCHANGE_ONLY" ) - void testSubThenProvExchangeOnly() - { - doAnswer(( any ) -> true).when(sut).doesDestinationConfigurationRequireUserTokenExchange(any()); - - // subscriber tenant is implied - sut.prepareSupplierForSubscriberThenProviderCase(EXCHANGE_ONLY).get(); - - verify(destinationRetriever, times(1)).apply(eq(new Strategy(NAMED_USER_CURRENT_TENANT, false))); - verifyNoMoreInteractions(destinationRetriever); - } - - @Test - @DisplayName( "Test using CURRENT_TENANT_THEN_PROVIDER with LOOKUP_THEN_EXCHANGE" ) - void testLookupThenExchangeWithCurrentTenantThenProvider() - { - doAnswer(( any ) -> true).when(sut).doesDestinationConfigurationRequireUserTokenExchange(any()); - - // subscriber tenant is implied - sut.prepareSupplierForSubscriberThenProviderCase(LOOKUP_THEN_EXCHANGE).get(); - - verify(destinationRetriever, times(1)).apply(eq(new Strategy(TECHNICAL_USER_CURRENT_TENANT, false))); - verify(destinationRetriever, times(1)).apply(eq(new Strategy(NAMED_USER_CURRENT_TENANT, false))); - verifyNoMoreInteractions(destinationRetriever); - } - - @Test - @DisplayName( "Test getting all destinations with CURRENT_TENANT_THEN_PROVIDER" ) - void testAllDestinationsCurrTenThenProv() - { - doThrow(DestinationAccessException.class).when(allDestinationRetriever).apply(TECHNICAL_USER_CURRENT_TENANT); - - // subscriber tenant is implied - sut.prepareSupplierAllDestinations(CURRENT_TENANT_THEN_PROVIDER).get(); - - verify(allDestinationRetriever, times(1)).apply(TECHNICAL_USER_CURRENT_TENANT); - verify(allDestinationRetriever, times(1)).apply(TECHNICAL_USER_PROVIDER); - - verifyNoMoreInteractions(allDestinationRetriever); - } - @Test @DisplayName( "Test default strategies are set correctly" ) void testDefaultStrategies() diff --git a/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationLoaderTest.java b/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationLoaderTest.java index 54223396a..28e49f508 100644 --- a/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationLoaderTest.java +++ b/cloudplatform/cloudplatform-connectivity-scp-cf/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/ScpCfDestinationLoaderTest.java @@ -22,7 +22,6 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -565,82 +564,6 @@ public void testLoadAllDestinationsProviderOnly() .containsOnly("CC8-HTTP-BASIC", "CC8-HTTP-CERT", "CC8-HTTP-CERT1"); } - @SuppressWarnings( "deprecation" ) - @Test - public void testLoadAllDestinationsWithProviderFallback() - { - // mock to return empty list as destinations for subscriber tenant. - doReturn(responseServiceInstanceDestination) - .when(scpCfDestinationServiceAdapter) - .getConfigurationAsJson("/instanceDestinations", OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - doReturn(responseSubaccountDestination) - .when(scpCfDestinationServiceAdapter) - .getConfigurationAsJson("/subaccountDestinations", OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - - final DestinationOptionsAugmenter optionsStrategy = - augmenter().retrievalStrategy(ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER); - final DestinationOptions options = DestinationOptions.builder().augmentBuilder(optionsStrategy).build(); - final Try> destinations = loader.tryGetAllDestinations(options); - - assertThat(destinations.get().iterator()).isNotNull(); - - final List destinationList = new ArrayList<>(); - destinations.get().forEach(destinationList::add); - - assertThat(destinationList.size()).isEqualTo(3); - assertThat(destinationList) - .extracting(d -> d.get(DestinationProperty.NAME).get()) - .containsOnly("CC8-HTTP-BASIC", "CC8-HTTP-CERT", "CC8-HTTP-CERT1"); - - // verify destinations for provider tenant are not called - verify(scpCfDestinationServiceAdapter, times(0)) - .getConfigurationAsJson("/instanceDestinations", OnBehalfOf.TECHNICAL_USER_PROVIDER); - verify(scpCfDestinationServiceAdapter, times(0)) - .getConfigurationAsJson("/subaccountDestinations", OnBehalfOf.TECHNICAL_USER_PROVIDER); - } - - @SuppressWarnings( "deprecation" ) - @Test - public void testLoadAllDestinationsCurrentTenantThenProviderWithProviderAsCurrentTenant() - { - TenantAccessor.setTenantFacade(() -> Try.success(providerTenant)); - - // mock to return destinations for current == provider tenant - doReturn(responseServiceInstanceDestination) - .when(scpCfDestinationServiceAdapter) - .getConfigurationAsJson("/instanceDestinations", OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - doReturn(responseSubaccountDestination) - .when(scpCfDestinationServiceAdapter) - .getConfigurationAsJson("/subaccountDestinations", OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - - final DestinationOptionsAugmenter optionsStrategy = - augmenter().retrievalStrategy(ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER); - final DestinationOptions options = DestinationOptions.builder().augmentBuilder(optionsStrategy).build(); - final Try> destinations = loader.tryGetAllDestinations(options); - - assertThat(destinations.get().iterator()).isNotNull(); - - final List destinationList = new ArrayList<>(); - destinations.get().forEach(destinationList::add); - - assertThat(destinationList.size()).isEqualTo(3); - assertThat(destinationList) - .extracting(d -> d.get(DestinationProperty.NAME).get()) - .containsOnly("CC8-HTTP-BASIC", "CC8-HTTP-CERT", "CC8-HTTP-CERT1"); - - // assert one lookup happens - verify(scpCfDestinationServiceAdapter, times(1)) - .getConfigurationAsJson("/instanceDestinations", OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - verify(scpCfDestinationServiceAdapter, times(1)) - .getConfigurationAsJson("/subaccountDestinations", OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - - // assert no other lookup has been attempted - verify(scpCfDestinationServiceAdapter, never()) - .getConfigurationAsJson(any(), eq(OnBehalfOf.TECHNICAL_USER_PROVIDER)); - verify(scpCfDestinationServiceAdapter, never()) - .getConfigurationAsJson(any(), eq(OnBehalfOf.NAMED_USER_CURRENT_TENANT)); - } - @Test public void testGetAllDestinationsOnlySubscriberStrategyReadsSubscriberDestinations() { @@ -749,67 +672,6 @@ public void testDestinationRetrievalSubscriberOnly() assertThat(loadedHttpDestination.getUri()).isEqualTo(URI.create(subscriberUrl)); } - @SuppressWarnings( "deprecation" ) - @Test - public void testDestinationRetrievalWithProviderFallback() - { - final ScpCfDestinationOptionsAugmenter optionsStrategy = - augmenter().retrievalStrategy(ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER); - final DestinationOptions options = DestinationOptions.builder().augmentBuilder(optionsStrategy).build(); - - final Try loadedDestination = loader.tryGetDestination(destinationName, options); - final HttpDestination loadedHttpDestination = loadedDestination.get().asHttp(); - - assertThat(loadedHttpDestination.getUri()).isEqualTo(URI.create(subscriberUrl)); - } - - @SuppressWarnings( "deprecation" ) - @Test - public void testDestinationRetrievalFallbackToSubscriber() - { - doReturn(createHttpDestinationServiceResponse(destinationName, providerUrl)) - .when(scpCfDestinationServiceAdapter) - .getConfigurationAsJson("/destinations/ProviderDestination", OnBehalfOf.TECHNICAL_USER_PROVIDER); - doThrow(DestinationNotFoundException.class) - .when(scpCfDestinationServiceAdapter) - .getConfigurationAsJsonWithUserToken( - "/destinations/ProviderDestination", - OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - - final ScpCfDestinationOptionsAugmenter optionsStrategy = - augmenter().retrievalStrategy(ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER); - final DestinationOptions options = DestinationOptions.builder().augmentBuilder(optionsStrategy).build(); - - final Try loadedDestination = loader.tryGetDestination("ProviderDestination", options); - final HttpDestination loadedHttpDestination = loadedDestination.get().asHttp(); - - assertThat(loadedHttpDestination.getUri()).isEqualTo(URI.create(providerUrl)); - } - - @SuppressWarnings( "deprecation" ) - @Test - // Seems to use the Strategy 'TECHNICAL_USER_CURRENT_TENANT/forwardToken=true' which is mocked to return subscriber information. - // Change in behavior? - public void testDestinationRetrievalCurrentTenantThenProviderWithProviderAsCurrentTenant() - { - TenantAccessor.setTenantFacade(() -> Try.success(providerTenant)); - - doReturn(createHttpDestinationServiceResponse(destinationName, providerUrl)) - .when(scpCfDestinationServiceAdapter) - .getConfigurationAsJsonWithUserToken( - "/destinations/" + destinationName, - OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT); - - final ScpCfDestinationOptionsAugmenter optionsStrategy = - augmenter().retrievalStrategy(ScpCfDestinationRetrievalStrategy.CURRENT_TENANT_THEN_PROVIDER); - final DestinationOptions options = DestinationOptions.builder().augmentBuilder(optionsStrategy).build(); - - final Try loadedDestination = loader.tryGetDestination(destinationName, options); - final HttpDestination loadedHttpDestination = loadedDestination.get().asHttp(); - - assertThat(loadedHttpDestination.getUri()).isEqualTo(URI.create(providerUrl)); - } - @Test public void testTokenExchangeLookupThenExchange() { diff --git a/release_notes_next_major.md b/release_notes_next_major.md index e0d5d1da3..d591b326c 100644 --- a/release_notes_next_major.md +++ b/release_notes_next_major.md @@ -102,6 +102,8 @@ blog: https://blogs.sap.com/?p=xxx - Removed the following elements from enum `com.sap.cloud.sdk.cloudplatform.connectivity.AuthenticationType`: - `APP_TO_APP_SSO` - `INTERNAL_SYSTEM_AUTHENTICATION` +- Removed the deprecated enum element `CURRENT_TENANT_THEN_PROVIDER` from `com.sap.cloud.sdk.cloudplatform.connectivity.ScpCfDestinationRetrievalStrategy`. + As alternative please run the destination lookup queries separately in your application for `ONLY_SUBSCRIBER` and `ALWAYS_PROVIDER` as fallback. - Removed the `javax.inject.Named` annotation from the OData Generator (v2, v4). - Other changes to the Destination API: - Retrieving a `Destination` from the Destination service (i.e. using the `ScpCfDestinationLoader`) will now throw an exception if any of the attached certificates isn't valid.