SPR-15819 - URI variables with MockRestRequestMatchers requestTo #1486
Conversation
|
@drumonii please create a Jira issue for this change. |
snicoll
changed the title
drumonii
force-pushed
the
MockRestRequestMatchers-requestTo-UriVars
branch
from
c022920
to
5166126
Compare
| Assert.notNull(expectedUri, "'uri' must not be null"); | ||
| return request -> assertEquals("Request URI", expectedUri, request.getURI().toString()); | ||
| String uri = UriComponentsBuilder.fromUriString(expectedUri).buildAndExpand(uriVars).encode().toString(); |
There was a problem hiding this comment.
This will cause expectedUri to always be encoded which wasn't the case until now.
One option would be to do this only if uriVars are actually provided but then the method behaves inconsistently with regards to encoding based on whether uriVars are provided or not.
Another option would be to have separate methods -- e.g. requestTo for exact string match, requestToUriTemplate to expand + encode, and possibly also requestToUri(String expectedUri, Function<UriBuilder, URI> uriFunction) for full control (as we've done on the new WebClient).
There was a problem hiding this comment.
In regards to the encoded expectedUri, I looked at the recent changes to use URI vars in MockMvcResultMatchers and modeled this chage off that.
Which option do you prefer? The first option is certainly more convenient. The separate method option also works well because if you are going back to refactor requestTo to use URI vars instead of concating, then you can change the method name while at it.
There was a problem hiding this comment.
Yes I see now that a similar change was made not long ago. The ticket also shows a comment about a subsequent regression https://jira.spring.io/browse/SPR-14790 .. :)
The first option isn't really an option because of the inconsistency I pointed out. I shouldn't have called it an option in the first place. Let's go for these two for now:
requestTo(String)
requestToUriTemplate(String, Object...)
The requestTouri(String, Function<UriBuilder, URI>) can always be added later.
There was a problem hiding this comment.
Sounds good! Wasn't aware of the regression but I'll work on these changes later.
Issue: SPR-15819
drumonii
force-pushed
the
MockRestRequestMatchers-requestTo-UriVars
branch
from
5166126
to
fed42af
Compare
|
This is merged, thanks, |
Adds ability to use URI variables with
MockRestRequestMatchers'srequestTo