Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add queryParam method to MockHttpServletRequestBuilder #23296

Closed
OrangeDog opened this issue Jul 16, 2019 · 4 comments
Closed

Add queryParam method to MockHttpServletRequestBuilder #23296

OrangeDog opened this issue Jul 16, 2019 · 4 comments
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another

Comments

@OrangeDog
Copy link

ServletUriComponentsBuilder uses HttpServletRequest#getQueryString to build on the current request.
However, in a GET or HEAD request, MockHttpServletRequestBuilder#param only adds to the combined parameter map and not the query string, causing it to fail.

The workaround is to provide the parameters directly in the URI string, but this is inferior to the fluent API. The different behaviour is probably surprising to many users.

See also spring-projects/spring-restdocs#26

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 16, 2019
@sbrannen sbrannen added the in: test Issues in the test module label Jul 16, 2019
@sbrannen sbrannen changed the title Use GET request parameters to build query string Use GET request parameters to build query string in MockHttpServletRequestBuilder Jul 16, 2019
@rstoyanchev
Copy link
Contributor

You can supply a URL with a query to MockMvcRequestBuilders. The query is parsed and appended to the params map. I suppose we could allow it in the reverse order too, if the request is GET or HEAD and there is no body, but isn't the first option a more self-explanatory way to do it?

@OrangeDog
Copy link
Author

@rstoyanchev I know, that's why I mentioned it.

The workaround is to provide the parameters directly in the URI string, but this is inferior to the fluent API. The different behaviour is probably surprising to many users.

Every other part of the the request has a structured way to set it except the querystring, which requires manual encoding and concatenation.

An alternative is to add queryParam or query methods as well as param. I'm not sure whether that would be more confusing, though it better supports the case of POST requests with both a querystring and a body.

@rstoyanchev
Copy link
Contributor

Apologies I missed that part of your comment.

I don't see providing the query in the URI template as a workaround. It is the primary way of specifying the target URI with all of its components including the query. The URI template by the way is encoded so I do not see a need for manual encoding. And you can also use UriComponentsBuilder to prepare the URI and pass that in.

That said we can add a queryParam method as an extra convenience, and that will append to query of the target URI.

@rstoyanchev rstoyanchev added status: ideal-for-contribution An issue that a contributor can help us with and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jul 17, 2019
@rstoyanchev rstoyanchev added this to the 5.x Backlog milestone Jul 17, 2019
@rstoyanchev rstoyanchev changed the title Use GET request parameters to build query string in MockHttpServletRequestBuilder Add queryParam method to MockHttpServletRequestBuilder Jul 17, 2019
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another and removed status: ideal-for-contribution An issue that a contributor can help us with labels Nov 12, 2019
@rstoyanchev
Copy link
Contributor

This is now superseded by #23980.

rstoyanchev added a commit that referenced this issue Nov 12, 2019
@jhoeller jhoeller removed this from the 5.x Backlog milestone Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another
Projects
None yet
5 participants