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

OPDS catalog must respect the used nameMapper #828

Closed
MohitMaliFtechiz opened this issue Oct 3, 2022 · 11 comments · Fixed by #837 · May be fixed by #830
Closed

OPDS catalog must respect the used nameMapper #828

MohitMaliFtechiz opened this issue Oct 3, 2022 · 11 comments · Fixed by #837 · May be fixed by #830
Assignees
Labels
Milestone

Comments

@MohitMaliFtechiz
Copy link

Describe the bug
On libkiwix 10.1.1 , zim file while opening over hotspot resulting The requested URL "/" was not found on this server..

Expected behavior
Its should open the zim file without any issue

Screenshots
Screenshot from 2022-10-03 18-52-57
Screenshot from 2022-10-03 18-53-01

Environment

  • Version of Kiwix Android : 3.7.0
  • Device : REDMI NOTE 9
  • OS version : 11
@MohitMaliFtechiz MohitMaliFtechiz self-assigned this Oct 3, 2022
@kelson42 kelson42 added the bug label Oct 3, 2022
@MohitMaliFtechiz
Copy link
Author

hi @mgautierfr ,

I debug the issue seems like its issue with the libkiwix lib not from android side , I have debug the path seems like we are getting the path url on android while creating Library object for calling getNativeServer(library). But after starting server home page works fine but inner url leads to not found. @kelson42 please transfer this ticket to libkiwix lib.

@mgautierfr mgautierfr transferred this issue from kiwix/kiwix-android Oct 4, 2022
@mgautierfr
Copy link
Member

I think I've found the bug.
By default, android's server are run without a nameMapper. (And so with a default namemapper using the book uuid as name)

namemapper is used by kiwix::Server to map a book uuid (used internally to identify books) to a name to use in url (both expected url and generated ones).

However, the opds_stream always uses the "humanReadableIdFromPath". It is the bug (to fix) and it always have been this way.
The "regression" comes from the switch from backend generated home page to dynamic js home page which use the catalog as source.

As kiwix-serve binary use a HumanReadableNameMapper, we never faced the bug.

@MohitMaliFtechiz you can check it is the actual issue by using the uuid of the zim instead of the name 100r-of-the-grid_2022-03
Doing a fulltext search should also works as we use the namemapper here.

@mgautierfr mgautierfr changed the title Zim file not opening over hotspot on libkiwix 10.1.1 opds catalog must respect the used nameMapper Oct 4, 2022
@kelson42
Copy link
Collaborator

kelson42 commented Oct 4, 2022

@mgautierfr @veloman-yunkan Sounds like a regression to me. Isn't it?

@kelson42 kelson42 added this to the 12.0.0 milestone Oct 4, 2022
@mgautierfr
Copy link
Member

However, the opds_stream always uses the "humanReadableIdFromPath". It is the bug (to fix) and it always have been this way.
The "regression" comes from the switch from backend generated home page to dynamic js home page which use the catalog as source.

This is not a regression from the opds_stream point of view as the bug was always here.
This is a regression from the homepage point of view and it dates from libkiwix 10.0.0 (but the bug is on opds_stream side)

@kelson42
Copy link
Collaborator

kelson42 commented Oct 4, 2022

@mgautierfr Thx for clarification. IMO, we better get it fixed befpre 12.0.0 release.

@kelson42 kelson42 changed the title opds catalog must respect the used nameMapper OPDS catalog must respect the used nameMapper Oct 5, 2022
@veloman-yunkan
Copy link
Collaborator

I will work on this.

@mgautierfr
Copy link
Member

mgautierfr commented Oct 6, 2022

I'm already working on this

@veloman-yunkan
Copy link
Collaborator

I've started with some clean-up of OPDS dumper. I've now shared my work as a self contained PR #829.

@mgautierfr
Copy link
Member

I've got the same thing here : #830
I've also started a cleanup, but by the other end (api).
We must synchronize our work else we go to a guaranteed merge conflict :)

@veloman-yunkan
Copy link
Collaborator

I guess that #829 in its current form can be merged first without causing too much pain for #830. I won't continue working on it.

@kelson42
Copy link
Collaborator

kelson42 commented Oct 6, 2022

Per default the Http daemon primitive should work with humanids, not uuids. It woukd be great to use this ticket to have it working like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants