Skip to content

Commit d7a6497

Browse files
committed
Update solution
1 parent 3a63cdd commit d7a6497

10 files changed

Lines changed: 168 additions & 74 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public ODataRequestBatch(
9393
this.uuidProvider = uuidProvider;
9494
this.batchUuid = uuidProvider.get();
9595
this.headers.remove(HttpHeaders.ACCEPT); // batch request does not require Accept header
96-
this.bufferStrategy = ResponseBufferStrategy.DISABLED;
96+
this.requestResultFactory = ODataRequestResultFactory.WITHOUT_BUFFER;
9797
}
9898

9999
@Nonnull

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public abstract class ODataRequestGeneric implements ODataRequestExecutable
9393
* The response buffer strategy to use for this request.
9494
*/
9595
@Nonnull
96-
ResponseBufferStrategy bufferStrategy = ResponseBufferStrategy.ENABLED;
96+
ODataRequestResultFactory requestResultFactory = ODataRequestResultFactory.WITH_BUFFER;
9797

9898
ODataRequestGeneric(
9999
@Nonnull final String servicePath,
@@ -149,14 +149,6 @@ public void addListener( @Nonnull final ODataRequestListener listener )
149149
listeners.add(listener);
150150
}
151151

152-
/**
153-
* Disable pre-buffering of http response entity.
154-
*/
155-
public void disableHttpResponseBuffering()
156-
{
157-
bufferStrategy = ResponseBufferStrategy.DISABLED;
158-
}
159-
160152
/**
161153
* Replace a header in the OData HTTP request.
162154
*
@@ -243,8 +235,7 @@ public void addQueryParameter( @Nonnull final String key, @Nullable final String
243235
{
244236
return Try
245237
.ofSupplier(httpOperation)
246-
.map(bufferStrategy.getHandler())
247-
.map(response -> new ODataRequestResultGeneric(this, response, httpClient))
238+
.map(response -> requestResultFactory.create(this, response, httpClient))
248239
.andThenTry(ODataHealthyResponseValidator::requireHealthyResponse);
249240
}
250241

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import org.apache.http.client.HttpClient;
99

10+
import com.google.common.annotations.Beta;
1011
import com.google.common.net.UrlEscapers;
1112
import com.sap.cloud.sdk.datamodel.odata.client.ODataProtocol;
1213
import com.sap.cloud.sdk.datamodel.odata.client.expression.ODataResourcePath;
@@ -134,4 +135,14 @@ public ODataRequestResultGeneric execute( @Nonnull final HttpClient httpClient )
134135
: tryExecuteWithCsrfToken(httpClient, request::requestGet);
135136
return result.get();
136137
}
138+
139+
/**
140+
* Disable pre-buffering of http response entity.
141+
*/
142+
@Beta
143+
public ODataRequestResultResource.Executable withoutResponseBuffering()
144+
{
145+
requestResultFactory = ODataRequestResultFactory.WITHOUT_BUFFER;
146+
return httpClient -> (ODataRequestResultResource) this.execute(httpClient);
147+
}
137148
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import org.apache.http.client.HttpClient;
99

10+
import com.google.common.annotations.Beta;
1011
import com.sap.cloud.sdk.datamodel.odata.client.ODataProtocol;
1112
import com.sap.cloud.sdk.datamodel.odata.client.expression.ODataResourcePath;
1213
import com.sap.cloud.sdk.datamodel.odata.client.query.StructuredQuery;
@@ -128,4 +129,14 @@ public ODataRequestResultGeneric execute( @Nonnull final HttpClient httpClient )
128129
: tryExecuteWithCsrfToken(httpClient, request::requestGet);
129130
return result.get();
130131
}
132+
133+
/**
134+
* Disable pre-buffering of http response entity.
135+
*/
136+
@Beta
137+
public ODataRequestResultResource.Executable withoutResponseBuffering()
138+
{
139+
requestResultFactory = ODataRequestResultFactory.WITHOUT_BUFFER;
140+
return httpClient -> (ODataRequestResultResource) this.execute(httpClient);
141+
}
131142
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package com.sap.cloud.sdk.datamodel.odata.client.request;
2+
3+
import javax.annotation.Nonnull;
4+
import javax.annotation.Nullable;
5+
6+
import org.apache.http.HttpResponse;
7+
import org.apache.http.client.HttpClient;
8+
import org.apache.http.entity.BufferedHttpEntity;
9+
import org.apache.http.message.BasicHttpResponse;
10+
11+
import io.vavr.control.Option;
12+
import io.vavr.control.Try;
13+
14+
/**
15+
* Enum representing the strategy for buffering HTTP responses.
16+
*/
17+
@FunctionalInterface
18+
interface ODataRequestResultFactory
19+
{
20+
ODataRequestResultGeneric create(
21+
@Nonnull final ODataRequestGeneric oDataRequest,
22+
@Nonnull final HttpResponse httpResponse,
23+
@Nullable final HttpClient httpClient );
24+
25+
/**
26+
* Strategy that does not buffer the response.
27+
*/
28+
ODataRequestResultFactory WITHOUT_BUFFER = ODataRequestResultResource::new;
29+
30+
/**
31+
* Strategy that buffers the response by creating a copy of it.
32+
*/
33+
ODataRequestResultFactory WITH_BUFFER = ( oDataRequest, httpResponse, httpClient ) -> {
34+
final BasicHttpResponse copy = new BasicHttpResponse(httpResponse.getStatusLine());
35+
Option.of(httpResponse.getLocale()).peek(copy::setLocale);
36+
Option.of(httpResponse.getAllHeaders()).peek(copy::setHeaders);
37+
Option
38+
.of(httpResponse.getEntity())
39+
.peek(entity -> Try.run(() -> copy.setEntity(new BufferedHttpEntity(entity))));
40+
return new ODataRequestResultGeneric(oDataRequest, copy, httpClient);
41+
};
42+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ public ODataRequestResultGeneric withNumberDeserializationStrategy(
147147
* Method that allows consumers to disable buffering HTTP response entity. Note that once this is disabled, HTTP
148148
* responses can only be streamed/read once
149149
*
150-
* @deprecated Please use {@link ODataRequestGeneric#disableHttpResponseBuffering()} instead.
150+
* @deprecated Please use {@link ODataRequestRead#withoutResponseBuffering()} or
151+
* {@link ODataRequestReadByKey#withoutResponseBuffering()} instead.
151152
*/
152153
@Deprecated
153154
public void disableBufferingHttpResponse()
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package com.sap.cloud.sdk.datamodel.odata.client.request;
2+
3+
import java.io.IOException;
4+
5+
import javax.annotation.Nonnull;
6+
import javax.annotation.Nullable;
7+
8+
import org.apache.http.HttpResponse;
9+
import org.apache.http.client.HttpClient;
10+
import org.apache.http.util.EntityUtils;
11+
12+
import com.google.common.annotations.Beta;
13+
14+
import lombok.EqualsAndHashCode;
15+
import lombok.extern.slf4j.Slf4j;
16+
17+
/**
18+
* OData request result for reading entities. The connection is not yet closed.
19+
*/
20+
@Slf4j
21+
@Beta
22+
@EqualsAndHashCode( callSuper = true )
23+
public class ODataRequestResultResource extends ODataRequestResultGeneric implements AutoCloseable
24+
{
25+
/**
26+
* Default constructor.
27+
*
28+
* @param oDataRequest
29+
* The original OData request
30+
* @param httpResponse
31+
* The original Http response
32+
* @param httpClient
33+
* The Http client used to execute the request
34+
*/
35+
ODataRequestResultResource(
36+
@Nonnull final ODataRequestGeneric oDataRequest,
37+
@Nonnull final HttpResponse httpResponse,
38+
@Nullable final HttpClient httpClient )
39+
{
40+
super(oDataRequest, httpResponse, httpClient);
41+
}
42+
43+
@Override
44+
public void close()
45+
{
46+
try {
47+
EntityUtils.consume(getHttpResponse().getEntity());
48+
}
49+
catch( final IOException e ) {
50+
log.warn("Failed to close the HTTP response entity.", e);
51+
}
52+
}
53+
54+
/**
55+
* Interface for executing OData requests that return a resource which must be closed.
56+
*/
57+
@FunctionalInterface
58+
public interface Executable extends ODataRequestExecutable
59+
{
60+
@Nonnull
61+
@Override
62+
ODataRequestResultResource execute( @Nonnull final HttpClient httpClient );
63+
}
64+
}

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

Lines changed: 0 additions & 40 deletions
This file was deleted.

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestResultGenericTest.java

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -182,19 +182,20 @@ void testDisabledBuffer()
182182
httpResponse.setEntity(new InputStreamEntity(inputStream, json.length()));
183183

184184
// system under test
185-
oDataRequest.disableHttpResponseBuffering();
186-
final ODataRequestResultGeneric testResult = new ODataRequestResultGeneric(oDataRequest, httpResponse);
187-
188-
// sanity checks do not consume the response
189-
assertThat(testResult.getHeaderValues("Content-Length")).isEmpty();
190-
assertThat(testResult.getHttpResponse().getStatusLine().getStatusCode()).isEqualTo(200);
191-
192-
// true-positive, successfully read once
193-
assertThat(testResult.asListOfMaps()).isEmpty();
194-
195-
// true-negative, no second read possible
196-
assertThatThrownBy(testResult::asListOfMaps)
197-
.isInstanceOf(ODataDeserializationException.class)
198-
.hasMessageContaining("Unable to read OData 4.0 response.");
185+
try(
186+
final ODataRequestResultResource testResult =
187+
new ODataRequestResultResource(oDataRequest, httpResponse, null) ) {
188+
// sanity checks do not consume the response
189+
assertThat(testResult.getHeaderValues("Content-Length")).isEmpty();
190+
assertThat(testResult.getHttpResponse().getStatusLine().getStatusCode()).isEqualTo(200);
191+
192+
// true-positive, successfully read once
193+
assertThat(testResult.asListOfMaps()).isEmpty();
194+
195+
// true-negative, no second read possible
196+
assertThatThrownBy(testResult::asListOfMaps)
197+
.isInstanceOf(ODataDeserializationException.class)
198+
.hasMessageContaining("Unable to read OData 4.0 response.");
199+
}
199200
}
200201
}

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataResponseParsingIntegrationTest.java

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static org.assertj.core.api.Assertions.assertThatCode;
55
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
66

7+
import java.io.IOException;
78
import java.time.OffsetDateTime;
89
import java.util.List;
910
import java.util.UUID;
@@ -310,10 +311,14 @@ void testExceptionWhenUnbufferedHttpEntityIsAccessedMultipleTimes()
310311
final ODataRequestRead request =
311312
new ODataRequestRead("TripPinRESTierService", "People", "$count=true&$format=json", ODataProtocol.V4);
312313

313-
request.disableHttpResponseBuffering();
314-
final ODataRequestResultGeneric result = request.execute(httpClient);
315-
assertThat(result.asMap()).containsKeys("value", "@odata.count");
316-
assertThatExceptionOfType(ODataDeserializationException.class).isThrownBy(() -> result.getInlineCount());
314+
try( final ODataRequestResultResource result = request.withoutResponseBuffering().execute(httpClient) ) {
315+
316+
// first access successful
317+
assertThat(result.asMap()).containsKeys("value", "@odata.count");
318+
319+
// second access fails
320+
assertThatExceptionOfType(ODataDeserializationException.class).isThrownBy(result::getInlineCount);
321+
}
317322
}
318323

319324
@Test
@@ -322,9 +327,17 @@ void testWhenUnbufferedHttpEntityIsAccessedAfterAccessingBufferedHttpEntity()
322327
final ODataRequestRead request =
323328
new ODataRequestRead("TripPinRESTierService", "People", "$count=true&$format=json", ODataProtocol.V4);
324329

325-
request.disableHttpResponseBuffering();
326-
final ODataRequestResultGeneric result = request.execute(httpClient);
330+
@SuppressWarnings( "resource" ) // let's assume user is forgetting try-with-resources
331+
final ODataRequestResultGeneric result = request.withoutResponseBuffering().execute(httpClient);
332+
333+
// first access successful
327334
assertThat(result.asMap()).containsKeys("value", "@odata.count");
328-
assertThat(result.getInlineCount()).isEqualTo(20);
335+
336+
// second access fails
337+
assertThatExceptionOfType(ODataDeserializationException.class)
338+
.isThrownBy(result::getInlineCount)
339+
.havingRootCause()
340+
.isInstanceOf(IOException.class)
341+
.withMessage("Attempted read from closed stream.");
329342
}
330343
}

0 commit comments

Comments
 (0)