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

Author: wilkinsona

spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/CurlRequestSnippet.java

@@ -69,7 +69,16 @@ protected Map<String, Object> createModel(Operation operation) {
 	}
 
 	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) {

spring-restdocs-core/src/main/java/org/springframework/restdocs/cli/HttpieRequestSnippet.java

@@ -24,6 +24,7 @@
 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 @@ private String getOptions(CliOperationRequest request) {
 		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 @@ private String getRequestItems(CliOperationRequest request) {
 	}
 
 	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();

spring-restdocs-core/src/main/java/org/springframework/restdocs/http/HttpRequestSnippet.java

@@ -66,14 +66,33 @@ protected HttpRequestSnippet(Map<String, Object> attributes) {
 	protected Map<String, Object> createModel(Operation operation) {
 		Map<String, Object> 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<Map<String, String>> getHeaders(OperationRequest request) {
 		List<Map<String, String>> headers = new ArrayList<>();
 
@@ -172,7 +191,8 @@ private void writeMultipartEnd(PrintWriter writer) {
 
 	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<String, String> header(String name, String value) {

spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/CurlRequestSnippetTests.java

@@ -61,6 +61,14 @@ public void getRequest() throws IOException {
 				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 void customHostHeaderIsIncluded() throws IOException {
 						.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());
+	}
+
 }

spring-restdocs-core/src/test/java/org/springframework/restdocs/cli/HttpieRequestSnippetTests.java

@@ -62,6 +62,14 @@ public void getRequest() throws IOException {
 				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 void customHostHeaderIsIncluded() throws IOException {
 						.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());
+	}
+
 }

spring-restdocs-core/src/test/java/org/springframework/restdocs/http/HttpRequestSnippetTests.java

@@ -59,6 +59,17 @@ public void getRequest() throws IOException {
 				.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 void postRequestWithContent() throws IOException {
 				.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";

spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java

@@ -175,6 +175,22 @@ public void curlSnippetWithQueryStringOnPost() throws Exception {
 								+ "-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 void httpieSnippetWithQueryStringOnPost() throws Exception {
 								+ "'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)