From c42671a78aa68f7ab8125311b2c9ce167b166c2c Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Tue, 14 Jun 2011 10:37:49 +0000 Subject: [PATCH] SPR-7911 - Better handling of 204 No Content in RestTemplate --- .../client/HttpMessageConverterExtractor.java | 29 ++++++++- .../client/RestTemplateIntegrationTests.java | 60 ++++++++++++++++--- 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/org.springframework.web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java b/org.springframework.web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java index 01e4877ed49..7dd17bae422 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java +++ b/org.springframework.web/src/main/java/org/springframework/web/client/HttpMessageConverterExtractor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2011 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. @@ -22,6 +22,7 @@ import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.converter.HttpMessageConverter; @@ -61,9 +62,15 @@ public class HttpMessageConverterExtractor implements ResponseExtractor { @SuppressWarnings("unchecked") public T extractData(ClientHttpResponse response) throws IOException { + if (!hasMessageBody(response)) { + return null; + } MediaType contentType = response.getHeaders().getContentType(); if (contentType == null) { - throw new RestClientException("Cannot extract response: no Content-Type found"); + if (logger.isTraceEnabled()) { + logger.trace("No Content-Type header found, defaulting to application/octet-stream"); + } + contentType = MediaType.APPLICATION_OCTET_STREAM; } for (HttpMessageConverter messageConverter : messageConverters) { if (messageConverter.canRead(responseType, contentType)) { @@ -79,4 +86,22 @@ public class HttpMessageConverterExtractor implements ResponseExtractor { this.responseType.getName() + "] and content type [" + contentType + "]"); } + /** + * Indicates whether the given response has a message body. + *

Default implementation returns {@code false} for a response status of {@code 204} or {@code 304}, or a + * {@code Content-Length} of {@code 0}. + * + * @param response the response to check for a message body + * @return {@code true} if the response has a body, {@code false} otherwise + * @throws IOException in case of I/O errors + */ + protected boolean hasMessageBody(ClientHttpResponse response) throws IOException { + HttpStatus responseStatus = response.getStatusCode(); + if (responseStatus == HttpStatus.NO_CONTENT || responseStatus == HttpStatus.NOT_MODIFIED) { + return false; + } + long contentLength = response.getHeaders().getContentLength(); + return contentLength != 0; + } + } diff --git a/org.springframework.web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java b/org.springframework.web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java index 16682b0a68a..a4838a27675 100644 --- a/org.springframework.web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java +++ b/org.springframework.web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java @@ -85,11 +85,14 @@ public class RestTemplateIntegrationTests { contentType = new MediaType("text", "plain", Collections.singletonMap("charset", "utf-8")); jettyContext.addServlet(new ServletHolder(new GetServlet(bytes, contentType)), "/get"); jettyContext.addServlet(new ServletHolder(new GetServlet(new byte[0], contentType)), "/get/nothing"); + jettyContext.addServlet(new ServletHolder(new GetServlet(bytes, null)), "/get/nocontenttype"); jettyContext.addServlet( new ServletHolder(new PostServlet(helloWorld, baseUrl + "/post/1", bytes, contentType)), "/post"); - jettyContext.addServlet(new ServletHolder(new ErrorServlet(404)), "/errors/notfound"); - jettyContext.addServlet(new ServletHolder(new ErrorServlet(500)), "/errors/server"); + jettyContext.addServlet(new ServletHolder(new StatusCodeServlet(204)), "/status/nocontent"); + jettyContext.addServlet(new ServletHolder(new StatusCodeServlet(304)), "/status/notmodified"); + jettyContext.addServlet(new ServletHolder(new ErrorServlet(404)), "/status/notfound"); + jettyContext.addServlet(new ServletHolder(new ErrorServlet(500)), "/status/server"); jettyContext.addServlet(new ServletHolder(new UriServlet()), "/uri/*"); jettyContext.addServlet(new ServletHolder(new MultipartServlet()), "/multipart"); jettyServer.start(); @@ -125,7 +128,33 @@ public class RestTemplateIntegrationTests { @Test public void getNoResponse() { String s = template.getForObject(baseUrl + "/get/nothing", String.class); - assertEquals("Invalid content", "", s); + assertNull("Invalid content", s); + } + + @Test + public void getNoContentTypeHeader() throws UnsupportedEncodingException { + byte[] bytes = template.getForObject(baseUrl + "/get/nocontenttype", byte[].class); + assertArrayEquals("Invalid content", helloWorld.getBytes("UTF-8"), bytes); + } + + @Test + public void getNoContent() { + String s = template.getForObject(baseUrl + "/status/nocontent", String.class); + assertNull("Invalid content", s); + + ResponseEntity entity = template.getForEntity(baseUrl + "/status/nocontent", String.class); + assertEquals("Invalid response code", HttpStatus.NO_CONTENT, entity.getStatusCode()); + assertNull("Invalid content", entity.getBody()); + } + + @Test + public void getNotModified() { + String s = template.getForObject(baseUrl + "/status/notmodified", String.class); + assertNull("Invalid content", s); + + ResponseEntity entity = template.getForEntity(baseUrl + "/status/notmodified", String.class); + assertEquals("Invalid response code", HttpStatus.NOT_MODIFIED, entity.getStatusCode()); + assertNull("Invalid content", entity.getBody()); } @Test @@ -152,7 +181,7 @@ public class RestTemplateIntegrationTests { @Test public void notFound() { try { - template.execute(baseUrl + "/errors/notfound", HttpMethod.GET, null, null); + template.execute(baseUrl + "/status/notfound", HttpMethod.GET, null, null); fail("HttpClientErrorException expected"); } catch (HttpClientErrorException ex) { @@ -165,7 +194,7 @@ public class RestTemplateIntegrationTests { @Test public void serverError() { try { - template.execute(baseUrl + "/errors/server", HttpMethod.GET, null, null); + template.execute(baseUrl + "/status/server", HttpMethod.GET, null, null); fail("HttpServerErrorException expected"); } catch (HttpServerErrorException ex) { @@ -227,7 +256,22 @@ public class RestTemplateIntegrationTests { assertFalse(result.hasBody()); } - /** Servlet that returns and error message for a given status code. */ + /** Servlet that sets the given status code. */ + private static class StatusCodeServlet extends GenericServlet { + + private final int sc; + + private StatusCodeServlet(int sc) { + this.sc = sc; + } + + @Override + public void service(ServletRequest request, ServletResponse response) throws ServletException, IOException { + ((HttpServletResponse) response).setStatus(sc); + } + } + + /** Servlet that returns an error message for a given status code. */ private static class ErrorServlet extends GenericServlet { private final int sc; @@ -256,7 +300,9 @@ public class RestTemplateIntegrationTests { @Override protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - response.setContentType(contentType.toString()); + if (contentType != null) { + response.setContentType(contentType.toString()); + } response.setContentLength(buf.length); FileCopyUtils.copy(buf, response.getOutputStream()); }