Skip to content

Commit bf9b0f2

Browse files
wilkinsonawilkinsona
committed
Ensure that query string is not duplicated when parameters overlap
Closes gh-286
1 parent 110fa23 commit bf9b0f2

File tree

8 files changed

+276
-59
lines changed

8 files changed

+276
-59
lines changed

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

-28
Original file line numberDiff line numberDiff line change
@@ -52,34 +52,6 @@ final class CliOperationRequest implements OperationRequest {
5252
new BasicAuthHeaderFilter(), new HostHeaderFilter(delegate.getUri())));
5353
}
5454

55-
Parameters getUniqueParameters() {
56-
Parameters queryStringParameters = new QueryStringParser()
57-
.parse(this.delegate.getUri());
58-
Parameters uniqueParameters = new Parameters();
59-
60-
for (Map.Entry<String, List<String>> parameter : this.delegate.getParameters()
61-
.entrySet()) {
62-
addIfUnique(parameter, queryStringParameters, uniqueParameters);
63-
}
64-
return uniqueParameters;
65-
}
66-
67-
private void addIfUnique(Map.Entry<String, List<String>> parameter,
68-
Parameters queryStringParameters, Parameters uniqueParameters) {
69-
if (!queryStringParameters.containsKey(parameter.getKey())) {
70-
uniqueParameters.put(parameter.getKey(), parameter.getValue());
71-
}
72-
else {
73-
List<String> candidates = parameter.getValue();
74-
List<String> existing = queryStringParameters.get(parameter.getKey());
75-
for (String candidate : candidates) {
76-
if (!existing.contains(candidate)) {
77-
uniqueParameters.add(parameter.getKey(), candidate);
78-
}
79-
}
80-
}
81-
}
82-
8355
boolean isPutOrPost() {
8456
return HttpMethod.PUT.equals(this.delegate.getMethod())
8557
|| HttpMethod.POST.equals(this.delegate.getMethod());

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

+9-5
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,12 @@ protected Map<String, Object> createModel(Operation operation) {
7070

7171
private String getUrl(Operation operation) {
7272
OperationRequest request = operation.getRequest();
73-
if (!request.getParameters().isEmpty() && includeParametersInUri(request)) {
74-
return String.format("'%s?%s'", request.getUri(),
75-
request.getParameters().toQueryString());
73+
Parameters uniqueParameters = request.getParameters()
74+
.getUniqueParameters(operation.getRequest().getUri());
75+
if (!uniqueParameters.isEmpty() && includeParametersInUri(request)) {
76+
return String.format("'%s%s%s'", request.getUri(),
77+
StringUtils.hasText(request.getUri().getRawQuery()) ? "&" : "?",
78+
uniqueParameters.toQueryString());
7679
}
7780
return String.format("'%s'", request.getUri());
7881
}
@@ -157,9 +160,10 @@ else if (request.isPutOrPost()) {
157160
}
158161
}
159162

160-
private void writeContentUsingParameters(CliOperationRequest request,
163+
private void writeContentUsingParameters(OperationRequest request,
161164
PrintWriter writer) {
162-
Parameters uniqueParameters = request.getUniqueParameters();
165+
Parameters uniqueParameters = request.getParameters()
166+
.getUniqueParameters(request.getUri());
163167
String queryString = uniqueParameters.toQueryString();
164168
if (StringUtils.hasText(queryString)) {
165169
writer.print(String.format(" -d '%s'", queryString));

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

+15-9
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,13 @@ private String getOptions(CliOperationRequest request) {
9090
return options.toString();
9191
}
9292

93-
private String getUrl(CliOperationRequest request) {
94-
if (!request.getUniqueParameters().isEmpty() && includeParametersInUri(request)) {
95-
return String.format("'%s?%s'", request.getUri(),
96-
request.getParameters().toQueryString());
93+
private String getUrl(OperationRequest request) {
94+
Parameters uniqueParameters = request.getParameters()
95+
.getUniqueParameters(request.getUri());
96+
if (!uniqueParameters.isEmpty() && includeParametersInUri(request)) {
97+
return String.format("'%s%s%s'", request.getUri(),
98+
StringUtils.hasText(request.getUri().getRawQuery()) ? "&" : "?",
99+
uniqueParameters.toQueryString());
97100
}
98101
return String.format("'%s'", request.getUri());
99102
}
@@ -107,14 +110,15 @@ private String getRequestItems(CliOperationRequest request) {
107110
return requestItems.toString();
108111
}
109112

110-
private void writeOptions(CliOperationRequest request, PrintWriter writer) {
111-
if (!request.getParts().isEmpty() || (!request.getUniqueParameters().isEmpty()
112-
&& !includeParametersInUri(request))) {
113+
private void writeOptions(OperationRequest request, PrintWriter writer) {
114+
if (!request.getParts().isEmpty()
115+
|| (!request.getParameters().getUniqueParameters(request.getUri())
116+
.isEmpty() && !includeParametersInUri(request))) {
113117
writer.print("--form ");
114118
}
115119
}
116120

117-
private boolean includeParametersInUri(CliOperationRequest request) {
121+
private boolean includeParametersInUri(OperationRequest request) {
118122
return request.getMethod() == HttpMethod.GET || request.getContent().length > 0;
119123
}
120124

@@ -167,7 +171,9 @@ private void writeParametersIfNecessary(CliOperationRequest request,
167171
writeContentUsingParameters(request.getParameters(), writer);
168172
}
169173
else if (request.isPutOrPost()) {
170-
writeContentUsingParameters(request.getUniqueParameters(), writer);
174+
writeContentUsingParameters(
175+
request.getParameters().getUniqueParameters(request.getUri()),
176+
writer);
171177
}
172178
}
173179

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.springframework.restdocs.operation.Operation;
3131
import org.springframework.restdocs.operation.OperationRequest;
3232
import org.springframework.restdocs.operation.OperationRequestPart;
33+
import org.springframework.restdocs.operation.Parameters;
3334
import org.springframework.restdocs.snippet.Snippet;
3435
import org.springframework.restdocs.snippet.TemplatedSnippet;
3536
import org.springframework.util.StringUtils;
@@ -75,12 +76,14 @@ protected Map<String, Object> createModel(Operation operation) {
7576
private String getPath(OperationRequest request) {
7677
String path = request.getUri().getRawPath();
7778
String queryString = request.getUri().getRawQuery();
78-
if (!request.getParameters().isEmpty() && includeParametersInUri(request)) {
79+
Parameters uniqueParameters = request.getParameters()
80+
.getUniqueParameters(request.getUri());
81+
if (!uniqueParameters.isEmpty() && includeParametersInUri(request)) {
7982
if (StringUtils.hasText(queryString)) {
80-
queryString = queryString + "&" + request.getParameters().toQueryString();
83+
queryString = queryString + "&" + uniqueParameters.toQueryString();
8184
}
8285
else {
83-
queryString = request.getParameters().toQueryString();
86+
queryString = uniqueParameters.toQueryString();
8487
}
8588
}
8689
if (StringUtils.hasText(queryString)) {

‎spring-restdocs-core/src/main/java/org/springframework/restdocs/operation/Parameters.java

+35
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717
package org.springframework.restdocs.operation;
1818

1919
import java.io.UnsupportedEncodingException;
20+
import java.net.URI;
2021
import java.net.URLEncoder;
2122
import java.util.List;
2223
import java.util.Map;
2324

25+
import org.springframework.restdocs.cli.QueryStringParser;
2426
import org.springframework.util.LinkedMultiValueMap;
2527
import org.springframework.util.StringUtils;
2628

@@ -53,6 +55,39 @@ public String toQueryString() {
5355
return sb.toString();
5456
}
5557

58+
/**
59+
* Returns a new {@code Parameters} containing only the parameters that do no appear
60+
* in the query string of the given {@code uri}.
61+
*
62+
* @param uri the uri
63+
* @return the unique parameters
64+
*/
65+
public Parameters getUniqueParameters(URI uri) {
66+
Parameters queryStringParameters = new QueryStringParser().parse(uri);
67+
Parameters uniqueParameters = new Parameters();
68+
69+
for (Map.Entry<String, List<String>> parameter : entrySet()) {
70+
addIfUnique(parameter, queryStringParameters, uniqueParameters);
71+
}
72+
return uniqueParameters;
73+
}
74+
75+
private void addIfUnique(Map.Entry<String, List<String>> parameter,
76+
Parameters queryStringParameters, Parameters uniqueParameters) {
77+
if (!queryStringParameters.containsKey(parameter.getKey())) {
78+
uniqueParameters.put(parameter.getKey(), parameter.getValue());
79+
}
80+
else {
81+
List<String> candidates = parameter.getValue();
82+
List<String> existing = queryStringParameters.get(parameter.getKey());
83+
for (String candidate : candidates) {
84+
if (!existing.contains(candidate)) {
85+
uniqueParameters.add(parameter.getKey(), candidate);
86+
}
87+
}
88+
}
89+
}
90+
5691
private static void append(StringBuilder sb, String key) {
5792
append(sb, key, "");
5893
}

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

+65-7
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,47 @@ public void getRequestWithQueryString() throws IOException {
9595
.request("http://localhost/foo?param=value").build());
9696
}
9797

98+
@Test
99+
public void getRequestWithTotallyOverlappingQueryStringAndParameters()
100+
throws IOException {
101+
this.snippet
102+
.expectCurlRequest(
103+
"request-with-totally-overlapping-query-string-and-parameters")
104+
.withContents(codeBlock("bash")
105+
.content("$ curl 'http://localhost/foo?param=value' -i"));
106+
new CurlRequestSnippet().document(operationBuilder(
107+
"request-with-totally-overlapping-query-string-and-parameters")
108+
.request("http://localhost/foo?param=value")
109+
.param("param", "value").build());
110+
}
111+
112+
@Test
113+
public void getRequestWithPartiallyOverlappingQueryStringAndParameters()
114+
throws IOException {
115+
this.snippet
116+
.expectCurlRequest(
117+
"request-with-partially-overlapping-query-string-and-parameters")
118+
.withContents(codeBlock("bash")
119+
.content("$ curl 'http://localhost/foo?a=alpha&b=bravo' -i"));
120+
new CurlRequestSnippet().document(operationBuilder(
121+
"request-with-partially-overlapping-query-string-and-parameters")
122+
.request("http://localhost/foo?a=alpha").param("a", "alpha")
123+
.param("b", "bravo").build());
124+
}
125+
126+
@Test
127+
public void getRequestWithDisjointQueryStringAndParameters() throws IOException {
128+
this.snippet
129+
.expectCurlRequest(
130+
"request-with-partially-overlapping-query-string-and-parameters")
131+
.withContents(codeBlock("bash")
132+
.content("$ curl 'http://localhost/foo?a=alpha&b=bravo' -i"));
133+
new CurlRequestSnippet().document(operationBuilder(
134+
"request-with-partially-overlapping-query-string-and-parameters")
135+
.request("http://localhost/foo?a=alpha").param("b", "bravo")
136+
.build());
137+
}
138+
98139
@Test
99140
public void getRequestWithQueryStringWithNoValue() throws IOException {
100141
this.snippet.expectCurlRequest("request-with-query-string-with-no-value")
@@ -172,25 +213,42 @@ public void postRequestWithUrlEncodedParameter() throws IOException {
172213
}
173214

174215
@Test
175-
public void postRequestWithQueryStringAndParameter() throws IOException {
176-
this.snippet.expectCurlRequest("post-request-with-query-string-and-parameter")
216+
public void postRequestWithDisjointQueryStringAndParameter() throws IOException {
217+
this.snippet
218+
.expectCurlRequest(
219+
"post-request-with-disjoint-query-string-and-parameter")
177220
.withContents(codeBlock("bash").content(
178221
"$ curl 'http://localhost/foo?a=alpha' -i -X POST -d 'b=bravo'"));
179-
new CurlRequestSnippet()
180-
.document(operationBuilder("post-request-with-query-string-and-parameter")
222+
new CurlRequestSnippet().document(
223+
operationBuilder("post-request-with-disjoint-query-string-and-parameter")
181224
.request("http://localhost/foo?a=alpha").method("POST")
182225
.param("b", "bravo").build());
183226
}
184227

185228
@Test
186-
public void postRequestWithOverlappingQueryStringAndParameters() throws IOException {
229+
public void postRequestWithTotallyOverlappingQueryStringAndParameters()
230+
throws IOException {
231+
this.snippet
232+
.expectCurlRequest(
233+
"post-request-with-totally-overlapping-query-string-and-parameters")
234+
.withContents(codeBlock("bash").content(
235+
"$ curl 'http://localhost/foo?a=alpha&b=bravo' -i -X POST"));
236+
new CurlRequestSnippet().document(operationBuilder(
237+
"post-request-with-totally-overlapping-query-string-and-parameters")
238+
.request("http://localhost/foo?a=alpha&b=bravo").method("POST")
239+
.param("a", "alpha").param("b", "bravo").build());
240+
}
241+
242+
@Test
243+
public void postRequestWithPartiallyOverlappingQueryStringAndParameters()
244+
throws IOException {
187245
this.snippet
188246
.expectCurlRequest(
189-
"post-request-with-overlapping-query-string-and-parameters")
247+
"post-request-with-partially-overlapping-query-string-and-parameters")
190248
.withContents(codeBlock("bash").content(
191249
"$ curl 'http://localhost/foo?a=alpha' -i -X POST -d 'b=bravo'"));
192250
new CurlRequestSnippet().document(operationBuilder(
193-
"post-request-with-overlapping-query-string-and-parameters")
251+
"post-request-with-partially-overlapping-query-string-and-parameters")
194252
.request("http://localhost/foo?a=alpha").method("POST")
195253
.param("a", "alpha").param("b", "bravo").build());
196254
}

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

+65-7
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,47 @@ public void getRequestWithQueryString() throws IOException {
9696
.request("http://localhost/foo?param=value").build());
9797
}
9898

99+
@Test
100+
public void getRequestWithTotallyOverlappingQueryStringAndParameters()
101+
throws IOException {
102+
this.snippet
103+
.expectHttpieRequest(
104+
"request-with-totally-overlapping-query-string-and-parameters")
105+
.withContents(codeBlock("bash")
106+
.content("$ http GET 'http://localhost/foo?param=value'"));
107+
new HttpieRequestSnippet().document(operationBuilder(
108+
"request-with-totally-overlapping-query-string-and-parameters")
109+
.request("http://localhost/foo?param=value")
110+
.param("param", "value").build());
111+
}
112+
113+
@Test
114+
public void getRequestWithPartiallyOverlappingQueryStringAndParameters()
115+
throws IOException {
116+
this.snippet
117+
.expectHttpieRequest(
118+
"request-with-partially-overlapping-query-string-and-parameters")
119+
.withContents(codeBlock("bash")
120+
.content("$ http GET 'http://localhost/foo?a=alpha&b=bravo'"));
121+
new HttpieRequestSnippet().document(operationBuilder(
122+
"request-with-partially-overlapping-query-string-and-parameters")
123+
.request("http://localhost/foo?a=alpha").param("a", "alpha")
124+
.param("b", "bravo").build());
125+
}
126+
127+
@Test
128+
public void getRequestWithDisjointQueryStringAndParameters() throws IOException {
129+
this.snippet
130+
.expectHttpieRequest(
131+
"request-with-partially-overlapping-query-string-and-parameters")
132+
.withContents(codeBlock("bash")
133+
.content("$ http GET 'http://localhost/foo?a=alpha&b=bravo'"));
134+
new HttpieRequestSnippet().document(operationBuilder(
135+
"request-with-partially-overlapping-query-string-and-parameters")
136+
.request("http://localhost/foo?a=alpha").param("b", "bravo")
137+
.build());
138+
}
139+
99140
@Test
100141
public void getRequestWithQueryStringWithNoValue() throws IOException {
101142
this.snippet.expectHttpieRequest("request-with-query-string-with-no-value")
@@ -173,25 +214,42 @@ public void postRequestWithUrlEncodedParameter() throws IOException {
173214
}
174215

175216
@Test
176-
public void postRequestWithQueryStringAndParameter() throws IOException {
177-
this.snippet.expectHttpieRequest("post-request-with-query-string-and-parameter")
217+
public void postRequestWithDisjointQueryStringAndParameter() throws IOException {
218+
this.snippet
219+
.expectHttpieRequest(
220+
"post-request-with-disjoint-query-string-and-parameter")
178221
.withContents(codeBlock("bash").content(
179222
"$ http --form POST 'http://localhost/foo?a=alpha' 'b=bravo'"));
180-
new HttpieRequestSnippet()
181-
.document(operationBuilder("post-request-with-query-string-and-parameter")
223+
new HttpieRequestSnippet().document(
224+
operationBuilder("post-request-with-disjoint-query-string-and-parameter")
182225
.request("http://localhost/foo?a=alpha").method("POST")
183226
.param("b", "bravo").build());
184227
}
185228

186229
@Test
187-
public void postRequestWithOverlappingQueryStringAndParameters() throws IOException {
230+
public void postRequestWithTotallyOverlappingQueryStringAndParameters()
231+
throws IOException {
232+
this.snippet
233+
.expectHttpieRequest(
234+
"post-request-with-totally-overlapping-query-string-and-parameters")
235+
.withContents(codeBlock("bash")
236+
.content("$ http POST 'http://localhost/foo?a=alpha&b=bravo'"));
237+
new HttpieRequestSnippet().document(operationBuilder(
238+
"post-request-with-totally-overlapping-query-string-and-parameters")
239+
.request("http://localhost/foo?a=alpha&b=bravo").method("POST")
240+
.param("a", "alpha").param("b", "bravo").build());
241+
}
242+
243+
@Test
244+
public void postRequestWithPartiallyOverlappingQueryStringAndParameters()
245+
throws IOException {
188246
this.snippet
189247
.expectHttpieRequest(
190-
"post-request-with-overlapping-query-string-and-parameters")
248+
"post-request-with-partially-overlapping-query-string-and-parameters")
191249
.withContents(codeBlock("bash").content(
192250
"$ http --form POST 'http://localhost/foo?a=alpha' 'b=bravo'"));
193251
new HttpieRequestSnippet().document(operationBuilder(
194-
"post-request-with-overlapping-query-string-and-parameters")
252+
"post-request-with-partially-overlapping-query-string-and-parameters")
195253
.request("http://localhost/foo?a=alpha").method("POST")
196254
.param("a", "alpha").param("b", "bravo").build());
197255
}

0 commit comments

Comments
 (0)