php-swagger-test icon indicating copy to clipboard operation
php-swagger-test copied to clipboard

Library does not support 'oneOf', 'anyOf', 'allOf' and 'not'

Open domiTEN opened this issue 4 years ago • 24 comments

Just to let you know that your library does not support oneOf, anyOf, allOf and not tags.

More information: https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/

domiTEN avatar Apr 22 '20 09:04 domiTEN

Hi, I saw there's a (partial) fix for this in the plusForta/php-swagger-test fork. Can we expect those changes to be merged back in the main project here?

Edit: that patch is a very naive implementation that doesn't actually work for nested references. So I'm changing my original question to: Can we expect full support for the oneOf, anyOf, allOf, and not tags?

iskrenvankov avatar Sep 11 '20 17:09 iskrenvankov

I am planning moving on this until the end of the month. But any help is appreciate. Another interesting point: if we see some fork with a cool feature anyone can suggest a PR and we'll analyse with care :+1:

byjg avatar Sep 11 '20 19:09 byjg

Sounds great! At this point I can't offer code assistance but once it's out on some dev branch I can test it against my codebase since I have already incorporated the library and I have particular test cases that fail now and should pass when it works.

iskrenvankov avatar Sep 12 '20 14:09 iskrenvankov

Just encountered this issue today as well. I applied the fix from that commit and it works 100% for my usecase if that helps.

I'm very in favour of getting that PR'd in otherwise I won't be able to use the assertRequest for any of my Schema's :sob:

JohnRoux avatar Sep 14 '20 14:09 JohnRoux

I sent a message since april to the PR owner and I didn't get any answer yet. So, I'll implement by myself then.

byjg avatar Sep 14 '20 14:09 byjg

I merged his PR. I still have to add unit test before integrate to the main branch. If you can produce any swagger.json with these use cases I'd appreaciate and will make it faster.

Here the instructions to use the dev branch: https://github.com/byjg/php-swagger-test/pull/57#issuecomment-692111707

byjg avatar Sep 14 '20 14:09 byjg

I'm actually using this via a laravel wrapper Probably want to write my own wrapper though as had to pull a lot of stuff out of that one

https://dev.saythanks.app/api/documentation https://dev.saythanks.app/docs/api-docs.json

I forked the wrapper and pushed through the 3.1.1.x-dev version. All my auth tests are working :rocket: (See the JWT Schema, it uses AllOf and then 4 props)

JohnRoux avatar Sep 15 '20 08:09 JohnRoux

https://gist.github.com/JohnRoux/9a85c2b7a98d4e5bc1a3a6a45434f244

That's how I'm using it at the moment. Want to pull a large chunk of the helper code out into a laravel package for it though

JohnRoux avatar Sep 15 '20 11:09 JohnRoux

Awesome! This will help a lot.

byjg avatar Sep 15 '20 14:09 byjg

For sure, scream if I can help at all :+1:

JohnRoux avatar Sep 15 '20 14:09 JohnRoux

Can I add your swagger implementation into the project?

byjg avatar Sep 15 '20 14:09 byjg

Ummm, it is a private project still, let me rather generate a fresh example doc for you with some of the specifics taken out.

JohnRoux avatar Sep 15 '20 15:09 JohnRoux

I can't promise that this is great or even good :sweat_smile: But it's technically allowed by the spec and seems to work well for my usecase.

https://gist.github.com/JohnRoux/d2e3963e29c040b3b503675ea95d89af

JohnRoux avatar Sep 16 '20 08:09 JohnRoux

I'll use this to create the unit tests. Thanks :)

byjg avatar Sep 16 '20 16:09 byjg

Bearer of bad news :( That PR didn't actually fix things entirely for allOf.

Context: I use an Item schema which my models "extend" in their own schema's like this

When running the response validation, I get this error: ByJG\ApiTools\Exception\NotMatchedException: The property(ies) 'name, email, type, roles_id, companies_id, created_at, updated_at' has not defined in '#/components/schemas/Item'

So it's clearly found the deepest "allOf" and is trying to match the whole object against that. :(

I think the allOfmatching needs to amalgamate an array all the properties from it's children shema/properties and then match against that single array

JohnRoux avatar Sep 17 '20 16:09 JohnRoux

hi, in our use cases our implementation of allOf works (mentioned in laravel-wrapper), did not have the time to make PR - should i make the time?

pionl avatar Nov 02 '20 13:11 pionl

Yeah please!

JohnRoux avatar Nov 04 '20 09:11 JohnRoux

P.S. a note that enums fall under the same error ByJG\ApiTools\Exception\GenericSwaggerException: Not all cases are defined. Please open an issue about this.

For example:

@OA\Property(
    property="my_enum_property",
    enum={"foo", "bar"}
)

iskrenvankov avatar Nov 08 '20 17:11 iskrenvankov

Ok, I'try to make the time today and send PR 👍

pionl avatar Nov 11 '20 10:11 pionl

Any update on this topic?

kachar avatar Jun 10 '21 16:06 kachar

The implementation was complete, but still some fixes to do.

I have this PR: https://github.com/byjg/php-swagger-test/pull/67 to test. I would be glad if you could help me to test it.

byjg avatar Jun 10 '21 17:06 byjg

@byjg @ODAEL Just tested the PR and it seems it includes only the allOf case and not oneOf scenario so even if that works it still errors with Not all cases are defined

I don't have test setup for allOf so I can't validate that right now.

kachar avatar Jun 11 '21 10:06 kachar

@kachar Could you please provide more details or some examples? As I know, oneOf case should work properly, and your Not all cases are defined may be caused by other problems.

ODAEL avatar Jun 11 '21 10:06 ODAEL

I'll try to setup a testcase, thanks for the update

kachar avatar Jun 11 '21 11:06 kachar