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 v3.1 schema and validation based on it #117

Closed
wants to merge 1 commit into from

Conversation

WyriHaximus
Copy link

No description provided.

@mariosimao
Copy link

Hi @cebe, any plans regarding this PR? Is there something missing that I could help?

@WyriHaximus
Copy link
Author

Hey @cebe I hate to ping, but ping?

@Mark-H
Copy link

Mark-H commented Sep 21, 2021

Had a quick play with this, validate seems to do the trick validating a 3.1 schema. Compared it with Spectral's validation for a spec written in the Stoplight editor.

With a valid schema:

➜  vendor/bin/php-openapi validate api-schema/openapi.yaml
The supplied API Description validates against the OpenAPI v3.1 schema.
➜  spectral lint api-schema/openapi.yaml
No results with a severity of 'error' or higher found!

And when breaking some random stuff in the schema to see it catches the issues it's also flagging what I'd expect:

➜  vendor/bin/php-openapi validate api-schema/openapi.yaml
Errors found while reading the API description from api-schema/openapi.yaml:
- [/info] Info is missing required property: version
- [/paths/~1recipes/get/parameters/1] Parameter is missing required property: name
- [/paths/~1recipes/get/parameters/1] Parameter is missing required property: in
OpenAPI v3.1 schema violations:
- [info.version] The property version is required
➜  spectral lint api-schema/openapi.yaml

/path/to/api-schema/openapi.yaml
   1:1   warning  oas3-api-servers  OpenAPI "servers" must be present and non-empty array.
   2:6     error  oas3-schema       "info" property must have required property "version".  info
 115:11    error  oas3-schema       "1" property must have required property "in".          paths./recipes.get.parameters[1]

✖ 3 problems (2 errors, 1 warning, 0 infos, 0 hints)

convert is also working as expected on the schema and I can also walk through the schema with the PHP API, e.g.:

<?php
require __DIR__ . "/vendor/autoload.php";

$openapi = \cebe\openapi\Reader::readFromYamlFile(realpath(__DIR__ . '/api-schema/openapi.yaml'));
$recipe = $openapi->paths['/recipes/{recipeId}'];
var_dump($recipe->get->responses->getResponse(200)->content['application/json']->schema->properties);

I'm not too well-versed in the openapi 3.0/3.1 spec differences to say if this might need more work to complete the 3.1 integration but so far this seems to be working well.

@Jean85
Copy link

Jean85 commented Sep 21, 2021

Other differences that we need to implement to fully support 3.1 are listed in #101

@WyriHaximus
Copy link
Author

Got a bunch of that ready to be PRed once this PR is in.

@cebe cebe added this to the 1.6.0 milestone Oct 13, 2021
@@ -0,0 +1,1347 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

@cebe you provide a make file script schemas/openapi-v3.0.json and schemas/openapi-v3.0.yaml.
Both scripts use the composer virtual dependency oai/openapi-specification which also needs to be updated.

This Pull Request didn't update these files. Should it be removed, or would you prefer an update of the package and make file?

Copy link
Author

Choose a reason for hiding this comment

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

@marcelthole Can look at that if desired, don't recall how I got the 3.1 files but probably just downloaded them.

Copy link
Owner

Choose a reason for hiding this comment

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

did that in #128

@cebe cebe mentioned this pull request Oct 18, 2021
3 tasks
@cebe
Copy link
Owner

cebe commented Oct 18, 2021

@WyriHaximus thanks for starting this, I made a slightly different implementation in #128, which replaces this PR but includes most of your changes.

@cebe cebe closed this Oct 18, 2021
@WyriHaximus
Copy link
Author

@cebe Sweet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants