From 0177791c29b652bae194bf8e3690522313e65063 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 15 Oct 2026 12:32:20 +0200 Subject: [PATCH 1/3] Show the digital files a reader cannot open yet, with the reason Pinakes 0.8.6 decides who may open each digital file (fabiodalez-dev/Pinakes#480) and tells the app per file: a closed one keeps its entry with url null and access login_required or loan_required. The detail page now keeps that card, with a lock and the reason ("For readers who have this book on loan", "Sign in to open it") in place of the button, in the four languages. Closed files all arrive with the same empty address, so the list no longer folds them into one: only the open files are told apart by their address. Servers before 0.8.6 send neither field and every file stays open, as before. The links to the files the reader may open are signed and last a day. A tap on one that has expired fetches the record again for a fresh link and asks to tap again, instead of opening a file the server would refuse. --- .../java/com/pinakes/app/data/model/Models.kt | 38 +++++++++++++++- .../app/ui/screens/detail/BookDetailScreen.kt | 33 +++++++++++++- .../pinakes/app/DigitalAvailabilityTest.kt | 43 +++++++++++++++++++ i18n/de.json | 6 ++- i18n/en.json | 6 ++- i18n/fr.json | 6 ++- i18n/it.json | 6 ++- 7 files changed, 130 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/pinakes/app/data/model/Models.kt b/app/src/main/java/com/pinakes/app/data/model/Models.kt index cc21cc9..f36f65e 100644 --- a/app/src/main/java/com/pinakes/app/data/model/Models.kt +++ b/app/src/main/java/com/pinakes/app/data/model/Models.kt @@ -347,17 +347,51 @@ data class BookDetail( audioUrl?.takeIf { hasAudio && it.isNotBlank() }?.let { DigitalAttachment(it, kind = "audio") }, ebookUrl?.takeIf { hasEbook && it.isNotBlank() }?.let { DigitalAttachment(it, kind = "ebook") }, ) - }.filter { it.safeUrl != null }.distinctBy { it.url } + }.filter { it.safeUrl != null || it.locked }.let { files -> + // A closed file has no address (the server sends url = null): several of them are + // all "", so only the open ones are told apart by their address. + val seen = HashSet() + files.filter { it.locked || seen.add(it.url) } + } } +/** + * One digital file of a record. Servers from Pinakes 0.8.6 (issue #480) say who may open + * it and what this reader may do: a closed file keeps its entry with an empty [url] and + * the reason in [access]. Older servers send neither field, and every file is open. + */ @Serializable -data class DigitalAttachment(val url: String = "", val label: String = "", val kind: String = "ebook") { +data class DigitalAttachment( + val url: String = "", + val label: String = "", + val kind: String = "ebook", + /** public | users | staff | loan, as resolved by the server; null on older servers. */ + val visibility: String? = null, + /** granted | login_required | loan_required | staff_only; null on older servers (= granted). */ + val access: String? = null, +) { + val locked: Boolean get() = access != null && access != ACCESS_GRANTED val safeUrl: String? get() = url.takeIf { val uri = runCatching { java.net.URI(it) }.getOrNull() uri?.scheme?.lowercase() in listOf("http", "https") && !uri?.host.isNullOrBlank() } + + /** When a signed link stops working (its pk_exp, in epoch seconds); null for an unsigned link. */ + val expiresAt: Long? get() = runCatching { java.net.URI(url).rawQuery }.getOrNull() + ?.split('&')?.firstOrNull { it.startsWith("pk_exp=") }?.substringAfter('=')?.toLongOrNull() + + /** The signed link has expired, or will within a minute: the record must be fetched again first. */ + fun linkExpired(nowSeconds: Long = System.currentTimeMillis() / 1000): Boolean = + expiresAt?.let { it <= nowSeconds + 60 } == true + val format: String get() = runCatching { java.net.URI(url).path.substringAfterLast('.').uppercase() }.getOrDefault("") val isPdf: Boolean get() = format == "PDF" + + companion object { + const val ACCESS_GRANTED = "granted" + const val ACCESS_LOGIN_REQUIRED = "login_required" + const val ACCESS_LOAN_REQUIRED = "loan_required" + } } @Serializable diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 6661e66..05f8a30 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -40,6 +40,7 @@ import androidx.compose.material.icons.outlined.FavoriteBorder import androidx.compose.material.icons.outlined.HourglassEmpty import androidx.compose.material.icons.outlined.Schedule import androidx.compose.material.icons.outlined.LocalOffer +import androidx.compose.material.icons.outlined.Lock import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon @@ -76,6 +77,7 @@ import com.pinakes.app.R import com.pinakes.app.data.model.AvailabilityCalendar import com.pinakes.app.data.model.BookDetail import com.pinakes.app.data.model.CirculationRequestKind +import com.pinakes.app.data.model.DigitalAttachment import com.pinakes.app.data.model.PersonalHistory import com.pinakes.app.ui.common.AppViewModel import com.pinakes.app.ui.common.OfflinePolicy @@ -200,6 +202,7 @@ fun BookDetailScreen( onReserve = vm::openLoanSheet, onToggleWishlist = vm::toggleWishlist, onFindWorks = onFindWorks, + onRefreshLinks = vm::load, onShowMessage = { msg -> scope.launch { snackbarHost.showSnackbar(msg) } }, offlineNotice = if (state.offlineNow) stringResource(R.string.offline_actions_unavailable) else null, offlineSyncedAt = state.syncedAt, @@ -338,6 +341,8 @@ internal fun DetailContent( onToggleWishlist: () -> Unit, onShowMessage: (String) -> Unit, onFindWorks: (String, Int?) -> Unit = { _, _ -> }, + /** Fetches the record again: its signed file links last a day (issue #480). */ + onRefreshLinks: () -> Unit = {}, /** Set when the page is built from the offline cache: shown above the cover. */ offlineNotice: String? = null, /** Last sync time of the offline copy: availability is shown as "last known", never current. */ @@ -516,11 +521,17 @@ internal fun DetailContent( DigitalFileCard(type = file.format.ifBlank { if (audio) "Audio" else "eBook" }, name = file.label.ifBlank { digitalFilename(file.url, book.title) }, kind = stringResource(if (audio) R.string.book_section_audiobook else if (file.kind == "supplement") R.string.book_section_related_documents else R.string.book_section_ebook)) { - if (audio && selectedAudio == file.url) { + if (file.locked) { + LockedDigitalFile(file.access) + } else if (audio && selectedAudio == file.url) { androidx.compose.runtime.key(file.url) { AudioPlayer(audioUrl = file.url) } } else PrimaryButton(label = stringResource(if (audio) R.string.collection_play else if (file.isPdf) R.string.book_read else R.string.ebook_open_external), onClick = { - if (audio) selectedAudio = file.url + // A signed link lasts a day: past that, fetch the record again for a fresh one. + if (file.linkExpired()) { + onRefreshLinks() + onShowMessage(context.getString(R.string.digital_link_refreshed)) + } else if (audio) selectedAudio = file.url else if (file.isPdf) selectedPdf = file.url else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } @@ -602,6 +613,24 @@ internal fun DetailContent( } } +/** + * A file this reader may not open yet: no button, and the reason instead (issue #480). + * The server leaves staff-only files out of the list, so only the other reasons reach here. + */ +@Composable +private fun LockedDigitalFile(access: String?) { + val colors = LocalPinakesColors.current + Row(verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.fillMaxWidth().testTag("digital-locked")) { + Icon(Icons.Outlined.Lock, contentDescription = null, tint = colors.muted, modifier = Modifier.size(18.dp)) + Text(stringResource(when (access) { + DigitalAttachment.ACCESS_LOGIN_REQUIRED -> R.string.digital_locked_login + DigitalAttachment.ACCESS_LOAN_REQUIRED -> R.string.digital_locked_loan + else -> R.string.digital_locked_other + }), style = MaterialTheme.typography.bodyMedium, color = colors.muted) + } +} + /** * Personal-status banner driven by [personalHistory]. Priority: active loan (primary-tinted) * → pending request (neutral secondary-tinted) → reservation (tertiary-tinted) → wishlist diff --git a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt index c00666c..cb3e09c 100644 --- a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt +++ b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt @@ -95,6 +95,49 @@ class DigitalAvailabilityTest { org.junit.Assert.assertFalse(old.copy(ebookUrl = "javascript:bad").digitalOnly) } + // The app's own parser settings (NetworkModule.json): a closed file's url = null becomes "". + private val api = Json { ignoreUnknownKeys = true; explicitNulls = false; isLenient = true; coerceInputValues = true } + + @Test fun closedFilesKeepTheirCardWithTheReasonInsteadOfAButton() { + val book = api.decodeFromString("""{"id":7,"title":"Access","availability":{"copies_total":1}, + "digital_attachments":[ + {"url":"https://library.example/uploads/digital/open.pdf?pk_exp=4102444800&pk_uid=3&pk_sig=ab","label":"Open edition","kind":"ebook","visibility":"public","access":"granted"}, + {"url":null,"label":"Readers edition","kind":"ebook","visibility":"users","access":"login_required"}, + {"url":null,"label":"Loan edition","kind":"audio","visibility":"loan","access":"loan_required"}]}""") + // Two closed files share the same empty address: both stay listed. + assertEquals(listOf("Open edition", "Readers edition", "Loan edition"), book.digitalFiles.map { it.label }) + show(book) + compose.onNodeWithText("Sign in to open it").performScrollTo().assertIsDisplayed() + compose.onNodeWithText("For readers who have this book on loan").performScrollTo().assertIsDisplayed() + compose.onAllNodesWithTag("digital-locked").assertCountEquals(2) + compose.onAllNodesWithText("Read").assertCountEquals(1) + compose.onAllNodesWithText("Play").assertCountEquals(0) + } + + @Test fun filesFromServersBeforeAccessControlAreOpen() { + val book = api.decodeFromString("""{"title":"Old","digital_attachments":[{"url":"https://library.example/a.pdf","label":"A","kind":"ebook"}]}""") + val file = book.digitalFiles.single() + org.junit.Assert.assertFalse(file.locked) + org.junit.Assert.assertFalse(file.linkExpired()) + } + + @Test fun anExpiredSignedLinkFetchesTheRecordAgainInsteadOfOpening() { + var refreshes = 0 + val messages = mutableListOf() + val expired = DigitalAttachment("https://library.example/uploads/digital/x.pdf?pk_exp=1000&pk_uid=3&pk_sig=ab", label = "Old link", access = "granted") + org.junit.Assert.assertTrue(expired.linkExpired()) + org.junit.Assert.assertFalse(expired.copy(url = "https://library.example/uploads/digital/x.pdf?pk_exp=4102444800").linkExpired()) + compose.setContent { PinakesTheme { + DetailContent(BookDetail(title = "Expired", digitalAttachments = listOf(expired)), wishlisted = false, reserveBusy = false, + canBorrow = true, showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, + onShowMessage = { messages += it }, onRefreshLinks = { refreshes++ }) + } } + compose.onNodeWithText("Read").performScrollTo().performClick() + compose.waitForIdle() + assertEquals(1, refreshes) + assertEquals(listOf("Link refreshed: tap again to open it."), messages) + } + @Test fun wantedCreditsKeepTranslatorSeparateAndSupportOlderServers() { val book = Json.decodeFromString("""{"id":133,"title":"Wanted","author":"Tomás Ibáñez","contributors":[{"id":1,"name":"Tomás Ibáñez","role":"principale"},{"id":2,"name":"Alice Zaffaroni","role":"traduttore"}]}""") val aliases = listOf(AuthorRef(3, "Italian alias", "autore"), AuthorRef(4, "English alias", "author"), diff --git a/i18n/de.json b/i18n/de.json index da7c81b..b1e83bf 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Kritische Ausgabe", "staff_expression_audio": "Audio", "staff_expression_other": "Andere", - "staff_field_title_plain": "Titel" + "staff_field_title_plain": "Titel", + "digital_locked_login": "Zum Öffnen anmelden", + "digital_locked_loan": "Für Leser, die dieses Buch ausgeliehen haben", + "digital_locked_other": "Für dein Konto nicht verfügbar", + "digital_link_refreshed": "Link aktualisiert: tippe erneut, um ihn zu öffnen." } diff --git a/i18n/en.json b/i18n/en.json index 24a1131..64d8b52 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Critical edition", "staff_expression_audio": "Audio", "staff_expression_other": "Other", - "staff_field_title_plain": "Title" + "staff_field_title_plain": "Title", + "digital_locked_login": "Sign in to open it", + "digital_locked_loan": "For readers who have this book on loan", + "digital_locked_other": "Not available to your account", + "digital_link_refreshed": "Link refreshed: tap again to open it." } diff --git a/i18n/fr.json b/i18n/fr.json index e7a4498..402ee39 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Édition critique", "staff_expression_audio": "Audio", "staff_expression_other": "Autre", - "staff_field_title_plain": "Titre" + "staff_field_title_plain": "Titre", + "digital_locked_login": "Connectez-vous pour l'ouvrir", + "digital_locked_loan": "Pour les lecteurs qui ont emprunté ce livre", + "digital_locked_other": "Non disponible pour votre compte", + "digital_link_refreshed": "Lien actualisé : touchez à nouveau pour l'ouvrir." } diff --git a/i18n/it.json b/i18n/it.json index 8c3191f..395eddc 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Edizione critica", "staff_expression_audio": "Audio", "staff_expression_other": "Altro", - "staff_field_title_plain": "Titolo" + "staff_field_title_plain": "Titolo", + "digital_locked_login": "Accedi per aprirlo", + "digital_locked_loan": "Per chi ha il libro in prestito", + "digital_locked_other": "Non disponibile per il tuo account", + "digital_link_refreshed": "Collegamento aggiornato: tocca di nuovo per aprire." } From 1568f7db6d71d14e42944ab21ae8a156787b03ab Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 00:19:03 +0200 Subject: [PATCH 2/3] Fix signed file renewal without device-clock or stale-cache assumptions --- .../staffe2e/DigitalLinkRefreshFlowTest.kt | 106 ++++++++++++ .../com/pinakes/app/staffe2e/FakePinakes.kt | 7 +- .../java/com/pinakes/app/data/model/Models.kt | 11 +- .../pinakes/app/data/network/PinakesApi.kt | 1 + .../app/data/repository/CatalogRepository.kt | 10 +- .../app/ui/screens/detail/BookDetailScreen.kt | 54 ++++-- .../ui/screens/detail/BookDetailViewModel.kt | 19 +++ .../pinakes/app/DigitalAvailabilityTest.kt | 11 +- .../com/pinakes/app/DigitalLinkRefreshTest.kt | 157 ++++++++++++++++++ i18n/de.json | 2 +- i18n/en.json | 2 +- i18n/fr.json | 2 +- i18n/it.json | 2 +- 13 files changed, 352 insertions(+), 32 deletions(-) create mode 100644 app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt create mode 100644 app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt diff --git a/app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt b/app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt new file mode 100644 index 0000000..4c59b84 --- /dev/null +++ b/app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt @@ -0,0 +1,106 @@ +package com.pinakes.app.staffe2e + +import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModelStore +import com.pinakes.app.data.model.BookDetail +import com.pinakes.app.data.model.DigitalAttachment +import com.pinakes.app.data.model.Envelope +import com.pinakes.app.data.network.ApiResult +import com.pinakes.app.data.network.ErrorCodes +import com.pinakes.app.ui.common.UiState +import com.pinakes.app.ui.navigation.Routes +import com.pinakes.app.ui.screens.detail.BookDetailViewModel +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.encodeToString +import kotlinx.serialization.json.Json +import org.junit.After +import org.junit.Assert.* +import org.junit.Test + +/** Real repository/ViewModel, HTTP cache and access decisions on a disposable emulator. */ +class DigitalLinkRefreshFlowTest : AppFlowTest() { + private val old = DigitalAttachment("https://library.example/a.epub?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + private val fresh = old.copy(url = old.url.replace("1000", "4102444800").replace("old", "new")) + private val book = BookDetail(id = 1, title = "Digital", digitalAttachments = listOf(old)) + private val store = ViewModelStore() + + private fun FakePinakes.answer(data: BookDetail = book) = body("GET catalog/books/1", 200, + Json.encodeToString(Envelope(data = data)), mapOf("ETag" to "\"digital-v1\"", "Cache-Control" to "private, max-age=3600")) + + private fun viewModel(): BookDetailViewModel { + lateinit var vm: BookDetailViewModel + instrumentation.runOnMainSync { + vm = BookDetailViewModel(SavedStateHandle(mapOf(Routes.ARG_BOOK_ID to 1)), + deps.catalogRepository(), deps.libraryRepository(), deps.wishlistRepository(), deps.connectivity()) + store.put("digital", vm) + } + compose.waitUntil(10_000) { vm.state.value.content is UiState.Success } + return vm + } + + @After fun clearViewModels() { instrumentation.runOnMainSync { store.clear() } } + + @Test fun forcedRenewalBypassesTheHttpCacheAndGetsANewSignature() { + launch { answer() } + val repository = deps.catalogRepository() + assertTrue(runBlocking { repository.book(1) } is ApiResult.Success) + api.answer(book.copy(digitalAttachments = listOf(fresh))) + val response = runBlocking { repository.book(1, forceRefresh = true) } + assertEquals(fresh.url, (response as ApiResult.Success).data.digitalFiles.single().url) + val request = api.recorded("GET", "catalog/books/1").last() + assertNull(request.getHeader("If-None-Match")) + assertEquals("no-cache, no-store", request.getHeader("Cache-Control")) + assertEquals(2, api.recorded("GET", "catalog/books/1").size) + } + + @Test fun aServerFailureDuringRenewalCannotReturnTheCachedSignedLink() { + launch { answer() } + val repository = deps.catalogRepository() + assertTrue(runBlocking { repository.book(1) } is ApiResult.Success) + api.body("GET catalog/books/1", 503, """{"error":{"code":"server_error","message":"Unavailable"}}""") + val response = runBlocking { repository.book(1, forceRefresh = true) } + assertTrue(response is ApiResult.Failure) + assertEquals(503, (response as ApiResult.Failure).httpStatus) + } + + @Test fun anUnexpected304CannotPretendTheExpiredPayloadWasRenewed() { + launch { answer() } + val repository = deps.catalogRepository() + assertTrue(runBlocking { repository.book(1) } is ApiResult.Success) + api.body("GET catalog/books/1", 304, "") + assertTrue(runBlocking { repository.book(1, forceRefresh = true) } is ApiResult.Failure) + } + + @Test fun renewalFailurePreservesTheLoadedPageAndASubsequentRetrySucceeds() { + launch { answer() } + val vm = viewModel() + val before = vm.state.value.content + api.body("GET catalog/books/1", 500, """{"error":{"code":"server_error","message":"Unavailable"}}""") + assertTrue(runBlocking { vm.refreshDigitalFile(old) } is ApiResult.Failure) + assertSame(before, vm.state.value.content) + api.answer(book.copy(digitalAttachments = listOf(fresh))) + val result = runBlocking { vm.refreshDigitalFile(old) } + assertEquals(fresh.url, (result as ApiResult.Success).data.url) + assertEquals(fresh.url, (vm.state.value.content as UiState.Success).data.digitalFiles.single().url) + } + + @Test fun renewalSelectsTheSameFileEvenWhenTheServerReordersAndRenamesEntries() { + launch { answer() } + val vm = viewModel() + val other = fresh.copy(url = fresh.url.replace("a.epub", "b.epub")) + api.answer(book.copy(digitalAttachments = listOf(other, fresh.copy(label = "Renamed")))) + val result = runBlocking { vm.refreshDigitalFile(old) } + assertEquals(fresh.url, (result as ApiResult.Success).data.url) + } + + @Test fun revokedFileAccessUpdatesTheCardAndReturnsNoOpenableAddress() { + launch { answer() } + val vm = viewModel() + api.answer(book.copy(digitalAttachments = listOf(old.copy(url = "", access = "loan_required")))) + val result = runBlocking { vm.refreshDigitalFile(old) } + assertEquals(ErrorCodes.FORBIDDEN, (result as ApiResult.Failure).code) + val file = (vm.state.value.content as UiState.Success).data.digitalFiles.single() + assertTrue(file.locked) + assertNull(file.safeUrl) + } +} diff --git a/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt b/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt index bee02aa..5366f8d 100644 --- a/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt +++ b/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt @@ -71,8 +71,11 @@ class FakePinakes { } } - fun body(route: String, status: Int, body: String) { - routes[route] = { MockResponse().setResponseCode(status).setHeader("Content-Type", "application/json").setBody(body) } + fun body(route: String, status: Int, body: String, headers: Map = emptyMap()) { + routes[route] = { + MockResponse().setResponseCode(status).setHeader("Content-Type", "application/json").setBody(body) + .apply { headers.forEach { (name, value) -> setHeader(name, value) } } + } } fun recorded(method: String, path: String): List = diff --git a/app/src/main/java/com/pinakes/app/data/model/Models.kt b/app/src/main/java/com/pinakes/app/data/model/Models.kt index f36f65e..b63b8a0 100644 --- a/app/src/main/java/com/pinakes/app/data/model/Models.kt +++ b/app/src/main/java/com/pinakes/app/data/model/Models.kt @@ -380,9 +380,14 @@ data class DigitalAttachment( val expiresAt: Long? get() = runCatching { java.net.URI(url).rawQuery }.getOrNull() ?.split('&')?.firstOrNull { it.startsWith("pk_exp=") }?.substringAfter('=')?.toLongOrNull() - /** The signed link has expired, or will within a minute: the record must be fetched again first. */ - fun linkExpired(nowSeconds: Long = System.currentTimeMillis() / 1000): Boolean = - expiresAt?.let { it <= nowSeconds + 60 } == true + /** Identity of the file across signature renewals; keep every non-signature query parameter. */ + val resourceKey: String get() { + val base = url.substringBefore('#').substringBefore('?') + val query = runCatching { java.net.URI(url).rawQuery }.getOrNull() + ?.split('&')?.filterNot { it.substringBefore('=') in setOf("pk_exp", "pk_uid", "pk_sig") } + ?.filter { it.isNotEmpty() }?.sorted().orEmpty() + return base + if (query.isEmpty()) "" else "?" + query.joinToString("&") + } val format: String get() = runCatching { java.net.URI(url).path.substringAfterLast('.').uppercase() }.getOrDefault("") val isPdf: Boolean get() = format == "PDF" diff --git a/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt b/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt index 9b78e0b..31ef01c 100644 --- a/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt +++ b/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt @@ -116,6 +116,7 @@ interface PinakesApi { suspend fun book( @Path("id") id: Int, @Header("If-None-Match") etag: String? = null, // -> 304 cache + @Header("Cache-Control") cacheControl: String? = null, ): Response> // Response<> to read ETag header + 304 @GET("catalog/books/{id}/availability") diff --git a/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt b/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt index e46d8ca..23b978c 100644 --- a/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt +++ b/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt @@ -256,13 +256,15 @@ class CatalogRepository( } } - suspend fun book(id: Int): ApiResult { + /** A signed-file renewal must reach the server and must never fall back to a stale payload. */ + suspend fun book(id: Int, forceRefresh: Boolean = false): ApiResult { val api = network.api() val cached = detailCache[id] - val (result, response) = apiResponse { api.book(id, etag = cached?.first) } + val (result, response) = apiResponse { api.book(id, etag = if (forceRefresh) null else cached?.first, + cacheControl = if (forceRefresh) "no-cache, no-store" else null) } // 304 Not Modified → reuse the cached payload. - if (response?.code() == 304 && cached != null) { + if (!forceRefresh && response?.code() == 304 && cached != null) { return ApiResult.Success(cached.second) } return when (result) { @@ -273,7 +275,7 @@ class CatalogRepository( } is ApiResult.Failure -> { // If the conditional request errored but we have a cached copy, serve it. - if (cached != null && result.code != ErrorCodes.NETWORK) { + if (!forceRefresh && cached != null && result.code != ErrorCodes.NETWORK) { ApiResult.Success(cached.second) } else { result diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 05f8a30..1c98c4a 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -79,6 +79,8 @@ import com.pinakes.app.data.model.BookDetail import com.pinakes.app.data.model.CirculationRequestKind import com.pinakes.app.data.model.DigitalAttachment import com.pinakes.app.data.model.PersonalHistory +import com.pinakes.app.data.network.ApiResult +import com.pinakes.app.data.network.ErrorCodes import com.pinakes.app.ui.common.AppViewModel import com.pinakes.app.ui.common.OfflinePolicy import com.pinakes.app.ui.components.PinakesBottomBar @@ -202,7 +204,7 @@ fun BookDetailScreen( onReserve = vm::openLoanSheet, onToggleWishlist = vm::toggleWishlist, onFindWorks = onFindWorks, - onRefreshLinks = vm::load, + onRefreshFile = vm::refreshDigitalFile, onShowMessage = { msg -> scope.launch { snackbarHost.showSnackbar(msg) } }, offlineNotice = if (state.offlineNow) stringResource(R.string.offline_actions_unavailable) else null, offlineSyncedAt = state.syncedAt, @@ -341,8 +343,10 @@ internal fun DetailContent( onToggleWishlist: () -> Unit, onShowMessage: (String) -> Unit, onFindWorks: (String, Int?) -> Unit = { _, _ -> }, - /** Fetches the record again: its signed file links last a day (issue #480). */ - onRefreshLinks: () -> Unit = {}, + /** Revalidates a signed file on the server, preserving this page while the request runs. */ + onRefreshFile: suspend (DigitalAttachment) -> ApiResult = { + ApiResult.Failure(ErrorCodes.NETWORK, "") + }, /** Set when the page is built from the offline cache: shown above the cover. */ offlineNotice: String? = null, /** Last sync time of the offline copy: availability is shown as "last known", never current. */ @@ -350,9 +354,20 @@ internal fun DetailContent( ) { val offline = offlineNotice != null val context = LocalContext.current + val scope = rememberCoroutineScope() + var refreshingFile by remember(book.id) { mutableStateOf(null) } var showCover by remember { mutableStateOf(false) } var selectedPdf by remember(book.id) { mutableStateOf(null) } - var selectedAudio by remember(book.id) { mutableStateOf(book.audioUrl?.takeIf { book.hasAudio }) } + var selectedAudio by remember(book.id) { mutableStateOf(null) } + + fun openFile(file: DigitalAttachment) { + if (file.locked || file.safeUrl == null) { + onShowMessage(context.getString(R.string.digital_locked_other)) + } else if (file.kind == "audio") selectedAudio = file.url + else if (file.isPdf) selectedPdf = file.url + else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } + .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } + } Column( Modifier @@ -527,15 +542,28 @@ internal fun DetailContent( androidx.compose.runtime.key(file.url) { AudioPlayer(audioUrl = file.url) } } else PrimaryButton(label = stringResource(if (audio) R.string.collection_play else if (file.isPdf) R.string.book_read else R.string.ebook_open_external), onClick = { - // A signed link lasts a day: past that, fetch the record again for a fresh one. - if (file.linkExpired()) { - onRefreshLinks() - onShowMessage(context.getString(R.string.digital_link_refreshed)) - } else if (audio) selectedAudio = file.url - else if (file.isPdf) selectedPdf = file.url - else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } - .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } - }, modifier = Modifier.fillMaxWidth(), dark = true) + // The server's clock and access decision are authoritative. Always renew + // a signed URL on demand, then open it in this same tap (no clock-skew loop). + if (refreshingFile == null) { + if (file.expiresAt == null) openFile(file) + else { + refreshingFile = file.resourceKey + scope.launch { + try { + when (val result = onRefreshFile(file)) { + is ApiResult.Success -> openFile(result.data) + is ApiResult.Failure -> onShowMessage(context.getString( + if (result.code == ErrorCodes.FORBIDDEN) R.string.digital_locked_other + else R.string.digital_link_refresh_error)) + } + } finally { + refreshingFile = null + } + } + } + } + }, enabled = refreshingFile == null, loading = refreshingFile == file.resourceKey, + modifier = Modifier.fillMaxWidth().testTag("digital-open"), dark = true) } Spacer(Modifier.height(16.dp)) } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt index 084def9..ffae9f7 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.pinakes.app.data.model.AvailabilityCalendar import com.pinakes.app.data.model.BookDetail +import com.pinakes.app.data.model.DigitalAttachment import com.pinakes.app.data.model.CirculationRequestKind import com.pinakes.app.data.network.ApiResult import com.pinakes.app.data.connectivity.ConnectivityObserver @@ -107,6 +108,24 @@ class BookDetailViewModel @Inject constructor( } } + /** Refresh in place: a failed renewal must not replace the page or reuse a cached URL. */ + suspend fun refreshDigitalFile(file: DigitalAttachment): ApiResult { + return when (val result = catalog.book(bookId, forceRefresh = true)) { + is ApiResult.Failure -> result + is ApiResult.Success -> { + _state.update { + it.copy(content = UiState.Success(result.data), offline = false, + wishlisted = result.data.personalHistory?.hasWishlisted ?: it.wishlisted) + } + val renewed = result.data.digitalFiles.firstOrNull { + it.resourceKey == file.resourceKey && it.kind == file.kind && !it.locked && it.safeUrl != null + } + if (renewed != null) ApiResult.Success(renewed) + else ApiResult.Failure(ErrorCodes.FORBIDDEN, "", 403) + } + } + } + fun toggleWishlist() { if (_state.value.wishlistBusy || _state.value.offlineNow) return val nowWishlisted = !_state.value.wishlisted diff --git a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt index cb3e09c..3dfecf5 100644 --- a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt +++ b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt @@ -118,24 +118,23 @@ class DigitalAvailabilityTest { val book = api.decodeFromString("""{"title":"Old","digital_attachments":[{"url":"https://library.example/a.pdf","label":"A","kind":"ebook"}]}""") val file = book.digitalFiles.single() org.junit.Assert.assertFalse(file.locked) - org.junit.Assert.assertFalse(file.linkExpired()) + org.junit.Assert.assertNull(file.expiresAt) } - @Test fun anExpiredSignedLinkFetchesTheRecordAgainInsteadOfOpening() { + @Test fun anExpiredSignedLinkDoesNotOpenOrClaimSuccessWhenRenewalFails() { var refreshes = 0 val messages = mutableListOf() val expired = DigitalAttachment("https://library.example/uploads/digital/x.pdf?pk_exp=1000&pk_uid=3&pk_sig=ab", label = "Old link", access = "granted") - org.junit.Assert.assertTrue(expired.linkExpired()) - org.junit.Assert.assertFalse(expired.copy(url = "https://library.example/uploads/digital/x.pdf?pk_exp=4102444800").linkExpired()) + assertEquals(1000L, expired.expiresAt) compose.setContent { PinakesTheme { DetailContent(BookDetail(title = "Expired", digitalAttachments = listOf(expired)), wishlisted = false, reserveBusy = false, canBorrow = true, showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, - onShowMessage = { messages += it }, onRefreshLinks = { refreshes++ }) + onShowMessage = { messages += it }, onRefreshFile = { refreshes++; com.pinakes.app.data.network.ApiResult.Failure("network", "") }) } } compose.onNodeWithText("Read").performScrollTo().performClick() compose.waitForIdle() assertEquals(1, refreshes) - assertEquals(listOf("Link refreshed: tap again to open it."), messages) + assertEquals(listOf("Could not refresh the link. Please try again."), messages) } @Test fun wantedCreditsKeepTranslatorSeparateAndSupportOlderServers() { diff --git a/app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt b/app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt new file mode 100644 index 0000000..e658dc4 --- /dev/null +++ b/app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt @@ -0,0 +1,157 @@ +package com.pinakes.app + +import android.app.Application +import android.content.ComponentName +import androidx.activity.ComponentActivity +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.* +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.test.core.app.ApplicationProvider +import com.pinakes.app.data.model.BookDetail +import com.pinakes.app.data.model.DigitalAttachment +import com.pinakes.app.data.network.ApiResult +import com.pinakes.app.data.network.ErrorCodes +import com.pinakes.app.ui.screens.detail.DetailContent +import com.pinakes.app.ui.theme.PinakesTheme +import kotlinx.coroutines.CompletableDeferred +import org.junit.Assert.* +import org.junit.Rule +import org.junit.Test +import org.junit.rules.RuleChain +import org.junit.rules.TestWatcher +import org.junit.runner.Description +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class, qualifiers = "w411dp-h891dp-xxhdpi") +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class DigitalLinkRefreshTest { + private val compose = createComposeRule() + private val app get() = ApplicationProvider.getApplicationContext() + @get:Rule val rules: RuleChain = RuleChain.outerRule(object : TestWatcher() { + override fun starting(description: Description) { + shadowOf(app.packageManager).addActivityIfNotPresent(ComponentName(app.packageName, ComponentActivity::class.java.name)) + } + }).around(compose) + + private val signed = DigitalAttachment("https://library.example/a.epub?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + private val fresh = signed.copy(url = signed.url.replace("1000", "4102444800").replace("old", "new")) + private val messages = mutableListOf() + + private fun show(file: DigitalAttachment = signed, refresh: suspend (DigitalAttachment) -> ApiResult) { + compose.setContent { PinakesTheme { + DetailContent(BookDetail(id = 7, title = "A digital book", digitalAttachments = listOf(file)), + wishlisted = false, reserveBusy = false, canBorrow = false, showWishlist = false, showReviews = false, + onReserve = {}, onToggleWishlist = {}, onShowMessage = { messages += it }, onRefreshFile = refresh) + } } + } + + private fun open() = compose.onNodeWithText(app.getString(R.string.ebook_open_external)).performScrollTo().performClick() + private fun nextOpenedUrl() = shadowOf(app).nextStartedActivity?.dataString + + @Test fun expiredLinksOpenTheirRenewedAddressInTheSameTap() { + var calls = 0 + show { calls++; ApiResult.Success(fresh) } + open() + compose.waitForIdle() + assertEquals(1, calls) + assertEquals(fresh.url, nextOpenedUrl()) + assertTrue(messages.isEmpty()) + } + + @Test fun evenFutureDatedLinksUseTheServerInsteadOfThePhonesClock() { + var calls = 0 + show(fresh) { calls++; ApiResult.Success(fresh) } + open() + compose.waitForIdle() + assertEquals(1, calls) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun pendingRenewalDisablesRepeatedTapsWithoutClaimingSuccess() { + val response = CompletableDeferred>() + var calls = 0 + show { calls++; response.await() } + open() + compose.onNodeWithTag("digital-open").assertIsNotEnabled() + assertNull(nextOpenedUrl()) + assertTrue(messages.isEmpty()) + compose.runOnIdle { response.complete(ApiResult.Success(fresh)) } + compose.waitForIdle() + assertEquals(1, calls) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun failedRenewalKeepsTheFileAndAllowsRetry() { + var calls = 0 + show { + if (++calls == 1) ApiResult.Failure(ErrorCodes.NETWORK, "offline") else ApiResult.Success(fresh) + } + open() + compose.onNodeWithText(app.getString(R.string.ebook_open_external)).assertIsDisplayed().assertIsEnabled() + assertEquals(listOf(app.getString(R.string.digital_link_refresh_error)), messages) + assertNull(nextOpenedUrl()) + open() + compose.waitForIdle() + assertEquals(2, calls) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun revokedAccessNeverOpensTheOldSignedAddress() { + show { ApiResult.Failure(ErrorCodes.FORBIDDEN, "", 403) } + open() + compose.waitForIdle() + assertNull(nextOpenedUrl()) + assertEquals(listOf(app.getString(R.string.digital_locked_other)), messages) + } + + @Test fun olderUnsignedLinksOpenWithoutARefreshRequest() { + val old = signed.copy(url = "https://library.example/legacy.epub", access = null) + var calls = 0 + show(old) { calls++; ApiResult.Success(fresh) } + open() + compose.waitForIdle() + assertEquals(0, calls) + assertEquals(old.url, nextOpenedUrl()) + } + + @Test fun refreshedBookDataPreservesThePositionOfTheOpenButton() { + var book by mutableStateOf(BookDetail(id = 7, title = "Book", digitalAttachments = listOf(signed))) + compose.setContent { PinakesTheme { + DetailContent(book, wishlisted = false, reserveBusy = false, canBorrow = false, + showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, onShowMessage = {}, + onRefreshFile = { book = book.copy(digitalAttachments = listOf(fresh)); ApiResult.Success(fresh) }) + } } + val button = compose.onNodeWithText(app.getString(R.string.ebook_open_external)).performScrollTo() + val before = button.getBoundsInRoot() + button.performClick() + compose.waitForIdle() + button.assertIsDisplayed() + assertEquals(before.top.value, button.getBoundsInRoot().top.value, .5f) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun signedAudioWaitsForTheReadersTapInsteadOfStartingAStalePlayer() { + val audio = signed.copy(url = signed.url.replace("epub", "mp3"), kind = "audio") + compose.setContent { PinakesTheme { + DetailContent(BookDetail(id = 7, title = "Audio", hasAudio = true, audioUrl = audio.url, + digitalAttachments = listOf(audio)), wishlisted = false, reserveBusy = false, canBorrow = false, + showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, onShowMessage = {}) + } } + compose.onNodeWithText(app.getString(R.string.collection_play)).performScrollTo().assertIsDisplayed() + } + + @Test fun signatureChangesKeepFileIdentityButOtherParametersDoNot() { + assertEquals(signed.resourceKey, fresh.resourceKey) + assertNotEquals(signed.resourceKey, signed.copy(url = signed.url + "&edition=2").resourceKey) + assertNotEquals(signed.resourceKey, signed.copy(url = signed.url.replace("a.epub", "b.epub")).resourceKey) + assertEquals(signed.copy(url = signed.url + "&b=2&a=1").resourceKey, + fresh.copy(url = fresh.url + "&a=1&b=2").resourceKey) + } +} diff --git a/i18n/de.json b/i18n/de.json index b1e83bf..f4d4c76 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Zum Öffnen anmelden", "digital_locked_loan": "Für Leser, die dieses Buch ausgeliehen haben", "digital_locked_other": "Für dein Konto nicht verfügbar", - "digital_link_refreshed": "Link aktualisiert: tippe erneut, um ihn zu öffnen." + "digital_link_refresh_error": "Der Link konnte nicht aktualisiert werden. Bitte versuche es erneut." } diff --git a/i18n/en.json b/i18n/en.json index 64d8b52..2d6a501 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Sign in to open it", "digital_locked_loan": "For readers who have this book on loan", "digital_locked_other": "Not available to your account", - "digital_link_refreshed": "Link refreshed: tap again to open it." + "digital_link_refresh_error": "Could not refresh the link. Please try again." } diff --git a/i18n/fr.json b/i18n/fr.json index 402ee39..d248452 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Connectez-vous pour l'ouvrir", "digital_locked_loan": "Pour les lecteurs qui ont emprunté ce livre", "digital_locked_other": "Non disponible pour votre compte", - "digital_link_refreshed": "Lien actualisé : touchez à nouveau pour l'ouvrir." + "digital_link_refresh_error": "Impossible d’actualiser le lien. Réessayez." } diff --git a/i18n/it.json b/i18n/it.json index 395eddc..4242c69 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Accedi per aprirlo", "digital_locked_loan": "Per chi ha il libro in prestito", "digital_locked_other": "Non disponibile per il tuo account", - "digital_link_refreshed": "Collegamento aggiornato: tocca di nuovo per aprire." + "digital_link_refresh_error": "Impossibile aggiornare il collegamento. Riprova." } From 2784ecd3ae9d0475c5bb5443cf8f7f984429cd38 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 05:45:27 +0000 Subject: [PATCH 3/3] Keep a playing audiobook when another file of the record is renewed Renewing any file swaps in a record whose URLs are all re-signed, so the player, matched to its card by the full signed URL, left composition and stopped. The started file is now remembered and matched by resourceKey, and the player keeps the URL it started with. --- .../app/ui/screens/detail/BookDetailScreen.kt | 19 +++++++++++--- .../com/pinakes/app/PlayingAudioUrlTest.kt | 25 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 1c98c4a..72923f6 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -358,12 +358,14 @@ internal fun DetailContent( var refreshingFile by remember(book.id) { mutableStateOf(null) } var showCover by remember { mutableStateOf(false) } var selectedPdf by remember(book.id) { mutableStateOf(null) } - var selectedAudio by remember(book.id) { mutableStateOf(null) } + // The audio file the reader started: renewing any file re-signs every URL, so the card + // is matched by resourceKey and the player keeps the URL it started with. + var selectedAudio by remember(book.id) { mutableStateOf(null) } fun openFile(file: DigitalAttachment) { if (file.locked || file.safeUrl == null) { onShowMessage(context.getString(R.string.digital_locked_other)) - } else if (file.kind == "audio") selectedAudio = file.url + } else if (file.kind == "audio") selectedAudio = file else if (file.isPdf) selectedPdf = file.url else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } @@ -538,8 +540,9 @@ internal fun DetailContent( kind = stringResource(if (audio) R.string.book_section_audiobook else if (file.kind == "supplement") R.string.book_section_related_documents else R.string.book_section_ebook)) { if (file.locked) { LockedDigitalFile(file.access) - } else if (audio && selectedAudio == file.url) { - androidx.compose.runtime.key(file.url) { AudioPlayer(audioUrl = file.url) } + } else if (audio && playingAudioUrl(selectedAudio, file) != null) { + val playingUrl = playingAudioUrl(selectedAudio, file)!! + androidx.compose.runtime.key(playingUrl) { AudioPlayer(audioUrl = playingUrl) } } else PrimaryButton(label = stringResource(if (audio) R.string.collection_play else if (file.isPdf) R.string.book_read else R.string.ebook_open_external), onClick = { // The server's clock and access decision are authoritative. Always renew @@ -758,6 +761,14 @@ private fun SectionTitle(text: String) { ) } +/** + * The URL the player started with when [file] is the audio the reader chose, else null. + * Matched by [DigitalAttachment.resourceKey], so a renewal of another file, which re-signs + * every URL of the record, does not stop or restart a playing audiobook. + */ +internal fun playingAudioUrl(selected: DigitalAttachment?, file: DigitalAttachment): String? = + selected?.takeIf { it.resourceKey == file.resourceKey }?.url + private fun digitalFilename(url: String, fallback: String): String = Uri.parse(url).lastPathSegment?.takeIf { it.isNotBlank() } ?: fallback diff --git a/app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt b/app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt new file mode 100644 index 0000000..d67084f --- /dev/null +++ b/app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt @@ -0,0 +1,25 @@ +package com.pinakes.app + +import com.pinakes.app.data.model.DigitalAttachment +import com.pinakes.app.ui.screens.detail.playingAudioUrl +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** A playing audiobook keeps its player when another file of the record is renewed. */ +class PlayingAudioUrlTest { + private val started = DigitalAttachment("https://library.example/uploads/digital/book.mp3?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + private val other = DigitalAttachment("https://library.example/uploads/digital/book.epub?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + + /** Renewing the eBook re-signs the audio URL too; the card still plays the URL it started with. */ + @Test fun aRenewedSignatureKeepsTheStartedUrl() { + val resigned = started.copy(url = started.url.replace("1000", "4102444800").replace("old", "new")) + assertEquals(started.url, playingAudioUrl(started, resigned)) + } + + /** Another file, or no started audio, shows the play button instead of a player. */ + @Test fun otherFilesAndNoSelectionHaveNoPlayer() { + assertNull(playingAudioUrl(started, other)) + assertNull(playingAudioUrl(null, started)) + } +}