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 7186524..7040ceb 100644 --- a/app/app/src/main/java/org/modg/bookshelf/AppContainer.kt +++ b/app/app/src/main/java/org/modg/bookshelf/AppContainer.kt @@ -11,6 +11,7 @@ import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import okhttp3.logging.HttpLoggingInterceptor import org.modg.bookshelf.data.local.BookshelfDatabase +import org.modg.bookshelf.data.metadata.BookSearchRepository import org.modg.bookshelf.data.metadata.MetadataRepository import org.modg.bookshelf.diagnostics.CrashReporter import org.modg.bookshelf.data.prefs.SettingsStore @@ -120,6 +121,9 @@ class AppContainer(private val context: Context) { val metadataRepository by lazy { MetadataRepository(metadataHttpClient, json, BuildConfig.GOOGLE_BOOKS_API_KEY) } + /** Library screen's online title/author search — shares the same client/timeouts/key as [metadataRepository]. */ + val bookSearchRepository by lazy { BookSearchRepository(metadataHttpClient, json, BuildConfig.GOOGLE_BOOKS_API_KEY) } + val syncEngine by lazy { SyncEngine( apiProvider = apiProvider, diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/BookSearchRepository.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/BookSearchRepository.kt new file mode 100644 index 0000000..a45aba0 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/BookSearchRepository.kt @@ -0,0 +1,33 @@ +package org.modg.bookshelf.data.metadata + +import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope +import kotlinx.serialization.json.Json +import okhttp3.OkHttpClient + +/** + * Entry point for the library screen's online title/author search — [MetadataRepository]'s + * counterpart for search rather than by-ISBN lookup. Unlike [MetadataRepository.lookup], + * this does NOT combine the two sources into one outcome: the library screen needs each + * source's own [SearchSourceResult] so it can show one source's results alongside the + * other's failure-with-retry (task: "per-source failure ... while still showing the + * OTHER source's results"). [org.modg.bookshelf.data.metadata.SearchResultMerger] is + * where the actual merge/dedupe happens, once local books are available to merge in too. + */ +class BookSearchRepository( + private val openLibraryClient: OpenLibrarySearchClient, + private val googleBooksClient: GoogleBooksSearchClient, +) { + constructor(httpClient: OkHttpClient, json: Json, googleBooksApiKey: String = "") : this( + OpenLibrarySearchClient(httpClient, json), + GoogleBooksSearchClient(httpClient, json, googleBooksApiKey), + ) + + data class Results(val openLibrary: SearchSourceResult, val googleBooks: SearchSourceResult) + + suspend fun search(query: String): Results = coroutineScope { + val openLibrary = async { openLibraryClient.search(query) } + val googleBooks = async { googleBooksClient.search(query) } + Results(openLibrary.await(), googleBooks.await()) + } +} 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 8ce5877..3869b5d 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 @@ -130,9 +130,5 @@ class GoogleBooksClient( * will. Package-visible so the scrub itself is unit-testable without a socket. */ internal fun redact(result: SourceResult): SourceResult = - if (result is SourceResult.Failed && apiKey.isNotBlank() && result.reason.contains(apiKey)) { - result.copy(reason = result.reason.replace(apiKey, "[REDACTED]")) - } else { - result - } + if (result is SourceResult.Failed) result.copy(reason = redactApiKey(result.reason, apiKey)) else result } diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksDtos.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksDtos.kt index f5361a8..3f12b9a 100644 --- a/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksDtos.kt +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksDtos.kt @@ -52,8 +52,46 @@ fun GoogleBooksVolumeInfoDto.toBookMetadata(): BookMetadata = BookMetadata( coverUrl = normalizeCoverUrl(imageLinks?.thumbnail ?: imageLinks?.smallThumbnail), ) -private fun normalizeCoverUrl(raw: String?): String? { +/** Forces https + zoom=2, per SPEC's cover chain. Internal (not private) so [GoogleBooksSearchClient] reuses it too. */ +internal fun normalizeCoverUrl(raw: String?): String? { if (raw.isNullOrBlank()) return null val https = raw.replaceFirst("http://", "https://") return if ("zoom=" in https) https.replace(Regex("zoom=\\d+"), "zoom=2") else "$https&zoom=2" } + +/** + * Maps one volume to a [SearchHit] for the library search screen. [isbn13s] collects + * every industry identifier this volume reports, normalized to ISBN-13 regardless of + * its declared type (ISBN-10 converted) — a volume usually carries 0 or 1, unlike an + * Open Library work's dozens, which is exactly what [SearchResultMerger]'s ISBN rule + * depends on to tell the two sources apart. + */ +fun GoogleBooksVolumeInfoDto.toSearchHit(): SearchHit = SearchHit( + title = title, + subtitle = subtitle, + authors = authors, + year = plausibleYear(publishedDate), + publisher = publisher, + pageCount = pageCount, + coverUrl = normalizeCoverUrl(imageLinks?.thumbnail ?: imageLinks?.smallThumbnail), + isbn13s = industryIdentifiers.mapNotNull { it.identifier }.mapNotNull(IsbnUtils::toIsbn13).distinct(), +) + +/** + * A [publishedDate] prefix is only trustworthy as a year if it's exactly four digits — + * real GB data has been observed to return garbage like "101-01-01" (METADATA-SOURCES.md), + * which this rejects because its first four characters ("101-") aren't all digits. + */ +internal fun plausibleYear(publishedDate: String?): Int? { + val prefix = publishedDate?.take(4) ?: return null + if (prefix.length != 4 || !prefix.all(Char::isDigit)) return null + return prefix.toIntOrNull()?.takeIf { it in 1000..2999 } +} + +/** + * [SourceResult.Failed.reason]/[SearchSourceResult.Failed.reason] must never carry the + * API key — shared by [GoogleBooksClient.redact] and [GoogleBooksSearchClient.redact] + * so there is one scrub implementation, not two that could drift apart. + */ +internal fun redactApiKey(reason: String, apiKey: String): String = + if (apiKey.isNotBlank() && reason.contains(apiKey)) reason.replace(apiKey, "[REDACTED]") else reason diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksSearchClient.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksSearchClient.kt new file mode 100644 index 0000000..96410a9 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/GoogleBooksSearchClient.kt @@ -0,0 +1,88 @@ +package org.modg.bookshelf.data.metadata + +import java.io.IOException +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request + +/** + * Google Books title/author search — `volumes?q=`, distinct from [GoogleBooksClient]'s + * by-ISBN lookup but sharing its DTOs ([GoogleBooksResponseDto]/[GoogleBooksVolumeInfoDto]): + * a search result and an ISBN lookup result are both a list of volumes, just a different + * query. Same key handling, same redaction, same never-throws contract — see + * [GoogleBooksClient]'s KDoc. + */ +class GoogleBooksSearchClient( + private val httpClient: OkHttpClient, + json: Json, + private val apiKey: String = "", +) { + private val json = Json(from = json) { ignoreUnknownKeys = true } + + suspend fun search(query: String): SearchSourceResult = + withContext(Dispatchers.IO) { + withSearchRetry(retryRateLimitedOnce = apiKey.isNotBlank()) { fetch(query) } + } + + /** Single un-retried attempt, for tests that need to count calls. */ + internal suspend fun searchOnce(query: String): SearchSourceResult = + withContext(Dispatchers.IO) { fetch(query) } + + private fun fetch(query: String): SearchSourceResult = redact( + try { + val request = Request.Builder().url(requestUrl(query)).build() + httpClient.newCall(request).execute().use { response -> + classify(response.code, response.body.string(), response.header("Retry-After")) + } + } catch (e: CancellationException) { + throw e + } catch (e: IOException) { + SearchSourceResult.fromException(e) + } catch (e: Throwable) { + SearchSourceResult.Failed("unexpected: ${e.javaClass.simpleName}", FailureKind.UNEXPECTED) + }, + ) + + /** Package-visible pure function — no socket — the key is appended only when non-blank, same as the ISBN lookup. */ + internal fun requestUrl(query: String): String { + val builder = HttpUrl.Builder() + .scheme("https") + .host("www.googleapis.com") + .addPathSegments("books/v1/volumes") + .addQueryParameter("q", query) + .addQueryParameter("maxResults", "20") + .addQueryParameter("printType", "books") + if (apiKey.isNotBlank()) { + builder.addQueryParameter("key", apiKey) + } + return builder.build().toString() + } + + /** Package-visible pure function — no socket — so it's exhaustively unit-testable offline. */ + internal fun classify(httpCode: Int, body: String?, retryAfterHeader: String? = null): SearchSourceResult { + if (httpCode !in 200..299) return SearchSourceResult.fromHttpCode(httpCode, retryAfterHeader) + if (body.isNullOrBlank()) return SearchSourceResult.Failed("empty body", FailureKind.MALFORMED) + return try { + val dto = json.decodeFromString(GoogleBooksResponseDto.serializer(), body) + val hits = dto.items.mapNotNull { it.volumeInfo?.toSearchHit() } + if (hits.isEmpty()) SearchSourceResult.NotFound else SearchSourceResult.Found(hits) + } catch (e: SerializationException) { + SearchSourceResult.Failed("malformed json", FailureKind.MALFORMED) + } catch (e: IllegalArgumentException) { + SearchSourceResult.Failed("malformed json", FailureKind.MALFORMED) + } + } + + /** Package-visible for offline fixture tests — parses a raw response body with no network involved. */ + internal fun parseResponse(body: String): List = + (classify(200, body) as? SearchSourceResult.Found)?.hits.orEmpty() + + /** Same reasoning as [GoogleBooksClient.redact] — the key must never survive into a shown/logged reason. */ + internal fun redact(result: SearchSourceResult): SearchSourceResult = + if (result is SearchSourceResult.Failed) result.copy(reason = redactApiKey(result.reason, apiKey)) else result +} diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/OnlineBook.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OnlineBook.kt new file mode 100644 index 0000000..3038383 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OnlineBook.kt @@ -0,0 +1,23 @@ +package org.modg.bookshelf.data.metadata + +/** Which source(s) contributed to a merged [OnlineBook] — surfaced so the UI/enrichment can tell. */ +enum class SearchSource { OPEN_LIBRARY, GOOGLE_BOOKS } + +/** + * One deduplicated online search result, after [SearchResultMerger] has grouped + * together every [SearchHit] (and any matching local [org.modg.bookshelf.data.local.BookEntity]) + * that identify the same book. [isbn13] is set only when [SearchResultMerger]'s ISBN + * rule could resolve a single unambiguous edition — see its KDoc; otherwise null, + * never guessed. + */ +data class OnlineBook( + val title: String?, + val subtitle: String? = null, + val authors: List = emptyList(), + val year: Int? = null, + val publisher: String? = null, + val pageCount: Int? = null, + val coverUrl: String? = null, + val isbn13: String? = null, + val sources: Set = emptySet(), +) diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchClient.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchClient.kt new file mode 100644 index 0000000..b976513 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchClient.kt @@ -0,0 +1,87 @@ +package org.modg.bookshelf.data.metadata + +import java.io.IOException +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request + +/** + * Open Library title/author search — `search.json`, distinct from [OpenLibraryClient]'s + * by-ISBN `api/books` lookup. Same never-throws contract as [OpenLibraryClient] (see its + * KDoc): every outcome is a [SearchSourceResult], [CancellationException] propagates, + * anything else becomes [FailureKind.UNEXPECTED]. + */ +class OpenLibrarySearchClient( + private val httpClient: OkHttpClient, + json: Json, +) { + private val json = Json(from = json) { ignoreUnknownKeys = true } + + /** Retries transient failures per [RetryPolicy] — same 13% TLS-reset story as the ISBN lookup. */ + suspend fun search(query: String): SearchSourceResult = + withContext(Dispatchers.IO) { withSearchRetry { fetch(query) } } + + /** Single un-retried attempt, for tests that need to count calls. */ + internal suspend fun searchOnce(query: String): SearchSourceResult = + withContext(Dispatchers.IO) { fetch(query) } + + private fun fetch(query: String): SearchSourceResult = try { + val request = Request.Builder().url(requestUrl(query)).build() + httpClient.newCall(request).execute().use { response -> + classify(response.code, response.body.string()) + } + } catch (e: CancellationException) { + throw e + } catch (e: IOException) { + SearchSourceResult.fromException(e) + } catch (e: Throwable) { + SearchSourceResult.Failed("unexpected: ${e.javaClass.simpleName}", FailureKind.UNEXPECTED) + } + + /** + * Package-visible pure function — no socket — for offline URL tests. `fields=` + * is load-bearing: without it `isbn` is absent from every result (verified live, + * see docs/METADATA-SOURCES.md). Built with [HttpUrl.Builder] so [query] is + * correctly percent-encoded regardless of what punctuation it contains. + */ + internal fun requestUrl(query: String): String = + HttpUrl.Builder() + .scheme("https") + .host("openlibrary.org") + .addPathSegment("search.json") + .addQueryParameter("q", query) + .addQueryParameter("limit", "20") + .addQueryParameter( + "fields", + "key,title,subtitle,author_name,first_publish_year,edition_count,cover_i,isbn,publisher,number_of_pages_median", + ) + .build() + .toString() + + /** + * Package-visible pure function — no socket — so it's exhaustively unit-testable + * offline. An empty `docs` list is [SearchSourceResult.NotFound] (OL answered + * authoritatively), never conflated with a transport [SearchSourceResult.Failed]. + */ + internal fun classify(httpCode: Int, body: String?): SearchSourceResult { + if (httpCode !in 200..299) return SearchSourceResult.fromHttpCode(httpCode) + if (body.isNullOrBlank()) return SearchSourceResult.Failed("empty body", FailureKind.MALFORMED) + return try { + val dto = json.decodeFromString(OpenLibrarySearchResponseDto.serializer(), body) + if (dto.docs.isEmpty()) SearchSourceResult.NotFound else SearchSourceResult.Found(dto.docs.map { it.toSearchHit() }) + } catch (e: SerializationException) { + SearchSourceResult.Failed("malformed json", FailureKind.MALFORMED) + } catch (e: IllegalArgumentException) { + SearchSourceResult.Failed("malformed json", FailureKind.MALFORMED) + } + } + + /** Package-visible for offline fixture tests — parses a raw response body with no network involved. */ + internal fun parseResponse(body: String): List = + (classify(200, body) as? SearchSourceResult.Found)?.hits.orEmpty() +} diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchDtos.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchDtos.kt new file mode 100644 index 0000000..cac8e5b --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchDtos.kt @@ -0,0 +1,46 @@ +package org.modg.bookshelf.data.metadata + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** + * https://openlibrary.org/search.json?q=&limit=20&fields=... — a WORKS index + * search, distinct from [OpenLibraryBookDto]'s by-ISBN edition lookup. `isbn` is + * NOT in OL's default response fields; the caller must request it explicitly (see + * [OpenLibrarySearchClient.requestUrl]) or every hit comes back with no ISBNs at all. + */ +@Serializable +data class OpenLibrarySearchResponseDto( + val docs: List = emptyList(), +) + +@Serializable +data class OpenLibrarySearchDocDto( + val key: String? = null, + val title: String? = null, + val subtitle: String? = null, + @SerialName("author_name") val authorName: List = emptyList(), + @SerialName("first_publish_year") val firstPublishYear: Int? = null, + @SerialName("cover_i") val coverI: Int? = null, + /** Flat list mixing ISBN-10 and ISBN-13 of ALL editions of this work — may be absent, may be dozens. */ + val isbn: List = emptyList(), + val publisher: List = emptyList(), + @SerialName("number_of_pages_median") val numberOfPagesMedian: Int? = null, +) + +/** + * Maps one work to a [SearchHit]. [isbn13s] normalizes every reported ISBN to + * ISBN-13 (ISBN-10 converted) and drops anything that doesn't pass a checksum — + * [SearchResultMerger]'s ISBN rule counts DISTINCT values here to decide whether + * this work resolves to a single unambiguous edition. + */ +fun OpenLibrarySearchDocDto.toSearchHit(): SearchHit = SearchHit( + title = title, + subtitle = subtitle, + authors = authorName, + year = firstPublishYear, + publisher = publisher.firstOrNull(), + pageCount = numberOfPagesMedian, + coverUrl = coverI?.let { "https://covers.openlibrary.org/b/id/$it-M.jpg" }, + isbn13s = isbn.mapNotNull(IsbnUtils::toIsbn13).distinct(), +) diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/RetryPolicy.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/RetryPolicy.kt index c2a3ad4..449b4f7 100644 --- a/app/app/src/main/java/org/modg/bookshelf/data/metadata/RetryPolicy.kt +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/RetryPolicy.kt @@ -130,22 +130,82 @@ suspend fun withRetry( nowMillis: () -> Long = { System.currentTimeMillis() }, sleep: suspend (Long) -> Unit = { delay(it) }, attempt: suspend () -> SourceResult, -): SourceResult { +): SourceResult = withRetryLoop( + maxAttempts = maxAttempts, + budgetMillis = budgetMillis, + retryRateLimitedOnce = retryRateLimitedOnce, + random = random, + nowMillis = nowMillis, + sleep = sleep, + failureOf = { result -> (result as? SourceResult.Failed)?.let { it.kind to it.retryAfterMillis } }, + appendAttempts = { result, attempts -> + (result as SourceResult.Failed).copy(reason = "${result.reason}, $attempts attempts") + }, + attempt = attempt, +) + +/** + * [OpenLibrarySearchClient]/[GoogleBooksSearchClient]'s counterpart to [withRetry] — + * same policy, same loop, applied to [SearchSourceResult] instead of [SourceResult] + * (search returns a list of hits, not one [BookMetadata], so it needs its own Found + * shape, but the retry decision is identical). Both delegate to [withRetryLoop] so + * there is exactly one loop implementation to keep correct. + */ +suspend fun withSearchRetry( + maxAttempts: Int = RetryPolicy.MAX_ATTEMPTS, + budgetMillis: Long = RetryPolicy.TOTAL_BUDGET_MILLIS, + retryRateLimitedOnce: Boolean = false, + random: Random = Random.Default, + nowMillis: () -> Long = { System.currentTimeMillis() }, + sleep: suspend (Long) -> Unit = { delay(it) }, + attempt: suspend () -> SearchSourceResult, +): SearchSourceResult = withRetryLoop( + maxAttempts = maxAttempts, + budgetMillis = budgetMillis, + retryRateLimitedOnce = retryRateLimitedOnce, + random = random, + nowMillis = nowMillis, + sleep = sleep, + failureOf = { result -> (result as? SearchSourceResult.Failed)?.let { it.kind to it.retryAfterMillis } }, + appendAttempts = { result, attempts -> + (result as SearchSourceResult.Failed).copy(reason = "${result.reason}, $attempts attempts") + }, + attempt = attempt, +) + +/** + * The actual retry loop, shared by [withRetry] and [withSearchRetry] so the policy + * (attempt/budget caps, keyed-429 one-shot retry, backoff) can never drift between + * ISBN lookup and title/author search. [failureOf] extracts (kind, retryAfterMillis) + * from a failed [T], or null if [T] isn't a failure; [appendAttempts] stamps the final + * attempt count into a surviving failure's reason, same diagnostic value as before. + */ +private suspend fun withRetryLoop( + maxAttempts: Int, + budgetMillis: Long, + retryRateLimitedOnce: Boolean, + random: Random, + nowMillis: () -> Long, + sleep: suspend (Long) -> Unit, + failureOf: (T) -> Pair?, + appendAttempts: (T, Int) -> T, + attempt: suspend () -> T, +): T { val started = nowMillis() - var last: SourceResult = attempt() + var last: T = attempt() var attemptsMade = 1 var rateLimitedRetryUsed = false while (attemptsMade < maxAttempts) { - val failure = last as? SourceResult.Failed ?: return last - val rateLimitedRetry = failure.kind == FailureKind.RATE_LIMITED && - retryRateLimitedOnce && !rateLimitedRetryUsed - if (!RetryPolicy.isRetryable(failure.kind) && !rateLimitedRetry) break + val failure = failureOf(last) ?: return last + val (kind, retryAfterMillis) = failure + val rateLimitedRetry = kind == FailureKind.RATE_LIMITED && retryRateLimitedOnce && !rateLimitedRetryUsed + if (!RetryPolicy.isRetryable(kind) && !rateLimitedRetry) break if (nowMillis() - started >= budgetMillis) break if (rateLimitedRetry) { rateLimitedRetryUsed = true - sleep(failure.retryAfterMillis ?: RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS) + sleep(retryAfterMillis ?: RetryPolicy.RATE_LIMIT_RETRY_CAP_MILLIS) } else { sleep(RetryPolicy.backoffMillis(attemptsMade + 1, random)) } @@ -153,6 +213,6 @@ suspend fun withRetry( attemptsMade++ } - val failure = last as? SourceResult.Failed ?: return last - return if (attemptsMade > 1) failure.copy(reason = "${failure.reason}, $attemptsMade attempts") else failure + if (failureOf(last) == null) return last + return if (attemptsMade > 1) appendAttempts(last, attemptsMade) else last } diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchHit.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchHit.kt new file mode 100644 index 0000000..3e9558f --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchHit.kt @@ -0,0 +1,21 @@ +package org.modg.bookshelf.data.metadata + +/** + * One hit from a title/author search against Open Library or Google Books, before + * [SearchResultMerger] groups and dedupes across sources. [isbn13s] carries EVERY + * ISBN-13 this hit reports, already normalized (via [IsbnUtils.toIsbn13], ISBN-10 + * converted): for an Open Library work that can be dozens across all its editions; + * for a Google Books volume it's the 0 or 1 ISBN-13 that volume itself carries. The + * merger's ISBN rule (SPEC-adjacent, see its KDoc) reads this list directly rather + * than a single [isbn13], so it can tell "one edition" from "many" per source. + */ +data class SearchHit( + val title: String?, + val subtitle: String? = null, + val authors: List = emptyList(), + val year: Int? = null, + val publisher: String? = null, + val pageCount: Int? = null, + val coverUrl: String? = null, + val isbn13s: List = emptyList(), +) diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchResultMerger.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchResultMerger.kt new file mode 100644 index 0000000..6519598 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchResultMerger.kt @@ -0,0 +1,176 @@ +package org.modg.bookshelf.data.metadata + +import org.modg.bookshelf.data.local.BookEntity +import org.modg.bookshelf.data.repo.decodeAuthors + +/** + * Combines local books with the two search sources' hits into one deduplicated result + * (task: library search screen, "Results are DEDUPLICATED across the three sources"). + * Pure, no Android/coroutines — a plain function over already-fetched data so it's + * exhaustively unit-testable. + * + * [local] is whatever candidate set of owned books the caller wants checked for a tie + * to an online hit — the library ViewModel passes the locally text-matched books plus + * any owned book sharing an ISBN with one of the online hits, so a book reached ONLY + * via an online hit (its own text didn't match the query, but an OL/GB record it + * shares an ISBN or title+author with did) still surfaces "In your library" rather + * than being duplicated as a new online result. See the class-level dedupe writeup in + * the wave report for where a caller that passes too narrow a [local] set would miss + * that case, and where too broad a one would falsely merge unrelated namesakes. + */ +object SearchResultMerger { + + data class SearchResults(val inLibrary: List, val online: List) + + /** + * Identity: two records are the same book if they share any ISBN-13 (after + * normalizing everything, ISBN-10 included) OR their normalized title AND first + * author's surname match — grouped transitively via union-find, not pairwise, so + * A~B by ISBN and B~C by title puts all three in one group. Any group containing + * a local book becomes an "In your library" entry (shown as the local book(s) in + * it, since two owned copies/editions of the same work both belong there); every + * other group becomes one [OnlineBook], ordered by the best (lowest) position + * either source returned any of its members at, so relevance from both sources + * is preserved and neither is buried under the other. + */ + fun merge(local: List, openLibrary: List, googleBooks: List): SearchResults { + val localCount = local.size + val olCount = openLibrary.size + val total = localCount + olCount + googleBooks.size + if (total == 0) return SearchResults(emptyList(), emptyList()) + + val keys = buildList { + local.forEach { book -> add(identityKeysOf(book)) } + openLibrary.forEach { hit -> add(identityKeysOf(hit)) } + googleBooks.forEach { hit -> add(identityKeysOf(hit)) } + } + + val unionFind = UnionFind(total) + unionByIsbn(unionFind, keys) + unionByTitleAuthor(unionFind, keys) + + val groups = LinkedHashMap>() + for (index in 0 until total) groups.getOrPut(unionFind.find(index)) { mutableListOf() }.add(index) + + val inLibrary = mutableListOf() + val rankedOnline = mutableListOf>() + + for (members in groups.values) { + val localMembers = members.filter { it < localCount } + if (localMembers.isNotEmpty()) { + inLibrary += localMembers.map { local[it] } + continue + } + val olMembers = members.filter { it in localCount until localCount + olCount }.map { it - localCount } + val gbMembers = members.filter { it >= localCount + olCount }.map { it - localCount - olCount } + if (olMembers.isEmpty() && gbMembers.isEmpty()) continue + + val olHits = olMembers.map { openLibrary[it] } + val gbHits = gbMembers.map { googleBooks[it] } + val bestRank = minOf( + olMembers.minOrNull() ?: Int.MAX_VALUE, + gbMembers.minOrNull() ?: Int.MAX_VALUE, + ) + rankedOnline += bestRank to buildOnlineBook(olHits, gbHits) + } + + return SearchResults(inLibrary, rankedOnline.sortedBy { it.first }.map { it.second }) + } + + private data class IdentityKeys(val isbns: List, val titleKey: String, val surname: String) + + private fun identityKeysOf(book: BookEntity): IdentityKeys { + val isbns = listOfNotNull( + book.isbn13?.let(IsbnUtils::toIsbn13), + book.isbn10?.let(IsbnUtils::toIsbn13), + ).distinct() + return IdentityKeys(isbns, TextNormalization.normalizedTitleKey(book.title), TextNormalization.authorSurnameKey(decodeAuthors(book.authorsJson))) + } + + private fun identityKeysOf(hit: SearchHit): IdentityKeys = + IdentityKeys(hit.isbn13s, TextNormalization.normalizedTitleKey(hit.title), TextNormalization.authorSurnameKey(hit.authors)) + + private fun unionByIsbn(unionFind: UnionFind, keys: List) { + val firstSeenAt = HashMap() + keys.forEachIndexed { index, key -> + for (isbn in key.isbns) { + val existing = firstSeenAt.putIfAbsent(isbn, index) + if (existing != null) unionFind.union(existing, index) + } + } + } + + private fun unionByTitleAuthor(unionFind: UnionFind, keys: List) { + val firstSeenAt = HashMap() + keys.forEachIndexed { index, key -> + if (key.titleKey.isBlank() || key.surname.isBlank()) return@forEachIndexed + val compositeKey = "${key.titleKey}|${key.surname}" + val existing = firstSeenAt.putIfAbsent(compositeKey, index) + if (existing != null) unionFind.union(existing, index) + } + } + + /** + * SPEC-adjacent field fill, mirroring [MetadataMerger]'s "prefer whichever source + * has a title, fill blanks from the other" — here per-field per the task's rule: + * title/subtitle/authors from the OL hit with a title if there is one (work-level + * data, cleaner than a single edition's), else the first GB hit with one; year, + * publisher, page count and cover each independently fall through OL -> GB. + */ + private fun buildOnlineBook(olHits: List, gbHits: List): OnlineBook { + val olPrimary = olHits.firstOrNull { !it.title.isNullOrBlank() } + val gbPrimary = gbHits.firstOrNull { !it.title.isNullOrBlank() } + val primary = olPrimary ?: gbPrimary + val secondary = if (primary === olPrimary) gbPrimary else olPrimary + + val sources = buildSet { + if (olHits.isNotEmpty()) add(SearchSource.OPEN_LIBRARY) + if (gbHits.isNotEmpty()) add(SearchSource.GOOGLE_BOOKS) + } + return OnlineBook( + title = primary?.title, + subtitle = primary?.subtitle ?: secondary?.subtitle, + authors = primary?.authors?.ifEmpty { null } ?: secondary?.authors ?: emptyList(), + year = primary?.year ?: secondary?.year, + publisher = primary?.publisher ?: secondary?.publisher, + pageCount = primary?.pageCount ?: secondary?.pageCount, + coverUrl = primary?.coverUrl ?: secondary?.coverUrl, + isbn13 = resolveIsbn(olHits, gbHits), + sources = sources, + ) + } + + /** + * The ISBN rule (decided by the user, implemented exactly): an OL work whose + * ISBNs normalize to exactly one distinct ISBN-13 wins; else a GB volume set that + * carries exactly one distinct ISBN-13 wins; otherwise no ISBN is recorded at all + * rather than guessing among several editions. + */ + private fun resolveIsbn(olHits: List, gbHits: List): String? { + for (hit in olHits) { + val distinct = hit.isbn13s.distinct() + if (distinct.size == 1) return distinct.single() + } + return gbHits.flatMap { it.isbn13s }.distinct().singleOrNull() + } + + /** Plain union-find over node indices `0 until n` — path-halved finds, union by attaching root to root. */ + private class UnionFind(n: Int) { + private val parent = IntArray(n) { it } + + fun find(x: Int): Int { + var current = x + while (parent[current] != current) { + parent[current] = parent[parent[current]] + current = parent[current] + } + return current + } + + fun union(a: Int, b: Int) { + val rootA = find(a) + val rootB = find(b) + if (rootA != rootB) parent[rootA] = rootB + } + } +} diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchSourceResult.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchSourceResult.kt new file mode 100644 index 0000000..5039521 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/SearchSourceResult.kt @@ -0,0 +1,35 @@ +package org.modg.bookshelf.data.metadata + +import java.io.IOException + +/** + * Per-source title/author search outcome — [SearchSourceResult]'s counterpart to + * [SourceResult] for the library screen's online search (task: "Extend it to also + * search Open Library and Google Books"). Kept as its own sealed type rather than + * reusing [SourceResult] because a search's [Found] carries a list of [SearchHit], + * not one [BookMetadata] — but it follows the exact same three-way rule (SPEC.md + * "Book metadata lookup": a source that could not be reached must never be reported + * as "no results") and the exact same [FailureKind] classification, via + * [fromException]/[fromHttpCode] delegating straight to [SourceResult]'s so the two + * can never disagree about what a given exception or HTTP code means. + */ +sealed interface SearchSourceResult { + data class Found(val hits: List) : SearchSourceResult + + /** The source answered (2xx) and, in good faith, found nothing for this query. */ + data object NotFound : SearchSourceResult + + data class Failed( + val reason: String, + val kind: FailureKind = FailureKind.TRANSPORT, + val retryAfterMillis: Long? = null, + ) : SearchSourceResult + + companion object { + fun fromException(e: IOException): Failed = + SourceResult.fromException(e).let { Failed(it.reason, it.kind, it.retryAfterMillis) } + + fun fromHttpCode(code: Int, retryAfterHeader: String? = null): Failed = + SourceResult.fromHttpCode(code, retryAfterHeader).let { Failed(it.reason, it.kind, it.retryAfterMillis) } + } +} diff --git a/app/app/src/main/java/org/modg/bookshelf/data/metadata/TextNormalization.kt b/app/app/src/main/java/org/modg/bookshelf/data/metadata/TextNormalization.kt new file mode 100644 index 0000000..603d06f --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/data/metadata/TextNormalization.kt @@ -0,0 +1,48 @@ +package org.modg.bookshelf.data.metadata + +import java.text.Normalizer + +/** + * Shared text normalization for the two places that need to match books by loose + * text rather than exact fields: [org.modg.bookshelf.ui.library.LocalBookMatcher] + * (query tokens against an owned book) and [SearchResultMerger] (title+author + * identity across sources). One implementation so "hobbit" ~ "Hobbit" and + * "Brontë" ~ "bronte" mean the same thing in both places. + */ +object TextNormalization { + private val leadingArticles = setOf("the", "a", "an") + + /** + * Lowercase, diacritics stripped (NFD decompose + drop combining marks), every + * non-alphanumeric character turned into a space, whitespace collapsed. "J.R.R." + * becomes "j r r" — a space-separated token run, not a single run-together word. + */ + fun normalizeForMatching(raw: String): String { + val decomposed = Normalizer.normalize(raw, Normalizer.Form.NFD) + val noDiacritics = decomposed.replace(Regex("\\p{Mn}+"), "") + val spaced = noDiacritics.map { c -> if (c.isLetterOrDigit()) c else ' ' }.joinToString("") + return spaced.lowercase().trim().replace(Regex("\\s+"), " ") + } + + fun tokens(raw: String): List { + val normalized = normalizeForMatching(raw) + return if (normalized.isBlank()) emptyList() else normalized.split(' ') + } + + /** + * Identity title key for [SearchResultMerger]: subtitle (everything from the + * first ':' on) dropped BEFORE normalizing (the colon itself would otherwise + * become a space and the split point would be lost), then a leading "the/a/an" + * dropped so "The Hobbit" and "Hobbit" key the same. + */ + fun normalizedTitleKey(title: String?): String { + if (title.isNullOrBlank()) return "" + val words = tokens(title.substringBefore(':')) + val withoutArticle = if (words.firstOrNull() in leadingArticles) words.drop(1) else words + return withoutArticle.joinToString(" ") + } + + /** The last alphabetic token of the first author — "Joanne S. Williamson" -> "williamson". */ + fun authorSurnameKey(authors: List): String = + authors.firstOrNull()?.let { tokens(it).lastOrNull() }.orEmpty() +} diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookModels.kt b/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookModels.kt index c0080cb..eb1756c 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookModels.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookModels.kt @@ -4,11 +4,13 @@ import org.modg.bookshelf.data.metadata.IsbnUtils /** * The add-by-hand form's fields, independent of any particular entry point. - * [org.modg.bookshelf.ui.nav.Routes.addBook] pre-fills [title]/[isbn] today — - * from the scan screen's manual-ISBN dialog, or the library FAB with neither. - * A follow-up wave (online search) is expected to pre-fill more fields from a - * search hit; [fromRouteArgs] is the one place the route-args-to-draft mapping - * lives, so extending it later is a small, localized change. + * [org.modg.bookshelf.ui.nav.Routes.addBook] pre-fills whatever the entry point + * knows — the scan screen's manual-ISBN dialog and library FAB pass just + * title/isbn; wave 10's online-search save sheet's "Edit details" passes every + * field. [fromRouteArgs] is the one place the route-args-to-draft mapping lives. + * [coverSourceUrl] isn't shown anywhere in the form (no cover picker in this + * screen) but rides along so a cover carried in from a search hit still gets + * saved — see [AddBookViewModel.performSave]. */ data class BookDraft( val title: String = "", @@ -19,10 +21,30 @@ data class BookDraft( val pages: String = "", val isbn: String = "", val description: String = "", + val coverSourceUrl: String = "", ) { companion object { - fun fromRouteArgs(title: String?, isbn: String?): BookDraft = - BookDraft(title = title.orEmpty(), isbn = isbn.orEmpty()) + fun fromRouteArgs( + title: String? = null, + isbn: String? = null, + subtitle: String? = null, + authors: String? = null, + publisher: String? = null, + publishedDate: String? = null, + pages: String? = null, + description: String? = null, + coverSourceUrl: String? = null, + ): BookDraft = BookDraft( + title = title.orEmpty(), + subtitle = subtitle.orEmpty(), + authors = authors.orEmpty(), + publisher = publisher.orEmpty(), + publishedDate = publishedDate.orEmpty(), + pages = pages.orEmpty(), + isbn = isbn.orEmpty(), + description = description.orEmpty(), + coverSourceUrl = coverSourceUrl.orEmpty(), + ) } } diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookScreen.kt b/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookScreen.kt index 3a95d79..992d947 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookScreen.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookScreen.kt @@ -56,6 +56,13 @@ fun AddBookScreen( onBack: () -> Unit, onSaved: (bookId: String) -> Unit, container: AppContainer, + initialSubtitle: String? = null, + initialAuthors: String? = null, + initialPublisher: String? = null, + initialPublishedDate: String? = null, + initialPages: String? = null, + initialDescription: String? = null, + initialCoverSourceUrl: String? = null, ) { val viewModel: AddBookViewModel = viewModel( factory = viewModelFactory { @@ -64,7 +71,17 @@ fun AddBookScreen( bookRepository = container.bookRepository, locationRepository = container.locationRepository, settingsStore = container.settingsStore, - initialDraft = BookDraft.fromRouteArgs(initialTitle, initialIsbn), + initialDraft = BookDraft.fromRouteArgs( + title = initialTitle, + isbn = initialIsbn, + subtitle = initialSubtitle, + authors = initialAuthors, + publisher = initialPublisher, + publishedDate = initialPublishedDate, + pages = initialPages, + description = initialDescription, + coverSourceUrl = initialCoverSourceUrl, + ), ) } }, diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookViewModel.kt b/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookViewModel.kt index cf39f57..96f8dbc 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookViewModel.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/add/AddBookViewModel.kt @@ -197,6 +197,7 @@ class AddBookViewModel( publishedDate = draft.publishedDate.trim().ifEmpty { null }, pageCount = pageCount, description = draft.description.trim().ifEmpty { null }, + coverSourceUrl = draft.coverSourceUrl.trim().ifEmpty { null }, shelfId = form.selectedShelfId, ) rememberShelf(form.selectedShelfId) diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryModels.kt b/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryModels.kt index fc379ae..a819e97 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryModels.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryModels.kt @@ -1,6 +1,8 @@ package org.modg.bookshelf.ui.library import org.modg.bookshelf.data.local.BookEntity +import org.modg.bookshelf.data.metadata.BookMetadata +import org.modg.bookshelf.data.metadata.OnlineBook import org.modg.bookshelf.data.repo.decodeAuthors /** SPEC.md library screen: "sort title/author/added". */ @@ -52,3 +54,87 @@ object LibraryFilterLogic { } } } + +/** + * Online search state (task: library search "Extend it to also search Open Library + * and Google Books"). Lives in [LibraryViewModel] so it survives rotation. Search + * runs ONLY on submit (IME action / the explicit row), never per keystroke — decided + * by the user, see the task brief on Google Books' 1,000/day quota. + */ +sealed interface OnlineSearchState { + /** No search has run for the current query yet — the prompt row is what's shown. */ + data object NotSearched : OnlineSearchState + + data class Searching(val query: String) : OnlineSearchState + + /** + * [inLibrary] is the FULL "In your library" set once a search has run — a superset + * of [LocalBookMatcher]'s plain text match, since [org.modg.bookshelf.data.metadata.SearchResultMerger] + * also pulls in an owned book reached only via matching one of the online hits (task: + * "an online hit that is a different edition of an owned book belongs in the library + * section"). [openLibraryFailure]/[googleBooksFailure] are that source's + * [org.modg.bookshelf.data.metadata.SourceResult.Failed.reason]-equivalent, null when + * that source answered (whether or not it had anything) — the UI keys off these to + * show a per-source failure line with Retry while still showing the other source's + * [online] results, and to tell "both failed" (retry affordance, never "no results") + * from an authoritative miss (both answered, nothing found anywhere). + */ + data class Done( + val query: String, + val inLibrary: List, + val online: List, + val openLibraryFailure: String?, + val googleBooksFailure: String?, + ) : OnlineSearchState +} + +/** The bottom sheet shown when an online result is tapped (task: "same idea as the scan screen's Found sheet"). */ +sealed interface OnlineSaveSheetState { + data object Hidden : OnlineSaveSheetState + + /** + * [enrichment] fills in once the background by-ISBN lookup (started the moment + * the sheet opens, only when [book] has an isbn13) completes — Save reads + * whatever's here at the moment it's tapped and never waits on it, per the task's + * "Save must NEVER wait for this". + */ + data class Shown( + val book: OnlineBook, + val selectedShelfId: String?, + val isSaving: Boolean = false, + val saveError: String? = null, + val enrichment: BookMetadata? = null, + ) : OnlineSaveSheetState +} + +/** + * The same OL-then-GB-then-enrichment field-fill precedence, used by BOTH + * [LibraryViewModel.performSaveOnlineResult] (what gets persisted) and + * [org.modg.bookshelf.ui.nav.BookshelfNavHost]'s "Edit details" route builder + * (what pre-fills the add-by-hand form) — one place so the two can't drift apart. + */ +data class OnlineBookDraftFields( + val title: String, + val subtitle: String?, + val authors: List, + val publisher: String?, + val publishedDate: String?, + val pageCount: Int?, + val isbn13: String?, + val isbn10: String?, + val description: String?, + val coverUrl: String?, +) + +fun resolveOnlineBookFields(book: OnlineBook, enrichment: BookMetadata?): OnlineBookDraftFields = OnlineBookDraftFields( + title = book.title?.ifBlank { null } ?: enrichment?.title?.ifBlank { null } ?: "Untitled", + subtitle = book.subtitle ?: enrichment?.subtitle, + authors = book.authors.ifEmpty { enrichment?.authors.orEmpty() }, + publisher = book.publisher ?: enrichment?.publisher, + publishedDate = enrichment?.publishedDate ?: book.year?.toString(), + pageCount = book.pageCount ?: enrichment?.pageCount, + isbn13 = book.isbn13 ?: enrichment?.isbn13, + isbn10 = enrichment?.isbn10, + description = enrichment?.description, + coverUrl = book.coverUrl ?: enrichment?.coverUrl, +) diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryScreen.kt b/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryScreen.kt index cf1d5b5..8ced445 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryScreen.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryScreen.kt @@ -1,18 +1,26 @@ package org.modg.bookshelf.ui.library import androidx.compose.foundation.clickable +import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.grid.GridCells import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.height +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.text.KeyboardActions +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.outlined.Clear import androidx.compose.material.icons.outlined.EditNote @@ -21,6 +29,7 @@ import androidx.compose.material.icons.outlined.QrCodeScanner import androidx.compose.material.icons.outlined.Search import androidx.compose.material.icons.outlined.Settings import androidx.compose.material.icons.outlined.Sort +import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.DropdownMenu import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.ExperimentalMaterial3Api @@ -28,10 +37,13 @@ import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.ModalBottomSheet import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.SmallFloatingActionButton import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.pulltorefresh.PullToRefreshBox +import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue @@ -41,6 +53,7 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.painterResource +import androidx.compose.ui.text.input.ImeAction import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp @@ -52,20 +65,30 @@ import org.modg.bookshelf.R import org.modg.bookshelf.data.local.BookEntity 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.OnlineBook import org.modg.bookshelf.data.repo.decodeAuthors import org.modg.bookshelf.ui.components.BookCover import org.modg.bookshelf.ui.components.EmptyState import org.modg.bookshelf.ui.components.BookshelfScaffold +import org.modg.bookshelf.ui.components.GoldDivider import org.modg.bookshelf.ui.components.PaperSurface import org.modg.bookshelf.ui.components.PrimaryButton import org.modg.bookshelf.ui.components.SecondaryButton import org.modg.bookshelf.ui.components.SyncStatus import org.modg.bookshelf.ui.components.SyncStatusBar +import org.modg.bookshelf.ui.scan.ShelfPicker /** * SPEC.md "library" screen: adaptive cover grid, search, filter, sort, empty * state, FAB to scan, sync status line. Covers carry the color; card chrome * stays minimal (cover + two lines of text) on purpose. + * + * Wave 10 (online search) extends the search bar: a blank query keeps the + * plain cover grid, but any query switches this screen into a sectioned + * search-results view — "In your library" (instant, local, as-you-type) above + * "Online" (Open Library + Google Books, submit-only) — see + * [LibrarySearchResultsContent]. */ @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -77,6 +100,8 @@ fun LibraryScreen( onLocationsClick: () -> Unit, onSettingsClick: () -> Unit, container: AppContainer, + onEnterByHandWithQuery: (String) -> Unit = { onAddByHandClick() }, + onEditOnlineResult: (OnlineBook, BookMetadata?) -> Unit = { _, _ -> }, ) { val viewModel: LibraryViewModel = viewModel( // A fresh VM per distinct shelf filter, so tapping a different shelf @@ -89,6 +114,8 @@ fun LibraryScreen( locationRepository = container.locationRepository, syncEngine = container.syncEngine, settingsStore = container.settingsStore, + bookSearchRepository = container.bookSearchRepository, + metadataRepository = container.metadataRepository, initialShelfId = shelfIdFilter, ) } @@ -99,6 +126,9 @@ fun LibraryScreen( val query by viewModel.query.collectAsState() val sortOption by viewModel.sortOption.collectAsState() val filter by viewModel.filter.collectAsState() + val onlineSearch by viewModel.onlineSearch.collectAsState() + val saveSheet by viewModel.saveSheet.collectAsState() + val recentShelfId by viewModel.recentShelfId.collectAsState() BookshelfScaffold( title = "Bookshelf", @@ -141,6 +171,7 @@ fun LibraryScreen( LibraryToolbar( query = query, onQueryChange = viewModel::onQueryChange, + onSearchSubmit = viewModel::submitOnlineSearch, sortOption = sortOption, onSortOptionChange = viewModel::onSortOptionChange, filter = filter, @@ -150,6 +181,16 @@ fun LibraryScreen( ) when { + query.isNotBlank() -> LibrarySearchResultsContent( + query = query, + localMatches = uiState.books, + onlineSearch = onlineSearch, + onBookClick = onBookClick, + onSubmitOnlineSearch = viewModel::submitOnlineSearch, + onRetryOnlineSearch = viewModel::retryOnlineSearch, + onOnlineResultClick = viewModel::onOnlineResultTapped, + onEnterByHand = { onEnterByHandWithQuery(query.trim()) }, + ) !uiState.hasAnyBooks -> EmptyState( title = "Your shelves are empty", message = "Scan a barcode to add your first book.", @@ -180,6 +221,25 @@ fun LibraryScreen( } } } + + val sheetState = saveSheet + if (sheetState is OnlineSaveSheetState.Shown) { + ModalBottomSheet( + onDismissRequest = viewModel::dismissSaveSheet, + sheetState = rememberModalBottomSheetState(), + ) { + OnlineResultSaveSheet( + state = sheetState, + bookcases = uiState.bookcases, + shelves = uiState.shelves, + recentShelfId = recentShelfId, + onShelfSelected = viewModel::onSaveSheetShelfSelected, + onSave = viewModel::saveOnlineResult, + onEditDetails = { onEditOnlineResult(sheetState.book, sheetState.enrichment) }, + onSkip = viewModel::dismissSaveSheet, + ) + } + } } @Composable @@ -192,6 +252,7 @@ internal fun LibraryToolbar( onFilterChange: (LibraryFilter) -> Unit, bookcases: List, shelves: List, + onSearchSubmit: () -> Unit = {}, ) { var filterMenuExpanded by remember { mutableStateOf(false) } var sortMenuExpanded by remember { mutableStateOf(false) } @@ -217,6 +278,10 @@ internal fun LibraryToolbar( } } }, + // Wave 10: IME Search triggers the online search (task: "ONLY ON SUBMIT + // (IME Search action, or an explicit button) — never per keystroke"). + keyboardOptions = KeyboardOptions(imeAction = ImeAction.Search), + keyboardActions = KeyboardActions(onSearch = { onSearchSubmit() }), ) Column { @@ -288,8 +353,8 @@ internal fun LibraryToolbar( } @Composable -internal fun LibraryBookCard(book: BookEntity, onClick: () -> Unit) { - Column(modifier = Modifier.fillMaxWidth().clickable(onClick = onClick)) { +internal fun LibraryBookCard(book: BookEntity, onClick: () -> Unit, modifier: Modifier = Modifier) { + Column(modifier = modifier.fillMaxWidth().clickable(onClick = onClick)) { BookCover( coverUrl = book.coverUrl ?: book.coverSourceUrl, contentDescription = book.title, @@ -321,3 +386,303 @@ internal fun LibraryBookCard(book: BookEntity, onClick: () -> Unit) { } } } + +// --- Wave 10: online search results --- + +/** + * The sectioned view shown whenever the search bar carries a non-blank query — + * "In your library" (instant local matches, existing card style, horizontally + * scrollable so it never fights the outer vertical scroll) above "Online" + * (Open Library + Google Books, submit-only, as rows). Split out as its own + * stateless composable — fed real state, not hand-rolled — so Paparazzi + * renders the actual screen (see HANDOFF.md wave-6 "soft spots"). + */ +@Composable +internal fun LibrarySearchResultsContent( + query: String, + localMatches: List, + onlineSearch: OnlineSearchState, + onBookClick: (String) -> Unit, + onSubmitOnlineSearch: () -> Unit, + onRetryOnlineSearch: () -> Unit, + onOnlineResultClick: (OnlineBook) -> Unit, + onEnterByHand: () -> Unit, + modifier: Modifier = Modifier, +) { + Column( + modifier = modifier + .fillMaxSize() + .verticalScroll(rememberScrollState()) + .padding(bottom = 24.dp), + ) { + SearchSectionHeader(text = "In your library") + if (localMatches.isEmpty()) { + // The header stays up even with nothing to show — task: the user must be + // able to tell "not owned" from "didn't look". + Text( + text = "No matches in your library", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp, vertical = 4.dp), + ) + } else { + Row( + modifier = Modifier + .fillMaxWidth() + .horizontalScroll(rememberScrollState()) + .padding(horizontal = 16.dp), + horizontalArrangement = Arrangement.spacedBy(12.dp), + ) { + localMatches.forEach { book -> + LibraryBookCard(book = book, onClick = { onBookClick(book.id) }, modifier = Modifier.width(112.dp)) + } + } + } + + GoldDivider(modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp)) + SearchSectionHeader(text = "Online") + OnlineSearchSection( + query = query, + state = onlineSearch, + onSubmit = onSubmitOnlineSearch, + onRetry = onRetryOnlineSearch, + onResultClick = onOnlineResultClick, + ) + + TextButton(onClick = onEnterByHand, modifier = Modifier.padding(horizontal = 12.dp, vertical = 8.dp)) { + Text("Can't find it? Enter it by hand") + } + } +} + +@Composable +private fun SearchSectionHeader(text: String) { + Text( + text = text, + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = 16.dp, bottom = 8.dp), + ) +} + +/** Minimum characters before the online-search prompt/submit is offered at all. */ +internal const val OnlineSearchMinQueryLength = 2 + +@Composable +private fun OnlineSearchSection( + query: String, + state: OnlineSearchState, + onSubmit: () -> Unit, + onRetry: () -> Unit, + onResultClick: (OnlineBook) -> Unit, +) { + val trimmed = query.trim() + when (state) { + is OnlineSearchState.NotSearched -> { + if (trimmed.length >= OnlineSearchMinQueryLength) { + SearchPromptRow(query = trimmed, onClick = onSubmit) + } else { + Text( + text = "Keep typing to search Open Library and Google Books.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp, vertical = 4.dp), + ) + } + } + is OnlineSearchState.Searching -> { + Row( + modifier = Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 16.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + CircularProgressIndicator(modifier = Modifier.size(20.dp), strokeWidth = 2.dp) + Text( + text = "Searching Open Library and Google Books…", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(start = 12.dp), + ) + } + } + is OnlineSearchState.Done -> OnlineSearchDoneSection(state = state, onRetry = onRetry, onResultClick = onResultClick) + } +} + +@Composable +private fun OnlineSearchDoneSection(state: OnlineSearchState.Done, onRetry: () -> Unit, onResultClick: (OnlineBook) -> Unit) { + val bothFailed = state.openLibraryFailure != null && state.googleBooksFailure != null + if (bothFailed) { + // Task: "both failed -> a retry affordance that does NOT say 'no results'". + Column(modifier = Modifier.padding(horizontal = 16.dp, vertical = 4.dp)) { + Text( + text = "Couldn't search online", + style = MaterialTheme.typography.titleSmall, + color = MaterialTheme.colorScheme.onSurface, + ) + Text( + text = "This doesn't mean there are no results — neither source could be reached.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 2.dp), + ) + PrimaryButton(text = "Retry", onClick = onRetry, modifier = Modifier.padding(top = 8.dp)) + } + return + } + + state.openLibraryFailure?.let { FailureLine(message = "Open Library couldn't be reached — $it", onRetry = onRetry) } + state.googleBooksFailure?.let { FailureLine(message = "Google Books couldn't be reached — $it", onRetry = onRetry) } + + if (state.online.isEmpty()) { + Text( + text = "No online results for \"${state.query}\".", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp, vertical = 4.dp), + ) + } else { + Column { + state.online.forEach { book -> OnlineResultRow(book = book, onClick = { onResultClick(book) }) } + } + } +} + +@Composable +private fun SearchPromptRow(query: String, onClick: () -> Unit) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable(onClick = onClick) + .padding(horizontal = 16.dp, vertical = 14.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon(Icons.Outlined.Search, contentDescription = null, tint = MaterialTheme.colorScheme.primary) + Text( + text = "Search Open Library & Google Books for \"$query\"", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.primary, + modifier = Modifier.padding(start = 12.dp), + ) + } +} + +@Composable +private fun FailureLine(message: String, onRetry: () -> Unit) { + Row( + modifier = Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 4.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = message, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.weight(1f), + ) + TextButton(onClick = onRetry) { Text("Retry") } + } +} + +@Composable +internal fun OnlineResultRow(book: OnlineBook, onClick: () -> Unit) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable(onClick = onClick) + .padding(horizontal = 16.dp, vertical = 8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + BookCover(coverUrl = book.coverUrl, contentDescription = book.title, modifier = Modifier.width(44.dp)) + Column(modifier = Modifier.padding(start = 12.dp).weight(1f)) { + Text( + text = book.title ?: "Untitled", + style = MaterialTheme.typography.titleSmall, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + ) + val subtitleParts = listOfNotNull( + book.authors.takeIf { it.isNotEmpty() }?.joinToString(", "), + book.year?.toString(), + ) + if (subtitleParts.isNotEmpty()) { + Text( + text = subtitleParts.joinToString(" • "), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.padding(top = 2.dp), + ) + } + } + } +} + +/** + * The save sheet for an online result — task: "same idea as the scan screen's + * Found sheet: book summary + shelf picker + Save / Skip", plus "Edit details". + */ +@Composable +internal fun OnlineResultSaveSheet( + state: OnlineSaveSheetState.Shown, + bookcases: List, + shelves: List, + recentShelfId: String?, + onShelfSelected: (String?) -> Unit, + onSave: () -> Unit, + onEditDetails: () -> Unit, + onSkip: () -> Unit, +) { + val book = state.book + Column(modifier = Modifier.fillMaxWidth().padding(16.dp)) { + Box( + modifier = Modifier + .align(Alignment.CenterHorizontally) + .size(width = 100.dp, height = 150.dp), + ) { + BookCover(coverUrl = book.coverUrl, contentDescription = book.title, modifier = Modifier.fillMaxSize()) + } + Text( + text = book.title ?: "Untitled", + style = MaterialTheme.typography.titleLarge, + modifier = Modifier.padding(top = 12.dp), + ) + if (book.authors.isNotEmpty()) { + Text(text = book.authors.joinToString(", "), style = MaterialTheme.typography.bodyLarge) + } + val meta = listOfNotNull(book.year?.toString(), book.isbn13) + if (meta.isNotEmpty()) { + Text( + text = meta.joinToString(" • "), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 2.dp), + ) + } + if (state.saveError != null) { + Text( + text = "Couldn't save: ${state.saveError}", + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.padding(top = 8.dp), + ) + } + ShelfPicker( + bookcases = bookcases, + shelves = shelves, + selectedShelfId = state.selectedShelfId, + recentShelfId = recentShelfId, + onShelfSelected = onShelfSelected, + ) + Row(modifier = Modifier.fillMaxWidth().padding(top = 16.dp), horizontalArrangement = Arrangement.spacedBy(12.dp)) { + SecondaryButton(text = "Skip", onClick = onSkip, modifier = Modifier.weight(1f)) + SecondaryButton(text = "Edit details", onClick = onEditDetails, modifier = Modifier.weight(1f)) + } + PrimaryButton( + text = if (state.isSaving) "Saving…" else "Save", + enabled = !state.isSaving, + onClick = onSave, + modifier = Modifier.fillMaxWidth().padding(top = 12.dp), + ) + } +} diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryViewModel.kt b/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryViewModel.kt index bc43bd6..22b7e20 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryViewModel.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/library/LibraryViewModel.kt @@ -2,19 +2,29 @@ package org.modg.bookshelf.ui.library import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import org.modg.bookshelf.data.local.BookEntity import org.modg.bookshelf.data.local.BookcaseEntity import org.modg.bookshelf.data.local.ShelfEntity +import org.modg.bookshelf.data.metadata.BookSearchRepository +import org.modg.bookshelf.data.metadata.IsbnUtils +import org.modg.bookshelf.data.metadata.LookupResult +import org.modg.bookshelf.data.metadata.MetadataRepository +import org.modg.bookshelf.data.metadata.OnlineBook +import org.modg.bookshelf.data.metadata.SearchHit +import org.modg.bookshelf.data.metadata.SearchResultMerger +import org.modg.bookshelf.data.metadata.SearchSourceResult import org.modg.bookshelf.data.prefs.SettingsStore import org.modg.bookshelf.data.repo.BookRepository import org.modg.bookshelf.data.repo.LocationRepository @@ -37,6 +47,8 @@ class LibraryViewModel( private val locationRepository: LocationRepository, private val syncEngine: SyncEngine, private val settingsStore: SettingsStore, + private val bookSearchRepository: BookSearchRepository, + private val metadataRepository: MetadataRepository, initialShelfId: String?, ) : ViewModel() { @@ -65,12 +77,26 @@ class LibraryViewModel( .map { shelves -> shelves.groupBy({ it.bookcaseId }, { it.id }).mapValues { it.value.toSet() } } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyMap()) - private val hasAnyBooks: StateFlow = bookRepository.observeAll() + /** Every non-deleted owned book — the base [allBooks] both the grid and the search/merge logic read from. */ + private val allBooks: StateFlow> = bookRepository.observeAll() + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) + + private val hasAnyBooks: StateFlow = allBooks .map { it.isNotEmpty() } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), true) + /** + * Instant, offline, as-you-type local match (task: "Own library filters AS THEY + * TYPE"). [LocalBookMatcher] replaces [org.modg.bookshelf.data.local.BookDao.search]'s + * single whole-string LIKE for this screen, which couldn't match "hobbit tolkien" + * at all. A blank query means "browse everything", not "match nothing". + */ + private val localMatches: StateFlow> = combine(_query, allBooks) { queryValue, books -> + if (queryValue.isBlank()) books else LocalBookMatcher.match(books, queryValue) + }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) + private val filteredSortedBooks: StateFlow> = combine( - _query.flatMapLatest { bookRepository.search(it) }, + localMatches, _filter, _sortOption, shelfIdsByBookcase, @@ -78,6 +104,62 @@ class LibraryViewModel( LibrarySort.sort(LibraryFilterLogic.apply(books, filterValue, shelfMap), sort) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) + // --- online search (task: extend the search bar to also search Open Library/Google Books) --- + + private sealed interface OnlineSearchPhase { + data object NotSearched : OnlineSearchPhase + data class Searching(val query: String) : OnlineSearchPhase + data class Done(val hits: RawHits) : OnlineSearchPhase + } + + private data class RawHits( + val query: String, + val openLibrary: List, + val googleBooks: List, + val openLibraryFailure: String?, + val googleBooksFailure: String?, + ) + + private val _onlineSearchPhase = MutableStateFlow(OnlineSearchPhase.NotSearched) + private var onlineSearchJob: Job? = null + + /** + * Recomputed from [allBooks] on every change, not frozen at search time — so a + * book saved from the online section's save sheet (or from anywhere else, e.g. + * sync) immediately migrates itself from "Online" into "In your library" via the + * normal Room flow, with no hand-written "move it over" step. + */ + val onlineSearch: StateFlow = combine(_onlineSearchPhase, allBooks) { phase, books -> + when (phase) { + is OnlineSearchPhase.NotSearched -> OnlineSearchState.NotSearched + is OnlineSearchPhase.Searching -> OnlineSearchState.Searching(phase.query) + is OnlineSearchPhase.Done -> buildDoneState(phase.hits, books) + } + }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), OnlineSearchState.NotSearched) + + private fun buildDoneState(raw: RawHits, books: List): OnlineSearchState.Done { + val onlineIsbns = (raw.openLibrary.asSequence() + raw.googleBooks.asSequence()) + .flatMap { it.isbn13s } + .toSet() + val isbnLinked = books.filter { book -> bookIsbn13s(book).any { it in onlineIsbns } } + val candidates = (LocalBookMatcher.match(books, raw.query) + isbnLinked).distinctBy { it.id } + val merged = SearchResultMerger.merge(candidates, raw.openLibrary, raw.googleBooks) + return OnlineSearchState.Done(raw.query, merged.inLibrary, merged.online, raw.openLibraryFailure, raw.googleBooksFailure) + } + + private fun bookIsbn13s(book: BookEntity): List = + listOfNotNull(book.isbn13?.let(IsbnUtils::toIsbn13), book.isbn10?.let(IsbnUtils::toIsbn13)) + + // --- online result save sheet --- + + private val _saveSheet = MutableStateFlow(OnlineSaveSheetState.Hidden) + val saveSheet: StateFlow = _saveSheet.asStateFlow() + private var enrichmentJob: Job? = null + + /** Surfaced as the save sheet's shelf picker default, same as the scan/add-by-hand sheets. */ + val recentShelfId: StateFlow = settingsStore.lastShelfId + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null) + private val syncBar: StateFlow = combine( settingsStore.lastSyncTime, _isSyncing, @@ -104,8 +186,16 @@ class LibraryViewModel( refreshSync() } + /** + * Editing the query after a submit clears the online section and cancels any + * in-flight search (task: stale results for a different query are misleading). + */ fun onQueryChange(value: String) { _query.value = value + if (_onlineSearchPhase.value !is OnlineSearchPhase.NotSearched) { + onlineSearchJob?.cancel() + _onlineSearchPhase.value = OnlineSearchPhase.NotSearched + } } fun onSortOptionChange(option: LibrarySortOption) { @@ -116,6 +206,125 @@ class LibraryViewModel( _filter.value = filter } + /** + * Runs on submit (IME Search action / the explicit prompt row) ONLY — never per + * keystroke, per the task's decision on Google Books' 1,000/day quota and Open + * Library's latency. [org.modg.bookshelf.ui.library.LibraryScreen]'s minimum + * query length (2) is enforced here too, so a stray programmatic call can't + * burn quota on a near-empty query either. + */ + fun submitOnlineSearch() { + val trimmed = _query.value.trim() + if (trimmed.length < 2) return + onlineSearchJob?.cancel() + onlineSearchJob = viewModelScope.launch { performOnlineSearch(trimmed) } + } + + /** Retries both sources — used by the "both failed" retry affordance and each per-source failure line. */ + fun retryOnlineSearch() = submitOnlineSearch() + + /** + * Internal (not private) so tests can await it directly, same reasoning as + * [org.modg.bookshelf.ui.scan.ScanViewModel.runLookup]. [BookSearchRepository]'s + * clients already never throw (see their KDoc), but this guard is the same + * belt-and-braces backstop that class documents. + */ + internal suspend fun performOnlineSearch(query: String) { + _onlineSearchPhase.value = OnlineSearchPhase.Searching(query) + val raw = try { + val results = bookSearchRepository.search(query) + RawHits( + query = query, + openLibrary = (results.openLibrary as? SearchSourceResult.Found)?.hits.orEmpty(), + googleBooks = (results.googleBooks as? SearchSourceResult.Found)?.hits.orEmpty(), + openLibraryFailure = (results.openLibrary as? SearchSourceResult.Failed)?.reason, + googleBooksFailure = (results.googleBooks as? SearchSourceResult.Failed)?.reason, + ) + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + val reason = "unexpected: ${e.javaClass.simpleName}" + RawHits(query, emptyList(), emptyList(), reason, reason) + } + _onlineSearchPhase.value = OnlineSearchPhase.Done(raw) + } + + /** + * Opens the save sheet and, when [book] carries an ISBN, kicks off the same + * by-ISBN enrichment lookup the scan flow uses in the background — Save never + * waits on it (see [performSaveOnlineResult]); it's cancelled when the sheet closes. + */ + fun onOnlineResultTapped(book: OnlineBook) { + enrichmentJob?.cancel() + _saveSheet.value = OnlineSaveSheetState.Shown(book = book, selectedShelfId = recentShelfId.value) + val isbn13 = book.isbn13 ?: return + enrichmentJob = viewModelScope.launch { + val result = metadataRepository.lookup(isbn13) + if (result is LookupResult.Found) { + _saveSheet.update { state -> + if (state is OnlineSaveSheetState.Shown && state.book == book) state.copy(enrichment = result.metadata) else state + } + } + } + } + + fun onSaveSheetShelfSelected(shelfId: String?) { + _saveSheet.update { if (it is OnlineSaveSheetState.Shown) it.copy(selectedShelfId = shelfId) else it } + } + + fun dismissSaveSheet() { + enrichmentJob?.cancel() + _saveSheet.value = OnlineSaveSheetState.Hidden + } + + fun saveOnlineResult() { + viewModelScope.launch { performSaveOnlineResult() } + } + + /** + * Split out so tests can await it directly (same pattern as + * [org.modg.bookshelf.ui.add.AddBookViewModel.performSave]). Re-entrancy guarded + * here so a genuine double-tap can't create two books. Reads whatever + * [OnlineSaveSheetState.Shown.enrichment] happens to hold at this instant — it + * never awaits the enrichment lookup, per the task's "Save must NEVER wait for this". + */ + internal suspend fun performSaveOnlineResult(): String? { + val state = _saveSheet.value as? OnlineSaveSheetState.Shown ?: return null + if (state.isSaving) return null + _saveSheet.update { if (it is OnlineSaveSheetState.Shown) it.copy(isSaving = true, saveError = null) else it } + + val fields = resolveOnlineBookFields(state.book, state.enrichment) + return try { + val id = bookRepository.createBook( + title = fields.title, + subtitle = fields.subtitle, + authors = fields.authors, + isbn13 = fields.isbn13, + isbn10 = fields.isbn10, + publisher = fields.publisher, + publishedDate = fields.publishedDate, + pageCount = fields.pageCount, + description = fields.description, + coverSourceUrl = fields.coverUrl, + shelfId = state.selectedShelfId, + ) + rememberShelf(state.selectedShelfId) + enrichmentJob?.cancel() + _saveSheet.value = OnlineSaveSheetState.Hidden + id + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + _saveSheet.update { if (it is OnlineSaveSheetState.Shown) it.copy(isSaving = false, saveError = e.javaClass.simpleName) else it } + null + } + } + + /** "Not shelved" (null) must never overwrite the memory — it isn't a shelf. */ + private suspend fun rememberShelf(shelfId: String?) { + if (shelfId != null) settingsStore.setLastShelfId(shelfId) + } + /** Manual trigger — app start (see init) and pull-to-refresh, per SPEC "Sync design". */ fun refreshSync() { if (_isSyncing.value) return diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/library/LocalBookMatcher.kt b/app/app/src/main/java/org/modg/bookshelf/ui/library/LocalBookMatcher.kt new file mode 100644 index 0000000..c491bf6 --- /dev/null +++ b/app/app/src/main/java/org/modg/bookshelf/ui/library/LocalBookMatcher.kt @@ -0,0 +1,41 @@ +package org.modg.bookshelf.ui.library + +import org.modg.bookshelf.data.local.BookEntity +import org.modg.bookshelf.data.metadata.TextNormalization +import org.modg.bookshelf.data.repo.decodeAuthors + +/** + * The library search bar's instant, offline, as-you-type local filter (task: "Own + * library filters AS THEY TYPE"). Replaces [org.modg.bookshelf.data.local.BookDao.search]'s + * single whole-string LIKE for this screen — that matches nothing for "hobbit tolkien" + * because neither column contains that exact substring. A home library is hundreds to + * low thousands of books, so a pure Kotlin scan is fine and far more testable than SQL. + */ +object LocalBookMatcher { + + /** + * Normalizes [query] and every candidate field, splits the query on whitespace, + * and requires EVERY token to appear in at least one of title/subtitle/authors/ + * isbn13/isbn10 (different tokens may match different fields). Matching also + * checks a space-stripped form of each field so an initials-heavy name like + * "J.R.R. Tolkien" (normalized to "j r r tolkien", per [TextNormalization]) is + * still found by the squashed query token "jrr", not just "j" or "r". + */ + fun match(books: List, query: String): List { + val queryTokens = TextNormalization.tokens(query) + if (queryTokens.isEmpty()) return emptyList() + return books.filter { book -> matches(book, queryTokens) } + } + + private fun matches(book: BookEntity, queryTokens: List): Boolean { + val haystacks = buildList { + add(TextNormalization.normalizeForMatching(book.title)) + book.subtitle?.let { add(TextNormalization.normalizeForMatching(it)) } + decodeAuthors(book.authorsJson).forEach { add(TextNormalization.normalizeForMatching(it)) } + book.isbn13?.let { add(TextNormalization.normalizeForMatching(it)) } + book.isbn10?.let { add(TextNormalization.normalizeForMatching(it)) } + } + val squashed = haystacks.map { it.replace(" ", "") } + return queryTokens.all { token -> haystacks.any { it.contains(token) } || squashed.any { it.contains(token) } } + } +} diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/nav/BookshelfNavHost.kt b/app/app/src/main/java/org/modg/bookshelf/ui/nav/BookshelfNavHost.kt index fd7f6a5..6b64b56 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/nav/BookshelfNavHost.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/nav/BookshelfNavHost.kt @@ -11,6 +11,7 @@ import org.modg.bookshelf.AppContainer import org.modg.bookshelf.ui.add.AddBookScreen import org.modg.bookshelf.ui.detail.DetailScreen import org.modg.bookshelf.ui.library.LibraryScreen +import org.modg.bookshelf.ui.library.resolveOnlineBookFields import org.modg.bookshelf.ui.locations.LocationsScreen import org.modg.bookshelf.ui.scan.ScanScreen import org.modg.bookshelf.ui.settings.SettingsScreen @@ -57,6 +58,28 @@ fun BookshelfNavHost( onLocationsClick = { navController.navigate(Routes.LOCATIONS) }, onSettingsClick = { navController.navigate(Routes.SETTINGS) }, container = container, + // Wave 10: "Can't find it? Enter it by hand" under the online results — + // pre-fills the add-by-hand form's title with the search query. + onEnterByHandWithQuery = { query -> navController.navigate(Routes.addBook(title = query)) }, + // Wave 10: the online-result save sheet's "Edit details" — carries the + // whole resolved draft (see resolveOnlineBookFields) through to the + // add-by-hand form via its per-field route args. + onEditOnlineResult = { book, enrichment -> + val fields = resolveOnlineBookFields(book, enrichment) + navController.navigate( + Routes.addBook( + title = fields.title, + subtitle = fields.subtitle, + authors = fields.authors.joinToString(", "), + publisher = fields.publisher, + publishedDate = fields.publishedDate, + pages = fields.pageCount?.toString(), + isbn = fields.isbn13 ?: fields.isbn10, + description = fields.description, + coverSourceUrl = fields.coverUrl, + ), + ) + }, ) } composable( @@ -79,21 +102,34 @@ fun BookshelfNavHost( composable( route = Routes.ADD_BOOK, arguments = listOf( - navArgument(Routes.ADD_BOOK_TITLE_ARG) { + Routes.ADD_BOOK_TITLE_ARG, + Routes.ADD_BOOK_ISBN_ARG, + Routes.ADD_BOOK_SUBTITLE_ARG, + Routes.ADD_BOOK_AUTHORS_ARG, + Routes.ADD_BOOK_PUBLISHER_ARG, + Routes.ADD_BOOK_PUBLISHED_DATE_ARG, + Routes.ADD_BOOK_PAGES_ARG, + Routes.ADD_BOOK_DESCRIPTION_ARG, + Routes.ADD_BOOK_COVER_URL_ARG, + ).map { argName -> + navArgument(argName) { type = NavType.StringType nullable = true defaultValue = null - }, - navArgument(Routes.ADD_BOOK_ISBN_ARG) { - type = NavType.StringType - nullable = true - defaultValue = null - }, - ), + } + }, ) { backStackEntry -> + fun arg(name: String) = backStackEntry.arguments?.getString(name) AddBookScreen( - initialTitle = backStackEntry.arguments?.getString(Routes.ADD_BOOK_TITLE_ARG), - initialIsbn = backStackEntry.arguments?.getString(Routes.ADD_BOOK_ISBN_ARG), + initialTitle = arg(Routes.ADD_BOOK_TITLE_ARG), + initialIsbn = arg(Routes.ADD_BOOK_ISBN_ARG), + initialSubtitle = arg(Routes.ADD_BOOK_SUBTITLE_ARG), + initialAuthors = arg(Routes.ADD_BOOK_AUTHORS_ARG), + initialPublisher = arg(Routes.ADD_BOOK_PUBLISHER_ARG), + initialPublishedDate = arg(Routes.ADD_BOOK_PUBLISHED_DATE_ARG), + initialPages = arg(Routes.ADD_BOOK_PAGES_ARG), + initialDescription = arg(Routes.ADD_BOOK_DESCRIPTION_ARG), + initialCoverSourceUrl = arg(Routes.ADD_BOOK_COVER_URL_ARG), onBack = { navController.popBackStack() }, onSaved = { bookId -> // Removes the add screen from the back stack, per the task's "Save" diff --git a/app/app/src/main/java/org/modg/bookshelf/ui/nav/Routes.kt b/app/app/src/main/java/org/modg/bookshelf/ui/nav/Routes.kt index 12b6f41..eb8cb27 100644 --- a/app/app/src/main/java/org/modg/bookshelf/ui/nav/Routes.kt +++ b/app/app/src/main/java/org/modg/bookshelf/ui/nav/Routes.kt @@ -20,21 +20,45 @@ object Routes { const val ADD_BOOK_TITLE_ARG = "title" const val ADD_BOOK_ISBN_ARG = "isbn" + + /** + * Wave 10 (online search): "Edit details" on an online search hit needs to carry + * the whole draft, not just title/isbn, to the add-by-hand screen. Extends the + * wave-9 route with the same per-field nullable-query-arg pattern rather than a + * new serialized-blob argument, so every arg keeps the identical encode/decode + * story [ADD_BOOK_TITLE_ARG]/[ADD_BOOK_ISBN_ARG] already have (and RoutesTest + * already covers). + */ + const val ADD_BOOK_SUBTITLE_ARG = "subtitle" + const val ADD_BOOK_AUTHORS_ARG = "authors" + const val ADD_BOOK_PUBLISHER_ARG = "publisher" + const val ADD_BOOK_PUBLISHED_DATE_ARG = "publishedDate" + const val ADD_BOOK_PAGES_ARG = "pages" + const val ADD_BOOK_DESCRIPTION_ARG = "description" + const val ADD_BOOK_COVER_URL_ARG = "coverUrl" private const val ADD_BOOK_BASE = "add" /** - * Both args are optional (wave 9: "add a book by hand" with nothing pre-filled) + * Every arg is optional (wave 9: "add a book by hand" with nothing pre-filled) * — nullable/defaultValue-null args, same pattern [LIBRARY_WITH_SHELF] uses so - * this one route also matches the bare "add" entry with neither supplied. + * this one route also matches the bare "add" entry with none supplied. */ - const val ADD_BOOK = "$ADD_BOOK_BASE?$ADD_BOOK_TITLE_ARG={$ADD_BOOK_TITLE_ARG}&$ADD_BOOK_ISBN_ARG={$ADD_BOOK_ISBN_ARG}" + const val ADD_BOOK = "$ADD_BOOK_BASE?$ADD_BOOK_TITLE_ARG={$ADD_BOOK_TITLE_ARG}" + + "&$ADD_BOOK_ISBN_ARG={$ADD_BOOK_ISBN_ARG}" + + "&$ADD_BOOK_SUBTITLE_ARG={$ADD_BOOK_SUBTITLE_ARG}" + + "&$ADD_BOOK_AUTHORS_ARG={$ADD_BOOK_AUTHORS_ARG}" + + "&$ADD_BOOK_PUBLISHER_ARG={$ADD_BOOK_PUBLISHER_ARG}" + + "&$ADD_BOOK_PUBLISHED_DATE_ARG={$ADD_BOOK_PUBLISHED_DATE_ARG}" + + "&$ADD_BOOK_PAGES_ARG={$ADD_BOOK_PAGES_ARG}" + + "&$ADD_BOOK_DESCRIPTION_ARG={$ADD_BOOK_DESCRIPTION_ARG}" + + "&$ADD_BOOK_COVER_URL_ARG={$ADD_BOOK_COVER_URL_ARG}" fun libraryFilteredByShelf(shelfId: String) = "library?shelfId=$shelfId" fun detail(bookId: String) = "detail/$bookId" /** - * Builds the add-by-hand route, percent-encoding [title]/[isbn] so values - * containing `&`, `?`, `/`, `#` etc. (titles routinely do) survive as a single + * Builds the add-by-hand route, percent-encoding every arg so values containing + * `&`, `?`, `/`, `#` etc. (titles/descriptions routinely do) survive as a single * query value instead of corrupting the route string. [Uri.encode] rather than * [java.net.URLEncoder] because the latter encodes space as `+`, which * Navigation Compose's Uri-pattern argument matching does NOT decode back to @@ -44,10 +68,27 @@ object Routes { * nothing to pre-fill (e.g. the plain library FAB) gets exactly the bare * "add" route wave 9's [org.modg.bookshelf.ui.add.AddBookScreen] expects. */ - fun addBook(title: String? = null, isbn: String? = null): String { + fun addBook( + title: String? = null, + isbn: String? = null, + subtitle: String? = null, + authors: String? = null, + publisher: String? = null, + publishedDate: String? = null, + pages: String? = null, + description: String? = null, + coverSourceUrl: String? = null, + ): String { val query = buildList { title?.let { add("$ADD_BOOK_TITLE_ARG=${Uri.encode(it)}") } isbn?.let { add("$ADD_BOOK_ISBN_ARG=${Uri.encode(it)}") } + subtitle?.let { add("$ADD_BOOK_SUBTITLE_ARG=${Uri.encode(it)}") } + authors?.let { add("$ADD_BOOK_AUTHORS_ARG=${Uri.encode(it)}") } + publisher?.let { add("$ADD_BOOK_PUBLISHER_ARG=${Uri.encode(it)}") } + publishedDate?.let { add("$ADD_BOOK_PUBLISHED_DATE_ARG=${Uri.encode(it)}") } + pages?.let { add("$ADD_BOOK_PAGES_ARG=${Uri.encode(it)}") } + description?.let { add("$ADD_BOOK_DESCRIPTION_ARG=${Uri.encode(it)}") } + coverSourceUrl?.let { add("$ADD_BOOK_COVER_URL_ARG=${Uri.encode(it)}") } } return if (query.isEmpty()) ADD_BOOK_BASE else "$ADD_BOOK_BASE?" + query.joinToString("&") } diff --git a/app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksSearchClientTest.kt b/app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksSearchClientTest.kt new file mode 100644 index 0000000..6e29acc --- /dev/null +++ b/app/app/src/test/java/org/modg/bookshelf/data/metadata/GoogleBooksSearchClientTest.kt @@ -0,0 +1,143 @@ +package org.modg.bookshelf.data.metadata + +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.Json +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test + +/** + * Parses checked-in sample JSON fixtures — no network. Reuses the existing + * googlebooks_*.json fixtures ([GoogleBooksClientTest] already established they're + * shaped like a real `volumes?q=` response, which search and ISBN lookup share). + */ +class GoogleBooksSearchClientTest { + + private val client = GoogleBooksSearchClient(OkHttpClient(), Json) + + @Test + fun `parses a successful response into SearchHits with null industryIdentifiers and no imageLinks handled`() { + val body = """ + {"items": [{"volumeInfo": {"title": "Some Title", "authors": ["Some Author"]}}]} + """.trimIndent() + val hits = client.parseResponse(body) + + assertEquals(1, hits.size) + assertEquals("Some Title", hits[0].title) + assertEquals(listOf("Some Author"), hits[0].authors) + assertTrue(hits[0].isbn13s.isEmpty()) + assertEquals(null, hits[0].coverUrl) + } + + @Test + fun `parses the real success fixture and forces https zoom=2 on the cover`() { + val hits = client.parseResponse(fixture("googlebooks_success.json")) + assertEquals(1, hits.size) + assertEquals("Effective Java", hits[0].title) + assertEquals(listOf("9780134685991"), hits[0].isbn13s) + assertEquals( + "https://books.google.com/books/content?id=ABC123XYZ&printsec=frontcover&img=1&zoom=2", + hits[0].coverUrl, + ) + } + + @Test + fun `a garbage publishedDate does not become a year`() { + val body = """ + {"items": [{"volumeInfo": {"title": "Old Book", "publishedDate": "101-01-01"}}]} + """.trimIndent() + val hits = client.parseResponse(body) + assertEquals(null, hits[0].year) + } + + @Test + fun `returns NotFound for no items, not an empty Found`() { + assertEquals(SearchSourceResult.NotFound, client.classify(200, fixture("googlebooks_no_items.json"))) + } + + @Test + fun `classify reports Failed for a non-2xx status`() { + assertEquals(SearchSourceResult.Failed("http 404", FailureKind.CLIENT_ERROR), client.classify(404, null)) + } + + @Test + fun `classify reports Failed for malformed json`() { + assertEquals(SearchSourceResult.Failed("malformed json", FailureKind.MALFORMED), client.classify(200, fixture("malformed.json"))) + } + + // --- requestUrl(): key only when configured, query url-encoded --- + + @Test + fun `requestUrl omits the key when blank`() { + val url = client.requestUrl("dune") + assertTrue(url.contains("q=dune")) + assertFalse(url.contains("key=")) + } + + @Test + fun `requestUrl carries the key only when configured`() { + val keyed = GoogleBooksSearchClient(OkHttpClient(), Json, apiKey = "test-key-123") + assertTrue(keyed.requestUrl("dune").contains("key=test-key-123")) + } + + @Test + fun `requestUrl percent-encodes a query with url-breaking characters`() { + val url = client.requestUrl("a&b c") + assertFalse(url.contains("q=a&b")) + assertTrue(url.contains("q=a%26b")) + } + + // --- redact(): the key must never survive into a shown reason --- + + @Test + fun `redact scrubs the api key from a leaked reason`() { + val key = "test-key-123" + val keyed = GoogleBooksSearchClient(OkHttpClient(), Json, apiKey = key) + val leakedUrl = keyed.requestUrl("dune") + check(leakedUrl.contains(key)) + + val leaking = SearchSourceResult.Failed("network error: connect to $leakedUrl failed", FailureKind.TRANSPORT) + val redacted = keyed.redact(leaking) as SearchSourceResult.Failed + + assertFalse(redacted.reason.contains(key)) + assertTrue(redacted.reason.contains("[REDACTED]")) + } + + // --- fetch(): non-IOException -> Failed(UNEXPECTED); CancellationException propagates --- + + @Test + fun `searchOnce reports Failed UNEXPECTED, not a crash, when the http call throws a non-IOException`() = runTest { + val client = clientThrowing(IllegalStateException("boom")) + val result = client.searchOnce("dune") + val failed = result as? SearchSourceResult.Failed + checkNotNull(failed) { "expected Failed, got $result" } + assertEquals(FailureKind.UNEXPECTED, failed.kind) + assertEquals("unexpected: IllegalStateException", failed.reason) + } + + @Test + fun `CancellationException propagates out of the client rather than becoming a Failed`() = runTest { + val client = clientThrowing(CancellationException("scope cancelled")) + try { + client.searchOnce("dune") + fail("expected CancellationException to propagate") + } catch (e: CancellationException) { + // expected + } + } + + private fun clientThrowing(t: Throwable): GoogleBooksSearchClient { + val httpClient = OkHttpClient.Builder().addInterceptor(Interceptor { throw t }).build() + return GoogleBooksSearchClient(httpClient, Json) + } + + private fun fixture(name: String): String = + checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" } + .bufferedReader() + .readText() +} diff --git a/app/app/src/test/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchClientTest.kt b/app/app/src/test/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchClientTest.kt new file mode 100644 index 0000000..9fe54fe --- /dev/null +++ b/app/app/src/test/java/org/modg/bookshelf/data/metadata/OpenLibrarySearchClientTest.kt @@ -0,0 +1,139 @@ +package org.modg.bookshelf.data.metadata + +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.Json +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test + +/** + * Parses checked-in sample JSON fixtures (src/test/resources/fixtures) — no network + * involved, offline-safe. Mirrors [OpenLibraryClientTest]'s structure for the sibling + * search endpoint. + */ +class OpenLibrarySearchClientTest { + + private val client = OpenLibrarySearchClient(OkHttpClient(), Json) + + @Test + fun `parses a successful response, normalizing mixed isbn10-13 to one distinct isbn13`() { + val hits = client.parseResponse(fixture("openlibrary_search_success.json")) + + assertEquals(2, hits.size) + val hittite = hits[0] + assertEquals("Hittite Warrior", hittite.title) + assertEquals(listOf("Joanne S. Williamson"), hittite.authors) + assertEquals(1960, hittite.year) + assertEquals("Bethlehem Books", hittite.publisher) + assertEquals(237, hittite.pageCount) + assertEquals("https://covers.openlibrary.org/b/id/930599-M.jpg", hittite.coverUrl) + // "1883937388" (isbn-10) and "9781883937386" (isbn-13) are the SAME edition -- + // must normalize to one distinct value, not two. + assertEquals(listOf("9781883937386"), hittite.isbn13s) + } + + @Test + fun `a doc with no isbn field parses with an empty isbn13s list, not a crash`() { + val hits = client.parseResponse(fixture("openlibrary_search_success.json")) + val noIsbn = hits[1] + assertEquals("A Book With No ISBN On File", noIsbn.title) + assertTrue(noIsbn.isbn13s.isEmpty()) + } + + @Test + fun `a doc with no cover_i has a null coverUrl, never a synthesized one`() { + val hits = client.parseResponse(fixture("openlibrary_search_success.json")) + assertNull(hits[1].coverUrl) + } + + @Test + fun `fails soft on malformed json instead of throwing`() { + assertTrue(client.parseResponse(fixture("malformed.json")).isEmpty()) + } + + // --- classify(): the three-way per-source outcome --- + + @Test + fun `classify reports Found for docs with results`() { + val result = client.classify(200, fixture("openlibrary_search_success.json")) + val found = result as? SearchSourceResult.Found + checkNotNull(found) { "expected Found, got $result" } + assertEquals(2, found.hits.size) + } + + @Test + fun `classify reports NotFound for an empty docs list, distinctly from a failure`() { + val result = client.classify(200, fixture("openlibrary_search_no_docs.json")) + assertEquals(SearchSourceResult.NotFound, result) + } + + @Test + fun `classify reports Failed for a non-2xx status, not NotFound`() { + assertEquals(SearchSourceResult.Failed("http 500 (server error)", FailureKind.SERVER_ERROR), client.classify(500, null)) + assertEquals(SearchSourceResult.Failed("http 429 (rate limited)", FailureKind.RATE_LIMITED), client.classify(429, null)) + } + + @Test + fun `classify reports Failed for malformed json even on a 2xx status`() { + assertEquals( + SearchSourceResult.Failed("malformed json", FailureKind.MALFORMED), + client.classify(200, fixture("malformed.json")), + ) + } + + // --- requestUrl(): fields= (with isbn) must be present, and the query URL-encoded --- + + @Test + fun `requestUrl includes fields= with isbn -- without it OL omits isbns entirely`() { + val url = client.requestUrl("dune") + assertTrue(url.contains("fields=")) + assertTrue(url.contains("isbn")) + } + + @Test + fun `requestUrl percent-encodes a query with url-breaking characters`() { + val url = client.requestUrl("a&b c") + // A literal, un-encoded "&" would split the query into two bogus parameters. + assertFalse(url.contains("q=a&b")) + assertTrue(url.contains("q=a%26b")) + } + + // --- fetch(): non-IOException -> Failed(UNEXPECTED); CancellationException propagates --- + + @Test + fun `searchOnce reports Failed UNEXPECTED, not a crash, when the http call throws a non-IOException`() = runTest { + val client = clientThrowing(IllegalStateException("boom")) + val result = client.searchOnce("dune") + val failed = result as? SearchSourceResult.Failed + checkNotNull(failed) { "expected Failed, got $result" } + assertEquals(FailureKind.UNEXPECTED, failed.kind) + assertEquals("unexpected: IllegalStateException", failed.reason) + } + + @Test + fun `CancellationException propagates out of the client rather than becoming a Failed`() = runTest { + val client = clientThrowing(CancellationException("scope cancelled")) + try { + client.searchOnce("dune") + fail("expected CancellationException to propagate") + } catch (e: CancellationException) { + // expected + } + } + + private fun clientThrowing(t: Throwable): OpenLibrarySearchClient { + val httpClient = OkHttpClient.Builder().addInterceptor(Interceptor { throw t }).build() + return OpenLibrarySearchClient(httpClient, Json) + } + + private fun fixture(name: String): String = + checkNotNull(javaClass.classLoader.getResourceAsStream("fixtures/$name")) { "missing fixture $name" } + .bufferedReader() + .readText() +} diff --git a/app/app/src/test/java/org/modg/bookshelf/data/metadata/SearchResultMergerTest.kt b/app/app/src/test/java/org/modg/bookshelf/data/metadata/SearchResultMergerTest.kt new file mode 100644 index 0000000..bbd8df3 --- /dev/null +++ b/app/app/src/test/java/org/modg/bookshelf/data/metadata/SearchResultMergerTest.kt @@ -0,0 +1,240 @@ +package org.modg.bookshelf.data.metadata + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.modg.bookshelf.data.local.BookEntity +import org.modg.bookshelf.data.local.SyncState +import org.modg.bookshelf.data.repo.encodeAuthors + +class SearchResultMergerTest { + + private fun book( + id: String, + title: String, + authors: List, + isbn13: String? = null, + isbn10: String? = null, + ) = BookEntity( + id = id, + title = title, + authorsJson = encodeAuthors(authors), + isbn13 = isbn13, + isbn10 = isbn10, + createdAt = 1, + updatedAt = 1, + syncState = SyncState.SYNCED, + ) + + private fun hit( + title: String?, + authors: List = emptyList(), + isbn13s: List = emptyList(), + year: Int? = null, + coverUrl: String? = null, + publisher: String? = null, + pageCount: Int? = null, + ) = SearchHit(title, authors = authors, isbn13s = isbn13s, year = year, coverUrl = coverUrl, publisher = publisher, pageCount = pageCount) + + // --- identity: ISBN --- + + @Test + fun `a GB edition joins an OL work by a shared isbn13`() { + val ol = hit("Dune", listOf("Frank Herbert"), isbn13s = listOf("9780441013593", "9780451128596")) + val gb = hit("Dune", listOf("Frank Herbert"), isbn13s = listOf("9780441013593")) + + val result = SearchResultMerger.merge(emptyList(), listOf(ol), listOf(gb)) + + assertEquals(1, result.online.size) + assertEquals(setOf(SearchSource.OPEN_LIBRARY, SearchSource.GOOGLE_BOOKS), result.online[0].sources) + } + + // --- identity: title + author surname, near-duplicate author spellings --- + + @Test + fun `near-duplicate author spellings for the same work collapse into one entry`() { + // Real example from the task brief: the same book returned as two OL works. + val a = hit("Hittite Warrior", listOf("Joanne S. Williamson")) + val b = hit("Hittite Warrior", listOf("Joanne Small Williamson")) + + val result = SearchResultMerger.merge(emptyList(), listOf(a, b), emptyList()) + + assertEquals(1, result.online.size) + } + + @Test + fun `initials vs full name for the same author collapse via surname matching`() { + val a = hit("The Hobbit", listOf("J.R.R. Tolkien")) + val b = hit("The Hobbit", listOf("John Ronald Reuel Tolkien")) + + val result = SearchResultMerger.merge(emptyList(), listOf(a), listOf(b)) + + assertEquals(1, result.online.size) + } + + @Test + fun `five GB editions of the same title plus one OL work collapse to ONE online entry`() { + val ol = hit("The Hobbit", listOf("J.R.R. Tolkien")) + val gbEditions = List(5) { hit("The Hobbit", listOf("J. R. R. Tolkien")) } + + val result = SearchResultMerger.merge(emptyList(), listOf(ol), gbEditions) + + assertEquals(1, result.online.size) + } + + @Test + fun `distinct books with the same title but different authors stay separate`() { + val a = hit("Circe", listOf("Madeline Miller")) + val b = hit("Circe", listOf("Someone Else")) + + val result = SearchResultMerger.merge(emptyList(), listOf(a, b), emptyList()) + + assertEquals(2, result.online.size) + } + + // --- transitivity --- + + @Test + fun `transitivity -- A tilde B by isbn, B tilde C by title, all three land in one group`() { + // A and C share NEITHER an isbn NOR a title+author -- only B bridges them, so + // this only passes if grouping is transitive (union-find) rather than pairwise. + val a = hit("Title A", listOf("Author A"), isbn13s = listOf("9781111111111")) + val b = hit("Title B", listOf("Author B"), isbn13s = listOf("9781111111111")) // shares A's isbn + val c = hit("Title B", listOf("Author B")) // shares B's title+author, nothing in common with A + + val result = SearchResultMerger.merge(emptyList(), listOf(a), listOf(b, c)) + + assertEquals(1, result.online.size) + } + + // --- local books: placement, and the "reached only via an online hit" case --- + + @Test + fun `an online hit of a different edition of an owned book lands in inLibrary, not online`() { + val owned = book("b1", "Dune", listOf("Frank Herbert"), isbn13 = "9780451128596") + val differentEditionOnline = hit("Dune", listOf("Frank Herbert"), isbn13s = listOf("9780441013593")) + + val result = SearchResultMerger.merge(listOf(owned), listOf(differentEditionOnline), emptyList()) + + assertEquals(listOf(owned), result.inLibrary) + assertTrue("the online hit must not ALSO surface as a new online result", result.online.isEmpty()) + } + + @Test + fun `an online hit unrelated to any local book stays in online`() { + val owned = book("b1", "Dune", listOf("Frank Herbert"), isbn13 = "9780451128596") + val unrelated = hit("Piranesi", listOf("Susanna Clarke")) + + val result = SearchResultMerger.merge(listOf(owned), listOf(unrelated), emptyList()) + + assertEquals(listOf(owned), result.inLibrary) + assertEquals(1, result.online.size) + assertEquals("Piranesi", result.online[0].title) + } + + @Test + fun `two local books sharing a group are both shown, not deduped away`() { + val copy1 = book("b1", "Dune", listOf("Frank Herbert"), isbn13 = "9780441013593") + val copy2 = book("b2", "Dune", listOf("Frank Herbert"), isbn13 = "9780441013593") + + val result = SearchResultMerger.merge(listOf(copy1, copy2), emptyList(), emptyList()) + + assertEquals(setOf("b1", "b2"), result.inLibrary.map { it.id }.toSet()) + } + + // --- ranking --- + + @Test + fun `online results are ordered by the best rank either source returned a member at`() { + // The real hit is OL position 0 AND GB position 0 (rank 0); the noise trails + // at OL position 1 with nothing in GB backing it (rank 1) -- it must sort after. + val olRealHit = hit("The Real Book", listOf("Real Author"), isbn13s = listOf("9781111111111")) + val olNoise = hit("Loosely Related Noise", listOf("Nobody")) + val gbRealHit = hit("The Real Book", listOf("Real Author"), isbn13s = listOf("9781111111111")) + + val result = SearchResultMerger.merge( + local = emptyList(), + openLibrary = listOf(olRealHit, olNoise), + googleBooks = listOf(gbRealHit), + ) + + assertEquals(2, result.online.size) + assertEquals("The Real Book", result.online[0].title) + assertEquals("Loosely Related Noise", result.online[1].title) + } + + // --- field fill --- + + @Test + fun `title and authors come from OL when OL has a title, year and cover fall through independently`() { + // Tied together by a shared isbn13 so they're one group despite OL and GB + // disagreeing on title/author string -- exactly the shape a real work-vs- + // edition pairing takes, and what makes the primary/secondary choice visible. + val ol = hit("Concrete Mathematics", listOf("Ronald L. Graham"), isbn13s = listOf("9780201558029"), year = null, coverUrl = null) + val gb = hit( + "A Different Title Entirely", + listOf("GB Author"), + isbn13s = listOf("9780201558029"), + year = 1994, + coverUrl = "https://example.com/cover.jpg", + ) + + val merged = SearchResultMerger.merge(emptyList(), listOf(ol), listOf(gb)).online.single() + + assertEquals("Concrete Mathematics", merged.title) // OL wins because it has a title + assertEquals(listOf("Ronald L. Graham"), merged.authors) + assertEquals(1994, merged.year) // OL's is null, falls through to GB + assertEquals("https://example.com/cover.jpg", merged.coverUrl) + } + + // --- ISBN rule: each branch --- + + @Test + fun `isbn rule -- OL isbn10 and isbn13 of the same edition normalize to one, and that one is used`() { + val ol = hit("Hittite Warrior", listOf("Joanne S. Williamson"), isbn13s = listOf("9781883937386")) // already normalized by the client + val merged = SearchResultMerger.merge(emptyList(), listOf(ol), emptyList()).online.single() + assertEquals("9781883937386", merged.isbn13) + } + + @Test + fun `isbn rule -- OL has many isbns, GB has exactly one -- GB's isbn wins`() { + val ol = hit("Popular Book", listOf("Author"), isbn13s = listOf("9781111111111", "9782222222222", "9783333333333")) + val gb = hit("Popular Book", listOf("Author"), isbn13s = listOf("9781111111111")) + val merged = SearchResultMerger.merge(emptyList(), listOf(ol), listOf(gb)).online.single() + assertEquals("9781111111111", merged.isbn13) + } + + @Test + fun `isbn rule -- several GB isbns and no single-edition OL work -- no isbn is recorded`() { + val ol = hit("Popular Book", listOf("Author"), isbn13s = listOf("9781111111111", "9782222222222")) + val gbA = hit("Popular Book", listOf("Author"), isbn13s = listOf("9784444444444")) + val gbB = hit("Popular Book", listOf("Author"), isbn13s = listOf("9785555555555")) + val merged = SearchResultMerger.merge(emptyList(), listOf(ol), listOf(gbA, gbB)).online.single() + assertNull(merged.isbn13) + } + + @Test + fun `isbn rule -- OL has no isbn field -- falls through to GB`() { + val ol = hit("Popular Book", listOf("Author"), isbn13s = emptyList()) + val gb = hit("Popular Book", listOf("Author"), isbn13s = listOf("9781111111111")) + val merged = SearchResultMerger.merge(emptyList(), listOf(ol), listOf(gb)).online.single() + assertEquals("9781111111111", merged.isbn13) + } + + @Test + fun `isbn rule -- neither source resolves to a single isbn -- null, never a guess`() { + val ol = hit("Popular Book", listOf("Author"), isbn13s = emptyList()) + val gbA = hit("Popular Book", listOf("Author"), isbn13s = listOf("9781111111111")) + val gbB = hit("Popular Book", listOf("Author"), isbn13s = listOf("9782222222222")) + val merged = SearchResultMerger.merge(emptyList(), listOf(ol), listOf(gbA, gbB)).online.single() + assertNull(merged.isbn13) + } + + @Test + fun `empty inputs produce empty results without throwing`() { + val result = SearchResultMerger.merge(emptyList(), emptyList(), emptyList()) + assertTrue(result.inLibrary.isEmpty()) + assertTrue(result.online.isEmpty()) + } +} diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/library/LibraryViewModelTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/library/LibraryViewModelTest.kt new file mode 100644 index 0000000..f6ac234 --- /dev/null +++ b/app/app/src/test/java/org/modg/bookshelf/ui/library/LibraryViewModelTest.kt @@ -0,0 +1,348 @@ +package org.modg.bookshelf.ui.library + +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import java.util.concurrent.CountDownLatch +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import kotlinx.serialization.json.Json +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.modg.bookshelf.data.local.BookDao +import org.modg.bookshelf.data.local.BookEntity +import org.modg.bookshelf.data.local.BookshelfDatabase +import org.modg.bookshelf.data.metadata.BookSearchRepository +import org.modg.bookshelf.data.metadata.MetadataRepository +import org.modg.bookshelf.data.metadata.OnlineBook +import org.modg.bookshelf.data.prefs.SettingsStore +import org.modg.bookshelf.data.repo.BookRepository +import org.modg.bookshelf.data.repo.LocationRepository +import org.modg.bookshelf.data.repo.SyncEngine +import org.modg.bookshelf.data.repo.decodeAuthors +import org.modg.bookshelf.data.remote.ApiProvider +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Wave 10 (online search). Covers the ViewModel-owned parts of the task's contract: + * typing never hits the network, submit does, editing after a submit clears/cancels, + * partial and total source failure, and the online-result save sheet's field mapping + * and error handling. Parsing/merge logic itself is covered by + * [org.modg.bookshelf.data.metadata.SearchResultMergerTest] and the client tests — + * this file only exercises what's specific to owning that state in a ViewModel. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class LibraryViewModelTest { + + private lateinit var db: BookshelfDatabase + private lateinit var settingsStore: SettingsStore + private lateinit var locationRepository: LocationRepository + private lateinit var bookRepository: BookRepository + private lateinit var context: android.content.Context + + @Before + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, BookshelfDatabase::class.java) + .allowMainThreadQueries() + .build() + settingsStore = SettingsStore(context) + locationRepository = LocationRepository(db.bookcaseDao(), db.shelfDao(), db.bookDao()) + bookRepository = BookRepository(db.bookDao(), context) + } + + @After + fun tearDown() { + db.close() + Dispatchers.resetMain() + } + + private fun viewModel(searchHttpClient: OkHttpClient = OkHttpClient()): LibraryViewModel = LibraryViewModel( + bookRepository = bookRepository, + locationRepository = locationRepository, + syncEngine = SyncEngine(ApiProvider { null }, db.bookDao(), db.bookcaseDao(), db.shelfDao(), settingsStore), + settingsStore = settingsStore, + bookSearchRepository = BookSearchRepository(searchHttpClient, Json { ignoreUnknownKeys = true }), + metadataRepository = MetadataRepository(OkHttpClient(), Json { ignoreUnknownKeys = true }), + initialShelfId = null, + ) + + // --- typing vs submit --- + + @Test + fun `typing does NOT trigger an online search`() = runTest { + val vm = viewModel() + vm.onQueryChange("d") + vm.onQueryChange("du") + vm.onQueryChange("dune") + + assertEquals(OnlineSearchState.NotSearched, vm.onlineSearch.first()) + } + + @Test + fun `submit with fewer than 2 characters does nothing`() = runTest { + val vm = viewModel() + vm.onQueryChange("d") + vm.submitOnlineSearch() + + assertEquals(OnlineSearchState.NotSearched, vm.onlineSearch.first()) + } + + @Test + fun `submit runs the online search and populates online results`() = runTest { + val vm = viewModel(fakeSearchClient(olBody = olHit("Dune", "Frank Herbert"), gbBody = gbEmpty())) + vm.onQueryChange("dune") + vm.submitOnlineSearch() + + val done = vm.onlineSearch.first { it is OnlineSearchState.Done } as OnlineSearchState.Done + assertEquals(1, done.online.size) + assertEquals("Dune", done.online[0].title) + assertNull(done.openLibraryFailure) + assertNull(done.googleBooksFailure) + } + + // --- editing after a submit --- + + @Test + fun `editing the query after a submit clears the online section and cancels the in-flight search`() = runTest { + val gate = CountDownLatch(1) + val vm = viewModel(gatedFakeSearchClient(gate, olBody = olHit("Dune", "Frank Herbert"), gbBody = gbEmpty())) + vm.onQueryChange("dune") + vm.submitOnlineSearch() + + // .first(), not .value -- see the KDoc convention this codebase already uses + // (e.g. AddBookViewModel.uiState): a bare .value read with no live collector + // only ever sees the seed passed to stateIn. The fake call is blocked on the + // gate, so the phase genuinely stays Searching until collection observes it. + val searching = vm.onlineSearch.first { it !is OnlineSearchState.NotSearched } + assertTrue("expected Searching, got $searching", searching is OnlineSearchState.Searching) + + vm.onQueryChange("something else") + + assertEquals(OnlineSearchState.NotSearched, vm.onlineSearch.first()) + gate.countDown() // release the blocked background call so its thread isn't leaked + } + + // --- partial and total source failure --- + + @Test + fun `one source failing still shows the other source's results, with a failure line naming it`() = runTest { + val vm = viewModel(fakeSearchClient(olCode = 404, olBody = "", gbBody = gbHit("Dune", "Frank Herbert"))) + vm.onQueryChange("dune") + vm.submitOnlineSearch() + + val done = vm.onlineSearch.first { it is OnlineSearchState.Done } as OnlineSearchState.Done + assertEquals(1, done.online.size) + assertEquals("Dune", done.online[0].title) + checkNotNull(done.openLibraryFailure) + assertNull(done.googleBooksFailure) + } + + @Test + fun `both sources failing is reported as failures, never as an authoritative no-results`() = runTest { + val vm = viewModel(fakeSearchClient(olCode = 404, olBody = "", gbCode = 404, gbBody = "")) + vm.onQueryChange("dune") + vm.submitOnlineSearch() + + val done = vm.onlineSearch.first { it is OnlineSearchState.Done } as OnlineSearchState.Done + assertTrue(done.online.isEmpty()) + checkNotNull(done.openLibraryFailure) + checkNotNull(done.googleBooksFailure) + } + + @Test + fun `both sources answering with nothing is an authoritative empty result, no failures`() = runTest { + val vm = viewModel(fakeSearchClient(olBody = olEmpty(), gbBody = gbEmpty())) + vm.onQueryChange("zzzznonexistentzzzz") + vm.submitOnlineSearch() + + val done = vm.onlineSearch.first { it is OnlineSearchState.Done } as OnlineSearchState.Done + assertTrue(done.online.isEmpty()) + assertNull(done.openLibraryFailure) + assertNull(done.googleBooksFailure) + } + + // --- save sheet --- + + @Test + fun `save maps every online book field to createBook`() = runTest { + val bookcaseId = locationRepository.createBookcase(name = "Living Room") + val shelfId = locationRepository.createShelf(bookcaseId, label = "Top shelf") + val vm = viewModel() + val onlineBook = OnlineBook( + title = "Dune", + subtitle = "A novel", + authors = listOf("Frank Herbert"), + year = 1965, + publisher = "Ace", + pageCount = 412, + coverUrl = "https://example.com/cover.jpg", + isbn13 = "9780441013593", + ) + vm.onOnlineResultTapped(onlineBook) + vm.onSaveSheetShelfSelected(shelfId) + + val bookId = vm.performSaveOnlineResult() + checkNotNull(bookId) + + val saved = bookRepository.getById(bookId) + checkNotNull(saved) + assertEquals("Dune", saved.title) + assertEquals("A novel", saved.subtitle) + assertEquals(listOf("Frank Herbert"), decodeAuthors(saved.authorsJson)) + assertEquals("Ace", saved.publisher) + assertEquals("1965", saved.publishedDate) + assertEquals(412, saved.pageCount) + assertEquals("9780441013593", saved.isbn13) + assertEquals("https://example.com/cover.jpg", saved.coverSourceUrl) + assertEquals(shelfId, saved.shelfId) + } + + @Test + fun `after a save the sheet closes and the book appears in inLibrary via the Room flow`() = runTest { + val vm = viewModel(fakeSearchClient(olBody = olHit("Dune", "Frank Herbert", isbn = "9780441013593"), gbBody = gbEmpty())) + vm.onQueryChange("dune") + vm.submitOnlineSearch() + val beforeSave = vm.onlineSearch.first { it is OnlineSearchState.Done } as OnlineSearchState.Done + assertEquals(1, beforeSave.online.size) + assertTrue(beforeSave.inLibrary.isEmpty()) + + vm.onOnlineResultTapped(beforeSave.online[0]) + vm.performSaveOnlineResult() + + assertEquals(OnlineSaveSheetState.Hidden, vm.saveSheet.first()) + val afterSave = vm.onlineSearch.first { (it as? OnlineSearchState.Done)?.online?.isEmpty() == true } as OnlineSearchState.Done + assertEquals(1, afterSave.inLibrary.size) + assertEquals("Dune", afterSave.inLibrary[0].title) + } + + @Test + fun `a throwing save keeps the sheet open with an error and does not throw`() = runTest { + val throwingBookRepository = BookRepository(ThrowingUpsertBookDao(db.bookDao(), IllegalStateException("disk full")), context) + val vm = LibraryViewModel( + bookRepository = throwingBookRepository, + locationRepository = locationRepository, + syncEngine = SyncEngine(ApiProvider { null }, db.bookDao(), db.bookcaseDao(), db.shelfDao(), settingsStore), + settingsStore = settingsStore, + bookSearchRepository = BookSearchRepository(OkHttpClient(), Json { ignoreUnknownKeys = true }), + metadataRepository = MetadataRepository(OkHttpClient(), Json { ignoreUnknownKeys = true }), + initialShelfId = null, + ) + vm.onOnlineResultTapped(OnlineBook(title = "Dune")) + + val result = vm.performSaveOnlineResult() // must not throw + + assertNull(result) + val shown = vm.saveSheet.first() as? OnlineSaveSheetState.Shown + checkNotNull(shown) + assertEquals("IllegalStateException", shown.saveError) + assertTrue(!shown.isSaving) + } + + @Test + fun `a CancellationException during save propagates rather than being swallowed`() = runTest { + val vm = LibraryViewModel( + bookRepository = BookRepository(ThrowingUpsertBookDao(db.bookDao(), CancellationException("scope cancelled")), context), + locationRepository = locationRepository, + syncEngine = SyncEngine(ApiProvider { null }, db.bookDao(), db.bookcaseDao(), db.shelfDao(), settingsStore), + settingsStore = settingsStore, + bookSearchRepository = BookSearchRepository(OkHttpClient(), Json { ignoreUnknownKeys = true }), + metadataRepository = MetadataRepository(OkHttpClient(), Json { ignoreUnknownKeys = true }), + initialShelfId = null, + ) + vm.onOnlineResultTapped(OnlineBook(title = "Dune")) + + try { + vm.performSaveOnlineResult() + org.junit.Assert.fail("expected CancellationException to propagate") + } catch (e: CancellationException) { + // expected + } + } + + @Test + fun `a second save while one is in flight does not create a second book`() = runTest { + val vm = viewModel() + vm.onOnlineResultTapped(OnlineBook(title = "Dune")) + + val first = kotlinx.coroutines.CoroutineScope(Dispatchers.Unconfined).launch { vm.performSaveOnlineResult() } + val second = kotlinx.coroutines.CoroutineScope(Dispatchers.Unconfined).launch { vm.performSaveOnlineResult() } + first.join() + second.join() + + assertEquals(1, bookRepository.observeAll().first().size) + } + + private class ThrowingUpsertBookDao( + private val delegate: BookDao, + private val toThrow: Throwable, + ) : BookDao by delegate { + override suspend fun upsert(book: BookEntity) = throw toThrow + } + + // --- fake HTTP for BookSearchRepository --- + + private fun fakeSearchClient( + olCode: Int = 200, + olBody: String, + gbCode: Int = 200, + gbBody: String, + ): OkHttpClient = OkHttpClient.Builder() + .addInterceptor { chain -> respond(chain.request(), olCode, olBody, gbCode, gbBody) } + .build() + + private fun gatedFakeSearchClient( + gate: CountDownLatch, + olBody: String, + gbBody: String, + ): OkHttpClient = OkHttpClient.Builder() + .addInterceptor { chain -> gate.await(); respond(chain.request(), 200, olBody, 200, gbBody) } + .build() + + private fun respond(request: Request, olCode: Int, olBody: String, gbCode: Int, gbBody: String): Response = when { + request.url.host.contains("openlibrary") -> jsonResponse(request, olCode, olBody) + request.url.host.contains("googleapis") -> jsonResponse(request, gbCode, gbBody) + else -> error("unexpected host: ${request.url.host}") + } + + private fun jsonResponse(request: Request, code: Int, body: String): Response = Response.Builder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .code(code) + .message("") + .body(body.toResponseBody("application/json".toMediaType())) + .build() + + private fun olHit(title: String, author: String, isbn: String? = null) = """ + {"docs": [{"title": "$title", "author_name": ["$author"]${isbn?.let { ", \"isbn\": [\"$it\"]" }.orEmpty()}}]} + """.trimIndent() + + private fun olEmpty() = """{"docs": []}""" + + private fun gbHit(title: String, author: String) = """ + {"items": [{"volumeInfo": {"title": "$title", "authors": ["$author"]}}]} + """.trimIndent() + + private fun gbEmpty() = """{"items": []}""" +} diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/library/LocalBookMatcherTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/library/LocalBookMatcherTest.kt new file mode 100644 index 0000000..9618114 --- /dev/null +++ b/app/app/src/test/java/org/modg/bookshelf/ui/library/LocalBookMatcherTest.kt @@ -0,0 +1,74 @@ +package org.modg.bookshelf.ui.library + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.modg.bookshelf.data.local.BookEntity +import org.modg.bookshelf.data.local.SyncState +import org.modg.bookshelf.data.repo.encodeAuthors + +class LocalBookMatcherTest { + + private fun book(id: String, title: String, authors: List, subtitle: String? = null, isbn13: String? = null) = BookEntity( + id = id, + title = title, + subtitle = subtitle, + authorsJson = encodeAuthors(authors), + isbn13 = isbn13, + createdAt = 1, + updatedAt = 1, + syncState = SyncState.SYNCED, + ) + + private val hobbit = book("b1", "The Hobbit", listOf("J.R.R. Tolkien"), isbn13 = "9780547928227") + private val bronte = book("b2", "Jane Eyre", listOf("Charlotte Brontë")) + private val dune = book("b3", "Dune", listOf("Frank Herbert")) + + @Test + fun `title plus author across two fields matches`() { + val result = LocalBookMatcher.match(listOf(hobbit, dune), "hobbit tolkien") + assertEquals(listOf(hobbit), result) + } + + @Test + fun `squashed initials match a token with no spaces`() { + val result = LocalBookMatcher.match(listOf(hobbit, dune), "jrr tolkien") + assertEquals(listOf(hobbit), result) + } + + @Test + fun `token order does not matter`() { + val result = LocalBookMatcher.match(listOf(hobbit, dune), "tolkien hobbit") + assertEquals(listOf(hobbit), result) + } + + @Test + fun `diacritics are stripped for matching`() { + val result = LocalBookMatcher.match(listOf(bronte, dune), "bronte") + assertEquals(listOf(bronte), result) + } + + @Test + fun `a query token matching nothing excludes the book`() { + val result = LocalBookMatcher.match(listOf(hobbit), "hobbit nonexistentword") + assertTrue(result.isEmpty()) + } + + @Test + fun `a token can match the isbn`() { + val result = LocalBookMatcher.match(listOf(hobbit, dune), "9780547928227") + assertEquals(listOf(hobbit), result) + } + + @Test + fun `blank query matches nothing`() { + assertTrue(LocalBookMatcher.match(listOf(hobbit, dune), " ").isEmpty()) + } + + @Test + fun `a subtitle token matches`() { + val withSubtitle = book("b4", "The Way of Kings", listOf("Brandon Sanderson"), subtitle = "The Stormlight Archive") + val result = LocalBookMatcher.match(listOf(withSubtitle, dune), "stormlight") + assertEquals(listOf(withSubtitle), result) + } +} diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/nav/RoutesTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/nav/RoutesTest.kt index faf6a64..53dd849 100644 --- a/app/app/src/test/java/org/modg/bookshelf/ui/nav/RoutesTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/ui/nav/RoutesTest.kt @@ -74,4 +74,39 @@ class RoutesTest { assertEquals("add", Routes.addBook().substringBefore("?")) assertEquals("add", Routes.ADD_BOOK.substringBefore("?")) } + + // --- wave 10: the full draft (online search's "Edit details") round-trips too --- + + @Test + fun `round-trips every draft field an online search result can carry`() { + val route = Routes.addBook( + title = "The Hobbit", + subtitle = "or There and Back Again", + authors = "J.R.R. Tolkien, Someone Else", + publisher = "Houghton Mifflin", + publishedDate = "1937", + pages = "310", + isbn = "9780547928227", + description = "A hobbit goes on an adventure & returns home.", + coverSourceUrl = "https://covers.openlibrary.org/b/id/1-M.jpg?x=1&y=2", + ) + + assertEquals("The Hobbit", decode(route, Routes.ADD_BOOK_TITLE_ARG)) + assertEquals("or There and Back Again", decode(route, Routes.ADD_BOOK_SUBTITLE_ARG)) + assertEquals("J.R.R. Tolkien, Someone Else", decode(route, Routes.ADD_BOOK_AUTHORS_ARG)) + assertEquals("Houghton Mifflin", decode(route, Routes.ADD_BOOK_PUBLISHER_ARG)) + assertEquals("1937", decode(route, Routes.ADD_BOOK_PUBLISHED_DATE_ARG)) + assertEquals("310", decode(route, Routes.ADD_BOOK_PAGES_ARG)) + assertEquals("9780547928227", decode(route, Routes.ADD_BOOK_ISBN_ARG)) + assertEquals("A hobbit goes on an adventure & returns home.", decode(route, Routes.ADD_BOOK_DESCRIPTION_ARG)) + assertEquals("https://covers.openlibrary.org/b/id/1-M.jpg?x=1&y=2", decode(route, Routes.ADD_BOOK_COVER_URL_ARG)) + } + + @Test + fun `omitting the new draft fields leaves them out entirely, not as literal nulls`() { + val route = Routes.addBook(title = "Dune") + assertNull(decode(route, Routes.ADD_BOOK_SUBTITLE_ARG)) + assertNull(decode(route, Routes.ADD_BOOK_DESCRIPTION_ARG)) + assertNull(decode(route, Routes.ADD_BOOK_COVER_URL_ARG)) + } } diff --git a/app/app/src/test/java/org/modg/bookshelf/ui/screens/LibraryScreenPaparazziTest.kt b/app/app/src/test/java/org/modg/bookshelf/ui/screens/LibraryScreenPaparazziTest.kt index c907751..9d6b5e6 100644 --- a/app/app/src/test/java/org/modg/bookshelf/ui/screens/LibraryScreenPaparazziTest.kt +++ b/app/app/src/test/java/org/modg/bookshelf/ui/screens/LibraryScreenPaparazziTest.kt @@ -21,6 +21,7 @@ import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.SmallFloatingActionButton +import androidx.compose.material3.Surface import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -30,6 +31,8 @@ import app.cash.paparazzi.Paparazzi import org.junit.Rule import org.junit.Test import org.modg.bookshelf.data.local.BookEntity +import org.modg.bookshelf.data.metadata.OnlineBook +import org.modg.bookshelf.data.metadata.SearchSource import org.modg.bookshelf.ui.components.BookshelfScaffold import org.modg.bookshelf.ui.components.EmptyState import org.modg.bookshelf.ui.components.PaperSurface @@ -40,8 +43,12 @@ import org.modg.bookshelf.ui.components.SyncStatusBar import org.modg.bookshelf.ui.library.LibraryBookCard import org.modg.bookshelf.ui.library.LibraryFilter import org.modg.bookshelf.ui.library.LibraryScreen +import org.modg.bookshelf.ui.library.LibrarySearchResultsContent import org.modg.bookshelf.ui.library.LibrarySortOption import org.modg.bookshelf.ui.library.LibraryToolbar +import org.modg.bookshelf.ui.library.OnlineResultSaveSheet +import org.modg.bookshelf.ui.library.OnlineSaveSheetState +import org.modg.bookshelf.ui.library.OnlineSearchState import org.modg.bookshelf.ui.theme.BookshelfTheme /** @@ -63,12 +70,127 @@ class LibraryScreenPaparazziTest { @Test fun libraryEmptyLight() = snapshotBoth("library-empty") { Empty() } + @Test + fun librarySearchBothSectionsLight() = snapshotBoth("library-search-both-sections") { + SearchShell( + localMatches = listOf(ScreenFixtures.books[1]), // "The Hobbit" -- already owned + onlineSearch = OnlineSearchState.Done( + query = "hobbit", + inLibrary = listOf(ScreenFixtures.books[1]), + online = onlineResults, + openLibraryFailure = null, + googleBooksFailure = null, + ), + ) + } + + @Test + fun librarySearchLoadingLight() = snapshotBoth("library-search-loading") { + SearchShell(localMatches = emptyList(), onlineSearch = OnlineSearchState.Searching(query = "hobbit")) + } + + @Test + fun librarySearchOneSourceFailedLight() = snapshotBoth("library-search-one-failed") { + SearchShell( + localMatches = emptyList(), + onlineSearch = OnlineSearchState.Done( + query = "hobbit", + inLibrary = emptyList(), + online = onlineResults, + openLibraryFailure = "tls connection reset, 3 attempts", + googleBooksFailure = null, + ), + ) + } + + @Test + fun libraryOnlineSaveSheetLight() = snapshotBoth("library-online-save-sheet") { + Surface { + OnlineResultSaveSheet( + state = OnlineSaveSheetState.Shown( + book = onlineResults[0], + selectedShelfId = ScreenFixtures.topShelf.id, + ), + bookcases = ScreenFixtures.bookcases, + shelves = ScreenFixtures.shelves, + recentShelfId = ScreenFixtures.topShelf.id, + onShelfSelected = {}, + onSave = {}, + onEditDetails = {}, + onSkip = {}, + ) + } + } + + /** Two online hits: one merged from both sources, one Open-Library-only -- the shape [SearchResultMerger] actually produces. */ + private val onlineResults = listOf( + OnlineBook( + title = "The Hobbit", + authors = listOf("J.R.R. Tolkien"), + year = 1937, + publisher = "Houghton Mifflin", + isbn13 = "9780547928227", + sources = setOf(SearchSource.OPEN_LIBRARY, SearchSource.GOOGLE_BOOKS), + ), + OnlineBook( + title = "The Annotated Hobbit", + authors = listOf("J.R.R. Tolkien", "Douglas A. Anderson"), + year = 2002, + sources = setOf(SearchSource.OPEN_LIBRARY), + ), + ) + @Composable private fun Populated() = Shell(books = ScreenFixtures.books, syncStatus = SyncStatus.Synced, syncLabel = "Synced • 2m ago") @Composable private fun Empty() = Shell(books = emptyList(), syncStatus = SyncStatus.Offline, syncLabel = "Not synced yet") + /** Same chrome as [Shell] but with a non-blank query, so [LibraryScreen] shows the sectioned search-results view instead of the grid. */ + @Composable + private fun SearchShell(localMatches: List, onlineSearch: OnlineSearchState) { + BookshelfScaffold( + title = "Bookshelf", + actions = { + IconButton(onClick = {}) { Icon(Icons.Outlined.Warehouse, contentDescription = "Bookcases & shelves") } + IconButton(onClick = {}) { Icon(Icons.Outlined.Settings, contentDescription = "Settings") } + }, + floatingActionButton = { + Column(horizontalAlignment = Alignment.End) { + SmallFloatingActionButton(onClick = {}) { Icon(Icons.Outlined.EditNote, contentDescription = "Add a book by hand") } + Spacer(modifier = Modifier.height(12.dp)) + FloatingActionButton(onClick = {}) { Icon(Icons.Outlined.QrCodeScanner, contentDescription = "Scan a book") } + } + }, + syncStatusBar = { SyncStatusBar(status = SyncStatus.Synced, label = "Synced • 2m ago") }, + ) { innerPadding -> + PaperSurface(modifier = Modifier.fillMaxSize()) { + Column(modifier = Modifier.fillMaxSize()) { + LibraryToolbar( + query = "hobbit", + onQueryChange = {}, + sortOption = LibrarySortOption.TITLE, + onSortOptionChange = {}, + filter = LibraryFilter.All, + onFilterChange = {}, + bookcases = ScreenFixtures.bookcases, + shelves = ScreenFixtures.shelves, + ) + LibrarySearchResultsContent( + query = "hobbit", + localMatches = localMatches, + onlineSearch = onlineSearch, + onBookClick = {}, + onSubmitOnlineSearch = {}, + onRetryOnlineSearch = {}, + onOnlineResultClick = {}, + onEnterByHand = {}, + ) + } + } + } + } + @Composable private fun Shell(books: List, syncStatus: SyncStatus, syncLabel: String) { BookshelfScaffold( diff --git a/app/app/src/test/resources/fixtures/openlibrary_search_no_docs.json b/app/app/src/test/resources/fixtures/openlibrary_search_no_docs.json new file mode 100644 index 0000000..c29ff25 --- /dev/null +++ b/app/app/src/test/resources/fixtures/openlibrary_search_no_docs.json @@ -0,0 +1,5 @@ +{ + "numFound": 0, + "start": 0, + "docs": [] +} diff --git a/app/app/src/test/resources/fixtures/openlibrary_search_success.json b/app/app/src/test/resources/fixtures/openlibrary_search_success.json new file mode 100644 index 0000000..9abcf01 --- /dev/null +++ b/app/app/src/test/resources/fixtures/openlibrary_search_success.json @@ -0,0 +1,22 @@ +{ + "numFound": 2, + "start": 0, + "docs": [ + { + "key": "/works/OL2028038W", + "title": "Hittite Warrior", + "author_name": ["Joanne S. Williamson"], + "first_publish_year": 1960, + "cover_i": 930599, + "isbn": ["1883937388", "9781883937386"], + "publisher": ["Bethlehem Books"], + "number_of_pages_median": 237 + }, + { + "key": "/works/OL9999999W", + "title": "A Book With No ISBN On File", + "author_name": ["Some Author"], + "first_publish_year": 1901 + } + ] +} diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_libraryOnlineSaveSheetLight_library-online-save-sheet-dark.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_libraryOnlineSaveSheetLight_library-online-save-sheet-dark.png new file mode 100644 index 0000000..38470b6 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_libraryOnlineSaveSheetLight_library-online-save-sheet-dark.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_libraryOnlineSaveSheetLight_library-online-save-sheet-light.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_libraryOnlineSaveSheetLight_library-online-save-sheet-light.png new file mode 100644 index 0000000..4b591c2 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_libraryOnlineSaveSheetLight_library-online-save-sheet-light.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchBothSectionsLight_library-search-both-sections-dark.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchBothSectionsLight_library-search-both-sections-dark.png new file mode 100644 index 0000000..f3b71dd Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchBothSectionsLight_library-search-both-sections-dark.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchBothSectionsLight_library-search-both-sections-light.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchBothSectionsLight_library-search-both-sections-light.png new file mode 100644 index 0000000..8d24023 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchBothSectionsLight_library-search-both-sections-light.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchLoadingLight_library-search-loading-dark.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchLoadingLight_library-search-loading-dark.png new file mode 100644 index 0000000..0266268 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchLoadingLight_library-search-loading-dark.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchLoadingLight_library-search-loading-light.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchLoadingLight_library-search-loading-light.png new file mode 100644 index 0000000..e0ff0f6 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchLoadingLight_library-search-loading-light.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchOneSourceFailedLight_library-search-one-failed-dark.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchOneSourceFailedLight_library-search-one-failed-dark.png new file mode 100644 index 0000000..0453fa4 Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchOneSourceFailedLight_library-search-one-failed-dark.png differ diff --git a/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchOneSourceFailedLight_library-search-one-failed-light.png b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchOneSourceFailedLight_library-search-one-failed-light.png new file mode 100644 index 0000000..dac16be Binary files /dev/null and b/app/app/src/test/snapshots/images/org.modg.bookshelf.ui.screens_LibraryScreenPaparazziTest_librarySearchOneSourceFailedLight_library-search-one-failed-light.png differ diff --git a/logs/L-search.sid b/logs/L-search.sid new file mode 100644 index 0000000..24149e1 --- /dev/null +++ b/logs/L-search.sid @@ -0,0 +1 @@ +26bd4716-bf10-4c6b-ab07-8bc6d4c170a1 diff --git a/logs/L-search.summary b/logs/L-search.summary new file mode 100644 index 0000000..28364fd --- /dev/null +++ b/logs/L-search.summary @@ -0,0 +1,12 @@ +=== L-search attempts=1 quota_waits=0 hard_fails=0 === +cost=$10.452535 turns=157 err=? +--- result (tail) --- +ange, verified by the untouched `RetryPolicyTest` still passing. +- **"Retry" re-runs both sources**, not just the failed one. The spec's "while still showing the OTHER source's results" is satisfied by the *resting* UI state (a failure line next to the other source's live results), but tapping Retry re-submits the whole query rather than re-querying only the failed source. A per-source-only retry would need to retain raw hit lists per source across a partial re-fetch; I judged the added state-machine complexity wasn't worth it for this pass. +- **In-flight cancellation doesn't abort the underlying OkHttp call** — cancelling the ViewModel job discards the result but the blocking `.execute()` call finishes on its IO thread regardless (same limitation the existing ISBN-lookup clients already have; not something new). +- **"In your library" section uses a horizontally-scrolling row of the existing card**, not a full grid, to avoid nesting a scrollable grid inside the outer vertically-scrolling search-results column (a real Compose layout hazard). "Online" results are a plain (non-lazy) `Column` of rows for the same reason — acceptable given result counts are capped at ~20/source before dedupe. + +### Out of scope / noticed but not touched + +- The pre-existing Paparazzi rendering quirk where the search/filter/sort toolbar row renders as a thin unlabeled bar (visible in both the old `library-populated` and my new search-results screenshots) — confirmed pre-existing by comparing against the baseline `library-populated-light.png`, not something I introduced or was asked to fix. +- `BookDao.search`/`BookRepository.search` are now unused by the library screen but left in place — not deleted, since the task only authorized touching them if I chose the SQL route, which I didn't. diff --git a/logs/WAVE9-DONE b/logs/WAVE9-DONE new file mode 100644 index 0000000..a11b026 --- /dev/null +++ b/logs/WAVE9-DONE @@ -0,0 +1,4 @@ +=== WAVE 9 (K-manual) passed the chain gate 2026-09-15T11:58:39+00:00 === +commit b600df6 (pushed); tests 241; worker $5.051114400000003, 108 turns +Mechanical gate only. Orchestrator must still review: read logs/K-manual.summary, +eyeball new Paparazzi PNGs, and read the diff (git show --stat b600df6).