Skip to content

Commit

Permalink
Update actix-web framework (#51)
Browse files Browse the repository at this point in the history
* Update actix-web framework to 4 series
* Change argument parsing to use path to parse argument names. Previously used fn arguments but it is not reliable with actix.
  • Loading branch information
juhaku authored Apr 1, 2022
1 parent 416b326 commit 4363b88
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 128 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ serde_json = { version = "1.0", optional = true }
utoipa-gen = { version = "0.1.0-beta7", path = "./utoipa-gen" }

[dev-dependencies]
actix-web = { version = "3.3" }
actix-web = { version = "4" }
paste = "1.0.6"

[workspace]
Expand Down
100 changes: 98 additions & 2 deletions tests/path_derive_actix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ mod mod_derive_path_actix {
)]
#[get("/foo/{id}")]
#[allow(unused)]
async fn get_foo_by_id(web::Path(id): web::Path<i32>) -> impl Responder {
HttpResponse::Ok().json(json!({ "foo": format!("{:?}", &id) }))
async fn get_foo_by_id(id: web::Path<i32>) -> impl Responder {
HttpResponse::Ok().json(json!({ "foo": format!("{:?}", &id.into_inner()) }))
}
}

Expand Down Expand Up @@ -135,6 +135,102 @@ fn derive_path_with_named_regex_actix_success() {
};
}

#[test]
fn derive_path_with_multiple_args() {
mod mod_derive_path_multiple_args {
use actix_web::{get, web, HttpResponse, Responder};
use serde_json::json;

#[utoipa::path(
responses(
(status = 200, description = "success response")
),
)]
#[get("/foo/{id}/bar/{digest}")]
#[allow(unused)]
async fn get_foo_by_id(path: web::Path<(i64, String)>) -> impl Responder {
let (id, digest) = path.into_inner();
HttpResponse::Ok().json(json!({ "id": &format!("{:?} {:?}", id, digest) }))
}
}

#[derive(OpenApi, Default)]
#[openapi(handlers(mod_derive_path_multiple_args::get_foo_by_id))]
struct ApiDoc;

let doc = serde_json::to_value(ApiDoc::openapi()).unwrap();
let parameters = common::get_json_path(&doc, "paths./foo/{id}/bar/{digest}.get.parameters");

common::assert_json_array_len(parameters, 2);
assert_value! {parameters=>
"[0].in" = r#""path""#, "Parameter in"
"[0].name" = r#""id""#, "Parameter name"
"[0].description" = r#"null"#, "Parameter description"
"[0].required" = r#"true"#, "Parameter required"
"[0].deprecated" = r#"false"#, "Parameter deprecated"
"[0].schema.type" = r#""integer""#, "Parameter schema type"
"[0].schema.format" = r#""int64""#, "Parameter schema format"

"[1].in" = r#""path""#, "Parameter in"
"[1].name" = r#""digest""#, "Parameter name"
"[1].description" = r#"null"#, "Parameter description"
"[1].required" = r#"true"#, "Parameter required"
"[1].deprecated" = r#"false"#, "Parameter deprecated"
"[1].schema.type" = r#""string""#, "Parameter schema type"
"[1].schema.format" = r#"null"#, "Parameter schema format"
};
}

#[test]
fn derive_path_with_multiple_args_with_descriptions() {
mod mod_derive_path_multiple_args {
use actix_web::{get, web, HttpResponse, Responder};
use serde_json::json;

#[utoipa::path(
responses(
(status = 200, description = "success response")
),
params(
("id", description = "Foo id"),
("digest", description = "Foo digest")
)
)]
#[get("/foo/{id}/bar/{digest}")]
#[allow(unused)]
async fn get_foo_by_id(path: web::Path<(i64, String)>) -> impl Responder {
let (id, digest) = path.into_inner();
HttpResponse::Ok().json(json!({ "id": &format!("{:?} {:?}", id, digest) }))
}
}

#[derive(OpenApi, Default)]
#[openapi(handlers(mod_derive_path_multiple_args::get_foo_by_id))]
struct ApiDoc;

let doc = serde_json::to_value(ApiDoc::openapi()).unwrap();
let parameters = common::get_json_path(&doc, "paths./foo/{id}/bar/{digest}.get.parameters");

common::assert_json_array_len(parameters, 2);
assert_value! {parameters=>
"[0].in" = r#""path""#, "Parameter in"
"[0].name" = r#""id""#, "Parameter name"
"[0].description" = r#""Foo id""#, "Parameter description"
"[0].required" = r#"true"#, "Parameter required"
"[0].deprecated" = r#"false"#, "Parameter deprecated"
"[0].schema.type" = r#""integer""#, "Parameter schema type"
"[0].schema.format" = r#""int64""#, "Parameter schema format"

"[1].in" = r#""path""#, "Parameter in"
"[1].name" = r#""digest""#, "Parameter name"
"[1].description" = r#""Foo digest""#, "Parameter description"
"[1].required" = r#"true"#, "Parameter required"
"[1].deprecated" = r#"false"#, "Parameter deprecated"
"[1].schema.type" = r#""string""#, "Parameter schema type"
"[1].schema.format" = r#"null"#, "Parameter schema format"
};
}

macro_rules! test_derive_path_operations {
( $( $name:ident, $mod:ident: $operation:ident)* ) => {
$(
Expand Down
6 changes: 4 additions & 2 deletions tests/path_parameter_derive_actix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ mod derive_params_multiple_actix {
)
)]
#[allow(unused)]
async fn get_foo_by_id(web::Path((id, digest)): web::Path<(i32, String)>) -> impl Responder {
async fn get_foo_by_id(path: web::Path<(i32, String)>) -> impl Responder {
let (id, digest) = path.into_inner();
HttpResponse::Ok().json(json!({ "foo": format!("{:?}{:?}", &id, &digest) }))
}
}
Expand Down Expand Up @@ -124,7 +125,8 @@ mod derive_params_from_method_args_actix {
),
)]
#[allow(unused)]
async fn get_foo_by_id(web::Path((id, digest)): web::Path<(i32, String)>) -> impl Responder {
async fn get_foo_by_id(path: web::Path<(i32, String)>) -> impl Responder {
let (id, digest) = path.into_inner();
HttpResponse::Ok().json(json!({ "foo": format!("{:?}{:?}", &id, &digest) }))
}
}
Expand Down
2 changes: 1 addition & 1 deletion utoipa-gen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ lazy_static = { version = "1.4", optional = true }
[dev-dependencies]
utoipa = { path = ".." }
serde_json = "1"
actix-web = { version = "3.3" }
actix-web = { version = "4" }


[features]
Expand Down
17 changes: 13 additions & 4 deletions utoipa-gen/src/ext.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(unused)]
use proc_macro2::Ident;
use syn::{punctuated::Punctuated, token::Comma, Attribute, FnArg, ItemFn};

Expand All @@ -6,7 +7,7 @@ pub mod actix;

#[cfg_attr(feature = "debug", derive(Debug))]
pub struct Argument<'a> {
pub name: Option<String>,
pub name: Option<&'a str>,
pub argument_in: ArgumentIn,
pub ident: &'a Ident,
}
Expand All @@ -23,20 +24,28 @@ pub enum ArgumentIn {
Path,
}

pub struct ResolvedPath {
pub path: String,
pub args: Vec<String>,
}

pub trait ArgumentResolver {
fn resolve_path_arguments(_: &Punctuated<FnArg, Comma>) -> Option<Vec<Argument<'_>>> {
fn resolve_path_arguments<'a>(
_: &'a Punctuated<FnArg, Comma>,
_: &'a Option<ResolvedPath>,
) -> Option<Vec<Argument<'a>>> {
None
}
}

pub trait PathResolver {
fn resolve_path(_: &Option<&Attribute>) -> Option<String> {
fn resolve_path(_: &Option<String>) -> Option<ResolvedPath> {
None
}
}

pub trait PathOperationResolver {
fn resolve_attribute(_: &ItemFn) -> Option<&Attribute> {
fn resolve_operation(_: &ItemFn) -> Option<&Attribute> {
None
}
}
Expand Down
138 changes: 60 additions & 78 deletions utoipa-gen/src/ext/actix.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg(feature = "actix_extras")]
use std::borrow::Cow;

use lazy_static::lazy_static;
use proc_macro2::Ident;
Expand All @@ -11,41 +11,30 @@ use syn::{

use super::{
Argument, ArgumentIn, ArgumentResolver, PathOperationResolver, PathOperations, PathResolver,
ResolvedPath,
};

impl ArgumentResolver for PathOperations {
fn resolve_path_arguments(fn_args: &Punctuated<FnArg, Comma>) -> Option<Vec<Argument<'_>>> {
if let Some((pat_type, path_segment)) = Self::find_path_pat_type_and_segment(fn_args) {
let names = Self::get_argument_names(pat_type);
let types = Self::get_argument_types(path_segment);

if names.len() != types.len() {
Some(
types
.into_iter()
.map(|ty| Argument {
argument_in: ArgumentIn::Path,
ident: ty,
name: None,
})
.collect::<Vec<_>>(),
)
} else {
Some(
names
.into_iter()
.zip(types.into_iter())
.map(|(name, ty)| Argument {
argument_in: ArgumentIn::Path,
ident: ty,
name: Some(name.to_string()),
})
.collect::<Vec<_>>(),
)
}
} else {
None
}
fn resolve_path_arguments<'a>(
fn_args: &'a Punctuated<FnArg, Comma>,
resolved_path: &'a Option<ResolvedPath>,
) -> Option<Vec<Argument<'a>>> {
resolved_path
.as_ref()
.zip(Self::find_path_pat_type_and_segment(fn_args))
.map(|(path, (_, path_segment))| {
let types = Self::get_argument_types(path_segment);

path.args
.iter()
.zip(types.into_iter())
.map(|(name, ty)| Argument {
argument_in: ArgumentIn::Path,
ident: ty,
name: Some(name),
})
.collect::<Vec<_>>()
})
}
}

Expand Down Expand Up @@ -141,15 +130,9 @@ impl PathOperations {
}

impl PathOperationResolver for PathOperations {
fn resolve_attribute(item_fn: &ItemFn) -> Option<&Attribute> {
fn resolve_operation(item_fn: &ItemFn) -> Option<&Attribute> {
item_fn.attrs.iter().find_map(|attribute| {
if is_valid_request_type(
&attribute
.path
.get_ident()
.map(ToString::to_string)
.unwrap_or_default(),
) {
if is_valid_request_type(attribute.path.get_ident()) {
Some(attribute)
} else {
None
Expand All @@ -159,45 +142,44 @@ impl PathOperationResolver for PathOperations {
}

impl PathResolver for PathOperations {
fn resolve_path(operation_attribute: &Option<&Attribute>) -> Option<String> {
operation_attribute.map(|attribute| {
let lit = attribute.parse_args::<LitStr>().unwrap();
format_path(&lit.value())
})
}
}
fn resolve_path(path: &Option<String>) -> Option<ResolvedPath> {
path.as_ref().map(|path| {
lazy_static! {
static ref RE: Regex = Regex::new(r"\{[a-zA-Z0-9_][^{}]*}").unwrap();
}

fn format_path(path: &str) -> String {
lazy_static! {
static ref RE: Regex = Regex::new(r"\{[a-zA-Z0-9_]*:[^{}]*}").unwrap();
let mut args = Vec::<String>::with_capacity(RE.find_iter(path).count());
ResolvedPath {
path: RE
.replace_all(path, |captures: &Captures| {
let mut capture = captures.get(0).unwrap().as_str().to_string();

if capture.contains("_:") {
// replace unnamed capture with generic 'arg0' name
args.push(String::from("arg0"));
"{arg0}".to_string()
} else if let Some(colon) = capture.find(':') {
// replace colon (:) separated regexp with empty string
let end = capture.len() - 1;
capture.replace_range(colon..end, "");

args.push(String::from(&capture[1..capture.len() - 1]));

capture
} else {
args.push(String::from(&capture[1..capture.len() - 1]));
// otherwise return the capture itself
capture
}
})
.to_string(),
args,
}
})
}

RE.replace_all(path, |captures: &Captures| {
let mut capture = captures.get(0).unwrap().as_str().to_string();

if capture.contains("_:") {
// replace unnamed capture with generic 'arg0' name
"{arg0}".to_string()
} else if capture.contains(':') {
// replace colon (:) separated regexp with empty string
let colon = capture.find(':').unwrap();
let end = capture.len() - 1;
capture.replace_range(colon..end, "");

capture
} else {
// otherwise return the capture itself
capture
}
})
.to_string()
}

fn is_valid_request_type(s: &str) -> bool {
match s {
"get" | "post" | "put" | "delete" | "head" | "connect" | "options" | "trace" | "patch" => {
true
}
_ => false,
}
fn is_valid_request_type(ident: Option<&Ident>) -> bool {
matches!(ident, Some(operation) if ["get", "post", "put", "delete", "head", "connect", "options", "trace", "patch"]
.iter().any(|expected_operation| operation == expected_operation))
}
Loading

0 comments on commit 4363b88

Please sign in to comment.