Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for serving customized resources #779

Merged
merged 3 commits into from
Jun 22, 2022

Conversation

veloman-yunkan
Copy link
Collaborator

During work on the kiwix-serve front-end, the edit-save-test cycle is a multistep procedure:

  1. build and install libkiwix
  2. build kiwix-tools
  3. run kiwix-serve
  4. reload the web-page in the browser

When making changes in static resources that are served by kiwix-serve unmodified, the steps 1-3 can be eliminated if kiwix-serve is capable of serving resources from the file-system. This commit adds such a functionality to kiwix-serve. Now, if during startup of kiwix-serve the environment variable KIWIX_SERVE_CUSTOMIZED_RESOURCES is defined it is assumed to point to a file here every line has the following format:

URL MIMETYPE RESOURCE_FILE_PATH

When a request is received by kiwix-serve and its URL matches any of the URLs read from the customized resource file, then the resource data is read from the respective file RESOURCE_FILE_PATH and served with mime-type MIMETYPE.

Though this feature was introduced in order to facilitate the development of the iframe-based content viewer, it can also be useful to users who would like to customize the kiwix-serve front-end on their own (without re-building all of kiwix-serve).

There is some overlap with a feature of the kiwix-compile-resources script that also allows to override resources. The differences are:

  1. The new way of customizing front-end resources has all such resources listed in a text file and there is a single environment variable from which the path of that file is read. kiwix-compile-resources associates a separate environment variable with each resource.

  2. The new way uses regular paths to identify a resource. The kiwix-compile-resources method encodes the resource path by replacing any non-alphanumeric characters (including the path separator) with underscores (so that the resulting resource identifier can be used to construct the name of the environment variable controlling that resource).

  3. The new method allows adding new front-end resources. The old method only allows to modify existing resources.

  4. The new method allows (actually requires) to specify the URL at which the overriden resource should be served (similarly, the MIME-type can/must be specified, too). The old method only allows to override the contents of a resource.

  5. The new method only allows to override front-end resources that are served without any preprocessing by kiwix-serve at runtime. The old method allows to override template resources as well (note that internationalization/translation resources cannot be overriden using the old method, either).

@codecov
Copy link

codecov bot commented Jun 4, 2022

Codecov Report

Merging #779 (1139f2c) into master (6938253) will increase coverage by 0.54%.
The diff coverage is 90.00%.

@@            Coverage Diff             @@
##           master     #779      +/-   ##
==========================================
+ Coverage   63.66%   64.21%   +0.54%     
==========================================
  Files          59       59              
  Lines        4073     4102      +29     
  Branches     2218     2227       +9     
==========================================
+ Hits         2593     2634      +41     
+ Misses       1478     1466      -12     
  Partials        2        2              
Impacted Files Coverage Δ
src/server/internalServer.h 89.47% <ø> (ø)
src/server/internalServer.cpp 85.46% <90.00%> (+0.22%) ⬆️
src/tools/pathTools.cpp 61.14% <0.00%> (+8.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6938253...1139f2c. Read the comment docs.

@veloman-yunkan
Copy link
Collaborator Author

This PR originates from #716 (comment). The new functionality in its current form doesn't fully take over the old solution that's why I didn't remove the old solution from kiwix-compile-resources. If we decide to enhance this PR and simplify kiwix-compile-resources instead, then we should also try to do it in a way that eliminates the need for #607 & kiwix/kiwix-tools#477 (--customIndexTemplate becoming a special case of a more general solution).

@kelson42 kelson42 added this to the 10.3.0 milestone Jun 4, 2022
@stale
Copy link

stale bot commented Jun 12, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot added the stale label Jun 12, 2022
@kelson42
Copy link
Collaborator

@mgautierfr Can you please review this Pr?

@kelson42 kelson42 force-pushed the serving_customized_resources branch from 7b7e4e2 to a6a2704 Compare June 13, 2022 04:47
During work on the kiwix-serve front-end, the edit-save-test cycle is
a multistep procedure:

1. build and install libkiwix
2. build kiwix-tools
3. run kiwix-serve
4. reload the web-page in the browser

When making changes in static resources that are served by kiwix-serve
unmodified, the steps 1-3 can be eliminated if kiwix-serve is capable of
serving resources from the file-system. This commit adds such a
functionality to kiwix-serve. Now, if during startup of kiwix-serve the
environment variable `KIWIX_SERVE_CUSTOMIZED_RESOURCES` is defined it is
assumed to point to a file where every line has the following format:

URL MIMETYPE RESOURCE_FILE_PATH

When a request is received by kiwix-serve and its URL matches any of the
URLs read from the customized resource file, then the resource data is
read from the respective file RESOURCE_FILE_PATH and served with
mime-type MIMETYPE.

Though this feature was introduced in order to facilitate the
development of the iframe-based content viewer, it can also be useful to
users who would like to customize the kiwix-serve front-end on their own
(without re-building all of kiwix-serve).

There is some overlap with a feature of the kiwix-compile-resources
script that also allows to override resources. The differences are:

1. The new way of customizing front-end resources has all such resources
   listed in a text file and there is a single environment variable
   from which the path of that file is read. kiwix-compile-resources
   associates a separate environment variable with each resource.

2. The new way uses regular paths to identify a resource. The
   kiwix-compile-resources method encodes the resource path by replacing
   any non-alphanumeric characters (including the path separator) with
   underscores (so that the resulting resource identifier can be used
   to construct the name of the environment variable controlling that
   resource).

3. The new method allows adding new front-end resources. The old method
   only allows to modify existing resources.

4. The new method allows (actually requires) to specify the URL at which
   the overriden resource should be served (similarly, the MIME-type can/must
   be specified, too). The old method only allows to override the contents of
   a resource.

5. The new method only allows to override front-end resources that are
   served without any preprocessing by kiwix-serve at runtime. The old
   method allows to override template resources as well (note that
   internationalization/translation resources cannot be overriden using the
   old method, either).
@kelson42 kelson42 force-pushed the serving_customized_resources branch from a6a2704 to e3e4bfa Compare June 22, 2022 08:59
@stale stale bot removed the stale label Jun 22, 2022
mgautierfr
mgautierfr previously approved these changes Jun 22, 2022
Copy link
Member

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. But it is missing some test :)

It would be good to have a homogenized code for this. I like the idea of having a file describing the resources (and the mimetype). We may move this on a lower level, in the "resource engine".
But for a next PR.

@mgautierfr mgautierfr self-requested a review June 22, 2022 09:38
@mgautierfr mgautierfr dismissed their stale review June 22, 2022 09:39

Tests are missing

Copy link
Member

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are missing

One important missing test is that the content of the customized
resource is read from storage every time rather than once. Testing
that requirement would involve creating temporary files which is a
little more work.
@veloman-yunkan
Copy link
Collaborator Author

Tests are missing

Added a couple of tests that provide only basic coverage of the new code. The last commit outlines an aspect of the desired functionality that is not tested.

@mgautierfr
Copy link
Member

Agree with the message of the last commit. Testing that file is read each request would be a bit too complex to test.

@mgautierfr mgautierfr merged commit 15cb902 into master Jun 22, 2022
@mgautierfr mgautierfr deleted the serving_customized_resources branch June 22, 2022 13:31
@kelson42 kelson42 modified the milestones: 12.1.0, 12.0.0 Jul 7, 2022
mgautierfr added a commit that referenced this pull request Nov 30, 2022
* [API Break] Remove wrapper around libzim (@mgautierfr #789)
* Allow kiwix-serve to use custom resource files (@veloman-yunkan #779)
* Properly handle searchProtocolPrefix when rendering search result (@veloman-yunkan #823)
* Prevent search on multi language content (@veloman-yunkan #838)
* Use new `zim::Archive::getMediaCount` from libzim (@mgautierfr #836)
* Catalog:
 - Include tags in free text catalog search (@veloman-yunkan #802)
 - Illustration's url is based on book's uuid (@veloman-yunkan #804)
 - Cleanup of the opds-dumper (@veloman-yunkan #829)
 - Allow filtering of catalog content using multiple languages (@veloman-yunkan #841)
 - Make opds-dumper respect the namemapper (@mgautierfr #837)
* Server:
 - Correctly handle `\` in suggestion json generation (@veloman-yunkan #843)
 - Better http caching (@veloman-yunkan #833)
 - Make `/suggest` endpoint thread-safe (@veloman-yunkan #834)
 - Better redirection of main page (@veloman-yunkan #827)
 - Remove jquery (@mgautierfr @juuz0 #796)
 - Better Viewer of zim content :
   . Introduce `/content` endpoints (@veloman-yunkan #806)
   . Switch to iframe based content viewer (@veloman-yunkan #716)
 - Optimised design of the welcome page:
   . Alignement (@juuz0 @kelson42 #786)
   . Exit download modal on pressing escape key (@Juzz0 #800)
   . Add favicon for different devices (@Juzz0 #805)
   . Fix auto hidding of the toolbar (@veloman-yunkan #821)
   . Allow user to filter books by tags in the front page (@juuz0 #711)
* CI :
  - Trigger CI on pull_request (@kelson42 #791)
  - Drop Ubuntu Impish packaging (@legoktm #825)
  - Add Ubuntu Kinetic packaging (@legoktm #801)
* Testing:
  - Test ICULanguageInfo (@veloman-yunkan #795)
  - Introduce fake `test` language to test i18n (@veloman-yunkan #848)
* Fix documentation (@kelson42 #816)
* Udpate translation (#787 #839 #847)
@mgautierfr mgautierfr mentioned this pull request Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants