Fix: Business Logging Service Binding removed from Endpoint URLs (#356)

Co-authored-by: Matthias Kuhr <[email protected]>
Co-authored-by: Johannes Schneider <[email protected]>

Author: 3 people

cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/BtpServicePropertySuppliers.java

@@ -6,6 +6,7 @@
 
 import static com.sap.cloud.sdk.cloudplatform.connectivity.BtpServiceOptions.IasOptions.IasCommunicationOptions;
 import static com.sap.cloud.sdk.cloudplatform.connectivity.BtpServiceOptions.IasOptions.IasTargetUri;
+import static com.sap.cloud.sdk.cloudplatform.connectivity.MultiUrlPropertySupplier.REMOVE_PATH;
 
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -71,10 +72,10 @@ class BtpServicePropertySuppliers
                 ServiceIdentifier.of("business-logging"),
                 MultiUrlPropertySupplier
                     .of(BusinessLoggingOptions.class)
-                    .withUrlKey(BusinessLoggingOptions.CONFIG_API, "configservice")
-                    .withUrlKey(BusinessLoggingOptions.TEXT_API, "textresourceservice")
-                    .withUrlKey(BusinessLoggingOptions.READ_API, "readservice")
-                    .withUrlKey(BusinessLoggingOptions.WRITE_API, "writeservice")
+                    .withUrlKey(BusinessLoggingOptions.CONFIG_API, "configservice", REMOVE_PATH)
+                    .withUrlKey(BusinessLoggingOptions.TEXT_API, "textresourceservice", REMOVE_PATH)
+                    .withUrlKey(BusinessLoggingOptions.READ_API, "readservice", REMOVE_PATH)
+                    .withUrlKey(BusinessLoggingOptions.WRITE_API, "writeservice", REMOVE_PATH)
                     .factory());
     static final OAuth2PropertySupplierResolver AI_CORE =
         OAuth2PropertySupplierResolver.forServiceIdentifier(ServiceIdentifier.of("aicore"), AiCore::new);

cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/MultiUrlPropertySupplier.java

@@ -29,17 +29,23 @@
 @Slf4j
 final class MultiUrlPropertySupplier<T extends OptionsEnhancer<T>> extends DefaultOAuth2PropertySupplier
 {
+    /**
+     * A URL transformation function that removes the entire path of the provided URL. Can be used with
+     * {@link Builder#withUrlKey(OptionsEnhancer, String, Function)}.
+     */
+    static final Function<URI, URI> REMOVE_PATH = uri -> uri.resolve("/");
+
     private final Class<T> enhancerClass;
-    private final Map<OptionsEnhancer<T>, String> urlKeys;
+    private final Map<OptionsEnhancer<T>, UrlExtractor> urlExtractors;
 
-    private MultiUrlPropertySupplier(
+    MultiUrlPropertySupplier(
         @Nonnull final ServiceBindingDestinationOptions options,
         @Nonnull final Class<T> enhancerClass,
-        @Nonnull final Map<OptionsEnhancer<T>, String> urlKeys )
+        @Nonnull final Map<OptionsEnhancer<T>, UrlExtractor> urlExtractors )
     {
         super(options);
         this.enhancerClass = enhancerClass;
-        this.urlKeys = urlKeys;
+        this.urlExtractors = urlExtractors;
     }
 
     @Nonnull
@@ -55,17 +61,20 @@ public URI getServiceUri()
         }
 
         final T option = maybeOption.get();
-        final String bindingKey = urlKeys.get(option);
-        if( bindingKey == null ) {
+        final UrlExtractor urlExtractor = urlExtractors.get(option);
+        if( urlExtractor == null ) {
             throw new IllegalStateException(
                 "Found option value "
                     + option
                     + " for "
                     + enhancerClass.getName()
                     + ", but no URL key was registered for this value. Please ensure that for each possible choice a URL key is registered.");
         }
-        log.debug("Option {} selected, using binding key {}.", option, bindingKey);
-        return getCredential(URI.class, "endpoints", bindingKey).get();
+
+        return urlExtractor.getUrl(key -> {
+            log.debug("Option {} selected, using binding key {}.", option, key);
+            return getCredentialOrThrow(URI.class, "endpoints", key);
+        });
     }
 
     /**
@@ -95,7 +104,7 @@ static <T extends OptionsEnhancer<T>> Builder<T> of( @Nonnull final Class<T> enh
     static final class Builder<T extends OptionsEnhancer<T>>
     {
         private final Class<T> enhancerClass;
-        private final Map<OptionsEnhancer<T>, String> urlKeys = new HashMap<>();
+        private final Map<OptionsEnhancer<T>, UrlExtractor> urlExtractors = new HashMap<>();
 
         /**
          * Add a key under which the URL is to be found in a service binding for the given option. Typically, the
@@ -114,14 +123,64 @@ static final class Builder<T extends OptionsEnhancer<T>>
         @Nonnull
         Builder<T> withUrlKey( @Nonnull final OptionsEnhancer<T> enhancer, @Nonnull final String urlKey )
         {
-            urlKeys.put(enhancer, urlKey);
+            urlExtractors.put(enhancer, new UrlExtractor(urlKey));
+            return this;
+        }
+
+        /**
+         * Add a key under which the URL is to be found in a service binding for the given option. Typically, the
+         * {@code enhancer} should be an enum, and you should add a key for each enum value. Upon extracting the URL,
+         * apply the provided transformation.
+         *
+         * @param enhancer
+         *            An instance of the {@link OptionsEnhancer} that represents one possible option choice. It will be
+         *            used to select the URL and compared to the instance that is eventually passed in the
+         *            {@link ServiceBindingDestinationOptions} via {@code hashCode()}. This should typically be an enum
+         *            value.
+         * @param urlKey
+         *            The key under which the URL is to be found in a service binding. It will be looked up in the
+         *            {@code endpoints} property of the {@code credentials} section of the service binding.
+         * @param urlTransformation
+         *            A transformation to apply to the extract service URL.
+         * @return This builder.
+         */
+        @Nonnull
+        Builder<T> withUrlKey(
+            @Nonnull final OptionsEnhancer<T> enhancer,
+            @Nonnull final String urlKey,
+            @Nonnull final Function<URI, URI> urlTransformation )
+        {
+            urlExtractors.put(enhancer, new UrlExtractor(urlKey, urlTransformation));
             return this;
         }
 
         @Nonnull
         Function<ServiceBindingDestinationOptions, OAuth2PropertySupplier> factory()
         {
-            return options -> new MultiUrlPropertySupplier<>(options, enhancerClass, urlKeys);
+            return options -> new MultiUrlPropertySupplier<>(options, enhancerClass, urlExtractors);
+        }
+    }
+
+    @RequiredArgsConstructor
+    static final class UrlExtractor
+    {
+        private static final Function<URI, URI> NO_TRANSFORMATION = url -> url;
+
+        @Nonnull
+        private final String urlKey;
+        @Nonnull
+        private final Function<URI, URI> urlTransformation;
+
+        UrlExtractor( @Nonnull final String urlKey )
+        {
+            this(urlKey, NO_TRANSFORMATION);
+        }
+
+        @Nonnull
+        URI getUrl( @Nonnull final Function<String, URI> urlReader )
+        {
+            final URI rawUri = urlReader.apply(urlKey);
+            return urlTransformation.apply(rawUri);
         }
     }
 }

cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/BtpServicePropertySuppliersTest.java

@@ -289,6 +289,18 @@ class BusinessLoggingTest
                 entry("endpoints.textresourceservice", "https://business-logging.text_api.example.com"),
                 entry("endpoints.writeservice", "https://business-logging.write_api.example.com"));
 
+        private final ServiceBinding bindingWithRedundantPaths =
+            bindingWithCredentials(
+                ServiceIdentifier.of("business-logging"),
+                entry("endpoints.configservice", "https://business-logging.config_api.example.com/buslogs/configs"),
+                entry(
+                    "endpoints.readservice",
+                    "https://business-logging.read_api.example.com/odata/v2/com.sap.bs.businesslogging.DisplayMessage"),
+                entry(
+                    "endpoints.textresourceservice",
+                    "https://business-logging.text_api.example.com/buslogs/configs/textresources"),
+                entry("endpoints.writeservice", "https://business-logging.write_api.example.com/buslogs/log"));
+
         @ParameterizedTest
         @EnumSource( BusinessLoggingOptions.class )
         void testApiSelection( final BusinessLoggingOptions api )
@@ -297,8 +309,21 @@ void testApiSelection( final BusinessLoggingOptions api )
                 ServiceBindingDestinationOptions.forService(binding).withOption(api).build();
 
             final OAuth2PropertySupplier sut = BUSINESS_LOGGING.resolve(options);
+            assertThat(sut).isNotNull();
 
-            assertThat(sut.getServiceUri().toString()).contains(api.name().toLowerCase());
+            final ServiceBindingDestinationOptions redundantOptions =
+                ServiceBindingDestinationOptions.forService(bindingWithRedundantPaths).withOption(api).build();
+
+            final OAuth2PropertySupplier redundantSut = BUSINESS_LOGGING.resolve(redundantOptions);
+            assertThat(redundantSut).isNotNull();
+
+            final URI uri = sut.getServiceUri();
+            assertThat(uri).hasPath("/");
+            assertThat(uri.toString()).contains(api.name().toLowerCase());
+
+            final URI redundantUri = redundantSut.getServiceUri();
+            assertThat(redundantUri).hasPath("/");
+            assertThat(redundantUri).isEqualTo(uri);
         }
 
         @Test

cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/MultiUrlPropertySupplierTest.java

@@ -0,0 +1,23 @@
+package com.sap.cloud.sdk.cloudplatform.connectivity;
+
+import static com.sap.cloud.sdk.cloudplatform.connectivity.MultiUrlPropertySupplier.REMOVE_PATH;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.URI;
+
+import org.junit.jupiter.api.Test;
+
+class MultiUrlPropertySupplierTest
+{
+    @Test
+    void testRemovePath()
+    {
+        assertThat(REMOVE_PATH.apply(URI.create("https://user:[email protected]/baz")))
+            .hasToString("https://user:[email protected]/");
+        assertThat(REMOVE_PATH.apply(URI.create("https://foo.bar/baz?$select=oof"))).hasToString("https://foo.bar/");
+        assertThat(REMOVE_PATH.apply(URI.create("https://foo.bar"))).hasToString("https://foo.bar/");
+        assertThat(REMOVE_PATH.apply(URI.create("https://foo.bar/"))).hasToString("https://foo.bar/");
+        assertThat(REMOVE_PATH.apply(URI.create("https://foo.bar?$select=oof"))).hasToString("https://foo.bar/");
+    }
+
+}

release_notes.md

@@ -22,5 +22,7 @@
 ### 🐛 Fixed Issues
 
 - Fix an issue where the same `HttpClient` would be used for different users when using `PrincipalPropagation` and thus could potentially share the same (session) cookies.
+- Fix an issue where destinations for the Business Logging service that are created from a service binding (using the `ServiceBindingDestinationLoader` API) contained the concrete API path.
+  This behavior caused problems when using such a destination in a client generated with the SAP Cloud SDK's OpenApi generator.
 - [DwC] Fix an issue where the `AuthTokenAccessor` would not recognise JWT tokens passed in via the `dwc-jwt` header.
 - [DwC] Fix an issue where the current tenant would not be resolved if the `dwc-subdomain` header was missing.