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

Simplify RWPM parsing for packaged publications #380

Merged
merged 4 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import org.readium.r2.shared.asset.AssetRetriever
import org.readium.r2.shared.extensions.tryOr
import org.readium.r2.shared.publication.protection.ContentProtection
import org.readium.r2.shared.util.Try
import org.readium.r2.shared.util.mediatype.FormatRegistry
import org.readium.r2.shared.util.mediatype.MediaType
import timber.log.Timber

Expand Down Expand Up @@ -260,12 +261,5 @@ internal class LicensesService(
}

private val MediaType.fileExtension: String get() =
when {
matches(MediaType.DIVINA) -> "divina"
matches(MediaType.EPUB) -> "epub"
matches(MediaType.LCP_PROTECTED_PDF) -> "pdf"
matches(MediaType.READIUM_AUDIOBOOK) -> "audiobook"
matches(MediaType.READIUM_WEBPUB) -> "webpub"
else -> "epub"
}
FormatRegistry().fileExtension(this) ?: "epub"
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ private val skipBackwardInterval: Duration = 30.seconds
@OptIn(ExperimentalTime::class)
public class MediaSessionNavigator(
override val publication: Publication,
internal val publicationId: PublicationId,
private val controller: MediaControllerCompat,
public val publicationId: PublicationId,
public val controller: MediaControllerCompat,
public var listener: Listener? = null
) : MediaNavigator, CoroutineScope by MainScope() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,28 +155,23 @@ public data class Manifest(
*/
public fun fromJSON(
json: JSONObject?,
packaged: Boolean = false,
mediaTypeRetriever: MediaTypeRetriever = MediaTypeRetriever(),
warnings: WarningLogger? = null
): Manifest? {
json ?: return null

val baseUrl =
if (packaged) {
"/"
} else {
Link.fromJSONArray(
json.optJSONArray("links"),
mediaTypeRetriever,
warnings = warnings
)
.firstWithRel("self")
?.href
?.toUrlOrNull()
?.removeLastComponent()
?.toString()
?: "/"
}
Link.fromJSONArray(
json.optJSONArray("links"),
mediaTypeRetriever,
warnings = warnings
)
.firstWithRel("self")
?.href
?.toUrlOrNull()
?.removeLastComponent()
?.toString()
?: "/"

val normalizeHref = { href: String -> Href(href, baseUrl).string }

Expand All @@ -199,13 +194,6 @@ public data class Manifest(
normalizeHref,
warnings
)
.map {
if (packaged && "self" in it.rels) {
it.copy(rels = it.rels - "self" + "alternate")
} else {
it
}
}

// [readingOrder] used to be [spine], so we parse [spine] as a fallback.
val readingOrderJSON = (json.remove("readingOrder") ?: json.remove("spine")) as? JSONArray
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,53 +268,12 @@ class ManifestTest {
}

@Test
fun `self link is replaced when parsing a package`() {
assertEquals(
Manifest(
metadata = Metadata(localizedTitle = LocalizedString("Title")),
links = listOf(Link(href = "/manifest.json", rels = setOf("alternate")))
),
Manifest.fromJSON(
JSONObject(
"""{
"metadata": {"title": "Title"},
"links": [
{"href": "/manifest.json", "rel": ["self"], "templated": false}
]
}"""
),
packaged = true
)
)
}

@Test
fun `self link is kept when parsing a remote manifest`() {
assertEquals(
Manifest(
metadata = Metadata(localizedTitle = LocalizedString("Title")),
links = listOf(Link(href = "/manifest.json", rels = setOf("self")))
),
Manifest.fromJSON(
JSONObject(
"""{
"metadata": {"title": "Title"},
"links": [
{"href": "/manifest.json", "rel": ["self"]}
]
}"""
)
)
)
}

@Test
fun `href are resolved to root when parsing a package`() {
fun `href are resolved to root with a relative self link`() {
val json = JSONObject(
"""{
"metadata": {"title": "Title"},
"links": [
{"href": "http://example.com/manifest.json", "rel": ["self"], "templated": false}
{"href": "manifest.json", "rel": ["self"], "templated": false}
],
"readingOrder": [
{"href": "chap1.html", "type": "text/html", "templated": false}
Expand All @@ -324,7 +283,7 @@ class ManifestTest {

assertEquals(
"/chap1.html",
Manifest.fromJSON(json, packaged = true)?.readingOrder?.first()?.href
Manifest.fromJSON(json)?.readingOrder?.first()?.href
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import org.readium.r2.shared.publication.Publication
import org.readium.r2.shared.resource.Resource
import org.readium.r2.shared.resource.ResourceContainer
import org.readium.r2.shared.resource.RoutingContainer
import org.readium.r2.shared.resource.StringResource
import org.readium.r2.shared.util.MessageError
import org.readium.r2.shared.util.ThrowableError
import org.readium.r2.shared.util.Try
Expand Down Expand Up @@ -65,7 +64,7 @@ internal class ParserAssetFactory(
private suspend fun createParserAssetForManifest(
asset: Asset.Resource
): Try<PublicationParser.Asset, Publication.OpeningException> {
val manifest = asset.resource.readAsRwpm(packaged = false)
val manifest = asset.resource.readAsRwpm()
.mapFailure { Publication.OpeningException.ParsingFailed(ThrowableError(it)) }
.getOrElse { return Try.failure(it) }

Expand All @@ -87,10 +86,7 @@ internal class ParserAssetFactory(

val container =
RoutingContainer(
local = ResourceContainer(
path = "/manifest.json",
resource = StringResource(manifest.toJSON().toString(), asset.mediaType)
),
local = ResourceContainer(path = "/manifest.json", asset.resource),
remote = HttpContainer(httpClient, baseUrl)
)

Expand Down Expand Up @@ -118,14 +114,13 @@ internal class ParserAssetFactory(
)
}

private suspend fun Resource.readAsRwpm(packaged: Boolean): Try<Manifest, Exception> =
private suspend fun Resource.readAsRwpm(): Try<Manifest, Exception> =
try {
val bytes = read().getOrThrow()
val string = String(bytes, Charset.defaultCharset())
val json = JSONObject(string)
val manifest = Manifest.fromJSON(
json,
packaged = packaged,
mediaTypeRetriever = mediaTypeRetriever
)
?: throw Exception("Failed to parse the RWPM Manifest")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ public class ReadiumWebPubParser(

val manifest = Manifest.fromJSON(
manifestJson,
packaged = true,
mediaTypeRetriever = mediaTypeRetriever
)
?: return Try.failure(
Expand Down