feat: Allow access to failed batch request (#347)

Co-authored-by: Alexander Dümont <alexander.duemont@sap.com> Co-authored-by: Alexander Dümont <22489773+newtork@users.noreply.github.com> Co-authored-by: Conner Panarella <connerp32@gmail.com> Co-authored-by: Matthias Kuhr <52661546+MatKuhr@users.noreply.github.com> Co-authored-by: Johannes Schneider <johannes.schneider03@sap.com>
SAP · Mar 15, 2024 · 1eb4153 · 1eb4153
1 parent caef1a9
commit 1eb4153
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 31 deletions.
diff --git a/...src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/MultipartHttpResponse.java b/...src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/MultipartHttpResponse.java
@@ -14,6 +14,7 @@
 import java.util.regex.Pattern;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
@@ -29,6 +30,7 @@
 import org.apache.http.message.BasicStatusLine;
 
 import io.vavr.control.Try;
+import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -41,25 +43,39 @@ class MultipartHttpResponse extends BasicHttpResponse
     private static final Pattern PATTERN_STATUS_LINE = Pattern.compile("^HTTP/(\\d).(\\d) (\\d+) (.*)");
     private static final Pattern PATTERN_NEW_LINE = Pattern.compile("\\R");
 
-    private MultipartHttpResponse( final StatusLine statusLine, final List<Header> headers, final HttpEntity entity )
+    @Nullable
+    @Getter
+    private final Integer contentId;
+
+    private MultipartHttpResponse(
+        @Nonnull final StatusLine statusLine,
+        @Nonnull final List<Header> headers,
+        @Nonnull final HttpEntity entity,
+        @Nullable final Integer contentId )
     {
         super(statusLine);
         headers.forEach(this::addHeader);
         setEntity(entity);
+        this.contentId = contentId;
     }
 
     /**
      * Factory method to construct an {@link MultipartHttpResponse} on behalf of serialized HTTP protocol content: First
      * line is the status line, the following lines are headers, the optional body is introduced with an empty line.
      *
-     * @param httpContent
+     * @param entry
      *            The HTTP protocol content, consisting of status line, headers, (emptyline) and payload.
      * @return A new HTTP response instance.
      */
     @Nonnull
-    public static MultipartHttpResponse ofHttpContent( @Nonnull final String httpContent )
+    public static MultipartHttpResponse ofHttpContent( @Nonnull final MultipartParser.Entry entry )
     {
-        final String[] lines = PATTERN_NEW_LINE.split(httpContent);
+        final Matcher contentIdMatcher =
+            Pattern
+                .compile("^Content-ID:\\s*(\\d+)\\s*$", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE)
+                .matcher(entry.getMeta());
+        final Integer contentId = contentIdMatcher.find() ? Integer.parseInt(contentIdMatcher.group(1)) : null;
+        final String[] lines = PATTERN_NEW_LINE.split(entry.getPayload());
 
         final StatusLine statusLine = getStatusLine(lines[0]);
 
@@ -82,7 +98,8 @@ public static MultipartHttpResponse ofHttpContent( @Nonnull final String httpCon
         final List<Header> headers = getHeadersFromString(header.toString());
         final ContentType contentType = getContentType(headers).orElse(ContentType.APPLICATION_JSON);
         final ContentType contentTypeCharset = withFallbackCharset(contentType, DEFAULT_CHARSET);
-        return new MultipartHttpResponse(statusLine, headers, new StringEntity(payload.toString(), contentTypeCharset));
+        final StringEntity httpEntity = new StringEntity(payload.toString(), contentTypeCharset);
+        return new MultipartHttpResponse(statusLine, headers, httpEntity, contentId);
     }
 
     @Nonnull

diff --git a/...lient/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/MultipartParser.java b/...lient/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/MultipartParser.java
@@ -32,6 +32,7 @@
 import io.vavr.control.Try;
 import lombok.AccessLevel;
 import lombok.RequiredArgsConstructor;
+import lombok.Value;
 import lombok.extern.slf4j.Slf4j;
 
 /**
@@ -122,7 +123,7 @@ public static MultipartParser ofInputStream(
      * @return An iterable multi-part response.
      */
     @Nonnull
-    public List<List<String>> toList()
+    public List<List<Entry>> toList()
     {
         return toList(Function.identity());
     }
@@ -140,7 +141,7 @@ public List<List<String>> toList()
      * @return An iterable multi-part response, with custom deserialization.
      */
     @Nonnull
-    public <T> List<List<T>> toList( @Nonnull final Function<String, T> transformation )
+    public <T> List<List<T>> toList( @Nonnull final Function<Entry, T> transformation )
     {
         return toStream()
             .map(segments -> segments.map(transformation).collect(Collectors.toList()))
@@ -155,14 +156,14 @@ public <T> List<List<T>> toList( @Nonnull final Function<String, T> transformati
      * @return An iterable multi-part response.
      */
     @Nonnull
-    public Stream<Stream<String>> toStream()
+    public Stream<Stream<Entry>> toStream()
     {
         // Create a stream from a spliterator but only retrieve the spliterator upon terminal operation of stream.
-        final Stream<Spliterator<String>> result =
+        final Stream<Spliterator<Entry>> result =
             StreamSupport.stream(this::createSpliterator, MultipartSpliterator.CHARACTERISTICS, false);
 
         // Translate stream of spliterators to stream of stream. Make sure all previous spliterators were fully consumed.
-        final AtomicReference<Spliterator<String>> previous = new AtomicReference<>(Spliterators.emptySpliterator());
+        final AtomicReference<Spliterator<Entry>> previous = new AtomicReference<>(Spliterators.emptySpliterator());
 
         return result.map(currentSpliterator -> {
             // if previous spliterator was not yet fully consumed, do so with the remaining elements
@@ -173,7 +174,7 @@ public Stream<Stream<String>> toStream()
         });
     }
 
-    private Spliterator<Spliterator<String>> createSpliterator()
+    private Spliterator<Spliterator<Entry>> createSpliterator()
     {
         final MultipartParserReader batchRead = new MultipartParserReader(reader, delimiter);
 
@@ -192,14 +193,14 @@ private Spliterator<Spliterator<String>> createSpliterator()
                 return getChangeset(maybeChangesetBoundary.get(), batchRead);
             } else { // single response
                 final String content = batchRead.untilDelimiter();
-                return Collections.singleton(content).spliterator();
+                return Collections.singleton(new Entry(segmentHead, content)).spliterator();
             }
         });
     }
 
     @Nonnull
     private
-        Spliterator<String>
+        Spliterator<Entry>
         getChangeset( @Nonnull final String changesetDelimiter, final MultipartParserReader batchRead )
     {
         final MultipartParserReader changesetRead = new MultipartParserReader(reader, changesetDelimiter);
@@ -218,7 +219,7 @@ private Spliterator<Spliterator<String>> createSpliterator()
             if( changesetRead.isFinished() ) {
                 batchRead.untilDelimiter(); // position reader to next batch delimiter
             }
-            return content;
+            return new Entry(subSegmentHead, content);
         });
     }
 
@@ -256,4 +257,11 @@ public void close()
     {
         Try.run(reader::close).onFailure(e -> log.warn("Failed to close HTTP entity multi-part parser.", e));
     }
+
+    @Value
+    static class Entry
+    {
+        String meta;
+        String payload;
+    }
 }
diff --git a/.../java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataHealthyResponseValidator.java b/.../java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataHealthyResponseValidator.java
@@ -5,6 +5,7 @@
 package com.sap.cloud.sdk.datamodel.odata.client.request;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
@@ -48,9 +49,19 @@ static void requireHealthyResponse( @Nonnull final ODataRequestResult result )
             return;
         }
 
+        ODataRequestGeneric batchFailedRequest = null;
+        if( request instanceof ODataRequestBatch oDataRequestBatch ) {
+            batchFailedRequest = findFailedBatchRequest(httpResponse, oDataRequestBatch);
+        }
+
         final Integer statusCode = statusLine == null ? null : statusLine.getStatusCode();
         final String msg = "The HTTP response code (" + statusCode + ") indicates an error.";
-        final ODataResponseException preparedException = new ODataResponseException(request, httpResponse, msg, null);
+        final ODataResponseException preparedException =
+            new ODataResponseException(
+                batchFailedRequest == null ? request : batchFailedRequest,
+                httpResponse,
+                msg,
+                null);
 
         final Try<ODataServiceError> odataError = Try.of(() -> loadErrorFromResponse(result));
         if( odataError.isSuccess() ) {
@@ -61,6 +72,35 @@ static void requireHealthyResponse( @Nonnull final ODataRequestResult result )
         throw preparedException;
     }
 
+    @Nullable
+    private static
+        ODataRequestGeneric
+        findFailedBatchRequest( final HttpResponse httpResponse, final ODataRequestBatch oDataRequestBatch )
+    {
+        final Integer failedBatchRequestNumber =
+            httpResponse instanceof MultipartHttpResponse
+                ? ((MultipartHttpResponse) httpResponse).getContentId()
+                : null;
+        if( failedBatchRequestNumber == null ) {
+            return null;
+        }
+
+        for( final ODataRequestBatch.BatchItem requestGeneric : oDataRequestBatch.getRequests() ) {
+            if( requestGeneric instanceof ODataRequestBatch.BatchItemChangeset changeset ) {
+                for( final ODataRequestBatch.BatchItemSingle single : changeset.getRequests() ) {
+                    if( single.getContentId() == failedBatchRequestNumber ) {
+                        return single.getRequest();
+                    }
+                }
+            } else if( requestGeneric instanceof ODataRequestBatch.BatchItemSingle single ) {
+                if( single.getContentId() == failedBatchRequestNumber ) {
+                    return single.getRequest();
+                }
+            }
+        }
+        return null;
+    }
+
     @Nonnull
     private static ODataServiceError loadErrorFromResponse( final ODataRequestResult result )
         throws ODataDeserializationException

diff --git a/.../com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultMultipartGeneric.java b/.../com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultMultipartGeneric.java
@@ -121,7 +121,8 @@ public ODataRequestResultGeneric getResult( @Nonnull final ODataRequestGeneric r
             throw new ODataDeserializationException(batchRequest, httpResponse, msg, null);
         }
 
-        final ODataRequestResultGeneric result = new ODataRequestResultGeneric(request, response);
+        final ODataRequestResultGeneric result = new ODataRequestResultGeneric(batchRequest, response);
+        result.disableBufferingHttpResponse(); // the artificial HttpResponse is static, no buffer required
         ODataHealthyResponseValidator.requireHealthyResponse(result);
         return result;
     }

diff --git a/...t/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/MultipartParserTest.java b/...t/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/MultipartParserTest.java
@@ -2,6 +2,7 @@
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import static com.sap.cloud.sdk.datamodel.odata.client.request.MultipartParser.Entry;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.mockito.Mockito.mock;
@@ -60,12 +61,12 @@ void testSimpleReadSuccess( @Nonnull final String newLine )
         final String delimiter = "--batchresponse_76ef6b0a-a0e2-4f31-9f70-f5d3f73a6bef";
 
         // user code
-        final List<List<String>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
+        final List<List<Entry>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
         final MultipartHttpResponse httpResponse = MultipartHttpResponse.ofHttpContent(result.get(0).get(0));
 
         assertThat(result).hasSize(1);
         assertThat(result.get(0)).hasSize(1);
-        assertThat(result.get(0).get(0)).startsWith("HTTP/1.1 200 OK");
+        assertThat(result.get(0).get(0).getPayload()).startsWith("HTTP/1.1 200 OK");
 
         assertThat(httpResponse.getAllHeaders()).satisfiesExactly(header -> {
             assertThat(header.getName()).isEqualTo("Content-Type");
@@ -93,7 +94,7 @@ void testEmptyReadSuccess( @Nonnull final String newLine )
         final String delimiter = "--batchresponse_76ef6b0a-a0e2-4f31-9f70-f5d3f73a6bef";
 
         // user code
-        final List<List<String>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
+        final List<List<Entry>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
         assertThat(result).isEmpty();
     }
 
@@ -106,7 +107,7 @@ void testEmpty()
         final String delimiter = "--batchresponse_76ef6b0a-a0e2-4f31-9f70-f5d3f73a6bef";
 
         // user code
-        final List<List<String>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
+        final List<List<Entry>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
         assertThat(result).isEmpty();
     }
 
@@ -120,7 +121,7 @@ void testWrongDelimiterSuccess( @Nonnull final String newLine )
         final String delimiter = "--some-delimiter";
 
         // user code
-        final List<List<String>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
+        final List<List<Entry>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
         assertThat(result).isEmpty();
     }
 
@@ -134,7 +135,7 @@ void testNewLineBeforeReadSuccess( @Nonnull final String newLine )
         final String delimiter = "--batchresponse_76ef6b0a-a0e2-4f31-9f70-f5d3f73a6bef";
 
         // user code
-        final List<List<String>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
+        final List<List<Entry>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toList();
         assertThat(result).isEmpty();
     }
 
@@ -184,7 +185,7 @@ void testReadResultUncached( @Nonnull final String newLine )
         final String delimiter = "--batchresponse_76ef6b0a-a0e2-4f31-9f70-f5d3f73a6bef";
 
         // user code
-        final Stream<Stream<String>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toStream();
+        final Stream<Stream<Entry>> result = MultipartParser.ofInputStream(response, UTF_8, delimiter).toStream();
 
         // assert behavior
         assertThat(result.count()).isEqualTo(2); // first access successful
@@ -204,7 +205,7 @@ void testWriteResultUncached( @Nonnull final String newLine )
         resp.setHeader("Content-Type", "multipart/mixed; boundary=batchresponse_76ef6b0a-a0e2-4f31-9f70-f5d3f73a6bef");
 
         // user code
-        final Stream<Stream<String>> result = MultipartParser.ofHttpResponse(resp).toStream();
+        final Stream<Stream<Entry>> result = MultipartParser.ofHttpResponse(resp).toStream();
 
         // assert behavior
         assertThat(result.count()).isEqualTo(2); // first access successful
@@ -224,13 +225,13 @@ void testWriteResultCached( @Nonnull final String newLine )
         resp.setHeader("Content-Type", "multipart/mixed; boundary=batchresponse_76ef6b0a-a0e2-4f31-9f70-f5d3f73a6bef");
 
         // user code
-        final List<List<String>> result = MultipartParser.ofHttpResponse(resp).toList();
+        final List<List<Entry>> result = MultipartParser.ofHttpResponse(resp).toList();
 
         // assert behavior
         assertThat(result).isNotEmpty();
 
-        for( final List<String> segments : result ) {
-            for( final String segment : segments ) {
+        for( final List<Entry> segments : result ) {
+            for( final Entry segment : segments ) {
                 // ignore
             }
         }
@@ -240,10 +241,10 @@ void testWriteResultCached( @Nonnull final String newLine )
             .satisfiesExactly(
                 segments1 -> assertThat(segments1)
                     .satisfiesExactly(
-                        payload1 -> assertThat(payload1).contains("HTTP/1.1 201 Created"),
-                        payload2 -> assertThat(payload2).contains("HTTP/1.1 201 Created")),
+                        entry1 -> assertThat(entry1.getPayload()).contains("HTTP/1.1 201 Created"),
+                        entry2 -> assertThat(entry2.getPayload()).contains("HTTP/1.1 201 Created")),
                 segments2 -> assertThat(segments2)
-                    .satisfiesExactly(payload1 -> assertThat(payload1).contains("HTTP/1.1 404 Not Found")));
+                    .satisfiesExactly(entry1 -> assertThat(entry1.getPayload()).contains("HTTP/1.1 404 Not Found")));
     }
 
     @SneakyThrows

diff --git a/...cloud/sdk/datamodel/odatav4/referenceservice/ODataClientBatchResponseParsingUnitTest.java b/...cloud/sdk/datamodel/odatav4/referenceservice/ODataClientBatchResponseParsingUnitTest.java
@@ -267,13 +267,15 @@ void testBatchWithErrorInChangeset()
             .satisfies(e -> {
                 assertThat(e.getHttpCode()).isEqualTo(400);
                 assertThat(e.getHttpBody().get()).contains("The FirstName field is required");
+                assertThat(e.getRequest()).isSameAs(create2);
             });
 
         assertThatExceptionOfType(ODataResponseException.class)
             .isThrownBy(() -> batchResponse.getResult(create2))
             .satisfies(e -> {
                 assertThat(e.getHttpCode()).isEqualTo(400);
                 assertThat(e.getHttpBody().get()).contains("The FirstName field is required");
+                assertThat(e.getRequest()).isSameAs(create2);
             });
 
         final ODataRequestResultGeneric resultReadByKey = batchResponse.getResult(readByKey);

diff --git a/release_notes.md b/release_notes.md
@@ -12,7 +12,8 @@
 
 ### ✨ New Functionality
 
-- Support service bindings to the [SAP BTP AI Core Service](https://api.sap.com/api/AI_CORE_API) by default in the `ServiceBindingDestinationLoader` API. 
+- Support service bindings to the [SAP BTP AI Core Service](https://api.sap.com/api/AI_CORE_API) by default in the `ServiceBindingDestinationLoader` API.
+- Failed OData v4 Batch requests now return the specific failed request from the exception: `ODataResponseException.getRequest()`.
 
 ### 📈 Improvements