From 65c0545a7a24fb2234f7793b4a9d4d7b6811c62c Mon Sep 17 00:00:00 2001 From: edgerunnergit Date: Mon, 6 Feb 2023 18:10:41 +0530 Subject: [PATCH] added support for creating custom methods with route macro (#2969) Co-authored-by: Rob Ede Closes https://github.com/actix/actix-web/issues/2893 --- actix-web-codegen/CHANGES.md | 3 + actix-web-codegen/src/lib.rs | 2 +- actix-web-codegen/src/route.rs | 149 ++++++++++++++---- actix-web-codegen/tests/test_macro.rs | 8 +- actix-web-codegen/tests/trybuild.rs | 4 +- .../tests/trybuild/route-custom-lowercase.rs | 19 +++ .../trybuild/route-custom-lowercase.stderr | 19 +++ ...-method-fail.rs => route-custom-method.rs} | 4 +- .../route-unexpected-method-fail.stderr | 19 --- actix-web/CHANGES.md | 1 - 10 files changed, 169 insertions(+), 59 deletions(-) create mode 100644 actix-web-codegen/tests/trybuild/route-custom-lowercase.rs create mode 100644 actix-web-codegen/tests/trybuild/route-custom-lowercase.stderr rename actix-web-codegen/tests/trybuild/{route-unexpected-method-fail.rs => route-custom-method.rs} (71%) delete mode 100644 actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index cb37bfdb0..ffa2cd252 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2022-xx-xx +- Add support for Custom Methods with `#[route]` macro. [#2969] + +[#2969]: https://github.com/actix/actix-web/pull/2969 ## 4.1.0 - 2022-09-11 diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index 4b6dc43c5..5d392be1d 100644 --- a/actix-web-codegen/src/lib.rs +++ b/actix-web-codegen/src/lib.rs @@ -105,7 +105,7 @@ mod route; /// ``` /// # use actix_web::HttpResponse; /// # use actix_web_codegen::route; -/// #[route("/test", method = "GET", method = "HEAD")] +/// #[route("/test", method = "GET", method = "HEAD", method = "CUSTOM")] /// async fn example() -> HttpResponse { /// HttpResponse::Ok().finish() /// } diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index e5493702d..594a58626 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -27,7 +27,13 @@ macro_rules! method_type { fn parse(method: &str) -> Result { match method { $(stringify!($upper) => Ok(Self::$variant),)+ - _ => Err(format!("Unexpected HTTP method: `{}`", method)), + _ => { + if method.chars().all(|c| c.is_ascii_uppercase()) { + Ok(Self::Method) + } else { + Err(format!("HTTP method must be uppercase: `{}`", method)) + } + }, } } @@ -41,6 +47,12 @@ macro_rules! method_type { }; } +#[derive(Eq, Hash, PartialEq)] +struct MethodTypeExt { + method: MethodType, + custom_method: Option, +} + method_type! { Get, GET, get, Post, POST, post, @@ -51,6 +63,7 @@ method_type! { Options, OPTIONS, options, Trace, TRACE, trace, Patch, PATCH, patch, + Method, METHOD, method, } impl ToTokens for MethodType { @@ -60,6 +73,21 @@ impl ToTokens for MethodType { } } +impl ToTokens for MethodTypeExt { + fn to_tokens(&self, stream: &mut TokenStream2) { + match self.method { + MethodType::Method => { + let ident = Ident::new( + self.custom_method.as_ref().unwrap().value().as_str(), + Span::call_site(), + ); + stream.append(ident); + } + _ => self.method.to_tokens(stream), + } + } +} + impl TryFrom<&syn::LitStr> for MethodType { type Error = syn::Error; @@ -74,7 +102,7 @@ struct Args { resource_name: Option, guards: Vec, wrappers: Vec, - methods: HashSet, + methods: HashSet, } impl Args { @@ -99,7 +127,12 @@ impl Args { let is_route_macro = method.is_none(); if let Some(method) = method { - methods.insert(method); + methods.insert({ + MethodTypeExt { + method, + custom_method: None, + } + }); } for arg in args { @@ -152,10 +185,22 @@ impl Args { )); } else if let syn::Lit::Str(ref lit) = nv.lit { let method = MethodType::try_from(lit)?; - if !methods.insert(method) { + if !methods.insert({ + if method == MethodType::Method { + MethodTypeExt { + method, + custom_method: Some(lit.clone()), + } + } else { + MethodTypeExt { + method, + custom_method: None, + } + } + }) { return Err(syn::Error::new_spanned( &nv.lit, - format!( + &format!( "HTTP method defined more than once: `{}`", lit.value() ), @@ -298,38 +343,72 @@ impl ToTokens for Route { .as_ref() .map_or_else(|| name.to_string(), LitStr::value); - let method_guards = { - let mut others = methods.iter(); - - // unwrapping since length is checked to be at least one - let first = others.next().unwrap(); - - if methods.len() > 1 { - quote! { - .guard( - ::actix_web::guard::Any(::actix_web::guard::#first()) - #(.or(::actix_web::guard::#others()))* - ) - } - } else { - quote! { - .guard(::actix_web::guard::#first()) + let method_guards = { + let mut others = methods.iter(); + let first = others.next().unwrap(); + let first_method = &first.method; + if methods.len() > 1 { + let mut mult_method_guards: Vec = Vec::new(); + for method_ext in methods { + let method_type = &method_ext.method; + let custom_method = &method_ext.custom_method; + match custom_method { + Some(lit) => { + mult_method_guards.push(quote! { + .or(::actix_web::guard::#method_type(::actix_web::http::Method::from_bytes(#lit.as_bytes()).unwrap())) + }); + } + None => { + mult_method_guards.push(quote! { + .or(::actix_web::guard::#method_type()) + }); + } + } + } + match &first.custom_method { + Some(lit) => { + quote! { + .guard( + ::actix_web::guard::Any(::actix_web::guard::#first_method(::actix_web::http::Method::from_bytes(#lit.as_bytes()).unwrap())) + #(#mult_method_guards)* + ) + } + } + None => { + quote! { + .guard( + ::actix_web::guard::Any(::actix_web::guard::#first_method()) + #(#mult_method_guards)* + ) + } + } + } + } else { + match &first.custom_method { + Some(lit) => { + quote! { + .guard(::actix_web::guard::#first_method(::actix_web::http::Method::from_bytes(#lit.as_bytes()).unwrap())) + } + } + None => { + quote! { + .guard(::actix_web::guard::#first_method()) + } + } + } } + }; + quote! { + let __resource = ::actix_web::Resource::new(#path) + .name(#resource_name) + #method_guards + #(.guard(::actix_web::guard::fn_guard(#guards)))* + #(.wrap(#wrappers))* + .to(#name); + ::actix_web::dev::HttpServiceFactory::register(__resource, __config); } - }; - - quote! { - let __resource = ::actix_web::Resource::new(#path) - .name(#resource_name) - #method_guards - #(.guard(::actix_web::guard::fn_guard(#guards)))* - #(.wrap(#wrappers))* - .to(#name); - - ::actix_web::dev::HttpServiceFactory::register(__resource, __config); - } - }) - .collect(); + }) + .collect(); let stream = quote! { #(#doc_attributes)* diff --git a/actix-web-codegen/tests/test_macro.rs b/actix-web-codegen/tests/test_macro.rs index 10e906967..a95d2aa37 100644 --- a/actix-web-codegen/tests/test_macro.rs +++ b/actix-web-codegen/tests/test_macro.rs @@ -86,7 +86,13 @@ async fn get_param_test(_: web::Path) -> impl Responder { HttpResponse::Ok() } -#[route("/multi", method = "GET", method = "POST", method = "HEAD")] +#[route( + "/multi", + method = "GET", + method = "POST", + method = "HEAD", + method = "HELLO" +)] async fn route_test() -> impl Responder { HttpResponse::Ok() } diff --git a/actix-web-codegen/tests/trybuild.rs b/actix-web-codegen/tests/trybuild.rs index 26aec7d28..8839dca3d 100644 --- a/actix-web-codegen/tests/trybuild.rs +++ b/actix-web-codegen/tests/trybuild.rs @@ -9,9 +9,11 @@ fn compile_macros() { t.pass("tests/trybuild/route-ok.rs"); t.compile_fail("tests/trybuild/route-missing-method-fail.rs"); t.compile_fail("tests/trybuild/route-duplicate-method-fail.rs"); - t.compile_fail("tests/trybuild/route-unexpected-method-fail.rs"); t.compile_fail("tests/trybuild/route-malformed-path-fail.rs"); + t.pass("tests/trybuild/route-custom-method.rs"); + t.compile_fail("tests/trybuild/route-custom-lowercase.rs"); + t.pass("tests/trybuild/routes-ok.rs"); t.compile_fail("tests/trybuild/routes-missing-method-fail.rs"); t.compile_fail("tests/trybuild/routes-missing-args-fail.rs"); diff --git a/actix-web-codegen/tests/trybuild/route-custom-lowercase.rs b/actix-web-codegen/tests/trybuild/route-custom-lowercase.rs new file mode 100644 index 000000000..61abb5bc2 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-custom-lowercase.rs @@ -0,0 +1,19 @@ +use actix_web_codegen::*; +use actix_web::http::Method; +use std::str::FromStr; + +#[route("/", method = "hello")] +async fn index() -> String { + "Hello World!".to_owned() +} + +#[actix_web::main] +async fn main() { + use actix_web::App; + + let srv = actix_test::start(|| App::new().service(index)); + + let request = srv.request(Method::from_str("hello").unwrap(), srv.url("/")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} diff --git a/actix-web-codegen/tests/trybuild/route-custom-lowercase.stderr b/actix-web-codegen/tests/trybuild/route-custom-lowercase.stderr new file mode 100644 index 000000000..243c4dd68 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-custom-lowercase.stderr @@ -0,0 +1,19 @@ +error: HTTP method must be uppercase: `hello` + --> tests/trybuild/route-custom-lowercase.rs:5:23 + | +5 | #[route("/", method = "hello")] + | ^^^^^^^ + +error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied + --> tests/trybuild/route-custom-lowercase.rs:14:55 + | +14 | let srv = actix_test::start(|| App::new().service(index)); + | ------- ^^^^^ the trait `HttpServiceFactory` is not implemented for `fn() -> impl std::future::Future {index}` + | | + | required by a bound introduced by this call + | +note: required by a bound in `App::::service` + --> $WORKSPACE/actix-web/src/app.rs + | + | F: HttpServiceFactory + 'static, + | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::::service` diff --git a/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.rs b/actix-web-codegen/tests/trybuild/route-custom-method.rs similarity index 71% rename from actix-web-codegen/tests/trybuild/route-unexpected-method-fail.rs rename to actix-web-codegen/tests/trybuild/route-custom-method.rs index 1a50e01bc..6cc6af5a8 100644 --- a/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.rs +++ b/actix-web-codegen/tests/trybuild/route-custom-method.rs @@ -1,4 +1,6 @@ use actix_web_codegen::*; +use actix_web::http::Method; +use std::str::FromStr; #[route("/", method="UNEXPECTED")] async fn index() -> String { @@ -11,7 +13,7 @@ async fn main() { let srv = actix_test::start(|| App::new().service(index)); - let request = srv.get("/"); + let request = srv.request(Method::from_str("UNEXPECTED").unwrap(), srv.url("/")); let response = request.send().await.unwrap(); assert!(response.status().is_success()); } diff --git a/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr b/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr deleted file mode 100644 index 3df5d9f5a..000000000 --- a/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr +++ /dev/null @@ -1,19 +0,0 @@ -error: Unexpected HTTP method: `UNEXPECTED` - --> tests/trybuild/route-unexpected-method-fail.rs:3:21 - | -3 | #[route("/", method="UNEXPECTED")] - | ^^^^^^^^^^^^ - -error[E0277]: the trait bound `fn() -> impl std::future::Future {index}: HttpServiceFactory` is not satisfied - --> tests/trybuild/route-unexpected-method-fail.rs:12:55 - | -12 | let srv = actix_test::start(|| App::new().service(index)); - | ------- ^^^^^ the trait `HttpServiceFactory` is not implemented for `fn() -> impl std::future::Future {index}` - | | - | required by a bound introduced by this call - | -note: required by a bound in `App::::service` - --> $WORKSPACE/actix-web/src/app.rs - | - | F: HttpServiceFactory + 'static, - | ^^^^^^^^^^^^^^^^^^ required by this bound in `App::::service` diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index bf386acba..1e6a953f5 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -2,7 +2,6 @@ ## Unreleased - 2022-xx-xx - ## 4.3.0 - 2023-01-21 ### Added - Add `ContentDisposition::attachment()` constructor. [#2867]