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

Implement optionalQueryParam Following Discussion in #1751 #1776

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

beng9re
Copy link
Contributor

@beng9re beng9re commented Mar 18, 2024

This PR introduces the optionalQueryParam method to the MockMvcRequestSpecification class, as discussed in issue #1751.

Implementation Details

The optionalQueryParam method enhances request building by conditionally adding query parameters:

  • When the QueryParam value is null: No query parameter is added to the request.
  • When the QueryParam value is not null: The query parameter is added with the specified name and value.

Discussion Reference

This implementation follows the specifications and considerations discussed in issue #1751, aiming to address the need for optional query parameters in our request handling logic.

Testing

Tests have been added to verify that the optionalQueryParam method behaves as expected, ensuring that parameters are correctly added only when their values are not null.

rest-assured#1751
If this QueryParam value is null, no query parameter is added
If not null, the query parameter is added with the given parameter name and value.
@johanhaleby
Copy link
Collaborator

@beng9re Thanks again for a nice PR. However, I'm a little hesitant to merge this because I suspect (but I could be wrong) that it's a rather slim use case and it would add significantly more methods to the API (we should add it for form parameters as well etc). I understand that's a bit cumbersome to deal with this in Java, but in Kotlin (which I use most of the time) it's easy to write your own extension function. Also, this PR only address it for mock-mvc, not the "core", kotlin or web-test API so it would require more work to make it consistent. So before I put effort into this, I'd like to hear your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants