SAP / cloud-sdk-java

Use the SAP Cloud SDK for Java to reduce development effort when building applications on SAP Business Technology Platform that communicate with SAP solutions and services such as SAP S/4HANA Cloud, SAP SuccessFactors, and many others.
Apache License 2.0
22 stars 11 forks source link

Connectivity: `getAllDestinationProperties()` no full replacement for deprecated `tryGetAllDestinations(options)` #558

Closed LukasHeimann closed 1 week ago

LukasHeimann commented 3 weeks ago

Issue Description

https://github.com/SAP/cloud-sdk-java/pull/199 deprecated some old destination list retrieval APIs in favor of getAllDestinationProperties() on the destination service. However, the old method allowed to pass in DestinationOptions, while the new version doesn't.

In our code, we want to make sure to now accidentally respond with the list of destinations of the provider tenant when requests should be happening in the subscriber tenants only. In the past, this was possible with:

  private final DestinationService service = new DestinationService();

  private static final DestinationOptions options = DestinationOptions
      .builder()
      .augmentBuilder(
          DestinationServiceOptionsAugmenter
              .augmenter()
              .retrievalStrategy(DestinationServiceRetrievalStrategy.ONLY_SUBSCRIBER))
      .build();

  public List<String> getAllDestinationsDeprecated() {
    var iterable = this.service.tryGetAllDestinations(options).get();
    var stream = StreamSupport.stream(iterable.spliterator(), false);
    return stream
        .filter(d -> DestinationType.HTTP.equals(d.get(DestinationProperty.TYPE).getOrNull()))
        .filter(d -> d.get(DestinationProperty.NAME).isDefined())
        .map(d -> d.get(DestinationProperty.NAME).get())
        .toList();
  }

The behavior is as expected -- go in as the provider tenant, and the request fails.

The new API is to be used in a similar fashion, it just doesn't fail when in the context of the provider tenant:

  public List<String> getAllDestinationsNew() {
    return this.service.getAllDestinationProperties().stream()
        .filter(d -> DestinationType.HTTP.equals(d.get(DestinationProperty.TYPE).getOrNull()))
        .filter(d -> d.get(DestinationProperty.NAME).isDefined())
        .map(d -> d.get(DestinationProperty.NAME).get())
        .toList();
  }

Ideally, the new API getAllDestinationProperties() would also allow to receive DestinationOptions.

Impact / Priority

Affected development phase: Production (degraded second line of defense for productive deployments)

Impact: Inconvenience

Timeline: Go-live planned for this year

Project Details


Checklist

KavithaSiva commented 3 weeks ago

Hi @LukasHeimann,

Thanks for reaching out to us. This is a valid ask and we will try to provide this feature in our upcoming release.

Regards, Kavitha

MatKuhr commented 1 week ago

Hi @LukasHeimann, glad to let you know the improvement has been implemented and shipped with version 5.12.0. Please let us know if something doesn't work as expected 😉

LukasHeimann commented 1 week ago

Hi @MatKuhr -- really great, that was way faster than expected! Looks good and works like a breeze!