Helper to migrate legacy HREFs (#399)

CHANGELOG.md

@@ -25,8 +25,8 @@ All notable changes to this project will be documented in this file. Take a look
 ### Changed
 
 * Readium resources are now prefixed with `readium_`. Take care of updating any overridden resource by following [the migration guide](docs/migration-guide.md#300).
-* `Link` and `Locator`'s `href` do not start with a `/` for packaged publications anymore.
-    * To ensure backward-compatibility, `href` starting with a `/` are still supported. But you may want to update the locators persisted in your database to drop the `/` prefix for packaged publications.
+* `Link` and `Locator`'s `href` are normalized as valid URLs to improve interoperability with the Readium Web toolkits.
+    * **You MUST migrate your database if you were persisting HREFs and Locators**. Take a look at [the migration guide](docs/migration-guide.md) for guidance.
 
 #### Shared
 

docs/migration-guide.md

@@ -29,6 +29,55 @@ dependencies {
 }
 ```
 
+### Migration of HREFs and Locators (bookmarks, annotations, etc.)
+
+:warning: This requires a database migration in your application, if you were persisting `Locator` objects.
+
+In Readium v2.x, a `Link` or `Locator`'s `href` could be either:
+
+* a valid absolute URL for a streamed publication, e.g. `https://domain.com/isbn/dir/my%20chapter.html`,
+* a percent-decoded path for a local archive such as an EPUB, e.g. `/dir/my chapter.html`.
+    * Note that it was relative to the root of the archive (`/`).
+
+To improve the interoperability with other Readium toolkits (in particular the Readium Web Toolkits, which only work in a streaming context) **Readium v3 now generates and expects valid URLs** for `Locator` and `Link`'s `href`.
+
+* `https://domain.com/isbn/dir/my%20chapter.html` is left unchanged, as it was already a valid URL.
+* `/dir/my chapter.html` becomes the relative URL path `dir/my%20chapter.html`
+    * We dropped the `/` prefix to avoid issues when resolving to a base URL.
+    * Special characters are percent-encoded.
+
+**You must migrate the HREFs or Locators stored in your database** when upgrading to Readium 3. To assist you, two helpers are provided: `Url.fromLegacyHref()` and `Locator.fromLegacyJSON()`.
+
+Here's an example of a Jetpack Room migration that can serve as inspiration:
+
+```kotlin
+val MIGRATION_HREF = object : Migration(1, 2) {
+
+    override fun migrate(db: SupportSQLiteDatabase) {
+        val normalizedHrefs: Map<Long, String> = buildMap {
+            db.query("SELECT id, href FROM bookmarks").use { cursor ->
+                while (cursor.moveToNext()) {
+                    val id = cursor.getLong(0)
+                    val href = cursor.getString(1)
+
+                    val normalizedHref = Url.fromLegacyHref(href)?.toString()
+                    if (normalizedHref != null) {
+                        put(id, normalizedHref)
+                    }
+                }
+            }
+        }
+
+        val stmt = db.compileStatement("UPDATE bookmarks SET href = ? WHERE id = ?")
+        for ((id, href) in normalizedHrefs) {
+            stmt.bindString(1, href)
+            stmt.bindLong(2, id)
+            stmt.executeUpdateDelete()
+        }
+    }
+}
+```
+
 ### All resources now have the prefix `readium_`.
 
 To avoid conflicts when merging your app resources, all resources declared in the Readium toolkit now have the prefix `readium_`. This means that you must rename any layouts or strings you have overridden. Here is a comprehensive list of the changes.

readium/shared/src/main/java/org/readium/r2/shared/publication/Locator.kt

@@ -14,6 +14,7 @@ import kotlinx.parcelize.Parcelize
 import kotlinx.parcelize.WriteWith
 import org.json.JSONArray
 import org.json.JSONObject
+import org.readium.r2.shared.DelicateReadiumApi
 import org.readium.r2.shared.JSONable
 import org.readium.r2.shared.extensions.*
 import org.readium.r2.shared.toJSON
@@ -197,10 +198,36 @@ public data class Locator(
 
     public companion object {
 
+        /**
+         * Creates a [Locator] from its JSON representation.
+         */
         public fun fromJSON(
             json: JSONObject?,
             mediaTypeRetriever: MediaTypeRetriever = MediaTypeRetriever(),
             warnings: WarningLogger? = null
+        ): Locator? =
+            fromJSON(json, mediaTypeRetriever, warnings, withLegacyHref = false)
+
+        /**
+         * Creates a [Locator] from its legacy JSON representation.
+         *
+         * Only use this API when you are upgrading to Readium 3.x and migrating the [Locator]
+         * objects stored in your database. See the migration guide for more information.
+         */
+        @DelicateReadiumApi
+        public fun fromLegacyJSON(
+            json: JSONObject?,
+            mediaTypeRetriever: MediaTypeRetriever = MediaTypeRetriever(),
+            warnings: WarningLogger? = null
+        ): Locator? =
+            fromJSON(json, mediaTypeRetriever, warnings, withLegacyHref = true)
+
+        @OptIn(DelicateReadiumApi::class)
+        private fun fromJSON(
+            json: JSONObject?,
+            mediaTypeRetriever: MediaTypeRetriever = MediaTypeRetriever(),
+            warnings: WarningLogger? = null,
+            withLegacyHref: Boolean = false
         ): Locator? {
             val href = json?.optNullableString("href")
             val type = json?.optNullableString("type")
@@ -209,7 +236,13 @@ public data class Locator(
                 return null
             }
 
-            val url = Url(href) ?: run {
+            val url = (
+                if (withLegacyHref) {
+                    Url.fromLegacyHref(href)
+                } else {
+                    Url(href)
+                }
+                ) ?: run {
                 warnings?.log(Locator::class.java, "[href] is not a valid URL", json)
                 return null
             }

readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt

@@ -13,6 +13,7 @@ import java.io.File
 import java.net.URI
 import java.net.URL
 import kotlinx.parcelize.Parcelize
+import org.readium.r2.shared.DelicateReadiumApi
 import org.readium.r2.shared.InternalReadiumApi
 import org.readium.r2.shared.extensions.percentEncodedPath
 import org.readium.r2.shared.extensions.tryOrNull
@@ -42,6 +43,19 @@ public sealed class Url : Parcelable {
             return invoke(Uri.parse(url))
         }
 
+        /**
+         * Creates an [Url] from a legacy HREF.
+         *
+         * For example, if it is a relative path such as `/dir/my chapter.html`, it will be
+         * converted to the valid relative URL `dir/my%20chapter.html`.
+         *
+         * Only use this API when you are upgrading to Readium 3.x and migrating the HREFs stored in
+         * your database. See the 3.0 migration guide for more information.
+         */
+        @DelicateReadiumApi
+        public fun fromLegacyHref(href: String): Url? =
+            AbsoluteUrl(href) ?: fromDecodedPath(href.removePrefix("/"))
+
         internal operator fun invoke(uri: Uri): Url? =
             if (uri.isAbsolute) {
                 AbsoluteUrl(uri)
@@ -286,7 +300,6 @@ public class RelativeUrl private constructor(override val uri: Uri) : Url() {
         internal operator fun invoke(uri: Uri): RelativeUrl? =
             tryOrNull {
                 require(uri.isRelative)
-                require(uri.isHierarchical)
                 RelativeUrl(uri)
             }
     }

readium/shared/src/test/java/org/readium/r2/shared/publication/LocatorTest.kt

@@ -14,6 +14,7 @@ import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.readium.r2.shared.DelicateReadiumApi
 import org.readium.r2.shared.assertJSONEquals
 import org.readium.r2.shared.util.Url
 import org.readium.r2.shared.util.mediatype.MediaType
@@ -71,6 +72,26 @@ class LocatorTest {
         assertNull(Locator.fromJSON(JSONObject("{ 'invalid': 'object' }")))
     }
 
+    @OptIn(DelicateReadiumApi::class)
+    @Test
+    fun `parse {Locator} with legacy HREF`() {
+        val json = JSONObject(
+            """
+            {
+                "href": "legacy href",
+                "type": "text/html"
+            }
+        """
+        )
+
+        assertNull(Locator.fromJSON(json))
+
+        assertEquals(
+            Locator(href = Url("legacy%20href")!!, mediaType = MediaType.HTML),
+            Locator.fromLegacyJSON(json)
+        )
+    }
+
     @Test fun `get {Locator} minimal JSON`() {
         assertJSONEquals(
             JSONObject(

readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt

@@ -4,10 +4,13 @@ import android.net.Uri
 import java.io.File
 import java.net.URI
 import java.net.URL
+import kotlin.test.assertIs
+import kotlin.test.assertNotNull
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.readium.r2.shared.DelicateReadiumApi
 import org.readium.r2.shared.util.Url.Query
 import org.readium.r2.shared.util.Url.QueryParameter
 import org.robolectric.RobolectricTestRunner
@@ -34,8 +37,45 @@ class UrlTest {
         assertEquals(RelativeUrl(Uri.parse("foo/bar")), Url("foo/bar"))
         assertEquals(RelativeUrl(Uri.parse("../bar")), Url("../bar"))
 
+        // Special characters valid in a path.
+        assertEquals("$&+,/=@", RelativeUrl("$&+,/=@")?.path)
+
         // Used in the EPUB parser
-        assertEquals(RelativeUrl(Uri.parse("#")), Url("#"))
+        val url = Url("#") as? RelativeUrl
+        assertNotNull(url)
+        assertEquals(null, url.path)
+        assertEquals(null, url.fragment)
+    }
+
+    @OptIn(DelicateReadiumApi::class)
+    @Test
+    fun createFromLegacyHref() {
+        testLegacy<RelativeUrl>("dir/chapter.xhtml", "dir/chapter.xhtml")
+        // Starting slash is removed.
+        testLegacy<RelativeUrl>("/dir/chapter.xhtml", "dir/chapter.xhtml")
+        // Special characters are percent-encoded.
+        testLegacy<RelativeUrl>("/dir/per%cent.xhtml", "dir/per%25cent.xhtml")
+        testLegacy<RelativeUrl>("/barré.xhtml", "barr%C3%A9.xhtml")
+        testLegacy<RelativeUrl>("/spa ce.xhtml", "spa%20ce.xhtml")
+        // We assume that a relative path is percent-decoded.
+        testLegacy<RelativeUrl>("/spa%20ce.xhtml", "spa%2520ce.xhtml")
+        // Some special characters are authorized in a path.
+        testLegacy<RelativeUrl>("/$&+,/=@", "$&+,/=@")
+        // Valid absolute URL are left untouched.
+        testLegacy<AbsoluteUrl>(
+            "http://domain.com/a%20book?page=3",
+            "http://domain.com/a%20book?page=3"
+        )
+        // Invalid absolute URL.
+        assertNull(Url.fromLegacyHref("http://domain.com/a book"))
+    }
+
+    @OptIn(DelicateReadiumApi::class)
+    private inline fun <reified T : Url> testLegacy(href: String, expected: String) {
+        val url = Url.fromLegacyHref(href)
+        assertNotNull(url)
+        assertIs<T>(url)
+        assertEquals(expected, url.toString())
     }
 
     @Test