Functions to read OPDS streams - languages and categories

[juuz0]

Fixes #968
Requirement for: kiwix/kiwix-desktop#960

[codecov]

Codecov Report

Patch coverage: 41.86% and project coverage change: +0.01% 🎉

Comparison is base (903dcd4) 38.87% compared to head (385931f) 38.89%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #967      +/-   ##
==========================================
+ Coverage   38.87%   38.89%   +0.01%     
==========================================
  Files          56       58       +2     
  Lines        3974     4006      +32     
  Branches     2187     2204      +17     
==========================================
+ Hits         1545     1558      +13     
- Misses       1097     1099       +2     
- Partials     1332     1349      +17     

Files Changed	Coverage Δ
src/library_dumper.cpp	61.29% <ø> (+4.14%)	⬆️
src/tools/opdsParsingTools.cpp	40.62% <40.62%> (ø)
src/tools/languageTools.cpp	45.45% <45.45%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kelson42]

@juuz0 Ready to review? Code coverage does not pass, new functions should be tested.

[juuz0]

@kelson42 added tests, CI/macOS seems to be a CI issue - failing for other PRs too

Yes, ready for review

[mgautierfr]

I'm not sure those functions are put at the right place.
The Manager has been created to manage a library. The Library being a "simple" container of Books.

But here, parseLanguages and parseCategories do not manage everything. They are "just" parsing a feed.
This could be simple free function instead of method of Manager. And if we want them to be part of Manager (to make it something that "parse content"), it should be static method to allow the user to call them without creating a Manager instance first.

---

If fact, they are the equivalent of Library::getBooksLanguages or Library::getBooksCategories, but for a abstract "remote Library". Maybe there is a new class (potentially sharing the same common interface than Library) to create to correctly handle remote library.

[kelson42]

@juuz0 Can you please create the corresponding issue to explain what is the problem you face with libkiwix?

[juuz0]

@kelson42 #968 created one

[juuz0]

If fact, they are the equivalent of Library::getBooksLanguages or Library::getBooksCategories, but for a abstract "remote Library". Maybe there is a new class (potentially sharing the same common interface than Library) to create to correctly handle remote library.

Having a remote library class felt like it should handle the download requests to /catalog/v2/languages too.
That's being done in kiwix-desktop right now (similar to how a book catalog is downloaded in kiwix desktop but parsed in libkiwix).

I put these functions in kiwix/tools.h for now - since these are tools for just parsing (no download requests etc).
But if we still want to go with the Remote Library class, let me know!

[juuz0]

Rebased

[kelson42]

@juuz0 @mgautierfr Please talk together, it's not good to see this PR somehow stuck since a week.

[juuz0]

@mgautierfr Review needed!

[mgautierfr]

Few small fixes.
And tests are failing.

[juuz0]

And tests are failing.

Fixed. On my PC, the value was "und" for getLanguageSelfName(""). On the CI, it was ""
I made it compulsory to return "und" for empty string.

[juuz0]

Rebased after the recent merge

[mgautierfr]

Fixed. On my PC, the value was "und" for getLanguageSelfName(""). On the CI, it was ""
I made it compulsory to return "und" for empty string.

I'm a bit disturbed by this.
Why getLanguageSelfName return to different things. This means that ICU return different things for icu::Local::getISOLanguages. We must understand why.
From where is "und" coming on your PC ?

[juuz0]

Why getLanguageSelfName return to different things. This means that ICU return different things for icu::Local::getISOLanguages. We must understand why.

@mgautierfr
I want to mention that the CI failed only for ubuntu focal earlier (which uses ICU 66).

After some investigation, I found that this happens when a icu::Locale is created using the und languageCode.
Such locale will return

• "" for getISO3Language()
• und for getDisplayName()

I suspect the ubuntu-focal behaviour was unwanted (see ticket) and fixed in a later ICU version (ICU 69.1. ubuntu jammy uses 70.1)
https://unicode-org.atlassian.net/browse/ICU-21406

I think we should keep the mapping so it stays consistent across all 3 ubuntu builds
Also, check #967 (comment)

[mgautierfr]

I think we should keep the mapping so it stays consistent across all 3 ubuntu builds

ok. I agree. And "Undetermined" is indeed better.