Bug 20134

Summary: Remove /api/v1/app.pl from the generated URLs
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P5 - low CC: josef.moravec, julian.maurice, katrin.fischer, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 20134: Remove /api/v1/app.pl from the generated URLs
Bug 20134: Remove /api/v1/app.pl from the generated URLs
Bug 20134: Remove /api/v1/app.pl from the generated URLs

Description Tomás Cohen Arazi 2018-02-05 13:25:11 UTC
The REST api is exposed using /api/v1, but that is rewritten to hit /api/v1/app.pl/api/v1 (app.pl being a Mojolicious::Command wrapper for running in Mojolicious under Plack). The problem is that Mojolicious then creates URLs containing that unneeded /api/v1/app.pl prefix. We should strip that out.

To reproduce:
- Point your browser to http://your_host_name/api/v1/
(don't forget about the trailing slash)
=> FAIL: Notice basePath: "/api/v1/app.pl/api/v1"
Comment 1 Tomás Cohen Arazi 2018-02-05 18:09:40 UTC
Created attachment 71239 [details] [review]
Bug 20134: Remove /api/v1/app.pl from the generated URLs

This patch removes the /api/v1/app.pl prefix from generated baseUrl.
It has the consequence of hardcoding / as the base path for the API.

This is ok because we don't currently support mounting Koha pieces in
different than hardcoded paths (/cgi-bin/koha?), but certainly worth
mentioning because this will need to depend on a syspref or config entry
when someone chages this behviour.

To test:
- On master, point your browser to:
  http://kohadev.mydnsname.org:8080/api/v1/
=> FAIL: basePath looks like /api/v1/app.pl/api/v1
- Apply this patch
- Restart memcached and plack:
  $ restart_all
or
  $ sudo systemctl restart memcached && sudo systemctl restart
  koha-common
- Reload http://kohadev.mydnsname.org:8080/api/v1/
=> SUCCESS: basePath is /api/v1
- Sign off :-D
Comment 2 Julian Maurice 2018-02-06 08:35:55 UTC
I just discovered that I can go to http://kohadev.mydnsname.org:8080/api/v1.html to have the API spec in a readable format, which is nice :)
And also that it doesn't work in CGI mode (http://kohadev.mydnsname.org/api/v1/.json works curiously, but not the html version ; etc/koha-httpd.conf probably needs some changes)

Anyway, the patch works, so, sign-off coming soon ;)
Comment 3 Julian Maurice 2018-02-06 08:36:24 UTC
Created attachment 71244 [details] [review]
Bug 20134: Remove /api/v1/app.pl from the generated URLs

This patch removes the /api/v1/app.pl prefix from generated baseUrl.
It has the consequence of hardcoding / as the base path for the API.

This is ok because we don't currently support mounting Koha pieces in
different than hardcoded paths (/cgi-bin/koha?), but certainly worth
mentioning because this will need to depend on a syspref or config entry
when someone chages this behviour.

To test:
- On master, point your browser to:
  http://kohadev.mydnsname.org:8080/api/v1/
=> FAIL: basePath looks like /api/v1/app.pl/api/v1
- Apply this patch
- Restart memcached and plack:
  $ restart_all
or
  $ sudo systemctl restart memcached && sudo systemctl restart
  koha-common
- Reload http://kohadev.mydnsname.org:8080/api/v1/
=> SUCCESS: basePath is /api/v1
- Sign off :-D

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 4 Katrin Fischer 2018-02-07 07:25:55 UTC
I get a not found trying to access:
http://localhost:8080/api/v1

Am I missing something?
Comment 5 Josef Moravec 2018-02-07 10:00:48 UTC
(In reply to Katrin Fischer from comment #4)
> I get a not found trying to access:
> http://localhost:8080/api/v1
> 
> Am I missing something?

Working for me:
http://localhost:8080/api/v1/
Comment 6 Katrin Fischer 2018-02-07 10:08:02 UTC
Josef, feel free to pass QA then - I think my devbox might just be a bit outdated.
Comment 7 Tomás Cohen Arazi 2018-02-07 10:26:12 UTC
(In reply to Katrin Fischer from comment #4)
> I get a not found trying to access:
> http://localhost:8080/api/v1
> 
> Am I missing something?

Yes, the trailing slash.
Comment 8 Katrin Fischer 2018-02-07 10:37:13 UTC
Hah! Will try again in the evening.
Comment 9 Katrin Fischer 2018-02-07 20:25:05 UTC
... and it sure was the trailing slash!
Comment 10 Katrin Fischer 2018-02-07 20:25:54 UTC
Created attachment 71325 [details] [review]
Bug 20134: Remove /api/v1/app.pl from the generated URLs

This patch removes the /api/v1/app.pl prefix from generated baseUrl.
It has the consequence of hardcoding / as the base path for the API.

This is ok because we don't currently support mounting Koha pieces in
different than hardcoded paths (/cgi-bin/koha?), but certainly worth
mentioning because this will need to depend on a syspref or config entry
when someone chages this behviour.

To test:
- On master, point your browser to:
  http://kohadev.mydnsname.org:8080/api/v1/
=> FAIL: basePath looks like /api/v1/app.pl/api/v1
- Apply this patch
- Restart memcached and plack:
  $ restart_all
or
  $ sudo systemctl restart memcached && sudo systemctl restart
  koha-common
- Reload http://kohadev.mydnsname.org:8080/api/v1/
=> SUCCESS: basePath is /api/v1
- Sign off :-D

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Jonathan Druart 2018-02-09 17:09:19 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 12 Nick Clemens 2018-02-16 02:32:51 UTC
Awesome work all, pushed to stable for 17.11.03