search

cebe / php-openapi

Read and write OpenAPI yaml/json files and make the content accessible in PHP objects.
MIT License
466 stars 87 forks source link

Add support for WebHooks #115

Closed WyriHaximus closed 2 years ago

cebe commented 2 years ago

@WyriHaximus please rebase this on the openapi-31 branch. We'll add all 3.1 features to that branch and merge it into master when ready.

WyriHaximus commented 2 years ago

@cebe Will rebase and push later tonight 👍

WyriHaximus commented 2 years ago

@cebe Rebased it, is there any specific testing I should add reside from the existing change?

cebe commented 2 years ago

Webhook property check should depend in the openapi Version. Only require webhook in 3.1 and keep old behavior in 3.0.

Also would be good to have a WebHookTest which checks basic webhook example

WyriHaximus commented 2 years ago

Webhook property check should depend in the openapi Version. Only require webhook in 3.1 and keep old behavior in 3.0.

Done that just now

Also would be good to have a WebHookTest which checks basic webhook example

Doing this tomorrow :+1:

WyriHaximus commented 2 years ago

Rebased now that #129 has been merged.

Also would be good to have a WebHookTest which checks basic webhook example

Doing this tomorrow +1

@cebe Basic variant for this is in (been since nearly two weeks but details :stuck_out_tongue: ), do you want me to expand it, or rely on the tests added from #129?

cebe commented 2 years ago

been since nearly two weeks but details

I'm not better in this, so not judging you ;-)

Tests are fine, I just wonder about implementation. I tried to follow the spec quite stricly in which Objects are described. There is a "Paths" object in the spec but no "Webhooks" object, which is not consistent. Need to decide if it's fine for the implementation to break with the strict dependence on the spec or not.

WyriHaximus commented 2 years ago

I tried to follow the spec quite stricly in which Objects are described. There is a "Paths" object in the spec but no "Webhooks" object, which is not consistent. Need to decide if it's fine for the implementation to break with the strict dependence on the spec or not.

Just dropped the WebHooks class of in a new commit so you can compare both.

cebe commented 2 years ago

Thanks, looks good.

cebe commented 2 years ago

Tests are failing with Error: Class 'cebe\openapi\spec\WebHooks' not found

WyriHaximus commented 2 years ago

Tests are failing with Error: Class 'cebe\openapi\spec\WebHooks' not found

Whoops, want me to squash both commits into one?

WyriHaximus commented 2 years ago

Squashed both commits together

cebe commented 2 years ago

Thank you!

WyriHaximus commented 2 years ago

No problem, looking forward to having full support in