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

Custom HTTP Method on MockServerHttpRequest #25109

Closed
GuyLewin opened this issue May 20, 2020 · 3 comments
Closed

Custom HTTP Method on MockServerHttpRequest #25109

GuyLewin opened this issue May 20, 2020 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@GuyLewin
Copy link

In some cases we'd like to use MockServerHttpRequest with methods not in Spring's HttpMethod enum (for testing corner cases for example).
My scenario was wanting to use MockServerHttpRequest with the CONNECT method, which isn't in HttpMethod.
When ServerHttpRequest receives a request from Reactor Netty it can accept CONNECT as a string method even though it's not in the enum.
I believe we should also add a string-based method initialization to MockServerHttpRequest.

This was originally described in #24820 but I was asked to create a separate issue for this.
@rstoyanchev FYI

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 20, 2020
@rstoyanchev rstoyanchev self-assigned this May 20, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 20, 2020
@rstoyanchev rstoyanchev added this to the 5.2.7 milestone May 20, 2020
@sbrannen
Copy link
Member

As pointed out by @eleftherias, the change associated with this issue is actually a breaking change for anyone that previously supplied null for the HttpMethod.

For example, the following used to be supported but now throws an IllegalArgumentException.

 MockServerHttpRequest.method(null, "/").build();

In light of that, perhaps we should reopen this issue to again support null for the HttpMethod.

@rstoyanchev Thoughts?


See also: spring-projects/spring-security#8592

@sbrannen sbrannen reopened this May 25, 2020
@rstoyanchev
Copy link
Contributor

My only concern is that Spring Security had no choice but to pass null since it was the only way to simulate a custom method. Aside from that there is no reason to pass null. My thought is to leave it the way it is but have a more helpful error message along the lines of "if you are passing null to simulate a custom method, please use ... instead". After all it's a test that's failing and it's easy to correct.

@sbrannen
Copy link
Member

My thought is to leave it the way it is but have a more helpful error message

Fair enough. Let's just hope there weren't many projects depending on the previous unintentional support for null.

midumitrescu added a commit to midumitrescu/spring-framework that referenced this issue May 28, 2020
midumitrescu added a commit to midumitrescu/spring-framework that referenced this issue May 28, 2020
midumitrescu added a commit to midumitrescu/spring-framework that referenced this issue May 28, 2020
….MockServerHttpRequest with org.springframework.mock.http.server.reactive.MockServerHttpRequest for spring-projectsgh-25109
rstoyanchev pushed a commit that referenced this issue May 29, 2020
kenny5he pushed a commit to kenny5he/spring-framework that referenced this issue Jun 21, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants