diff --git a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java index 546fde34d11..b45d36e3295 100644 --- a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java @@ -19,7 +19,6 @@ package org.springframework.http.client; import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; -import java.net.URL; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -73,7 +72,7 @@ public class OkHttp3ClientHttpRequestFactory /** * Sets the underlying read timeout in milliseconds. * A value of 0 specifies an infinite timeout. - * @see okhttp3.OkHttpClient.Builder#readTimeout(long, TimeUnit) + * @see OkHttpClient.Builder#readTimeout(long, TimeUnit) */ public void setReadTimeout(int readTimeout) { this.client = this.client.newBuilder() @@ -84,7 +83,7 @@ public class OkHttp3ClientHttpRequestFactory /** * Sets the underlying write timeout in milliseconds. * A value of 0 specifies an infinite timeout. - * @see okhttp3.OkHttpClient.Builder#writeTimeout(long, TimeUnit) + * @see OkHttpClient.Builder#writeTimeout(long, TimeUnit) */ public void setWriteTimeout(int writeTimeout) { this.client = this.client.newBuilder() @@ -95,7 +94,7 @@ public class OkHttp3ClientHttpRequestFactory /** * Sets the underlying connect timeout in milliseconds. * A value of 0 specifies an infinite timeout. - * @see okhttp3.OkHttpClient.Builder#connectTimeout(long, TimeUnit) + * @see OkHttpClient.Builder#connectTimeout(long, TimeUnit) */ public void setConnectTimeout(int connectTimeout) { this.client = this.client.newBuilder() @@ -127,23 +126,21 @@ public class OkHttp3ClientHttpRequestFactory } - static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, - HttpMethod method) throws MalformedURLException { + static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method) + throws MalformedURLException { okhttp3.MediaType contentType = getContentType(headers); - RequestBody body = (content.length > 0 ? RequestBody.create(contentType, content) : null); - - URL url = uri.toURL(); - String methodName = method.name(); - Request.Builder builder = new Request.Builder().url(url).method(methodName, body); + RequestBody body = (content.length > 0 || + okhttp3.internal.http.HttpMethod.requiresRequestBody(method.name()) ? + RequestBody.create(contentType, content) : null); + Request.Builder builder = new Request.Builder().url(uri.toURL()).method(method.name(), body); for (Map.Entry> entry : headers.entrySet()) { String headerName = entry.getKey(); for (String headerValue : entry.getValue()) { builder.addHeader(headerName, headerValue); } } - return builder.build(); } diff --git a/spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java index d5ae9e97eee..d0522f49782 100644 --- a/spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/OkHttpClientHttpRequestFactory.java @@ -19,7 +19,6 @@ package org.springframework.http.client; import java.io.IOException; import java.net.MalformedURLException; import java.net.URI; -import java.net.URL; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -121,23 +120,21 @@ public class OkHttpClientHttpRequestFactory } - static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, - HttpMethod method) throws MalformedURLException { + static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method) + throws MalformedURLException { com.squareup.okhttp.MediaType contentType = getContentType(headers); - RequestBody body = (content.length > 0 ? RequestBody.create(contentType, content) : null); - - URL url = uri.toURL(); - String methodName = method.name(); - Request.Builder builder = new Request.Builder().url(url).method(methodName, body); + RequestBody body = (content.length > 0 || + com.squareup.okhttp.internal.http.HttpMethod.requiresRequestBody(method.name()) ? + RequestBody.create(contentType, content) : null); + Request.Builder builder = new Request.Builder().url(uri.toURL()).method(method.name(), body); for (Map.Entry> entry : headers.entrySet()) { String headerName = entry.getKey(); for (String headerValue : entry.getValue()) { builder.addHeader(headerName, headerValue); } } - return builder.build(); } diff --git a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java b/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java index c4e4538b301..b703a805baf 100644 --- a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java +++ b/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTestCase.java @@ -1,3 +1,19 @@ +/* + * Copyright 2002-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package org.springframework.web.client; import java.io.EOFException; @@ -15,21 +31,18 @@ import org.junit.Before; import org.springframework.http.MediaType; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * @author Brian Clozel */ public class AbstractMockWebServerTestCase { - protected static final String helloWorld = "H\u00e9llo W\u00f6rld"; + private static final Charset UTF_8 = Charset.forName("UTF-8"); - private static final Charset UTF_8 = Charset.forName("UTF-8"); + private static final Charset ISO_8859_1 = Charset.forName("ISO-8859-1"); - private static final Charset ISO_8859_1 = Charset.forName("ISO-8859-1"); + protected static final String helloWorld = "H\u00e9llo W\u00f6rld"; protected static final MediaType textContentType = new MediaType("text", "plain", Collections.singletonMap("charset", "UTF-8")); @@ -40,6 +53,7 @@ public class AbstractMockWebServerTestCase { protected String baseUrl; + @Before public void setUp() throws Exception { this.server = new MockWebServer(); @@ -54,68 +68,9 @@ public class AbstractMockWebServerTestCase { this.server.shutdown(); } - protected class TestDispatcher extends Dispatcher { - @Override - public MockResponse dispatch(RecordedRequest request) throws InterruptedException { - try { - byte[] helloWorldBytes = helloWorld.getBytes(UTF_8); - - if (request.getPath().equals("/get")) { - return getRequest(request, helloWorldBytes, textContentType.toString()); - } - else if (request.getPath().equals("/get/nothing")) { - return getRequest(request, new byte[0], textContentType.toString()); - } - else if (request.getPath().equals("/get/nocontenttype")) { - return getRequest(request, helloWorldBytes, null); - } - else if (request.getPath().equals("/post")) { - return postRequest(request, helloWorld, "/post/1", textContentType.toString(), helloWorldBytes); - } - else if (request.getPath().equals("/jsonpost")) { - return jsonPostRequest(request, "/jsonpost/1", "application/json; charset=utf-8"); - } - else if (request.getPath().equals("/status/nocontent")) { - return new MockResponse().setResponseCode(204); - } - else if (request.getPath().equals("/status/notmodified")) { - return new MockResponse().setResponseCode(304); - } - else if (request.getPath().equals("/status/notfound")) { - return new MockResponse().setResponseCode(404); - } - else if (request.getPath().equals("/status/server")) { - return new MockResponse().setResponseCode(500); - } - else if (request.getPath().contains("/uri/")) { - return new MockResponse().setBody(request.getPath()).setHeader("Content-Type", "text/plain"); - } - else if (request.getPath().equals("/multipart")) { - return multipartRequest(request); - } - else if (request.getPath().equals("/form")) { - return formRequest(request); - } - else if (request.getPath().equals("/delete")) { - return new MockResponse().setResponseCode(200); - } - else if (request.getPath().equals("/patch")) { - return patchRequest(request, helloWorld, textContentType.toString(), helloWorldBytes); - } - else if (request.getPath().equals("/put")) { - return putRequest(request, helloWorld); - } - return new MockResponse().setResponseCode(404); - } - catch (Throwable exc) { - return new MockResponse().setResponseCode(500).setBody(exc.toString()); - } - } - } - private MockResponse getRequest(RecordedRequest request, byte[] body, String contentType) { - if(request.getMethod().equals("OPTIONS")) { + if (request.getMethod().equals("OPTIONS")) { return new MockResponse().setResponseCode(200).setHeader("Allow", "GET, OPTIONS, HEAD, TRACE"); } Buffer buf = new Buffer(); @@ -138,7 +93,7 @@ public class AbstractMockWebServerTestCase { String requestContentType = request.getHeader("Content-Type"); assertNotNull("No content-type", requestContentType); Charset charset = ISO_8859_1; - if(requestContentType.indexOf("charset=") > -1) { + if (requestContentType.contains("charset=")) { String charsetName = requestContentType.split("charset=")[1]; charset = Charset.forName(charsetName); } @@ -154,10 +109,11 @@ public class AbstractMockWebServerTestCase { } private MockResponse jsonPostRequest(RecordedRequest request, String location, String contentType) { - - assertTrue("Invalid request content-length", - Integer.parseInt(request.getHeader("Content-Length")) > 0); - assertNotNull("No content-type", request.getHeader("Content-Type")); + if (request.getBodySize() > 0) { + assertTrue("Invalid request content-length", + Integer.parseInt(request.getHeader("Content-Length")) > 0); + assertNotNull("No content-type", request.getHeader("Content-Type")); + } return new MockResponse() .setHeader("Location", baseUrl + location) .setHeader("Content-Type", contentType) @@ -177,8 +133,8 @@ public class AbstractMockWebServerTestCase { assertPart(body, "form-data", boundary, "name 2", "text/plain", "value 2+2"); assertFilePart(body, "form-data", boundary, "logo", "logo.jpg", "image/jpeg"); } - catch (EOFException e) { - throw new RuntimeException(e); + catch (EOFException ex) { + throw new IllegalStateException(ex); } return new MockResponse().setResponseCode(200); } @@ -221,13 +177,14 @@ public class AbstractMockWebServerTestCase { private MockResponse patchRequest(RecordedRequest request, String expectedRequestContent, String contentType, byte[] responseBody) { + assertEquals("PATCH", request.getMethod()); assertTrue("Invalid request content-length", Integer.parseInt(request.getHeader("Content-Length")) > 0); String requestContentType = request.getHeader("Content-Type"); assertNotNull("No content-type", requestContentType); Charset charset = ISO_8859_1; - if(requestContentType.indexOf("charset=") > -1) { + if (requestContentType.contains("charset=")) { String charsetName = requestContentType.split("charset=")[1]; charset = Charset.forName(charsetName); } @@ -246,7 +203,7 @@ public class AbstractMockWebServerTestCase { String requestContentType = request.getHeader("Content-Type"); assertNotNull("No content-type", requestContentType); Charset charset = ISO_8859_1; - if(requestContentType.indexOf("charset=") > -1) { + if (requestContentType.contains("charset=")) { String charsetName = requestContentType.split("charset=")[1]; charset = Charset.forName(charsetName); } @@ -254,4 +211,65 @@ public class AbstractMockWebServerTestCase { return new MockResponse().setResponseCode(202); } + + protected class TestDispatcher extends Dispatcher { + + @Override + public MockResponse dispatch(RecordedRequest request) throws InterruptedException { + try { + byte[] helloWorldBytes = helloWorld.getBytes(UTF_8); + + if (request.getPath().equals("/get")) { + return getRequest(request, helloWorldBytes, textContentType.toString()); + } + else if (request.getPath().equals("/get/nothing")) { + return getRequest(request, new byte[0], textContentType.toString()); + } + else if (request.getPath().equals("/get/nocontenttype")) { + return getRequest(request, helloWorldBytes, null); + } + else if (request.getPath().equals("/post")) { + return postRequest(request, helloWorld, "/post/1", textContentType.toString(), helloWorldBytes); + } + else if (request.getPath().equals("/jsonpost")) { + return jsonPostRequest(request, "/jsonpost/1", "application/json; charset=utf-8"); + } + else if (request.getPath().equals("/status/nocontent")) { + return new MockResponse().setResponseCode(204); + } + else if (request.getPath().equals("/status/notmodified")) { + return new MockResponse().setResponseCode(304); + } + else if (request.getPath().equals("/status/notfound")) { + return new MockResponse().setResponseCode(404); + } + else if (request.getPath().equals("/status/server")) { + return new MockResponse().setResponseCode(500); + } + else if (request.getPath().contains("/uri/")) { + return new MockResponse().setBody(request.getPath()).setHeader("Content-Type", "text/plain"); + } + else if (request.getPath().equals("/multipart")) { + return multipartRequest(request); + } + else if (request.getPath().equals("/form")) { + return formRequest(request); + } + else if (request.getPath().equals("/delete")) { + return new MockResponse().setResponseCode(200); + } + else if (request.getPath().equals("/patch")) { + return patchRequest(request, helloWorld, textContentType.toString(), helloWorldBytes); + } + else if (request.getPath().equals("/put")) { + return putRequest(request, helloWorld); + } + return new MockResponse().setResponseCode(404); + } + catch (Throwable ex) { + return new MockResponse().setResponseCode(500).setBody(ex.toString()); + } + } + } + } diff --git a/spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java b/spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java index 28629cf1b09..9bcea2cc247 100644 --- a/spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java @@ -50,6 +50,7 @@ import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; import org.springframework.http.client.Netty4ClientHttpRequestFactory; import org.springframework.http.client.OkHttp3ClientHttpRequestFactory; +import org.springframework.http.client.OkHttpClientHttpRequestFactory; import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.http.converter.json.MappingJacksonValue; import org.springframework.util.LinkedMultiValueMap; @@ -72,10 +73,11 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase @Parameterized.Parameters public static Iterable data() { return Arrays.asList( + new SimpleClientHttpRequestFactory(), new HttpComponentsClientHttpRequestFactory(), new Netty4ClientHttpRequestFactory(), new OkHttp3ClientHttpRequestFactory(), - new SimpleClientHttpRequestFactory() + new OkHttpClientHttpRequestFactory() ); } @@ -141,7 +143,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase public void postForLocationEntity() throws URISyntaxException { HttpHeaders entityHeaders = new HttpHeaders(); entityHeaders.setContentType(new MediaType("text", "plain", Charset.forName("ISO-8859-15"))); - HttpEntity entity = new HttpEntity(helloWorld, entityHeaders); + HttpEntity entity = new HttpEntity<>(helloWorld, entityHeaders); URI location = template.postForLocation(baseUrl + "/{method}", entity, "post"); assertEquals("Invalid location", new URI(baseUrl + "/post/1"), location); } @@ -199,7 +201,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase @Test public void multipart() throws UnsupportedEncodingException { - MultiValueMap parts = new LinkedMultiValueMap(); + MultiValueMap parts = new LinkedMultiValueMap<>(); parts.add("name 1", "value 1"); parts.add("name 2", "value 2+1"); parts.add("name 2", "value 2+2"); @@ -211,7 +213,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase @Test public void form() throws UnsupportedEncodingException { - MultiValueMap form = new LinkedMultiValueMap(); + MultiValueMap form = new LinkedMultiValueMap<>(); form.add("name 1", "value 1"); form.add("name 2", "value 2+1"); form.add("name 2", "value 2+2"); @@ -223,7 +225,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase public void exchangeGet() throws Exception { HttpHeaders requestHeaders = new HttpHeaders(); requestHeaders.set("MyHeader", "MyValue"); - HttpEntity requestEntity = new HttpEntity(requestHeaders); + HttpEntity requestEntity = new HttpEntity<>(requestHeaders); ResponseEntity response = template.exchange(baseUrl + "/{method}", HttpMethod.GET, requestEntity, String.class, "get"); assertEquals("Invalid content", helloWorld, response.getBody()); @@ -234,7 +236,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase HttpHeaders requestHeaders = new HttpHeaders(); requestHeaders.set("MyHeader", "MyValue"); requestHeaders.setContentType(MediaType.TEXT_PLAIN); - HttpEntity requestEntity = new HttpEntity(helloWorld, requestHeaders); + HttpEntity requestEntity = new HttpEntity<>(helloWorld, requestHeaders); HttpEntity result = template.exchange(baseUrl + "/{method}", HttpMethod.POST, requestEntity, Void.class, "post"); assertEquals("Invalid location", new URI(baseUrl + "/post/1"), result.getHeaders().getLocation()); assertFalse(result.hasBody()); @@ -248,7 +250,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase bean.setWith1("with"); bean.setWith2("with"); bean.setWithout("without"); - HttpEntity entity = new HttpEntity(bean, entityHeaders); + HttpEntity entity = new HttpEntity<>(bean, entityHeaders); String s = template.postForObject(baseUrl + "/jsonpost", entity, String.class); assertTrue(s.contains("\"with1\":\"with\"")); assertTrue(s.contains("\"with2\":\"with\"")); @@ -262,7 +264,7 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase MySampleBean bean = new MySampleBean("with", "with", "without"); MappingJacksonValue jacksonValue = new MappingJacksonValue(bean); jacksonValue.setSerializationView(MyJacksonView1.class); - HttpEntity entity = new HttpEntity(jacksonValue, entityHeaders); + HttpEntity entity = new HttpEntity<>(jacksonValue, entityHeaders); String s = template.postForObject(baseUrl + "/jsonpost", entity, String.class); assertTrue(s.contains("\"with1\":\"with\"")); assertFalse(s.contains("\"with2\":\"with\"")); @@ -290,10 +292,15 @@ public class RestTemplateIntegrationTests extends AbstractMockWebServerTestCase assertTrue(content.contains("\"type\":\"bar\"")); } + @Test // SPR-15015 + public void postWithoutBody() throws Exception { + assertNull(template.postForObject(baseUrl + "/jsonpost", null, String.class)); + } + - public interface MyJacksonView1 {}; + public interface MyJacksonView1 {} - public interface MyJacksonView2 {}; + public interface MyJacksonView2 {} public static class MySampleBean {