Support for multiple illustrations in OPDS entry

[veloman-yunkan]

Fixes #533

Things to note:

• the output is modified in both old and new (v2) OPDS API
• the code is not tested on a ZIM file with multiple illustrations for the lack of such a ZIM file

[codecov]

Codecov Report

Merging #577 (452283c) into master (b8aee8a) will increase coverage by 0.18%.
The diff coverage is 96.00%.

@@            Coverage Diff             @@
##           master     #577      +/-   ##
==========================================
+ Coverage   64.78%   64.97%   +0.18%     
==========================================
  Files          53       53              
  Lines        3729     3749      +20     
  Branches     1856     1867      +11     
==========================================
+ Hits         2416     2436      +20     
  Misses       1311     1311              
  Partials        2        2              

Impacted Files	Coverage Δ
src/reader.cpp	28.49% <0.00%> (ø)
src/book.cpp	94.89% <100.00%> (ø)
src/opds_dumper.cpp	100.00% <100.00%> (ø)
src/server/internalServer.cpp	83.89% <100.00%> (+0.30%)	⬆️
src/tools/archiveTools.cpp	89.85% <100.00%> (ø)

---

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 b8aee8a...452283c. Read the comment docs.

[mgautierfr]

We also have to update the handle_meta method to handle the illustration_<with>x<height> name. (https://github.com/kiwix/libkiwix/blob/master/src/server/internalServer.cpp#L348-L368)

(This probably should have been done in PR adapting libkiwix to new api in libzim but I forget about it)

[kelson42]

@veloman-yunkan Can you please rebase on master? Is this PR again ready to review (then please request a new review)?

[veloman-yunkan]

@veloman-yunkan Can you please rebase on master? Is this PR again ready to review (then please request a new review)?

Done without adding a test case utilizing a ZIM archive with multiple illustrations (because of no easy way of creating such a ZIM archive using existing tools, and the complexities of the procedure of modifying test ZIM data).

[kelson42]

@veloman-yunkan I'm worried about your last comment. Having a proper test set of ZIM and libkiwix properly tested with the most common scenarios is important. What is exactly unclear, after @rgaudin feedback to your last question at #533 (comment), I thought this is OK now... but it seems this is not. We should get that sorted out. If this is a not a detail which stops you, please open a ticket so we can resolve the problem.

[veloman-yunkan]

@veloman-yunkan I'm worried about your last comment. Having a proper test set of ZIM and libkiwix properly tested with the most common scenarios is important. What is exactly unclear, after @rgaudin feedback to your last question at #533 (comment), I thought this is OK now... but it seems this is not. We should get that sorted out. If this is a not a detail which stops you, please open a ticket so we can resolve the problem.

@kelson42

@rgaudin's suggestion requires a custom build of zimwriterfs (using modified source code). I could figure out such a "solution" myself and it is not what I was looking for. I think that the test data must be created in a reproducible way, using official versions of publicly available tools. Regarding the complexity of test data management, note that libkiwix doesn't yet rely on zim-testing-suite and so far hasn't respected the requirement of keeping old test data and continuing to use it for backward compatibility testing.

[stale]

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.

[mgautierfr]

I agree with this PR. (Code and comment)

• This is pretty difficult to test this (to have test data).
• This code is pretty easy to change if there is a problem and is not really part of the API.

I think we can merge like this and add correct tests later when we will attack the zim testing suite problem.

@veloman-yunkan please rebase-fixup on master.

[veloman-yunkan]

@veloman-yunkan please rebase-fixup on master.

Done

[kelson42]

I'm not understanding the arguments to not test this. I would like to have a talk with @mgautierfr about this ticket before merging this.

[veloman-yunkan]

Rebased again (had to resolve a conflict with #576)

[kelson42]

I understand that this PR can not easily be tested properly (at least) because of:

• Libkiwix CI should use zim-testing-suite #601
• Source code recipe to build the test ZIM files should be here openzim/zim-testing-suite#3
• Add ZIM file multiple illustrations openzim/zim-testing-suite#4

I have there open the tickets above to help in the future and will merge this PR now.