Use archive entry length instead of original length for EPUB positions (#162)

mickael-menu · web-flow · commit c8b5f50ee35b · 2021-09-17T16:26:29.000+02:00
diff --git a/readium/streamer/CHANGELOG.md b/readium/streamer/CHANGELOG.md
@@ -15,6 +15,8 @@ All notable changes to this project will be documented in this file.
 ### Changed
 
 * Upgraded to Kotlin 1.5.21 and Gradle 7.1.1
+* The default EPUB positions service now uses the archive entry length when available. [This is similar to how Adobe RMSDK generates page numbers](https://github.com/readium/architecture/issues/123).
+    * To use the former strategy, create the `Streamer` with: `Streamer(parsers = listOf(EpubParser(reflowablePositionsStrategy = OriginalLength(pageLength = 1024))))`
 
 ### Fixed
 
diff --git a/readium/streamer/r2-streamer/src/main/java/org/readium/r2/streamer/parser/epub/EpubParser.kt b/readium/streamer/r2-streamer/src/main/java/org/readium/r2/streamer/parser/epub/EpubParser.kt
@@ -78,8 +78,13 @@ object EPUBConstant {
 
 /**
  * Parses a Publication from an EPUB publication.
+ *
+ * @param reflowablePositionsStrategy Strategy used to calculate the number of positions in a
+ *        reflowable resource.
  */
-class EpubParser : PublicationParser, org.readium.r2.streamer.parser.PublicationParser {
+class EpubParser(
+    private val reflowablePositionsStrategy: EpubPositionsService.ReflowableStrategy = EpubPositionsService.ReflowableStrategy.recommended
+) : PublicationParser, org.readium.r2.streamer.parser.PublicationParser {
 
     override suspend fun parse(asset: PublicationAsset, fetcher: Fetcher, warnings: WarningLogger?): Publication.Builder? =
         _parse(asset, fetcher, asset.name)
@@ -113,7 +118,7 @@ class EpubParser : PublicationParser, org.readium.r2.streamer.parser.Publication
             manifest = manifest,
             fetcher = fetcher,
             servicesBuilder = Publication.ServicesBuilder(
-                positions = (EpubPositionsService)::create,
+                positions = EpubPositionsService.createFactory(reflowablePositionsStrategy),
                 search = StringSearchService.createDefaultFactory(),
             )
         )
diff --git a/readium/streamer/r2-streamer/src/main/java/org/readium/r2/streamer/parser/epub/EpubPositionsService.kt b/readium/streamer/r2-streamer/src/main/java/org/readium/r2/streamer/parser/epub/EpubPositionsService.kt
@@ -1,18 +1,17 @@
 /*
- * Module: r2-streamer-kotlin
- * Developers: Mickaël Menu
- *
- * Copyright (c) 2020. Readium Foundation. All rights reserved.
- * Use of this source code is governed by a BSD-style license which is detailed in the
- * LICENSE file present in the project repository where this source code is maintained.
+ * Copyright 2020 Readium Foundation. All rights reserved.
+ * Use of this source code is governed by the BSD-style license
+ * available in the top-level LICENSE file of the project.
  */
 
 package org.readium.r2.streamer.parser.epub
 
 import org.readium.r2.shared.fetcher.Fetcher
+import org.readium.r2.shared.fetcher.Resource
 import org.readium.r2.shared.publication.Link
 import org.readium.r2.shared.publication.Locator
 import org.readium.r2.shared.publication.Publication
+import org.readium.r2.shared.publication.archive.archive
 import org.readium.r2.shared.publication.encryption.encryption
 import org.readium.r2.shared.publication.epub.EpubLayout
 import org.readium.r2.shared.publication.epub.layoutOf
@@ -30,17 +29,75 @@ import kotlin.math.ceil
  *
  * https://github.com/readium/architecture/blob/master/models/locators/best-practices/format.md#epub
  * https://github.com/readium/architecture/issues/101
- *
- * @param reflowablePositionLength Length in bytes of a position in a reflowable resource. This is
- *        used to split a single reflowable resource into several positions.
  */
-internal class EpubPositionsService(
+class EpubPositionsService(
     private val readingOrder: List<Link>,
     private val presentation: Presentation,
     private val fetcher: Fetcher,
-    private val reflowablePositionLength: Long
+    private val reflowableStrategy: ReflowableStrategy
 ) : PositionsService {
 
+    companion object {
+
+        fun createFactory(reflowableStrategy: ReflowableStrategy = ReflowableStrategy.recommended): (Publication.Service.Context) -> EpubPositionsService =
+            { context ->
+                EpubPositionsService(
+                    readingOrder = context.manifest.readingOrder,
+                    presentation = context.manifest.metadata.presentation,
+                    fetcher = context.fetcher,
+                    reflowableStrategy = reflowableStrategy
+                )
+            }
+    }
+
+    /**
+     * Strategy used to calculate the number of positions in a reflowable resource.
+     *
+     * Note that a fixed-layout resource always has a single position.
+     */
+    sealed class ReflowableStrategy {
+        /** Returns the number of positions in the given [resource] according to the strategy. */
+        abstract suspend fun positionCount(resource: Resource): Int
+
+        /**
+         * Use the original length of each resource (before compression and encryption) and split it
+         * by the given [pageLength].
+         */
+        data class OriginalLength(val pageLength: Int) : ReflowableStrategy() {
+            override suspend fun positionCount(resource: Resource): Int {
+                val length = resource.link().properties.encryption?.originalLength
+                    ?: resource.length().getOrNull()
+                    ?: 0
+                return ceil(length.toDouble() / pageLength.toDouble()).toInt()
+                    .coerceAtLeast(1)
+            }
+        }
+
+        /**
+         * Use the archive entry length (whether it is compressed or stored) and split it by the
+         * given [pageLength].
+         */
+        data class ArchiveEntryLength(val pageLength: Int) : ReflowableStrategy() {
+            override suspend fun positionCount(resource: Resource): Int {
+                val length = resource.link().properties.archive?.entryLength
+                    ?: resource.length().getOrNull()
+                    ?: 0
+                return ceil(length.toDouble() / pageLength.toDouble()).toInt()
+                    .coerceAtLeast(1)
+            }
+        }
+
+        companion object {
+            /**
+             * Recommended historical strategy: archive entry length split by 1024 bytes pages.
+             *
+             * This strategy is used by Adobe RMSDK as well.
+             * See https://github.com/readium/architecture/issues/123
+             */
+            val recommended = ArchiveEntryLength(pageLength = 1024)
+        }
+    }
+
     override suspend fun positionsByReadingOrder(): List<List<Locator>> {
         if (!::_positions.isInitialized)
             _positions = computePositions()
@@ -93,18 +150,13 @@ internal class EpubPositionsService(
     )
 
     private suspend fun createReflowable(link: Link, startPosition: Int, fetcher: Fetcher): List<Locator> {
-        // If the resource is encrypted, we use the `originalLength` declared in `encryption.xml`
-        // instead of the ZIP entry length.
-        val length = link.properties.encryption?.originalLength
-            ?: fetcher.get(link).use { it.length().getOrNull() }
-            ?: return emptyList()
-
-        val pageCount = ceil(length / reflowablePositionLength.toDouble()).toInt()
-            .coerceAtLeast(1)
+        val positionCount = fetcher.get(link).use { resource ->
+            reflowableStrategy.positionCount(resource)
+        }
 
-        return (1..pageCount).map { position ->
+        return (1..positionCount).map { position ->
             createLocator(link,
-                progression = (position - 1) / pageCount.toDouble(),
+                progression = (position - 1) / positionCount.toDouble(),
                 position = startPosition + position
             )
         }
@@ -119,17 +171,4 @@ internal class EpubPositionsService(
             position = position
         )
     )
-
-    companion object {
-
-        fun create(context: Publication.Service.Context): EpubPositionsService {
-            return EpubPositionsService(
-                readingOrder = context.manifest.readingOrder,
-                presentation = context.manifest.metadata.presentation,
-                fetcher = context.fetcher,
-                reflowablePositionLength = 1024L
-            )
-        }
-
-    }
 }
diff --git a/readium/streamer/r2-streamer/src/test/java/org/readium/r2/streamer/parser/epub/EpubPositionsServiceTest.kt b/readium/streamer/r2-streamer/src/test/java/org/readium/r2/streamer/parser/epub/EpubPositionsServiceTest.kt
@@ -189,7 +189,7 @@ class EpubPositionsServiceTest {
                 Pair(51L, Link(href = "chap4")),
                 Pair(120L, Link(href = "chap5"))
             ),
-            reflowablePositionLength = 50L
+            reflowableStrategy = EpubPositionsService.ReflowableStrategy.ArchiveEntryLength(pageLength = 50)
         )
 
         assertEquals(
@@ -280,7 +280,7 @@ class EpubPositionsServiceTest {
             readingOrder = listOf(
                 Pair(60L, Link(href = "chap1"))
             ),
-            reflowablePositionLength = 50L
+            reflowableStrategy = EpubPositionsService.ReflowableStrategy.ArchiveEntryLength(pageLength = 50)
         )
 
         assertEquals(
@@ -317,7 +317,7 @@ class EpubPositionsServiceTest {
                 Pair(60L, Link(href = "chap2", properties = createProperties(layout = EpubLayout.REFLOWABLE))),
                 Pair(20000L, Link(href = "chap3", properties = createProperties(layout = EpubLayout.FIXED)))
             ),
-            reflowablePositionLength = 50L
+            reflowableStrategy = EpubPositionsService.ReflowableStrategy.ArchiveEntryLength(pageLength = 50)
         )
 
         assertEquals(
@@ -364,14 +364,63 @@ class EpubPositionsServiceTest {
     }
 
     @Test
-    fun `Use the encrypted {originalLength} if available, instead of the {Container}'s file length`() {
+    fun `Use the {ArchiveEntryLength} reflowable strategy`() {
         val service = createService(
             layout = EpubLayout.REFLOWABLE,
             readingOrder = listOf(
-                Pair(60L, Link(href = "chap1", properties = createProperties(encryptedOriginalLength = 20L))),
+                Pair(60L, Link(href = "chap1", properties = createProperties(archiveEntryLength = 20L))),
                 Pair(60L, Link(href = "chap2"))
             ),
-            reflowablePositionLength = 50L
+            reflowableStrategy = EpubPositionsService.ReflowableStrategy.ArchiveEntryLength(pageLength = 50)
+        )
+
+        assertEquals(
+            listOf(
+                listOf(
+                    Locator(
+                        href = "chap1",
+                        type = "text/html",
+                        locations = Locator.Locations(
+                            progression = 0.0,
+                            position = 1,
+                            totalProgression = 0.0
+                        )
+                    ),
+                ),
+                listOf(
+                    Locator(
+                        href = "chap2",
+                        type = "text/html",
+                        locations = Locator.Locations(
+                            progression = 0.0,
+                            position = 2,
+                            totalProgression = 1.0/3.0
+                        )
+                    ),
+                    Locator(
+                        href = "chap2",
+                        type = "text/html",
+                        locations = Locator.Locations(
+                            progression = 0.5,
+                            position = 3,
+                            totalProgression = 2.0/3.0
+                        )
+                    )
+                )
+            ),
+            runBlocking { service.positionsByReadingOrder() }
+        )
+    }
+
+    @Test
+    fun `Use the {OriginalLength} reflowable strategy`() {
+        val service = createService(
+            layout = EpubLayout.REFLOWABLE,
+            readingOrder = listOf(
+                Pair(60L, Link(href = "chap1", properties = createProperties(originalLength = 20L))),
+                Pair(60L, Link(href = "chap2"))
+            ),
+            reflowableStrategy = EpubPositionsService.ReflowableStrategy.OriginalLength(pageLength = 50)
         )
 
         assertEquals(
@@ -411,7 +460,7 @@ class EpubPositionsServiceTest {
     private fun createService(
         layout: EpubLayout? = null,
         readingOrder: List<Pair<Long, Link>>,
-        reflowablePositionLength: Long = 50L
+        reflowableStrategy: EpubPositionsService.ReflowableStrategy = EpubPositionsService.ReflowableStrategy.ArchiveEntryLength(pageLength = 50)
     ) = EpubPositionsService(
         readingOrder = readingOrder.map { it.second },
         fetcher = object : Fetcher {
@@ -436,21 +485,26 @@ class EpubPositionsServiceTest {
             override suspend fun close() {}
         },
         presentation = Presentation(layout = layout),
-        reflowablePositionLength = reflowablePositionLength
+        reflowableStrategy = reflowableStrategy
     )
 
-    private fun createProperties(layout: EpubLayout? = null, encryptedOriginalLength: Long? = null): Properties {
+    private fun createProperties(layout: EpubLayout? = null, archiveEntryLength: Long? = null, originalLength: Long? = null): Properties {
         val properties = mutableMapOf<String, Any>()
         if (layout != null) {
             properties["layout"] = layout.value
         }
-        if (encryptedOriginalLength != null) {
+        if (originalLength != null) {
             properties["encrypted"] = mapOf(
                 "algorithm" to "algo",
-                "originalLength" to encryptedOriginalLength
+                "originalLength" to originalLength
+            )
+        }
+        if (archiveEntryLength != null) {
+            properties["archive"] = mapOf(
+                "entryLength" to archiveEntryLength,
+                "isEntryCompressed" to true
             )
         }
-
         return Properties(otherProperties = properties)
     }
 
