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

added support for creating custom methods with route macro #2969

Merged
merged 19 commits into from
Feb 6, 2023

Conversation

edgerunnergit
Copy link
Contributor

@edgerunnergit edgerunnergit commented Jan 25, 2023

PR Type

An improvement to existing feature.

PR Checklist

  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt.
  • (Team) Label with affected crates and semver status.

Overview

Current Behavior:

#[route("/my/path", method = "CREATE")]
async fn create() -> HttpResponse {
  // ...

If we wish to use custom methods using the route macro, it causes panic due to Unexpected HTTP method.

New Behavior:

#[route("/heya", method = "NOLAN", method = "BET", method = "GET")]
async fn manual_heya() -> impl Responder {
    HttpResponse::Ok().body("Hey there ya!\n")
}

This would be a valid format - and we can curl with either of the three methods (Provided it's an uppercase ASCII).

To achieve this, I added a Custom Method in MethodTypes which supported any uppercase Method that isn't one of the standard ones.

Extra:

I haven't yet added the tests and documentation for it - but I plan to do it slowly as the issue progresses.

Closes #2893

@edgerunnergit edgerunnergit marked this pull request as ready for review January 30, 2023 05:26
@edgerunnergit edgerunnergit force-pushed the custom_methods branch 2 times, most recently from b2f9c7f to d358919 Compare January 30, 2023 08:53
@robjtede robjtede added A-codegen project: actix-web-codegen B-semver-minor labels Jan 30, 2023
@robjtede
Copy link
Member

robjtede commented Feb 4, 2023

I'd like to avoid needing a new type in actix_web. We already have the Method guard which should be used for this feature.

@edgerunnergit
Copy link
Contributor Author

I'd like to avoid needing a new type in actix_web. We already have the Method guard which should be used for this feature.

Thanks. I tried to make it work. Sorry for all the bad commits. I'm not familiar with git enough. Need to brush up my skills.

@robjtede
Copy link
Member

robjtede commented Feb 6, 2023

would be good to add at least one test for failure cases like lowercase method = "hello"

@edgerunnergit
Copy link
Contributor Author

would be good to add at least one test for failure cases like lowercase method = "hello"

done. added a trybuild method for that with stderr similar to other tests.

@edgerunnergit
Copy link
Contributor Author

@robjtede sorry for the repeated small fixes. I'm having trouble testing out the trybuild tests or the github action tests in local environment hence not able to fix them in one go.

@robjtede
Copy link
Member

robjtede commented Feb 6, 2023

It should be enough to install the 1.59.0 toolchain and run TRYBUILD=override cargo +1.59.0 test locally.

@edgerunnergit
Copy link
Contributor Author

TRYBUILD=override cargo +1.59.0 test

I tried to make it work this way earlier but always greeted with the following error:

❯ TRYBUILD=override cargo +1.59.0 test
    Updating crates.io index
error: failed to select a version for the requirement `time = "^0.3"`
candidate versions found which didn't match: 0.3.15, 0.3.14, 0.3.13, ...
location searched: crates.io index
required by package `actix-web v4.3.0 (/home/user/actix-web/actix-web)`

@robjtede
Copy link
Member

robjtede commented Feb 6, 2023

Ahh of course, need to follow what the CI does here and here with some tweaks for time probably.

Copy link
Member

@robjtede robjtede left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, thanks 👍

@robjtede robjtede merged commit 65c0545 into actix:master Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen project: actix-web-codegen B-semver-minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow custom HTTP methods in route macro
2 participants