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.

Sign up for GitHub

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

Jump to bottom

Refactor HttpMethod from enum to class #27697

Closed
poutsma opened this issue Nov 18, 2021 · 8 comments
Closed

Refactor HttpMethod from enum to class #27697

poutsma opened this issue Nov 18, 2021 · 8 comments
Assignees
@poutsma
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone
6.0.0-M1

Comments

@poutsma
Copy link
Contributor

poutsma commented Nov 18, 2021 •
edited
Loading

According to the HTTP specification, the HTTP method is not limited to the well known set (GET, HEAD, PUT, POST, etc.), but can also be an "extension-method". Well known extensions include WebDAV, which added methods like LOCK, COPY, and MOVE.

In Spring Framework, HTTP methods are enumerated in HttpMethod. Because this type is an Java enum, Spring framework needs several workarounds, to allow for HTTP methods not in the enum, such as having both HttpRequest::getMethod as well as HttpRequest::getMethodValue.

If we change HttpMethod from enum to class, we no longer need these workarounds. If we make sure that the new class has the same methods that java.lang.Enum exposes, and given that upgrading to 6.0 requires a recompilation anyway, I believe that now is the time to make this long overdue change.

Note that this issue does not include support for non-standard HTTP (i.e. WebDAV) methods in Spring MVC and/or WebFlux.
@poutsma poutsma added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Nov 18, 2021
@poutsma poutsma added this to the 6.0 M1 milestone Nov 18, 2021
@poutsma poutsma self-assigned this Nov 18, 2021
@jhoeller jhoeller added the type: enhancement A general enhancement label Nov 24, 2021
@quaff
Copy link
Contributor

quaff commented Nov 25, 2021

such as having both HttpMethod::getMethod as well as HttpRequest::getMethodValue.

HttpMethod::getMethod should be HttpRequest::getMethod

@poutsma
Copy link
Contributor Author

poutsma commented Nov 25, 2021

@quaff Fixed, thanks!

poutsma added a commit that referenced this issue Nov 30, 2021
@poutsma
Changes because HttpMethod changed to class
7a4207c 
This commit contains changes made because HttpMethod changed from enum
to class.

See gh-27697
poutsma added a commit that referenced this issue Nov 30, 2021
@poutsma
Use HttpMethod::valueOf in HttpMethod::resolve
445f25c 
This commit makes sure that HttpMethod::resolve uses HttpMethod::valueOf
and returns an HttpMethod for non-standard methods.

See gh-27697
@sjohnr sjohnr mentioned this issue Nov 30, 2021
Closed
@quaff
Copy link
Contributor

quaff commented Dec 1, 2021

Can we use extension-method for RequestMapping?

@poutsma
Copy link
Contributor Author

poutsma commented Dec 1, 2021

Can we use extension-method for RequestMapping?

I am not sure what you mean by that, can you elaborate? Because annotations can refer to enum elements but not classes, RequestMapping uses the RequestMethod enumeration.

@quaff
Copy link
Contributor

quaff commented Dec 2, 2021

Can we use extension-method for RequestMapping?

I am not sure what you mean by that, can you elaborate? Because annotations can refer to enum elements but not classes, RequestMapping uses the RequestMethod enumeration.

I mean should RequestMapping introduce String[] methodValue() to supports extension-method?

@poutsma
Copy link
Contributor Author

poutsma commented Dec 2, 2021

I mean should RequestMapping introduce String[] methodValue() to supports extension-method?

RequestMapping uses RequestMethod, and that's fine the way it is. As I wrote in the description, we have no intention of supporting non-standard HTTP methods. HttpMethod is a lower-level component that is used for our HTTP abstraction, and that did need support for non-standard methods.

sbrannen added a commit that referenced this issue Jan 3, 2022
@sbrannen
Additional changes because HttpMethod changed from enum to class
97625e3 
See gh-27697
@poutsma poutsma mentioned this issue Mar 22, 2022
Closed
poutsma added a commit that referenced this issue Feb 17, 2023
@poutsma
Fix regression in WebFlux support for WebDAV methods
88e6544 
This commit ensures that WebFlux's RequestMethodsRequestCondition
supports HTTP methods that are not in the RequestMethod enum.

- RequestMethod::resolve is introduced, to convert from a HttpMethod
(name) to enum values.
- RequestMethod::asHttpMethod is introduced, to convert from enum value
to HttpMethod.
- HttpMethod::valueOf replaced Map-based lookup to a switch statement
- Enabled tests that check for WebDAV methods

See gh-27697
Closes gh-29981
@tamizh-m
Copy link

Hi @poutsma , After upgrading to Spring 6, I am unable to serialize and deserialize 'HttpMethod' using 'ObjectMapper' because the class does not have a public constructor or a getter for 'name' attribute. This issue did not arise previously when HttpMethod was an enum. Would be helpful if you can provide a workaround for this?

@poutsma
Copy link
Contributor Author

poutsma commented May 28, 2024

@tamizh-m Please file a new issue. This issue is closed.

@jmle jmle mentioned this issue Oct 2, 2024
Closed
@mwcodeslave mwcodeslave mentioned this issue Nov 11, 2024
Closed
@yihtserns yihtserns mentioned this issue Nov 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@poutsma poutsma

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Milestone
6.0.0-M1
Development

No branches or pull requests
4 participants
@quaff @poutsma @jhoeller @tamizh-m