Improve handling of the Content-Length header
Previously, some MockMvc-specific logic would add a Content-Length header to every request that had content. This led to the curl request snippet containing a -H option for the Content-Length header. This is unnecessary as curl will automatically generate a Content-Length header based on the data that's being sent to the server. A secondary problem was the inconsistent automatic addition of a Content-Length header; the header was not automatically added to responses. This commit remove the MockMvc-specific logic in favour of some new logic in the core project to automatically add a Content-Length header to both requests and responses. The curl request snippet has been updated to supress the header in favour of curl's automatic generation. Closes gh-111
This commit is contained in:
@@ -115,8 +115,10 @@ public class CurlRequestSnippet extends TemplatedSnippet {
|
||||
|
||||
private void writeHeaders(HttpHeaders headers, PrintWriter writer) {
|
||||
for (Entry<String, List<String>> entry : headers.entrySet()) {
|
||||
for (String header : entry.getValue()) {
|
||||
writer.print(String.format(" -H '%s: %s'", entry.getKey(), header));
|
||||
if (!HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(entry.getKey())) {
|
||||
for (String header : entry.getValue()) {
|
||||
writer.print(String.format(" -H '%s: %s'", entry.getKey(), header));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,7 +35,18 @@ abstract class AbstractOperationMessage {
|
||||
|
||||
AbstractOperationMessage(byte[] content, HttpHeaders headers) {
|
||||
this.content = content == null ? new byte[0] : content;
|
||||
this.headers = headers;
|
||||
this.headers = createHeaders(content, headers);
|
||||
}
|
||||
|
||||
private static HttpHeaders createHeaders(byte[] content, HttpHeaders input) {
|
||||
HttpHeaders headers = new HttpHeaders();
|
||||
if (input != null) {
|
||||
headers.putAll(input);
|
||||
}
|
||||
if (content != null && content.length > 0 && headers.getContentLength() == -1) {
|
||||
headers.setContentLength(content.length);
|
||||
}
|
||||
return headers;
|
||||
}
|
||||
|
||||
public byte[] getContent() {
|
||||
|
||||
@@ -63,9 +63,12 @@ public class ContentModifyingOperationPreprocessor implements OperationPreproces
|
||||
private HttpHeaders getUpdatedHeaders(HttpHeaders headers, byte[] updatedContent) {
|
||||
HttpHeaders updatedHeaders = new HttpHeaders();
|
||||
updatedHeaders.putAll(headers);
|
||||
if (updatedHeaders.getContentLength() > -1) {
|
||||
if (updatedContent.length > 0) {
|
||||
updatedHeaders.setContentLength(updatedContent.length);
|
||||
}
|
||||
else {
|
||||
updatedHeaders.remove(HttpHeaders.CONTENT_LENGTH);
|
||||
}
|
||||
return updatedHeaders;
|
||||
}
|
||||
|
||||
|
||||
@@ -75,30 +75,33 @@ public class HttpRequestSnippetTests {
|
||||
|
||||
@Test
|
||||
public void postRequestWithContent() throws IOException {
|
||||
String content = "Hello, world";
|
||||
this.snippet.expectHttpRequest("post-request-with-content").withContents(
|
||||
httpRequest(RequestMethod.POST, "/foo").header(HttpHeaders.HOST,
|
||||
"localhost").content("Hello, world"));
|
||||
httpRequest(RequestMethod.POST, "/foo")
|
||||
.header(HttpHeaders.HOST, "localhost").content(content)
|
||||
.header(HttpHeaders.CONTENT_LENGTH, content.getBytes().length));
|
||||
|
||||
new HttpRequestSnippet().document(new OperationBuilder(
|
||||
"post-request-with-content", this.snippet.getOutputDirectory())
|
||||
.request("http://localhost/foo").method("POST").content("Hello, world")
|
||||
.build());
|
||||
.request("http://localhost/foo").method("POST").content(content).build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void postRequestWithCharset() throws IOException {
|
||||
String japaneseContent = "\u30b3\u30f3\u30c6\u30f3\u30c4";
|
||||
byte[] contentBytes = japaneseContent.getBytes("UTF-8");
|
||||
this.snippet.expectHttpRequest("post-request-with-charset").withContents(
|
||||
httpRequest(RequestMethod.POST, "/foo")
|
||||
.header(HttpHeaders.HOST, "localhost")
|
||||
.header("Content-Type", "text/plain;charset=UTF-8")
|
||||
.header(HttpHeaders.CONTENT_LENGTH, contentBytes.length)
|
||||
.content(japaneseContent));
|
||||
|
||||
new HttpRequestSnippet().document(new OperationBuilder(
|
||||
"post-request-with-charset", this.snippet.getOutputDirectory())
|
||||
.request("http://localhost/foo").method("POST")
|
||||
.header("Content-Type", "text/plain;charset=UTF-8")
|
||||
.content(japaneseContent.getBytes("UTF-8")).build());
|
||||
.header("Content-Type", "text/plain;charset=UTF-8").content(contentBytes)
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -117,14 +120,15 @@ public class HttpRequestSnippetTests {
|
||||
|
||||
@Test
|
||||
public void putRequestWithContent() throws IOException {
|
||||
String content = "Hello, world";
|
||||
this.snippet.expectHttpRequest("put-request-with-content").withContents(
|
||||
httpRequest(RequestMethod.PUT, "/foo").header(HttpHeaders.HOST,
|
||||
"localhost").content("Hello, world"));
|
||||
httpRequest(RequestMethod.PUT, "/foo")
|
||||
.header(HttpHeaders.HOST, "localhost").content(content)
|
||||
.header(HttpHeaders.CONTENT_LENGTH, content.getBytes().length));
|
||||
|
||||
new HttpRequestSnippet().document(new OperationBuilder(
|
||||
"put-request-with-content", this.snippet.getOutputDirectory())
|
||||
.request("http://localhost/foo").method("PUT").content("Hello, world")
|
||||
.build());
|
||||
.request("http://localhost/foo").method("PUT").content(content).build());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -78,22 +78,27 @@ public class HttpResponseSnippetTests {
|
||||
|
||||
@Test
|
||||
public void responseWithContent() throws IOException {
|
||||
String content = "content";
|
||||
this.snippet.expectHttpResponse("response-with-content").withContents(
|
||||
httpResponse(HttpStatus.OK).content("content"));
|
||||
httpResponse(HttpStatus.OK).content(content).header(
|
||||
HttpHeaders.CONTENT_LENGTH, content.getBytes().length));
|
||||
new HttpResponseSnippet().document(new OperationBuilder("response-with-content",
|
||||
this.snippet.getOutputDirectory()).response().content("content").build());
|
||||
this.snippet.getOutputDirectory()).response().content(content).build());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void responseWithCharset() throws IOException {
|
||||
String japaneseContent = "\u30b3\u30f3\u30c6\u30f3\u30c4";
|
||||
byte[] contentBytes = japaneseContent.getBytes("UTF-8");
|
||||
this.snippet.expectHttpResponse("response-with-charset").withContents(
|
||||
httpResponse(HttpStatus.OK).header("Content-Type",
|
||||
"text/plain;charset=UTF-8").content(japaneseContent));
|
||||
httpResponse(HttpStatus.OK)
|
||||
.header("Content-Type", "text/plain;charset=UTF-8")
|
||||
.content(japaneseContent)
|
||||
.header(HttpHeaders.CONTENT_LENGTH, contentBytes.length));
|
||||
new HttpResponseSnippet().document(new OperationBuilder("response-with-charset",
|
||||
this.snippet.getOutputDirectory()).response()
|
||||
.header("Content-Type", "text/plain;charset=UTF-8")
|
||||
.content(japaneseContent.getBytes("UTF-8")).build());
|
||||
.header("Content-Type", "text/plain;charset=UTF-8").content(contentBytes)
|
||||
.build());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -71,16 +71,6 @@ public class ContentModifyingOperationPreprocessorTests {
|
||||
assertThat(preprocessed.getContent(), is(equalTo("modified".getBytes())));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void unknownContentLengthIsUnchanged() {
|
||||
StandardOperationRequest request = new StandardOperationRequest(
|
||||
URI.create("http://localhost"), HttpMethod.GET, "content".getBytes(),
|
||||
new HttpHeaders(), new Parameters(),
|
||||
Collections.<OperationRequestPart>emptyList());
|
||||
OperationRequest preprocessed = this.preprocessor.preprocess(request);
|
||||
assertThat(preprocessed.getHeaders().getContentLength(), is(equalTo(-1L)));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void contentLengthIsUpdated() {
|
||||
HttpHeaders httpHeaders = new HttpHeaders();
|
||||
|
||||
@@ -164,6 +164,12 @@ public final class SnippetMatchers {
|
||||
return (T) this;
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
public T header(String name, long value) {
|
||||
this.addLine(this.headerOffset++, name + ": " + value);
|
||||
return (T) this;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -29,7 +29,6 @@ import org.springframework.test.web.servlet.request.RequestPostProcessor;
|
||||
import org.springframework.test.web.servlet.setup.ConfigurableMockMvcBuilder;
|
||||
import org.springframework.test.web.servlet.setup.MockMvcConfigurer;
|
||||
import org.springframework.test.web.servlet.setup.MockMvcConfigurerAdapter;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.context.WebApplicationContext;
|
||||
|
||||
/**
|
||||
@@ -62,8 +61,7 @@ public class RestDocumentationMockMvcConfigurer extends MockMvcConfigurerAdapter
|
||||
RestDocumentationMockMvcConfigurer(RestDocumentation restDocumentation) {
|
||||
this.requestPostProcessor = new ConfigurerApplyingRequestPostProcessor(
|
||||
restDocumentation, this.uriConfigurer, this.writerResolverConfigurer,
|
||||
this.snippetConfigurer, new ContentLengthHeaderConfigurer(),
|
||||
this.templateEngineConfigurer);
|
||||
this.snippetConfigurer, this.templateEngineConfigurer);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -115,19 +113,6 @@ public class RestDocumentationMockMvcConfigurer extends MockMvcConfigurerAdapter
|
||||
return this.requestPostProcessor;
|
||||
}
|
||||
|
||||
private static final class ContentLengthHeaderConfigurer extends AbstractConfigurer {
|
||||
|
||||
@Override
|
||||
void apply(MockHttpServletRequest request) {
|
||||
long contentLength = request.getContentLengthLong();
|
||||
if (contentLength > 0
|
||||
&& !StringUtils.hasText(request.getHeader("Content-Length"))) {
|
||||
request.addHeader("Content-Length", request.getContentLengthLong());
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private static final class TemplateEngineConfigurer extends AbstractConfigurer {
|
||||
|
||||
private TemplateEngine templateEngine = new MustacheTemplateEngine(
|
||||
|
||||
@@ -157,7 +157,8 @@ public class MockMvcOperationRequestFactoryTests {
|
||||
OperationRequestPart part = request.getParts().iterator().next();
|
||||
assertThat(part.getName(), is(equalTo("file")));
|
||||
assertThat(part.getSubmittedFileName(), is(nullValue()));
|
||||
assertThat(part.getHeaders().isEmpty(), is(true));
|
||||
assertThat(part.getHeaders().size(), is(1));
|
||||
assertThat(part.getHeaders().getContentLength(), is(4L));
|
||||
assertThat(part.getContent(), is(equalTo(new byte[] { 1, 2, 3, 4 })));
|
||||
}
|
||||
|
||||
|
||||
@@ -79,6 +79,7 @@ import static org.springframework.restdocs.test.SnippetMatchers.codeBlock;
|
||||
import static org.springframework.restdocs.test.SnippetMatchers.httpRequest;
|
||||
import static org.springframework.restdocs.test.SnippetMatchers.httpResponse;
|
||||
import static org.springframework.restdocs.test.SnippetMatchers.snippet;
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
|
||||
|
||||
/**
|
||||
@@ -120,6 +121,21 @@ public class MockMvcRestDocumentationIntegrationTests {
|
||||
"http-request.adoc", "http-response.adoc", "curl-request.adoc");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void curlSnippetWithContent() throws Exception {
|
||||
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context)
|
||||
.apply(documentationConfiguration(this.restDocumentation)).build();
|
||||
|
||||
mockMvc.perform(post("/").accept(MediaType.APPLICATION_JSON).content("content"))
|
||||
.andExpect(status().isOk()).andDo(document("curl-snippet-with-content"));
|
||||
assertThat(new File(
|
||||
"build/generated-snippets/curl-snippet-with-content/curl-request.adoc"),
|
||||
is(snippet().withContents(
|
||||
codeBlock("bash").content(
|
||||
"$ curl " + "'http://localhost:8080/' -i -X POST "
|
||||
+ "-H 'Accept: application/json' -d 'content'"))));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void linksSnippet() throws Exception {
|
||||
MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context)
|
||||
@@ -298,24 +314,27 @@ public class MockMvcRestDocumentationIntegrationTests {
|
||||
removeHeaders("a"),
|
||||
replacePattern(pattern, "\"<<beta>>\""))));
|
||||
|
||||
String original = "{\"a\":\"alpha\",\"links\":[{\"rel\":\"rel\","
|
||||
+ "\"href\":\"href\"}]}";
|
||||
assertThat(
|
||||
new File("build/generated-snippets/original-response/http-response.adoc"),
|
||||
is(snippet().withContents(
|
||||
httpResponse(HttpStatus.OK)
|
||||
.header("a", "alpha")
|
||||
.header("Content-Type", "application/json")
|
||||
.content(
|
||||
"{\"a\":\"alpha\",\"links\":[{\"rel\":\"rel\","
|
||||
+ "\"href\":\"href\"}]}"))));
|
||||
.header(HttpHeaders.CONTENT_LENGTH,
|
||||
original.getBytes().length).content(original))));
|
||||
String prettyPrinted = String.format("{%n \"a\" : \"<<beta>>\",%n \"links\" : "
|
||||
+ "[ {%n \"rel\" : \"rel\",%n \"href\" : \"...\"%n } ]%n}");
|
||||
assertThat(
|
||||
new File(
|
||||
"build/generated-snippets/preprocessed-response/http-response.adoc"),
|
||||
is(snippet().withContents(
|
||||
httpResponse(HttpStatus.OK).header("Content-Type",
|
||||
"application/json").content(
|
||||
String.format("{%n \"a\" : \"<<beta>>\",%n \"links\" :"
|
||||
+ " [ {%n \"rel\" : \"rel\",%n \"href\" :"
|
||||
+ " \"...\"%n } ]%n}")))));
|
||||
httpResponse(HttpStatus.OK)
|
||||
.header("Content-Type", "application/json")
|
||||
.header(HttpHeaders.CONTENT_LENGTH,
|
||||
prettyPrinted.getBytes().length)
|
||||
.content(prettyPrinted))));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -27,7 +27,6 @@ import org.springframework.test.web.servlet.request.RequestPostProcessor;
|
||||
import org.springframework.web.context.request.RequestContextHolder;
|
||||
import org.springframework.web.context.request.ServletRequestAttributes;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.equalTo;
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
import static org.hamcrest.CoreMatchers.nullValue;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
@@ -94,17 +93,6 @@ public class RestDocumentationConfigurerTests {
|
||||
assertThat(this.request.getHeader("Content-Length"), is(nullValue()));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void contentLengthHeaderIsSetWhenRequestHasContent() {
|
||||
RequestPostProcessor postProcessor = new RestDocumentationMockMvcConfigurer(
|
||||
this.restDocumentation).beforeMockMvcCreated(null, null);
|
||||
byte[] content = "Hello, world".getBytes();
|
||||
this.request.setContent(content);
|
||||
postProcessor.postProcessRequest(this.request);
|
||||
assertThat(this.request.getHeader("Content-Length"),
|
||||
is(equalTo(Integer.toString(content.length))));
|
||||
}
|
||||
|
||||
private void assertUriConfiguration(String scheme, String host, int port) {
|
||||
assertEquals(scheme, this.request.getScheme());
|
||||
assertEquals(host, this.request.getServerName());
|
||||
|
||||
Reference in New Issue
Block a user