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

consider a different handling of allOf constructs #176

Closed
ahl opened this issue Jan 25, 2023 · 2 comments
Closed

consider a different handling of allOf constructs #176

ahl opened this issue Jan 25, 2023 · 2 comments

Comments

@ahl
Copy link
Collaborator

ahl commented Jan 25, 2023

Currently an allOf results in a struct with the component schemas generated and flattened. This is often fine; it has the nice attribute that subordinate types are present in the final type. It can also be imprecise or awkward to use the generated type as the relevant components may be scattered around.

Another way to handle these schemas would be to first "consolidate" all members of the allOf construct. This would effectively mean taking the intersection of all validation criteria (paying attention to the value of additionalProperties). In order to preserve the relationship with the original component types, we could impl From<AllOfType> for ComponentType { ... } and allow users to convert between them (or clone().into::<ComponentType>() to do so non-destructively).

This could be an interesting use case to consider: https://github.com/Marwes/debugserver-types/blob/master/src/schema.json

In particular, we'd want to see a type like:

struct ContinuedEvent {
    event: Continued, // enum Continued { Continued }
    body: ContinuedEventBody,
    type_: EventType, // enum EventType { Event }
    seq: i64
}

impl From<ContinuedEvent> for Event {
    fn from(value: ConteinuedEvent) -> Event {
        Event {
            event: value.event.to_string(),
            body: value.body.to_value(),
            type_: value.type_,
            seq: value.seq,
        }
    }
}

This would likely focus on TypeSpace::maybe_all_of_subclass

ahl added a commit that referenced this issue Sep 23, 2023
ahl added a commit that referenced this issue Sep 23, 2023
Before this PR, the handling of allOf was basically wrong. It worked in a small number of cases, but on the whole it was flawed in concept. Most of the time, it would result in a struct with all subschemas embedded with a #[serde(flatten)] directive. This was broken in many ways, but in particular because allOf constructs are often used to augment, narrow, and expand other types.

This PR instead "merges" the subschemas of an allOf construction. This results in many more types, but also (per our testing), types that compile and are usable which was often not the case previously.

This implements much of what is discussed in #176, but doesn't include the impl From that issue imagines to go from the "merged" type to the component type (i.e. the type that appeared in the allOf array).

Fixes #315 and #370
@ahl
Copy link
Collaborator Author

ahl commented Sep 25, 2023

Note that a big chunk of this is now done. What's missing is the impl From to get to the original type.

@ahl
Copy link
Collaborator Author

ahl commented Dec 24, 2024

Closed in favor of #726

@ahl ahl closed this as completed Dec 24, 2024
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

No branches or pull requests

1 participant