From eea7f70cf46697328368f49d1dec341ee189356e Mon Sep 17 00:00:00 2001 From: dietrich ayala Date: Tue, 13 Oct 2026 06:56:17 +0200 Subject: [PATCH] catalog-rows: keep failures per row and make row saves all-or-nothing - CatalogRowsRefresher: a row deleted mid-run is left out; any other per-row failure marks the row stale (UNEXPECTED) and the run goes on; a title storage would refuse is dropped, not the whole row. - A last attempt time later than now counts as due, and stored future times are moved back to now before the next attempt is written. - ensureCatalogRows inserts defaults turned off once 20 rows are shown; a TMDB catalog save stores unticked lists off in the same transaction as the ticks, so a TOO_MANY refusal stores nothing. - PUT /settings saves catalog switches and the row list in one transaction (HomeSettingsStore.saveRows, LibraryStore.saveHomeRows); an unknown catalog row is named in details.catalogRowId. - Phone page: an unsaved row list drops catalog rows the TV no longer has, on load and when a save is refused for one. --- .../app/catalogrows/CatalogRowsRefresher.kt | 97 ++++++++++++++----- .../app/home/RoomHomeSettingsStore.kt | 23 +++-- .../app/sources/RoomSourceCatalogStore.kt | 9 +- .../catalogrows/CatalogRowsRefresherTest.kt | 78 ++++++++++++++- .../app/home/RoomHomeSettingsStoreTest.kt | 17 +++- .../app/sources/RoomSourceCatalogStoreTest.kt | 12 +++ .../tv/streamer/core/database/LibraryStore.kt | 72 ++++++++++++-- .../core/database/CatalogRowStorageTest.kt | 40 ++++++-- docs/catalog-rows.md | 32 ++++-- docs/decisions.md | 2 +- docs/home-settings-api.md | 12 ++- docs/sources-api.md | 4 +- docs/storage.md | 6 +- docs/web-server.md | 4 +- .../tv/streamer/webserver/api/Settings.kt | 56 ++++++----- .../tv/streamer/webserver/page/app.js | 31 ++++++ .../webserver/api/HomeSettingsApiTest.kt | 39 +++++--- .../streamer/webserver/api/PhonePageTest.kt | 36 +++++-- 18 files changed, 452 insertions(+), 118 deletions(-) diff --git a/app/src/main/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresher.kt b/app/src/main/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresher.kt index 758d398..997f1f9 100644 --- a/app/src/main/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresher.kt +++ b/app/src/main/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresher.kt @@ -10,6 +10,7 @@ import tv.streamer.accounts.toItem import tv.streamer.app.credentials.CredentialStore import tv.streamer.app.resolver.AddonConfiguration import tv.streamer.app.resolver.ResolverAssembly +import tv.streamer.core.database.CatalogRow import tv.streamer.core.database.FeedEntry import tv.streamer.core.database.FeedFailure import tv.streamer.core.database.FeedUpdate @@ -45,6 +46,9 @@ enum class CatalogRowFailure { /** TMDB answered with a server error, refused the call, or could not be reached. */ TMDB_UNAVAILABLE, + + /** Anything else failed for this row, such as storage refusing its titles. The run went on. */ + UNEXPECTED, } /** What one refresh did with one catalog row. */ @@ -86,7 +90,11 @@ sealed interface CatalogRowsRefreshResult { * TMDB does not know, is left out. A TMDB row is page 1 of its list. A row is written whole * ([FeedUpdate.Complete]) or not at all: a failure keeps its titles and marks it stale * ([FeedUpdate.Failed]), creating its feed if needed so the hourly gate holds for a failing source - * too. A TMDB 429 skips the remaining rows of the run, since every row needs TMDB. + * too. A TMDB 429 skips the remaining rows of the run, since every row needs TMDB. Any other + * failure stays with its row: a row deleted during the run is left out of the result, anything else + * marks the row stale as [CatalogRowFailure.UNEXPECTED], and the run goes on. A title storage would + * refuse is dropped rather than failing its row. A last attempt later than the clock counts as due + * ([isDue]) and is moved back to the clock when the row is next written. * * [tmdbFor] gives the TMDB client for a key; sharing For you's keeps one rate limit and one `find` * cache. Runs are serialized. Nothing here logs; the caller records events from the result. @@ -117,7 +125,7 @@ class CatalogRowsRefresher( val due = snapshot.catalogRows.sortedBy { it.id }.filter { row -> row.shown && row.accountId in authorized && (row.provider == ProviderType.STREMIO_ADDON || row.accountId == tmdbAccount.id) && - (row.feedId?.let { feeds[it]?.lastAttemptAt }?.let { now - it >= REFRESH_INTERVAL_MILLIS } ?: true) + (row.feedId?.let { feeds[it]?.lastAttemptAt }?.let { isDue(it, now) } ?: true) } if (due.isEmpty()) return@withLock CatalogRowsRefreshResult.Refreshed(emptyMap()) @@ -127,30 +135,46 @@ class CatalogRowsRefresher( var rateLimited = false for (row in due) { val outcome = if (rateLimited) CatalogRowOutcome.Skipped else try { - val entries = when (row.provider) { - ProviderType.TMDB -> tmdbRow(row, tmdb) - else -> addonRow(row, accountsById[row.accountId], tmdb, stored) - } - when (entries) { - is Fetch.Titles -> { - val feedId = library.catalogRowFeedId(row.id) - library.applyFeedUpdate(feedId, FeedUpdate.Complete( - entries.items.mapIndexed { rank, item -> FeedEntry(item, rank, 0L) }, nextAttempt(feedId))) - CatalogRowOutcome.Updated(entries.items.size, entries.dropped) - } - is Fetch.Failed -> fail(row, entries.failure) - } + refreshRow(row, accountsById[row.accountId], tmdb, stored) } catch (e: CancellationException) { throw e } catch (e: TmdbFailure) { if (e.failure == CatalogRowFailure.TMDB_RATE_LIMITED) rateLimited = true - fail(row, e.failure) + failOrGone(row, e.failure) + } catch (e: Exception) { + failOrGone(row, CatalogRowFailure.UNEXPECTED) } - outcomes[row.id] = CatalogRowRefresh(row.provider, outcome) + // A row deleted during the run is left out: there is nothing left to show or mark. + outcome?.let { outcomes[row.id] = CatalogRowRefresh(row.provider, it) } } CatalogRowsRefreshResult.Refreshed(outcomes) } + /** One row's fetch and write; null when the row was deleted during the run. */ + private suspend fun refreshRow( + row: LibraryCatalogRow, + account: Account?, + tmdb: TmdbClient, + stored: Map>, + ): CatalogRowOutcome? { + val catalog = library.catalogRow(row.id) ?: return null + val entries = when (row.provider) { + ProviderType.TMDB -> tmdbRow(catalog, tmdb) + else -> addonRow(row, catalog, account, tmdb, stored) + } + return when (entries) { + is Fetch.Titles -> { + // A title storage would refuse is dropped, so it cannot fail the whole row. + val items = entries.items.filter(library::isStorableTitle).distinctBy { it.canonicalId } + val feedId = library.catalogRowFeedId(row.id) + library.applyFeedUpdate(feedId, FeedUpdate.Complete( + items.mapIndexed { rank, item -> FeedEntry(item, rank, 0L) }, nextAttempt(feedId))) + CatalogRowOutcome.Updated(items.size, entries.dropped + entries.items.size - items.size) + } + is Fetch.Failed -> fail(row, entries.failure) + } + } + /** The lowest-id TMDB account whose key can be read, as the resolver picks it. */ private suspend fun tmdbKey(accounts: List): Pair? { for (account in accounts) { @@ -167,19 +191,18 @@ class CatalogRowsRefresher( private class TmdbFailure(val failure: CatalogRowFailure) : Exception() - private suspend fun tmdbRow(row: LibraryCatalogRow, tmdb: TmdbClient): Fetch { - val stored = requireNotNull(library.catalogRow(row.id)) - val list = TMDB_LISTS.getValue(stored.catalogType to stored.catalogId) + private suspend fun tmdbRow(catalog: CatalogRow, tmdb: TmdbClient): Fetch { + val list = TMDB_LISTS.getValue(catalog.catalogType to catalog.catalogId) return Fetch.Titles(tmdbCall { tmdb.list(list) }, dropped = 0) } private suspend fun addonRow( row: LibraryCatalogRow, + catalog: CatalogRow, account: Account?, tmdb: TmdbClient, stored: Map>, ): Fetch { - val catalog = requireNotNull(library.catalogRow(row.id)) val manifestUrl = account?.let { AddonConfiguration.read(credentials, it.credentialRef) } ?: return Fetch.Failed(CatalogRowFailure.ADDON_NOT_CONFIGURED) val endpoints = AddonEndpoints.fromManifestUrl(manifestUrl) as? AddonDataResult.Complete @@ -243,21 +266,49 @@ class CatalogRowsRefresher( return CatalogRowOutcome.Failed(failure) } + /** + * Marks the row stale as [fail] does, or null when the row no longer exists. If even marking it + * fails, the outcome is still [failure], so the run goes on to the next row. + */ + private suspend fun failOrGone(row: LibraryCatalogRow, failure: CatalogRowFailure): CatalogRowOutcome? { + if (library.catalogRow(row.id) == null) return null + return try { + fail(row, failure) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + if (library.catalogRow(row.id) == null) null else CatalogRowOutcome.Failed(failure) + } + } + private fun addonHttp(sourceKey: String): AddonMetadataHttp = synchronized(addons) { addons.getOrPut(sourceKey) { AddonMetadataHttp(addonClient(sourceKey), metadataLimits) } } - /** The clock, moved past the feed's last attempt so a fast repeat still commits. */ + /** + * The clock, moved past the feed's last attempt so a fast repeat still commits. Stored times + * later than the clock (written while it ran ahead) are first moved back to it, so this attempt + * is stamped with the real time and the hourly gate counts from it. + */ private suspend fun nextAttempt(feedId: Long): Long { + val now = clock() + library.clampFeedTimes(feedId, now) val feed = requireNotNull(library.cachedFeed(feedId)) val last = maxOf(feed.lastAttemptAt ?: -1, feed.fetchedAt ?: -1) - return maxOf(clock(), last + 1) + return maxOf(now, last + 1) } companion object { /** A row is asked again once its last attempt is this old (`spec.md` refresh table: at most hourly). */ const val REFRESH_INTERVAL_MILLIS = 3_600_000L + /** + * Whether a row last attempted at [lastAttemptAt] is due at [now]: an hour or more ago, or + * later than now, which only a clock that ran ahead (as before network time) can have stored. + */ + fun isDue(lastAttemptAt: Long, now: Long): Boolean = + lastAttemptAt > now || now - lastAttemptAt >= REFRESH_INTERVAL_MILLIS + /** Titles kept from an addon's first page. */ const val MAX_CATALOG_ROW_TITLES = 40 diff --git a/app/src/main/kotlin/tv/streamer/app/home/RoomHomeSettingsStore.kt b/app/src/main/kotlin/tv/streamer/app/home/RoomHomeSettingsStore.kt index 5b85f8d..272065e 100644 --- a/app/src/main/kotlin/tv/streamer/app/home/RoomHomeSettingsStore.kt +++ b/app/src/main/kotlin/tv/streamer/app/home/RoomHomeSettingsStore.kt @@ -5,10 +5,10 @@ import tv.streamer.core.database.CatalogRowChange import tv.streamer.core.database.LibraryStore import tv.streamer.core.database.StreamerDatabase import tv.streamer.core.model.HomeRow -import tv.streamer.webserver.api.CatalogRowsSave import tv.streamer.webserver.api.HomeApps import tv.streamer.webserver.api.HomeCatalogRow import tv.streamer.webserver.api.HomeLayout +import tv.streamer.webserver.api.HomeRowsSave import tv.streamer.webserver.api.HomeSettingsStore import tv.streamer.webserver.api.InstalledHomeApp @@ -36,16 +36,19 @@ class RoomHomeSettingsStore(database: StreamerDatabase, private val home: HomeRe ) } - override suspend fun setHomeRows(rows: List) { - library.setHomeRows(rows) - } - - override suspend fun setCatalogRowsShown(shown: Map): CatalogRowsSave = - when (library.setCatalogRowsShown(shown)) { - CatalogRowChange.SAVED -> CatalogRowsSave.SAVED - CatalogRowChange.TOO_MANY -> CatalogRowsSave.TOO_MANY - else -> CatalogRowsSave.UNKNOWN_ROW + /** Both writes in one transaction ([LibraryStore.saveHomeRows]), so a refusal changes nothing. */ + override suspend fun saveRows(rows: List?, catalogRowsShown: Map): HomeRowsSave { + val change = try { + library.saveHomeRows(rows, catalogRowsShown) + } catch (e: IllegalArgumentException) { + return HomeRowsSave.ROWS_REFUSED } + return when (change) { + CatalogRowChange.SAVED -> HomeRowsSave.SAVED + CatalogRowChange.TOO_MANY -> HomeRowsSave.TOO_MANY + else -> HomeRowsSave.UNKNOWN_ROW + } + } override suspend fun homeApps(): HomeApps { val installed = inventory() diff --git a/app/src/main/kotlin/tv/streamer/app/sources/RoomSourceCatalogStore.kt b/app/src/main/kotlin/tv/streamer/app/sources/RoomSourceCatalogStore.kt index 8b4030a..853ed98 100644 --- a/app/src/main/kotlin/tv/streamer/app/sources/RoomSourceCatalogStore.kt +++ b/app/src/main/kotlin/tv/streamer/app/sources/RoomSourceCatalogStore.kt @@ -33,7 +33,8 @@ import tv.streamer.webserver.api.SourceCatalogs * addon's own scoped client, so the resolver's size, redirect and time bounds hold. A manifest * catalog whose type or id storage cannot hold is left out. TMDB's catalogs are the four built-in * lists ([LibraryStore.TMDB_CATALOGS]); one not stored yet is listed as on, as the next refresh - * stores it, and a save stores all four first so an unticked one stays off. + * stores it, and a save stores every unticked one turned off in the same transaction, so it stays + * off and a refused save changes nothing. */ class RoomSourceCatalogStore( database: StreamerDatabase, @@ -61,7 +62,6 @@ class RoomSourceCatalogStore( LibraryStore.TMDB_CATALOGS.firstOrNull { it.catalogType == key.type && it.catalogId == key.id } ?: return CatalogSaveResult.NotOffered } - library.ensureCatalogRows(sourceId, LibraryStore.TMDB_CATALOGS) } else { manifest = manifest(account) ?: return CatalogSaveResult.Unavailable val offered = rowCatalogs(manifest) @@ -71,7 +71,10 @@ class RoomSourceCatalogStore( CatalogSpec(catalog.type, catalog.id, displayName(catalog)) } } - return when (library.setShownCatalogs(sourceId, specs)) { + // An unticked TMDB list is stored turned off with the save, so the next refresh's defaults + // leave it off; a refused save stores nothing. + val storeOff = if (account.provider == ProviderType.TMDB) LibraryStore.TMDB_CATALOGS else emptyList() + return when (library.setShownCatalogs(sourceId, specs, storeOff)) { CatalogRowChange.SAVED -> { val stored = library.catalogRows(sourceId) CatalogSaveResult.Saved(if (manifest == null) tmdbCatalogs(stored) else joined(manifest, stored)) diff --git a/app/src/test/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresherTest.kt b/app/src/test/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresherTest.kt index 7871450..ac98443 100644 --- a/app/src/test/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresherTest.kt +++ b/app/src/test/kotlin/tv/streamer/app/catalogrows/CatalogRowsRefresherTest.kt @@ -96,9 +96,13 @@ class CatalogRowsRefresherTest { val paths: MutableList = Collections.synchronizedList(ArrayList()) @Volatile var everything: (() -> MockResponse)? = null + /** Runs on each request's path before it is answered. */ + @Volatile var onRequest: ((String) -> Unit)? = null + override fun dispatch(request: RecordedRequest): MockResponse { val path = request.requestUrl!!.encodedPath.removePrefix("/addon/") paths += path + onRequest?.invoke(path) everything?.let { return it() } val page = pages[path.removePrefix("catalog/").removeSuffix(".json")] ?: return MockResponse().setResponseCode(404) return MockResponse().setBody("""{"metas":[${page.joinToString(",") { (type, id) -> @@ -108,6 +112,7 @@ class CatalogRowsRefresherTest { } private inner class Fixture( + val db: StreamerDatabase, val library: LibraryStore, val credentials: CredentialStore, val refresher: CatalogRowsRefresher, @@ -162,7 +167,7 @@ class CatalogRowsRefresherTest { metadataLimits = MetadataLimits(attempts = 1), clock = clock::get, ) - Fixture(LibraryStore(db), credentials, refresher, tmdb, addon, + Fixture(db, LibraryStore(db), credentials, refresher, tmdb, addon, addonServer.url("/addon/manifest.json").toString(), clock).block() } finally { tmdbServer.close() @@ -391,6 +396,77 @@ class CatalogRowsRefresherTest { assertNull(feed.failure) } + @Test fun aRowDeletedMidRunIsLeftOutAnInvalidTitleIsDroppedAndLaterRowsStillRun() = fixture("mid-run") { + val tmdbAccount = saveTmdbKey() + tmdb.lists["movie/popular"] = listOf(1) + val goneAccount = saveAddon() + library.setShownCatalogs(goneAccount, listOf(CatalogSpec("movie", "gone", "Gone"))) + val keptAccount = saveAddon() + library.setShownCatalogs(keptAccount, listOf(CatalogSpec("movie", "top", "Popular"))) + library.putItem(Item("tmdb:movie:603", ItemType.MOVIE, 603, imdbId = "tt0133093", title = "The Matrix")) + // A stored title with a blank name, as storage would refuse it now. + db.openHelper.writableDatabase.execSQL( + "INSERT INTO items (canonicalId, type, tmdbId, imdbId, title) VALUES ('tmdb:movie:5', 'MOVIE', 5, 'tt0000005', '')") + addon.pages["movie/gone"] = listOf("movie" to "tt0133093") + addon.pages["movie/top"] = listOf("movie" to "tt0133093", "movie" to "tt0000005") + val gone = rowId(goneAccount, "movie", "gone") + val kept = rowId(keptAccount, "movie", "top") + // The first addon's source is removed while its page is being fetched. + addon.onRequest = { path -> if (path == "catalog/movie/gone.json") runBlocking { library.deleteAccount(goneAccount) } } + + val result = refresher.refresh() as CatalogRowsRefreshResult.Refreshed + + assertTrue(gone !in result.rows) + assertNull(library.catalogRow(gone)) + assertEquals(CatalogRowRefresh(ProviderType.STREMIO_ADDON, updated(1, dropped = 1)), result.rows[kept]) + assertEquals(listOf("tmdb:movie:603"), titles(kept)) + assertEquals(updated(1), result.rows.getValue(rowId(tmdbAccount, "movie", "popular")).outcome) + assertEquals(5, result.rows.size) + } + + @Test fun anyOtherFailureMarksItsRowStaleAndTheRunGoesOn() = fixture("unexpected") { + val tmdbAccount = saveTmdbKey() + tmdb.lists["movie/popular"] = listOf(1) + val addonAccount = saveAddon() + library.setShownCatalogs(addonAccount, listOf(CatalogSpec("movie", "top", "Popular"))) + library.putItem(Item("tmdb:movie:603", ItemType.MOVIE, 603, imdbId = "tt0133093", title = "The Matrix")) + addon.pages["movie/top"] = listOf("movie" to "tt0133093") + // The addon's account needs signing in again by the time its titles are written. + addon.onRequest = { runBlocking { library.setAccountAuthorized(addonAccount, false) } } + + val result = refresher.refresh() as CatalogRowsRefreshResult.Refreshed + + val row = rowId(addonAccount, "movie", "top") + assertEquals(CatalogRowOutcome.Failed(CatalogRowFailure.UNEXPECTED), result.rows.getValue(row).outcome) + val feed = requireNotNull(feed(row)) + assertEquals(clock.get(), feed.lastAttemptAt) + assertEquals(FeedFailure.UNAVAILABLE, feed.failure) + assertEquals(updated(1), result.rows.getValue(rowId(tmdbAccount, "movie", "popular")).outcome) + } + + @Test fun aLastAttemptLaterThanNowIsDueAndRestampedWithTheRealTime() = fixture("clock-ahead") { + val account = saveTmdbKey() + tmdb.lists["movie/popular"] = listOf(1) + // The first run happens while the TV's clock is a day ahead. + clock.addAndGet(86_400_000) + refresher.refresh() + clock.addAndGet(-86_400_000) + + tmdb.lists["movie/popular"] = listOf(2) + val result = refresher.refresh() as CatalogRowsRefreshResult.Refreshed + + val row = rowId(account, "movie", "popular") + assertEquals(updated(1), result.rows.getValue(row).outcome) + assertEquals(listOf("tmdb:movie:2"), titles(row)) + val feed = requireNotNull(feed(row)) + assertTrue(feed.lastAttemptAt!! in clock.get()..clock.get() + 1, "stamped ${feed.lastAttemptAt} at ${clock.get()}") + // The hourly gate now counts from the real time. + clock.addAndGet(CatalogRowsRefresher.REFRESH_INTERVAL_MILLIS - 2) + assertEquals(CatalogRowsRefreshResult.Refreshed(emptyMap()), refresher.refresh()) + assertTrue(CatalogRowsRefresher.isDue(lastAttemptAt = 10, now = 9)) + assertTrue(!CatalogRowsRefresher.isDue(lastAttemptAt = 10, now = 10)) + } + private class SoftwareKeys : CredentialKeys { val values = mutableMapOf() override fun contains(reference: CredentialReference) = values.containsKey(reference) diff --git a/app/src/test/kotlin/tv/streamer/app/home/RoomHomeSettingsStoreTest.kt b/app/src/test/kotlin/tv/streamer/app/home/RoomHomeSettingsStoreTest.kt index a676fed..ada0342 100644 --- a/app/src/test/kotlin/tv/streamer/app/home/RoomHomeSettingsStoreTest.kt +++ b/app/src/test/kotlin/tv/streamer/app/home/RoomHomeSettingsStoreTest.kt @@ -30,8 +30,8 @@ import tv.streamer.core.model.FeedKind import tv.streamer.core.model.HomeLayout import tv.streamer.core.model.HomeRow import tv.streamer.core.model.ProviderType -import tv.streamer.webserver.api.CatalogRowsSave import tv.streamer.webserver.api.HomeCatalogRow +import tv.streamer.webserver.api.HomeRowsSave import tv.streamer.webserver.api.InstalledHomeApp import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -107,12 +107,21 @@ class RoomHomeSettingsStoreTest { derived.catalogRows, ) - store.setHomeRows(listOf(HomeRow.catalog(series), HomeRow(FeedKind.WATCHLIST))) + assertEquals(HomeRowsSave.SAVED, store.saveRows(listOf(HomeRow.catalog(series), HomeRow(FeedKind.WATCHLIST)), emptyMap())) assertEquals(listOf(HomeRow.catalog(series), HomeRow(FeedKind.WATCHLIST), HomeRow.catalog(top)), store.homeLayout().rows) - assertEquals(CatalogRowsSave.SAVED, store.setCatalogRowsShown(mapOf(series to false))) + assertEquals(HomeRowsSave.SAVED, store.saveRows(null, mapOf(series to false))) assertEquals(listOf(HomeRow(FeedKind.WATCHLIST), HomeRow.catalog(top)), store.homeLayout().rows) assertEquals(listOf(true, false), store.homeLayout().catalogRows.map { it.shown }) - assertEquals(CatalogRowsSave.UNKNOWN_ROW, store.setCatalogRowsShown(mapOf(9_999L to true))) + assertEquals(HomeRowsSave.UNKNOWN_ROW, store.saveRows(listOf(HomeRow(FeedKind.RATINGS)), mapOf(9_999L to true))) + + // A row list storage refuses leaves the switches sent with it as they were, and the reverse. + val before = store.homeLayout() + assertEquals(HomeRowsSave.ROWS_REFUSED, store.saveRows(listOf(HomeRow.catalog(9_999)), mapOf(top to false, series to true))) + assertEquals(HomeRowsSave.ROWS_REFUSED, + store.saveRows(listOf(HomeRow(FeedKind.WATCHLIST), HomeRow(FeedKind.WATCHLIST)), mapOf(top to false))) + assertEquals(before, store.homeLayout()) + assertEquals(HomeRowsSave.SAVED, store.saveRows(listOf(HomeRow(FeedKind.RATINGS)), mapOf(top to false, series to true))) + assertEquals(listOf(HomeRow(FeedKind.RATINGS), HomeRow.catalog(series)), store.homeLayout().rows) } private companion object { diff --git a/app/src/test/kotlin/tv/streamer/app/sources/RoomSourceCatalogStoreTest.kt b/app/src/test/kotlin/tv/streamer/app/sources/RoomSourceCatalogStoreTest.kt index 9f5ebd2..4751765 100644 --- a/app/src/test/kotlin/tv/streamer/app/sources/RoomSourceCatalogStoreTest.kt +++ b/app/src/test/kotlin/tv/streamer/app/sources/RoomSourceCatalogStoreTest.kt @@ -166,6 +166,18 @@ class RoomSourceCatalogStoreTest { assertSame(CatalogSaveResult.NotOffered, store.setShown(id, listOf(CatalogKey("movie", "top")))) } + @Test fun aTmdbSaveRefusedForTooManyRowsStoresNothing() = fixture("tmdb-full") { + val addonId = account(ProviderType.STREMIO_ADDON, addonUrl) + library.setShownCatalogs(addonId, (1..LibraryStore.MAX_SHOWN_CATALOG_ROWS).map { CatalogSpec("movie", "c$it", "C$it") }) + val id = account(ProviderType.TMDB, "key-1") + + assertSame(CatalogSaveResult.TooMany, store.setShown(id, listOf(CatalogKey("movie", "popular")))) + assertTrue(library.catalogRows(id).isEmpty()) + // Unticking everything turns nothing on, so it is saved, all four off. + assertIs(store.setShown(id, emptyList())) + assertEquals(List(4) { false }, library.catalogRows(id).map { it.shown }) + } + @Test fun onlyHouseholdAddonAndTmdbSourcesHaveCatalogs() = fixture("scope") { val other = account(ProviderType.TRAKT, "token") assertNull(store.catalogs(other)) diff --git a/core/database/src/main/kotlin/tv/streamer/core/database/LibraryStore.kt b/core/database/src/main/kotlin/tv/streamer/core/database/LibraryStore.kt index 7d5a4c0..a6d90d1 100644 --- a/core/database/src/main/kotlin/tv/streamer/core/database/LibraryStore.kt +++ b/core/database/src/main/kotlin/tv/streamer/core/database/LibraryStore.kt @@ -417,6 +417,17 @@ class LibraryStore(private val database: StreamerDatabase) { }) } + /** + * [setCatalogRowsShown], then [setHomeRows] when [rows] is given, in one transaction: a refused + * switch saves no rows, and a row list [setHomeRows] refuses (it throws) leaves the switches as + * they were. + */ + suspend fun saveHomeRows(rows: List?, shown: Map): CatalogRowChange = database.withTransaction { + val change = setCatalogRowsShown(shown) + if (change == CatalogRowChange.SAVED && rows != null) setHomeRows(rows) + change + } + /** Every catalog row in id order, shown or not. */ suspend fun catalogRows(): List = dao.catalogRows().map { it.toDomain() } @@ -430,12 +441,19 @@ class LibraryStore(private val database: StreamerDatabase) { * renamed to its given name when stored; every other catalog row of the account is turned off * and kept. Refused as a whole ([CatalogRowChange.TOO_MANY]) when it turns a row on and more * than [MAX_SHOWN_CATALOG_ROWS] would then be shown across all sources. [shown] must name each - * catalog once. + * catalog once. Each of [storeOff] that is neither stored nor in [shown] is stored turned off, in + * the same transaction, so a later default ([ensureCatalogRows]) cannot turn it on; a refusal + * stores nothing. */ - suspend fun setShownCatalogs(accountId: Long, shown: List): CatalogRowChange = database.withTransaction { + suspend fun setShownCatalogs( + accountId: Long, + shown: List, + storeOff: List = emptyList(), + ): CatalogRowChange = database.withTransaction { val account = dao.account(accountId) ?: return@withTransaction CatalogRowChange.UNKNOWN_ACCOUNT if (account.provider !in CatalogRowRules.SOURCES) return@withTransaction CatalogRowChange.NOT_A_CATALOG_SOURCE shown.forEach { CatalogRowRules.validate(account.provider, it) } + storeOff.forEach { CatalogRowRules.validate(account.provider, it) } val keys = shown.map { it.catalogType to it.catalogId } require(keys.distinct().size == keys.size) { "Duplicate catalog" } val stored = dao.catalogRows(accountId).associateBy { it.catalogType to it.catalogId } @@ -454,6 +472,10 @@ class LibraryStore(private val database: StreamerDatabase) { dao.updateCatalogRow(row.id, spec.name, true) } } + storeOff.filter { (it.catalogType to it.catalogId) !in keys && (it.catalogType to it.catalogId) !in stored }.forEach { + dao.insertCatalogRow(CatalogRowEntity(accountId = accountId, catalogType = it.catalogType, + catalogId = it.catalogId, name = it.name, shown = false)) + } CatalogRowChange.SAVED } @@ -474,17 +496,22 @@ class LibraryStore(private val database: StreamerDatabase) { } /** - * Inserts each of [defaults] the account does not yet have, turned on, regardless of - * [MAX_SHOWN_CATALOG_ROWS]; a stored row is never changed, so one turned off stays off. For the - * built-in TMDB lists ([TMDB_CATALOGS]). Returns the account's catalog rows. + * Inserts each of [defaults] the account does not yet have, in order, turned on while fewer than + * [MAX_SHOWN_CATALOG_ROWS] are shown and turned off after that; a stored row is never changed, so + * one turned off stays off. For the built-in TMDB lists ([TMDB_CATALOGS]). Returns the account's + * catalog rows. */ suspend fun ensureCatalogRows(accountId: Long, defaults: List): List = database.withTransaction { val account = requireNotNull(dao.account(accountId)) { "Unknown account" } require(account.provider in CatalogRowRules.SOURCES) { "Not a catalog source" } defaults.forEach { CatalogRowRules.validate(account.provider, it) } - defaults.forEach { + val stored = dao.catalogRows(accountId).map { it.catalogType to it.catalogId }.toSet() + var shown = dao.countShownCatalogRows() + defaults.distinctBy { it.catalogType to it.catalogId }.filter { (it.catalogType to it.catalogId) !in stored }.forEach { + val on = shown < MAX_SHOWN_CATALOG_ROWS dao.insertCatalogRow(CatalogRowEntity(accountId = accountId, catalogType = it.catalogType, - catalogId = it.catalogId, name = it.name, shown = true)) + catalogId = it.catalogId, name = it.name, shown = on)) + if (on) shown++ } dao.catalogRows(accountId).map { it.toDomain() } } @@ -518,13 +545,42 @@ class LibraryStore(private val database: StreamerDatabase) { PrimaryAccountPolicy.primary(dao.accounts(user.value).map { it.toDomain() }, capability, dao.primary(user.value, capability)) } - private suspend fun saveItem(item: Item) { + /** + * Whether [item] is a title [putItem] and feed updates accept: a canonical title id matching its + * type and TMDB id, and a title that is not blank. + */ + fun isStorableTitle(item: Item): Boolean = try { + requireStorableTitle(item) + true + } catch (e: IllegalArgumentException) { + false + } + + /** + * Moves feed [id]'s last attempt and fetch times back to [now] where they are later, as when they + * were stored while the clock ran ahead, so the next attempt at the real time commits and is not + * taken for an older one. A feed that does not exist is left alone. + */ + suspend fun clampFeedTimes(id: Long, now: Long) = database.withTransaction { + val feed = dao.feed(id) ?: return@withTransaction + val lastAttemptAt = feed.lastAttemptAt?.coerceAtMost(now) + val fetchedAt = feed.fetchedAt?.coerceAtMost(now) + if (lastAttemptAt != feed.lastAttemptAt || fetchedAt != feed.fetchedAt) { + dao.saveFeed(feed.copy(lastAttemptAt = lastAttemptAt, fetchedAt = fetchedAt)) + } + } + + private fun requireStorableTitle(item: Item) { val media = validateTitleId(item.canonicalId) require(item.tmdbId > 0 && item.title.isNotBlank()) require(when (item.type) { ItemType.MOVIE -> media == MediaRef.Movie(item.tmdbId) ItemType.SERIES -> media == MediaRef.Series(item.tmdbId) }) { "Item identity does not match metadata" } + } + + private suspend fun saveItem(item: Item) { + requireStorableTitle(item) val old = dao.item(item.canonicalId)?.item // Sparse provider entries must not erase richer cached metadata. dao.saveItem(ItemEntity(item.copy(imdbId = item.imdbId ?: old?.imdbId, tvdbId = item.tvdbId ?: old?.tvdbId, diff --git a/core/database/src/test/kotlin/tv/streamer/core/database/CatalogRowStorageTest.kt b/core/database/src/test/kotlin/tv/streamer/core/database/CatalogRowStorageTest.kt index b27653b..b1d51f3 100644 --- a/core/database/src/test/kotlin/tv/streamer/core/database/CatalogRowStorageTest.kt +++ b/core/database/src/test/kotlin/tv/streamer/core/database/CatalogRowStorageTest.kt @@ -112,7 +112,7 @@ class CatalogRowStorageTest { assertTrue(store.ensureCatalogRows(tmdb(), LibraryStore.TMDB_CATALOGS).all { it.shown }) } - @Test fun turningRowsOnStopsAtTheLimitButDefaultsAndTurningOffDoNot() = runTest { + @Test fun turningRowsOnAndInsertingDefaultsStopAtTheLimit() = runTest { val cinemeta = addon() val ids = (1..LibraryStore.MAX_SHOWN_CATALOG_ROWS).map { "c$it" } assertEquals(CatalogRowChange.SAVED, store.setShownCatalogs(cinemeta, ids.map { spec(it) })) @@ -125,15 +125,19 @@ class CatalogRowStorageTest { assertFalse(c1.shown) assertEquals(CatalogRowChange.TOO_MANY, store.setCatalogRowsShown(mapOf(c1.id to true))) - // The TMDB defaults go in regardless, leaving 24 shown; turning rows off, or saving without - // turning any on, is still allowed above the limit. - store.ensureCatalogRows(tmdb(), LibraryStore.TMDB_CATALOGS) - assertEquals(LibraryStore.MAX_SHOWN_CATALOG_ROWS + 4, store.catalogRows().count { it.shown }) + // At the limit the TMDB defaults go in turned off; saving without turning any on is allowed. + assertEquals(List(4) { false }, store.ensureCatalogRows(tmdb(), LibraryStore.TMDB_CATALOGS).map { it.shown }) + assertEquals(LibraryStore.MAX_SHOWN_CATALOG_ROWS, store.catalogRows().count { it.shown }) assertEquals(CatalogRowChange.SAVED, store.setShownCatalogs(cinemeta, (ids.drop(1) + "c21").map { spec(it, "New $it") })) + + // With room for two, the first two defaults go in on and the rest off. val shown = store.catalogRows().filter { it.shown } - assertEquals(CatalogRowChange.TOO_MANY, store.setCatalogRowsShown(mapOf(c1.id to true, shown[0].id to false))) - assertEquals(CatalogRowChange.SAVED, store.setCatalogRowsShown(shown.take(5).associate { it.id to false })) - assertEquals(CatalogRowChange.SAVED, store.setCatalogRowsShown(mapOf(c1.id to true))) + assertEquals(CatalogRowChange.SAVED, store.setCatalogRowsShown(shown.take(2).associate { it.id to false })) + assertEquals(listOf(true, true, false, false), store.ensureCatalogRows(tmdb(), LibraryStore.TMDB_CATALOGS).map { it.shown }) + assertEquals(LibraryStore.MAX_SHOWN_CATALOG_ROWS, store.catalogRows().count { it.shown }) + + // Swapping one row for another at the limit is allowed. + assertEquals(CatalogRowChange.SAVED, store.setCatalogRowsShown(mapOf(c1.id to true, shown[2].id to false))) assertEquals(LibraryStore.MAX_SHOWN_CATALOG_ROWS, store.catalogRows().count { it.shown }) val before = store.catalogRows() @@ -141,6 +145,26 @@ class CatalogRowStorageTest { assertEquals(before, store.catalogRows()) } + @Test fun aTmdbSaveStoresUntickedListsOffAndARefusedOneStoresNothing() = runTest { + val cinemeta = addon() + val ids = (1 until LibraryStore.MAX_SHOWN_CATALOG_ROWS).map { "c$it" } + assertEquals(CatalogRowChange.SAVED, store.setShownCatalogs(cinemeta, ids.map { spec(it) })) + val key = tmdb() + val (popularMovies, popularSeries) = LibraryStore.TMDB_CATALOGS + + // One place left: ticking two lists is refused and stores none of the four. + assertEquals(CatalogRowChange.TOO_MANY, + store.setShownCatalogs(key, listOf(popularMovies, popularSeries), storeOff = LibraryStore.TMDB_CATALOGS)) + assertEquals(emptyList(), store.catalogRows(key)) + + // Ticking one stores it on and the other three off. + assertEquals(CatalogRowChange.SAVED, + store.setShownCatalogs(key, listOf(popularSeries), storeOff = LibraryStore.TMDB_CATALOGS)) + assertEquals(listOf("series-popular" to true, "movie-popular" to false, "movie-trending" to false, "series-trending" to false), + store.catalogRows(key).map { "${it.catalogType}-${it.catalogId}" to it.shown }) + assertEquals(LibraryStore.MAX_SHOWN_CATALOG_ROWS, store.catalogRows().count { it.shown }) + } + @Test fun eachRowGetsItsOwnCatalogFeedAndTheSnapshotCarriesItsOwnerLabelAndFeed() = runTest { val ana = store.createUser("Ana", 1) val household = addon() diff --git a/docs/catalog-rows.md b/docs/catalog-rows.md index bd6d8f3..ae514cc 100644 --- a/docs/catalog-rows.md +++ b/docs/catalog-rows.md @@ -40,7 +40,10 @@ The routes under "Phone page routes" are built: `webserver/.../api/SourceCatalog The phone page uses them (`app.js`; `docs/web-server.md` "Phone page"): a Catalogs list with ticks under each addon and the TMDB key (`catalogPanel`, `saveCatalogs`), and in the Home screen section catalog rows kept in the row list by id (`rowToSend`) plus an on/off switch per catalog -row (`renderCatalogRows`), sent as `hiddenCatalogRows` by `saveHome`. `PhonePageTest +row (`renderCatalogRows`), sent as `hiddenCatalogRows` by `saveHome`. An unsaved row list that +names a catalog row the TV no longer has loses that row (`dropGoneCatalogRows`) when the settings +reload and when a save is refused with `details.catalogRowId`, keeping the rest of the edit for +another Save. `PhonePageTest theScriptsCatalogCallsMatchTheApi` checks the script's calls and bodies against the routes; the script has not run in a browser. @@ -172,7 +175,11 @@ Four built-in lists of the household TMDB key, each a catalog row of the `TMDB` ticks with `shown` true and the manifest's name, and sets `shown` false for unticked ones. At most 20 catalog rows may be shown across all sources (`MAX_SHOWN_CATALOG_ROWS`), the four TMDB rows included, to bound the work of one refresh. The limit is checked when a tick or a switch - turns a row on; the TMDB defaults are inserted regardless. + turns a row on, and holds for the TMDB defaults too: `ensureCatalogRows` inserts a missing one + turned on only while fewer than 20 rows are shown, and turned off after that. Ticking TMDB's lists + stores the unticked ones turned off in the same transaction as the ticks + (`LibraryStore.setShownCatalogs` with `storeOff`), so they stay off, and a save refused for too + many rows stores none of them. ## Refresh @@ -183,9 +190,12 @@ beside `ForYouOnOpen` and in the same way (`docs/for-you.md`; decision D14): und - **Which rows.** Every shown catalog row whose feed's `lastAttemptAt` is an hour old or more, or that has no feed yet (`spec.md` refresh table: "On launcher open, at most hourly"). The gate is the stored attempt time, so it holds across process restarts, and a failing source is asked at - most hourly too. Rows turned off, and rows of an account that needs reauthorization, are not -refreshed. Rows run one after another; one row's failure - never stops the next. + most hourly too. A stored attempt time later than now, which only a clock that ran ahead (as + before network time is set) can leave, counts as due (`CatalogRowsRefresher.isDue`); the row's + stored times are moved back to now (`LibraryStore.clampFeedTimes`) before the new attempt is + written, so the hour then counts from the real time. Rows turned off, and rows of an account that + needs reauthorization, are not refreshed. Rows run one after another; one row's failure never + stops the next. - **Paging.** One page per row: the addon's first page (no `skip`), of which the first 40 titles are kept (`MAX_CATALOG_ROW_TITLES`); TMDB's page 1 (20 titles). - **Mapping addon titles to canonical ids.** A catalog entry whose id is an IMDb id (`tt` and @@ -222,6 +232,14 @@ Every write is `FeedUpdate.Complete` after a whole page and all its mappings suc holds for a source that has never answered. - **A complete empty page** clears the row (`FeedUpdate.Complete` with no entries), as the storage contract allows. +- **A title storage would refuse** (`LibraryStore.isStorableTitle`: a blank name, or an id that does + not match its type and TMDB id, as a stored record written before today's checks can have) is + dropped and counted in `dropped`; the rest of the row is stored. +- **A row deleted during the run** (its source or catalog removed while its page was fetched) is + left out of the result with nothing written and no event. +- **Anything else failing for one row** (storage refusing the write, say because its account needs + signing in again by then) marks that row stale with its attempt time, as `UNEXPECTED`, and the + run goes on to the next row, so the row is not retried on every open and later rows still run. Events, recorded by `CatalogRowsOnOpen` from the refresher's result: `catalog_row_refreshed` (`source`, `titles`, `dropped`) and `catalog_row_refresh_failed` (`source`, `failure`, a @@ -235,10 +253,10 @@ Under `/api/v1`, through the same gate as the Sources and Settings routes. | Route | Body | Answer | | --- | --- | --- | -| `GET /sources/{id}/catalogs` | | 200 `{"available":true,"catalogs":[{"type":"movie","id":"top","name":"Popular","homeRow":true,"offered":true,"needsExtra":false,"rowId":7}]}`. For the TMDB source, the four built-in lists without a network call; a list not stored yet shows `homeRow` true and `rowId` null, and a save stores all four first. 404 for an unknown id or a source that is neither an addon nor TMDB. | +| `GET /sources/{id}/catalogs` | | 200 `{"available":true,"catalogs":[{"type":"movie","id":"top","name":"Popular","homeRow":true,"offered":true,"needsExtra":false,"rowId":7}]}`. For the TMDB source, the four built-in lists without a network call; a list not stored yet shows `homeRow` true and `rowId` null, and a save stores the unticked ones turned off with the ticks. 404 for an unknown id or a source that is neither an addon nor TMDB. | | `PUT /sources/{id}/catalogs` | `{"shown":[{"type":"movie","id":"top"}]}` | 200 and the same shape as `GET`, after saving. 400 `VALIDATION` for a catalog not offered, one that needs an extra, a duplicate, or more than `MAX_SHOWN_CATALOG_ROWS` shown in total; 502 `PROVIDER_UNAVAILABLE` when the manifest cannot be fetched (nothing changes). | | `GET /settings` | | Adds `catalogRows`: every stored catalog row as `{"id":7,"label":"Popular · v3-cinemeta.strem.io","userId":1,"shown":true}` (the TMDB defaults once a refresh has stored them). `homeRows` lists Home's actual rows, shown catalog rows included as `{"catalogRowId":7}`. | -| `PUT /settings` | `homeRows` may carry `{"catalogRowId":7}` entries; new `hiddenCatalogRows`, the whole list of catalog row ids turned off | 200 as `GET`. 400 `VALIDATION` for an unknown id, a catalog row listed twice, a catalog row with a pin, a bare `CATALOG` kind, or one more shown row than the limit. | +| `PUT /settings` | `homeRows` may carry `{"catalogRowId":7}` entries; new `hiddenCatalogRows`, the whole list of catalog row ids turned off | 200 as `GET`. 400 `VALIDATION` for an unknown id (with `details.catalogRowId`), a catalog row listed twice, a catalog row with a pin, a bare `CATALOG` kind, or one more shown row than the limit. The switches and the row list are saved in one transaction, so any refusal changes neither. | - The Sources list and the Home screen section are two views of the same switch (`shown`). - A Household catalog row is open to every tailnet browser. A person's catalog row is that diff --git a/docs/decisions.md b/docs/decisions.md index 166f467..8cecc65 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -27,4 +27,4 @@ Deviations from the spec and the reasons. Anything not listed here follows the s | D21 | Who may change a Home row pinned to a person | `PUT /settings` saves the whole row list; any tailnet browser may reorder it, pinned rows included, but adding, removing or moving a pin to a person (a pin is the pair of person and row kind) needs a browser that acts for that person, else 403 `NOT_OWNER`. A Household browser changes no one's pins | `spec.md` makes Home screen Household work and rows pinned to a person that person's, without saying whether reordering moves someone's row. Counting position would let one person's pin freeze the whole list for everyone else, so only which rows are pinned to whom is the person's. See `docs/home-settings-api.md`. | | D22 | SMTP client for sign-in links | A small SMTP submission client written in `:webserver` (`webserver/.../mail/SmtpClient`) over `java.net.Socket` and `javax.net.ssl`, with no mail library: `STARTTLS` or implicit TLS only, never plain text; `AUTH PLAIN` or `AUTH LOGIN`; one plain-text ASCII message to one recipient. Trust comes from an injected `SSLSocketFactory` and `HostnameVerifier` (`:app`: the platform defaults). Tests use a scripted fake server on loopback with a test-time certificate from `okhttp-tls`, a test-only dependency | The TV sends one kind of message, a fixed ASCII text with a link, so the protocol needed is the handful of commands above, and `java.net` and `javax.net.ssl` behave the same on Android and the JVM test side. `spec.md` asks for minimal dependencies under the privacy goals. Jakarta Mail brings Jakarta Activation and finds its protocol providers through resource files and reflection, which R8 has to be told to keep and which fail only in a shrunk release build, and it carries IMAP, POP3 and MIME parsing nothing here uses. Apache Commons Net's SMTP client is plain Java, but its exceptions and reply accessors carry server text, which echoes addresses back and would have to be scrubbed at every call. Writing the subset keeps every failure a closed `SourceTestFailure`, every wait bounded and cancellable, and the release APK's dependency graph unchanged. Hostname checks go through `HostnameVerifier` because `SSLParameters.endpointIdentificationAlgorithm` needs API 24 and minSdk is 23. What this gives up: servers that require OAuth (`XOAUTH2`) or offer only `CRAM-MD5`, and relays without authentication, are not supported. See `docs/email-signin.md`. | | D23 | Who may change a person's email address | Setting, changing or clearing a person's sign-in address takes effect only after OK on the TV, through `OwnerApprovals` (`ApprovalKind.SET_EMAIL`), with the same five-minute expiry and Back to refuse. `PATCH /users/{id}` with `email` answers 202 whether or not another person has the address; a duplicate is found when the TV approves, and the TV screen says so and changes nothing. Name, colour and avatar in the same request still change at once | A sign-in link pairs a browser as the person without the TV, so whoever chooses the address chooses who can become that person. Any tailnet browser may act as a person no other browser holds without asking the TV, so without this a browser could set that person's address to an inbox it controls, switch back, and later redeem a link as them with no one at the TV having seen anything. Mailing a code to the new address would prove nothing, since the asking browser controls that inbox; the person at the TV is the only check. Answering the same 202 for a taken address keeps `PATCH` from telling a browser which addresses are stored, which `POST /signin-link` is already careful not to reveal. See `docs/email-signin.md` "Setting an address". | -| D24 | Catalog rows on Home | Each ticked addon catalog and each of four TMDB lists (popular and trending, movies and series: `/movie/popular`, `/tv/popular`, `/trending/movie/week`, `/trending/tv/week`) is its own Home row. A new `catalog_rows` table (schema 12) holds one row per catalog of a source account (account, catalog type and id, name copied at tick time, `shown`); its titles are a `CATALOG` feed on that account. `home_rows` gains a nullable `catalogRowId`, so the row list orders catalog rows next to library rows; a shown catalog row the saved list does not name goes at the end, and a catalog row is hidden only by turning it off, never by leaving it out. Catalog rows are never pinned. The TMDB lists are on by default: every refresh inserts any of the four that is missing as shown and never changes an existing one. Refresh runs on Home open, at most hourly per row by the feed's stored attempt time, one page per row (addon titles capped at 40, TMDB page 1); addon titles become canonical through a stored item's IMDb id, then TMDB `find`, and titles without a TMDB match are left out | `spec.md` asks for one row per selected catalog but stores rows by feed kind, so several catalogs as `CATALOG` feeds would merge into one row. A table of its own keeps a turned-off TMDB list off across refreshes and keeps a re-ticked catalog's titles, which deleting the record would lose; holding the owner through the account makes a personal addon's catalogs that person's with no new ownership rule. Appending unnamed rows makes a ticked catalog appear at once even after the row list was saved, which is why leaving a row out cannot mean hiding it. The TMDB lists are new: Home otherwise shows only what people already saved plus For you, so a household with no tracking account sees almost nothing; they use the key the TV already holds and are on by default so they get tried, one switch each. A stored attempt time, unlike the in-memory TMDB cache, survives process restarts, so neither an addon nor TMDB is asked more than hourly; popular and trending answers stay out of the 7-day cache for the same reason. Home holds canonical titles only (`spec.md` "ID mapping"), so an unmapped title has no place in a row. Not in the configuration backup, like the row list itself. See `docs/catalog-rows.md`. | +| D24 | Catalog rows on Home | Each ticked addon catalog and each of four TMDB lists (popular and trending, movies and series: `/movie/popular`, `/tv/popular`, `/trending/movie/week`, `/trending/tv/week`) is its own Home row. A new `catalog_rows` table (schema 12) holds one row per catalog of a source account (account, catalog type and id, name copied at tick time, `shown`); its titles are a `CATALOG` feed on that account. `home_rows` gains a nullable `catalogRowId`, so the row list orders catalog rows next to library rows; a shown catalog row the saved list does not name goes at the end, and a catalog row is hidden only by turning it off, never by leaving it out. Catalog rows are never pinned. The TMDB lists are on by default: every refresh inserts any of the four that is missing as shown (turned off instead once 20 catalog rows are shown) and never changes an existing one. Refresh runs on Home open, at most hourly per row by the feed's stored attempt time, one page per row (addon titles capped at 40, TMDB page 1); addon titles become canonical through a stored item's IMDb id, then TMDB `find`, and titles without a TMDB match are left out | `spec.md` asks for one row per selected catalog but stores rows by feed kind, so several catalogs as `CATALOG` feeds would merge into one row. A table of its own keeps a turned-off TMDB list off across refreshes and keeps a re-ticked catalog's titles, which deleting the record would lose; holding the owner through the account makes a personal addon's catalogs that person's with no new ownership rule. Appending unnamed rows makes a ticked catalog appear at once even after the row list was saved, which is why leaving a row out cannot mean hiding it. The TMDB lists are new: Home otherwise shows only what people already saved plus For you, so a household with no tracking account sees almost nothing; they use the key the TV already holds and are on by default so they get tried, one switch each. A stored attempt time, unlike the in-memory TMDB cache, survives process restarts, so neither an addon nor TMDB is asked more than hourly; popular and trending answers stay out of the 7-day cache for the same reason. Home holds canonical titles only (`spec.md` "ID mapping"), so an unmapped title has no place in a row. Not in the configuration backup, like the row list itself. See `docs/catalog-rows.md`. | diff --git a/docs/home-settings-api.md b/docs/home-settings-api.md index 1375067..323a514 100644 --- a/docs/home-settings-api.md +++ b/docs/home-settings-api.md @@ -8,8 +8,8 @@ default view and region have no fields yet. Code: `webserver/src/main/kotlin/tv/streamer/webserver/api/Settings.kt`, installed by `Application.streamerApi` when `StreamerApiDeps.settings` is set. The `:app` store is `app/src/main/kotlin/tv/streamer/app/home/RoomHomeSettingsStore.kt`: rows over -`LibraryStore.homeRowSettings()` and `LibraryStore.setHomeRows()`, catalog switches over -`LibraryStore.setCatalogRowsShown()`, apps over `HomeRepository` +`LibraryStore.homeRowSettings()`, rows and catalog switches saved together through +`LibraryStore.saveHomeRows()` (one transaction), apps over `HomeRepository` (`layout`, `setLayout`, and its installed-app inventory); wired in `WebServerApi`. Tests: `webserver/src/test/kotlin/tv/streamer/webserver/api/HomeSettingsApiTest.kt`, `core/database/src/test/kotlin/tv/streamer/core/database/HomeRowStorageTest.kt`, @@ -53,7 +53,8 @@ Both routes pass the gate `streamerApi` describes; the LAN reads, and a `PUT` fr rows), a catalog row with a pin or another kind, an unknown catalog row id (in `homeRows` or `hiddenCatalogRows`), a row that appears twice (the same kind may appear once unpinned and once per person), a catalog row id listed twice in `hiddenCatalogRows`, a pin to Household or to - someone not on the TV, or a list the store refuses. + someone not on the TV, or a list the store refuses. A catalog row id the TV does not have is + named in `details.catalogRowId`, so the phone page can leave it out of an unsaved list. ## Apps @@ -71,8 +72,9 @@ Both routes pass the gate `streamerApi` describes; the LAN reads, and a `PUT` fr - Apps belong to the household: every tailnet browser may change them, whoever it acts for. - A body may carry rows, catalog switches and apps at once. Every check, the ownership check included, runs before anything is saved, so a refused body changes nothing. The catalog switches - are saved first, since only the store can tell that too many rows would be shown; that refusal - also leaves rows and apps unchanged. + and the row list are saved in one transaction (`HomeSettingsStore.saveRows`), before the apps, + since only the store can tell that too many rows would be shown or that it refuses the list; + either refusal leaves switches, rows and apps unchanged. ## Whose rows these are diff --git a/docs/sources-api.md b/docs/sources-api.md index 285a69d..a88645f 100644 --- a/docs/sources-api.md +++ b/docs/sources-api.md @@ -87,7 +87,9 @@ manifest's `type` and `id`, its name, whether Home shows it as a row, whether th now, whether it needs a value such as `search` (so it cannot be a row), and its catalog row id (null for a catalog never ticked). What ticking does, and why, is `docs/catalog-rows.md` ("Addon catalogs: listing and ticking"). A TMDB list not stored yet is listed with `homeRow` true, since the -next refresh on Home stores it on; a save stores all four first, so an unticked one stays off. +next refresh on Home stores it on while fewer than 20 rows are shown; a save stores the unticked +ones turned off in the same transaction as the ticks, so they stay off, and a save refused for too +many rows stores none of them. Catalog `type` is 1 to 64 characters, `id` 1 to 128, neither with surrounding spaces or control characters. The catalog routes are installed only when `SourcesDeps.catalogs` is set; the `:app` store is `app/.../sources/RoomSourceCatalogStore.kt` (`RoomSourceCatalogStoreTest`), which fetches diff --git a/docs/storage.md b/docs/storage.md index c918bb7..8e7909b 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -67,8 +67,10 @@ but no live-provider or application-lifecycle binding. No provider, device servi ticked, and `shown`. A row turned off is kept. The owner is the account's user. Each row's titles are one `CATALOG` feed on the same account with catalog key `stremio::` or `tmdb:-`, made by `LibraryStore.catalogRowFeedId`. `LibraryStore.setShownCatalogs` - saves one source's ticks, `setCatalogRowsShown` flips switches, and `ensureCatalogRows` inserts - the TMDB defaults without touching a stored row; turning a row on is refused (`TOO_MANY`) when + saves one source's ticks (storing any `storeOff` catalogs not yet stored turned off, in the same + transaction), `setCatalogRowsShown` flips switches, `saveHomeRows` flips switches and replaces + the Home row list in one transaction, and `ensureCatalogRows` inserts the TMDB defaults without + touching a stored row, turned off once 20 are shown; turning a row on is refused (`TOO_MANY`) when more than `LibraryStore.MAX_SHOWN_CATALOG_ROWS` (20) would be shown. A catalog type never contains `:`, and every part is trimmed, non-empty and free of control characters. Not in the configuration export. diff --git a/docs/web-server.md b/docs/web-server.md index 746d2e2..b9eb9c8 100644 --- a/docs/web-server.md +++ b/docs/web-server.md @@ -84,7 +84,9 @@ Streamer is not verified; the request waits either way and the activity can be s "Add a row" choice of the kinds with no row yet. A row pinned to someone else shows whose it is and cannot have its pin changed or be removed (`docs/decisions.md` D21), though it can move. A catalog row in that list is labelled from `catalogRows`, has Up and Down only (no pin, no - Remove) and is kept as `{"catalogRowId":…}`. Under Catalog rows, every catalog row on the TV + Remove) and is kept as `{"catalogRowId":…}`; one the TV no longer has is left out of an unsaved + list when the settings reload or a save is refused naming it (`details.catalogRowId`), and the + rest of the edit is kept for another Save. Under Catalog rows, every catalog row on the TV with an on/off switch, a person's row named after them and switchable only from a browser acting for them; Save sends the switches as `hiddenCatalogRows` (`docs/catalog-rows.md`). Below the rows, Home's apps in three lists: Pinned apps in order with Up, Down and Unpin; every diff --git a/webserver/src/main/kotlin/tv/streamer/webserver/api/Settings.kt b/webserver/src/main/kotlin/tv/streamer/webserver/api/Settings.kt index d0c0850..b0e289f 100644 --- a/webserver/src/main/kotlin/tv/streamer/webserver/api/Settings.kt +++ b/webserver/src/main/kotlin/tv/streamer/webserver/api/Settings.kt @@ -31,15 +31,18 @@ data class HomeLayout(val rows: List, val saved: Boolean, val catalogRo */ data class HomeCatalogRow(val id: Long, val label: String, val userId: Long, val shown: Boolean) -/** What [HomeSettingsStore.setCatalogRowsShown] did. */ -enum class CatalogRowsSave { +/** What [HomeSettingsStore.saveRows] did. */ +enum class HomeRowsSave { SAVED, /** More catalog rows would be shown than the TV allows. Nothing changed. */ TOO_MANY, - /** An id names no catalog row. Nothing changed. */ + /** A catalog row id names no catalog row. Nothing changed. */ UNKNOWN_ROW, + + /** The store refused the row list (for example, a row named twice). Nothing changed. */ + ROWS_REFUSED, } /** An app installed on the TV, by [packageName], with the [label] its Home card shows. */ @@ -58,11 +61,12 @@ data class HomeApps(val installed: List, val pinned: List) - - /** Turns catalog rows on (true) or off (false) by id, all or none; rows not named stay as they are. */ - suspend fun setCatalogRowsShown(shown: Map): CatalogRowsSave + /** + * Saves Home's rows as one change: [catalogRowsShown] turns catalog rows on (true) or off (false) + * by id, rows not named staying as they are, and [rows], when given, replaces the saved rows (an + * empty list returns Home to its derived rows). Anything but [HomeRowsSave.SAVED] changes nothing. + */ + suspend fun saveRows(rows: List?, catalogRowsShown: Map): HomeRowsSave suspend fun homeApps(): HomeApps @@ -127,8 +131,9 @@ internal suspend fun ApplicationCall.showSettings(deps: StreamerApiDeps, setting * person is that person's: adding, removing or moving such a pin needs a browser that acts for * them (403 `NOT_OWNER`), and so does turning a person's catalog row on or off. Reordering rows, * pinned and catalog ones included, does not; nor does any app change. Everything is checked - * before anything is saved. The catalog switches are saved first, because only the store can tell - * that too many rows would be shown, and that refusal then leaves rows and apps unchanged too. + * before anything is saved. The catalog switches and the row list are saved together in one store + * call ([HomeSettingsStore.saveRows]), since only the store can tell that too many rows would be + * shown or that it refuses the row list; either refusal leaves switches, rows and apps unchanged. */ internal suspend fun ApplicationCall.updateSettings(deps: StreamerApiDeps, settings: SettingsState, log: ApiEventLog) { val device = gate(deps, pairing = false)!! @@ -159,32 +164,31 @@ internal suspend fun ApplicationCall.updateSettings(deps: StreamerApiDeps, setti if (pinnedApps != null && hiddenApps != null) checkApps(pinnedApps, hiddenApps, store.homeApps()) val layout = store.homeLayout() val catalogIds = layout.catalogRows.map { it.id }.toSet() - if (rows != null && rows.any { row -> row.catalogRowId?.let { it !in catalogIds } == true }) { - throw validation("That catalog row is not on the TV.", "field" to "catalogRowId") + // A catalog row not on the TV names its id in details.catalogRowId, so the page can leave it out. + rows?.firstNotNullOfOrNull { row -> row.catalogRowId?.takeIf { it !in catalogIds } }?.let { gone -> + throw validation("That catalog row is not on the TV.", "field" to "catalogRowId", "catalogRowId" to gone.toString()) } // The catalog rows whose switch this request flips. val flipped = hiddenCatalogRows?.let { hidden -> - if (hidden.any { it !in catalogIds }) throw validation("That catalog row is not on the TV.", "field" to "hiddenCatalogRows") + hidden.firstOrNull { it !in catalogIds }?.let { gone -> + throw validation("That catalog row is not on the TV.", "field" to "hiddenCatalogRows", "catalogRowId" to gone.toString()) + } layout.catalogRows.filter { it.shown == (it.id in hidden) } }.orEmpty() val owners = (rows?.let { changedPins(layout.rows, it) }.orEmpty() + flipped.map { it.userId }.filter { it != HOUSEHOLD_ID }).distinct().sorted() owners.firstOrNull { it != device.ownerId }?.let { requireOwner(device, it) } - if (flipped.isNotEmpty()) { - when (store.setCatalogRowsShown(flipped.associate { it.id to !it.shown })) { - CatalogRowsSave.SAVED -> record(log, LogLevel.INFO, ApiEvents.CATALOG_ROWS_SWITCHED) - CatalogRowsSave.TOO_MANY -> throw validation( + if (flipped.isNotEmpty() || rows != null) { + when (store.saveRows(rows, flipped.associate { it.id to !it.shown })) { + HomeRowsSave.SAVED -> { + if (flipped.isNotEmpty()) record(log, LogLevel.INFO, ApiEvents.CATALOG_ROWS_SWITCHED) + if (rows != null) record(log, LogLevel.INFO, ApiEvents.HOME_ROWS_SAVED) + } + HomeRowsSave.TOO_MANY -> throw validation( "At most $MAX_SHOWN_CATALOG_ROWS catalog rows can be on Home. Turn one off first.", "field" to "hiddenCatalogRows") - CatalogRowsSave.UNKNOWN_ROW -> throw validation("That catalog row is not on the TV.", "field" to "hiddenCatalogRows") - } - } - if (rows != null) { - try { - store.setHomeRows(rows) - } catch (e: IllegalArgumentException) { - throw validation("The TV refused that row list.", "field" to "homeRows") + HomeRowsSave.UNKNOWN_ROW -> throw validation("That catalog row is not on the TV.", "field" to "hiddenCatalogRows") + HomeRowsSave.ROWS_REFUSED -> throw validation("The TV refused that row list.", "field" to "homeRows") } - record(log, LogLevel.INFO, ApiEvents.HOME_ROWS_SAVED) } if (pinnedApps != null && hiddenApps != null) { store.setHomeApps(pinnedApps, hiddenApps) diff --git a/webserver/src/main/resources/tv/streamer/webserver/page/app.js b/webserver/src/main/resources/tv/streamer/webserver/page/app.js index d96ac7c..ff226b2 100644 --- a/webserver/src/main/resources/tv/streamer/webserver/page/app.js +++ b/webserver/src/main/resources/tv/streamer/webserver/page/app.js @@ -262,6 +262,7 @@ async function loadHome() { if (!homeEdited) homeRows = result.data.homeRows.map(rowToSend); rowKinds = result.data.rowKinds; takeCatalogRows(result.data); + dropGoneCatalogRows(); takeApps(result.data, !appsEdited); renderHome(); renderCatalogRows(); @@ -281,6 +282,24 @@ function takeCatalogRows(data) { if (!catalogEdited) catalogSwitches = {}; } +// An unsaved row list can name a catalog row the TV has since removed (its source or catalog was +// removed); the TV refuses such a list, so those rows are left out and the rest of the edit kept. +// Answers whether any were left out. +function dropGoneCatalogRows() { + const kept = homeRows.filter(function (row) { + return row.catalogRowId == null || catalogRows.some(function (r) { return r.id === row.catalogRowId; }); + }); + const dropped = kept.length !== homeRows.length; + homeRows = kept; + return dropped; +} + +// A save refused because a catalog row is no longer on the TV: the refusal names it in details.catalogRowId. +function catalogRowGone(result) { + const error = result.data && result.data.error; + return result.status === 400 && !!(error && error.details && error.details.catalogRowId); +} + function catalogRowShown(row) { return Object.prototype.hasOwnProperty.call(catalogSwitches, row.id) ? catalogSwitches[row.id] : row.shown; } @@ -518,6 +537,18 @@ async function saveHome() { renderCatalogRows(); renderApps(); if (switched) await reloadCatalogPanels(); + } else if (catalogRowGone(result)) { + // Fetch the TV's catalog rows, leave out the ones gone and keep the rest of the edit for another Save. + const fresh = await call("GET", "/settings"); + if (fresh.status === 200) { + takeCatalogRows(fresh.data); + dropGoneCatalogRows(); + renderHome(); + renderCatalogRows(); + say("A catalog row is no longer on the TV, so it was left out. Check the rows and save again.", true); + } else { + say(errorText(result), true); + } } else { say(errorText(result), true); } diff --git a/webserver/src/test/kotlin/tv/streamer/webserver/api/HomeSettingsApiTest.kt b/webserver/src/test/kotlin/tv/streamer/webserver/api/HomeSettingsApiTest.kt index 4fbc92d..c9c6167 100644 --- a/webserver/src/test/kotlin/tv/streamer/webserver/api/HomeSettingsApiTest.kt +++ b/webserver/src/test/kotlin/tv/streamer/webserver/api/HomeSettingsApiTest.kt @@ -107,18 +107,20 @@ private class FakeHomeSettings : HomeSettingsStore { return HomeLayout(kept + shown.filter { it !in named }.map { HomeRow.catalog(it) }, saved.isNotEmpty(), catalogRows) } - override suspend fun setHomeRows(rows: List) { - require(rows.none { it.pinnedUser == UserId(99) }) { "Unknown user" } - writes++ - saved = rows - } - - override suspend fun setCatalogRowsShown(shown: Map): CatalogRowsSave { - if (shown.keys.any { id -> catalogRows.none { it.id == id } }) return CatalogRowsSave.UNKNOWN_ROW - if (full && shown.values.any { it }) return CatalogRowsSave.TOO_MANY - catalogWrites++ - catalogRows = catalogRows.map { row -> shown[row.id]?.let { row.copy(shown = it) } ?: row } - return CatalogRowsSave.SAVED + /** Refuses a row pinned to user 99, as storage refuses an unknown user; a refusal writes nothing. */ + override suspend fun saveRows(rows: List?, catalogRowsShown: Map): HomeRowsSave { + if (catalogRowsShown.keys.any { id -> catalogRows.none { it.id == id } }) return HomeRowsSave.UNKNOWN_ROW + if (full && catalogRowsShown.values.any { it }) return HomeRowsSave.TOO_MANY + if (rows != null && rows.any { it.pinnedUser == UserId(99) }) return HomeRowsSave.ROWS_REFUSED + if (catalogRowsShown.isNotEmpty()) { + catalogWrites++ + catalogRows = catalogRows.map { row -> catalogRowsShown[row.id]?.let { row.copy(shown = it) } ?: row } + } + if (rows != null) { + writes++ + saved = rows + } + return HomeRowsSave.SAVED } } @@ -446,6 +448,19 @@ class HomeSettingsApiTest { assertEquals(CATALOG_ROWS, f.home.catalogRows) } + @Test fun aRowListTheStoreRefusesLeavesTheCatalogSwitchesSentWithItUnchanged() = settingsTest(withUsers = false) { f -> + f.home.catalogRows = CATALOG_ROWS + f.tokens.setOwner(f.tokens.authenticate(f.ana)!!.id, 99) + val response = client.putSettings("""{"homeRows":[{"kind":"WATCHLIST","pinnedUserId":99}],"hiddenCatalogRows":[7,9]}""", f.ana) + assertEquals(HttpStatusCode.BadRequest, response.status) + assertEquals("VALIDATION", response.errorCode()) + assertEquals("homeRows", response.json().getValue("error").jsonObject.getValue("details").jsonObject.getValue("field").jsonPrimitive.content) + assertEquals(CATALOG_ROWS, f.home.catalogRows) + assertEquals(0, f.home.catalogWrites) + assertEquals(0, f.home.writes) + assertTrue(f.log.history.after(0).none { it.name.value == "api_catalog_rows_switched" }) + } + @Test fun catalogSwitchEventsCarryNoRowOrLabel() = settingsTest { f -> f.home.catalogRows = CATALOG_ROWS client.putSettings("""{"hiddenCatalogRows":[7]}""", f.laptop) diff --git a/webserver/src/test/kotlin/tv/streamer/webserver/api/PhonePageTest.kt b/webserver/src/test/kotlin/tv/streamer/webserver/api/PhonePageTest.kt index 5124018..5b2fe06 100644 --- a/webserver/src/test/kotlin/tv/streamer/webserver/api/PhonePageTest.kt +++ b/webserver/src/test/kotlin/tv/streamer/webserver/api/PhonePageTest.kt @@ -186,8 +186,11 @@ class PhonePageTest { var apps = HomeApps(installed, listOf("com.netflix.ninja"), emptyList()) val home = object : HomeSettingsStore { override suspend fun homeLayout() = HomeLayout(saved, saved = true) - override suspend fun setHomeRows(rows: List) { saved = rows } - override suspend fun setCatalogRowsShown(shown: Map) = CatalogRowsSave.UNKNOWN_ROW + override suspend fun saveRows(rows: List?, catalogRowsShown: Map): HomeRowsSave { + if (catalogRowsShown.isNotEmpty()) return HomeRowsSave.UNKNOWN_ROW + rows?.let { saved = it } + return HomeRowsSave.SAVED + } override suspend fun homeApps() = apps override suspend fun setHomeApps(pinned: List, hidden: List) { apps = HomeApps(installed, pinned, hidden) } } @@ -247,6 +250,13 @@ class PhonePageTest { assertTrue("body.homeRows = homeRows.map(rowToSend)" in script && "homeRows = result.data.homeRows.map(rowToSend)" in script) assertTrue("body.hiddenCatalogRows = catalogRows.filter(" in script) assertTrue("{ type: catalog.type, id: catalog.id }" in script) + // An unsaved row list naming a catalog row the TV no longer has: left out on load and when a + // save is refused for it, keeping the rest of the edit. + assertTrue("takeCatalogRows(result.data);\n dropGoneCatalogRows();" in script, "loadHome no longer leaves out gone catalog rows") + assertTrue("} else if (catalogRowGone(result)) {" in script && "takeCatalogRows(fresh.data);\n dropGoneCatalogRows();" in script, + "saveHome no longer recovers from a gone catalog row") + assertTrue("error.details.catalogRowId" in script) + assertTrue("return row.catalogRowId == null || catalogRows.some(function (r) { return r.id === row.catalogRowId; });" in script) for (field in listOf("data.catalogRows", "row.label", "row.userId", "row.shown", "catalog.homeRow", "catalog.offered", "catalog.needsExtra", "catalog.name", "panel.available")) { assertTrue(field in script, "app.js no longer reads $field") @@ -266,10 +276,10 @@ class PhonePageTest { val home = object : HomeSettingsStore { override suspend fun homeLayout() = HomeLayout(saved.filter { row -> row.catalogRowId == null || catalogRows.single { it.id == row.catalogRowId }.shown } + catalogRows.filter { it.shown && HomeRow.catalog(it.id) !in saved }.map { HomeRow.catalog(it.id) }, saved = true, catalogRows) - override suspend fun setHomeRows(rows: List) { saved = rows } - override suspend fun setCatalogRowsShown(shown: Map): CatalogRowsSave { - catalogRows = catalogRows.map { row -> shown[row.id]?.let { row.copy(shown = it) } ?: row } - return CatalogRowsSave.SAVED + override suspend fun saveRows(rows: List?, catalogRowsShown: Map): HomeRowsSave { + catalogRows = catalogRows.map { row -> catalogRowsShown[row.id]?.let { row.copy(shown = it) } ?: row } + rows?.let { saved = it } + return HomeRowsSave.SAVED } override suspend fun homeApps() = HomeApps(emptyList(), emptyList(), emptyList()) override suspend fun setHomeApps(pinned: List, hidden: List) = error("not used") @@ -311,6 +321,20 @@ class PhonePageTest { val answered = Json.parseToJsonElement(put.bodyAsText()).jsonObject.getValue("homeRows").jsonArray.map { it.jsonObject } assertEquals(listOf(null, "8"), answered.map { it["catalogRowId"]?.jsonPrimitive?.content }) + // A row list or switch naming a catalog row the TV no longer has is refused with its id in + // details.catalogRowId, which the page reads to leave it out; nothing is saved. + for (gone in listOf("""{"homeRows":[{"catalogRowId":9},{"kind":"WATCHLIST","pinnedUserId":null}]}""", """{"hiddenCatalogRows":[9]}""")) { + val refused = client.put("$API_PREFIX/settings") { asThePage(cookie, gone) } + assertEquals(HttpStatusCode.BadRequest, refused.status, gone) + val details = Json.parseToJsonElement(refused.bodyAsText()).jsonObject.getValue("error").jsonObject.getValue("details").jsonObject + assertEquals("9", details.getValue("catalogRowId").jsonPrimitive.content, gone) + } + assertEquals(listOf(HomeRow.catalog(7), HomeRow(FeedKind.WATCHLIST)), saved) + // Another refusal on the same field names no row, so the page keeps the edit as it is. + assertTrue("catalogRowId" !in Json.parseToJsonElement(client.put("$API_PREFIX/settings") { + asThePage(cookie, """{"hiddenCatalogRows":[7,7]}""") + }.bodyAsText()).jsonObject.getValue("error").jsonObject.getValue("details").jsonObject) + // The addon's catalogs, then its ticks saved whole. val got = client.get("$API_PREFIX/sources/${addon.id}/catalogs") { asThePage(cookie) } assertEquals(HttpStatusCode.OK, got.status) -- knotmirror 0.0.1