Skip to content

Feature/replace deprecated body #959

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
May 9, 2019
Merged

Feature/replace deprecated body #959

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad04144
replaced old body with new Body.asBytes()
mstrYoda May 8, 2019
0deb0e8
mvn clean install to format files
mstrYoda May 8, 2019
d2478df
changed body() to requestBody().asBytes() for java11
mstrYoda May 8, 2019
a9c1743
put it back removed deprecated method
mstrYoda May 8, 2019
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
4 changes: 2 additions & 2 deletions core/src/main/java/feign/Client.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
connection.addRequestProperty("Accept", "*/*");
}

if (request.body() != null) {
if (request.requestBody().asBytes() != null) {
if (contentLength != null) {
connection.setFixedLengthStreamingMode(contentLength);
} else {
Expand All @@ -128,7 +128,7 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
out = new DeflaterOutputStream(out);
}
try {
out.write(request.body());
out.write(request.requestBody().asBytes());
} finally {
try {
out.close();
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/feign/FeignException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ static FeignException errorReading(Request request, Response response, IOExcepti
response.status(),
format("%s reading %s %s", cause.getMessage(), request.httpMethod(), request.url()),
cause,
request.body());
request.requestBody().asBytes());
}

public static FeignException errorStatus(String methodKey, Response response) {
Expand Down
8 changes: 5 additions & 3 deletions core/src/main/java/feign/Logger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ protected void logRequest(String configKey, Level logLevel, Request request) {
}

int bodyLength = 0;
if (request.body() != null) {
bodyLength = request.body().length;
if (request.requestBody().asBytes() != null) {
bodyLength = request.requestBody().asBytes().length;
if (logLevel.ordinal() >= Level.FULL.ordinal()) {
String bodyText =
request.charset() != null ? new String(request.body(), request.charset()) : null;
request.charset() != null
? new String(request.requestBody().asBytes(), request.charset())
: null;
log(configKey, ""); // CRLF
log(configKey, "%s", bodyText != null ? bodyText : "Binary data");
}
Expand Down
11 changes: 0 additions & 11 deletions core/src/main/java/feign/Request.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,17 +190,6 @@ public Charset charset() {
return body.encoding;
}

/**
* If present, this is the replayable body to send to the server. In some cases, this may be
* interpretable as text.
*
* @see #charset()
* @deprecated use {@link #requestBody()} instead
*/
public byte[] body() {
Copy link
Member

Choose a reason for hiding this comment

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

I don't recall when we deprecated this.

If was on 9.x think is ok to drop it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If deprecated method is completely removed from usage, is not that ok to remove the method itself? I think removing it prevents further usage. Should I keep it or remove it?

Copy link
Member

Choose a reason for hiding this comment

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

A decoder may exists out there that still uses it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.. Sorry I am not fully familiar on source code. I'll put it back.

Just a question out of this context, is it acceptable to use a 3rd party library (like apache commons lang) for some requirement?

Copy link
Member

Choose a reason for hiding this comment

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

core has being clean from external dependencies and we are most likely to want to keep it that way.

Particularly about apache commons-*, every time I see them I read "old deprecated code that somehow still survived" =)

return body.data;
}

public Body requestBody() {
return body;
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/feign/TargetTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public Request apply(RequestTemplate input) {
urlEncoded.httpMethod(),
urlEncoded.url().replace("%2F", "/"),
urlEncoded.headers(),
urlEncoded.body(), urlEncoded.charset());
urlEncoded.requestBody().asBytes(), urlEncoded.charset());
}
};

Expand Down
6 changes: 3 additions & 3 deletions httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,14 @@ HttpUriRequest toHttpUriRequest(Request request, Request.Options options)
}

// request body
if (request.body() != null) {
if (request.requestBody().asBytes() != null) {
HttpEntity entity = null;
if (request.charset() != null) {
ContentType contentType = getContentType(request);
String content = new String(request.body(), request.charset());
String content = new String(request.requestBody().asBytes(), request.charset());
entity = new StringEntity(content, contentType);
} else {
entity = new ByteArrayEntity(request.body());
entity = new ByteArrayEntity(request.requestBody().asBytes());
}

requestBuilder.setEntity(entity);
Expand Down
4 changes: 2 additions & 2 deletions java11/src/main/java/feign/http2client/Http2Client.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ private Builder newRequestBuilder(Request request) throws IOException {
}

final BodyPublisher body;
if (request.body() == null) {
if (request.requestBody().asBytes() == null) {
body = BodyPublishers.noBody();
} else {
body = BodyPublishers.ofByteArray(request.body());
body = BodyPublishers.ofByteArray(request.requestBody().asBytes());
}

final Builder requestBuilder = HttpRequest.newBuilder()
Expand Down
4 changes: 2 additions & 2 deletions jaxrs2/src/main/java/feign/jaxrs2/JAXRSClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ public feign.Response execute(feign.Request request, Options options) throws IOE
}

private Entity<byte[]> createRequestEntity(feign.Request request) {
if (request.body() == null) {
if (request.requestBody().asBytes() == null) {
return null;
}

return Entity.entity(
request.body(),
request.requestBody().asBytes(),
new Variant(mediaType(request.headers()), locale(request.headers()),
encoding(request.charset())));
}
Expand Down
2 changes: 1 addition & 1 deletion mock/src/main/java/feign/mock/RequestKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private RequestKey(Request request) {
this.url = buildUrl(request);
this.headers = RequestHeaders.of(request.headers());
this.charset = request.charset();
this.body = request.body();
this.body = request.requestBody().asBytes();
}

public HttpMethod getMethod() {
Expand Down
3 changes: 2 additions & 1 deletion mock/src/test/java/feign/mock/MockClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ public void verifyInvocation() {
mockClient.verifyTimes(HttpMethod.POST, "/repos/netflix/feign/contributors", 1);
assertThat(results, hasSize(1));

byte[] body = mockClient.verifyOne(HttpMethod.POST, "/repos/netflix/feign/contributors").body();
byte[] body = mockClient.verifyOne(HttpMethod.POST, "/repos/netflix/feign/contributors")
.requestBody().asBytes();
assertThat(body, notNullValue());

String message = new String(body);
Expand Down
2 changes: 1 addition & 1 deletion okhttp/src/main/java/feign/okhttp/OkHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ static Request toOkHttpRequest(feign.Request input) {
requestBuilder.addHeader("Accept", "*/*");
}

byte[] inputBody = input.body();
byte[] inputBody = input.requestBody().asBytes();
boolean isMethodWithBody =
HttpMethod.POST == input.httpMethod() || HttpMethod.PUT == input.httpMethod()
|| HttpMethod.PATCH == input.httpMethod();
Expand Down
2 changes: 1 addition & 1 deletion ribbon/src/main/java/feign/ribbon/LBClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ static class RibbonRequest extends ClientRequest implements Cloneable {

Request toRequest() {
// add header "Content-Length" according to the request body
final byte[] body = request.body();
final byte[] body = request.requestBody().asBytes();
final int bodyLength = body != null ? body.length : 0;
// create a new Map to avoid side effect, not to change the old headers
Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>();
Expand Down