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 validation for mixing fixed and additionalProps #26

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

bhelx
Copy link
Contributor

@bhelx bhelx commented Nov 22, 2024

This is a place where OpenAPI and XTP diverge so let's make an explicit error message

Copy link
Member

@nilslice nilslice left a comment

Choose a reason for hiding this comment

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

LGTM!

Just curious, if someone did want to do roughly this, they would instead add another fixed property that is actually a $ref to a map ?

@bhelx bhelx merged commit 71962b8 into main Nov 22, 2024
3 checks passed
@bhelx bhelx deleted the validate-mixing-fixed-and-additional-props-2 branch November 22, 2024 23:21
@bhelx
Copy link
Contributor Author

bhelx commented Nov 22, 2024

There isn't really a way to model this right now. I'm not sure how we'd model it in every typed languages. In typescript it's simple. in rust maybe, in go, csharp, c no idea? But we will look into it. The recommendation i'd give would be to move the untyped parts into their own property. Example:

   MixedObject:
      description: should not allow mixing fixed and additional props for now
      properties:
        hello:
          type: string
      additionalProperties:
        type: string

becomes

   MixedObject:
      description: should not allow mixing fixed and additional props for now
      properties:
        hello:
          type: string
        metadata:
          type: object
          additionalProperties:
            type: string

but of course if you can't control what the shape looks like, might be tricky.

@nilslice
Copy link
Member

Ah that's even better than I had assumed! Thanks

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.

2 participants