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

fix: openapi schema for literal and enum unions with None. #2550

Merged
merged 10 commits into from Nov 1, 2023

Conversation

peterschutt
Copy link
Contributor

@peterschutt peterschutt commented Oct 28, 2023

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR
  • Pre-Commit Checks were ran and passed
  • Tests were ran and passed

Description

Existing behavior is to make the schema for every type that is a union with None a "one_of" schema, that includes OpenAPIType.NULL in the "one_of" types.

def for_optional_field(self, field_definition: FieldDefinition) -> Schema:
"""Create a Schema for an optional FieldDefinition.
Args:
field_definition: A signature field instance.
Returns:
A schema instance.
"""
schema_or_reference = self.for_field_definition(
FieldDefinition.from_kwarg(
annotation=make_non_optional_union(field_definition.annotation),
name=field_definition.name,
default=field_definition.default,
)
)
if isinstance(schema_or_reference, Schema) and isinstance(schema_or_reference.one_of, list):
result = schema_or_reference.one_of
else:
result = [schema_or_reference]
return Schema(one_of=[Schema(type=OpenAPIType.NULL), *result])

When a Literal or Enum type is in a union with None, this behavior is not desirable, as we want to have null available in the list of available options on the type's schema.

This PR changes literal and enum schema generation so that it can be identified that the types are in a union with None, allowing null to be included in Schema.enum values.

Close Issue(s)

Closes #2546

Copy link
Member

@JacobCoffee JacobCoffee left a comment

Choose a reason for hiding this comment

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

Im going to try this out at work since i hit this issue

litestar/_openapi/schema_generation/utils.py Outdated Show resolved Hide resolved
@JacobCoffee
Copy link
Member

Running with field: Enum | None works well, extra none option, and execute works. But without | None im hitting this still?

{
  "status_code": 400,
  "detail": "Validation failed for GET http://127.0.0.1:8000/",
  "extra": [
    {
      "message": "Expected `str`, got `null`",
      "key": "environment",
      "source": "query"
    }
  ]
}

@JacobCoffee JacobCoffee self-requested a review October 28, 2023 05:00
@peterschutt
Copy link
Contributor Author

Running with field: Enum | None works well, extra none option, and execute works. But without | None im hitting this still?

{

  "status_code": 400,

  "detail": "Validation failed for GET http://127.0.0.1:8000/",

  "extra": [

    {

      "message": "Expected `str`, got `null`",

      "key": "environment",

      "source": "query"

    }

  ]

}

Is this with default=None still declared in Parameter?

@provinzkraut
Copy link
Member

Running with field: Enum | None works well, extra none option, and execute works. But without | None im hitting this still?

{
  "status_code": 400,
  "detail": "Validation failed for GET http://127.0.0.1:8000/",
  "extra": [
    {
      "message": "Expected `str`, got `null`",
      "key": "environment",
      "source": "query"
    }
  ]
}

Can you elaborate? If it's not | None then that error seems to be correct?

Copy link
Member

@provinzkraut provinzkraut left a comment

Choose a reason for hiding this comment

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

Good stuff, just some nits you can feel free to ignore :)

litestar/_openapi/schema_generation/utils.py Outdated Show resolved Hide resolved
litestar/_openapi/schema_generation/utils.py Outdated Show resolved Hide resolved
@peterschutt peterschutt force-pushed the 2456-oia-opt-enums branch 2 times, most recently from 8f62802 to fcc0ee9 Compare November 1, 2023 00:23
peterschutt and others added 10 commits November 1, 2023 13:08
Existing behavior is to make the schema for every type that is a union with `None` a "one_of" schema, that includes `OpenAPIType.NULL` in the "one_of" types.

https://github.com/litestar-org/litestar/blob/5db8d81fbf24788d38d210e90300e5f0926f830a/litestar/_openapi/schema_generation/schema.py#L319-L340

When a `Literal` or `Enum` type is in a union with `None`, this behavior is not desirable, as we want to have `null` available in the list of available options on the type's schema.

This PR changes literal and enum schema generation so that it can be identified that the types are in a union with `None`, allowing `null` to be included in `Schema.enum` values.

Closes #2456
…refactored) (#2551)

'Refactored by Sourcery'

Co-authored-by: Sourcery AI <>
Co-authored-by: Janek Nouvertné <provinzkraut@posteo.de>
Co-authored-by: Janek Nouvertné <provinzkraut@posteo.de>
Copy link

sonarcloud bot commented Nov 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@peterschutt peterschutt merged commit 055607e into main Nov 1, 2023
16 checks passed
@peterschutt peterschutt deleted the 2456-oia-opt-enums branch November 1, 2023 03:18
Copy link

github-actions bot commented Nov 1, 2023

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/2550

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.

Bug: Optionally typed parameters do not render in OpenAPI docs properly
4 participants