From 8ca1dfa1ad7e84c4abde73db62a398bb1b9bed48 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 23 Jun 2016 17:53:32 +0100 Subject: [PATCH] Improve handling of requests with both parameters and content Previously, if a MockMvc request was made with parameters and body content, the parameters were omitted from the resulting curl, HTTPie and HTTP request snippets. This commit updates the affected snippets to ensure that the parameters are included. The user-provided content is interpreted as indicating that the parameters should be sent in the query string rather than as form-encoded content. Closes gh-239 --- .../restdocs/cli/CurlRequestSnippet.java | 11 ++++++- .../restdocs/cli/HttpieRequestSnippet.java | 14 ++++++-- .../restdocs/http/HttpRequestSnippet.java | 28 +++++++++++++--- .../restdocs/cli/CurlRequestSnippetTests.java | 21 ++++++++++++ .../cli/HttpieRequestSnippetTests.java | 20 ++++++++++++ .../http/HttpRequestSnippetTests.java | 25 +++++++++++++++ ...kMvcRestDocumentationIntegrationTests.java | 32 +++++++++++++++++++ 7 files changed, 144 insertions(+), 7 deletions(-) diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/CurlRequestSnippet.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/CurlRequestSnippet.java index 73eeb72a..31127c3c 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/CurlRequestSnippet.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/CurlRequestSnippet.java @@ -69,7 +69,16 @@ public class CurlRequestSnippet extends TemplatedSnippet { } private String getUrl(Operation operation) { - return String.format("'%s'", operation.getRequest().getUri()); + OperationRequest request = operation.getRequest(); + if (!request.getParameters().isEmpty() && includeParametersInUri(request)) { + return String.format("'%s?%s'", request.getUri(), + request.getParameters().toQueryString()); + } + return String.format("'%s'", request.getUri()); + } + + private boolean includeParametersInUri(OperationRequest request) { + return request.getMethod() == HttpMethod.GET || request.getContent().length > 0; } private String getOptions(Operation operation) { diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/HttpieRequestSnippet.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/HttpieRequestSnippet.java index 57673b9c..4c95c04d 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/HttpieRequestSnippet.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/HttpieRequestSnippet.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.Map.Entry; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.restdocs.operation.Operation; import org.springframework.restdocs.operation.OperationRequest; @@ -89,7 +90,11 @@ public class HttpieRequestSnippet extends TemplatedSnippet { return options.toString(); } - private String getUrl(OperationRequest request) { + private String getUrl(CliOperationRequest request) { + if (!request.getUniqueParameters().isEmpty() && includeParametersInUri(request)) { + return String.format("'%s?%s'", request.getUri(), + request.getParameters().toQueryString()); + } return String.format("'%s'", request.getUri()); } @@ -103,11 +108,16 @@ public class HttpieRequestSnippet extends TemplatedSnippet { } private void writeOptions(CliOperationRequest request, PrintWriter writer) { - if (!request.getParts().isEmpty() || !request.getUniqueParameters().isEmpty()) { + if (!request.getParts().isEmpty() || (!request.getUniqueParameters().isEmpty() + && !includeParametersInUri(request))) { writer.print("--form "); } } + private boolean includeParametersInUri(CliOperationRequest request) { + return request.getMethod() == HttpMethod.GET || request.getContent().length > 0; + } + private void writeUserOptionIfNecessary(CliOperationRequest request, PrintWriter writer) { String credentials = request.getBasicAuthCredentials(); diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/http/HttpRequestSnippet.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/http/HttpRequestSnippet.java index cd855f7b..c966f137 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/http/HttpRequestSnippet.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/http/HttpRequestSnippet.java @@ -66,14 +66,33 @@ public class HttpRequestSnippet extends TemplatedSnippet { protected Map createModel(Operation operation) { Map model = new HashMap<>(); model.put("method", operation.getRequest().getMethod()); - model.put("path", operation.getRequest().getUri().getRawPath() - + (StringUtils.hasText(operation.getRequest().getUri().getRawQuery()) - ? "?" + operation.getRequest().getUri().getRawQuery() : "")); + model.put("path", getPath(operation.getRequest())); model.put("headers", getHeaders(operation.getRequest())); model.put("requestBody", getRequestBody(operation.getRequest())); return model; } + private String getPath(OperationRequest request) { + String path = request.getUri().getRawPath(); + String queryString = request.getUri().getRawQuery(); + if (!request.getParameters().isEmpty() && includeParametersInUri(request)) { + if (StringUtils.hasText(queryString)) { + queryString = queryString + "&" + request.getParameters().toQueryString(); + } + else { + queryString = request.getParameters().toQueryString(); + } + } + if (StringUtils.hasText(queryString)) { + path = path + "?" + queryString; + } + return path; + } + + private boolean includeParametersInUri(OperationRequest request) { + return request.getMethod() == HttpMethod.GET || request.getContent().length > 0; + } + private List> getHeaders(OperationRequest request) { List> headers = new ArrayList<>(); @@ -172,7 +191,8 @@ public class HttpRequestSnippet extends TemplatedSnippet { private boolean requiresFormEncodingContentTypeHeader(OperationRequest request) { return request.getHeaders().get(HttpHeaders.CONTENT_TYPE) == null - && isPutOrPost(request) && !request.getParameters().isEmpty(); + && isPutOrPost(request) && (!request.getParameters().isEmpty() + && !includeParametersInUri(request)); } private Map header(String name, String value) { diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/CurlRequestSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/CurlRequestSnippetTests.java index ba953fed..e26ca217 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/CurlRequestSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/CurlRequestSnippetTests.java @@ -61,6 +61,14 @@ public class CurlRequestSnippetTests extends AbstractSnippetTests { operationBuilder("get-request").request("http://localhost/foo").build()); } + @Test + public void getRequestWithParameter() throws IOException { + this.snippet.expectCurlRequest("get-request").withContents( + codeBlock("bash").content("$ curl 'http://localhost/foo?a=alpha' -i")); + new CurlRequestSnippet().document(operationBuilder("get-request") + .request("http://localhost/foo").param("a", "alpha").build()); + } + @Test public void nonGetRequest() throws IOException { this.snippet.expectCurlRequest("non-get-request").withContents( @@ -344,4 +352,17 @@ public class CurlRequestSnippetTests extends AbstractSnippetTests { .header("a", "alpha").build()); } + @Test + public void postWithContentAndParameters() throws IOException { + this.snippet.expectCurlRequest("post-with-content-and-parameters") + .withContents(codeBlock("bash") + .content("$ curl 'http://localhost/foo?a=alpha&b=bravo' -i " + + "-X POST -d 'Some content'")); + new CurlRequestSnippet() + .document(operationBuilder("post-with-content-and-parameters") + .request("http://localhost/foo").param("a", "alpha") + .method("POST").param("b", "bravo").content("Some content") + .build()); + } + } diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/HttpieRequestSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/HttpieRequestSnippetTests.java index c3b9a398..9f8164b3 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/HttpieRequestSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/HttpieRequestSnippetTests.java @@ -62,6 +62,14 @@ public class HttpieRequestSnippetTests extends AbstractSnippetTests { operationBuilder("get-request").request("http://localhost/foo").build()); } + @Test + public void getRequestWithParameter() throws IOException { + this.snippet.expectHttpieRequest("get-request-with-parameter").withContents( + codeBlock("bash").content("$ http GET 'http://localhost/foo?a=alpha'")); + new HttpieRequestSnippet().document(operationBuilder("get-request-with-parameter") + .request("http://localhost/foo").param("a", "alpha").build()); + } + @Test public void nonGetRequest() throws IOException { this.snippet.expectHttpieRequest("non-get-request").withContents( @@ -346,4 +354,16 @@ public class HttpieRequestSnippetTests extends AbstractSnippetTests { .header("a", "alpha").build()); } + @Test + public void postWithContentAndParameters() throws IOException { + this.snippet.expectHttpieRequest("post-with-content-and-parameters").withContents( + codeBlock("bash").content("$ echo 'Some content' | http POST " + + "'http://localhost/foo?a=alpha&b=bravo'")); + new HttpieRequestSnippet() + .document(operationBuilder("post-with-content-and-parameters") + .request("http://localhost/foo").method("POST") + .param("a", "alpha").param("b", "bravo").content("Some content") + .build()); + } + } diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java index df1401ac..6f7057b3 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java @@ -59,6 +59,17 @@ public class HttpRequestSnippetTests extends AbstractSnippetTests { .request("http://localhost/foo").header("Alpha", "a").build()); } + @Test + public void getRequestWithParameters() throws IOException { + this.snippet.expectHttpRequest("get-request-with-parameters") + .withContents(httpRequest(RequestMethod.GET, "/foo?b=bravo") + .header("Alpha", "a").header(HttpHeaders.HOST, "localhost")); + + new HttpRequestSnippet().document(operationBuilder("get-request-with-parameters") + .request("http://localhost/foo").header("Alpha", "a").param("b", "bravo") + .build()); + } + @Test public void getRequestWithPort() throws IOException { this.snippet.expectHttpRequest("get-request") @@ -103,6 +114,20 @@ public class HttpRequestSnippetTests extends AbstractSnippetTests { .request("http://localhost/foo").method("POST").content(content).build()); } + @Test + public void postRequestWithContentAndParameters() throws IOException { + String content = "Hello, world"; + this.snippet.expectHttpRequest("post-request-with-content-and-parameters") + .withContents(httpRequest(RequestMethod.POST, "/foo?a=alpha") + .header(HttpHeaders.HOST, "localhost").content(content).header( + HttpHeaders.CONTENT_LENGTH, content.getBytes().length)); + + new HttpRequestSnippet() + .document(operationBuilder("post-request-with-content-and-parameters") + .request("http://localhost/foo").method("POST") + .param("a", "alpha").content(content).build()); + } + @Test public void postRequestWithCharset() throws IOException { String japaneseContent = "\u30b3\u30f3\u30c6\u30f3\u30c4"; diff --git a/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java b/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java index 0094abc8..2b2df2e8 100644 --- a/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java +++ b/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java @@ -175,6 +175,22 @@ public class MockMvcRestDocumentationIntegrationTests { + "-H 'Accept: application/json' -d 'a=alpha'")))); } + @Test + public void curlSnippetWithContentAndParametersOnPost() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context) + .apply(documentationConfiguration(this.restDocumentation)).build(); + mockMvc.perform(post("/").param("a", "alpha").accept(MediaType.APPLICATION_JSON) + .content("some content")).andExpect(status().isOk()) + .andDo(document("curl-snippet-with-content-and-parameters")); + assertThat( + new File( + "build/generated-snippets/curl-snippet-with-content-and-parameters/curl-request.adoc"), + is(snippet(asciidoctor()) + .withContents(codeBlock(asciidoctor(), "bash").content("$ curl " + + "'http://localhost:8080/?a=alpha' -i -X POST " + + "-H 'Accept: application/json' -d 'some content'")))); + } + @Test public void httpieSnippetWithContent() throws Exception { MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context) @@ -207,6 +223,22 @@ public class MockMvcRestDocumentationIntegrationTests { + "'Accept:application/json' 'a=alpha'")))); } + @Test + public void httpieSnippetWithContentAndParametersOnPost() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context) + .apply(documentationConfiguration(this.restDocumentation)).build(); + mockMvc.perform(post("/").param("a", "alpha").content("some content") + .accept(MediaType.APPLICATION_JSON)).andExpect(status().isOk()) + .andDo(document("httpie-snippet-post-with-content-and-parameters")); + assertThat( + new File( + "build/generated-snippets/httpie-snippet-post-with-content-and-parameters/httpie-request.adoc"), + is(snippet(asciidoctor()).withContents(codeBlock(asciidoctor(), "bash") + .content("$ echo " + "'some content' | http POST " + + "'http://localhost:8080/?a=alpha' " + + "'Accept:application/json'")))); + } + @Test public void linksSnippet() throws Exception { MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context)