Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #328: Allow injecting mapper used for request serialization #329

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package com.redhat.lightblue.client.request;

import org.apache.commons.lang.StringUtils;

import com.fasterxml.jackson.databind.JsonNode;

import com.redhat.lightblue.client.http.HttpMethod;
import com.redhat.lightblue.client.Operation;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public HttpMethod getHttpMethod() {
/**
* Requst body as string, defaults to getBodyJson().toString()
*/
@Deprecated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Or perhaps what should be used instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getBodyJson should be used instead. This uses a default global object mapper so it can't be configured. I'll add a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it can be configured, but isn't configured the same as for responses. Details in #328

public String getBody() {
JsonNode body=getBodyJson();
if(body!=null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ protected HttpResponse callService(LightblueRequest request, String baseUri) thr

long t1 = System.currentTimeMillis();

HttpResponse response = httpTransport.executeRequest(request, baseUri);
HttpResponse response = httpTransport.executeRequest(request, baseUri, mapper);

if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Response received from service: {}", response.getBody());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
import com.redhat.lightblue.client.http.LightblueHttpClientException;
import com.redhat.lightblue.client.request.LightblueRequest;

public interface HttpTransport extends Closeable {

HttpResponse executeRequest(LightblueRequest request, String baseUri) throws LightblueHttpClientException;
import com.fasterxml.jackson.databind.ObjectMapper;

public interface HttpTransport extends Closeable {
HttpResponse executeRequest(LightblueRequest request, String baseUri, ObjectMapper mapper)
throws LightblueHttpClientException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLSocketFactory;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -114,7 +115,8 @@ public static JavaNetHttpTransport fromLightblueClientConfiguration(LightblueCli
}

@Override
public HttpResponse executeRequest(LightblueRequest request, String baseUri) throws LightblueHttpClientException {
public HttpResponse executeRequest(LightblueRequest request, String baseUri, ObjectMapper mapper)
throws LightblueHttpClientException {
try {
String url = request.getRestURI(baseUri);
LOGGER.debug("Executing request, url={}", url);
Expand All @@ -128,7 +130,9 @@ public HttpResponse executeRequest(LightblueRequest request, String baseUri) thr
}
}

connection.setRequestMethod((request.getHttpMethod() != null ? request.getHttpMethod().toString() : HttpMethod.GET.name()));
connection.setRequestMethod((request.getHttpMethod() != null
? request.getHttpMethod().toString()
: HttpMethod.GET.name()));
connection.setRequestProperty("Accept", "application/json");
connection.setRequestProperty("Accept-Charset", "utf-8");

Expand All @@ -141,13 +145,13 @@ public HttpResponse executeRequest(LightblueRequest request, String baseUri) thr
}

if (this.compression == Compression.LZF) {
LOGGER.debug("Advertising lzf decoding capabilities to lightblue");
LOGGER.trace("Advertising lzf decoding capabilities to lightblue");
connection.setRequestProperty("Accept-Encoding", "lzf");
}

String body = request.getBody();
if (StringUtils.isNotBlank(body)) {
sendRequestBody(body, connection);
JsonNode body = request.getBodyJson();
if (body != null && !body.isNull()) {
sendRequestBody(body, connection, mapper);
}

return new HttpResponse(response(connection), connection.getHeaderFields());
Expand All @@ -163,19 +167,13 @@ public void close() throws IOException {
// Nothing to do
}

private void sendRequestBody(String body, HttpURLConnection connection)
private void sendRequestBody(Object body, HttpURLConnection connection, ObjectMapper mapper)
throws IOException {
byte[] bodyUtf8Bytes = body.getBytes(UTF_8);
int length = bodyUtf8Bytes.length;

connection.setDoOutput(true);
connection.setFixedLengthStreamingMode(length);

connection.setRequestProperty("Content-Length", Integer.toString(length));
connection.setRequestProperty("Content-Type", "application/json; charset=utf-8");

try (OutputStream requestStream = connection.getOutputStream()) {
requestStream.write(bodyUtf8Bytes);
mapper.writeValue(requestStream, body);
}
}

Expand Down Expand Up @@ -214,10 +212,10 @@ private String response(HttpURLConnection connection) throws LightblueHttpClient
*/
private String readResponseStream(InputStream responseStream, HttpURLConnection connection)
throws IOException {
LOGGER.debug("Reading response stream");
LOGGER.trace("Reading response stream");
InputStream decodedStream = responseStream;
if (compression == Compression.LZF && "lzf".equals(connection.getHeaderField("Content-Encoding"))) {
LOGGER.debug("Decoding lzf");
LOGGER.trace("Decoding lzf");
decodedStream = new LZFInputStream(responseStream);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.List;
import java.util.Map;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -42,7 +43,8 @@ public void testPojoMapping() throws Exception {

String response = "{\"matchCount\": 1, \"modifiedCount\": 0, \"processed\": [{\"_id\": \"idhash\", \"field\":\"value\"}], \"status\": \"COMPLETE\"}";

when(httpTransport.executeRequest(any(LightblueRequest.class), anyString())).thenReturn(new FakeResponse(response, null));
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString(), any(ObjectMapper.class)))
.thenReturn(new FakeResponse(response, null));

SimpleModelObject[] results = client.data(findRequest, SimpleModelObject[].class);

Expand All @@ -56,7 +58,8 @@ public void testPrimitiveMapping() throws Exception {
GenerateRequest request = new GenerateRequest("foo", "bar");
request.path("x").nValues(3);
String response = "{ \"processed\": [\"1\",\"2\",\"3\"], \"status\": \"COMPLETE\"}";
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString())).thenReturn(new FakeResponse(response, null));
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString(), any(ObjectMapper.class)))
.thenReturn(new FakeResponse(response, null));

String[] results = client.data(request, String[].class);

Expand All @@ -75,7 +78,8 @@ public void testPojoMappingWithParsingError() throws Exception {

String response = "{\"processed\":\"<p>This is not json</p>\"}";

when(httpTransport.executeRequest(any(LightblueRequest.class), anyString())).thenReturn(new FakeResponse(response, null));
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString(), any(ObjectMapper.class)))
.thenReturn(new FakeResponse(response, null));

client.data(findRequest, SimpleModelObject[].class);
}
Expand Down Expand Up @@ -105,7 +109,8 @@ public void testParseInvalidJson() throws Exception {

@Test
public void testUsingDefaultExecution_ReadPreference() throws Exception {
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString())).thenReturn(new FakeResponse("{}", null));
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString(), any(ObjectMapper.class)))
.thenReturn(new FakeResponse("{}", null));

LightblueClientConfiguration c = new LightblueClientConfiguration();
c.setReadPreference(ReadPreference.primary);
Expand All @@ -125,7 +130,8 @@ public void testUsingDefaultExecution_ReadPreference() throws Exception {

@Test
public void testUsingDefaultExecution_WriteConcern() throws Exception {
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString())).thenReturn(new FakeResponse("{}", null));
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString(), any(ObjectMapper.class)))
.thenReturn(new FakeResponse("{}", null));

LightblueClientConfiguration c = new LightblueClientConfiguration();
c.setWriteConcern("majority");
Expand All @@ -145,7 +151,8 @@ public void testUsingDefaultExecution_WriteConcern() throws Exception {

@Test
public void testUsingDefaultExecution_MaxQueryTimeMS() throws Exception {
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString())).thenReturn(new FakeResponse("{}", null));
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString(), any(ObjectMapper.class)))
.thenReturn(new FakeResponse("{}", null));

LightblueClientConfiguration c = new LightblueClientConfiguration();
c.setMaxQueryTimeMS(1000);
Expand All @@ -168,7 +175,8 @@ public void testUsingDefaultExecution_MaxQueryTimeMS() throws Exception {
*/
@Test
public void testUsingDefaultExecution_OverrideExecution() throws Exception {
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString())).thenReturn(new FakeResponse("{}", null));
when(httpTransport.executeRequest(any(LightblueRequest.class), anyString(), any(ObjectMapper.class)))
.thenReturn(new FakeResponse("{}", null));

LightblueClientConfiguration c = new LightblueClientConfiguration();
c.setReadPreference(ReadPreference.primary);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public String getBody() {

@Override
public JsonNode getBodyJson() {
return JSON.toJsonNode(body);
return body == null ? null : JSON.toJsonNode(body);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.io.PrintWriter;
import java.net.HttpURLConnection;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -26,6 +27,7 @@
import com.redhat.lightblue.client.http.testing.doubles.FakeLightblueRequest;
import com.redhat.lightblue.client.http.transport.JavaNetHttpTransportTest.CallConnectAndReturn;
import com.redhat.lightblue.client.request.LightblueRequest;
import com.redhat.lightblue.client.util.JSON;

/**
* Testing {@link JavaNetHttpTransport}'s lzf compression support.
Expand All @@ -36,6 +38,8 @@
@RunWith(JUnit4.class)
public class JavaNetHttpTransportCompressionTest {

private static final ObjectMapper mapper = JSON.getDefaultObjectMapper();

private HttpURLConnection mockConnection = mock(HttpURLConnection.class);
private JavaNetHttpTransport.ConnectionFactory mockConnectionFactory = mock(JavaNetHttpTransport.ConnectionFactory.class);

Expand All @@ -51,7 +55,7 @@ public void before() throws IOException {
public void shouldSetAcceptEncodingHeaderToLzfByDefault() throws LightblueHttpClientException {
LightblueRequest request = new FakeLightblueRequest("", HttpMethod.GET, "/foo/bar");

client.executeRequest(request, "");
client.executeRequest(request, "", mapper);

Mockito.verify(mockConnection).setRequestProperty("Accept-Encoding", "lzf");
}
Expand All @@ -61,7 +65,7 @@ public void shouldNotSetAcceptEncodingHeaderWhenCompressionIsDisabled() throws L
LightblueRequest request = new FakeLightblueRequest("", HttpMethod.GET, "/foo/bar");

client.setCompression(Compression.NONE);
client.executeRequest(request, "");
client.executeRequest(request, "", mapper);

Mockito.verify(mockConnection, never()).setRequestProperty(Mockito.eq("Accept-Encoding"), Mockito.anyString());
}
Expand All @@ -84,7 +88,7 @@ public void shouldDecodeResponseWhenCompressionEnabledAndContentEncodingIsDefine

when(mockConnection.getInputStream()).thenReturn(inResponseBody);

String response = client.executeRequest(request, "").getBody();
String response = client.executeRequest(request, "", mapper).getBody();

Assert.assertEquals(TEST_RESPONSE, response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -18,6 +19,7 @@
import com.redhat.lightblue.client.http.LightblueHttpClientException;
import com.redhat.lightblue.client.http.testing.doubles.FakeLightblueRequest;
import com.redhat.lightblue.client.request.LightblueRequest;
import com.redhat.lightblue.client.util.JSON;

/**
* Tests the somewhat complicated and vaguely documented semantics of JDK's
Expand All @@ -28,6 +30,8 @@
*/
@RunWith(JUnit4.class)
public class JavaNetHttpTransportIntegrationTest {
private static final ObjectMapper mapper = JSON.getDefaultObjectMapper();

@Rule
public WireMockRule wireMockRule = new WireMockRule(wireMockConfig().dynamicPort());

Expand All @@ -38,9 +42,9 @@ public void shouldReturnResponseBodyOfSuccessfulRequest() throws LightblueHttpCl
wireMockRule.stubFor(any(urlMatching(".*"))
.willReturn(aResponse().withBody("The body").withStatus(200)));

LightblueRequest request = new FakeLightblueRequest("", HttpMethod.GET, "/");
LightblueRequest request = new FakeLightblueRequest(null, HttpMethod.GET, "/");

String response = client.executeRequest(request, wireMockUrl()).getBody();
String response = client.executeRequest(request, wireMockUrl(), mapper).getBody();

assertThat(response, is("The body"));
}
Expand All @@ -50,20 +54,20 @@ public void shouldReturnResponseBodyOfUnsuccessfulRequest() throws LightblueHttp
wireMockRule.stubFor(any(urlMatching(".*"))
.willReturn(aResponse().withBody("The body").withStatus(500)));

LightblueRequest request = new FakeLightblueRequest("", HttpMethod.GET, "/");
LightblueRequest request = new FakeLightblueRequest(null, HttpMethod.GET, "/");

Assert.assertEquals("The body", client.executeRequest(request, wireMockUrl()).getBody());
Assert.assertEquals("The body", client.executeRequest(request, wireMockUrl(), mapper).getBody());
}

@Test
public void shouldThrowExceptionOnUnsuccessfulRequestWithNoResponseBody() {
wireMockRule.stubFor(any(urlMatching(".*"))
.willReturn(aResponse().withStatus(500)));

LightblueRequest request = new FakeLightblueRequest("", HttpMethod.GET, "/");
LightblueRequest request = new FakeLightblueRequest(null, HttpMethod.GET, "/");

try {
client.executeRequest(request, wireMockUrl());
client.executeRequest(request, wireMockUrl(), mapper);
Assert.fail();
} catch (LightblueHttpClientException e) {
Assert.assertEquals(500, e.getHttpStatus());
Expand Down
Loading