From 97ddf3dcec524d0742f2b4a144d4da19b7816212 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 4 Oct 2019 18:17:53 +0100 Subject: [PATCH] Fix handling of empty querty string parameters Previously empty parameters in a query string were handled inconsistently such that they sometimes had a value of "" in a list and sometimes were represented as an empty list. This inconsistency lead to the parameter appearing twice in the cURL request snippet. This commit removes the inconsistency by always using an empty string to represent an empty query parameter value. Fixes gh-647 --- .../restdocs/operation/QueryStringParser.java | 6 ++---- .../restdocs/operation/QueryStringParserTests.java | 14 ++++++++++++++ .../MockMvcRestDocumentationIntegrationTests.java | 13 +++++++++++++ ...stAssuredRestDocumentationIntegrationTests.java | 12 ++++++++++++ ...estClientRestDocumentationIntegrationTests.java | 11 +++++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/QueryStringParser.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/QueryStringParser.java index f0256376..49d637dd 100644 --- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/QueryStringParser.java +++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/QueryStringParser.java @@ -64,10 +64,8 @@ public class QueryStringParser { parameters.add(decode(name), decode(value)); } else { - List values = parameters.get(components[0]); - if (values == null) { - parameters.put(components[0], new LinkedList()); - } + List values = parameters.computeIfAbsent(components[0], (p) -> new LinkedList()); + values.add(""); } } else { diff --git a/spring-restdocs-core/src/test/java/org/springframework/restdocs/operation/QueryStringParserTests.java b/spring-restdocs-core/src/test/java/org/springframework/restdocs/operation/QueryStringParserTests.java index 75450b3b..b287d441 100644 --- a/spring-restdocs-core/src/test/java/org/springframework/restdocs/operation/QueryStringParserTests.java +++ b/spring-restdocs-core/src/test/java/org/springframework/restdocs/operation/QueryStringParserTests.java @@ -81,4 +81,18 @@ public class QueryStringParserTests { this.queryStringParser.parse(URI.create("http://localhost?a=apple=avocado")); } + @Test + public void emptyParameter() { + Parameters parameters = this.queryStringParser.parse(URI.create("http://localhost?a=")); + assertThat(parameters.size()).isEqualTo(1); + assertThat(parameters).containsEntry("a", Arrays.asList("")); + } + + @Test + public void emptyAndNotEmptyParameter() { + Parameters parameters = this.queryStringParser.parse(URI.create("http://localhost?a=&a=alpha")); + assertThat(parameters.size()).isEqualTo(1); + assertThat(parameters).containsEntry("a", Arrays.asList("", "alpha")); + } + } 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 129952b7..b2da36ad 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 @@ -185,6 +185,19 @@ public class MockMvcRestDocumentationIntegrationTests { + " -H 'Accept: application/json' \\%n" + " -d 'a=alpha'")))); } + @Test + public void curlSnippetWithEmptyParameterQueryString() throws Exception { + MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context) + .apply(documentationConfiguration(this.restDocumentation)).build(); + mockMvc.perform(get("/").param("a", "").accept(MediaType.APPLICATION_JSON)).andExpect(status().isOk()) + .andDo(document("curl-snippet-with-empty-parameter-query-string")); + assertThat( + new File("build/generated-snippets/curl-snippet-with-empty-parameter-query-string/curl-request.adoc")) + .has(content(codeBlock(TemplateFormats.asciidoctor(), "bash") + .withContent(String.format("$ curl 'http://localhost:8080/?a=' -i -X GET \\%n" + + " -H 'Accept: application/json'")))); + } + @Test public void curlSnippetWithContentAndParametersOnPost() throws Exception { MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context) diff --git a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java index 492e71b2..aa86fc1f 100644 --- a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java +++ b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java @@ -120,6 +120,18 @@ public class RestAssuredRestDocumentationIntegrationTests { + " -H 'Content-Type: " + contentType + "' \\%n" + " --cookie 'cookieName=cookieVal'")))); } + @Test + public void curlSnippetWithEmptyParameterQueryString() throws Exception { + given().port(tomcat.getPort()).filter(documentationConfiguration(this.restDocumentation)) + .filter(document("curl-snippet-with-empty-parameter-query-string")).accept("application/json") + .param("a", "").get("/").then().statusCode(200); + assertThat( + new File("build/generated-snippets/curl-snippet-with-empty-parameter-query-string/curl-request.adoc")) + .has(content(codeBlock(TemplateFormats.asciidoctor(), "bash") + .withContent(String.format("$ curl 'http://localhost:" + tomcat.getPort() + + "/?a=' -i -X GET \\%n -H 'Accept: application/json'")))); + } + @Test public void curlSnippetWithQueryStringOnPost() throws Exception { given().port(tomcat.getPort()).filter(documentationConfiguration(this.restDocumentation)) diff --git a/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java b/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java index 06199f8c..3c59dc60 100644 --- a/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java +++ b/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java @@ -165,6 +165,17 @@ public class WebTestClientRestDocumentationIntegrationTests { + " -H 'Accept: application/json' \\%n" + " --cookie 'cookieName=cookieVal'")))); } + @Test + public void curlSnippetWithEmptyParameterQueryString() throws Exception { + this.webTestClient.get().uri("/?a=").accept(MediaType.APPLICATION_JSON).exchange().expectStatus().isOk() + .expectBody().consumeWith(document("curl-snippet-with-empty-parameter-query-string")); + assertThat( + new File("build/generated-snippets/curl-snippet-with-empty-parameter-query-string/curl-request.adoc")) + .has(content(codeBlock(TemplateFormats.asciidoctor(), "bash") + .withContent(String.format("$ curl 'https://api.example.com/?a=' -i -X GET \\%n" + + " -H 'Accept: application/json'")))); + } + @Test public void httpieSnippetWithCookies() throws Exception { this.webTestClient.get().uri("/").cookie("cookieName", "cookieVal").accept(MediaType.APPLICATION_JSON)