From 828be281994de79b242dfce1185158b2d39f8573 Mon Sep 17 00:00:00 2001 From: Jon Lim Date: Tue, 2 Apr 2024 22:13:13 -0700 Subject: [PATCH] clean up code. fix bugs with route and method attributes with parameters --- actix-web-codegen/src/lib.rs | 69 ++--------- actix-web-codegen/src/route.rs | 64 +++++----- actix-web-codegen/tests/test_macro.rs | 166 ++++++++++---------------- 3 files changed, 111 insertions(+), 188 deletions(-) diff --git a/actix-web-codegen/src/lib.rs b/actix-web-codegen/src/lib.rs index 5d7a2b46b..1aeb6dd35 100644 --- a/actix-web-codegen/src/lib.rs +++ b/actix-web-codegen/src/lib.rs @@ -254,70 +254,19 @@ pub fn test(_: TokenStream, item: TokenStream) -> TokenStream { /// /// # Example /// -/// scope("/test") /// ```rust /// use actix_web_codegen::{scope}; /// -/// const mod_inner: () = { -/// use actix_web::{HttpResponse, HttpRequest, Responder, put, head, connect, options, trace, patch, delete, web }; -/// use actix_web::web::Json; -/// -/// #[actix_web::get("/test")] -/// pub async fn test() -> impl Responder { -/// mod_test2() +/// #[scope("/test")] +/// mod scope_module { +/// use actix_web::{get, HttpResponse, Responder}; +/// +/// #[get("/test")] +/// pub async fn test() -> impl Responder { +/// // this has path /test/test +/// HttpResponse::Ok().finish() +/// } /// } -/// -/// #[actix_web::get("/twicetest/{value}")] -/// pub async fn test_twice(value: web::Path) -> impl actix_web::Responder { -/// let int_value: i32 = value.parse().unwrap_or(0); -/// let doubled = int_value * 2; -/// HttpResponse::Ok().body(format!("Twice value: {}", doubled)) -/// } -/// -/// #[actix_web::post("/test")] -/// pub async fn test_post() -> impl Responder { -/// HttpResponse::Ok().body(format!("post works")) -/// } -/// -/// #[put("/test")] -/// pub async fn test_put() -> impl Responder { -/// HttpResponse::Ok().body(format!("put works")) -/// } -/// -/// #[head("/test")] -/// pub async fn test_head() -> impl Responder { -/// HttpResponse::Ok().finish() -/// } -/// -/// #[connect("/test")] -/// pub async fn test_connect() -> impl Responder { -/// HttpResponse::Ok().body("CONNECT works") -/// } -/// -/// #[options("/test")] -/// pub async fn test_options() -> impl Responder { -/// HttpResponse::Ok().body("OPTIONS works") -/// } -/// -/// #[trace("/test")] -/// pub async fn test_trace(req: HttpRequest) -> impl Responder { -/// HttpResponse::Ok().body(format!("TRACE works: {:?}", req)) -/// } -/// -/// #[patch("/test")] -/// pub async fn test_patch() -> impl Responder { -/// HttpResponse::Ok().body(format!("patch works")) -/// } -/// -/// #[delete("/test")] -/// pub async fn test_delete() -> impl Responder { -/// HttpResponse::Ok().body("DELETE works") -/// } -/// -/// pub fn mod_test2() -> impl actix_web::Responder { -/// actix_web::HttpResponse::Ok().finish() -/// } -///}; /// ``` /// #[proc_macro_attribute] diff --git a/actix-web-codegen/src/route.rs b/actix-web-codegen/src/route.rs index 87ba92bfb..ea2fd48aa 100644 --- a/actix-web-codegen/src/route.rs +++ b/actix-web-codegen/src/route.rs @@ -4,8 +4,7 @@ use actix_router::ResourceDef; use proc_macro::TokenStream; use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{quote, ToTokens, TokenStreamExt}; -use syn::Item::Fn; -use syn::{punctuated::Punctuated, Ident, ItemFn, LitStr, Path, Token}; // todo: cleanup +use syn::{punctuated::Punctuated, Ident, LitStr, Path, Token}; #[derive(Debug)] pub struct RouteArgs { @@ -332,14 +331,14 @@ pub struct Route { args: Vec, /// AST of the handler function being annotated. - ast: ItemFn, + ast: syn::ItemFn, /// The doc comment attributes to copy to generated struct, if any. doc_attributes: Vec, } impl Route { - pub fn new(args: RouteArgs, ast: ItemFn, method: Option) -> syn::Result { + pub fn new(args: RouteArgs, ast: syn::ItemFn, method: Option) -> syn::Result { let name = ast.sig.ident.clone(); // Try and pull out the doc comments so that we can reapply them to the generated struct. @@ -375,7 +374,7 @@ impl Route { }) } - fn multiple(args: Vec, ast: ItemFn) -> syn::Result { + fn multiple(args: Vec, ast: syn::ItemFn) -> syn::Result { let name = ast.sig.ident.clone(); // Try and pull out the doc comments so that we can reapply them to the generated struct. @@ -484,7 +483,7 @@ pub(crate) fn with_method( Err(err) => return input_and_compile_error(input, err), }; - let ast = match syn::parse::(input.clone()) { + let ast = match syn::parse::(input.clone()) { Ok(ast) => ast, // on parse error, make IDEs happy; see fn docs Err(err) => return input_and_compile_error(input, err), @@ -498,7 +497,7 @@ pub(crate) fn with_method( } pub(crate) fn with_methods(input: TokenStream) -> TokenStream { - let mut ast = match syn::parse::(input.clone()) { + let mut ast = match syn::parse::(input.clone()) { Ok(ast) => ast, // on parse error, make IDEs happy; see fn docs Err(err) => return input_and_compile_error(input, err), @@ -556,15 +555,6 @@ fn input_and_compile_error(mut item: TokenStream, err: syn::Error) -> TokenStrea item } -// todo: regular new scoped path test... -// todo: test with enums in module... -// todo: test with different namespaces... -// todo: test with multiple get/post methods... -// todo: test with doc strings in attributes... -// todo: clean up the nested tree of death, tell it about the input_and_compile_error convenience function... -// todo: add in the group functions for convenient handling of group... -// todo: see if can cleanup the Attribute with a clone plus one field change... -// todo: #[actix_web::get("/test")] the namespace is messing matching up pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream { // Attempt to parse the scope path, returning on error if args.is_empty() { @@ -576,7 +566,7 @@ pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream { ), ); } - let scope_path = syn::parse::(args.clone().into()); + let scope_path = syn::parse::(args.clone()); if let Err(_err) = scope_path { return input_and_compile_error( args.clone(), @@ -590,10 +580,10 @@ pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream { // Expect macro to be for a module match syn::parse::(input) { Ok(mut ast) => { - // Modify the attributes of functions within the module with method with scope, if any + // Modify the attributes of functions with method or route(s) by adding scope argument as prefix, if any if let Some((_, ref mut items)) = ast.content { items.iter_mut().for_each(|item| { - if let Fn(fun) = item { + if let syn::Item::Fn(fun) = item { fun.attrs = fun .attrs .iter() @@ -615,19 +605,37 @@ pub fn with_scope(args: TokenStream, input: TokenStream) -> TokenStream { } } +fn has_allowed_methods_in_scope(attr: &syn::Attribute) -> bool { + MethodType::from_path(attr.path()).is_ok() + || attr.path().is_ident("route") + || attr.path().is_ident("ROUTE") +} + // Check if the attribute is a method type and has a route path, then modify it fn modify_attribute_with_scope(attr: &syn::Attribute, scope_path: &str) -> syn::Attribute { - match ( - MethodType::from_path(attr.path()), - attr.parse_args::(), - attr.clone().meta, - ) { - (Ok(_method), Ok(route_args), syn::Meta::List(meta_list)) => { + match (attr.parse_args::(), attr.clone().meta) { + (Ok(route_args), syn::Meta::List(meta_list)) if has_allowed_methods_in_scope(attr) => { let modified_path = format!("{}{}", scope_path, route_args.path.value()); - let modified_tokens = quote! { #modified_path }; - syn::Attribute { + + let options_tokens: Vec = route_args + .options + .iter() + .map(|option| { + quote! { ,#option } + }) + .collect(); + + let combined_options_tokens: TokenStream2 = + options_tokens + .into_iter() + .fold(TokenStream2::new(), |mut acc, ts| { + acc.extend(std::iter::once(ts)); + acc + }); + + return syn::Attribute { meta: syn::Meta::List(syn::MetaList { - tokens: modified_tokens, + tokens: quote! { #modified_path #combined_options_tokens }, ..meta_list.clone() }), ..attr.clone() diff --git a/actix-web-codegen/tests/test_macro.rs b/actix-web-codegen/tests/test_macro.rs index 418812c1b..b6cacd7d9 100644 --- a/actix-web-codegen/tests/test_macro.rs +++ b/actix-web-codegen/tests/test_macro.rs @@ -6,7 +6,6 @@ use actix_web::{ http::{ self, header::{HeaderName, HeaderValue}, - StatusCode, }, web, App, Error, HttpRequest, HttpResponse, Responder, }; @@ -371,26 +370,16 @@ async fn test_auto_async() { let response = request.send().await.unwrap(); assert!(response.status().is_success()); } -/* -#[actix_web::test] -async fn test_wrap() { - let srv = actix_test::start(|| App::new().service(get_wrap)); - let request = srv.request(http::Method::GET, srv.url("/test/wrap")); - let mut response = request.send().await.unwrap(); - assert_eq!(response.status(), StatusCode::NOT_FOUND); - assert!(response.headers().contains_key("custom-header")); - let body = response.body().await.unwrap(); - let body = String::from_utf8(body.to_vec()).unwrap(); - assert!(body.contains("wrong number of parameters")); -} -*/ #[scope("/test")] mod scope_module { - use actix_web::{ - get, post, connect, delete, head, options, patch, put, trace, web, HttpRequest, - HttpResponse, Responder, - }; + use crate::guard_module; + use actix_web::{delete, get, post, route, routes, web, HttpResponse, Responder}; + + #[get("/test/guard", guard = "guard_module::guard")] + pub async fn test_guard() -> impl Responder { + HttpResponse::Ok() + } #[get("/test")] pub async fn test() -> impl Responder { @@ -409,67 +398,59 @@ mod scope_module { HttpResponse::Ok().body(format!("post works")) } - #[put("/test")] - pub async fn test_put() -> impl Responder { - HttpResponse::Ok().body(format!("put works")) - } - - #[head("/test")] - pub async fn test_head() -> impl Responder { - HttpResponse::Ok().finish() - } - - #[connect("/test")] - pub async fn test_connect() -> impl Responder { - HttpResponse::Ok().body("connect works") - } - - #[options("/test")] - pub async fn test_options() -> impl Responder { - HttpResponse::Ok().body("options works") - } - - #[trace("/test")] - pub async fn test_trace(req: HttpRequest) -> impl Responder { - HttpResponse::Ok().body(format!("trace works: {:?}", req)) - } - - #[patch("/test")] - pub async fn test_patch() -> impl Responder { - HttpResponse::Ok().body(format!("patch works")) - } - #[delete("/test")] pub async fn test_delete() -> impl Responder { HttpResponse::Ok().body("delete works") } - pub fn mod_test2() -> impl actix_web::Responder { - actix_web::HttpResponse::Ok().finish() + #[route("/test", method = "PUT", method = "PATCH", method = "CUSTOM")] + pub async fn test_multiple_shared_path() -> impl Responder { + HttpResponse::Ok().finish() } + #[routes] + #[head("/test")] + #[connect("/test")] + #[options("/test")] + #[trace("/test")] + async fn test_multiple_separate_paths() -> impl Responder { + HttpResponse::Ok().finish() + } + + // test calling this from other mod scope with scope attribute... pub fn mod_common(message: String) -> impl actix_web::Responder { HttpResponse::Ok().body(message) } } #[scope("/v1")] -mod mod_inner_v1 { +mod mod_scope_v1 { use actix_web::{get, Responder}; #[get("/test")] + #[doc = "doc string to check in cargo expand"] pub async fn test() -> impl Responder { super::scope_module::mod_common("version1 works".to_string()) } } #[scope("/v2")] -mod mod_inner_v2 { +mod mod_scope_v2 { use actix_web::{get, Responder}; + // check to make sure non-function tokens in the scope block are preserved... + enum TestEnum { + Works, + } + #[get("/test")] pub async fn test() -> impl Responder { - super::scope_module::mod_common("version2 works".to_string()) + // make sure this type still exists... + let test_enum = TestEnum::Works; + + match test_enum { + TestEnum::Works => super::scope_module::mod_common("version2 works".to_string()), + } } } @@ -505,67 +486,37 @@ async fn test_scope_post_async() { } #[actix_rt::test] -async fn test_scope_put_async() { - let srv = actix_test::start(|| App::new().service(scope_module::test_put)); +async fn test_multiple_shared_path_async() { + let srv = actix_test::start(|| App::new().service(scope_module::test_multiple_shared_path)); let request = srv.request(http::Method::PUT, srv.url("/test/test")); - let mut response = request.send().await.unwrap(); - let body = response.body().await.unwrap(); - let body_str = String::from_utf8(body.to_vec()).unwrap(); - assert_eq!(body_str, "put works"); -} + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); -#[actix_rt::test] -async fn test_scope_head_async() { - let srv = actix_test::start(|| App::new().service(scope_module::test_head)); - - let request = srv.request(http::Method::HEAD, srv.url("/test/test")); + let request = srv.request(http::Method::PATCH, srv.url("/test/test")); let response = request.send().await.unwrap(); assert!(response.status().is_success()); } #[actix_rt::test] -async fn test_scope_connect_async() { - let srv = actix_test::start(|| App::new().service(scope_module::test_connect)); +async fn test_multiple_multipaths_async() { + let srv = actix_test::start(|| App::new().service(scope_module::test_multiple_separate_paths)); let request = srv.request(http::Method::CONNECT, srv.url("/test/test")); - let mut response = request.send().await.unwrap(); - let body = response.body().await.unwrap(); - let body_str = String::from_utf8(body.to_vec()).unwrap(); - assert_eq!(body_str, "connect works"); -} - -#[actix_rt::test] -async fn test_scope_options_async() { - let srv = actix_test::start(|| App::new().service(scope_module::test_options)); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); let request = srv.request(http::Method::OPTIONS, srv.url("/test/test")); - let mut response = request.send().await.unwrap(); - let body = response.body().await.unwrap(); - let body_str = String::from_utf8(body.to_vec()).unwrap(); - assert_eq!(body_str, "options works"); -} - -#[actix_rt::test] -async fn test_scope_trace_async() { - let srv = actix_test::start(|| App::new().service(scope_module::test_trace)); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); let request = srv.request(http::Method::TRACE, srv.url("/test/test")); - let mut response = request.send().await.unwrap(); - let body = response.body().await.unwrap(); - let body_str = String::from_utf8(body.to_vec()).unwrap(); - assert!(body_str.contains("trace works")); -} + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); -#[actix_rt::test] -async fn test_scope_patch_async() { - let srv = actix_test::start(|| App::new().service(scope_module::test_patch)); - - let request = srv.request(http::Method::PATCH, srv.url("/test/test")); - let mut response = request.send().await.unwrap(); - let body = response.body().await.unwrap(); - let body_str = String::from_utf8(body.to_vec()).unwrap(); - assert_eq!(body_str, "patch works"); + let request = srv.request(http::Method::HEAD, srv.url("/test/test")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); } #[actix_rt::test] @@ -579,9 +530,24 @@ async fn test_scope_delete_async() { assert_eq!(body_str, "delete works"); } +#[actix_rt::test] +async fn test_scope_get_with_guard_async() { + let srv = actix_test::start(|| App::new().service(scope_module::test_guard)); + + let request = srv + .request(http::Method::GET, srv.url("/test/test/guard")) + .insert_header(("Accept", "image/*")); + let response = request.send().await.unwrap(); + assert!(response.status().is_success()); +} + #[actix_rt::test] async fn test_scope_v1_v2_async() { - let srv = actix_test::start(|| App::new().service(mod_inner_v1::test).service(mod_inner_v2::test)); + let srv = actix_test::start(|| { + App::new() + .service(mod_scope_v1::test) + .service(mod_scope_v2::test) + }); let request = srv.request(http::Method::GET, srv.url("/v1/test")); let mut response = request.send().await.unwrap();