Summary: | Move the base swagger file to YAML | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | dcook, fridolin.somers, jonathan.druart, kyle, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
21.05.00
|
|
Circulation function: | |||
Bug Depends on: | 28157 | ||
Bug Blocks: | 17314, 29072 | ||
Attachments: |
Bug 28189: Move swagger file to YAML format
Bug 28189: Move swagger file to YAML format Bug 28189: Move swagger file to YAML format Bug 28189: (QA folloq-up) Fix fallback case Bug 28189: Fix PluginRoutes.t |
Description
Tomás Cohen Arazi (tcohen)
2021-04-21 17:18:18 UTC
I decided to make this dependent on bug 28157, which would really benefit from adding some documentation like this. So it is a good first addition. Created attachment 119963 [details] [review] Bug 28189: Move swagger file to YAML format This patch changes the base OpenAPI file (swagger.json) into YAML. The motivation for this, is adding more documentation, in Markdown. JSON doesn't accept multiline strings, so this seems like a good move for readability. To test: 1. Verify your API is functional 2. Apply this patch 3. Repeat 1 => SUCCESS: No changes, really 4. Point your browser to /api/v1/.html => SUCCESS: Some nicely formatted description of the API can be seen. It includes information about x-koha-library. 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> No argument here. YAML is way nicer to work with as a human. Created attachment 120004 [details] [review] Bug 28189: Move swagger file to YAML format This patch changes the base OpenAPI file (swagger.json) into YAML. The motivation for this, is adding more documentation, in Markdown. JSON doesn't accept multiline strings, so this seems like a good move for readability. To test: 1. Verify your API is functional 2. Apply this patch 3. Repeat 1 => SUCCESS: No changes, really 4. Point your browser to /api/v1/.html => SUCCESS: Some nicely formatted description of the API can be seen. It includes information about x-koha-library. 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Tested the api, and api docs and they both work as expected.. A positive change.. going straight for QA. Passed Created attachment 120005 [details] [review] Bug 28189: Move swagger file to YAML format This patch changes the base OpenAPI file (swagger.json) into YAML. The motivation for this, is adding more documentation, in Markdown. JSON doesn't accept multiline strings, so this seems like a good move for readability. To test: 1. Verify your API is functional 2. Apply this patch 3. Repeat 1 => SUCCESS: No changes, really 4. Point your browser to /api/v1/.html => SUCCESS: Some nicely formatted description of the API can be seen. It includes information about x-koha-library. 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Koha/REST/V1.pm: $self->home->rel_file("api/v1/swagger/swagger.json"), Not this one? (In reply to Jonathan Druart from comment #7) > Koha/REST/V1.pm: > $self->home->rel_file("api/v1/swagger/swagger.json"), > > > Not this one? That's interesting.. I didn't look deep into the code when I tested.. I just tested that routes continued to work and that the docs route continued to work.. and in both cases they worked fine.. seems the plugin doesn't care about the file extension.. that's surprising.. I agree though.. it should be corrected here.. well spotted. I'm not sure I'm following the flow but I'm guessing Jonathan was referring to th swagger.json line in the fallback section? (In reply to David Cook from comment #9) > I'm not sure I'm following the flow but I'm guessing Jonathan was referring > to th swagger.json line in the fallback section? Yes, it was a busy Friday. Will fix it tomorrow. Created attachment 120766 [details] [review] Bug 28189: (QA folloq-up) Fix fallback case Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Sorry for the delay. Pushed to master for 21.05, thanks to everybody involved! t/db_dependent/Koha/REST/Plugin/PluginRoutes.t is failing, please fix ASAP. In my understanding the tests are telling us that we are doing something terribly wrong here. We are removing support for routes injected by plugin (as they are written in json). Is that correct? Created attachment 121053 [details] [review] Bug 28189: Fix PluginRoutes.t So, this one was hidden. The failures were: # Failed test 'Bad plugins raise warning' # at t/db_dependent/Koha/REST/Plugin/PluginRoutes.t line 75. # found warning: Warning: Could not load REST API spec bundle: Invalid JSON specification HASH(0x556972b22da0): # found warning: Warning: Could not load REST API spec bundle: Invalid JSON specification HASH(0x5569735b8368): # expected to find warning: (?^u:Could not load REST API spec bundle: Invalid JSON specification) # expected to find warning: (?^u:The resulting spec is invalid. Skipping Bad API Route Plugin) And the correct error was (after a debug warn in JSON::Validator): Warning: Could not load REST API spec bundle: Invalid JSON specification HASH(0x55fd0c3d3160): - /info/version: Expected string - got number. at /usr/share/perl5/JSON/Validator.pm line 165. So this patch fixes it, but I don't understand why it's only failing for plugin routes however. Follow-up pushed to master. Enhancement not pushed to 20.11.x |