[Bug 30536] New: Embeds should be defined in a single place
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Bug ID: 30536 Summary: Embeds should be defined in a single place Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com CC: tomascohen@gmail.com Right now, our specs need two places: (1) path top level x-koha-embed entry (2) path header parameters definition for x-koha-embed as an enum The first one was the right one in the first place, but JSON::Validator didn't manage that correctly (reliably I'd say). So we used it as in (1). Bug 29810 added (2), which is great and correct, but our 'stash_embed' helper [1] called from Koha/REST/V1/Auth.pm is relying on (1) to build the allow-list for embeds. So we need to have both, and we should fix it. [1] Koha/REST/Plugin/Query.pm:237 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29810 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29810 [Bug 29810] Add embed options documentation -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- *** Bug 30542 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Description copied from bug 30542 - I jumped before I found this bug.. haha
Now that we're documenting, and properly validating, the x-koha-embed parameter we can actually use this parameter definition list instead of the custom x-koha-embed array from the top level of the endpoint definition.
This would reduce overhead of adding embeds.. we would only need to add them to the enum list in the parameter definition instead of to both the parameter definition and a top level x-koha-embed key.
-- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133303 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133303&action=edit Bug 30536: Refer to parameters enum This patch updates the stash_embed method to use the parameters hash enum instead of the top level x-koha-embed custom definition. This way we can remove the requirement for defining this list twice. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Attached a first draft for golfing.. I have a hunch we can now drop at least some of the local validation we're doing in this method as we now validate properly at the openapi plugin level? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133309 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133309&action=edit Bug 30536: Remove validation overhead With the introduction of proper validation of collection headers in OpenAPI we no longer need to do our own validation here. This patch removes the early validation of x-koha-embed headers in preference to letting OpenAPI do it once and only once instead. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm still pottering here.. if we can move the call later, after validation has taken place, we can save ourselves both the validation and param extraction overhead. Not sure how far down the rabbit hole it's worth going for now. Of course.. we'd also want to clean up our spec files here, removing existing x-koha-embed top level keys. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133303|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133313 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133313&action=edit Bug 30536: Refer to parameters enum This patch updates the stash_embed method to use the parameters hash enum instead of the top level x-koha-embed custom definition. This way we can remove the requirement for defining this list twice. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133309|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133314 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133314&action=edit Bug 30536: Remove validation overhead With the introduction of proper validation of collection headers in OpenAPI we no longer need to do our own validation here. This patch removes the early validation of x-koha-embed headers in preference to letting OpenAPI do it once and only once instead. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133315 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133315&action=edit Bug 30536: Use objects->find in Cashups controller This patch removes the direct reference to the koha.embed stashed variable in the Cashups controller and switches us to the modern objects->find approach. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133316 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133316&action=edit Bug 30536: Use objects->find in SMTP Servers controller This patch removes the direct reference to the koha.embed stashed variable in the SMTP controller and switches us to the modern objects->find approach. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133315|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133316|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133314|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133324 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133324&action=edit Bug 30536: Remove validation overhead With the introduction of proper validation of collection headers in OpenAPI we no longer need to do our own validation here. This patch removes the early validation of x-koha-embed headers in preference to letting OpenAPI do it once and only once instead. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 133325 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133325&action=edit Bug 30536: Update spec files This patch removes superflous x-koha-embed defintions at the top level of the endpoint specifications. It also replaces a few x-koha-embeds at the top level with parameter lists where this had been missed in preceeding patches. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Marking as NSO to gather other's opinions -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133313|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133324|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133325|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 133327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133327&action=edit Bug 30536: Unit tests This patch adds tests to make sure there's no behavior change regarding error conditions. When requests include wrong x-koha-embed values, a 400 error should be returned, both in our original implementation or relying on the Mojolicious::Plugin::OpenAPI features. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/query.t => SUCCESS: Tests pass 3. Apply the rest of the patches 4. Repeat 2 => SUCCESS: Tests still pass 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 133328 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133328&action=edit Bug 30536: Refer to parameters enum This patch updates the stash_embed method to use the parameters hash enum instead of the top level x-koha-embed custom definition. This way we can remove the requirement for defining this list twice. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 133329 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133329&action=edit Bug 30536: Remove validation overhead With the introduction of proper validation of collection headers in OpenAPI we no longer need to do our own validation here. This patch removes the early validation of x-koha-embed headers in preference to letting OpenAPI do it once and only once instead. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 133330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=133330&action=edit Bug 30536: Update spec files This patch removes superflous x-koha-embed defintions at the top level of the endpoint specifications. It also replaces a few x-koha-embeds at the top level with parameter lists where this had been missed in preceeding patches. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Bug 30536 depends on bug 29810, which changed state. Bug 29810 Summary: Add embed options documentation https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29810 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30385 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30385 [Bug 30385] Standardize our swagger yaml spec files -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch CC| |victor@tuxayo.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |BLOCKED QA Contact| |m.de.rooy@rijksmuseum.nl CC| |m.de.rooy@rijksmuseum.nl --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QAing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Failed QA --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: [1] I would recommend to add a little bit more text on this POD: =head3 stash_embed - $c->stash_embed( $c->match->endpoint->pattern->defaults->{'openapi.op_spec'} ); + $c->stash_embed( ); =cut [2] api/v1/swagger/paths/acquisitions_baskets.yaml Here you do not specify collectionFormat: csv while you do at acquisitions_funds.yaml. And again you dont in suggestions.yaml. Note that as I understand that it is the default array format, we should not need them. But this looks inconsistent. [3] Please fix t/Koha/REST/Plugin/Query.t It fails now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133327|0 |1 is obsolete| | --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134479 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134479&action=edit Bug 30536: Unit tests This patch adds tests to make sure there's no behavior change regarding error conditions. When requests include wrong x-koha-embed values, a 400 error should be returned, both in our original implementation or relying on the Mojolicious::Plugin::OpenAPI features. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/query.t => SUCCESS: Tests pass 3. Apply the rest of the patches 4. Repeat 2 => SUCCESS: Tests still pass 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133328|0 |1 is obsolete| | --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134480 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134480&action=edit Bug 30536: Refer to parameters enum This patch updates the stash_embed method to use the parameters hash enum instead of the top level x-koha-embed custom definition. This way we can remove the requirement for defining this list twice. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133329|0 |1 is obsolete| | --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134481 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134481&action=edit Bug 30536: Remove validation overhead With the introduction of proper validation of collection headers in OpenAPI we no longer need to do our own validation here. This patch removes the early validation of x-koha-embed headers in preference to letting OpenAPI do it once and only once instead. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #133330|0 |1 is obsolete| | --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134482 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134482&action=edit Bug 30536: Update spec files This patch removes superflous x-koha-embed defintions at the top level of the endpoint specifications. It also replaces a few x-koha-embeds at the top level with parameter lists where this had been missed in preceeding patches. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134483 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134483&action=edit Bug 30536: (QA follow-up) POD + Spec Consistency Well spotted, this patch fixes the specs to be consistent and adds a little detail to the POD as requested. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134484 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134484&action=edit Bug 30536: Fix Query.t Unit Test The stash_embed method no longer validates the query so this patch drops the validation tests and instead just focuses on varifying the correct output structure of the stash after the method call. We have not lost validation however, it now happens only once at the OpenAPI level and tests are present in t/db_dependent/api/v1/query.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Marcel de Rooy from comment #19) I updated the POD a little for you, hopefully it's a little clearer.
[2] api/v1/swagger/paths/acquisitions_baskets.yaml Here you do not specify collectionFormat: csv while you do at acquisitions_funds.yaml. And again you dont in suggestions.yaml. Note that as I understand that it is the default array format, we should not need them. But this looks inconsistent.
Good catch, all sorted in the follow-up.
[3] Please fix t/Koha/REST/Plugin/Query.t It fails now.
Another good catch, I somehow missed this test entirely when I wrote this. Follow-up cleans up the test a little.. as we no longer validate in this method I dropped the validation side of the test and focused instead on just the structure testing. Validation is however now tested in the test I added above at the API level. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|Medium patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134479|0 |1 is obsolete| | --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 134506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134506&action=edit Bug 30536: Unit tests This patch adds tests to make sure there's no behavior change regarding error conditions. When requests include wrong x-koha-embed values, a 400 error should be returned, both in our original implementation or relying on the Mojolicious::Plugin::OpenAPI features. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/query.t => SUCCESS: Tests pass 3. Apply the rest of the patches 4. Repeat 2 => SUCCESS: Tests still pass 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134480|0 |1 is obsolete| | --- Comment #28 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 134507 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134507&action=edit Bug 30536: Refer to parameters enum This patch updates the stash_embed method to use the parameters hash enum instead of the top level x-koha-embed custom definition. This way we can remove the requirement for defining this list twice. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134481|0 |1 is obsolete| | --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 134508 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134508&action=edit Bug 30536: Remove validation overhead With the introduction of proper validation of collection headers in OpenAPI we no longer need to do our own validation here. This patch removes the early validation of x-koha-embed headers in preference to letting OpenAPI do it once and only once instead. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134482|0 |1 is obsolete| | --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 134509 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134509&action=edit Bug 30536: Update spec files This patch removes superflous x-koha-embed defintions at the top level of the endpoint specifications. It also replaces a few x-koha-embeds at the top level with parameter lists where this had been missed in preceeding patches. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134483|0 |1 is obsolete| | --- Comment #31 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 134510 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134510&action=edit Bug 30536: (QA follow-up) POD + Spec Consistency Well spotted, this patch fixes the specs to be consistent and adds a little detail to the POD as requested. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134484|0 |1 is obsolete| | --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 134511 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134511&action=edit Bug 30536: Fix Query.t Unit Test The stash_embed method no longer validates the query so this patch drops the validation tests and instead just focuses on varifying the correct output structure of the stash after the method call. We have not lost validation however, it now happens only once at the OpenAPI level and tests are present in t/db_dependent/api/v1/query.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #33 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to master |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32713 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #34 from Jonathan Druart <jonathan.druart+koha@gmail.com> ---
(2) path header parameters definition for x-koha-embed as an enum
I totally missed that, and regarding the number of occurrences, I am not the only one. Is this a list that need to be updated? % git grep x-koha-embed api/v1/swagger/paths/|grep -v 'name: x-koha-embed' api/v1/swagger/paths/biblios_item_groups.yaml: x-koha-embed: api/v1/swagger/paths/biblios_item_groups.yaml: x-koha-embed: api/v1/swagger/paths/biblios_item_groups.yaml: x-koha-embed: api/v1/swagger/paths/biblios_item_groups.yaml: x-koha-embed: api/v1/swagger/paths/checkouts.yaml: x-koha-embed: api/v1/swagger/paths/erm_agreements.yaml: x-koha-embed: api/v1/swagger/paths/erm_agreements.yaml: x-koha-embed: api/v1/swagger/paths/erm_eholdings_packages.yaml: x-koha-embed: api/v1/swagger/paths/erm_eholdings_packages.yaml: x-koha-embed: api/v1/swagger/paths/erm_eholdings_packages.yaml: x-koha-embed: api/v1/swagger/paths/erm_eholdings_packages_resources.yaml: x-koha-embed: api/v1/swagger/paths/erm_eholdings_titles.yaml: x-koha-embed: api/v1/swagger/paths/erm_eholdings_titles.yaml: x-koha-embed: api/v1/swagger/paths/erm_eholdings_titles_resources.yaml: x-koha-embed: api/v1/swagger/paths/erm_licenses.yaml: x-koha-embed: api/v1/swagger/paths/erm_licenses.yaml: x-koha-embed: api/v1/swagger/paths/erm_users.yaml: x-koha-embed: api/v1/swagger/paths/items.yaml: x-koha-embed: -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I spotted this the other day too and was surprised to see it had come out of sync so quickly. Yes, we should update the guidelines and perhaps even add a QA script to prevent new occurrences. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30536 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32923 --- Comment #36 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I opened bug 30536. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 [Bug 32923] x-koha-embed must be an enum -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org