Skip to content
This repository has been archived by the owner on Jul 29, 2022. It is now read-only.

Rewrite Epub parsing with the new XmlParser implementation and the new Publication model #89

Merged
merged 65 commits into from
Feb 25, 2020

Conversation

qnga
Copy link
Contributor

@qnga qnga commented Jan 18, 2020

Fixes readium/kotlin-toolkit#203 Fixes readium/kotlin-toolkit#202 Fixes readium/r2-lcp-kotlin#50

Elements and attributes are every time picked from the right namespace. Epub 3.x Property Data Type default vocabularies and prefix mechanism are carefully implemented.

As data are not organized in the same way in an Epub Package Document and in a Readium Publication, a specific intermediate model is used to parse completely an Epub before converting it into a Publication. This allows clearer code and should make further feature additions easier.

UPDATE: This PR now includes the adaptation of the streamer to the new Publication model introduced by @mickael-menu in Shared PR 88 .

Elements and attributes are every time picked from the right namespace.
Epub 3.x Property Data Type default vocabularies and prefix mechanism are carefully implemented.

As data are not organized in the same way in an Epub Package Document and in a Readium Publication,
a specific intermediate model is used to parse completely an Epub before converting it into a Publication.
This allows clearer code and should make further feature additions easier.
After SMIL parsing, media overlays are stored in the mediaOverlays property of a dedicated link.
@llemeurfr
Copy link

Hi @qnga, very interesting proposal indeed, but quite a large PR(s), which will take some time to be reviewed. It would be good if you can join the weekly Readium call to explain the rationale of this evolution and give details about its risks. Please contact me in PM if you need info on how to join.

@mickael-menu
Copy link
Member

Great job on the namespaces and vocabularies!
I'm not sure an intermediate model was really warranted for the parsing but it probably doesn't hurt.

Since it's a quite large PR, maybe it would be worth adding a unit tests suite for the parser? We can reuse the test cases from Swift: https://github.com/readium/r2-streamer-swift/tree/develop/r2-streamer-swiftTests/Parser/EPUB
This would help validate the changes.

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.

@qnga Great job, it's well-structured 👍

I made a few cosmetic changes that you can review and merge (branch review/pull/89), and left some comments on the PR that we can tackle together.

mickael-menu
mickael-menu previously approved these changes Feb 12, 2020
@mickael-menu mickael-menu merged commit b5131fb into readium:develop Feb 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants