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 support for unit type () #464

Merged
merged 7 commits into from
Feb 6, 2023
Merged

Add support for unit type () #464

merged 7 commits into from
Feb 6, 2023

Conversation

biwecka
Copy link
Contributor

@biwecka biwecka commented Jan 27, 2023

Fixes #448.

…ple' has no elements and map to 'TypeTreeValue::UnitType' in this case; implement conversion from 'TypeTreeValue::UnitType' to 'TypeTree'
…for 'SchemaProperty<'_>' to create an OpenAPI object for the unit type; change conversion implementation from 'TypeTreeValue::UnitType' to 'TypeTree' to mark the unit type as 'ValueType::Tupple'
@biwecka biwecka marked this pull request as ready for review January 27, 2023 10:44
Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty good. Then there is 2 places where the ValueType::Tuple is being used. You might want to update them as well.

  • into_params.rs
  • media_type.rs

Also add a test cases for these in utoipa-gen/test/schema_derive_test.rs and for IntoParams you can add test somewhere where IntoParams tests have been added before.

Run clippy and cargo fmt for the project as well. :)

utoipa-gen/src/component/schema.rs Outdated Show resolved Hide resolved
Apply code suggestion.

Co-authored-by: Juha Kukkonen <juha7kukkonen@gmail.com>
@biwecka
Copy link
Contributor Author

biwecka commented Jan 27, 2023

What exactly do you mean with IntoParams tests? Should I simply use #[derive(ToSchema)] on a struct?

@juhaku
Copy link
Owner

juhaku commented Jan 27, 2023

What exactly do you mean with IntoParams tests? Should I simply use #[derive(ToSchema)] on a struct?

For ToSchema you can find tests here: https://github.com/juhaku/utoipa/blob/master/utoipa-gen/tests/schema_derive_test.rs

But for IntoParams you can find tests for example here: https://github.com/juhaku/utoipa/blob/master/utoipa-gen/tests/path_derive.rs

IntoParams is another struct what is used to parse path parameters for the operation.

…nit type; implement tests for unit type support
@biwecka
Copy link
Contributor Author

biwecka commented Jan 28, 2023

Thank you for your help! I applied your code suggestion, updated the other two places where ValueType::Tuple is used, implemented tests and made sure clippy and fmt don't complain about my code. Unfortunately I'm still not too sure about the tests. So please review them and tell me if that's not what you expected.

Comment on lines +387 to +398
impl<'__s> ToSchema<'__s> for () {
fn schema() -> (&'__s str, openapi::RefOr<openapi::schema::Schema>) {
(
"UnitType",
openapi::schema::ObjectBuilder::new()
.nullable(true)
.default(Some(serde_json::Value::Null))
.into(),
)
}
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I belive this is unnecessary since the implementation is handled in utoipa-gen where the tuple type with zero arguments is being tokenized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this originates in me trying to use () in a generic like this:

type UT = ();
#[derive(ToSchema)]
#[aliases(StrData = Data<UT>)]
pub struct Data<T> {
    #[allow(unused)]
    value: T
}

#[utoipa::path(
    get,
    path = "/root",
    responses (
        ( status = 200, description = "", body = StrData )
    )
)]
pub fn root() -> () {}

It's not possible to use () in aliases like this #[aliases(StrData = Data<()>)]. Therefore I created the type alias type UT = (); and used it in #[aliases(...)]. With this change the "value" property is a reference with "$ref": "#/components/schemas/UT". And for this reference to resolve I need to add UT to the schemas components(schemas(StrData, UT)).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

#[utoipa::path(
post,
path = "/unit_type_test",
request_body = UT
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering whether you are able now to directly use the request_body = () because the change you made to media_type_schema.rs file for the tuple

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked and it's not possible. I get the following error message: "ValueType::Primitive must have path".

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Hm.. then there is something else missing as well.. Though I can check this later as well. It should not be primitive type but ValueType::Tuple

@juhaku
Copy link
Owner

juhaku commented Jan 29, 2023

Here is a example test for IntoParams usage. IntoParams will derive from struct argument path operation parameters.
You could create another test like this but to use IntoParams as follows. It should work because the change you did in the to_tokens in into_params.rs and to the TypeTree

#[derive(IntoParams)]
struct Params {
    value: (),
}

fn derive_path_params_into_params_with_raw_identifier() {
#[derive(IntoParams)]
#[into_params(parameter_in = Path)]
struct Filter {
#[allow(unused)]
r#in: String,
}
#[utoipa::path(
get,
path = "foo",
responses(
(status = 200, description = "success response")
),
params(
Filter
)
)]
#[allow(unused)]
fn get_foo(query: Filter) {}
#[derive(OpenApi, Default)]
#[openapi(paths(get_foo))]
struct ApiDoc;
let doc = serde_json::to_value(ApiDoc::openapi()).unwrap();
let parameters = doc.pointer("/paths/foo/get/parameters").unwrap();
assert_json_eq!(
parameters,
json!([{
"in": "path",
"name": "in",
"required": true,
"schema": {
"type": "string"
}
}])
)
}

@biwecka
Copy link
Contributor Author

biwecka commented Jan 30, 2023

Thank's again for your help! It's still hard for me to understand how everything works together in utoipa. As you suggested I created a test very similar to the one you mentioned and it passes as expected 👍

@juhaku juhaku merged commit c4564ce into juhaku:master Feb 6, 2023
@juhaku
Copy link
Owner

juhaku commented Feb 6, 2023

Sorry to keep you await, merged it finally

@biwecka
Copy link
Contributor Author

biwecka commented Feb 7, 2023

No problem, thank you very much 👍

@juhaku juhaku mentioned this pull request Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for unit type
2 participants