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

Implement actix schema generator for tagged enums #389

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Siphalor
Copy link

Resolves #97.

About

This pr implements schema generation for complex enums with serde's tag system.

Supported are all types of tagging (see here), except for the enum tuples.
Tuple schema generation is currently not implemented correctly anyway, so that should go into another pr.

This pr currently uses anyOf instead of oneOf to cover some possible edge cases, where serde would just use the first variant that matches (especially visible in untagged enums).

To be able to implement this, this pr adds any_of and const to the internal schema representation.

Up for discussion

  • Usage of anyOf vs. oneOf as described above.
  • Currently the old enum system is kept for enums that use the default tagging (external) and that only have unit variants.
    This makes it effectively impossible to add descriptions for the enum variants.
    Possible workarounds:
    • Check for documentation on each variant and if present, go for the new any_of system.
    • Get rid of the old enum system and just use any_of for everything.

Other notes

  • The files that I worked on in this pr were incredibly huge, making editing more painful than it needs to be.
  • Let me know if there's any more documentation needed :)

@tiagolobocastro
Copy link
Collaborator

Hey @Siphalor, please correct me if I'm wrong, but anyOf is not part of openapi version 2?

@Siphalor
Copy link
Author

Sadly you seem to be right.

It seems like I was over-enthusiastic and got lost in the JSON Schema spec without noticing that OpenAPIv2 doesn't support the latest draft.

Well, maybe this is at least useful as a reference for the v3 implementation then :)

@tiagolobocastro
Copy link
Collaborator

Yes this is not all for naught, it is very useful indeed because we want to eventually output proper openapi v3.
I added the initial dumb v2-v3 conversion but sadly for lack of time did not get to the next step.

@@ -54,6 +54,16 @@ pub trait Schema: Sized {
/// - `serde_json::Value` works for both JSON and YAML.
fn enum_variants(&self) -> Option<&[serde_json::Value]>;

/// More complex enum variants, where each has it's own schema.
fn any_of(&self) -> Option<&Vec<Resolvable<Self>>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

rather than just any_of I wonder if we could have a schema kind that would allow for all the other variants (oneOf, etc)

Copy link
Collaborator

Choose a reason for hiding this comment

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

example:

pub enum SchemaKind {
    Type(crate::v2::models::DataType),
    OneOf {
        #[serde(rename = "oneOf")]
        one_of: Vec<crate::v2::models::Resolvable<Self>>,
    },
    AllOf {
        #[serde(rename = "allOf")]
        all_of: Vec<crate::v2::models::Resolvable<Self>>,
    },
    AnyOf {
        #[serde(rename = "anyOf")]
        any_of: Vec<crate::v2::models::Resolvable<Self>>,
    },
}

@tiagolobocastro
Copy link
Collaborator

If you were so inclined, how about adding a new ApiV3Schema trait (eg in core/v3/schema.rs) containing these new additions?

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.

Actix-web: Support enum variants with named fields
2 participants