Adding ->fromSchema() related methods

[Ilyes512]

Can I create a PR to add for ->fromSchema() related methods so one can pass cebe\openapi\spec\OpenApi to it.

The reason I would like to do this is because I would like to read the openapi spec from an url and that seems to be possible with:

<?php

$openapi = Reader::readFromYamlFile('https://raw.githubusercontent.com/OAI/OpenAPI-Specification/3.0.2/examples/v3.0/petstore-expanded.yaml');

Let me know if you would accept a PR for this?

[osteel]

Hi there! I'm not entirely following, sorry.

Can I create a PR to add for ->fromSchema()

Add it to what?

so one can pass cebe\openapi\spec\OpenApi to it

What's that?

Reader::readFromYamlFile

Where is this coming from?

I haven't looked into this package in a while, so as much context as possible is welcome 🙂

[Ilyes512]

So this package uses https://github.com/thephpleague/openapi-psr7-validator which in turn uses https://github.com/DEVizzent/cebe-php-openapi.

DEVizzent/cebe-php-openapi does the heavy lifting in regards to the OpenApi spec.

What I want to achieve in my Symfony application is to say there is an openapi.json schema available at location x. Lets say that openapi schema is: https://raw.githubusercontent.com/OAI/OpenAPI-Specification/3.0.2/examples/v3.0/petstore-expanded.yaml (In my case it's served by the same application at a known endpoint/route).

The cebe\openapi\Reader class has a method readFromJsonFile and readFromYamlFile that both can take a path OR a URL. After reading the file or url you get an instance of cebe\openapi\spec\OpenApi. That instance can be passed to https://github.com/thephpleague/openapi-psr7-validator/blob/master/src/PSR7/ValidatorBuilder.php#L101-L106.

This package already uses the fromYaml, fromJson etc methods in the ValidatorBuilder. With my PR I have also added fromSchema: #49.

[osteel]

Ok thanks, I think I'm with you now. So basically, you need to be able to load an OpenAPI definition from a URL.

cebe\openapi allows for it through Reader::readFromJsonFile and Reader::readFromYamlFile, but this package uses OpenAPI PSR-7 Message Validator which only allows for passing a file or a string, because of the realpath calls it does here, for instance.

But they also provide League\OpenAPIValidation\PSR7\ValidatorBuilder::fromSchema, to which one can pass an instance of cebe\openapi\spec\OpenApi that one has to build themselves, which is what you want to do.

What I don't like so much about this is that, for it to work, one needs to be aware of cebe\openapi\spec\OpenApi and of ways to build an instance of that class (which would need to be documented). The goal of this package is to abstract away as much complexity as possible, and this goes against that spirit IMO.

I do acknowledge that not being able to load an OpenAPI definition from a URL is a limitation though, and would welcome support for that.

We could add some fromYamlUrl and fromJsonUrl methods, but those would also need to be added to ValidatorBuilderInterface, which would be a breaking change.

So ideally, ValidatorBuilder::fromYamlFile and ValidatorBuilder::fromJsonFile should also accept URLs, which they would detect and in which case an instance of cebe\openapi\spec\OpenApi would be built using cebe\openapi's Reader::readFromJsonFile or Reader::readFromYamlFile directly, and that instance would then be passed on to League\OpenAPIValidation\PSR7\ValidatorBuilder::fromSchema.

And Osteel\OpenApi\Testing\ValidatorBuilder::fromYaml and ::fromJson should also be able to route to the right methods in case the submitted definition is an URL.

This way, this package's API remains the same and all we'd need to change in the documentation is that URLs are now also supported.

Does that make sense?

[Ilyes512]

Ok thanks, I think I'm with you now. So basically, you need to be able to load an OpenAPI definition from a URL.

cebe\openapi allows for it through Reader::readFromJsonFile and Reader::readFromYamlFile, but this package uses OpenAPI PSR-7 Message Validator which only allows for passing a file or a string, because of the realpath calls it does here, for instance.

But they also provide League\OpenAPIValidation\PSR7\ValidatorBuilder::fromSchema, to which one can pass an instance of cebe\openapi\spec\OpenApi that one has to build themselves, which is what you want to do.

What I don't like so much about this is that, for it to work, one needs to be aware of cebe\openapi\spec\OpenApi and of ways to build an instance of that class (which would need to be documented). The goal of this package is to abstract away as much complexity as possible, and this goes against that spirit IMO.

I do acknowledge that not being able to load an OpenAPI definition from a URL is a limitation though, and would welcome support for that.

We could add some fromYamlUrl and fromJsonUrl methods, but those would also need to be added to ValidatorBuilderInterface, which would be a breaking change.

So ideally, ValidatorBuilder::fromYamlFile and ValidatorBuilder::fromJsonFile should also accept URLs, which they would detect and in which case an instance of cebe\openapi\spec\OpenApi would be built using cebe\openapi's Reader::readFromJsonFile or Reader::readFromYamlFile directly, and that instance would then be passed on to League\OpenAPIValidation\PSR7\ValidatorBuilder::fromSchema.

And Osteel\OpenApi\Testing\ValidatorBuilder::fromYaml and ::fromJson should also be able to route to the right methods in case the submitted definition is an URL.

This way, this package's API remains the same and all we'd need to change in the documentation is that URLs are now also supported.

Does that make sense?

Makes sense.

1. Check if file exist locally
2. Otherwise check if given string is an absolute url by checking if it passes filter_var($url, FILTER_VALIDATE_URL) and parse_url($url, PHP_URL_SCHEME) should be either http or https.

Do we want to catch all possible exceptions thrown by the underlying package and re-throw a "wrapped" one?

Also see https://github.com/cebe/php-openapi?tab=readme-ov-file#reading-api-description-files-and-resolving-references where it talks about absolute urls.

[osteel]

I would actually swap 1. and 2., as filter_var/parse_url are most likely faster than is_file.

Do we want to catch all possible exceptions thrown by the underlying package and re-throw a "wrapped" one?

Nah, I wouldn't worry about that. I wanted to unify/simplify validation exceptions for predictability/clarity (hence Osteel\OpenApi\Testing\Exceptions\ValidationException), but any exception occurring before that would be related to the setup, and I don't think wrapping them up would bring much value

[Ilyes512]

Not fully happy with the result. At first I thought this was a Symfony bundle..., but that was just an assumption.

I could not really add a test that would actually fetch an remote (absolute) OpenApi spec.

edit:

Note that I have force pushed the changes to the original PR.

[osteel]

Ok thank you. I'll add a look asap but can't promise it'll be this week as I've got quite a bit going on

[osteel]

Hey @Ilyes512, as a sanity check and before I create a new release for this, would you mind pulling main in your project and check that URLs are now correctly supported through fromYamlFile and fromJsonFile?

[Ilyes512]

Hey @Ilyes512, as a sanity check and before I create a new release for this, would you mind pulling main in your project and check that URLs are now correctly supported through fromYamlFile and fromJsonFile?

Hi, I am currently not using this package. I might change that since you added the support, but that would involve a little more than just trying it out quickly. I will eventually look at this and see if I can re-implement my current needs by using this package, but I cannot give you a timeline. So it might take to long before I come back. I would not wait for that :) (could be tomorrow, could also be in a few months).

[osteel]

No worries! I'm not currently using this package for any ongoing project either haha. It's ok, will come up with some test project to make sure it all works as expected