Distinguish can't-scan, can't-look-up, and genuinely-not-found

Implemented by a Sonnet worker (tasks/G-diagnostics.txt) against the
three-way contract added to SPEC in 1969b74; independently re-verified by
the orchestrator rather than accepted on the worker's own report.

The app previously conflated three outcomes. A barcode that failed the
ISBN-13 checksum produced NOTHING — no sheet, no message — which is
indistinguishable from a dead camera. A lookup that failed at the
transport layer (Google Books answers HTTP 429 to keyless callers) was
reported as "No match found", telling the user a book does not exist when
the app never managed to ask. Only the third case was ever honest.

Now: each client returns SourceResult{Found,NotFound,Failed} from a pure
classify() so the status-code matrix is testable offline without a
MockWebServer; MetadataRepository.combine folds those into
LookupResult{Found,NotFound,Unavailable}, where NotFound requires EVERY
source to have answered authoritatively. A rejected barcode raises a
throttled banner on the camera screen, sharing ScanCodeFilter's existing
debounce so a non-book barcode sitting in frame shows the message once
instead of flickering per analyzed frame. A failed lookup gets its own
sheet with Retry / Enter by hand / Skip that never claims the book is
unknown. The metadata OkHttpClient finally has a call timeout.

Orchestrator's own addition: the manual-ISBN dialog silently discarded an
unparseable entry — the same silent failure on the same screen, missed
because it sat just outside the worker's brief. It now marks the field in
error and disables Look up until the checksum passes.

assembleDebug + verifyPaparazziDebug exit 0; 138 tests, 1 skipped, 0
failures (was 107). Boundary check clean: no build files, no data/local,
data/remote, data/repo, ui/settings, ui/locations or ui/detail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Sprite
2026-09-09 12:44:29 +00:00
co-authored by claude
parent 0aff56f97e
commit 93f972b7d1
22 changed files with 676 additions and 99 deletions
@@ -1,6 +1,7 @@
package org.modg.bookshelf package org.modg.bookshelf
import android.content.Context import android.content.Context
import java.util.concurrent.TimeUnit
import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.Mutex
@@ -86,11 +87,19 @@ class AppContainer(private val context: Context) {
val authRepository by lazy { AuthRepository(apiProvider, settingsStore) } val authRepository by lazy { AuthRepository(apiProvider, settingsStore) }
// A bare client — deliberately NOT [okHttpClient] above, which carries our // Deliberately NOT [okHttpClient] above, which carries our PocketBase bearer
// PocketBase bearer token via PbAuthInterceptor. Open Library/Google Books // token via PbAuthInterceptor. Open Library/Google Books are third-party
// are third-party services; that token must never leave this device's // services; that token must never leave this device's requests to our own
// requests to our own server. // server. A call timeout is load-bearing here: with none, a stalled
private val metadataHttpClient: OkHttpClient by lazy { OkHttpClient() } // 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) } val metadataRepository by lazy { MetadataRepository(metadataHttpClient, json) }
@@ -1,6 +1,7 @@
package org.modg.bookshelf.data.metadata package org.modg.bookshelf.data.metadata
import java.io.IOException import java.io.IOException
import java.net.SocketTimeoutException
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import kotlinx.serialization.SerializationException import kotlinx.serialization.SerializationException
@@ -10,7 +11,8 @@ import okhttp3.Request
/** /**
* Google Books lookup — SPEC.md "Book metadata lookup" fallback source. No API key. * 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( class GoogleBooksClient(
private val httpClient: OkHttpClient, private val httpClient: OkHttpClient,
@@ -18,29 +20,42 @@ class GoogleBooksClient(
) { ) {
private val json = Json(from = json) { ignoreUnknownKeys = true } private val json = Json(from = json) { ignoreUnknownKeys = true }
suspend fun lookup(isbn13: String): BookMetadata? = withContext(Dispatchers.IO) { suspend fun lookup(isbn13: String): SourceResult = withContext(Dispatchers.IO) { fetch(isbn13) }
val body = fetchBody(isbn13) ?: return@withContext null
parseResponse(body)
}
private fun fetchBody(isbn13: String): String? = try { private fun fetch(isbn13: String): SourceResult = try {
val request = Request.Builder() val request = Request.Builder()
.url("https://www.googleapis.com/books/v1/volumes?q=isbn:$isbn13") .url("https://www.googleapis.com/books/v1/volumes?q=isbn:$isbn13")
.build() .build()
httpClient.newCall(request).execute().use { response -> 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) { } 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. */ /** Package-visible for offline fixture tests — parses a raw response body with no network involved. */
internal fun parseResponse(body: String): BookMetadata? = try { internal fun parseResponse(body: String): BookMetadata? =
val dto = json.decodeFromString(GoogleBooksResponseDto.serializer(), body) (classify(200, body) as? SourceResult.Found)?.metadata
dto.items.firstOrNull()?.volumeInfo?.toBookMetadata()
} catch (e: SerializationException) {
null
} catch (e: IllegalArgumentException) {
null
}
} }
@@ -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
}
@@ -7,9 +7,10 @@ import okhttp3.OkHttpClient
/** /**
* Single entry point for book metadata lookup (SPEC.md "Book metadata lookup"). * Single entry point for book metadata lookup (SPEC.md "Book metadata lookup").
* Queries both sources concurrently and merges per [MetadataMerger]. Returns null * Queries both sources concurrently and combines their [SourceResult]s into one
* if [isbn] doesn't checksum-validate or if both sources miss — callers (the scan * [LookupResult] per [combine]. [isbn] must already be a checksum-valid ISBN-10/13
* screen) must then fall back to manual entry pre-filled with the scanned ISBN. * 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( class MetadataRepository(
private val openLibraryClient: OpenLibraryClient, private val openLibraryClient: OpenLibraryClient,
@@ -20,21 +21,46 @@ class MetadataRepository(
GoogleBooksClient(httpClient, json), GoogleBooksClient(httpClient, json),
) )
suspend fun lookup(isbn: String): BookMetadata? { suspend fun lookup(isbn: String): LookupResult {
val isbn13 = IsbnUtils.toIsbn13(isbn) ?: return null val isbn13 = checkNotNull(IsbnUtils.toIsbn13(isbn)) {
val merged = coroutineScope { "MetadataRepository.lookup requires an already-validated ISBN-10/13; got: $isbn"
}
return coroutineScope {
val openLibrary = async { openLibraryClient.lookup(isbn13) } val openLibrary = async { openLibraryClient.lookup(isbn13) }
val googleBooks = async { googleBooksClient.lookup(isbn13) } val googleBooks = async { googleBooksClient.lookup(isbn13) }
MetadataMerger.merge(openLibrary.await(), googleBooks.await()) combine(openLibrary.await(), googleBooks.await(), isbn13)
} ?: return null
return if (merged.coverUrl.isNullOrBlank()) {
merged.copy(coverUrl = byIsbnCoverUrl(isbn13))
} else {
merged
} }
} }
companion object { 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. * 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 * `default=false` is load-bearing: without it this endpoint answers 200 with a
@@ -1,6 +1,7 @@
package org.modg.bookshelf.data.metadata package org.modg.bookshelf.data.metadata
import java.io.IOException import java.io.IOException
import java.net.SocketTimeoutException
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import kotlinx.serialization.SerializationException import kotlinx.serialization.SerializationException
@@ -12,7 +13,8 @@ import okhttp3.Request
/** /**
* Open Library lookup — SPEC.md "Book metadata lookup" primary source. * 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( class OpenLibraryClient(
private val httpClient: OkHttpClient, 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. // Real responses carry fields this DTO doesn't model; never let an unknown key throw.
private val json = Json(from = json) { ignoreUnknownKeys = true } private val json = Json(from = json) { ignoreUnknownKeys = true }
suspend fun lookup(isbn13: String): BookMetadata? = withContext(Dispatchers.IO) { suspend fun lookup(isbn13: String): SourceResult = withContext(Dispatchers.IO) { fetch(isbn13) }
val body = fetchBody(isbn13) ?: return@withContext null
parseResponse(body, isbn13)
}
private fun fetchBody(isbn13: String): String? = try { private fun fetch(isbn13: String): SourceResult = try {
val request = Request.Builder() val request = Request.Builder()
.url("https://openlibrary.org/api/books?bibkeys=ISBN:$isbn13&format=json&jscmd=data") .url("https://openlibrary.org/api/books?bibkeys=ISBN:$isbn13&format=json&jscmd=data")
.build() .build()
httpClient.newCall(request).execute().use { response -> 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) { } 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<OpenLibraryBookDto>(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. */ /** Package-visible for offline fixture tests — parses a raw response body with no network involved. */
internal fun parseResponse(body: String, isbn13: String): BookMetadata? = try { internal fun parseResponse(body: String, isbn13: String): BookMetadata? =
val root = json.parseToJsonElement(body).jsonObject (classify(200, body, isbn13) as? SourceResult.Found)?.metadata
val entry = root["ISBN:$isbn13"]?.jsonObject ?: return null
json.decodeFromJsonElement<OpenLibraryBookDto>(entry).toBookMetadata(isbn13)
} catch (e: SerializationException) {
null
} catch (e: IllegalArgumentException) {
null
}
} }
@@ -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
}
@@ -2,11 +2,31 @@ package org.modg.bookshelf.ui.scan
import org.modg.bookshelf.data.metadata.IsbnUtils 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
/** /**
* Pure decode-to-ISBN pipeline shared by the analyzer and its tests: normalizes a raw * Decoded fine, but its checksum rules it out as a book ISBN (SPEC.md
* barcode value, validates the ISBN-13 checksum (this is what makes EAN_8/UPC_A reads * "Barcode scanning": "ignore non-book barcodes" — but not silently, see
* fall out as "non-book barcodes" per SPEC.md "Barcode scanning" — they can never be * [rawValue]). Carries the raw value so the camera screen can echo back what
* 13 digits), and debounces repeat reads of the same code. * 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-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( class ScanCodeFilter(
private val debounceMillis: Long = 2000L, private val debounceMillis: Long = 2000L,
@@ -15,15 +35,19 @@ class ScanCodeFilter(
private var lastCode: String? = null private var lastCode: String? = null
private var lastEmitMillis: Long = Long.MIN_VALUE private var lastEmitMillis: Long = Long.MIN_VALUE
/** Returns the normalized ISBN-13 if [rawValue] is a valid, non-debounced hit; null otherwise. */ /** Classifies [rawValue] per [ScanOutcome], applying the debounce window. */
fun accept(rawValue: String?): String? { fun accept(rawValue: String?): ScanOutcome {
val normalized = IsbnUtils.normalize(rawValue ?: return null) val raw = rawValue ?: return ScanOutcome.Ignored
if (!IsbnUtils.isValidIsbn13(normalized)) return null val normalized = IsbnUtils.normalize(raw)
val now = nowMillis() val now = nowMillis()
if (normalized == lastCode && now - lastEmitMillis < debounceMillis) return null if (normalized == lastCode && now - lastEmitMillis < debounceMillis) return ScanOutcome.Ignored
lastCode = normalized lastCode = normalized
lastEmitMillis = now 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. */ /** Allows the next scan of any code (including a repeat) to emit immediately, e.g. after Skip. */
@@ -2,6 +2,7 @@ package org.modg.bookshelf.ui.scan
import org.modg.bookshelf.data.local.BookEntity import org.modg.bookshelf.data.local.BookEntity
import org.modg.bookshelf.data.metadata.BookMetadata import org.modg.bookshelf.data.metadata.BookMetadata
import org.modg.bookshelf.data.metadata.LookupResult
/** SPEC.md scan screen: "Duplicate-ISBN warning if already owned." */ /** SPEC.md scan screen: "Duplicate-ISBN warning if already owned." */
sealed interface DuplicateStatus { sealed interface DuplicateStatus {
@@ -27,14 +28,30 @@ sealed interface ScanSheetState {
*/ */
data class Loading(val isbn13: String) : ScanSheetState data class Loading(val isbn13: String) : ScanSheetState
data class Found(val isbn13: String, val metadata: BookMetadata, val duplicate: DuplicateStatus) : 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. */ /** Combines a metadata lookup result with duplicate status into the sheet state to show. */
object ScanMetadataOutcome { object ScanMetadataOutcome {
fun from(isbn13: String, metadata: BookMetadata?, duplicate: DuplicateStatus): ScanSheetState = when (metadata) { fun from(isbn13: String, result: LookupResult, duplicate: DuplicateStatus): ScanSheetState = when (result) {
null -> ScanSheetState.NotFound(isbn13) is LookupResult.Found -> ScanSheetState.Found(isbn13, result.metadata, duplicate)
else -> ScanSheetState.Found(isbn13, metadata, duplicate) is LookupResult.NotFound -> ScanSheetState.NotFound(isbn13)
is LookupResult.Unavailable -> ScanSheetState.LookupFailed(isbn13, result.reason)
} }
} }
@@ -18,6 +18,7 @@ import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.size
import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.foundation.text.KeyboardOptions
import androidx.compose.material.icons.Icons import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.outlined.ArrowBack import androidx.compose.material.icons.automirrored.outlined.ArrowBack
import androidx.compose.material.icons.outlined.FlashOff 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.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.text.input.KeyboardType
import androidx.compose.ui.platform.LocalLifecycleOwner import androidx.compose.ui.platform.LocalLifecycleOwner
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
import androidx.compose.ui.viewinterop.AndroidView 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.BookcaseEntity
import org.modg.bookshelf.data.local.ShelfEntity import org.modg.bookshelf.data.local.ShelfEntity
import org.modg.bookshelf.data.metadata.BookMetadata 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.BookCover
import org.modg.bookshelf.ui.components.BookshelfScaffold import org.modg.bookshelf.ui.components.BookshelfScaffold
import org.modg.bookshelf.ui.components.EmptyState import org.modg.bookshelf.ui.components.EmptyState
@@ -89,6 +92,7 @@ fun ScanScreen(
val sheetState by viewModel.sheetState.collectAsState() val sheetState by viewModel.sheetState.collectAsState()
val sessionState by viewModel.sessionState.collectAsState() val sessionState by viewModel.sessionState.collectAsState()
val rejectedMessage by viewModel.rejectedMessage.collectAsState()
val torchEnabled by viewModel.scannerController.torchEnabled.collectAsState() val torchEnabled by viewModel.scannerController.torchEnabled.collectAsState()
val bookcases by viewModel.bookcases.collectAsState() val bookcases by viewModel.bookcases.collectAsState()
val shelves by viewModel.shelves.collectAsState() val shelves by viewModel.shelves.collectAsState()
@@ -128,6 +132,12 @@ fun ScanScreen(
CameraPreview(controller = viewModel.scannerController, torchEnabled = torchEnabled) CameraPreview(controller = viewModel.scannerController, torchEnabled = torchEnabled)
ScanReticle(modifier = Modifier.align(Alignment.Center)) ScanReticle(modifier = Modifier.align(Alignment.Center))
SessionBadge(count = sessionState.savedCount, modifier = Modifier.align(Alignment.TopCenter).padding(16.dp)) 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 { } else {
PermissionDeniedContent(onRequestAgain = { permissionState.launchPermissionRequest() }) PermissionDeniedContent(onRequestAgain = { permissionState.launchPermissionRequest() })
} }
@@ -160,6 +170,7 @@ fun ScanScreen(
) { ) {
ManualEntrySheet( ManualEntrySheet(
isbn13 = state.isbn13, isbn13 = state.isbn13,
authoritative = !state.viaLookupFailure,
bookcases = bookcases, bookcases = bookcases,
shelves = shelves, shelves = shelves,
selectedShelfId = selectedShelfId, selectedShelfId = selectedShelfId,
@@ -168,6 +179,17 @@ fun ScanScreen(
onSkip = { viewModel.skip() }, 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) { 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 @Composable
internal fun ManualEntrySheet( internal fun ManualEntrySheet(
isbn13: String, isbn13: String,
authoritative: Boolean,
bookcases: List<BookcaseEntity>, bookcases: List<BookcaseEntity>,
shelves: List<ShelfEntity>, shelves: List<ShelfEntity>,
selectedShelfId: String?, selectedShelfId: String?,
@@ -354,9 +383,12 @@ internal fun ManualEntrySheet(
var authors by remember { mutableStateOf("") } var authors by remember { mutableStateOf("") }
Column(modifier = Modifier.fillMaxWidth().padding(16.dp)) { Column(modifier = Modifier.fillMaxWidth().padding(16.dp)) {
Text(text = "No match found", style = MaterialTheme.typography.titleLarge)
Text( 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, style = MaterialTheme.typography.bodyMedium,
color = MaterialTheme.colorScheme.onSurfaceVariant, color = MaterialTheme.colorScheme.onSurfaceVariant,
modifier = Modifier.padding(bottom = 12.dp), 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 @Composable
internal fun ShelfPicker( internal fun ShelfPicker(
bookcases: List<BookcaseEntity>, bookcases: List<BookcaseEntity>,
@@ -431,19 +511,37 @@ internal fun ShelfPicker(
@Composable @Composable
private fun ManualIsbnDialog(onDismiss: () -> Unit, onSubmit: (String) -> Unit) { private fun ManualIsbnDialog(onDismiss: () -> Unit, onSubmit: (String) -> Unit) {
var text by remember { mutableStateOf("") } 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( AlertDialog(
onDismissRequest = onDismiss, onDismissRequest = onDismiss,
title = { Text("Enter ISBN") }, title = { Text("Enter ISBN") },
text = { text = {
Column {
OutlinedTextField( OutlinedTextField(
value = text, value = text,
onValueChange = { text = it }, onValueChange = { text = it },
label = { Text("ISBN-10 or ISBN-13") }, label = { Text("ISBN-10 or ISBN-13") },
singleLine = true, 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 = { confirmButton = {
PrimaryButton(text = "Look up", onClick = { onSubmit(text) }, enabled = text.isNotBlank()) PrimaryButton(text = "Look up", onClick = { onSubmit(text) }, enabled = isbn13 != null)
}, },
dismissButton = { dismissButton = {
SecondaryButton(text = "Cancel", onClick = onDismiss) SecondaryButton(text = "Cancel", onClick = onDismiss)
@@ -2,6 +2,8 @@ package org.modg.bookshelf.ui.scan
import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
@@ -35,6 +37,11 @@ class ScanViewModel(
private val _sessionState = MutableStateFlow(ScanSessionState()) private val _sessionState = MutableStateFlow(ScanSessionState())
val sessionState: StateFlow<ScanSessionState> = _sessionState.asStateFlow() val sessionState: StateFlow<ScanSessionState> = _sessionState.asStateFlow()
private val _rejectedMessage = MutableStateFlow<String?>(null)
/** Transient "read but not a book barcode" message for the camera overlay; auto-clears. */
val rejectedMessage: StateFlow<String?> = _rejectedMessage.asStateFlow()
private var rejectedMessageClearJob: Job? = null
val bookcases: StateFlow<List<BookcaseEntity>> = locationRepository.observeBookcases() val bookcases: StateFlow<List<BookcaseEntity>> = locationRepository.observeBookcases()
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList())
val shelves: StateFlow<List<ShelfEntity>> = locationRepository.observeShelves() val shelves: StateFlow<List<ShelfEntity>> = locationRepository.observeShelves()
@@ -48,14 +55,41 @@ class ScanViewModel(
viewModelScope.launch { viewModelScope.launch {
scannerController.scanResults.collect { isbn13 -> onScanned(isbn13) } scannerController.scanResults.collect { isbn13 -> onScanned(isbn13) }
} }
viewModelScope.launch {
scannerController.rejectedCodes.collect { rawValue -> showRejectedMessage(rawValue) }
}
} }
private suspend fun onScanned(isbn13: String) { private suspend fun onScanned(isbn13: String) {
if (_sheetState.value !is ScanSheetState.Hidden) return // a sheet is already up for a previous hit 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) _sheetState.value = ScanSheetState.Loading(isbn13)
val duplicate = DuplicateCheck.check(bookRepository.findByIsbn13(isbn13)) val duplicate = DuplicateCheck.check(bookRepository.findByIsbn13(isbn13))
val metadata = metadataRepository.lookup(isbn13) val result = metadataRepository.lookup(isbn13)
_sheetState.value = ScanMetadataOutcome.from(isbn13, metadata, duplicate) _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). */ /** The manual-ISBN-entry escape hatch (SPEC: for when a barcode won't scan). */
@@ -114,4 +148,9 @@ class ScanViewModel(
_sheetState.value = ScanSheetState.Hidden _sheetState.value = ScanSheetState.Hidden
scannerController.resetDebounce() scannerController.resetDebounce()
} }
private companion object {
/** How long a rejected-barcode message stays on screen before it auto-clears. */
const val REJECTED_MESSAGE_MILLIS = 3000L
}
} }
@@ -19,6 +19,14 @@ class ScannerController(
/** Emits a debounced, checksum-valid ISBN-13 each time [IsbnBarcodeAnalyzer] sees a new book barcode. */ /** Emits a debounced, checksum-valid ISBN-13 each time [IsbnBarcodeAnalyzer] sees a new book barcode. */
val scanResults: SharedFlow<String> = _scanResults.asSharedFlow() val scanResults: SharedFlow<String> = _scanResults.asSharedFlow()
private val _rejectedCodes = MutableSharedFlow<String>(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<String> = _rejectedCodes.asSharedFlow()
private val _torchEnabled = MutableStateFlow(false) private val _torchEnabled = MutableStateFlow(false)
val torchEnabled: StateFlow<Boolean> = _torchEnabled.asStateFlow() val torchEnabled: StateFlow<Boolean> = _torchEnabled.asStateFlow()
@@ -27,7 +35,11 @@ class ScannerController(
/** Called by [IsbnBarcodeAnalyzer] for every decoded barcode's raw value. */ /** Called by [IsbnBarcodeAnalyzer] for every decoded barcode's raw value. */
fun onBarcodeScanned(rawValue: String?) { 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() { fun toggleTorch() {
@@ -46,6 +46,47 @@ class GoogleBooksClientTest {
assertNull(client.parseResponse(body)) 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 = private fun fixture(name: String): String =
checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" } checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" }
.bufferedReader() .bufferedReader()
@@ -6,6 +6,80 @@ import org.junit.Test
class MetadataRepositoryTest { 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 * `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 * answers 200 with a 43-byte 1x1 transparent GIF for an edition it holds no art for
@@ -105,6 +105,47 @@ class OpenLibraryClientTest {
assertNull(client.parseResponse(body, "9780201558029")) 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 = private fun fixture(name: String): String =
checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" } checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" }
.bufferedReader() .bufferedReader()
@@ -1,7 +1,6 @@
package org.modg.bookshelf.ui.scan package org.modg.bookshelf.ui.scan
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Test import org.junit.Test
class ScanCodeFilterTest { class ScanCodeFilterTest {
@@ -9,45 +8,45 @@ class ScanCodeFilterTest {
@Test @Test
fun `accepts a checksum-valid isbn13`() { fun `accepts a checksum-valid isbn13`() {
val filter = ScanCodeFilter() val filter = ScanCodeFilter()
assertEquals("9780201558029", filter.accept("9780201558029")) assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029"))
} }
@Test @Test
fun `rejects a checksum-invalid isbn13-shaped code`() { fun `rejects a checksum-invalid isbn13-shaped code as not-an-isbn`() {
val filter = ScanCodeFilter() val filter = ScanCodeFilter()
assertNull(filter.accept("9780201558020")) assertEquals(ScanOutcome.NotAnIsbn("9780201558020"), filter.accept("9780201558020"))
} }
@Test @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() val filter = ScanCodeFilter()
assertNull(filter.accept("12345670")) // EAN-8 shaped assertEquals(ScanOutcome.NotAnIsbn("12345670"), filter.accept("12345670")) // EAN-8 shaped
assertNull(filter.accept("012345678905")) // UPC-A shaped, 12 digits assertEquals(ScanOutcome.NotAnIsbn("012345678905"), filter.accept("012345678905")) // UPC-A shaped, 12 digits
} }
@Test @Test
fun `rejects null raw value`() { fun `ignores null raw value`() {
assertNull(ScanCodeFilter().accept(null)) assertEquals(ScanOutcome.Ignored, ScanCodeFilter().accept(null))
} }
@Test @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 var now = 0L
val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now })
assertEquals("9780201558029", filter.accept("9780201558029")) assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029"))
now += 500 now += 500
assertNull(filter.accept("9780201558029")) assertEquals(ScanOutcome.Ignored, filter.accept("9780201558029"))
} }
@Test @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 var now = 0L
val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now })
assertEquals("9780201558029", filter.accept("9780201558029")) assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029"))
now += 2001 now += 2001
assertEquals("9780201558029", filter.accept("9780201558029")) assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029"))
} }
@Test @Test
@@ -55,9 +54,9 @@ class ScanCodeFilterTest {
var now = 0L var now = 0L
val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now })
assertEquals("9780201558029", filter.accept("9780201558029")) assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029"))
now += 10 now += 10
assertEquals("9780134685991", filter.accept("9780134685991")) assertEquals(ScanOutcome.Isbn("9780134685991"), filter.accept("9780134685991"))
} }
@Test @Test
@@ -65,8 +64,56 @@ class ScanCodeFilterTest {
var now = 0L var now = 0L
val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now }) val filter = ScanCodeFilter(debounceMillis = 2000L, nowMillis = { now })
assertEquals("9780201558029", filter.accept("9780201558029")) assertEquals(ScanOutcome.Isbn("9780201558029"), filter.accept("9780201558029"))
filter.reset() 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"))
} }
} }
@@ -5,6 +5,7 @@ import org.junit.Assert.assertTrue
import org.junit.Test import org.junit.Test
import org.modg.bookshelf.data.local.BookEntity import org.modg.bookshelf.data.local.BookEntity
import org.modg.bookshelf.data.metadata.BookMetadata import org.modg.bookshelf.data.metadata.BookMetadata
import org.modg.bookshelf.data.metadata.LookupResult
class ScanModelsTest { class ScanModelsTest {
@@ -25,21 +26,27 @@ class ScanModelsTest {
} }
@Test @Test
fun `metadata miss maps to NotFound regardless of duplicate status`() { fun `an authoritative NotFound result maps to NotFound regardless of duplicate status`() {
val outcome = ScanMetadataOutcome.from("9780201558029", null, DuplicateStatus.New) val outcome = ScanMetadataOutcome.from("9780201558029", LookupResult.NotFound, DuplicateStatus.New)
assertEquals(ScanSheetState.NotFound("9780201558029"), outcome) assertEquals(ScanSheetState.NotFound("9780201558029"), outcome)
} }
@Test @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 metadata = BookMetadata(title = "Dune", isbn13 = "9780201558029")
val duplicate = DuplicateStatus.AlreadyOwned("abc123", "Dune") 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) 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 @Test
fun `session count starts at zero and increments per save, not per skip`() { fun `session count starts at zero and increments per save, not per skip`() {
var session = ScanSessionState() var session = ScanSessionState()
@@ -46,6 +46,39 @@ class ScannerControllerTest {
assertEquals(listOf("9780201558029"), received) assertEquals(listOf("9780201558029"), received)
} }
@Test
fun `non-book barcode emits on rejectedCodes, not scanResults`() = runTest {
val controller = ScannerController()
val scanned = mutableListOf<String>()
val rejected = mutableListOf<String>()
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<String>()
val rejected = mutableListOf<String>()
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 @Test
fun `torch toggles from its default off state`() { fun `torch toggles from its default off state`() {
val controller = ScannerController() val controller = ScannerController()
@@ -27,6 +27,8 @@ import org.modg.bookshelf.data.metadata.BookMetadata
import org.modg.bookshelf.ui.components.BookshelfScaffold import org.modg.bookshelf.ui.components.BookshelfScaffold
import org.modg.bookshelf.ui.scan.DuplicateStatus import org.modg.bookshelf.ui.scan.DuplicateStatus
import org.modg.bookshelf.ui.scan.FoundBookSheet 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.ScanReticle
import org.modg.bookshelf.ui.scan.SearchingSheet import org.modg.bookshelf.ui.scan.SearchingSheet
import org.modg.bookshelf.ui.scan.SessionBadge import org.modg.bookshelf.ui.scan.SessionBadge
@@ -57,6 +59,14 @@ class ScanScreenPaparazziTest {
@Test @Test
fun scanSearchingSheetLight() = snapshotBoth("scan-searching-sheet") { SearchingSheetOverlay() } 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 @Composable
private fun ReticleOverlay() = Shell { private fun ReticleOverlay() = Shell {
Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { 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 @Composable
private fun Shell(overlay: @Composable () -> Unit) { private fun Shell(overlay: @Composable () -> Unit) {
BookshelfScaffold( BookshelfScaffold(