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 f4954c3..54a47a3 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 @@ -17,6 +17,10 @@ data class Meta( @SerialName("total_count") val totalCount: Int? = null, val https: Boolean? = null, val warning: String? = null, // "insecure_transport" on /health + // Set by endpoints that cap their result set (periodicals year issues: 400 max) to say + // the list was cut short. Nullable with a null default on purpose: servers older than + // the field simply omit it, and absent must read as "not truncated", never as "true". + val truncated: Boolean? = null, ) @Serializable diff --git a/app/src/main/java/com/pinakes/app/data/model/PeriodicalsModels.kt b/app/src/main/java/com/pinakes/app/data/model/PeriodicalsModels.kt index 6c6a1dd..0f529de 100644 --- a/app/src/main/java/com/pinakes/app/data/model/PeriodicalsModels.kt +++ b/app/src/main/java/com/pinakes/app/data/model/PeriodicalsModels.kt @@ -115,6 +115,8 @@ data class PeriodicalIssueDetail( val pages: Int? = null, val status: String = "", @SerialName("cover_url") val coverUrl: String? = null, + /** Free-text note on inserts bound with this issue ("Supplemento letterario", …). */ + val supplements: String? = null, /** Only present when the digitised PDF is public; null → hide the "Open PDF" action. */ @SerialName("pdf_url") val pdfUrl: String? = null, val masthead: IssueMasthead? = null, diff --git a/app/src/main/java/com/pinakes/app/data/network/NetworkModule.kt b/app/src/main/java/com/pinakes/app/data/network/NetworkModule.kt index 5df18e3..0e0ea7f 100644 --- a/app/src/main/java/com/pinakes/app/data/network/NetworkModule.kt +++ b/app/src/main/java/com/pinakes/app/data/network/NetworkModule.kt @@ -3,11 +3,15 @@ package com.pinakes.app.data.network import com.jakewharton.retrofit2.converter.kotlinx.serialization.asConverterFactory import com.pinakes.app.BuildConfig import com.pinakes.app.data.store.SessionStore +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import kotlinx.serialization.json.Json +import okhttp3.Cache import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import okhttp3.logging.HttpLoggingInterceptor import retrofit2.Retrofit +import java.io.File import java.util.concurrent.TimeUnit /** @@ -16,8 +20,21 @@ import java.util.concurrent.TimeUnit * The base URL is per-instance and only known after onboarding, so the Retrofit instance is * (re)created whenever the instance URL changes. The bearer token is read live from * [SessionStore] by the [AuthInterceptor], so the same client survives login/logout. + * + * [cacheDir] (the app's cache directory) enables a disk HTTP cache. The server tags its + * cacheable GETs — the whole periodicals surface, for one — with an ETag and + * `Cache-Control: private, max-age=0, must-revalidate`, but without a cache OkHttp has no + * stored validator to send, so `If-None-Match` never goes out and every request pays for a + * full body. With the cache wired in, revalidation becomes transparent: OkHttp attaches the + * stored ETag and a 304 replays the cached body instead of downloading it again. + * `max-age=0` means nothing is ever served without asking the server first, so this saves + * bandwidth without ever serving stale data. + * + * The cache is keyed by URL only. Two accounts, or two instances that share a path, would + * otherwise collide — so it is purged on logout and on instance switch (see + * [clearHttpCache]) and no response body outlives the session that fetched it. */ -class NetworkModule(private val session: SessionStore) { +class NetworkModule(private val session: SessionStore, cacheDir: File? = null) { val json: Json = Json { ignoreUnknownKeys = true @@ -26,6 +43,15 @@ class NetworkModule(private val session: SessionStore) { coerceInputValues = true } + /** + * Disk cache for conditional GETs, or null when no cache directory was supplied (unit + * tests build the module without an Android context). Constructing a [Cache] only records + * the directory and the size budget — the on-disk journal is opened lazily on first use — + * so this is safe to do off the IO dispatcher. + */ + private val httpCache: Cache? = + cacheDir?.let { Cache(File(it, HTTP_CACHE_DIR), HTTP_CACHE_MAX_BYTES) } + private val okHttpClient: OkHttpClient by lazy { val builder = OkHttpClient.Builder() // Also honour the transient onboarding opt-in: during discovery the instance isn't @@ -36,6 +62,7 @@ class NetworkModule(private val session: SessionStore) { .connectTimeout(20, TimeUnit.SECONDS) .readTimeout(30, TimeUnit.SECONDS) .writeTimeout(30, TimeUnit.SECONDS) + httpCache?.let { builder.cache(it) } // Complete incomplete server certificate chains via AIA, like a browser does. Self-hosted // instances behind QNAP/Synology proxies often serve a chain missing its intermediate, which // the default Android TLS stack rejects ("Trust anchor for certification path not found") @@ -133,7 +160,29 @@ class NetworkModule(private val session: SessionStore) { cachedPeriodicalsApi = null } + /** + * Evict every stored response. Called on logout and on instance switch: cache entries are + * keyed by URL alone, so a body fetched for one account or one library must never be + * revalidated — let alone replayed — under the next one. + * + * Runs on the IO dispatcher (evicting walks the on-disk journal) and swallows failures: + * a cache that cannot be purged is a bandwidth problem, and must not be allowed to turn + * signing out into an error the user has to fight. + */ + suspend fun clearHttpCache() { + val cache = httpCache ?: return + withContext(Dispatchers.IO) { + runCatching { cache.evictAll() } + } + } + companion object { + /** Subdirectory of the app cache dir holding the OkHttp response cache. */ + private const val HTTP_CACHE_DIR = "http" + + /** ~10 MB: the cached surface is JSON, and OkHttp prunes the directory to fit. */ + private const val HTTP_CACHE_MAX_BYTES = 10L * 1024 * 1024 + /** * Derive the API base URL from a user-entered instance URL. * - when no scheme is given, prepends `https://` — or `http://` when the user has diff --git a/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt b/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt index 5450da3..71c58f7 100644 --- a/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt +++ b/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt @@ -173,6 +173,9 @@ class AuthRepository( val api = network.api() val result = apiCall { api.logout() } session.clearToken() + // Drop the cached HTTP responses of the session that just ended: they were fetched + // with a bearer token that no longer exists, and the cache is keyed by URL only. + network.clearHttpCache() return result } @@ -184,6 +187,9 @@ class AuthRepository( // instance and must never surface under the next library's name. catalog.clearCache() network.invalidate() + // Same reasoning as the Room purge: a URL-keyed HTTP cache entry from the old + // instance could otherwise be revalidated against the next library's server. + network.clearHttpCache() } private fun deviceName(): String { diff --git a/app/src/main/java/com/pinakes/app/di/AppModule.kt b/app/src/main/java/com/pinakes/app/di/AppModule.kt index bdc9d6f..6b76d30 100644 --- a/app/src/main/java/com/pinakes/app/di/AppModule.kt +++ b/app/src/main/java/com/pinakes/app/di/AppModule.kt @@ -41,8 +41,12 @@ object AppModule { @Provides @Singleton fun features(@ApplicationContext context: Context): FeatureStore = FeatureStore(context) + // The app cache dir backs the OkHttp response cache, so the server's ETags actually turn + // into conditional requests instead of full re-downloads. Android reclaims this directory + // under storage pressure, which is exactly the right lifetime for it. @Provides @Singleton - fun network(session: SessionStore): NetworkModule = NetworkModule(session) + fun network(@ApplicationContext context: Context, session: SessionStore): NetworkModule = + NetworkModule(session, context.cacheDir) @Provides @Singleton fun database(@ApplicationContext context: Context): AppDatabase = AppDatabase.get(context) diff --git a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailScreen.kt index 5f39bb7..a47d243 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailScreen.kt @@ -37,6 +37,7 @@ import com.pinakes.app.ui.common.DateFormat import com.pinakes.app.ui.common.UiState import com.pinakes.app.ui.common.resolvedMessage import com.pinakes.app.ui.components.AvailabilityChip +import com.pinakes.app.ui.components.EmptyState import com.pinakes.app.ui.components.ErrorState import com.pinakes.app.ui.components.LoadingState import com.pinakes.app.ui.components.PinakesTopBar @@ -69,7 +70,13 @@ fun IssueDetailScreen(onNavigateUp: () -> Unit) { ) { when (val content = state.content) { is UiState.Loading -> LoadingState(label = stringResource(R.string.periodicals_issue_loading)) - is UiState.Error -> ErrorState(message = content.resolvedMessage(), onRetry = vm::refresh) + is UiState.Error -> + // Plugin deactivated server-side: a terminal state, not a retryable + // error — retrying can only 404 again (see periodicalsFailureKind). + if (state.pluginGone) EmptyState( + title = stringResource(R.string.periodicals_gone_title), + subtitle = stringResource(R.string.periodicals_gone_subtitle), + ) else ErrorState(message = content.resolvedMessage(), onRetry = vm::refresh) is UiState.Success -> { val issue = content.data LazyColumn( @@ -163,6 +170,12 @@ private fun IssueHeader(issue: PeriodicalIssueDetail) { status = issueStatusBadge(issue.status), label = stringResource(issueStatusLabelRes(issue.status)), ) + // Free text and often absent — shown only when the server actually has something + // to say, using the same label/value row as the masthead detail. + issue.supplements?.takeIf { it.isNotBlank() }?.let { + Spacer(Modifier.height(Spacing.md)) + InfoRow(stringResource(R.string.periodicals_label_supplements), it) + } } } } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailViewModel.kt index 1d697b6..4c7d49d 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueDetailViewModel.kt @@ -20,6 +20,8 @@ import kotlinx.coroutines.launch data class IssueDetailUiState( val content: UiState = UiState.Loading, val refreshing: Boolean = false, + /** The plugin was deactivated server-side (confirmed via health re-probe). */ + val pluginGone: Boolean = false, ) @HiltViewModel @@ -45,12 +47,26 @@ class IssueDetailViewModel @Inject constructor( is ApiResult.Success -> _state.update { it.copy(content = UiState.Success(res.data), refreshing = false) } - is ApiResult.Failure -> _state.update { - it.copy( - content = if (it.content is UiState.Success) it.content - else UiState.Error(res.message, res.code, R.string.periodicals_issue_error), - refreshing = false, + is ApiResult.Failure -> { + // A 404 here is either a deleted fascicolo or a deactivated plugin: + // health decides which (see periodicalsFailureKind). + val kind = periodicalsFailureKind( + res, + goneConfirmed = isNotFoundFailure(res) && repo.confirmGone(), ) + _state.update { + it.copy( + content = if (it.content is UiState.Success) it.content + else periodicalsErrorState( + failure = res, + kind = kind, + genericRes = R.string.periodicals_issue_error, + notFoundRes = R.string.periodicals_issue_not_found, + ), + refreshing = false, + pluginGone = kind == PeriodicalsFailure.Gone, + ) + } } } } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListScreen.kt index b7b47de..c0f43e6 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListScreen.kt @@ -14,8 +14,10 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.outlined.Info import androidx.compose.material.icons.outlined.Newspaper import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold import androidx.compose.material3.Surface @@ -68,7 +70,13 @@ fun IssueListScreen( ) { when (val content = state.content) { is UiState.Loading -> LoadingState(label = stringResource(R.string.periodicals_issues_loading)) - is UiState.Error -> ErrorState(message = content.resolvedMessage(), onRetry = vm::refresh) + is UiState.Error -> + // Plugin deactivated server-side: a terminal state, not a retryable + // error — retrying can only 404 again (see periodicalsFailureKind). + if (state.pluginGone) EmptyState( + title = stringResource(R.string.periodicals_gone_title), + subtitle = stringResource(R.string.periodicals_gone_subtitle), + ) else ErrorState(message = content.resolvedMessage(), onRetry = vm::refresh) is UiState.Success -> if (content.data.isEmpty()) { EmptyState( @@ -82,6 +90,11 @@ fun IssueListScreen( contentPadding = PaddingValues(Spacing.lg), verticalArrangement = Arrangement.spacedBy(Spacing.md), ) { + // Informational, never blocking: the issues below are real and + // browsable, there are simply more of them on the server. + if (state.truncated) { + item { TruncatedNotice(shown = content.data.size) } + } items(content.data, key = { it.id }) { issue -> IssueRow(issue = issue, onClick = { onOpenIssue(issue.id) }) } @@ -92,6 +105,47 @@ fun IssueListScreen( } } +/** + * Partial-list notice for a year the server capped. Styled as an informational banner + * (secondary container, like the book detail's status banners) rather than an error: nothing + * failed and there is nothing to retry. + */ +@Composable +private fun TruncatedNotice(shown: Int) { + Surface( + modifier = Modifier.fillMaxWidth(), + shape = MaterialTheme.shapes.medium, + color = MaterialTheme.colorScheme.secondaryContainer, + contentColor = MaterialTheme.colorScheme.onSecondaryContainer, + ) { + Row( + modifier = Modifier.padding(horizontal = Spacing.lg, vertical = Spacing.md), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon( + Icons.Outlined.Info, + contentDescription = null, + tint = MaterialTheme.colorScheme.onSecondaryContainer, + modifier = Modifier.size(24.dp), + ) + Spacer(Modifier.width(Spacing.md)) + Column(Modifier.weight(1f)) { + Text( + stringResource(R.string.periodicals_issues_truncated_title), + style = MaterialTheme.typography.titleSmall, + color = MaterialTheme.colorScheme.onSecondaryContainer, + ) + Spacer(Modifier.height(Spacing.xs)) + Text( + stringResource(R.string.periodicals_issues_truncated_subtitle, shown), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSecondaryContainer, + ) + } + } + } +} + /** "No. 12 · Title" heading, or just the localized fallback when both are absent. */ @Composable internal fun issueHeading(number: String?, title: String?): String { diff --git a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListViewModel.kt index 94b7099..f675826 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/IssueListViewModel.kt @@ -20,6 +20,10 @@ import kotlinx.coroutines.launch data class IssueListUiState( val content: UiState> = UiState.Loading, val refreshing: Boolean = false, + /** The plugin was deactivated server-side (confirmed via health re-probe). */ + val pluginGone: Boolean = false, + /** The server capped this year's issues and said so via `meta.truncated`. */ + val truncated: Boolean = false, ) @HiltViewModel @@ -46,14 +50,32 @@ class IssueListViewModel @Inject constructor( viewModelScope.launch { when (val res = repo.yearIssues(yearId)) { is ApiResult.Success -> _state.update { - it.copy(content = UiState.Success(res.data), refreshing = false) - } - is ApiResult.Failure -> _state.update { it.copy( - content = if (it.content is UiState.Success) it.content - else UiState.Error(res.message, res.code, R.string.periodicals_issues_error), + content = UiState.Success(res.data), refreshing = false, + truncated = isTruncatedList(res.meta), + ) + } + is ApiResult.Failure -> { + // A 404 is either a year that no longer exists or a deactivated plugin: + // health decides which (see periodicalsFailureKind). + val kind = periodicalsFailureKind( + res, + goneConfirmed = isNotFoundFailure(res) && repo.confirmGone(), ) + _state.update { + it.copy( + content = if (it.content is UiState.Success) it.content + else periodicalsErrorState( + failure = res, + kind = kind, + genericRes = R.string.periodicals_issues_error, + notFoundRes = R.string.periodicals_issues_not_found, + ), + refreshing = false, + pluginGone = kind == PeriodicalsFailure.Gone, + ) + } } } } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailScreen.kt index dca022b..980731f 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailScreen.kt @@ -37,6 +37,7 @@ import com.pinakes.app.data.model.PeriodicalDetail import com.pinakes.app.data.model.PeriodicalYear import com.pinakes.app.ui.common.UiState import com.pinakes.app.ui.common.resolvedMessage +import com.pinakes.app.ui.components.EmptyState import com.pinakes.app.ui.components.ErrorState import com.pinakes.app.ui.components.LoadingState import com.pinakes.app.ui.components.PinakesTopBar @@ -65,7 +66,13 @@ fun PeriodicalDetailScreen( ) { when (val content = state.content) { is UiState.Loading -> LoadingState(label = stringResource(R.string.periodicals_detail_loading)) - is UiState.Error -> ErrorState(message = content.resolvedMessage(), onRetry = vm::refresh) + is UiState.Error -> + // Plugin deactivated server-side: a terminal state, not a retryable + // error — retrying can only 404 again (see periodicalsFailureKind). + if (state.pluginGone) EmptyState( + title = stringResource(R.string.periodicals_gone_title), + subtitle = stringResource(R.string.periodicals_gone_subtitle), + ) else ErrorState(message = content.resolvedMessage(), onRetry = vm::refresh) is UiState.Success -> { val detail = content.data LazyColumn( @@ -174,8 +181,9 @@ private fun coverageLabel(start: Int?, end: Int?): String? = when { else -> null } +/** Label/value detail row, shared with the issue detail header so both read identically. */ @Composable -private fun InfoRow(label: String, value: String) { +internal fun InfoRow(label: String, value: String) { Row(Modifier.fillMaxWidth().padding(vertical = Spacing.xxs)) { Text( label, diff --git a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailViewModel.kt index 42bf2f7..0294b9e 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalDetailViewModel.kt @@ -20,6 +20,8 @@ import kotlinx.coroutines.launch data class PeriodicalDetailUiState( val content: UiState = UiState.Loading, val refreshing: Boolean = false, + /** The plugin was deactivated server-side (confirmed via health re-probe). */ + val pluginGone: Boolean = false, ) @HiltViewModel @@ -46,12 +48,27 @@ class PeriodicalDetailViewModel @Inject constructor( is ApiResult.Success -> _state.update { it.copy(content = UiState.Success(res.data), refreshing = false) } - is ApiResult.Failure -> _state.update { - it.copy( - content = if (it.content is UiState.Success) it.content - else UiState.Error(res.message, res.code, R.string.periodicals_detail_error), - refreshing = false, + is ApiResult.Failure -> { + // Spend a health probe only on a 404, and only to tell "this masthead is + // gone" apart from "the whole section is gone" (which also flips the + // feature flag, hiding every entry point). + val kind = periodicalsFailureKind( + res, + goneConfirmed = isNotFoundFailure(res) && repo.confirmGone(), ) + _state.update { + it.copy( + content = if (it.content is UiState.Success) it.content + else periodicalsErrorState( + failure = res, + kind = kind, + genericRes = R.string.periodicals_detail_error, + notFoundRes = R.string.periodicals_detail_not_found, + ), + refreshing = false, + pluginGone = kind == PeriodicalsFailure.Gone, + ) + } } } } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUi.kt b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUi.kt index 0d73181..19d03a6 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUi.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUi.kt @@ -2,11 +2,16 @@ package com.pinakes.app.ui.screens.periodicals import androidx.annotation.StringRes import com.pinakes.app.R +import com.pinakes.app.data.model.Meta +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.components.AvailabilityStatus /** - * Pure helpers for the Periodicals screens: enum → localized label lookups and the - * issue-status → badge mapping. Kept free of Compose so they are unit-testable + * Pure helpers for the Periodicals screens: enum → localized label lookups, the + * issue-status → badge mapping, the truncated-list decision and the failure classification + * the detail screens share. Kept free of Compose so they are unit-testable * (see PeriodicalsUiStateTest). */ @@ -61,3 +66,70 @@ fun issueStatusBadge(status: String): AvailabilityStatus = when (status) { "danneggiato", "in_restauro" -> AvailabilityStatus.DueSoon else -> AvailabilityStatus.Returned } + +/** + * True when the server says it cut the list short (`meta.truncated`), which is the only + * signal for the 400-issue cap on a year's fascicoli. + * + * Servers that predate the field omit it, so `null` MUST read as "complete": inferring + * truncation from the item count instead would cry wolf on any year that happens to sit + * exactly on the cap, and would be flatly wrong the day the cap changes. + */ +internal fun isTruncatedList(meta: Meta?): Boolean = meta?.truncated == true + +/** How a Periodicals detail failure should be presented. */ +internal enum class PeriodicalsFailure { + /** The plugin itself is off: a terminal, non-retryable state. */ + Gone, + + /** The section is alive, this one masthead/year/issue is not. */ + NotFound, + + /** Anything else — network, 5xx, auth — worth a retry. */ + Error, +} + +/** True for a 404, the only failure worth spending a health probe on. */ +internal fun isNotFoundFailure(failure: ApiResult.Failure): Boolean = + failure.httpStatus == 404 || failure.code == ErrorCodes.NOT_FOUND + +/** + * Classify a detail failure. A 404 is ambiguous — the single resource may have been deleted, + * or the whole plugin may have been switched off server-side — and the two deserve opposite + * treatments: "this issue no longer exists" (the section still works, go back and browse) vs + * "the periodicals section is gone" (nothing here will ever load again). + * + * The health endpoint is the oracle, exactly as it is for the availability probe: + * [goneConfirmed] is `PeriodicalsRepository.confirmGone()`, which re-probes health and only + * answers true when health 404s too. Callers must evaluate it ONLY for a 404 — probing on + * every timeout would turn a network blip into an extra doomed request. + */ +internal fun periodicalsFailureKind( + failure: ApiResult.Failure, + goneConfirmed: Boolean, +): PeriodicalsFailure = when { + !isNotFoundFailure(failure) -> PeriodicalsFailure.Error + goneConfirmed -> PeriodicalsFailure.Gone + else -> PeriodicalsFailure.NotFound +} + +/** + * Build the [UiState.Error] for a classified detail failure. + * + * Gone and NotFound deliberately drop the server's message: `resolvedMessage()` prefers a + * non-blank message over the localized fallback, so keeping the bare "Not found." would show + * that instead of wording that tells the user which of the two situations they are in. + */ +internal fun periodicalsErrorState( + failure: ApiResult.Failure, + kind: PeriodicalsFailure, + @StringRes genericRes: Int, + @StringRes notFoundRes: Int, +): UiState.Error = when (kind) { + PeriodicalsFailure.Gone -> + UiState.Error("", failure.code, R.string.periodicals_gone_subtitle) + PeriodicalsFailure.NotFound -> + UiState.Error("", failure.code, notFoundRes) + PeriodicalsFailure.Error -> + UiState.Error(failure.message, failure.code, genericRes) +} diff --git a/app/src/test/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUiStateTest.kt b/app/src/test/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUiStateTest.kt index 5d57731..ff3865c 100644 --- a/app/src/test/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUiStateTest.kt +++ b/app/src/test/java/com/pinakes/app/ui/screens/periodicals/PeriodicalsUiStateTest.kt @@ -1,7 +1,12 @@ package com.pinakes.app.ui.screens.periodicals +import com.pinakes.app.R +import com.pinakes.app.data.model.Meta import com.pinakes.app.data.model.PeriodicalIssueDetail import com.pinakes.app.data.model.PeriodicalSummary +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.components.AvailabilityStatus import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -112,4 +117,112 @@ class PeriodicalsUiStateTest { assertFalse(PeriodicalIssueDetail(id = 1, pdfUrl = "").canOpenPdf) assertFalse(PeriodicalIssueDetail(id = 1, pdfUrl = " ").canOpenPdf) } + + // ---- Truncated issue list ---- + + @Test fun truncatedMetaRaisesTheBanner() { + assertTrue(isTruncatedList(Meta(truncated = true))) + } + + @Test fun explicitlyUntruncatedMetaDoesNotRaiseTheBanner() { + assertFalse(isTruncatedList(Meta(truncated = false))) + } + + @Test fun aServerThatOmitsTheFieldIsTreatedAsComplete() { + // Older servers have no `truncated` key at all: absent must never read as true, + // or every year would claim to be partial against an un-upgraded instance. + assertFalse(isTruncatedList(Meta(truncated = null))) + assertFalse(isTruncatedList(Meta(nextCursor = "c1"))) + assertFalse(isTruncatedList(null)) + } + + // ---- 404 classification: missing resource vs deactivated plugin ---- + + private fun failure(status: Int, code: String = ErrorCodes.NOT_FOUND, message: String = "Not found.") = + ApiResult.Failure(code = code, message = message, httpStatus = status) + + @Test fun a404IsRecognisedByStatusOrByCode() { + assertTrue(isNotFoundFailure(failure(404))) + // The envelope can carry the code with no HTTP status attached (apiCall maps a + // body-level error with httpStatus = 0). + assertTrue(isNotFoundFailure(failure(0, code = ErrorCodes.NOT_FOUND))) + } + + @Test fun otherFailuresAreNotWorthAHealthProbe() { + assertFalse(isNotFoundFailure(failure(500, code = ErrorCodes.SERVER_ERROR))) + assertFalse(isNotFoundFailure(failure(0, code = ErrorCodes.NETWORK))) + assertFalse(isNotFoundFailure(failure(403, code = ErrorCodes.FORBIDDEN))) + } + + @Test fun a404WithHealthAlsoGoneMeansThePluginIsOff() { + assertEquals( + PeriodicalsFailure.Gone, + periodicalsFailureKind(failure(404), goneConfirmed = true), + ) + } + + @Test fun a404WithHealthStillUpMeansOnlyThisResourceIsMissing() { + assertEquals( + PeriodicalsFailure.NotFound, + periodicalsFailureKind(failure(404), goneConfirmed = false), + ) + } + + @Test fun aNon404NeverDegradesToGoneEvenIfTheProbeSaysSo() { + // Guards the call site's short-circuit: confirmGone() must not be consulted for a + // network blip, and even a stale true must not hide a retryable error. + assertEquals( + PeriodicalsFailure.Error, + periodicalsFailureKind(failure(0, code = ErrorCodes.NETWORK), goneConfirmed = true), + ) + assertEquals( + PeriodicalsFailure.Error, + periodicalsFailureKind(failure(500, code = ErrorCodes.SERVER_ERROR), goneConfirmed = false), + ) + } + + // ---- Error state built from the classification ---- + + private fun errorState(kind: PeriodicalsFailure) = periodicalsErrorState( + failure = failure(404), + kind = kind, + genericRes = R.string.periodicals_issue_error, + notFoundRes = R.string.periodicals_issue_not_found, + ) + + @Test fun notFoundDropsTheServerMessageSoTheLocalizedWordingWins() { + val state = errorState(PeriodicalsFailure.NotFound) + + // resolvedMessage() prefers a non-blank message: leaving the server's bare + // "Not found." would show that instead of "This issue no longer exists." + assertEquals("", state.message) + assertEquals(R.string.periodicals_issue_not_found, state.messageRes) + } + + @Test fun goneUsesTheSectionWideWording() { + val state = errorState(PeriodicalsFailure.Gone) + + assertEquals("", state.message) + assertEquals(R.string.periodicals_gone_subtitle, state.messageRes) + } + + @Test fun aGenericFailureKeepsTheServerMessageAndTheScreenFallback() { + val state = periodicalsErrorState( + failure = ApiResult.Failure(ErrorCodes.SERVER_ERROR, "Upstream exploded", 500), + kind = PeriodicalsFailure.Error, + genericRes = R.string.periodicals_issue_error, + notFoundRes = R.string.periodicals_issue_not_found, + ) + + assertEquals("Upstream exploded", state.message) + assertEquals(R.string.periodicals_issue_error, state.messageRes) + } + + @Test fun everyClassifiedFailureCodeSurvivesIntoTheState() { + // The code is what auth-expiry checks and telemetry key off: it must never be lost. + PeriodicalsFailure.entries.forEach { kind -> + val state: UiState.Error = errorState(kind) + assertEquals(ErrorCodes.NOT_FOUND, state.code) + } + } } diff --git a/i18n/de.json b/i18n/de.json index e3398b5..ca1a860 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -449,10 +449,12 @@ "periodicals_issues_count": "%1$d Hefte", "periodicals_detail_loading": "Zeitschrift wird geladen…", "periodicals_detail_error": "Diese Zeitschrift konnte nicht geladen werden.", + "periodicals_detail_not_found": "Diese Zeitschrift existiert nicht mehr.", "periodicals_label_publisher": "Verlag", "periodicals_label_place": "Erscheinen", "periodicals_label_years": "Zeitraum", "periodicals_label_holdings": "Bestand", + "periodicals_label_supplements": "Beilagen", "periodicals_year_since": "Seit %1$d", "periodicals_years_section": "Jahrgänge", "periodicals_year_volume": "Bd. %1$s", @@ -461,13 +463,17 @@ "periodicals_issues_title": "Jahrgang %1$d", "periodicals_issues_loading": "Hefte werden geladen…", "periodicals_issues_error": "Die Hefte konnten nicht geladen werden.", + "periodicals_issues_not_found": "Dieser Jahrgang existiert nicht mehr.", "periodicals_issues_empty_title": "Keine Hefte", "periodicals_issues_empty_subtitle": "Für diesen Jahrgang sind noch keine Hefte erfasst.", + "periodicals_issues_truncated_title": "Unvollständige Liste", + "periodicals_issues_truncated_subtitle": "Es werden die ersten %1$d Hefte dieses Jahrgangs angezeigt.", "periodicals_issue_fallback": "Heft", "periodicals_issue_number": "Nr. %1$s", "periodicals_issue_pages": "%1$d Seiten", "periodicals_issue_loading": "Heft wird geladen…", "periodicals_issue_error": "Dieses Heft konnte nicht geladen werden.", + "periodicals_issue_not_found": "Dieses Heft existiert nicht mehr.", "periodicals_articles_section": "Inhalt", "periodicals_article_pages": "S. %1$d–%2$d", "periodicals_article_page": "S. %1$d", diff --git a/i18n/en.json b/i18n/en.json index 1254535..8253623 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -449,10 +449,12 @@ "periodicals_issues_count": "%1$d issues", "periodicals_detail_loading": "Loading periodical…", "periodicals_detail_error": "Couldn't load this periodical.", + "periodicals_detail_not_found": "This periodical no longer exists.", "periodicals_label_publisher": "Publisher", "periodicals_label_place": "Publication", "periodicals_label_years": "Coverage", "periodicals_label_holdings": "Holdings", + "periodicals_label_supplements": "Supplements", "periodicals_year_since": "Since %1$d", "periodicals_years_section": "Years", "periodicals_year_volume": "Vol. %1$s", @@ -461,13 +463,17 @@ "periodicals_issues_title": "Year %1$d", "periodicals_issues_loading": "Loading issues…", "periodicals_issues_error": "Couldn't load the issues.", + "periodicals_issues_not_found": "This year no longer exists.", "periodicals_issues_empty_title": "No issues", "periodicals_issues_empty_subtitle": "This year has no catalogued issues yet.", + "periodicals_issues_truncated_title": "Partial list", + "periodicals_issues_truncated_subtitle": "Showing the first %1$d issues of this year.", "periodicals_issue_fallback": "Issue", "periodicals_issue_number": "No. %1$s", "periodicals_issue_pages": "%1$d pages", "periodicals_issue_loading": "Loading issue…", "periodicals_issue_error": "Couldn't load this issue.", + "periodicals_issue_not_found": "This issue no longer exists.", "periodicals_articles_section": "Contents", "periodicals_article_pages": "pp. %1$d–%2$d", "periodicals_article_page": "p. %1$d", diff --git a/i18n/fr.json b/i18n/fr.json index 5163bae..a5169dd 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -449,10 +449,12 @@ "periodicals_issues_count": "%1$d fascicules", "periodicals_detail_loading": "Chargement du périodique…", "periodicals_detail_error": "Impossible de charger ce périodique.", + "periodicals_detail_not_found": "Ce périodique n'existe plus.", "periodicals_label_publisher": "Éditeur", "periodicals_label_place": "Publication", "periodicals_label_years": "Période", "periodicals_label_holdings": "État de collection", + "periodicals_label_supplements": "Suppléments", "periodicals_year_since": "Depuis %1$d", "periodicals_years_section": "Années", "periodicals_year_volume": "Vol. %1$s", @@ -461,13 +463,17 @@ "periodicals_issues_title": "Année %1$d", "periodicals_issues_loading": "Chargement des fascicules…", "periodicals_issues_error": "Impossible de charger les fascicules.", + "periodicals_issues_not_found": "Cette année n'existe plus.", "periodicals_issues_empty_title": "Aucun fascicule", "periodicals_issues_empty_subtitle": "Cette année n'a pas encore de fascicules catalogués.", + "periodicals_issues_truncated_title": "Liste partielle", + "periodicals_issues_truncated_subtitle": "Affichage des %1$d premiers fascicules de cette année.", "periodicals_issue_fallback": "Fascicule", "periodicals_issue_number": "N° %1$s", "periodicals_issue_pages": "%1$d pages", "periodicals_issue_loading": "Chargement du fascicule…", "periodicals_issue_error": "Impossible de charger ce fascicule.", + "periodicals_issue_not_found": "Ce fascicule n'existe plus.", "periodicals_articles_section": "Sommaire", "periodicals_article_pages": "pp. %1$d–%2$d", "periodicals_article_page": "p. %1$d", diff --git a/i18n/it.json b/i18n/it.json index 9c6e0ea..38ead9f 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -449,10 +449,12 @@ "periodicals_issues_count": "%1$d fascicoli", "periodicals_detail_loading": "Caricamento della testata…", "periodicals_detail_error": "Impossibile caricare questa testata.", + "periodicals_detail_not_found": "Questa testata non esiste più.", "periodicals_label_publisher": "Editore", "periodicals_label_place": "Pubblicazione", "periodicals_label_years": "Periodo", "periodicals_label_holdings": "Consistenza", + "periodicals_label_supplements": "Supplementi", "periodicals_year_since": "Dal %1$d", "periodicals_years_section": "Annate", "periodicals_year_volume": "Vol. %1$s", @@ -461,13 +463,17 @@ "periodicals_issues_title": "Annata %1$d", "periodicals_issues_loading": "Caricamento dei fascicoli…", "periodicals_issues_error": "Impossibile caricare i fascicoli.", + "periodicals_issues_not_found": "Questa annata non esiste più.", "periodicals_issues_empty_title": "Nessun fascicolo", "periodicals_issues_empty_subtitle": "Questa annata non ha ancora fascicoli catalogati.", + "periodicals_issues_truncated_title": "Elenco parziale", + "periodicals_issues_truncated_subtitle": "Mostrati i primi %1$d fascicoli di questa annata.", "periodicals_issue_fallback": "Fascicolo", "periodicals_issue_number": "N. %1$s", "periodicals_issue_pages": "%1$d pagine", "periodicals_issue_loading": "Caricamento del fascicolo…", "periodicals_issue_error": "Impossibile caricare questo fascicolo.", + "periodicals_issue_not_found": "Questo fascicolo non esiste più.", "periodicals_articles_section": "Spoglio", "periodicals_article_pages": "pp. %1$d–%2$d", "periodicals_article_page": "p. %1$d",