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

Add the module path to the definition name to avoid name collisions #373

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ actix2 = ["actix-base", "paperclip-actix/actix2"]
actix3 = ["actix-base", "paperclip-actix/actix3"]
actix-base = ["v2", "paperclip-macros/actix"]
swagger-ui = ["paperclip-actix/swagger-ui"]
path-in-definition = ["paperclip-macros/path-in-definition"]

# OpenAPI support (v2 and codegen)
cli = ["env_logger", "structopt", "git2", "v2", "codegen"]
Expand Down Expand Up @@ -104,4 +105,4 @@ required-features = ["v2", "codegen"]

[[test]]
name = "test_app"
required-features = ["cli", "actix", "uuid", "chrono"]
required-features = ["cli", "actix", "uuid", "chrono", "swagger-ui"]
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ build:

test:
cargo test --all --features "actix cli chrono uuid swagger-ui"

# We test this one seperately as it affects the generated spec, which'd fail the other tests
cargo test test_module_path_in_definition_name --features "actix cli chrono uuid swagger-ui path-in-definition"

# Compile the code generated through tests.
cd tests/test_pet && cargo check
cd tests/test_pet/cli && CARGO_TARGET_DIR=../target cargo check
Expand Down
1 change: 1 addition & 0 deletions macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ strum_macros = { version = "0.23", optional = true }
actix = ["heck", "http", "lazy_static", "strum", "strum_macros"]
v2 = []
nightly = []
path-in-definition = []
70 changes: 63 additions & 7 deletions macros/src/actix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ pub fn emit_v2_definition(input: TokenStream) -> TokenStream {
let docs = docs.trim();

let props = SerdeProps::from_item_attrs(&item_ast.attrs);

let name = &item_ast.ident;

// Add `Apiv2Schema` bound for impl if the type is generic.
Expand Down Expand Up @@ -743,26 +744,81 @@ pub fn emit_v2_definition(input: TokenStream) -> TokenStream {

let schema_name = extract_rename(&item_ast.attrs).unwrap_or_else(|| name.to_string());
let props_gen_empty = props_gen.is_empty();

#[cfg(not(feature = "path-in-definition"))]
let default_schema_raw_def = quote! {
let mut schema = DefaultSchemaRaw {
name: Some(#schema_name.into()),
..Default::default()
};
};

#[cfg(feature = "path-in-definition")]
let default_schema_raw_def = quote! {
let mut schema = DefaultSchemaRaw {
name: Some(Self::__paperclip_schema_name()), // Add name for later use.
.. Default::default()
};
};

#[cfg(not(feature = "path-in-definition"))]
let paperclip_schema_name_def = quote!();

#[cfg(feature = "path-in-definition")]
let paperclip_schema_name_def = quote! {
fn __paperclip_schema_name() -> String {
// The module path itself, e.g cratename::module
let full_module_path = std::module_path!().to_string();
// We're not interested in the crate name, nor do we want :: as a seperator
let trimmed_module_path = full_module_path.split("::")
.enumerate()
.filter(|(index, _)| *index != 0) // Skip the first element, i.e the crate name
.map(|(_, component)| component)
.collect::<Vec<_>>()
.join("_");
format!("{}_{}", trimmed_module_path, #schema_name)
}
};

#[cfg(not(feature = "path-in-definition"))]
let const_name_def = quote! {
const NAME: Option<&'static str> = Some(#schema_name);
};

#[cfg(feature = "path-in-definition")]
let const_name_def = quote!();

#[cfg(not(feature = "path-in-definition"))]
let props_gen_empty_name_def = quote! {
schema.name = Some(#schema_name.into());
};

#[cfg(feature = "path-in-definition")]
let props_gen_empty_name_def = quote! {
schema.name = Some(Self::__paperclip_schema_name());
};

let gen = quote! {
impl #impl_generics paperclip::v2::schema::Apiv2Schema for #name #ty_generics #where_clause {
const NAME: Option<&'static str> = Some(#schema_name);
impl #impl_generics #name #ty_generics #where_clause {
#paperclip_schema_name_def
}

impl #impl_generics paperclip::v2::schema::Apiv2Schema for #name #ty_generics #where_clause {
#const_name_def
const DESCRIPTION: &'static str = #docs;

fn raw_schema() -> paperclip::v2::models::DefaultSchemaRaw {
use paperclip::v2::models::{DataType, DataTypeFormat, DefaultSchemaRaw};
use paperclip::v2::schema::TypedData;

let mut schema = DefaultSchemaRaw {
name: Some(#schema_name.into()), // Add name for later use.
.. Default::default()
};
#default_schema_raw_def

#props_gen
// props_gen may override the schema for unnamed structs with 1 element
// as it replaces the struct type with inner type.
// make sure we set the name properly if props_gen is not empty
if !#props_gen_empty {
schema.name = Some(#schema_name.into());
#props_gen_empty_name_def
}
schema
}
Expand Down
127 changes: 127 additions & 0 deletions tests/test_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3225,3 +3225,130 @@ fn test_rename() {
},
);
}

mod module_path_in_definition_name {
pub mod foo {
pub mod bar {
#[derive(serde::Serialize, paperclip::actix::Apiv2Schema)]
pub struct Baz {
pub a: i32,
pub b: i32,
}
}

pub mod other_bar {
#[derive(serde::Serialize, paperclip::actix::Apiv2Schema)]
pub struct Baz {
pub a: String,
pub b: bool,
}
}
}
}

#[test]
#[cfg(feature = "path-in-definition")]
fn test_module_path_in_definition_name() {
use paperclip::actix::{api_v2_operation, web, OpenApiExt};

#[api_v2_operation]
fn a() -> web::Json<module_path_in_definition_name::foo::bar::Baz> {
web::Json(module_path_in_definition_name::foo::bar::Baz { a: 10, b: 10 })
}

#[api_v2_operation]
fn b() -> web::Json<module_path_in_definition_name::foo::other_bar::Baz> {
web::Json(module_path_in_definition_name::foo::other_bar::Baz {
a: String::default(),
b: true,
})
}

run_and_check_app(
|| {
App::new()
.wrap_api()
.with_json_spec_at("/spec")
.route("/a", web::get().to(a))
.route("/b", web::get().to(b))
.build()
},
|addr| {
let resp = CLIENT
.get(&format!("http://{}/spec", addr))
.send()
.expect("request failed?");

check_json(
resp,
json!({
"definitions": {
"module_path_in_definition_name_foo_bar_Baz": {
"properties": {
"a": {
"format": "int32",
"type": "integer"
},
"b": {
"format": "int32",
"type": "integer"
}
},
"required": [
"a",
"b"
],
"type": "object"
},
"module_path_in_definition_name_foo_other_bar_Baz": {
"properties": {
"a": {
"type": "string"
},
"b": {
"type": "boolean"
}
},
"required": [
"a",
"b"
],
"type": "object"
}
},
"info": {
"title": "",
"version": ""
},
"paths": {
"/a": {
"get": {
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/module_path_in_definition_name_foo_bar_Baz"
}
}
}
}
},
"/b": {
"get": {
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/module_path_in_definition_name_foo_other_bar_Baz"
}
}
}
}
}
},
"swagger": "2.0"
}),
)
},
)
}