diff --git a/actix-web-codegen/CHANGES.md b/actix-web-codegen/CHANGES.md index 5c0ce828..793864d4 100644 --- a/actix-web-codegen/CHANGES.md +++ b/actix-web-codegen/CHANGES.md @@ -2,8 +2,10 @@ ## Unreleased - 2020-xx-xx * Added compile success and failure testing. [#1677] +* Add `route` macro for supporting multiple HTTP methods guards. [#1677]: https://github.com/actix/actix-web/pull/1677 +[#1674]: https://github.com/actix/actix-web/pull/1674 ## 0.3.0 - 2020-09-11 diff --git a/actix-web-codegen/Cargo.toml b/actix-web-codegen/Cargo.toml index 1bf78f99..da37b8de 100644 --- a/actix-web-codegen/Cargo.toml +++ b/actix-web-codegen/Cargo.toml @@ -23,3 +23,4 @@ actix-rt = "1.0.0" actix-web = "3.0.0" futures-util = { version = "0.3.5", default-features = false } trybuild = "1" +rustversion = "1" diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index 445fe924..7ae6a26b 100644 --- a/actix-web-codegen/src/lib.rs +++ b/actix-web-codegen/src/lib.rs @@ -141,6 +141,23 @@ pub fn patch(args: TokenStream, input: TokenStream) -> TokenStream { route::generate(args, input, route::GuardType::Patch) } +/// Creates resource handler, allowing multiple HTTP method guards. +/// +/// Syntax: `#[route("path"[, attributes])]` +/// +/// Example: `#[route("/", method="GET", method="HEAD")]` +/// +/// ## Attributes +/// +/// - `"path"` - Raw literal string with path for which to register handler. Mandatory. +/// - `method="HTTP_METHOD"` - Registers HTTP method to provide guard for. +/// - `guard="function_name"` - Registers function as guard using `actix_web::guard::fn_guard` +/// - `wrap="Middleware"` - Registers a resource middleware. +#[proc_macro_attribute] +pub fn route(args: TokenStream, input: TokenStream) -> TokenStream { + route::generate(args, input, route::GuardType::Multi) +} + /// Marks async main function as the actix system entry-point. /// /// ## Usage diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index 676e75e0..394ced21 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -1,5 +1,8 @@ extern crate proc_macro; +use std::collections::HashSet; +use std::convert::TryFrom; + use proc_macro::TokenStream; use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{format_ident, quote, ToTokens, TokenStreamExt}; @@ -17,7 +20,7 @@ impl ToTokens for ResourceType { } } -#[derive(PartialEq)] +#[derive(Debug, PartialEq, Eq, Hash)] pub enum GuardType { Get, Post, @@ -28,6 +31,7 @@ pub enum GuardType { Options, Trace, Patch, + Multi, } impl GuardType { @@ -42,6 +46,7 @@ impl GuardType { GuardType::Options => "Options", GuardType::Trace => "Trace", GuardType::Patch => "Patch", + GuardType::Multi => "Multi", } } } @@ -53,10 +58,33 @@ impl ToTokens for GuardType { } } +impl TryFrom<&syn::LitStr> for GuardType { + type Error = syn::Error; + + fn try_from(value: &syn::LitStr) -> Result { + match value.value().as_str() { + "CONNECT" => Ok(GuardType::Connect), + "DELETE" => Ok(GuardType::Delete), + "GET" => Ok(GuardType::Get), + "HEAD" => Ok(GuardType::Head), + "OPTIONS" => Ok(GuardType::Options), + "PATCH" => Ok(GuardType::Patch), + "POST" => Ok(GuardType::Post), + "PUT" => Ok(GuardType::Put), + "TRACE" => Ok(GuardType::Trace), + _ => Err(syn::Error::new_spanned( + value, + &format!("Unexpected HTTP Method: `{}`", value.value()), + )), + } + } +} + struct Args { path: syn::LitStr, guards: Vec, wrappers: Vec, + methods: HashSet, } impl Args { @@ -64,6 +92,7 @@ impl Args { let mut path = None; let mut guards = Vec::new(); let mut wrappers = Vec::new(); + let mut methods = HashSet::new(); for arg in args { match arg { NestedMeta::Lit(syn::Lit::Str(lit)) => match path { @@ -96,10 +125,28 @@ impl Args { "Attribute wrap expects type", )); } + } else if nv.path.is_ident("method") { + if let syn::Lit::Str(ref lit) = nv.lit { + let guard = GuardType::try_from(lit)?; + if !methods.insert(guard) { + return Err(syn::Error::new_spanned( + &nv.lit, + &format!( + "HTTP Method defined more than once: `{}`", + lit.value() + ), + )); + } + } else { + return Err(syn::Error::new_spanned( + nv.lit, + "Attribute method expects literal string!", + )); + } } else { return Err(syn::Error::new_spanned( nv.path, - "Unknown attribute key is specified. Allowed: guard and wrap", + "Unknown attribute key is specified. Allowed: guard, method and wrap", )); } } @@ -112,6 +159,7 @@ impl Args { path: path.unwrap(), guards, wrappers, + methods, }) } } @@ -166,6 +214,13 @@ impl Route { let args = Args::new(args)?; + if guard == GuardType::Multi && args.methods.is_empty() { + return Err(syn::Error::new( + Span::call_site(), + "The #[route(..)] macro requires at least one `method` attribute", + )); + } + let resource_type = if ast.sig.asyncness.is_some() { ResourceType::Async } else { @@ -201,10 +256,29 @@ impl ToTokens for Route { path, guards, wrappers, + methods, }, resource_type, } = self; let resource_name = name.to_string(); + let mut methods = methods.iter(); + + let method_guards = if *guard == GuardType::Multi { + // unwrapping since length is checked to be at least one + let first = methods.next().unwrap(); + + quote! { + .guard( + actix_web::guard::Any(actix_web::guard::#first()) + #(.or(actix_web::guard::#methods()))* + ) + } + } else { + quote! { + .guard(actix_web::guard::#guard()) + } + }; + let stream = quote! { #[allow(non_camel_case_types, missing_docs)] pub struct #name; @@ -214,7 +288,7 @@ impl ToTokens for Route { #ast let __resource = actix_web::Resource::new(#path) .name(#resource_name) - .guard(actix_web::guard::#guard()) + #method_guards #(.guard(actix_web::guard::fn_guard(#guards)))* #(.wrap(#wrappers))* .#resource_type(#name); diff --git a/actix-web-codegen/tests/test_macro.rs b/actix-web-codegen/tests/test_macro.rs index 13e9120f..dd2bccd7 100644 --- a/actix-web-codegen/tests/test_macro.rs +++ b/actix-web-codegen/tests/test_macro.rs @@ -5,7 +5,9 @@ use std::task::{Context, Poll}; use actix_web::dev::{Service, ServiceRequest, ServiceResponse, Transform}; use actix_web::http::header::{HeaderName, HeaderValue}; use actix_web::{http, test, web::Path, App, Error, HttpResponse, Responder}; -use actix_web_codegen::{connect, delete, get, head, options, patch, post, put, trace}; +use actix_web_codegen::{ + connect, delete, get, head, options, patch, post, put, route, trace, +}; use futures_util::future; // Make sure that we can name function as 'config' @@ -79,6 +81,11 @@ async fn get_param_test(_: Path) -> impl Responder { HttpResponse::Ok() } +#[route("/multi", method = "GET", method = "POST", method = "HEAD")] +async fn route_test() -> impl Responder { + HttpResponse::Ok() +} + pub struct ChangeStatusCode; impl Transform for ChangeStatusCode @@ -172,6 +179,7 @@ async fn test_body() { .service(trace_test) .service(patch_test) .service(test_handler) + .service(route_test) }); let request = srv.request(http::Method::GET, srv.url("/test")); let response = request.send().await.unwrap(); @@ -210,6 +218,22 @@ async fn test_body() { let request = srv.request(http::Method::GET, srv.url("/test")); let response = request.send().await.unwrap(); assert!(response.status().is_success()); + + let request = srv.request(http::Method::GET, srv.url("/multi")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::POST, srv.url("/multi")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::HEAD, srv.url("/multi")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); + + let request = srv.request(http::Method::PATCH, srv.url("/multi")); + let response = request.send().await.unwrap(); + assert!(!response.status().is_success()); } #[actix_rt::test] diff --git a/actix-web-codegen/tests/trybuild.rs b/actix-web-codegen/tests/trybuild.rs index b675947d..1bc2bd25 100644 --- a/actix-web-codegen/tests/trybuild.rs +++ b/actix-web-codegen/tests/trybuild.rs @@ -4,4 +4,24 @@ fn compile_macros() { t.pass("tests/trybuild/simple.rs"); t.compile_fail("tests/trybuild/simple-fail.rs"); + + t.pass("tests/trybuild/route-ok.rs"); + t.compile_fail("tests/trybuild/route-duplicate-method-fail.rs"); + t.compile_fail("tests/trybuild/route-unexpected-method-fail.rs"); + + test_route_missing_method(&t) } + +#[rustversion::stable(1.42)] +fn test_route_missing_method(t: &trybuild::TestCases) { + t.compile_fail("tests/trybuild/route-missing-method-fail-msrv.rs"); +} + +#[rustversion::not(stable(1.42))] +#[rustversion::not(nightly)] +fn test_route_missing_method(t: &trybuild::TestCases) { + t.compile_fail("tests/trybuild/route-missing-method-fail.rs"); +} + +#[rustversion::nightly] +fn test_route_missing_method(_t: &trybuild::TestCases) {} diff --git a/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.rs b/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.rs new file mode 100644 index 00000000..9ce98025 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.rs @@ -0,0 +1,15 @@ +use actix_web::*; + +#[route("/", method="GET", method="GET")] +async fn index() -> impl Responder { + HttpResponse::Ok() +} + +#[actix_web::main] +async fn main() { + let srv = test::start(|| App::new().service(index)); + + let request = srv.get("/"); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} diff --git a/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.stderr b/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.stderr new file mode 100644 index 00000000..8bf857c4 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-duplicate-method-fail.stderr @@ -0,0 +1,11 @@ +error: HTTP Method defined more than once: `GET` + --> $DIR/route-duplicate-method-fail.rs:3:35 + | +3 | #[route("/", method="GET", method="GET")] + | ^^^^^ + +error[E0425]: cannot find value `index` in this scope + --> $DIR/route-duplicate-method-fail.rs:10:49 + | +10 | let srv = test::start(|| App::new().service(index)); + | ^^^^^ not found in this scope diff --git a/actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.rs b/actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.rs new file mode 100644 index 00000000..5c30b57c --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.rs @@ -0,0 +1,15 @@ +use actix_web::*; + +#[route("/")] +async fn index() -> impl Responder { + HttpResponse::Ok() +} + +#[actix_web::main] +async fn main() { + let srv = test::start(|| App::new().service(index)); + + let request = srv.get("/"); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} diff --git a/actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.stderr b/actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.stderr new file mode 100644 index 00000000..f59f6c27 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-missing-method-fail-msrv.stderr @@ -0,0 +1,11 @@ +error: The #[route(..)] macro requires at least one `method` attribute + --> $DIR/route-missing-method-fail-msrv.rs:3:1 + | +3 | #[route("/")] + | ^^^^^^^^^^^^^ + +error[E0425]: cannot find value `index` in this scope + --> $DIR/route-missing-method-fail-msrv.rs:10:49 + | +10 | let srv = test::start(|| App::new().service(index)); + | ^^^^^ not found in this scope diff --git a/actix-web-codegen/tests/trybuild/route-missing-method-fail.rs b/actix-web-codegen/tests/trybuild/route-missing-method-fail.rs new file mode 100644 index 00000000..5c30b57c --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-missing-method-fail.rs @@ -0,0 +1,15 @@ +use actix_web::*; + +#[route("/")] +async fn index() -> impl Responder { + HttpResponse::Ok() +} + +#[actix_web::main] +async fn main() { + let srv = test::start(|| App::new().service(index)); + + let request = srv.get("/"); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} diff --git a/actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr b/actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr new file mode 100644 index 00000000..6d35ea60 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-missing-method-fail.stderr @@ -0,0 +1,13 @@ +error: The #[route(..)] macro requires at least one `method` attribute + --> $DIR/route-missing-method-fail.rs:3:1 + | +3 | #[route("/")] + | ^^^^^^^^^^^^^ + | + = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0425]: cannot find value `index` in this scope + --> $DIR/route-missing-method-fail.rs:10:49 + | +10 | let srv = test::start(|| App::new().service(index)); + | ^^^^^ not found in this scope diff --git a/actix-web-codegen/tests/trybuild/route-ok.rs b/actix-web-codegen/tests/trybuild/route-ok.rs new file mode 100644 index 00000000..bfac56e1 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-ok.rs @@ -0,0 +1,15 @@ +use actix_web::*; + +#[route("/", method="GET", method="HEAD")] +async fn index() -> impl Responder { + HttpResponse::Ok() +} + +#[actix_web::main] +async fn main() { + let srv = test::start(|| App::new().service(index)); + + let request = srv.get("/"); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} diff --git a/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.rs b/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.rs new file mode 100644 index 00000000..f4d8d944 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.rs @@ -0,0 +1,15 @@ +use actix_web::*; + +#[route("/", method="UNEXPECTED")] +async fn index() -> impl Responder { + HttpResponse::Ok() +} + +#[actix_web::main] +async fn main() { + let srv = test::start(|| App::new().service(index)); + + let request = srv.get("/"); + 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 new file mode 100644 index 00000000..3fe49f77 --- /dev/null +++ b/actix-web-codegen/tests/trybuild/route-unexpected-method-fail.stderr @@ -0,0 +1,11 @@ +error: Unexpected HTTP Method: `UNEXPECTED` + --> $DIR/route-unexpected-method-fail.rs:3:21 + | +3 | #[route("/", method="UNEXPECTED")] + | ^^^^^^^^^^^^ + +error[E0425]: cannot find value `index` in this scope + --> $DIR/route-unexpected-method-fail.rs:10:49 + | +10 | let srv = test::start(|| App::new().service(index)); + | ^^^^^ not found in this scope