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 cc21cc9..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 @@ -347,17 +347,56 @@ 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() + + /** 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" + + 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/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 6661e66..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 @@ -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,7 +77,10 @@ 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.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 @@ -200,6 +204,7 @@ fun BookDetailScreen( onReserve = vm::openLoanSheet, onToggleWishlist = vm::toggleWishlist, onFindWorks = onFindWorks, + 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, @@ -338,6 +343,10 @@ internal fun DetailContent( onToggleWishlist: () -> Unit, onShowMessage: (String) -> Unit, onFindWorks: (String, Int?) -> 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. */ @@ -345,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 @@ -516,15 +536,34 @@ 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 - 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)) } @@ -602,6 +641,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/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 c00666c..3dfecf5 100644 --- a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt +++ b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt @@ -95,6 +95,48 @@ 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.assertNull(file.expiresAt) + } + + @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") + 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 }, onRefreshFile = { refreshes++; com.pinakes.app.data.network.ApiResult.Failure("network", "") }) + } } + compose.onNodeWithText("Read").performScrollTo().performClick() + compose.waitForIdle() + assertEquals(1, refreshes) + assertEquals(listOf("Could not refresh the link. Please try again."), 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/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 da7c81b..f4d4c76 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_refresh_error": "Der Link konnte nicht aktualisiert werden. Bitte versuche es erneut." } diff --git a/i18n/en.json b/i18n/en.json index 24a1131..2d6a501 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_refresh_error": "Could not refresh the link. Please try again." } diff --git a/i18n/fr.json b/i18n/fr.json index e7a4498..d248452 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_refresh_error": "Impossible d’actualiser le lien. Réessayez." } diff --git a/i18n/it.json b/i18n/it.json index 8c3191f..4242c69 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_refresh_error": "Impossibile aggiornare il collegamento. Riprova." }