Bug 22835 - Serve static files from plugins through the API
Summary: Serve static files from plugins through the API
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature
Assignee: Agustín Moyano
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on: 22834
Blocks:
  Show dependency tree / graph
 
Reported: 2019-05-02 18:59 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2021-06-14 21:28 UTC (History)
10 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This new feature allows plugin authors to serve static files through the API without the requirement to tweak the Apache configuration files. Routes to the static files tree are automatically loaded from a specially crafted file the plugin authors need to include in the distributed .kpz files. **Warning**: Care should be taken when installing any plugins and only plugins you trust should be used.
Version(s) released in:
19.11.00, 19.05.08
Circulation function:

Attachments
Bug 22835: Serve plugin static files through API (6.44 KB, patch)
2019-05-03 00:12 UTC, Agustín Moyano 		Details | Diff | Splinter Review
Bug 22835: Serve plugin static files through API (6.46 KB, patch)
2019-05-03 00:52 UTC, Agustín Moyano 		Details | Diff | Splinter Review
Bug 22835: (follow-up) Remove unwanted warn (1.29 KB, patch)
2019-05-03 03:17 UTC, Agustín Moyano 		Details | Diff | Splinter Review
Bug 22835: Serve plugin static files through API (6.52 KB, patch)
2019-05-13 14:24 UTC, Arthur Suzuki 		Details | Diff | Splinter Review
Bug 22835: (follow-up) Remove unwanted warn (1.35 KB, patch)
2019-05-13 14:24 UTC, Arthur Suzuki 		Details | Diff | Splinter Review
Bug 22835: (follow-up) Remove comented code (1.55 KB, patch)
2019-05-16 03:10 UTC, Agustín Moyano 		Details | Diff | Splinter Review
Bug 22835: Serve plugin static files through API (6.58 KB, patch)
2019-05-28 11:20 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 22835: (follow-up) Remove unwanted warn (1.42 KB, patch)
2019-05-28 11:37 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 22835: (follow-up) Remove comented code (1.62 KB, patch)
2019-05-28 11:37 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2019-05-02 18:59:16 UTC 
The last step to avoid having to tweak (or even depend on) Apache for plugins is having a way to serve static files through the OpenAPI spec.

The plugins serving static files would supply a static_files_routes (or similar) method that should return the OpenAPI spec for a directory tree bundled with the plugin itself. This could be done manually, but a way to auto-generate this spec will be added to the KitchenSink plugin so it can be automated through the use of gulp.
Comment 1 Agustín Moyano 2019-05-03 00:12:45 UTC 
Created attachment 89262 [details] [review]
Bug 22835: Serve plugin static files through API

This patch serves static files declared within plugins.

To declare static files the plugin must implement the method 'static_routes' which retrieves the spec of static file routes to add to the API.

Once those routes are added to the API, the become available through the /api/v1/contrib/<api_namespace>/static/<path>/<to>/<file>/<filename> endpoint.

To test:
1) Install bug-22835-plugin.kpz
2) Point your browser to /api/v1/contrib/kitchensink/static/static_files/mm.gif
CHECK => No file is served
3) Apply this patch
4) restart_all
5) Repeat step 2.
SUCCESS => File is served!
6) Sign off
Comment 2 Agustín Moyano 2019-05-03 00:32:01 UTC 
> 1) Install bug-22835-plugin.kpz

KPZ available at 

https://github.com/agmoyano/koha-plugin-kitchen-sink/releases/download/bug-22835/bug-22835-plugin.kpz
Comment 3 Agustín Moyano 2019-05-03 00:52:03 UTC 
Created attachment 89263 [details] [review]
Bug 22835: Serve plugin static files through API

This patch serves static files declared within plugins.

To declare static files the plugin must implement the method 'static_routes' which retrieves the spec of static file routes to add to the API.

Once those routes are added to the API, the become available through the /api/v1/contrib/<api_namespace>/static/<path>/<to>/<file>/<filename> endpoint.

To test:
1) Install bug-22835-plugin.kpz
2) Point your browser to /api/v1/contrib/kitchensink/static/static_files/mm.gif
CHECK => No file is served
3) Apply this patch
4) restart_all
5) Repeat step 2.
SUCCESS => File is served!
6) Sign off

Sponsored-by: Theke
Comment 4 Agustín Moyano 2019-05-03 03:17:47 UTC 
Created attachment 89280 [details] [review]
Bug 22835: (follow-up) Remove unwanted warn
Comment 5 Arthur Suzuki 2019-05-13 14:24:30 UTC 
Created attachment 89663 [details] [review]
Bug 22835: Serve plugin static files through API

This patch serves static files declared within plugins.

To declare static files the plugin must implement the method 'static_routes' which retrieves the spec of static file routes to add to the API.

Once those routes are added to the API, the become available through the /api/v1/contrib/<api_namespace>/static/<path>/<to>/<file>/<filename> endpoint.

To test:
1) Install bug-22835-plugin.kpz
2) Point your browser to /api/v1/contrib/kitchensink/static/static_files/mm.gif
CHECK => No file is served
3) Apply this patch
4) restart_all
5) Repeat step 2.
SUCCESS => File is served!
6) Sign off

Sponsored-by: Theke
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Comment 6 Arthur Suzuki 2019-05-13 14:24:51 UTC 
Created attachment 89664 [details] [review]
Bug 22835: (follow-up) Remove unwanted warn

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>
Comment 7 Arthur Suzuki 2019-05-13 14:30:03 UTC 
Works as expected, the gif provided as an example is very rewarding, thanks ;)
Made my day!
Comment 8 Fridolin Somers 2019-05-15 14:10:57 UTC 
The patch contains some unexplained commented code :

-        @plugins = grep { $_->api_namespace } @plugins;
+        #@plugins = grep { $_->api_namespace } @plugins;

+            # $c->res->headers->content_type("image/jpeg");
Comment 9 Agustín Moyano 2019-05-16 03:10:57 UTC 
Created attachment 89812 [details] [review]
Bug 22835: (follow-up) Remove comented code
Comment 10 Martin Renvoize (ashimema) 2019-05-16 09:58:58 UTC 
I like the idea behind this, but it feels somewhat wrong that it's under the /api path.. it's not really serving an api.  Trying to work out if we could instead use `/contrib/<plugin_namespace>/path/to/file.js` or something similar.  Yes, it's one more route in the vhosts we'd need to ship, but would serve the same purpose of not requiring server admins to mess with the vhost for each plugin.
Comment 11 Fridolin Somers 2019-05-22 09:43:58 UTC 
What do you say Tomás ?
Comment 12 Tomás Cohen Arazi (tcohen) 2019-05-22 12:34:06 UTC 
(In reply to Fridolin SOMERS from comment #11)
> What do you say Tomás ?

I haven't pondered the effort required to implement a whole new endpoint in our current complex setup
- New Apache config
- New plack tweaks
- New mojo app and controller?

It all seems too much given the simplicity of this dev. I'd rather just allow an array of openapi specs and leave to the plugin authors the decision of making it a real API or not. The main dev here is the controller class for static files and I think it is pretty cool :-)
Comment 13 Arthur Suzuki 2019-05-27 12:31:16 UTC 
Thomas, I definitely agree with you :)

Plus I've played a bit around the plugin system during this KohaCon!
Tweaking into Plack + Apache/Nginx configuration quickly becomes a nightmare whenever several <plugin_dir> are defined in koha-conf.xml

Also : I've had the opportunity to meet in person with Martin (great to meet IRL btw!!!) who also agreed on the greatness of the feature and saying we might just ignore his comment and take another opportunity to refactor/improve later on.

Let's push it for QA! \o/
Arthur
Comment 14 Tomás Cohen Arazi (tcohen) 2019-05-27 13:54:33 UTC 
(In reply to Arthur Suzuki from comment #13)
> Thomas, I definitely agree with you :)
> 
> Plus I've played a bit around the plugin system during this KohaCon!
> Tweaking into Plack + Apache/Nginx configuration quickly becomes a nightmare
> whenever several <plugin_dir> are defined in koha-conf.xml
> 
> Also : I've had the opportunity to meet in person with Martin (great to meet
> IRL btw!!!) who also agreed on the greatness of the feature and saying we
> might just ignore his comment and take another opportunity to
> refactor/improve later on.
> 
> Let's push it for QA! \o/
> Arthur

So now you made our day :-D
Comment 15 Katrin Fischer 2019-05-27 17:57:15 UTC 
We don't really use 'pushed for QA' (meaning pushed to a branch in the repo) right now. Changing it to the more fitting "Signed off" :)
Comment 16 Kyle M Hall (khall) 2019-05-28 11:20:31 UTC 
Created attachment 90148 [details] [review]
Bug 22835: Serve plugin static files through API

This patch serves static files declared within plugins.

To declare static files the plugin must implement the method 'static_routes' which retrieves the spec of static file routes to add to the API.

Once those routes are added to the API, the become available through the /api/v1/contrib/<api_namespace>/static/<path>/<to>/<file>/<filename> endpoint.

To test:
1) Install bug-22835-plugin.kpz
2) Point your browser to /api/v1/contrib/kitchensink/static/static_files/mm.gif
CHECK => No file is served
3) Apply this patch
4) restart_all
5) Repeat step 2.
SUCCESS => File is served!
6) Sign off

Sponsored-by: Theke
Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Kyle M Hall (khall) 2019-05-28 11:37:02 UTC 
Created attachment 90149 [details] [review]
Bug 22835: (follow-up) Remove unwanted warn

Signed-off-by: Arthur Suzuki <arthur.suzuki@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall (khall) 2019-05-28 11:37:19 UTC 
Created attachment 90150 [details] [review]
Bug 22835: (follow-up) Remove comented code

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Martin Renvoize (ashimema) 2019-06-18 17:07:07 UTC 
Nice work!

Pushed to master for 19.11.00
Comment 20 Arthur Suzuki 2019-06-18 17:08:22 UTC 
Yeay! Thanks everyone!
\o/
Comment 21 Martin Renvoize (ashimema) 2019-06-18 17:12:43 UTC 
And now we talk nicely to Fridolin and ask if he's going to carry forward my policy during tenure as RMaint to backport non-breaking Plugin and API enhancement/new features to the stable branch ;)

I believe in this case the whole dependency list should apply without issue and not cause any regressions.
Comment 22 Arthur Suzuki 2019-06-18 17:17:03 UTC 
Oh, that would be great :) he's already in the CC's list btw!
Comment 23 Katrin Fischer 2019-09-09 18:39:00 UTC 
> This development allows plugins to serve static files through the API without the requirement to tweak the Apache configuration files. Routes to the static files tree are automatically loaded from a specially crafted file the plugin authors need to include in the distributed .kpz files.

Could we document this somewhere?
Comment 24 Tomás Cohen Arazi (tcohen) 2019-11-22 16:26:57 UTC 
@fridolin can we backport this?
Comment 25 Fridolin Somers 2019-11-26 08:39:50 UTC 
(In reply to Tomás Cohen Arazi from comment #24)
> @fridolin can we backport this?

I'd love to but I'm actually on last release.
You will have to see this with next maintainer.

Best regards,
Comment 26 Tomás Cohen Arazi (tcohen) 2020-01-28 01:57:14 UTC 
It applies to the 19.05.x branch! And works!
Comment 27 Lucas Gass (lukeg) 2020-02-06 21:21:40 UTC 
backported to 19.05.x for 19.05.08
Comment 28 Hayley Pelham 2020-02-10 22:30:18 UTC 
Enhancement will not be backported to 18.11.x series.