diff --git a/app/app/src/main/java/org/modg/bookshelf/AppContainer.kt b/app/app/src/main/java/org/modg/bookshelf/AppContainer.kt index 2df9d24..b8ea20a 100644 --- a/app/app/src/main/java/org/modg/bookshelf/AppContainer.kt +++ b/app/app/src/main/java/org/modg/bookshelf/AppContainer.kt @@ -1,6 +1,7 @@ package org.modg.bookshelf import android.content.Context +import java.util.concurrent.TimeUnit import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex @@ -86,11 +87,19 @@ class AppContainer(private val context: Context) { val authRepository by lazy { AuthRepository(apiProvider, settingsStore) } - // A bare client — deliberately NOT [okHttpClient] above, which carries our - // PocketBase bearer token via PbAuthInterceptor. Open Library/Google Books - // are third-party services; that token must never leave this device's - // requests to our own server. - private val metadataHttpClient: OkHttpClient by lazy { OkHttpClient() } + // Deliberately NOT [okHttpClient] above, which carries our PocketBase bearer + // token via PbAuthInterceptor. Open Library/Google Books are third-party + // services; that token must never leave this device's requests to our own + // server. A call timeout is load-bearing here: with none, a stalled + // connection hangs on OkHttp's default (unbounded) socket timeouts, and the + // user is standing at a bookshelf waiting on it. + private val metadataHttpClient: OkHttpClient by lazy { + OkHttpClient.Builder() + .callTimeout(12, TimeUnit.SECONDS) + .connectTimeout(10, TimeUnit.SECONDS) + .readTimeout(10, TimeUnit.SECONDS) + .build() + } val metadataRepository by lazy { MetadataRepository(metadataHttpClient, json) } diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksClient.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksClient.kt index 604cfa5..ff61fc3 100644 --- a/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksClient.kt +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksClient.kt @@ -1,6 +1,7 @@ package org.modg.bookshelf.data.metadata import java.io.IOException +import java.net.SocketTimeoutException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import kotlinx.serialization.SerializationException @@ -10,7 +11,8 @@ import okhttp3.Request /** * Google Books lookup — SPEC.md "Book metadata lookup" fallback source. No API key. - * Never throws: network/parse failures fail soft and return null. + * Never throws: every outcome, including transport failure, comes back as a + * [SourceResult] rather than a swallowed null. */ class GoogleBooksClient( private val httpClient: OkHttpClient, @@ -18,29 +20,42 @@ class GoogleBooksClient( ) { private val json = Json(from = json) { ignoreUnknownKeys = true } - suspend fun lookup(isbn13: String): BookMetadata? = withContext(Dispatchers.IO) { - val body = fetchBody(isbn13) ?: return@withContext null - parseResponse(body) - } + suspend fun lookup(isbn13: String): SourceResult = withContext(Dispatchers.IO) { fetch(isbn13) } - private fun fetchBody(isbn13: String): String? = try { + private fun fetch(isbn13: String): SourceResult = try { val request = Request.Builder() .url("https://www.googleapis.com/books/v1/volumes?q=isbn:$isbn13") .build() httpClient.newCall(request).execute().use { response -> - if (!response.isSuccessful) null else response.body?.string() + classify(response.code, response.body.string()) } + } catch (e: SocketTimeoutException) { + SourceResult.Failed("timeout") } catch (e: IOException) { - null + SourceResult.Failed("network error") + } + + /** + * Package-visible pure function — no socket involved — so it's exhaustively + * unit-testable offline (2xx-with-record, 2xx-without-record, 404, 429, 500, + * malformed body). [parseResponse] is defined in terms of this so the two + * can never disagree about what a body means. + */ + internal fun classify(httpCode: Int, body: String?): SourceResult { + if (httpCode !in 200..299) return SourceResult.Failed("http $httpCode") + if (body.isNullOrBlank()) return SourceResult.Failed("empty body") + return try { + val dto = json.decodeFromString(GoogleBooksResponseDto.serializer(), body) + val metadata = dto.items.firstOrNull()?.volumeInfo?.toBookMetadata() + if (metadata != null) SourceResult.Found(metadata) else SourceResult.NotFound + } catch (e: SerializationException) { + SourceResult.Failed("malformed json") + } catch (e: IllegalArgumentException) { + SourceResult.Failed("malformed json") + } } /** Package-visible for offline fixture tests — parses a raw response body with no network involved. */ - internal fun parseResponse(body: String): BookMetadata? = try { - val dto = json.decodeFromString(GoogleBooksResponseDto.serializer(), body) - dto.items.firstOrNull()?.volumeInfo?.toBookMetadata() - } catch (e: SerializationException) { - null - } catch (e: IllegalArgumentException) { - null - } + internal fun parseResponse(body: String): BookMetadata? = + (classify(200, body) as? SourceResult.Found)?.metadata } diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/LookupResult.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/LookupResult.kt new file mode 100644 index 0000000..acbb6ad --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/LookupResult.kt @@ -0,0 +1,18 @@ +package org.modg.bookshelf.data.metadata + +/** + * The combined outcome of a metadata lookup across both sources (SPEC.md "Book + * metadata lookup"). Produced by [MetadataRepository.lookup] from the two + * [SourceResult]s per [MetadataRepository.combine]'s rules: + * - any source [SourceResult.Found] -> [Found] + * - every source [SourceResult.NotFound] -> [NotFound] + * - otherwise (at least one [SourceResult.Failed], none Found) -> [Unavailable] + * + * The last rule is the whole point: one reachable source answering "no" is not + * authoritative while the other source couldn't be asked at all. + */ +sealed interface LookupResult { + data class Found(val metadata: BookMetadata) : LookupResult + data object NotFound : LookupResult + data class Unavailable(val reason: String) : LookupResult +} diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/MetadataRepository.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/MetadataRepository.kt index 800af60..82680cf 100644 --- a/app/app/src/main/java/org/modg/bookshelf/data/metadata/MetadataRepository.kt +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/MetadataRepository.kt @@ -7,9 +7,10 @@ import okhttp3.OkHttpClient /** * Single entry point for book metadata lookup (SPEC.md "Book metadata lookup"). - * Queries both sources concurrently and merges per [MetadataMerger]. Returns null - * if [isbn] doesn't checksum-validate or if both sources miss — callers (the scan - * screen) must then fall back to manual entry pre-filled with the scanned ISBN. + * Queries both sources concurrently and combines their [SourceResult]s into one + * [LookupResult] per [combine]. [isbn] must already be a checksum-valid ISBN-10/13 + * by the time it reaches here — the scan layer is responsible for that, and an + * invalid one is a programming error at this layer, not a lookup outcome. */ class MetadataRepository( private val openLibraryClient: OpenLibraryClient, @@ -20,21 +21,46 @@ class MetadataRepository( GoogleBooksClient(httpClient, json), ) - suspend fun lookup(isbn: String): BookMetadata? { - val isbn13 = IsbnUtils.toIsbn13(isbn) ?: return null - val merged = coroutineScope { + suspend fun lookup(isbn: String): LookupResult { + val isbn13 = checkNotNull(IsbnUtils.toIsbn13(isbn)) { + "MetadataRepository.lookup requires an already-validated ISBN-10/13; got: $isbn" + } + return coroutineScope { val openLibrary = async { openLibraryClient.lookup(isbn13) } val googleBooks = async { googleBooksClient.lookup(isbn13) } - MetadataMerger.merge(openLibrary.await(), googleBooks.await()) - } ?: return null - return if (merged.coverUrl.isNullOrBlank()) { - merged.copy(coverUrl = byIsbnCoverUrl(isbn13)) - } else { - merged + combine(openLibrary.await(), googleBooks.await(), isbn13) } } companion object { + /** + * Combines the two sources' outcomes per SPEC.md's three-way rule: any Found + * wins (merged via [MetadataMerger] and given the last-resort cover), all + * NotFound is an honest miss, and anything else — at least one Failed and + * nothing Found — is Unavailable, because a source that couldn't be reached + * must never be reported as a book that doesn't exist. Package-visible and + * pure (no I/O) so the full 3x3 matrix is unit-testable offline. + */ + internal fun combine(openLibrary: SourceResult, googleBooks: SourceResult, isbn13: String): LookupResult { + val merged = MetadataMerger.merge( + (openLibrary as? SourceResult.Found)?.metadata, + (googleBooks as? SourceResult.Found)?.metadata, + ) + if (merged != null) { + val withCover = if (merged.coverUrl.isNullOrBlank()) { + merged.copy(coverUrl = byIsbnCoverUrl(isbn13)) + } else { + merged + } + return LookupResult.Found(withCover) + } + val failures = listOfNotNull( + (openLibrary as? SourceResult.Failed)?.reason?.let { "open library: $it" }, + (googleBooks as? SourceResult.Failed)?.reason?.let { "google books: $it" }, + ) + return if (failures.isNotEmpty()) LookupResult.Unavailable(failures.joinToString("; ")) else LookupResult.NotFound + } + /** * Last resort when neither source reported cover art, per SPEC's cover chain. * `default=false` is load-bearing: without it this endpoint answers 200 with a diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibraryClient.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibraryClient.kt index 5383487..fefe696 100644 --- a/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibraryClient.kt +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibraryClient.kt @@ -1,6 +1,7 @@ package org.modg.bookshelf.data.metadata import java.io.IOException +import java.net.SocketTimeoutException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import kotlinx.serialization.SerializationException @@ -12,7 +13,8 @@ import okhttp3.Request /** * Open Library lookup — SPEC.md "Book metadata lookup" primary source. - * Never throws: network/parse failures fail soft and return null. + * Never throws: every outcome, including transport failure, comes back as a + * [SourceResult] rather than a swallowed null. */ class OpenLibraryClient( private val httpClient: OkHttpClient, @@ -21,30 +23,42 @@ class OpenLibraryClient( // Real responses carry fields this DTO doesn't model; never let an unknown key throw. private val json = Json(from = json) { ignoreUnknownKeys = true } - suspend fun lookup(isbn13: String): BookMetadata? = withContext(Dispatchers.IO) { - val body = fetchBody(isbn13) ?: return@withContext null - parseResponse(body, isbn13) - } + suspend fun lookup(isbn13: String): SourceResult = withContext(Dispatchers.IO) { fetch(isbn13) } - private fun fetchBody(isbn13: String): String? = try { + private fun fetch(isbn13: String): SourceResult = try { val request = Request.Builder() .url("https://openlibrary.org/api/books?bibkeys=ISBN:$isbn13&format=json&jscmd=data") .build() httpClient.newCall(request).execute().use { response -> - if (!response.isSuccessful) null else response.body?.string() + classify(response.code, response.body.string(), isbn13) } + } catch (e: SocketTimeoutException) { + SourceResult.Failed("timeout") } catch (e: IOException) { - null + SourceResult.Failed("network error") + } + + /** + * Package-visible pure function — no socket involved — so it's exhaustively + * unit-testable offline (2xx-with-record, 2xx-without-record, 404, 429, 500, + * malformed body). [parseResponse] is defined in terms of this so the two + * can never disagree about what a body means. + */ + internal fun classify(httpCode: Int, body: String?, isbn13: String): SourceResult { + if (httpCode !in 200..299) return SourceResult.Failed("http $httpCode") + if (body.isNullOrBlank()) return SourceResult.Failed("empty body") + return try { + val root = json.parseToJsonElement(body).jsonObject + val entry = root["ISBN:$isbn13"]?.jsonObject ?: return SourceResult.NotFound + SourceResult.Found(json.decodeFromJsonElement(entry).toBookMetadata(isbn13)) + } catch (e: SerializationException) { + SourceResult.Failed("malformed json") + } catch (e: IllegalArgumentException) { + SourceResult.Failed("malformed json") + } } /** Package-visible for offline fixture tests — parses a raw response body with no network involved. */ - internal fun parseResponse(body: String, isbn13: String): BookMetadata? = try { - val root = json.parseToJsonElement(body).jsonObject - val entry = root["ISBN:$isbn13"]?.jsonObject ?: return null - json.decodeFromJsonElement(entry).toBookMetadata(isbn13) - } catch (e: SerializationException) { - null - } catch (e: IllegalArgumentException) { - null - } + internal fun parseResponse(body: String, isbn13: String): BookMetadata? = + (classify(200, body, isbn13) as? SourceResult.Found)?.metadata } diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/SourceResult.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SourceResult.kt new file mode 100644 index 0000000..bd4ed25 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SourceResult.kt @@ -0,0 +1,23 @@ +package org.modg.bookshelf.data.metadata + +/** + * Per-source lookup outcome (SPEC.md "Book metadata lookup": "Lookup outcome is + * THREE-WAY, never a bare null"). [OpenLibraryClient] and [GoogleBooksClient] each + * produce one of these instead of collapsing "couldn't be reached" and "answered, + * doesn't have it" into the same `null`. [MetadataRepository] combines the two + * source results into a [LookupResult]. + */ +sealed interface SourceResult { + data class Found(val metadata: BookMetadata) : SourceResult + + /** The source answered (2xx) and, in good faith, has no record for this ISBN. */ + data object NotFound : SourceResult + + /** + * The source could not be asked, or its answer couldn't be trusted: non-2xx, + * timeout, transport error, or a body that didn't parse. [reason] is a short + * diagnostic ("http 429", "timeout", "malformed json") for logs — never shown + * to the user verbatim. + */ + data class Failed(val reason: String) : SourceResult +} diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanCodeFilter.kt b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanCodeFilter.kt index 58ebb8e..9f00253 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanCodeFilter.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanCodeFilter.kt @@ -2,11 +2,31 @@ package org.modg.bookshelf.ui.scan import org.modg.bookshelf.data.metadata.IsbnUtils +/** What a single decoded barcode read means, once debouncing has been applied. */ +sealed interface ScanOutcome { + /** A checksum-valid ISBN-13, ready for metadata lookup. */ + data class Isbn(val isbn13: String) : ScanOutcome + + /** + * Decoded fine, but its checksum rules it out as a book ISBN (SPEC.md + * "Barcode scanning": "ignore non-book barcodes" — but not silently, see + * [rawValue]). Carries the raw value so the camera screen can echo back what + * it read. + */ + data class NotAnIsbn(val rawValue: String) : ScanOutcome + + /** A debounced repeat of the last code (valid or not) — emit no UI at all. */ + data object Ignored : ScanOutcome +} + /** - * Pure decode-to-ISBN pipeline shared by the analyzer and its tests: normalizes a raw - * barcode value, validates the ISBN-13 checksum (this is what makes EAN_8/UPC_A reads - * fall out as "non-book barcodes" per SPEC.md "Barcode scanning" — they can never be - * 13 digits), and debounces repeat reads of the same code. + * Pure decode-to-outcome pipeline shared by the analyzer and its tests: normalizes + * a raw barcode value, classifies it as a valid ISBN-13 or not (this is what makes + * EAN_8/UPC_A reads fall out as "non-book barcodes" per SPEC.md "Barcode scanning" + * — they can never be 13 digits), and debounces repeats of the same code, whether + * or not it validated. The debounce is what keeps a non-book barcode sitting in + * frame — which decodes on nearly every analyzed frame — from flickering a + * rejection message instead of showing it once. */ class ScanCodeFilter( private val debounceMillis: Long = 2000L, @@ -15,15 +35,19 @@ class ScanCodeFilter( private var lastCode: String? = null private var lastEmitMillis: Long = Long.MIN_VALUE - /** Returns the normalized ISBN-13 if [rawValue] is a valid, non-debounced hit; null otherwise. */ - fun accept(rawValue: String?): String? { - val normalized = IsbnUtils.normalize(rawValue ?: return null) - if (!IsbnUtils.isValidIsbn13(normalized)) return null + /** Classifies [rawValue] per [ScanOutcome], applying the debounce window. */ + fun accept(rawValue: String?): ScanOutcome { + val raw = rawValue ?: return ScanOutcome.Ignored + val normalized = IsbnUtils.normalize(raw) val now = nowMillis() - if (normalized == lastCode && now - lastEmitMillis < debounceMillis) return null + if (normalized == lastCode && now - lastEmitMillis < debounceMillis) return ScanOutcome.Ignored lastCode = normalized lastEmitMillis = now - return normalized + return if (IsbnUtils.isValidIsbn13(normalized)) { + ScanOutcome.Isbn(normalized) + } else { + ScanOutcome.NotAnIsbn(raw) + } } /** Allows the next scan of any code (including a repeat) to emit immediately, e.g. after Skip. */ diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanModels.kt b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanModels.kt index 8d9325e..f6ce03d 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanModels.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanModels.kt @@ -2,6 +2,7 @@ package org.modg.bookshelf.ui.scan import org.modg.bookshelf.data.local.BookEntity import org.modg.bookshelf.data.metadata.BookMetadata +import org.modg.bookshelf.data.metadata.LookupResult /** SPEC.md scan screen: "Duplicate-ISBN warning if already owned." */ sealed interface DuplicateStatus { @@ -27,14 +28,30 @@ sealed interface ScanSheetState { */ data class Loading(val isbn13: String) : ScanSheetState data class Found(val isbn13: String, val metadata: BookMetadata, val duplicate: DuplicateStatus) : ScanSheetState - data class NotFound(val isbn13: String) : ScanSheetState + + /** + * Every source answered and none had the book (SPEC: an honest "no", not a + * cover story for a failed request). [viaLookupFailure] is true only when the + * user reached this form through [LookupFailed]'s "Enter by hand" escape hatch + * rather than a genuine miss, so the sheet's copy can stop short of claiming + * the book is unknown. + */ + data class NotFound(val isbn13: String, val viaLookupFailure: Boolean = false) : ScanSheetState + + /** + * At least one source couldn't be reached (non-2xx, timeout, transport error) + * and neither had the book — SPEC: "must NOT claim the book is unknown". Offers + * retry, manual entry, and skip instead of a manual-entry form captioned as a miss. + */ + data class LookupFailed(val isbn13: String, val reason: String) : ScanSheetState } /** Combines a metadata lookup result with duplicate status into the sheet state to show. */ object ScanMetadataOutcome { - fun from(isbn13: String, metadata: BookMetadata?, duplicate: DuplicateStatus): ScanSheetState = when (metadata) { - null -> ScanSheetState.NotFound(isbn13) - else -> ScanSheetState.Found(isbn13, metadata, duplicate) + fun from(isbn13: String, result: LookupResult, duplicate: DuplicateStatus): ScanSheetState = when (result) { + is LookupResult.Found -> ScanSheetState.Found(isbn13, result.metadata, duplicate) + is LookupResult.NotFound -> ScanSheetState.NotFound(isbn13) + is LookupResult.Unavailable -> ScanSheetState.LookupFailed(isbn13, result.reason) } } diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanScreen.kt b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanScreen.kt index 839c7e2..bcb8dd3 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanScreen.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanScreen.kt @@ -18,6 +18,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.outlined.ArrowBack import androidx.compose.material.icons.outlined.FlashOff @@ -44,6 +45,7 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.platform.LocalLifecycleOwner import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView @@ -58,6 +60,7 @@ import org.modg.bookshelf.AppContainer import org.modg.bookshelf.data.local.BookcaseEntity import org.modg.bookshelf.data.local.ShelfEntity import org.modg.bookshelf.data.metadata.BookMetadata +import org.modg.bookshelf.data.metadata.IsbnUtils import org.modg.bookshelf.ui.components.BookCover import org.modg.bookshelf.ui.components.BookshelfScaffold import org.modg.bookshelf.ui.components.EmptyState @@ -89,6 +92,7 @@ fun ScanScreen( val sheetState by viewModel.sheetState.collectAsState() val sessionState by viewModel.sessionState.collectAsState() + val rejectedMessage by viewModel.rejectedMessage.collectAsState() val torchEnabled by viewModel.scannerController.torchEnabled.collectAsState() val bookcases by viewModel.bookcases.collectAsState() val shelves by viewModel.shelves.collectAsState() @@ -128,6 +132,12 @@ fun ScanScreen( CameraPreview(controller = viewModel.scannerController, torchEnabled = torchEnabled) ScanReticle(modifier = Modifier.align(Alignment.Center)) SessionBadge(count = sessionState.savedCount, modifier = Modifier.align(Alignment.TopCenter).padding(16.dp)) + rejectedMessage?.let { message -> + RejectedBarcodeBanner( + message = message, + modifier = Modifier.align(Alignment.BottomCenter).padding(24.dp), + ) + } } else { PermissionDeniedContent(onRequestAgain = { permissionState.launchPermissionRequest() }) } @@ -160,6 +170,7 @@ fun ScanScreen( ) { ManualEntrySheet( isbn13 = state.isbn13, + authoritative = !state.viaLookupFailure, bookcases = bookcases, shelves = shelves, selectedShelfId = selectedShelfId, @@ -168,6 +179,17 @@ fun ScanScreen( onSkip = { viewModel.skip() }, ) } + is ScanSheetState.LookupFailed -> ModalBottomSheet( + onDismissRequest = { viewModel.dismissSheet() }, + sheetState = rememberModalBottomSheetState(), + ) { + LookupFailedSheet( + isbn13 = state.isbn13, + onRetry = { viewModel.retryLookup(state.isbn13) }, + onEnterByHand = { viewModel.enterByHand(state.isbn13) }, + onSkip = { viewModel.skip() }, + ) + } } if (showManualEntry) { @@ -340,9 +362,16 @@ internal fun FoundBookSheet( } } +/** + * Shown when every source answered and none had the book — SPEC: word this as the + * authoritative negative it is, not as a generic failure. [authoritative] is false + * only when this form was reached via [LookupFailedSheet]'s "Enter by hand", where + * the lookup never completed and the copy must not imply the book is unknown. + */ @Composable internal fun ManualEntrySheet( isbn13: String, + authoritative: Boolean, bookcases: List, shelves: List, selectedShelfId: String?, @@ -354,9 +383,12 @@ internal fun ManualEntrySheet( var authors by remember { mutableStateOf("") } Column(modifier = Modifier.fillMaxWidth().padding(16.dp)) { - Text(text = "No match found", style = MaterialTheme.typography.titleLarge) Text( - text = "ISBN $isbn13 — enter the details by hand.", + text = if (authoritative) "Not in Open Library or Google Books" else "Enter the details by hand", + style = MaterialTheme.typography.titleLarge, + ) + Text( + text = if (authoritative) "ISBN $isbn13 — enter the details by hand." else "ISBN $isbn13", style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.padding(bottom = 12.dp), @@ -393,6 +425,54 @@ internal fun ManualEntrySheet( } } +/** + * Shown when a lookup couldn't be completed — SPEC: "a retry affordance, with + * manual entry as the escape hatch; it must NOT claim the book is unknown." No + * shelf picker here: saving isn't offered from this sheet, only a path onward to + * one that does (Retry, or the manual-entry form via "Enter by hand"). + */ +@Composable +internal fun LookupFailedSheet( + isbn13: String, + onRetry: () -> Unit, + onEnterByHand: () -> Unit, + onSkip: () -> Unit, +) { + Column(modifier = Modifier.fillMaxWidth().padding(16.dp)) { + Text(text = "Couldn't complete the lookup", style = MaterialTheme.typography.titleLarge) + Text( + text = "ISBN $isbn13 — one or more sources couldn't be reached. " + + "This doesn't mean the book is unknown.", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 8.dp, bottom = 16.dp), + ) + PrimaryButton(text = "Retry", onClick = onRetry, modifier = Modifier.fillMaxWidth()) + Row(modifier = Modifier.fillMaxWidth().padding(top = 12.dp), horizontalArrangement = Arrangement.spacedBy(12.dp)) { + SecondaryButton(text = "Enter by hand", onClick = onEnterByHand, modifier = Modifier.weight(1f)) + SecondaryButton(text = "Skip", onClick = onSkip, modifier = Modifier.weight(1f)) + } + } +} + +/** + * Transient banner for a decoded-but-rejected barcode (SPEC.md "Barcode scanning": + * "A rejected barcode is NOT silent"). [ScanViewModel] owns the debounce (via + * [ScanCodeFilter]) and the auto-clear timer behind [message] — this composable + * just renders whatever it's handed. + */ +@Composable +internal fun RejectedBarcodeBanner(message: String, modifier: Modifier = Modifier) { + Text( + text = message, + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.onErrorContainer, + modifier = modifier + .background(MaterialTheme.colorScheme.errorContainer, RoundedCornerShape(20.dp)) + .padding(horizontal = 16.dp, vertical = 8.dp), + ) +} + @Composable internal fun ShelfPicker( bookcases: List, @@ -431,19 +511,37 @@ internal fun ShelfPicker( @Composable private fun ManualIsbnDialog(onDismiss: () -> Unit, onSubmit: (String) -> Unit) { var text by remember { mutableStateOf("") } + // Validate here rather than letting ScanViewModel.manualIsbnEntered drop an + // unparseable ISBN on the floor. A typed-in check digit is easy to get wrong, + // and a dialog whose button does nothing is the same silent failure this whole + // screen was just rebuilt to eliminate. + val isbn13 = remember(text) { IsbnUtils.toIsbn13(text) } + val malformed = text.isNotBlank() && isbn13 == null AlertDialog( onDismissRequest = onDismiss, title = { Text("Enter ISBN") }, text = { - OutlinedTextField( - value = text, - onValueChange = { text = it }, - label = { Text("ISBN-10 or ISBN-13") }, - singleLine = true, - ) + Column { + OutlinedTextField( + value = text, + onValueChange = { text = it }, + label = { Text("ISBN-10 or ISBN-13") }, + singleLine = true, + isError = malformed, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), + ) + if (malformed) { + Text( + text = "That isn't a valid ISBN — check the digits.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.padding(top = 4.dp), + ) + } + } }, confirmButton = { - PrimaryButton(text = "Look up", onClick = { onSubmit(text) }, enabled = text.isNotBlank()) + PrimaryButton(text = "Look up", onClick = { onSubmit(text) }, enabled = isbn13 != null) }, dismissButton = { SecondaryButton(text = "Cancel", onClick = onDismiss) diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanViewModel.kt b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanViewModel.kt index b7b834e..7bd7194 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanViewModel.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScanViewModel.kt @@ -2,6 +2,8 @@ package org.modg.bookshelf.ui.scan import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -35,6 +37,11 @@ class ScanViewModel( private val _sessionState = MutableStateFlow(ScanSessionState()) val sessionState: StateFlow = _sessionState.asStateFlow() + private val _rejectedMessage = MutableStateFlow(null) + /** Transient "read but not a book barcode" message for the camera overlay; auto-clears. */ + val rejectedMessage: StateFlow = _rejectedMessage.asStateFlow() + private var rejectedMessageClearJob: Job? = null + val bookcases: StateFlow> = locationRepository.observeBookcases() .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) val shelves: StateFlow> = locationRepository.observeShelves() @@ -48,14 +55,41 @@ class ScanViewModel( viewModelScope.launch { scannerController.scanResults.collect { isbn13 -> onScanned(isbn13) } } + viewModelScope.launch { + scannerController.rejectedCodes.collect { rawValue -> showRejectedMessage(rawValue) } + } } private suspend fun onScanned(isbn13: String) { if (_sheetState.value !is ScanSheetState.Hidden) return // a sheet is already up for a previous hit + runLookup(isbn13) + } + + private suspend fun runLookup(isbn13: String) { _sheetState.value = ScanSheetState.Loading(isbn13) val duplicate = DuplicateCheck.check(bookRepository.findByIsbn13(isbn13)) - val metadata = metadataRepository.lookup(isbn13) - _sheetState.value = ScanMetadataOutcome.from(isbn13, metadata, duplicate) + val result = metadataRepository.lookup(isbn13) + _sheetState.value = ScanMetadataOutcome.from(isbn13, result, duplicate) + } + + /** [ScanSheetState.LookupFailed]'s Retry — re-enters loading and re-runs the same lookup. */ + fun retryLookup(isbn13: String) { + viewModelScope.launch { runLookup(isbn13) } + } + + /** [ScanSheetState.LookupFailed]'s "Enter by hand" — falls through to the manual-entry form. */ + fun enterByHand(isbn13: String) { + _sheetState.value = ScanSheetState.NotFound(isbn13, viaLookupFailure = true) + } + + /** Throttled per [ScanCodeFilter]'s debounce; this just owns the auto-clear timer on top. */ + private fun showRejectedMessage(rawValue: String) { + rejectedMessageClearJob?.cancel() + _rejectedMessage.value = "Read $rawValue — not a book barcode" + rejectedMessageClearJob = viewModelScope.launch { + delay(REJECTED_MESSAGE_MILLIS) + _rejectedMessage.value = null + } } /** The manual-ISBN-entry escape hatch (SPEC: for when a barcode won't scan). */ @@ -114,4 +148,9 @@ class ScanViewModel( _sheetState.value = ScanSheetState.Hidden scannerController.resetDebounce() } + + private companion object { + /** How long a rejected-barcode message stays on screen before it auto-clears. */ + const val REJECTED_MESSAGE_MILLIS = 3000L + } } diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScannerController.kt b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScannerController.kt index 703bde6..911c400 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScannerController.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/scan/ScannerController.kt @@ -19,6 +19,14 @@ class ScannerController( /** Emits a debounced, checksum-valid ISBN-13 each time [IsbnBarcodeAnalyzer] sees a new book barcode. */ val scanResults: SharedFlow = _scanResults.asSharedFlow() + private val _rejectedCodes = MutableSharedFlow(extraBufferCapacity = 1) + /** + * Emits the raw value of a debounced, non-book barcode read (SPEC.md "Barcode + * scanning": "A rejected barcode is NOT silent"). [ScanOutcome.Ignored] reads + * (debounced repeats) never reach here. + */ + val rejectedCodes: SharedFlow = _rejectedCodes.asSharedFlow() + private val _torchEnabled = MutableStateFlow(false) val torchEnabled: StateFlow = _torchEnabled.asStateFlow() @@ -27,7 +35,11 @@ class ScannerController( /** Called by [IsbnBarcodeAnalyzer] for every decoded barcode's raw value. */ fun onBarcodeScanned(rawValue: String?) { - codeFilter.accept(rawValue)?.let { _scanResults.tryEmit(it) } + when (val outcome = codeFilter.accept(rawValue)) { + is ScanOutcome.Isbn -> _scanResults.tryEmit(outcome.isbn13) + is ScanOutcome.NotAnIsbn -> _rejectedCodes.tryEmit(outcome.rawValue) + ScanOutcome.Ignored -> Unit + } } fun toggleTorch() { diff --git a/app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksClientTest.kt b/app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksClientTest.kt index 237d75c..26611a3 100644 --- a/app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksClientTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksClientTest.kt @@ -46,6 +46,47 @@ class GoogleBooksClientTest { assertNull(client.parseResponse(body)) } + // --- classify(): the three-way per-source outcome (SPEC.md "Book metadata lookup"). --- + + @Test + fun `classify reports Found for a 2xx response with a record`() { + val result = client.classify(200, fixture("googlebooks_success.json")) + + val found = result as? SourceResult.Found + checkNotNull(found) { "expected Found, got $result" } + assertEquals("Effective Java", found.metadata.title) + } + + @Test + fun `classify reports NotFound for a 2xx response with no items`() { + val result = client.classify(200, fixture("googlebooks_no_items.json")) + assertEquals(SourceResult.NotFound, result) + } + + @Test + fun `classify reports Failed for a 404`() { + val result = client.classify(404, null) + assertEquals(SourceResult.Failed("http 404"), result) + } + + @Test + fun `classify reports Failed for a 429 keyless-quota response, distinctly from NotFound`() { + val result = client.classify(429, null) + assertEquals(SourceResult.Failed("http 429"), result) + } + + @Test + fun `classify reports Failed for a 500`() { + val result = client.classify(500, "Internal Server Error") + assertEquals(SourceResult.Failed("http 500"), result) + } + + @Test + fun `classify reports Failed for a malformed body even on a 2xx status`() { + val result = client.classify(200, fixture("malformed.json")) + assertEquals(SourceResult.Failed("malformed json"), result) + } + private fun fixture(name: String): String = checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" } .bufferedReader() diff --git a/app/app/src/test/java/org/modg/bookshelf/data/metadata/MetadataRepositoryTest.kt b/app/app/src/test/java/org/modg/bookshelf/data/metadata/MetadataRepositoryTest.kt index d147cc7..1555700 100644 --- a/app/app/src/test/java/org/modg/bookshelf/data/metadata/MetadataRepositoryTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/data/metadata/MetadataRepositoryTest.kt @@ -6,6 +6,80 @@ import org.junit.Test class MetadataRepositoryTest { + private val isbn13 = "9780201558029" + private val openLibraryHit = SourceResult.Found(BookMetadata(title = "Open Library Title", isbn13 = isbn13)) + private val googleBooksHit = SourceResult.Found(BookMetadata(title = "Google Books Title", isbn13 = isbn13)) + + // --- The full 3x3 (Open Library outcome x Google Books outcome) combination matrix + // from SPEC.md "Book metadata lookup": any Found wins, all-NotFound is an honest + // miss, anything else with a Failed in it is Unavailable — never a false negative. --- + + @Test + fun `Found + Found merges and returns Found`() { + val result = MetadataRepository.combine(openLibraryHit, googleBooksHit, isbn13) + assertTrue(result is LookupResult.Found) + } + + @Test + fun `Found + NotFound returns Found from the one source that had it`() { + val result = MetadataRepository.combine(openLibraryHit, SourceResult.NotFound, isbn13) + assertEquals("Open Library Title", (result as LookupResult.Found).metadata.title) + } + + @Test + fun `Found + Failed returns Found -- a reachable hit is not overruled by the other failing`() { + val result = MetadataRepository.combine(openLibraryHit, SourceResult.Failed("http 429"), isbn13) + assertEquals("Open Library Title", (result as LookupResult.Found).metadata.title) + } + + @Test + fun `NotFound + Found returns Found from the one source that had it`() { + val result = MetadataRepository.combine(SourceResult.NotFound, googleBooksHit, isbn13) + assertEquals("Google Books Title", (result as LookupResult.Found).metadata.title) + } + + @Test + fun `NotFound + NotFound returns NotFound -- both sources answered and neither had it`() { + val result = MetadataRepository.combine(SourceResult.NotFound, SourceResult.NotFound, isbn13) + assertEquals(LookupResult.NotFound, result) + } + + @Test + fun `NotFound + Failed returns Unavailable -- one honest miss is not authoritative alone`() { + val result = MetadataRepository.combine(SourceResult.NotFound, SourceResult.Failed("timeout"), isbn13) + assertTrue(result is LookupResult.Unavailable) + } + + @Test + fun `Failed + Found returns Found from the one source that had it`() { + val result = MetadataRepository.combine(SourceResult.Failed("http 500"), googleBooksHit, isbn13) + assertEquals("Google Books Title", (result as LookupResult.Found).metadata.title) + } + + @Test + fun `Failed + NotFound returns Unavailable -- one honest miss is not authoritative alone`() { + val result = MetadataRepository.combine(SourceResult.Failed("timeout"), SourceResult.NotFound, isbn13) + assertTrue(result is LookupResult.Unavailable) + } + + @Test + fun `Failed + Failed returns Unavailable and preserves both reasons`() { + val result = MetadataRepository.combine(SourceResult.Failed("http 429"), SourceResult.Failed("timeout"), isbn13) + + val unavailable = result as? LookupResult.Unavailable + checkNotNull(unavailable) { "expected Unavailable, got $result" } + assertTrue(unavailable.reason.contains("http 429")) + assertTrue(unavailable.reason.contains("timeout")) + } + + @Test + fun `a Found result without cover art falls back to the by-isbn cover url`() { + val bare = SourceResult.Found(BookMetadata(title = "No Cover", isbn13 = isbn13)) + val result = MetadataRepository.combine(bare, SourceResult.NotFound, isbn13) + + assertEquals(MetadataRepository.byIsbnCoverUrl(isbn13), (result as LookupResult.Found).metadata.coverUrl) + } + /** * `default=false` is the whole point of this URL. Without it covers.openlibrary.org * answers 200 with a 43-byte 1x1 transparent GIF for an edition it holds no art for diff --git a/app/app/src/test/java/org/modg/bookshelf/data/metadata/OpenLibraryClientTest.kt b/app/app/src/test/java/org/modg/bookshelf/data/metadata/OpenLibraryClientTest.kt index 8251309..cc73301 100644 --- a/app/app/src/test/java/org/modg/bookshelf/data/metadata/OpenLibraryClientTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/data/metadata/OpenLibraryClientTest.kt @@ -105,6 +105,47 @@ class OpenLibraryClientTest { assertNull(client.parseResponse(body, "9780201558029")) } + // --- classify(): the three-way per-source outcome (SPEC.md "Book metadata lookup"). --- + + @Test + fun `classify reports Found for a 2xx response with a record`() { + val result = client.classify(200, fixture("openlibrary_success.json"), "9780201558029") + + val found = result as? SourceResult.Found + checkNotNull(found) { "expected Found, got $result" } + assertEquals("Concrete Mathematics", found.metadata.title) + } + + @Test + fun `classify reports NotFound for a 2xx response with no record for that isbn`() { + val result = client.classify(200, fixture("openlibrary_not_found.json"), "9780201558029") + assertEquals(SourceResult.NotFound, result) + } + + @Test + fun `classify reports Failed for a 404`() { + val result = client.classify(404, null, "9780201558029") + assertEquals(SourceResult.Failed("http 404"), result) + } + + @Test + fun `classify reports Failed for a 429, distinctly from NotFound`() { + val result = client.classify(429, null, "9780201558029") + assertEquals(SourceResult.Failed("http 429"), result) + } + + @Test + fun `classify reports Failed for a 500`() { + val result = client.classify(500, "Internal Server Error", "9780201558029") + assertEquals(SourceResult.Failed("http 500"), result) + } + + @Test + fun `classify reports Failed for a malformed body even on a 2xx status`() { + val result = client.classify(200, fixture("malformed.json"), "9780201558029") + assertEquals(SourceResult.Failed("malformed json"), result) + } + private fun fixture(name: String): String = checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" } .bufferedReader() diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanCodeFilterTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanCodeFilterTest.kt index 8def9dd..2e7c090 100644 --- a/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanCodeFilterTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanCodeFilterTest.kt @@ -1,7 +1,6 @@ package org.modg.bookshelf.ui.scan import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull import org.junit.Test class ScanCodeFilterTest { @@ -9,45 +8,45 @@ class ScanCodeFilterTest { @Test fun `accepts a checksum-valid isbn13`() { val filter = ScanCodeFilter() - assertEquals("9780201558029", filter.accept("9780201558029")) + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) } @Test - fun `rejects a checksum-invalid isbn13-shaped code`() { + fun `rejects a checksum-invalid isbn13-shaped code as not-an-isbn`() { val filter = ScanCodeFilter() - assertNull(filter.accept("9780201558020")) + assertEquals(ScanOutcome.NotAnIsbn("9780201558020"), filter.accept("9780201558020")) } @Test - fun `rejects non-book barcode lengths such as EAN-8 or UPC-A`() { + fun `rejects non-book barcode lengths such as EAN-8 or UPC-A as not-an-isbn`() { val filter = ScanCodeFilter() - assertNull(filter.accept("12345670")) // EAN-8 shaped - assertNull(filter.accept("012345678905")) // UPC-A shaped, 12 digits + assertEquals(ScanOutcome.NotAnIsbn("12345670"), filter.accept("12345670")) // EAN-8 shaped + assertEquals(ScanOutcome.NotAnIsbn("012345678905"), filter.accept("012345678905")) // UPC-A shaped, 12 digits } @Test - fun `rejects null raw value`() { - assertNull(ScanCodeFilter().accept(null)) + fun `ignores null raw value`() { + assertEquals(ScanOutcome.Ignored, ScanCodeFilter().accept(null)) } @Test - fun `debounces a repeat of the same code within the window`() { + fun `debounces a repeat of the same valid code within the window`() { var now = 0L val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) - assertEquals("9780201558029", filter.accept("9780201558029")) + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) now += 500 - assertNull(filter.accept("9780201558029")) + assertEquals(ScanOutcome.Ignored, filter.accept("9780201558029")) } @Test - fun `re-emits the same code once the debounce window elapses`() { + fun `re-emits the same valid code once the debounce window elapses`() { var now = 0L val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) - assertEquals("9780201558029", filter.accept("9780201558029")) + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) now += 2001 - assertEquals("9780201558029", filter.accept("9780201558029")) + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) } @Test @@ -55,9 +54,9 @@ class ScanCodeFilterTest { var now = 0L val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) - assertEquals("9780201558029", filter.accept("9780201558029")) + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) now += 10 - assertEquals("9780134685991", filter.accept("9780134685991")) + assertEquals(ScanOutcome.Isbn("9780134685991"), filter.accept("9780134685991")) } @Test @@ -65,8 +64,56 @@ class ScanCodeFilterTest { var now = 0L val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) - assertEquals("9780201558029", filter.accept("9780201558029")) + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) filter.reset() - assertEquals("9780201558029", filter.accept("9780201558029")) + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) + } + + // --- Rejected-code throttle (SPEC.md "Barcode scanning": a non-book barcode sitting + // in frame decodes on almost every analyzed frame, so this must not flicker). --- + + @Test + fun `a rejected code emits once, then is throttled on every subsequent frame within the window`() { + var now = 0L + val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) + + assertEquals(ScanOutcome.NotAnIsbn("12345670"), filter.accept("12345670")) + // Simulate several more analyzed frames, all still within the debounce window. + now += 50 + assertEquals(ScanOutcome.Ignored, filter.accept("12345670")) + now += 50 + assertEquals(ScanOutcome.Ignored, filter.accept("12345670")) + now += 1000 + assertEquals(ScanOutcome.Ignored, filter.accept("12345670")) + } + + @Test + fun `a rejected code re-emits once the debounce window elapses`() { + var now = 0L + val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) + + assertEquals(ScanOutcome.NotAnIsbn("12345670"), filter.accept("12345670")) + now += 2001 + assertEquals(ScanOutcome.NotAnIsbn("12345670"), filter.accept("12345670")) + } + + @Test + fun `a rejected code does not suppress a genuinely different valid isbn read right after it`() { + var now = 0L + val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) + + assertEquals(ScanOutcome.NotAnIsbn("12345670"), filter.accept("12345670")) + now += 10 + assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029")) + } + + @Test + fun `switching between two different rejected codes does not throttle either`() { + var now = 0L + val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) + + assertEquals(ScanOutcome.NotAnIsbn("12345670"), filter.accept("12345670")) + now += 10 + assertEquals(ScanOutcome.NotAnIsbn("012345678905"), filter.accept("012345678905")) } } diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanModelsTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanModelsTest.kt index 5477883..5ff1ad3 100644 --- a/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanModelsTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScanModelsTest.kt @@ -5,6 +5,7 @@ import org.junit.Assert.assertTrue import org.junit.Test import org.modg.bookshelf.data.local.BookEntity import org.modg.bookshelf.data.metadata.BookMetadata +import org.modg.bookshelf.data.metadata.LookupResult class ScanModelsTest { @@ -25,21 +26,27 @@ class ScanModelsTest { } @Test - fun `metadata miss maps to NotFound regardless of duplicate status`() { - val outcome = ScanMetadataOutcome.from("9780201558029", null, DuplicateStatus.New) + fun `an authoritative NotFound result maps to NotFound regardless of duplicate status`() { + val outcome = ScanMetadataOutcome.from("9780201558029", LookupResult.NotFound, DuplicateStatus.New) assertEquals(ScanSheetState.NotFound("9780201558029"), outcome) } @Test - fun `metadata hit maps to Found carrying the duplicate status through`() { + fun `a Found result maps to Found carrying the duplicate status through`() { val metadata = BookMetadata(title = "Dune", isbn13 = "9780201558029") val duplicate = DuplicateStatus.AlreadyOwned("abc123", "Dune") - val outcome = ScanMetadataOutcome.from("9780201558029", metadata, duplicate) + val outcome = ScanMetadataOutcome.from("9780201558029", LookupResult.Found(metadata), duplicate) assertEquals(ScanSheetState.Found("9780201558029", metadata, duplicate), outcome) } + @Test + fun `an Unavailable result maps to LookupFailed and never claims the book is unknown`() { + val outcome = ScanMetadataOutcome.from("9780201558029", LookupResult.Unavailable("http 429"), DuplicateStatus.New) + assertEquals(ScanSheetState.LookupFailed("9780201558029", "http 429"), outcome) + } + @Test fun `session count starts at zero and increments per save, not per skip`() { var session = ScanSessionState() diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScannerControllerTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScannerControllerTest.kt index ff1ca2e..6daaa02 100644 --- a/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScannerControllerTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/ui/scan/ScannerControllerTest.kt @@ -46,6 +46,39 @@ class ScannerControllerTest { assertEquals(listOf("9780201558029"), received) } + @Test + fun `non-book barcode emits on rejectedCodes, not scanResults`() = runTest { + val controller = ScannerController() + val scanned = mutableListOf() + val rejected = mutableListOf() + backgroundScope.launch { controller.scanResults.toList(scanned) } + backgroundScope.launch { controller.rejectedCodes.toList(rejected) } + runCurrent() + + controller.onBarcodeScanned("012345678905") // UPC-A shaped, fails the ISBN-13 checksum + runCurrent() + + assertTrue(scanned.isEmpty()) + assertEquals(listOf("012345678905"), rejected) + } + + @Test + fun `a debounced repeat reaches neither scanResults nor rejectedCodes`() = runTest { + val controller = ScannerController() + val scanned = mutableListOf() + val rejected = mutableListOf() + backgroundScope.launch { controller.scanResults.toList(scanned) } + backgroundScope.launch { controller.rejectedCodes.toList(rejected) } + runCurrent() + + controller.onBarcodeScanned("012345678905") + controller.onBarcodeScanned("012345678905") // same frame's worth of repeat reads + runCurrent() + + assertEquals(listOf("012345678905"), rejected) + assertTrue(scanned.isEmpty()) + } + @Test fun `torch toggles from its default off state`() { val controller = ScannerController() diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/screens/ScanScreenPaparazziTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/screens/ScanScreenPaparazziTest.kt index b88df94..eaa8f0f 100644 --- a/app/app/src/test/java/org/modg/bookshelf/ui/screens/ScanScreenPaparazziTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/ui/screens/ScanScreenPaparazziTest.kt @@ -27,6 +27,8 @@ import org.modg.bookshelf.data.metadata.BookMetadata import org.modg.bookshelf.ui.components.BookshelfScaffold import org.modg.bookshelf.ui.scan.DuplicateStatus import org.modg.bookshelf.ui.scan.FoundBookSheet +import org.modg.bookshelf.ui.scan.LookupFailedSheet +import org.modg.bookshelf.ui.scan.RejectedBarcodeBanner import org.modg.bookshelf.ui.scan.ScanReticle import org.modg.bookshelf.ui.scan.SearchingSheet import org.modg.bookshelf.ui.scan.SessionBadge @@ -57,6 +59,14 @@ class ScanScreenPaparazziTest { @Test fun scanSearchingSheetLight() = snapshotBoth("scan-searching-sheet") { SearchingSheetOverlay() } + /** Neither source could be reached — see [LookupFailedSheet]. Must not read as "not found". */ + @Test + fun scanLookupFailedSheetLight() = snapshotBoth("scan-lookup-failed-sheet") { LookupFailedSheetOverlay() } + + /** A decoded barcode that failed the ISBN-13 checksum — see [RejectedBarcodeBanner]. */ + @Test + fun scanRejectedBarcodeLight() = snapshotBoth("scan-rejected-barcode") { RejectedBarcodeOverlay() } + @Composable private fun ReticleOverlay() = Shell { Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { @@ -107,6 +117,35 @@ class ScanScreenPaparazziTest { } } + @Composable + private fun LookupFailedSheetOverlay() = Shell { + Box(modifier = Modifier.fillMaxSize()) { + Surface( + modifier = Modifier.align(Alignment.BottomCenter).fillMaxWidth(), + shape = RoundedCornerShape(topStart = 16.dp, topEnd = 16.dp), + color = MaterialTheme.colorScheme.surface, + ) { + LookupFailedSheet( + isbn13 = "9780765326355", + onRetry = {}, + onEnterByHand = {}, + onSkip = {}, + ) + } + } + } + + @Composable + private fun RejectedBarcodeOverlay() = Shell { + Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { + ScanReticle(modifier = Modifier.align(Alignment.Center)) + RejectedBarcodeBanner( + message = "Read 012345678905 — not a book barcode", + modifier = Modifier.align(Alignment.BottomCenter).padding(24.dp), + ) + } + } + @Composable private fun Shell(overlay: @Composable () -> Unit) { BookshelfScaffold( diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanLookupFailedSheetLight_scan-lookup-failed-sheet-dark.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanLookupFailedSheetLight_scan-lookup-failed-sheet-dark.png new file mode 100644 index 0000000..a1dd69e Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanLookupFailedSheetLight_scan-lookup-failed-sheet-dark.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanLookupFailedSheetLight_scan-lookup-failed-sheet-light.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanLookupFailedSheetLight_scan-lookup-failed-sheet-light.png new file mode 100644 index 0000000..a6e6d04 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanLookupFailedSheetLight_scan-lookup-failed-sheet-light.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanRejectedBarcodeLight_scan-rejected-barcode-dark.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanRejectedBarcodeLight_scan-rejected-barcode-dark.png new file mode 100644 index 0000000..d3a341b Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanRejectedBarcodeLight_scan-rejected-barcode-dark.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanRejectedBarcodeLight_scan-rejected-barcode-light.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanRejectedBarcodeLight_scan-rejected-barcode-light.png new file mode 100644 index 0000000..ab5dcd5 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_ScanScreenPaparazziTest_scanRejectedBarcodeLight_scan-rejected-barcode-light.png differ