Skip to content

Commit

Permalink
Avoid buffering and stream the response if the headers indicate it is…
Browse files Browse the repository at this point in the history
… definitely JSON.

Signed-off-by: Peter Runge <[email protected]>
  • Loading branch information
causalnet authored and rohanKanojia committed Dec 13, 2022
1 parent 165b557 commit e7de2d3
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
1 change: 1 addition & 0 deletions doc/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Allow having build args with same name but different value in various sources, which are overriden in the order of precedence in resulting build args map ([1407](https://github.com/fabric8io/docker-maven-plugin/issues/1407)) @pavelsmolensky
- Use double for `docker.cpus` property and interpret this value in the same way as Docker config option `--cpus` ([1609](https://github.com/fabric8io/docker-maven-plugin/pull/1609)) @vjuranek
- NPE from Assembly plugin when POM packaging is used ([1146](https://github.com/fabric8io/docker-maven-plugin/issues/1146)) @slawekjaranowski
- Docker pulling progress only shown after pull has completed and not in real-time ([1598](https://github.com/fabric8io/docker-maven-plugin/issues/1598)) @causalnet
- Bump `org.yaml:snakeyaml` to v1.32 ([1619](https://github.com/fabric8io/docker-maven-plugin/pull/1619)) @pen4
- Bump `com.google.cloud.tools:jib-core` to v0.23.0 ([1620](https://github.com/fabric8io/docker-maven-plugin/pull/1620)) @pen4
- Bump `com.google.guava:guava` to v31.1-jre @rohanKanojia
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ public class HcChunkedResponseHandlerWrapper implements ResponseHandler<Object>
@Override
public Object handleResponse(HttpResponse response) throws IOException {
try (InputStream stream = response.getEntity().getContent()) {
ByteArrayOutputStream baos = getMultipleReadbleOutputStream(stream);
InputStream is = new ByteArrayInputStream(baos.toByteArray());
// In the previous version of this file the following if() was as follows. The methode isJson() checks
// by header (not by body):
// if(isJson(response)) {
Expand All @@ -39,11 +37,18 @@ public Object handleResponse(HttpResponse response) throws IOException {
//
// If no error is detected, the Maven-build goes on despite there was a problem building the
// image!
// The following if() first checks for the application/json Content-Type. If no Content-Type is set,
// it tries to detect if the body is JSON. If so, the handler is called.
if(isJsonCheckedByHeader(response) || (isMissingContentType(response) && isJsonCheckedByBody(is))){
is = new ByteArrayInputStream(baos.toByteArray());
EntityStreamReaderUtil.processJsonStream(handler, is);
// If the header indicates application/json Content-Type, stream the response to the handler.
// If there is no Content-Type header it tries to detect if the body is JSON. If so, the handler is called
// with a buffered response.
if (isJsonCheckedByHeader(response)) {
EntityStreamReaderUtil.processJsonStream(handler, stream);
} else if (isMissingContentType(response)) {
ByteArrayOutputStream baos = getMultipleReadbleOutputStream(stream);
InputStream is = new ByteArrayInputStream(baos.toByteArray());
if (isJsonCheckedByBody(is)) {
is = new ByteArrayInputStream(baos.toByteArray());
EntityStreamReaderUtil.processJsonStream(handler, is);
}
}
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.entity.StringEntity;
import org.apache.http.entity.InputStreamEntity;
import org.apache.http.message.BasicHeader;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.AdditionalMatchers;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;

Expand All @@ -26,24 +29,34 @@ class HcChunkedResponseHandlerWrapperTest {
private EntityStreamReaderUtil.JsonEntityResponseHandler handler;
@Mock
private HttpResponse response;
@Mock
private HttpEntity entity;
@Mock
private EntityStreamReaderUtil entityStreamReaderUtil;

private MockedStatic<EntityStreamReaderUtil> entityStreamReaderUtilMock;

private Header[] headers;
private HcChunkedResponseHandlerWrapper hcChunkedResponseHandlerWrapper;
private InputStream responseInputStream;
private HttpEntity entity;

@BeforeEach
void setUp() {
responseInputStream = new ByteArrayInputStream("{}".getBytes(StandardCharsets.UTF_8));
entity = new InputStreamEntity(responseInputStream);
hcChunkedResponseHandlerWrapper = new HcChunkedResponseHandlerWrapper(handler);
entityStreamReaderUtilMock = Mockito.mockStatic(EntityStreamReaderUtil.class);
entityStreamReaderUtilMock.when(() -> EntityStreamReaderUtil.processJsonStream(Mockito.any(), Mockito.any())).thenCallRealMethod();
}

@AfterEach
void tearDown() {
entityStreamReaderUtilMock.close();
}

@Test
void handleResponseWithJsonResponse() throws IOException {
givenResponseHeaders(new BasicHeader("ConTenT-Type", "application/json; charset=UTF-8"));
hcChunkedResponseHandlerWrapper.handleResponse(response);
verifyProcessJsonStream(1);
verifyResponseUnbuffered();
}

@Test
Expand All @@ -60,14 +73,25 @@ void handleResponseWithNoContentType() throws IOException {
// timesCalled is 1 here because without "Content-Type" handleResponse() tries to parse the body to
// detect if it is JSON or not. See HcChunkedResponseHandlerWrapper.handleResponse() for more details.
verifyProcessJsonStream(1);
verifyResponseBuffered();
}

private void givenResponseHeaders(Header... headers) throws IOException {
Mockito.doReturn(headers).when(response).getAllHeaders();
Mockito.doReturn(new StringEntity("{}")).when(response).getEntity();
Mockito.doReturn(entity).when(response).getEntity();
}

private void verifyProcessJsonStream(int timesCalled) throws IOException {
Mockito.verify(handler, Mockito.times(timesCalled)).stop();
}

// Response is unbuffered when processJsonStream() called on original stream
private void verifyResponseUnbuffered() {
entityStreamReaderUtilMock.verify(() -> EntityStreamReaderUtil.processJsonStream(Mockito.eq(handler), Mockito.eq(responseInputStream)));
}

// Response is buffered when processJsonStream() not called on original stream
private void verifyResponseBuffered() {
entityStreamReaderUtilMock.verify(() -> EntityStreamReaderUtil.processJsonStream(Mockito.eq(handler), AdditionalMatchers.not(Mockito.eq(responseInputStream))));
}
}

0 comments on commit e7de2d3

Please sign in to comment.