FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[JSON Schema] Manage correctly nullability by alanpoulain · Pull Request #3817 · api-platform/core · GitHub

/ core Public

[JSON Schema] Manage correctly nullability - #3817

Merged
alanpoulain merged 1 commit into
api-platform:2.5from
alanpoulain:fix/json-schema-nullability
Nov 8, 2020
ConversationCommits (1)ChecksFiles changed
Merged

[JSON Schema] Manage correctly nullability#3817
alanpoulain merged 1 commit into
api-platform:2.5from
alanpoulain:fix/json-schema-nullability

Conversation

alanpoulain commented Nov 6, 2020

Copy link
Copy Markdown
Member
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tickets N/A
License MIT
Doc PR N/A

Following #3402 and the list of PR from #3807.

In order to validate correctly nullable data against the generated JSON Schema (using assertMatchesResourceItemJsonSchema and assertMatchesResourceCollectionJsonSchema), the schema needs to be compliant with the specification for nullability:

{"type": ["string", "null"]}

However OpenAPI < 3.1 doesn't understand this syntax (OpenAPI 3.1 will at least! https://github.com/OAI/OpenAPI-Specification/releases/tag/3.1.0-rc0) and only understands:

{"type": "string", "nullable": true}

That's why we need a condition when the schema is generated for JSON Schema only.

@Ocramius you may be interested.

Ocramius commented Nov 6, 2020

Copy link
Copy Markdown
Contributor

Overall OK for me - would be easier to just say that we generate OpenAPI 3.1 schema here though 🤷

alanpoulain commented Nov 6, 2020

Copy link
Copy Markdown
Member Author

Not really because we are not in an OpenAPI context here (pure JSON Schema use). However we could also introduce a new key for OpenAPI 3.1, like you suggested in your PR. But maybe when it will be released, don't you think?

Ocramius commented Nov 6, 2020

Copy link
Copy Markdown
Contributor

Wait... OpenAPI 3.1 is still not out yet? :O

alanpoulain commented Nov 6, 2020

Copy link
Copy Markdown
Member Author

Sadly 😢 https://github.com/OAI/OpenAPI-Specification/releases

soyuka commented Nov 7, 2020

Copy link
Copy Markdown
Member

Anyways we're going to support only the latest openapi version, by that I mean that we're probably not going to handle multiple open api versions, too hard to maintain.

@alanpoulain you should also try this on master as the OpenAPI code has been refactored there.

alanpoulain commented Nov 7, 2020

Copy link
Copy Markdown
Member Author

🤔 The code is exactly the same on master, isn't it?

core/src/JsonSchema/TypeFactory.php

Line 172 in eae59df

return array_merge($jsonSchema, ['nullable' => true]);

I think it would be merged without conflicts.

soyuka commented Nov 8, 2020

Copy link
Copy Markdown
Member

The code for the schema yes the code for the open api generation no.

soyuka added the Ready label Nov 8, 2020
alanpoulain merged commit 8a62553 into api-platform:2.5 Nov 8, 2020
guilliamxavier mentioned this pull request Apr 9, 2021
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

soyuka soyuka approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL