fix: (odata-client) Move disable-buffer toggle from result object to request builder (#850)

Author: newtork

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestBatch.java

@@ -92,6 +92,7 @@ public ODataRequestBatch(
         this.uuidProvider = uuidProvider;
         this.batchUuid = uuidProvider.get();
         this.headers.remove(HttpHeaders.ACCEPT); // batch request does not require Accept header
+        this.requestResultFactory = ODataRequestResultFactory.WITHOUT_BUFFER;
     }
 
     @Nonnull

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestGeneric.java

@@ -89,6 +89,12 @@ public abstract class ODataRequestGeneric implements ODataRequestExecutable
     @Setter
     protected CsrfTokenRetriever csrfTokenRetriever;
 
+    /**
+     * The response buffer strategy to use for this request.
+     */
+    @Nonnull
+    ODataRequestResultFactory requestResultFactory = ODataRequestResultFactory.WITH_BUFFER;
+
     ODataRequestGeneric(
         @Nonnull final String servicePath,
         @Nonnull final ODataResourcePath resourcePath,
@@ -229,7 +235,7 @@ public void addQueryParameter( @Nonnull final String key, @Nullable final String
     {
         return Try
             .ofSupplier(httpOperation)
-            .map(response -> new ODataRequestResultGeneric(this, response, httpClient))
+            .map(response -> requestResultFactory.create(this, response, httpClient))
             .andThenTry(ODataHealthyResponseValidator::requireHealthyResponse);
     }
 

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestRead.java

@@ -7,6 +7,7 @@
 
 import org.apache.http.client.HttpClient;
 
+import com.google.common.annotations.Beta;
 import com.google.common.net.UrlEscapers;
 import com.sap.cloud.sdk.datamodel.odata.client.ODataProtocol;
 import com.sap.cloud.sdk.datamodel.odata.client.expression.ODataResourcePath;
@@ -134,4 +135,15 @@ public ODataRequestResultGeneric execute( @Nonnull final HttpClient httpClient )
                 : tryExecuteWithCsrfToken(httpClient, request::requestGet);
         return result.get();
     }
+
+    /**
+     * Disable pre-buffering of http response entity.
+     */
+    @Beta
+    @Nonnull
+    public ODataRequestResultResource.Executable withoutResponseBuffering()
+    {
+        requestResultFactory = ODataRequestResultFactory.WITHOUT_BUFFER;
+        return httpClient -> (ODataRequestResultResource) this.execute(httpClient);
+    }
 }

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestReadByKey.java

@@ -7,6 +7,7 @@
 
 import org.apache.http.client.HttpClient;
 
+import com.google.common.annotations.Beta;
 import com.sap.cloud.sdk.datamodel.odata.client.ODataProtocol;
 import com.sap.cloud.sdk.datamodel.odata.client.expression.ODataResourcePath;
 import com.sap.cloud.sdk.datamodel.odata.client.query.StructuredQuery;
@@ -128,4 +129,17 @@ public ODataRequestResultGeneric execute( @Nonnull final HttpClient httpClient )
                 : tryExecuteWithCsrfToken(httpClient, request::requestGet);
         return result.get();
     }
+
+    /**
+     * Disable pre-buffering of http response entity.
+     *
+     * @since 5.21.0
+     */
+    @Beta
+    @Nonnull
+    public ODataRequestResultResource.Executable withoutResponseBuffering()
+    {
+        requestResultFactory = ODataRequestResultFactory.WITHOUT_BUFFER;
+        return httpClient -> (ODataRequestResultResource) this.execute(httpClient);
+    }
 }

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultFactory.java

@@ -0,0 +1,53 @@
+package com.sap.cloud.sdk.datamodel.odata.client.request;
+
+import static org.slf4j.LoggerFactory.getLogger;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.StatusLine;
+import org.apache.http.client.HttpClient;
+import org.apache.http.entity.BufferedHttpEntity;
+import org.apache.http.message.BasicHttpResponse;
+import org.slf4j.Logger;
+
+import io.vavr.control.Option;
+import io.vavr.control.Try;
+
+/**
+ * Enum representing the strategy for buffering HTTP responses.
+ */
+@FunctionalInterface
+interface ODataRequestResultFactory
+{
+    /**
+     * Strategy that does not buffer the response.
+     */
+    ODataRequestResultFactory WITHOUT_BUFFER = ODataRequestResultResource::new;
+
+    /**
+     * Strategy that buffers the response by creating a copy of it.
+     */
+    ODataRequestResultFactory WITH_BUFFER = ( oDataRequest, httpResponse, httpClient ) -> {
+        final StatusLine status = httpResponse.getStatusLine();
+        final BasicHttpResponse copy = new BasicHttpResponse(status);
+        Option.of(httpResponse.getLocale()).peek(copy::setLocale);
+        Option.of(httpResponse.getAllHeaders()).peek(copy::setHeaders);
+
+        final Logger log = getLogger(ODataRequestResultFactory.class);
+        Option
+            .of(httpResponse.getEntity())
+            .onEmpty(() -> log.debug("HTTP response entity is empty: {}", status))
+            .map(entity -> Try.run(() -> copy.setEntity(new BufferedHttpEntity(entity))))
+            .peek(b -> b.onSuccess(v -> log.debug("Successfully buffered the HTTP response entity.")))
+            .peek(b -> b.onFailure(e -> log.warn("Failed to buffer HTTP response entity: {}", status, e)));
+
+        return new ODataRequestResultGeneric(oDataRequest, copy, httpClient);
+    };
+
+    ODataRequestResultGeneric create(
+        @Nonnull final ODataRequestGeneric oDataRequest,
+        @Nonnull final HttpResponse httpResponse,
+        @Nullable final HttpClient httpClient );
+}

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultGeneric.java

@@ -25,9 +25,7 @@
 import org.apache.http.HttpResponse;
 import org.apache.http.StatusLine;
 import org.apache.http.client.HttpClient;
-import org.apache.http.entity.BufferedHttpEntity;
 import org.apache.http.entity.ContentType;
-import org.apache.http.message.BasicHttpResponse;
 import org.apache.http.util.EntityUtils;
 
 import com.google.common.base.Strings;
@@ -70,15 +68,12 @@ public class ODataRequestResultGeneric
 {
     private final ODataResponseDeserializer deserializer;
 
-    @Nullable
-    private volatile HttpResponse bufferedHttpResponse;
-    private volatile boolean isBufferHttpResponse = true;
-
     @Getter
     @Nonnull
     private final ODataRequestGeneric oDataRequest;
 
     @Nonnull
+    @Getter
     private final HttpResponse httpResponse;
 
     private NumberDeserializationStrategy numberStrategy = NumberDeserializationStrategy.DOUBLE;
@@ -155,63 +150,12 @@ public ODataRequestResultGeneric withNumberDeserializationStrategy(
      * Method that allows consumers to disable buffering HTTP response entity. Note that once this is disabled, HTTP
      * responses can only be streamed/read once
      *
+     * @deprecated Please use {@link ODataRequestRead#withoutResponseBuffering()} or
+     *             {@link ODataRequestReadByKey#withoutResponseBuffering()} instead.
      */
+    @Deprecated
     public void disableBufferingHttpResponse()
     {
-        if( bufferedHttpResponse == null ) {
-            isBufferHttpResponse = false;
-        } else {
-            log.warn("Buffering the HTTP response cannot be disabled! The content has already been buffered.");
-        }
-    }
-
-    /**
-     * Method that creates a {@link BufferedHttpEntity} from the {@link HttpEntity} if buffering the HTTP response is
-     * not turned off by using {@link ODataRequestResultGeneric#disableBufferingHttpResponse()}.
-     *
-     * @return An HttpResponse
-     */
-    @Nonnull
-    @Override
-    public HttpResponse getHttpResponse()
-    {
-        if( !isBufferHttpResponse ) {
-            log.debug("Buffering is disabled, returning unbuffered http response");
-            return httpResponse;
-        }
-
-        if( bufferedHttpResponse != null ) {
-            return Objects.requireNonNull(bufferedHttpResponse);
-        }
-
-        synchronized( this ) {
-            if( bufferedHttpResponse != null ) {
-                return Objects.requireNonNull(bufferedHttpResponse);
-            }
-
-            final StatusLine statusLine = httpResponse.getStatusLine();
-            final HttpEntity httpEntity = httpResponse.getEntity();
-            if( statusLine == null || httpEntity == null ) {
-                log
-                    .debug(
-                        "skipping buffering of http entity as either there is no http entity or response does not include a status-line.");
-                return httpResponse;
-            }
-
-            final Try<HttpEntity> entity = Try.of(() -> new BufferedHttpEntity(httpEntity));
-            if( entity.isFailure() ) {
-                log.warn("Failed to buffer HTTP response. Unable to buffer HTTP entity.", entity.getCause());
-                return httpResponse;
-            }
-
-            final BasicHttpResponse proxyResponse = new BasicHttpResponse(statusLine);
-            proxyResponse.setHeaders(httpResponse.getAllHeaders());
-            proxyResponse.setEntity(entity.get());
-            Option.of(httpResponse.getLocale()).peek(proxyResponse::setLocale);
-            bufferedHttpResponse = proxyResponse;
-        }
-
-        return Objects.requireNonNull(bufferedHttpResponse);
     }
 
     @Override

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultMultipartGeneric.java

@@ -125,7 +125,6 @@ public ODataRequestResultGeneric getResult( @Nonnull final ODataRequestGeneric r
         }
 
         final ODataRequestResultGeneric result = new ODataRequestResultGeneric(batchRequest, response);
-        result.disableBufferingHttpResponse(); // the artificial HttpResponse is static, no buffer required
         ODataHealthyResponseValidator.requireHealthyResponse(result);
         return result;
     }

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultResource.java

@@ -0,0 +1,73 @@
+package com.sap.cloud.sdk.datamodel.odata.client.request;
+
+import java.io.IOException;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
+import org.apache.http.util.EntityUtils;
+
+import com.google.common.annotations.Beta;
+
+import lombok.EqualsAndHashCode;
+import lombok.extern.slf4j.Slf4j;
+
+/**
+ * OData request result for reading entities. The request is not yet closed. The response is not consumed. The
+ * connection is not yet released.
+ * <p>
+ * <b>Note:</b> This class implements {@link AutoCloseable} and should be closed after use to ensure that the underlying
+ * connection is released.
+ * <p>
+ * <b>Note:</b> This class is not thread-safe. The HTTP response object should not be consumed by multiple threads at
+ * the same time.
+ *
+ * @since 5.21.0
+ */
+@Slf4j
+@Beta
+@EqualsAndHashCode( callSuper = true )
+public class ODataRequestResultResource extends ODataRequestResultGeneric implements AutoCloseable
+{
+    /**
+     * Default constructor.
+     *
+     * @param oDataRequest
+     *            The original OData request
+     * @param httpResponse
+     *            The original Http response
+     * @param httpClient
+     *            The Http client used to execute the request
+     */
+    ODataRequestResultResource(
+        @Nonnull final ODataRequestGeneric oDataRequest,
+        @Nonnull final HttpResponse httpResponse,
+        @Nullable final HttpClient httpClient )
+    {
+        super(oDataRequest, httpResponse, httpClient);
+    }
+
+    @Override
+    public void close()
+    {
+        try {
+            EntityUtils.consume(getHttpResponse().getEntity());
+        }
+        catch( final IOException e ) {
+            log.warn("Failed to close the HTTP response entity.", e);
+        }
+    }
+
+    /**
+     * Interface for executing OData requests that return a resource which must be closed.
+     */
+    @FunctionalInterface
+    public interface Executable extends ODataRequestExecutable
+    {
+        @Nonnull
+        @Override
+        ODataRequestResultResource execute( @Nonnull final HttpClient httpClient );
+    }
+}