Skip to content

Guided navigation EPUB implementation #100

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

Merged
merged 7 commits into from
Sep 11, 2024
Merged

Guided navigation EPUB implementation #100

merged 7 commits into from
Sep 11, 2024

Conversation

chocolatkey
Copy link
Member

@chocolatkey chocolatkey commented Sep 3, 2024

This PR adds preliminary support for the new Guided Navigation Document specification, and the most related use case of parsing SMIL documents in EPUBs with Media Overlay support.

Misc. changes include:

  • Minor improvements in the rwp server's serving of files
  • Using github.com/go-viper/mapstructure/v2 to simplify decoding and encoding of embedded json structs
  • Tweaks to the mediatype package variable naming and mimetype descriptions, and adding a couple of modern image formats, and using the mediatype variables themselves instead of mimetype strings in the toolkit
  • Improve GH actions speed by disabling cache for setup-go (it's slow on self-hosted runners because it has to keep re-downloading from GH's cloud storage)

@@ -244,6 +250,12 @@ func (ll LinkList) IndexOfFirstWithHref(href string) int {
// Finds the first link matching the given HREF.
func (ll LinkList) FirstWithHref(href string) *Link {
for _, link := range ll {
if link.Templated {
Copy link
Member

Choose a reason for hiding this comment

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

Did you encounter a use case where you needed to find a templated link from an HREF? I don't see how that can work if the input link is expanded with some parameters.

Copy link
Member Author

@chocolatkey chocolatkey Sep 10, 2024

Choose a reason for hiding this comment

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

@mickael-menu Using this link in the manifest links as an example:

{
  "href": "~readium/guided-navigation.json{?ref}",
  "templated": true,
  "type": "application/guided-navigation+json"
}

The way the (HTTP) server works in terms of retrieving this link is like this:

  • publication.Find(href) is called, where href is the path of the requested asset in the publication, e.g. ~readium/guided-navigation.json
  • The FirstWithHref function ends up getting called, provided with ~readium/guided-navigation.json. Because the Guided Navigation link is templated, it is expanded to ~readium/guided-navigation.json. Therefore a match is found
  • The found link (in original, templated form) is returned to the client application (in this example, rwp serve)
  • Because the found link is templated, it is expanded, this time with the actual query parameters in the URL, for example a query like ~readium/guided-navigation.json?ref=OPS/chapter_001.xhtml
  • publication.Get(link) is called, with the provided link, now in expanded form. That function goes through all services and links. It ends up finding a match in the Guided Navigation service due to its own logic for matching the link.

Let me know if you think this makes sense. It was the least disruptive way I had to integrate the possibility of templated links into the toolkit's logic now that we have the Guided Navigation document endpoint exposed by the service

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why in the first step we don't already have ~readium/guided-navigation.json with the query parameters, as the frontend has the templated link in the manifest? But I'm sure you had good reasons to set it up like that, I'm probably just missing some pieces to understand.

Copy link
Member Author

@chocolatkey chocolatkey Sep 11, 2024

Choose a reason for hiding this comment

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

Part of the reason was to take the path of least resistance and attempt to change as few public functions of the toolkit as possible.

But it's also that because with template parameters, when they're part of the query, we don't necessarily need/want a full match, you want to find the base "endpoint" a.k.a. service that it matches. For example, you could be attempting to match ~readium/guided-navigation.json{?ref}, and have ~readium/guided-navigation.json?ref=abc&random=546&token=abc and still want it to match. If we strictly tried to match ~readium/guided-navigation.json?ref=abc it wouldn't work.. I think that fits the purpose of the publication.Find function more appropriately, since a Link is not necessarily just a single asset in a publication, it's also possibly a service.

I also want to make is possible for the service itself to be able to impose its own custom matching/parameter extracting logic on the provided link itself before it definitely becomes a match for that service, so the logic needs to be in the publication.Get function, especially since the logic for the matching could be more expensive (in terms of compute). The publication.Find property iterates through all the resources every time a resource is requested, and if, in index 0 for example, you had a to run the full logic of the GetForGuidedNavigationService function each time, you would be wasting unnecessary CPU. In a manner, we're matching twice, once for a "weak" match, then again for a "strong" match in a service.

Copy link
Member

@mickael-menu mickael-menu left a comment

Choose a reason for hiding this comment

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

Looking good, thank you 🙏

@chocolatkey chocolatkey merged commit 7cd7937 into develop Sep 11, 2024
1 check passed
@chocolatkey chocolatkey deleted the media-overlay branch September 11, 2024 21:33
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.

None yet

2 participants