From 2ea7e8bbd861070612fe2c1315e15dc75dc64c2b Mon Sep 17 00:00:00 2001 From: Timo <6156589+Shik3i@users.noreply.github.com> Date: Sat, 15 Aug 2026 12:20:47 +0200 Subject: [PATCH 1/2] Fix cross-device sync, notifications and the episode share control Sync between the web client and Android was failing in ways that all looked like "the app just doesn't update", and the audit found four independent causes plus two that stopped notifications from arriving at all. Sync - The web client aborted its whole push on a 400. The watermark never moved, so the same rejected record went back up every 45 seconds and every other local change queued behind it forever, while pull kept working and made sync look healthy. It now halves a rejected batch to isolate the offending operation, reports it, and lets the rest through, as Android already did. - The server replaced a playback_state payload with its own parsed struct, dropping the title, artwork, podcast id, enclosure, duration and categories the clients denormalize into it. The sync log is the only place that data lives, so the receiving device rebuilt progress rows with no title and overwrote good local ones. Normalization now merges into the original object and unknown keys round-trip. - Listening sessions could exceed the server's own ceilings (a player paused across a holiday spans more than seven days), making them permanently unpushable. Both clients clamp before sending, holding the end timestamp that last-writer comparisons key on. - An empty folder in a subscription payload cleared the folder on the peer; Android already guarded this, the web did not. Android additionally lost updated_at on pulled subscriptions. Notifications - The Web Push Topic header was "podcast-" plus a UUID: 44 characters against RFC 8030's 32-character cap, so push services rejected every new-episode notification. It is now a hash of the podcast id, keeping the collapsing behaviour within the allowed length. - Healthy feeds were rescheduled 24 hours out, so a daily show could be announced most of a day late. The interval is configurable via FEED_REFRESH_INTERVAL_MS and defaults to an hour; conditional requests keep it cheap. The push send loop no longer holds a SQLite read cursor across third-party network calls while deleting from the same table. - The Android new-episode notification was built from English literals, so German listeners got English text in the one place they see without opening the app. It uses string resources now, with tests pinning both locales and the plural's format arguments. Android - Pull-to-refresh reached only the Inbox. It now covers Library, Profile and the community statistics (which sync) and Discover and a podcast (which re-read the feed). Downloads is deliberately excluded: it is device-local, and a gesture that does nothing is worse than none. - Returning from the background waited out the remainder of the 45-second tick, and a cached process is frozen, so an episode finished elsewhere could sit there unplayed for most of a minute. The coordinator now syncs on foreground; the periodic tick stays ungated so progress from screen-off listening keeps reaching the account. Sharing - The episode share control was labelled "continue on another device" and quietly wrote to the clipboard on any browser without a native share sheet. It is a share control now: it opens, shows the exact link, offers the system share sheet where one exists and copy or email everywhere else, and lets the listener choose whether the timestamp is included. Android's button carried the same wrong name for what was already a real share sheet. Hardening - A registration losing the unique-index race answers 409 rather than 500. - Recovery-code verification spends the same work as a real check, so it cannot be used to enumerate usernames the way login already prevented. - The SSRF blocklist covers the NAT64 well-known prefixes, benchmarking and IETF protocol assignment ranges. Verified on an Android emulator against a local instance: subscriptions, folders and playback state cross over, pull-to-refresh triggers a sync, the foreground trigger fires within half a second, and the share sheet opens with the timestamped link. Co-Authored-By: Claude Opus 5 --- .env.example | 5 + README.md | 3 +- apps/android/app/build.gradle.kts | 4 +- .../koalacast/KoalaCastApplication.kt | 30 ++++ apps/android/core/data/build.gradle.kts | 7 + .../data/repository/ContentRefreshWorker.kt | 18 +- .../core/data/repository/SyncCoordinator.kt | 35 +++- .../core/data/repository/SyncRepository.kt | 36 +++- .../data/src/main/res/values-de/strings.xml | 7 + .../core/data/src/main/res/values/strings.xml | 7 + .../NewEpisodeNotificationStringsTest.kt | 76 ++++++++ .../data/repository/SyncRepositoryTest.kt | 32 ++++ .../src/test/resources/robolectric.properties | 7 + .../feature/discover/DiscoverScreen.kt | 9 + .../feature/episode/EpisodeScreen.kt | 6 +- .../src/main/res/values-de/strings.xml | 3 +- .../episode/src/main/res/values/strings.xml | 3 +- .../feature/globalstats/GlobalStatsScreen.kt | 14 +- .../globalstats/GlobalStatsViewModel.kt | 26 ++- .../feature/library/LibraryScreen.kt | 76 ++++---- .../feature/library/LibraryViewModel.kt | 24 +++ .../feature/podcast/PodcastScreen.kt | 15 +- .../feature/podcast/PodcastViewModel.kt | 18 ++ .../feature/profile/ProfileScreen.kt | 17 +- .../feature/profile/ProfileViewModel.kt | 23 +++ apps/web/package.json | 2 +- apps/web/src/lib/i18n/messages/de.json | 13 +- apps/web/src/lib/i18n/messages/en.json | 13 +- .../lib/stores/sync-push-isolation.test.ts | 166 ++++++++++++++++++ apps/web/src/lib/stores/sync.svelte.ts | 149 +++++++++++----- apps/web/src/lib/sync-payload.test.ts | 27 +++ apps/web/src/lib/sync-payload.ts | 26 ++- apps/web/src/routes/episode/[id]/+page.svelte | 152 ++++++++++++++-- docs/architecture/overview.md | 1 + docs/current-status.md | 11 +- docs/sync-protocol/specification.md | 29 +++ services/api/internal/config/config.go | 25 +++ services/api/internal/config/config_test.go | 21 +++ services/api/internal/push/service.go | 37 +++- services/api/internal/push/topic_test.go | 44 +++++ services/api/internal/rss/ssrf.go | 7 + services/api/internal/server/handlers/auth.go | 22 +++ services/api/internal/server/handlers/sync.go | 38 +++- .../handlers/sync_playback_payload_test.go | 133 ++++++++++++++ services/api/internal/worker/worker.go | 17 +- 45 files changed, 1282 insertions(+), 152 deletions(-) create mode 100644 apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/NewEpisodeNotificationStringsTest.kt create mode 100644 apps/android/core/data/src/test/resources/robolectric.properties create mode 100644 apps/web/src/lib/stores/sync-push-isolation.test.ts create mode 100644 services/api/internal/push/topic_test.go create mode 100644 services/api/internal/server/handlers/sync_playback_payload_test.go diff --git a/.env.example b/.env.example index d34d564b..a09222a7 100644 --- a/.env.example +++ b/.env.example @@ -40,6 +40,11 @@ FEED_WORKER_CONCURRENCY=5 FEED_REQUEST_TIMEOUT_MS=15000 FEED_MAX_RESPONSE_BYTES=33554432 # 32 MiB limit for large RSS XML feeds FEED_MAX_STORED_EPISODES=200 +# How long a healthy feed waits before the background worker checks it again. +# Only subscriptions that asked for new-episode notifications are refreshed on +# this schedule; everything else refreshes when a listener opens it. Clamped to +# 15 minutes .. 24 hours. Conditional requests keep it cheap (304, no body). +FEED_REFRESH_INTERVAL_MS=3600000 # Web Push (required for notifications while the browser is closed). # Generate once with: cd services/api && go run ./cmd/vapid diff --git a/README.md b/README.md index 79d241e8..ad13787c 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ Calm, distraction-free listening — with optional account-backed cross-device s | :--- | :--- | | **Discovery & Search** | iTunes Top Charts discovery, iTunes/Podcast Index search, multi-select preferred/hidden genres, per-podcast hiding, add any feed by direct RSS URL | | **Languages** | Spoken-language filtering (not just storefront region) for Discover and Search, language + genre search filters, fully translated English/German interface (add a language with one JSON file) | -| **Playback** | Web Audio player, Media Session and Remote Playback APIs, playback-speed control, per-podcast controls, live scrubbing with chapter markers, jump-back after a large seek, a sleep timer that counts listening time rather than wall-clock time, a transcript that follows the playhead, timestamp handoff links, listening-time tracking, keyboard shortcuts | +| **Playback** | Web Audio player, Media Session and Remote Playback APIs, playback-speed control, per-podcast controls, live scrubbing with chapter markers, jump-back after a large seek, a sleep timer that counts listening time rather than wall-clock time, a transcript that follows the playhead, episode sharing with a timestamped link, listening-time tracking, keyboard shortcuts | | **Library** | Subscriptions with folders, queue plus reusable named queues, smart queues built from saved rules, favorites, timestamp bookmarks, OPML import/export plus an optional auto-updating OPML backup file | | **Accounts (optional)** | Argon2id hashing, Base32 recovery codes, HttpOnly session cookies, Bearer device tokens | | **Sync** | Subscriptions, favorites, playback state, listening sessions, queue, podcast settings, and global settings via monotonic cursor pull/push and idempotent writes; settings merge per field, so two devices editing different preferences do not revert each other | @@ -206,6 +206,7 @@ The backend is configured entirely through environment variables. Copy [`.env.ex | `FEED_WORKER_CONCURRENCY` | `5` | Background feed-refresh workers | | `FEED_MAX_RESPONSE_BYTES` | `33554432` | Max RSS body size in bytes (32 MiB; SSRF/DoS guard) | | `FEED_MAX_STORED_EPISODES` | `200` | Recent metadata-cache rows retained per podcast; rows referenced by user state are preserved | +| `FEED_REFRESH_INTERVAL_MS` | `3600000` | How long a healthy feed waits before the background worker rechecks it (15 min – 24 h). Governs how promptly new-episode notifications arrive | | `WEB_PUSH_VAPID_PUBLIC_KEY` / `_PRIVATE_KEY` | empty | Enables server-sent browser notifications; generate once with `cd services/api && go run ./cmd/vapid` | | `WEB_PUSH_VAPID_SUBJECT` | `PUBLIC_BASE_URL` | VAPID contact URI (`https:` or `mailto:`) | | `KC_AUDIO_EFFECTS_PROXY_ENABLED` | `true` | Relay fallback for CORS-blocked browser effects/downloads; set `false` to avoid relay bandwidth | diff --git a/apps/android/app/build.gradle.kts b/apps/android/app/build.gradle.kts index 42f575c8..296eddd4 100644 --- a/apps/android/app/build.gradle.kts +++ b/apps/android/app/build.gradle.kts @@ -26,8 +26,8 @@ android { defaultConfig { applicationId = "net.koalastuff.koalacast" - versionCode = 42 - versionName = "0.11.2" + versionCode = 43 + versionName = "0.11.3" } signingConfigs { diff --git a/apps/android/app/src/main/kotlin/net/koalastuff/koalacast/KoalaCastApplication.kt b/apps/android/app/src/main/kotlin/net/koalastuff/koalacast/KoalaCastApplication.kt index ba2d5584..b87a64f9 100644 --- a/apps/android/app/src/main/kotlin/net/koalastuff/koalacast/KoalaCastApplication.kt +++ b/apps/android/app/src/main/kotlin/net/koalastuff/koalacast/KoalaCastApplication.kt @@ -1,6 +1,8 @@ package net.koalastuff.koalacast +import android.app.Activity import android.app.Application +import android.os.Bundle import androidx.hilt.work.HiltWorkerFactory import androidx.work.Configuration import androidx.annotation.OptIn @@ -88,6 +90,7 @@ class KoalaCastApplication : Application(), SingletonImageLoader.Factory, Config // here makes the route button usable before playback starts; failures are // non-fatal on devices without Google Play services. runCatching { Cast.getSingletonInstance(this).initialize() } + registerActivityLifecycleCallbacks(foregroundWatcher) applicationScope.launch { // Must run before account origin selection, sync startup, workers, or any // repository request. A token created for an HTTP origin is never moved @@ -156,6 +159,33 @@ class KoalaCastApplication : Application(), SingletonImageLoader.Factory, Config .crossfade(true) .build() + /** + * Tells the sync coordinator when the app is actually on screen, so a return + * from the background syncs at once instead of waiting out the remainder of + * the periodic tick. Counting started activities rather than tracking a + * single one keeps a configuration change or a second task from reading as a + * trip to the background. + */ + private val foregroundWatcher = object : Application.ActivityLifecycleCallbacks { + private var startedActivities = 0 + + override fun onActivityStarted(activity: Activity) { + startedActivities += 1 + if (startedActivities == 1) syncCoordinator.setForeground(true) + } + + override fun onActivityStopped(activity: Activity) { + startedActivities = (startedActivities - 1).coerceAtLeast(0) + if (startedActivities == 0) syncCoordinator.setForeground(false) + } + + override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) = Unit + override fun onActivityResumed(activity: Activity) = Unit + override fun onActivityPaused(activity: Activity) = Unit + override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) = Unit + override fun onActivityDestroyed(activity: Activity) = Unit + } + private companion object { val SUBSCRIPTION_ARTWORK_DP = intArrayOf(40, 56, 160) } diff --git a/apps/android/core/data/build.gradle.kts b/apps/android/core/data/build.gradle.kts index 45ffdf5d..12b1b327 100644 --- a/apps/android/core/data/build.gradle.kts +++ b/apps/android/core/data/build.gradle.kts @@ -7,6 +7,13 @@ plugins { android { namespace = "net.koalastuff.koalacast.core.data" + + // This module owns the notification strings the background workers show, and + // those are the only user-facing text a listener sees without opening the + // app. Merged resources on the unit-test classpath are what lets a test + // assert the German ones are actually translated instead of silently + // falling back to English. + testOptions.unitTests.isIncludeAndroidResources = true } androidComponents { diff --git a/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/ContentRefreshWorker.kt b/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/ContentRefreshWorker.kt index fd3248a4..7b3793db 100644 --- a/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/ContentRefreshWorker.kt +++ b/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/ContentRefreshWorker.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.sync.Semaphore import kotlinx.coroutines.sync.withPermit import kotlinx.coroutines.withContext +import net.koalastuff.koalacast.core.data.R import net.koalastuff.koalacast.core.data.db.PodcastSettingsDao import net.koalastuff.koalacast.core.data.db.SubscriptionDao import net.koalastuff.koalacast.core.data.auth.SecureAccountStore @@ -125,7 +126,7 @@ class ContentRefreshWorker @AssistedInject constructor( manager.createNotificationChannel( NotificationChannel( CHANNEL_ID, - "New podcast episodes", + applicationContext.getString(R.string.new_episodes_channel_name), NotificationManager.IMPORTANCE_DEFAULT, ), ) @@ -139,15 +140,24 @@ class ContentRefreshWorker @AssistedInject constructor( PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) } + // Every one of these strings used to be an English literal, so the one + // part of the app a listener sees without opening it was the one part + // that ignored their language. val first = updates.first() + val shows = updates.map { it.first }.distinct().size val text = if (updates.size == 1) { - "${first.first}: ${first.second}" + applicationContext.getString(R.string.new_episodes_single, first.first, first.second) } else { - "${updates.size} new episodes from ${updates.map { it.first }.distinct().size} shows" + applicationContext.resources.getQuantityString( + R.plurals.new_episodes_shows, + shows, + updates.size, + shows, + ) } val notification = NotificationCompat.Builder(applicationContext, CHANNEL_ID) .setSmallIcon(android.R.drawable.stat_sys_download_done) - .setContentTitle("New in KoalaCast") + .setContentTitle(applicationContext.getString(R.string.new_episodes_title)) .setContentText(text) .setStyle(NotificationCompat.BigTextStyle().bigText(text)) .setAutoCancel(true) diff --git a/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncCoordinator.kt b/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncCoordinator.kt index ef7e9182..7bd07c7b 100644 --- a/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncCoordinator.kt +++ b/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncCoordinator.kt @@ -2,8 +2,11 @@ package net.koalastuff.koalacast.core.data.repository import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collectLatest +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.launch import net.koalastuff.koalacast.core.data.auth.SecureAccountStore import net.koalastuff.koalacast.core.data.di.ApplicationScope @@ -17,6 +20,21 @@ class SyncCoordinator @Inject constructor( @ApplicationScope private val scope: CoroutineScope, ) { private var job: Job? = null + private val foreground = MutableStateFlow(false) + + /** + * Reported by the application from the activity lifecycle. + * + * Returning to the app after listening somewhere else is exactly when a + * stale library is most visible, and the periodic tick alone made that a + * coin flip: the process is frozen while it is cached, so the remainder of + * a 45-second delay is only counted once the app is on screen again. An + * episode finished in the browser could therefore still be sitting there + * unplayed for the better part of a minute after opening the app. + */ + fun setForeground(value: Boolean) { + foreground.value = value + } fun start() { if (job != null) return @@ -26,9 +44,20 @@ class SyncCoordinator @Inject constructor( sync.signedOut() return@collectLatest } - while (true) { - sync.syncNow() - delay(INTERVAL_MS) + coroutineScope { + // `drop(1)` discards the state at subscription time: the loop + // below already syncs once here, so only a real transition + // back into the foreground should add a run. + launch { + foreground.drop(1).collect { visible -> if (visible) sync.syncNow() } + } + // The tick is deliberately not gated on the foreground. + // Playback continues with the screen off, and progress from + // that listening has to keep reaching the account. + while (true) { + sync.syncNow() + delay(INTERVAL_MS) + } } } } diff --git a/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepository.kt b/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepository.kt index a3ae23ad..39aa4217 100644 --- a/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepository.kt +++ b/apps/android/core/data/src/main/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepository.kt @@ -592,6 +592,13 @@ class SyncRepository @Inject constructor( addedAt = payload.long("added_at").takeIf { it > 0 } ?: change.clientTimestamp.takeIf { it > 0 } ?: System.currentTimeMillis(), + // Without this the entity default silently rewrote updatedAt to + // addedAt, so a folder or inbox-mode edit made elsewhere landed + // here dated to the day the show was subscribed. + updatedAt = payload.long("updated_at").takeIf { it > 0 } + ?: payload.long("added_at").takeIf { it > 0 } + ?: change.clientTimestamp.takeIf { it > 0 } + ?: System.currentTimeMillis(), inboxMode = payload.string("inbox_mode") .takeIf { it == SubscriptionEntity.INBOX_MODE_LATEST } ?: SubscriptionEntity.INBOX_MODE_ALL, @@ -912,22 +919,31 @@ class SyncRepository @Inject constructor( put("client_timestamp", item.lastPlayedAt) } + /** + * Clamped to the server's own ceilings for a listening session. Past them the + * push is rejected with a 400, so this is not cosmetic: an unclamped session + * is a record that can never be uploaded. A session paused and resumed across + * days reaches the span limit on its own. + */ private fun listeningPayload(item: ListeningSessionEntity) = buildJsonObject { + val endedAt = maxOf(item.endedAt, item.startedAt) put("id", item.id) put("episode_id", item.episodeId) put("podcast_id", item.podcastId) put("title", item.title) put("podcast_title", item.podcastTitle) put("categories", JsonArray(item.categories.map(::JsonPrimitive))) - put("started_at", item.startedAt) - put("ended_at", item.endedAt) - put("wall_clock_ms", item.wallClockMs) - put("audio_listened_ms", item.audioListenedMs) - put("speed_saved_ms", item.speedSavedMs) - put("silence_saved_ms", item.silenceSavedMs) - put("manual_skipped_ms", item.manualSkippedMs) - put("intro_outro_skipped_ms", item.introOutroSkippedMs) - put("speed_weighted_ms", item.speedWeightedMs) + // Hold the span, not the end: the end is what every last-writer-wins + // comparison keys on. + put("started_at", maxOf(item.startedAt, endedAt - MAX_SESSION_SPAN_MS)) + put("ended_at", endedAt) + put("wall_clock_ms", item.wallClockMs.coerceIn(0, MAX_SESSION_SPAN_MS)) + put("audio_listened_ms", item.audioListenedMs.coerceIn(0, MAX_SESSION_METRIC_MS)) + put("speed_saved_ms", item.speedSavedMs.coerceIn(0, MAX_SESSION_METRIC_MS)) + put("silence_saved_ms", item.silenceSavedMs.coerceIn(0, MAX_SESSION_METRIC_MS)) + put("manual_skipped_ms", item.manualSkippedMs.coerceIn(0, MAX_SESSION_METRIC_MS)) + put("intro_outro_skipped_ms", item.introOutroSkippedMs.coerceIn(0, MAX_SESSION_METRIC_MS)) + put("speed_weighted_ms", item.speedWeightedMs.coerceIn(0, MAX_SESSION_METRIC_MS)) } private fun queuePayload(items: List, updatedAt: Long) = buildJsonObject { @@ -1089,6 +1105,8 @@ class SyncRepository @Inject constructor( const val PAGE_LIMIT = 500 const val PUSH_BATCH = 250 const val MAX_SYNCED_DOWNLOAD_BUDGET_BYTES = 10L * 1024 * 1024 * 1024 + const val MAX_SESSION_SPAN_MS = 7L * 24 * 60 * 60 * 1000 + const val MAX_SESSION_METRIC_MS = MAX_SESSION_SPAN_MS * 4 val GENERAL_SYNC_ENTITIES = setOf( "subscription", "favorite", diff --git a/apps/android/core/data/src/main/res/values-de/strings.xml b/apps/android/core/data/src/main/res/values-de/strings.xml index d7bce134..9cc6e549 100644 --- a/apps/android/core/data/src/main/res/values-de/strings.xml +++ b/apps/android/core/data/src/main/res/values-de/strings.xml @@ -4,4 +4,11 @@ Podcast-Folge Wird geladen … %1$d %% + Neue Podcast-Folgen + Neu in KoalaCast + %1$s: %2$s + + %1$d neue Folgen von einem Podcast + %1$d neue Folgen von %2$d Podcasts + diff --git a/apps/android/core/data/src/main/res/values/strings.xml b/apps/android/core/data/src/main/res/values/strings.xml index 80019183..9e171c9d 100644 --- a/apps/android/core/data/src/main/res/values/strings.xml +++ b/apps/android/core/data/src/main/res/values/strings.xml @@ -4,4 +4,11 @@ Podcast episode Downloading… %1$d%% + New podcast episodes + New in KoalaCast + %1$s: %2$s + + %1$d new episodes from one show + %1$d new episodes from %2$d shows + diff --git a/apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/NewEpisodeNotificationStringsTest.kt b/apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/NewEpisodeNotificationStringsTest.kt new file mode 100644 index 00000000..03bfe9c9 --- /dev/null +++ b/apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/NewEpisodeNotificationStringsTest.kt @@ -0,0 +1,76 @@ +package net.koalastuff.koalacast.core.data.repository + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import java.util.Locale +import net.koalastuff.koalacast.core.data.R +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * The new-episode notification is the one part of the app a listener sees + * without opening it, and it used to be the one part that ignored their + * language: every string was an English literal in the worker. + * + * These also pin the format arguments. A plural whose placeholders disagree with + * the call site does not fail to compile — it throws while building the + * notification, in a background worker, where nobody sees it. + */ +@RunWith(RobolectricTestRunner::class) +class NewEpisodeNotificationStringsTest { + + private fun context(): Context = ApplicationProvider.getApplicationContext() + + @Test + @Config(qualifiers = "en") + fun `english strings resolve and format`() { + val context = context() + assertEquals("New in KoalaCast", context.getString(R.string.new_episodes_title)) + assertEquals( + "The Vergecast: Episode 1", + context.getString(R.string.new_episodes_single, "The Vergecast", "Episode 1"), + ) + assertEquals( + "5 new episodes from 2 shows", + context.resources.getQuantityString(R.plurals.new_episodes_shows, 2, 5, 2), + ) + } + + @Test + @Config(qualifiers = "de") + fun `german strings are translated, not the english fallback`() { + val context = context() + val title = context.getString(R.string.new_episodes_title) + assertEquals("Neu in KoalaCast", title) + assertEquals( + "Neue Podcast-Folgen", + context.getString(R.string.new_episodes_channel_name), + ) + + val summary = context.resources.getQuantityString(R.plurals.new_episodes_shows, 2, 5, 2) + assertEquals("5 neue Folgen von 2 Podcasts", summary) + assertTrue( + "the German summary must not fall back to English", + !summary.contains("new episodes"), + ) + } + + @Test + @Config(qualifiers = "de") + fun `the single-episode line carries both arguments in german`() { + val line = context().getString(R.string.new_episodes_single, "Crime Junkie", "Folge 12") + assertEquals("Crime Junkie: Folge 12", line) + } + + @Test + @Config(qualifiers = "de") + fun `one show still reads correctly`() { + val summary = context().resources.getQuantityString(R.plurals.new_episodes_shows, 1, 3, 1) + assertEquals("3 neue Folgen von einem Podcast", summary) + assertEquals(Locale.GERMAN.language, context().resources.configuration.locales[0].language) + } +} diff --git a/apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepositoryTest.kt b/apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepositoryTest.kt index 10d71adf..be750f6e 100644 --- a/apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepositoryTest.kt +++ b/apps/android/core/data/src/test/kotlin/net/koalastuff/koalacast/core/data/repository/SyncRepositoryTest.kt @@ -186,6 +186,38 @@ class SyncRepositoryTest { assertEquals(listOf("new"), operations.filter { it.entityType == "listening_session" }.map { it.entityId }) } + @Test + fun `listening sessions are clamped to what the server will accept`() = runTest { + val week = 7L * 24 * 60 * 60 * 1000 + val endedAt = 1_800_000_000_000L + // A player paused and resumed over a holiday: the span alone is past the + // server's ceiling, and an unclamped payload is a permanent 400. + database.listeningSessionDao().upsert( + ListeningSessionEntity( + id = "long", + episodeId = "episode", + podcastId = "show", + title = "Episode", + podcastTitle = "Show", + startedAt = endedAt - week * 3, + endedAt = endedAt, + wallClockMs = week * 2, + audioListenedMs = week * 9, + ), + ) + + val payload = repository.buildOperations("device") + .first { it.entityType == "listening_session" } + .payload as JsonObject + + val started = payload["started_at"]!!.toString().toLong() + val ended = payload["ended_at"]!!.toString().toLong() + assertEquals(endedAt, ended) + assertEquals(week, ended - started) + assertEquals(week, payload["wall_clock_ms"]!!.toString().toLong()) + assertEquals(week * 4, payload["audio_listened_ms"]!!.toString().toLong()) + } + @Test fun `pull follows next cursor while server says more pages exist`() = runTest { val requestedCursors = mutableListOf() diff --git a/apps/android/core/data/src/test/resources/robolectric.properties b/apps/android/core/data/src/test/resources/robolectric.properties new file mode 100644 index 00000000..6ebadaaf --- /dev/null +++ b/apps/android/core/data/src/test/resources/robolectric.properties @@ -0,0 +1,7 @@ +# Robolectric runs against a real android-all runtime, and the newest one it +# ships is API 36. Without this it reads targetSdkVersion from the merged +# manifest (37) and refuses to start every test in the module. +# +# This only pins the runtime the tests execute on; the app's own +# targetSdkVersion is unaffected. Raise it when Robolectric ships API 37. +sdk=36 diff --git a/apps/android/feature/discover/src/main/kotlin/net/koalastuff/koalacast/feature/discover/DiscoverScreen.kt b/apps/android/feature/discover/src/main/kotlin/net/koalastuff/koalacast/feature/discover/DiscoverScreen.kt index c5bef61e..7c2d9a48 100644 --- a/apps/android/feature/discover/src/main/kotlin/net/koalastuff/koalacast/feature/discover/DiscoverScreen.kt +++ b/apps/android/feature/discover/src/main/kotlin/net/koalastuff/koalacast/feature/discover/DiscoverScreen.kt @@ -18,7 +18,9 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyRow import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.itemsIndexed +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Text +import androidx.compose.material3.pulltorefresh.PullToRefreshBox import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment @@ -81,6 +83,7 @@ fun DiscoverScreen( ) } +@OptIn(ExperimentalMaterial3Api::class) @Composable internal fun DiscoverContent( state: DiscoverUiState, @@ -98,6 +101,11 @@ internal fun DiscoverContent( val colors = KoalaTheme.colors Box(modifier = modifier.fillMaxSize().background(colors.bgPanel)) { + PullToRefreshBox( + isRefreshing = state.refreshing, + onRefresh = onRetry, + modifier = Modifier.fillMaxSize(), + ) { LazyColumn( modifier = Modifier.fillMaxSize(), contentPadding = contentPadding, @@ -184,6 +192,7 @@ internal fun DiscoverContent( RowSeparator(modifier = Modifier.padding(horizontal = KoalaSpacing.screenH)) } } + } } // Over the list rather than in it: the row that was hidden may be far from diff --git a/apps/android/feature/episode/src/main/kotlin/net/koalastuff/koalacast/feature/episode/EpisodeScreen.kt b/apps/android/feature/episode/src/main/kotlin/net/koalastuff/koalacast/feature/episode/EpisodeScreen.kt index 33d23e0d..2f21717a 100644 --- a/apps/android/feature/episode/src/main/kotlin/net/koalastuff/koalacast/feature/episode/EpisodeScreen.kt +++ b/apps/android/feature/episode/src/main/kotlin/net/koalastuff/koalacast/feature/episode/EpisodeScreen.kt @@ -59,7 +59,7 @@ fun EpisodeScreen( ) { val state by viewModel.state.collectAsStateWithLifecycle() val context = LocalContext.current - val handoffChooserTitle = stringResource(R.string.episode_handoff) + val shareChooserTitle = stringResource(R.string.episode_share_chooser) EpisodeContent( state = state, @@ -88,7 +88,7 @@ fun EpisodeScreen( putExtra(Intent.EXTRA_TEXT, url) } context.startActivity( - Intent.createChooser(intent, handoffChooserTitle), + Intent.createChooser(intent, shareChooserTitle), ) } }, @@ -326,7 +326,7 @@ internal fun EpisodeContent( modifier = Modifier.weight(1f), ) OutlineButton( - text = stringResource(R.string.episode_handoff), + text = stringResource(R.string.episode_share), onClick = onShareHandoff, leadingIcon = PhosphorIcons.ArrowSquareOut, modifier = Modifier.weight(1f), diff --git a/apps/android/feature/episode/src/main/res/values-de/strings.xml b/apps/android/feature/episode/src/main/res/values-de/strings.xml index b8102eb9..6d064595 100644 --- a/apps/android/feature/episode/src/main/res/values-de/strings.xml +++ b/apps/android/feature/episode/src/main/res/values-de/strings.xml @@ -39,5 +39,6 @@ Zeitmarken Zeitmarke %1$s Zeitmarke bei %1$s entfernen - Auf anderem Gerät fortsetzen + Teilen + Folge teilen diff --git a/apps/android/feature/episode/src/main/res/values/strings.xml b/apps/android/feature/episode/src/main/res/values/strings.xml index 0a9dab84..2d721927 100644 --- a/apps/android/feature/episode/src/main/res/values/strings.xml +++ b/apps/android/feature/episode/src/main/res/values/strings.xml @@ -39,5 +39,6 @@ Time bookmarks Bookmark %1$s Remove bookmark at %1$s - Continue on another device + Share + Share episode diff --git a/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsScreen.kt b/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsScreen.kt index ed0bd1f8..b406eb73 100644 --- a/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsScreen.kt +++ b/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsScreen.kt @@ -17,7 +17,9 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Text +import androidx.compose.material3.pulltorefresh.PullToRefreshBox import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment @@ -57,6 +59,7 @@ fun GlobalStatsScreen( state = state, onSetRange = viewModel::setRange, onRetry = viewModel::retry, + onRefresh = viewModel::refresh, onOpenPodcast = onOpenPodcast, onOpenSettings = onOpenSettings, scopeSelector = scopeSelector, @@ -65,6 +68,7 @@ fun GlobalStatsScreen( ) } +@OptIn(ExperimentalMaterial3Api::class) @Composable internal fun GlobalStatsContent( state: GlobalStatsUiState, @@ -73,14 +77,19 @@ internal fun GlobalStatsContent( onOpenPodcast: (String) -> Unit, modifier: Modifier = Modifier, contentPadding: PaddingValues = PaddingValues(0.dp), + onRefresh: () -> Unit = onRetry, onOpenSettings: () -> Unit = {}, scopeSelector: @Composable () -> Unit = {}, ) { val colors = KoalaTheme.colors + PullToRefreshBox( + isRefreshing = state.refreshing, + onRefresh = onRefresh, + modifier = modifier.fillMaxSize().background(colors.bgPanel), + ) { Column( - modifier = modifier + modifier = Modifier .fillMaxSize() - .background(colors.bgPanel) .verticalScroll(rememberScrollState()) .padding(contentPadding) .padding(horizontal = KoalaSpacing.screenH, vertical = KoalaSpacing.sectionV), @@ -140,6 +149,7 @@ internal fun GlobalStatsContent( state.stats != null -> Stats(state.stats, onOpenPodcast) } } + } } @Composable diff --git a/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsViewModel.kt b/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsViewModel.kt index 7db0a5c7..d0534ae8 100644 --- a/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsViewModel.kt +++ b/apps/android/feature/globalstats/src/main/kotlin/net/koalastuff/koalacast/feature/globalstats/GlobalStatsViewModel.kt @@ -23,6 +23,8 @@ enum class GlobalRange(val wireName: String) { data class GlobalStatsUiState( val range: GlobalRange = GlobalRange.YEAR, val loading: Boolean = true, + /** A pull-to-refresh in flight, as opposed to the first load's skeleton. */ + val refreshing: Boolean = false, val error: DataError? = null, val stats: GlobalStats? = null, ) @@ -46,19 +48,35 @@ class GlobalStatsViewModel @Inject constructor( fun retry() = load() - private fun load() { + /** + * Pull-to-refresh: the same reload, but the numbers already on screen stay + * put behind the refresh indicator instead of collapsing into a skeleton. + */ + fun refresh() = load(refreshing = true) + + private fun load(refreshing: Boolean = false) { viewModelScope.launch { val requestedRange = _state.value.range - _state.update { it.copy(loading = true, error = null) } + _state.update { + it.copy( + loading = !refreshing || it.stats == null, + refreshing = refreshing, + error = null, + ) + } when (val result = repository.load(requestedRange.wireName)) { is DataResult.Success -> { if (_state.value.range == requestedRange) { - _state.update { it.copy(loading = false, stats = result.data) } + _state.update { + it.copy(loading = false, refreshing = false, stats = result.data) + } } } is DataResult.Failure -> { if (_state.value.range == requestedRange) { - _state.update { it.copy(loading = false, error = result.error) } + _state.update { + it.copy(loading = false, refreshing = false, error = result.error) + } } } } diff --git a/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryScreen.kt b/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryScreen.kt index 55238406..fedfd5de 100644 --- a/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryScreen.kt +++ b/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryScreen.kt @@ -15,8 +15,10 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.itemsIndexed +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Text import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.pulltorefresh.PullToRefreshBox import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -61,10 +63,13 @@ fun LibraryScreen( ) { val state by viewModel.state.collectAsStateWithLifecycle() val tab by viewModel.tab.collectAsStateWithLifecycle() + val refreshing by viewModel.refreshing.collectAsStateWithLifecycle() LibraryContent( state = state, tab = tab, + refreshing = refreshing, + onRefresh = viewModel::refresh, onSelectTab = viewModel::selectTab, onOpenPodcast = onOpenPodcast, onOpenEpisode = onOpenEpisode, @@ -90,10 +95,13 @@ fun LibraryScreen( ) } +@OptIn(ExperimentalMaterial3Api::class) @Composable internal fun LibraryContent( state: LibraryUiState, tab: LibraryTab, + refreshing: Boolean = false, + onRefresh: () -> Unit = {}, onSelectTab: (LibraryTab) -> Unit, onOpenPodcast: (String, String?) -> Unit, onOpenEpisode: (String) -> Unit, @@ -139,40 +147,46 @@ internal fun LibraryContent( ) } - when (tab) { - LibraryTab.SUBSCRIPTIONS -> SubscriptionGrid( - subscriptions = state.subscriptions, - onOpen = onOpenPodcast, - onUnsubscribe = onUnsubscribe, - onSetFolder = onSetFolder, - onOpenDiscover = onOpenDiscover, - ) + PullToRefreshBox( + isRefreshing = refreshing, + onRefresh = onRefresh, + modifier = Modifier.weight(1f), + ) { + when (tab) { + LibraryTab.SUBSCRIPTIONS -> SubscriptionGrid( + subscriptions = state.subscriptions, + onOpen = onOpenPodcast, + onUnsubscribe = onUnsubscribe, + onSetFolder = onSetFolder, + onOpenDiscover = onOpenDiscover, + ) - LibraryTab.IN_PROGRESS -> InProgressList( - items = state.inProgress, - onOpenEpisode = onOpenEpisode, - onPlay = onPlay, - ) + LibraryTab.IN_PROGRESS -> InProgressList( + items = state.inProgress, + onOpenEpisode = onOpenEpisode, + onPlay = onPlay, + ) - LibraryTab.QUEUE -> QueueList( - items = state.queue, - onOpenEpisode = onOpenEpisode, - onPlay = onPlay, - onRemove = onRemoveFromQueue, - onMove = onMoveInQueue, - onClear = onClearQueue, - namedQueues = state.namedQueues, - onSaveNamedQueue = onSaveNamedQueue, - onRestoreNamedQueue = onRestoreNamedQueue, - onDeleteNamedQueue = onDeleteNamedQueue, - ) + LibraryTab.QUEUE -> QueueList( + items = state.queue, + onOpenEpisode = onOpenEpisode, + onPlay = onPlay, + onRemove = onRemoveFromQueue, + onMove = onMoveInQueue, + onClear = onClearQueue, + namedQueues = state.namedQueues, + onSaveNamedQueue = onSaveNamedQueue, + onRestoreNamedQueue = onRestoreNamedQueue, + onDeleteNamedQueue = onDeleteNamedQueue, + ) - LibraryTab.FAVORITES -> FavoritesList( - items = state.favorites, - onOpenEpisode = onOpenEpisode, - onPlay = onPlay, - onRemove = onRemoveFavorite, - ) + LibraryTab.FAVORITES -> FavoritesList( + items = state.favorites, + onOpenEpisode = onOpenEpisode, + onPlay = onPlay, + onRemove = onRemoveFavorite, + ) + } } } } diff --git a/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryViewModel.kt b/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryViewModel.kt index e2c0bb40..0bb43d8c 100644 --- a/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryViewModel.kt +++ b/apps/android/feature/library/src/main/kotlin/net/koalastuff/koalacast/feature/library/LibraryViewModel.kt @@ -14,6 +14,7 @@ import net.koalastuff.koalacast.core.data.repository.AccountRepository import net.koalastuff.koalacast.core.data.repository.ProgressRepository import net.koalastuff.koalacast.core.data.repository.QueueRepository import net.koalastuff.koalacast.core.data.repository.NamedQueueRepository +import net.koalastuff.koalacast.core.data.repository.SyncRepository import net.koalastuff.koalacast.core.data.prefs.PreferencesRepository import net.koalastuff.koalacast.core.player.PlayerConnection import net.koalastuff.koalacast.core.model.Favorite @@ -52,6 +53,7 @@ class LibraryViewModel @Inject constructor( private val progress: ProgressRepository, private val player: PlayerConnection, private val preferences: PreferencesRepository, + private val sync: SyncRepository, ) : ViewModel() { init { @@ -61,6 +63,28 @@ class LibraryViewModel @Inject constructor( private val _tab = MutableStateFlow(LibraryTab.SUBSCRIPTIONS) val tab: StateFlow = _tab + private val _refreshing = MutableStateFlow(false) + val refreshing: StateFlow = _refreshing + + /** + * Pull-to-refresh. Every list here is fed from Room and updates itself, so + * the only thing a listener can actually be waiting for is the account + * catching up with another device — which is exactly what a pull means on + * this screen. + */ + fun refresh() { + if (_refreshing.value) return + viewModelScope.launch { + _refreshing.value = true + try { + account.resolvePendingSubscriptions() + sync.syncNow() + } finally { + _refreshing.value = false + } + } + } + private val storedState = combine( library.allSubscriptions, progress.inProgress, diff --git a/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastScreen.kt b/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastScreen.kt index 74f0abe8..d8dc5b3c 100644 --- a/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastScreen.kt +++ b/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastScreen.kt @@ -22,8 +22,10 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Text import androidx.compose.material3.Switch +import androidx.compose.material3.pulltorefresh.PullToRefreshBox import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf @@ -123,6 +125,7 @@ fun PodcastScreen( onBack = onBack, onOpenEpisode = onOpenEpisode, onRetry = viewModel::retry, + onRefresh = viewModel::refresh, onOpenSettings = onOpenSettings, onPlay = viewModel::play, onToggleSubscribe = viewModel::toggleSubscribe, @@ -146,6 +149,7 @@ fun PodcastScreen( } } +@OptIn(ExperimentalMaterial3Api::class) @Composable internal fun PodcastContent( state: PodcastUiState, @@ -153,6 +157,7 @@ internal fun PodcastContent( onBack: () -> Unit, onOpenEpisode: (String) -> Unit, onRetry: () -> Unit, + onRefresh: () -> Unit = onRetry, onOpenSettings: () -> Unit = {}, onPlay: (Episode) -> Unit, onToggleSubscribe: () -> Unit, @@ -199,10 +204,13 @@ internal fun PodcastContent( ) } + PullToRefreshBox( + isRefreshing = state.refreshing, + onRefresh = onRefresh, + modifier = modifier.fillMaxSize().background(colors.bgPanel), + ) { LazyColumn( - modifier = modifier - .fillMaxSize() - .background(colors.bgPanel), + modifier = Modifier.fillMaxSize(), state = listState, contentPadding = contentPadding, ) { @@ -359,6 +367,7 @@ internal fun PodcastContent( } } } + } } @Composable diff --git a/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastViewModel.kt b/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastViewModel.kt index 82fbb519..2d669ba8 100644 --- a/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastViewModel.kt +++ b/apps/android/feature/podcast/src/main/kotlin/net/koalastuff/koalacast/feature/podcast/PodcastViewModel.kt @@ -41,6 +41,8 @@ data class PodcastUiState( val explicitBlocked: Boolean = false, val episodes: List = emptyList(), val loadingMore: Boolean = false, + /** A pull-to-refresh in flight, as opposed to the first load's skeleton. */ + val refreshing: Boolean = false, val endReached: Boolean = false, val paginationError: Boolean = false, val subscribed: Boolean = false, @@ -94,6 +96,22 @@ class PodcastViewModel @Inject constructor( fun retry() = load(force = true) + /** + * Pull-to-refresh: the same forced reload as [retry], but it drives the + * refresh indicator instead of the first-load skeleton, so an already + * populated episode list stays on screen while the feed is re-read. + */ + fun refresh() { + if (_state.value.refreshing) return + _state.update { it.copy(refreshing = true) } + load(force = true) + val running = loadJob + viewModelScope.launch { + running?.join() + _state.update { it.copy(refreshing = false) } + } + } + private fun load(force: Boolean) { loadJob?.cancel() loadJob = viewModelScope.launch { diff --git a/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileScreen.kt b/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileScreen.kt index 0f382a30..e142bad2 100644 --- a/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileScreen.kt +++ b/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileScreen.kt @@ -22,7 +22,9 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Text +import androidx.compose.material3.pulltorefresh.PullToRefreshBox import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf @@ -75,6 +77,7 @@ fun ProfileScreen( viewModel: ProfileViewModel = hiltViewModel(), ) { val state by viewModel.state.collectAsStateWithLifecycle() + val refreshing by viewModel.refreshing.collectAsStateWithLifecycle() val context = LocalContext.current val coroutineScope = rememberCoroutineScope() val export = rememberLauncherForActivityResult( @@ -85,6 +88,8 @@ fun ProfileScreen( ProfileContent( state = state, + refreshing = refreshing, + onRefresh = viewModel::refresh, onSetRange = viewModel::setRange, onOpenPodcast = onOpenPodcast, onOpenSettings = onOpenSettings, @@ -97,9 +102,12 @@ fun ProfileScreen( ) } +@OptIn(ExperimentalMaterial3Api::class) @Composable internal fun ProfileContent( state: ProfileUiState, + refreshing: Boolean = false, + onRefresh: () -> Unit = {}, onSetRange: (StatsRange) -> Unit, onOpenPodcast: (String) -> Unit, onOpenSettings: () -> Unit, @@ -115,10 +123,14 @@ internal fun ProfileContent( val stats = state.stats var visibleHistory by remember { mutableIntStateOf(20) } + PullToRefreshBox( + isRefreshing = refreshing, + onRefresh = onRefresh, + modifier = modifier.fillMaxSize().background(colors.bgPanel), + ) { Column( - modifier = modifier + modifier = Modifier .fillMaxSize() - .background(colors.bgPanel) .verticalScroll(rememberScrollState()) .padding(contentPadding) .padding(horizontal = KoalaSpacing.screenH, vertical = KoalaSpacing.sectionV), @@ -390,6 +402,7 @@ internal fun ProfileContent( }, ) } + } } @Composable diff --git a/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileViewModel.kt b/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileViewModel.kt index 6932f00c..47c70aba 100644 --- a/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileViewModel.kt +++ b/apps/android/feature/profile/src/main/kotlin/net/koalastuff/koalacast/feature/profile/ProfileViewModel.kt @@ -8,9 +8,11 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.launch import net.koalastuff.koalacast.core.data.repository.AccountRepository import net.koalastuff.koalacast.core.data.repository.LibraryRepository import net.koalastuff.koalacast.core.data.repository.ProgressRepository +import net.koalastuff.koalacast.core.data.repository.SyncRepository import net.koalastuff.koalacast.core.model.ListeningSession import net.koalastuff.koalacast.core.model.PlaybackProgress import java.time.ZonedDateTime @@ -35,10 +37,31 @@ class ProfileViewModel @Inject constructor( progress: ProgressRepository, library: LibraryRepository, accounts: AccountRepository, + private val sync: SyncRepository, ) : ViewModel() { private val range = MutableStateFlow(StatsRange.YEAR) + private val _refreshing = MutableStateFlow(false) + val refreshing: StateFlow = _refreshing + + /** + * Pull-to-refresh. Statistics are computed from local listening sessions, so + * the only thing that can be missing is what another device has not handed + * over yet. + */ + fun refresh() { + if (_refreshing.value) return + viewModelScope.launch { + _refreshing.value = true + try { + sync.syncNow() + } finally { + _refreshing.value = false + } + } + } + val state: StateFlow = combine( progress.listeningHistory, progress.recentHistory, diff --git a/apps/web/package.json b/apps/web/package.json index c8a3dae5..9ba65284 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,6 +1,6 @@ { "name": "koalacast-web", - "version": "0.11.4", + "version": "0.11.5", "private": true, "packageManager": "npm@11.16.0", "type": "module", diff --git a/apps/web/src/lib/i18n/messages/de.json b/apps/web/src/lib/i18n/messages/de.json index f83cec53..38a81268 100644 --- a/apps/web/src/lib/i18n/messages/de.json +++ b/apps/web/src/lib/i18n/messages/de.json @@ -794,10 +794,15 @@ "addBookmarkAt": "Zeitmarke {time}", "removeBookmarkAt": "Zeitmarke bei {time} entfernen", "bookmarkAdded": "Zeitmarke gespeichert.", - "handoff": "Auf anderem Gerät fortsetzen", - "handoffText": "Diese KoalaCast-Folge an der geteilten Position fortsetzen.", - "handoffCopied": "Fortsetzungslink kopiert.", - "handoffFailed": "Fortsetzungslink konnte nicht geteilt werden.", + "share": "Teilen", + "shareNative": "Teilen …", + "shareText": "Diese Folge auf KoalaCast anhören.", + "shareLinkLabel": "Link zur Folge", + "shareFromPosition": "Ab {time} starten", + "shareCopy": "Link kopieren", + "shareCopied": "Link kopiert.", + "shareEmail": "Per E-Mail teilen", + "shareFailed": "Der Link konnte nicht geteilt werden.", "unknownDuration": "Unbekannte Dauer", "chaptersLoadError": "Kapitel konnten nicht geladen werden.", "transcriptLoadError": "Transkript konnte nicht geladen werden.", diff --git a/apps/web/src/lib/i18n/messages/en.json b/apps/web/src/lib/i18n/messages/en.json index 2fa6548f..5e5f157d 100644 --- a/apps/web/src/lib/i18n/messages/en.json +++ b/apps/web/src/lib/i18n/messages/en.json @@ -794,10 +794,15 @@ "addBookmarkAt": "Bookmark {time}", "removeBookmarkAt": "Remove bookmark at {time}", "bookmarkAdded": "Time bookmark saved.", - "handoff": "Continue on another device", - "handoffText": "Continue this KoalaCast episode from the shared position.", - "handoffCopied": "Continue link copied.", - "handoffFailed": "Continue link could not be shared.", + "share": "Share", + "shareNative": "Share…", + "shareText": "Listen to this episode on KoalaCast.", + "shareLinkLabel": "Episode link", + "shareFromPosition": "Start at {time}", + "shareCopy": "Copy link", + "shareCopied": "Link copied.", + "shareEmail": "Share by email", + "shareFailed": "The link could not be shared.", "unknownDuration": "Unknown duration", "chaptersLoadError": "Chapters could not be loaded.", "transcriptLoadError": "Transcript could not be loaded.", diff --git a/apps/web/src/lib/stores/sync-push-isolation.test.ts b/apps/web/src/lib/stores/sync-push-isolation.test.ts new file mode 100644 index 00000000..8cb6164e --- /dev/null +++ b/apps/web/src/lib/stores/sync-push-isolation.test.ts @@ -0,0 +1,166 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// The sync store reaches into IndexedDB, the podcast-settings store and the +// player's own state. None of that is what this file is about: it is about what +// the push loop does when the server refuses one record, so everything else is +// stubbed down to "there is exactly one favourite to send". +const favorite = { + episode_id: 'ep-bad', + added_at: 1_000, + podcast_id: 'pod-1', + title: 'Episode' +}; +const secondFavorite = { + episode_id: 'ep-good', + added_at: 1_001, + podcast_id: 'pod-1', + title: 'Another episode' +}; + +vi.mock('$lib/idb/db', () => ({ + getLocalSubscriptions: async () => [], + getLocalFavorites: async () => [favorite, secondFavorite], + getAllLocalPlaybackStates: async () => [], + getLocalListeningSessions: async () => [], + getLocalListeningSession: async () => undefined, + getLocalQueue: async () => [], + getLocalQueueUpdatedAt: async () => 0, + getTombstones: async () => [], + getTombstone: async () => undefined, + acknowledgeTombstone: async () => {}, + getLocalPlaybackState: async () => undefined, + saveLocalSubscription: async () => {}, + saveLocalPlaybackState: async () => {}, + saveLocalListeningSession: async () => {}, + addLocalFavorite: async () => {}, + removeLocalSubscriptionSilent: async () => {}, + removeLocalFavoriteSilent: async () => {}, + replaceLocalQueueFromSync: async () => {}, + replaceLocalSyncSnapshot: async () => {} +})); + +vi.mock('$lib/stores/podcast-settings', () => ({ + applySyncedPodcastPlaybackSettings: () => {}, + clearPodcastPlaybackSettingsContext: () => {}, + getAllPodcastPlaybackSettings: () => [], + removePodcastPlaybackSettings: () => {} +})); + +vi.mock('$lib/stores/prefs.svelte', () => ({ + prefs: { + updatedAt: 0, + syncPayload: () => ({ updated_at: 0 }), + applySynced: () => {}, + resetSynced: () => {} + } +})); + +class MemoryStorage { + #values = new Map(); + getItem(key: string) { + return this.#values.get(key) ?? null; + } + setItem(key: string, value: string) { + this.#values.set(key, value); + } + removeItem(key: string) { + this.#values.delete(key); + } +} + +interface PushBody { + operations: { entity_id: string }[]; +} + +/** Every push body the store sent, in order. */ +let pushes: PushBody[] = []; + +function installFetch(rejectedEntityId: string | null) { + pushes = []; + vi.stubGlobal('fetch', async (input: string, init?: RequestInit) => { + if (!init || init.method !== 'POST') { + return new Response( + JSON.stringify({ changesets: [], next_cursor: 0, has_more: false, data_generation: 0 }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + const body = JSON.parse(String(init.body)) as PushBody; + pushes.push(body); + const offender = body.operations.find((op) => op.entity_id === rejectedEntityId); + if (offender) { + return new Response( + JSON.stringify({ error: 'invalid favorite payload', operation_index: 0 }), + { status: 400, headers: { 'Content-Type': 'application/json' } } + ); + } + return new Response(JSON.stringify({ applied_ops: body.operations.length }), { status: 200 }); + }); +} + +async function loadSync() { + vi.stubGlobal('localStorage', new MemoryStorage()); + vi.stubGlobal('document', { visibilityState: 'visible', addEventListener() {}, removeEventListener() {} }); + vi.resetModules(); + const module = await import('./sync.svelte'); + return module.sync; +} + +describe('sync push isolation', () => { + beforeEach(() => { + vi.unstubAllGlobals(); + }); + + it('sends everything when the server accepts the batch', async () => { + installFetch(null); + const sync = await loadSync(); + sync.userId = 'user-1'; + await sync.syncNow(); + + expect(sync.status).toBe('idle'); + expect(sync.rejectedOperations).toBe(0); + expect(pushes).toHaveLength(1); + expect(pushes[0].operations.map((op) => op.entity_id).sort()).toEqual(['ep-bad', 'ep-good']); + }); + + it('isolates the one record the server refuses and still ships the rest', async () => { + installFetch('ep-bad'); + const sync = await loadSync(); + sync.userId = 'user-1'; + await sync.syncNow(); + + // A 400 used to abort the run, leaving the watermark where it was so the + // same batch went back up every 45 seconds forever. + expect(sync.status).toBe('idle'); + expect(sync.rejectedOperations).toBe(1); + expect(sync.lastError).toContain('1 record(s) rejected'); + + const accepted = pushes.filter( + (body) => + body.operations.length === 1 && body.operations[0].entity_id === 'ep-good' + ); + expect(accepted).toHaveLength(1); + + // And the run must not repeat the rejected record on the next pass. + const before = pushes.length; + await sync.syncNow(); + expect(pushes.length).toBe(before); + }); + + it('keeps failing the run on a server error so it retries later', async () => { + vi.stubGlobal('fetch', async (input: string, init?: RequestInit) => { + if (!init || init.method !== 'POST') { + return new Response( + JSON.stringify({ changesets: [], next_cursor: 0, has_more: false, data_generation: 0 }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ); + } + return new Response('{}', { status: 503 }); + }); + const sync = await loadSync(); + sync.userId = 'user-1'; + await sync.syncNow(); + + expect(sync.status).toBe('error'); + expect(sync.rejectedOperations).toBe(0); + }); +}); diff --git a/apps/web/src/lib/stores/sync.svelte.ts b/apps/web/src/lib/stores/sync.svelte.ts index 418438bc..5f640d62 100644 --- a/apps/web/src/lib/stores/sync.svelte.ts +++ b/apps/web/src/lib/stores/sync.svelte.ts @@ -110,6 +110,8 @@ class SyncStore { lastError = $state(null); /** Records the server sent that this build could not read. */ skippedChangesets = $state(0); + /** Local records the server refused, isolated so the rest still goes up. */ + rejectedOperations = $state(0); #timer: ReturnType | null = null; #onVisible: (() => void) | null = null; @@ -317,6 +319,7 @@ class SyncStore { this.#inFlight = true; this.status = 'syncing'; this.skippedChangesets = 0; + this.rejectedOperations = 0; try { await this.#pull(userId, generation, controller.signal); this.#assertRun(userId, generation, controller.signal); @@ -326,10 +329,14 @@ class SyncStore { this.lastSyncedAt = Date.now(); // A skipped record is not a failed sync — the rest went through — but it // must still be said out loud, or the data silently goes missing. - this.lastError = - this.skippedChangesets > 0 - ? `${this.skippedChangesets} unreadable record(s) skipped, rest synced` - : null; + const problems: string[] = []; + if (this.skippedChangesets > 0) { + problems.push(`${this.skippedChangesets} unreadable record(s) skipped`); + } + if (this.rejectedOperations > 0) { + problems.push(`${this.rejectedOperations} record(s) rejected by the server`); + } + this.lastError = problems.length > 0 ? `${problems.join(', ')}, rest synced` : null; } catch (err) { if (err instanceof DOMException && err.name === 'AbortError') return; if (generation !== this.#generation || this.userId !== userId) return; @@ -580,50 +587,26 @@ class SyncStore { return; } - for (let index = 0; index < ops.length; index += 250) { - this.#assertRun(userId, generation, signal); - const batch = ops.slice(index, index + 250); - const res = await fetch('/api/v1/sync', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - operations: batch, - client_schema_version: 2, - data_generation: this.#getDataGeneration(userId) - }), - signal - }); - if (res.status === 401) throw new SyncAuthError(); - if (res.status === 409) { - const conflict = await res.json().catch(() => null); - if (conflict?.code !== 'DATA_GENERATION_MISMATCH') { - throw new Error('sync push conflict without generation'); - } - await this.#adoptDataGeneration( - userId, - conflict.data_generation, - generation, - signal - ); - return; - } - if (!res.ok) { - const failure = await res.json().catch(() => null) as { - error?: unknown; - operation_index?: unknown; - entity_type?: unknown; - } | null; - const detail = typeof failure?.error === 'string' ? `: ${failure.error}` : ''; - const operation = Number.isInteger(failure?.operation_index) && typeof failure?.entity_type === 'string' - ? ` (operation ${failure.operation_index}: ${failure.entity_type})` - : ''; - throw new Error(`sync push failed: ${res.status}${detail}${operation}`); - } - for (const op of batch) { - if (op.entity_type === 'listening_session') sessionWatermarks[op.entity_id] = op.client_timestamp; + try { + for (let index = 0; index < ops.length; index += 250) { + await this.#pushBatch(userId, ops.slice(index, index + 250), generation, signal); } - this.#setSessionWatermarks(userId, sessionWatermarks); + } catch (err) { + // The account was wiped on another device. The local copy has already + // been reset by #adoptDataGeneration; nothing from before the reset may + // be marked as pushed. + if (err instanceof SyncDataResetError) return; + throw err; + } + + // Every operation is now accounted for: accepted, deduplicated, or isolated + // and reported as rejected. Advancing past a rejected record is deliberate — + // re-sending it forever is what used to keep the whole account off the + // server. + for (const op of ops) { + if (op.entity_type === 'listening_session') sessionWatermarks[op.entity_id] = op.client_timestamp; } + this.#setSessionWatermarks(userId, sessionWatermarks); const nextWatermarks = { ...previousWatermarks }; for (const op of ops) { if (op.entity_type === 'listening_session') continue; @@ -637,9 +620,75 @@ class SyncStore { // our own ops (idempotent) avoids skipping a concurrent device's ops that // landed at a lower cursor. } + + /** + * Sends one batch, and refuses to let a single bad record stop everything. + * + * A 400 used to abort the whole push, which meant the watermark never moved + * and the next attempt sent the same rejected operation again — every 45 + * seconds, forever. One record the server will not accept (a listening + * session spanning longer than the server's ceiling, an episode the server + * evicted, a payload from a newer build) therefore kept an entire account's + * subscriptions, progress and queue off the server permanently, while pull + * kept working and made it look like sync was fine. + * + * On a rejection the batch is halved and retried until the offending + * operation is alone; that one is counted and skipped, and everything else + * goes through. This mirrors what the Android client already does. + */ + async #pushBatch( + userId: string, + batch: SyncOperation[], + generation: number, + signal: AbortSignal + ): Promise { + if (batch.length === 0) return; + this.#assertRun(userId, generation, signal); + const res = await fetch('/api/v1/sync', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + operations: batch, + client_schema_version: 2, + data_generation: this.#getDataGeneration(userId) + }), + signal + }); + if (res.status === 401) throw new SyncAuthError(); + if (res.status === 409) { + const conflict = await res.json().catch(() => null); + if (conflict?.code !== 'DATA_GENERATION_MISMATCH') { + throw new Error('sync push conflict without generation'); + } + await this.#adoptDataGeneration(userId, conflict.data_generation, generation, signal); + throw new SyncDataResetError(); + } + if (res.ok) return; + + const failure = (await res.json().catch(() => null)) as { + error?: unknown; + operation_index?: unknown; + entity_type?: unknown; + } | null; + const detail = typeof failure?.error === 'string' ? `: ${failure.error}` : ''; + // Only a rejection of the *content* is worth isolating. A 500 or a 429 is + // about the request as a whole and must still fail the sync so it retries. + if (res.status !== 400) throw new Error(`sync push failed: ${res.status}${detail}`); + if (batch.length === 1) { + const op = batch[0]; + this.rejectedOperations++; + console.warn(`sync: dropping ${op.entity_type}/${op.entity_id}${detail}`); + return; + } + const half = Math.floor(batch.length / 2); + await this.#pushBatch(userId, batch.slice(0, half), generation, signal); + await this.#pushBatch(userId, batch.slice(half), generation, signal); + } } class SyncAuthError extends Error {} +/** The account's data was reset elsewhere; this push run is void. */ +class SyncDataResetError extends Error {} async function applyChangeset(cs: Changeset): Promise { if ( @@ -682,7 +731,13 @@ async function applyChangeset(cs: Changeset): Promise { added_at: p.added_at || cs.client_timestamp || Date.now(), updated_at: p.updated_at || cs.client_timestamp || p.added_at || Date.now(), inbox_mode: p.inbox_mode, - folder: p.folder + // An empty folder in the payload means "this device files it + // nowhere", not "remove the folder the other device put it in". + // Passing the blank straight through cleared the local folder every + // time a peer that never used folders pushed the same subscription; + // undefined lets saveLocalSubscription keep what is already stored, + // which is what the Android client has always done. + folder: p.folder || undefined }); } else throw new Error('invalid subscription changeset payload'); } else if (cs.entity_type === 'favorite') { diff --git a/apps/web/src/lib/sync-payload.test.ts b/apps/web/src/lib/sync-payload.test.ts index dd09db74..1bafe4a9 100644 --- a/apps/web/src/lib/sync-payload.test.ts +++ b/apps/web/src/lib/sync-payload.test.ts @@ -32,4 +32,31 @@ describe('normalizeListeningSessionForSync', () => { speed_weighted_ms: 801 }); }); + + it('clamps a session longer than the server accepts instead of shipping a 400', () => { + const week = 7 * 24 * 60 * 60 * 1000; + const endedAt = 1_800_000_000_000; + const normalized = normalizeListeningSessionForSync({ + id: 'session-2', + episode_id: 'episode-1', + podcast_id: 'podcast-1', + title: 'Episode', + podcast_title: 'Podcast', + // A tab left open with the player paused over a long holiday. + started_at: endedAt - week * 3, + ended_at: endedAt, + wall_clock_ms: week * 2, + audio_listened_ms: week * 9, + speed_saved_ms: 0, + silence_saved_ms: 0, + manual_skipped_ms: 0, + intro_outro_skipped_ms: 0, + speed_weighted_ms: 0 + }); + + expect(normalized.ended_at).toBe(endedAt); + expect(normalized.ended_at - normalized.started_at).toBe(week); + expect(normalized.wall_clock_ms).toBe(week); + expect(normalized.audio_listened_ms).toBe(week * 4); + }); }); diff --git a/apps/web/src/lib/sync-payload.ts b/apps/web/src/lib/sync-payload.ts index 714bd7cd..05a2c3ef 100644 --- a/apps/web/src/lib/sync-payload.ts +++ b/apps/web/src/lib/sync-payload.ts @@ -1,7 +1,20 @@ import type { LocalListeningSession } from '$lib/idb/db'; -function integerMilliseconds(value: number): number { - return Number.isFinite(value) ? Math.max(0, Math.round(value)) : 0; +/** + * The server's own ceilings for a listening session (see + * `validateListeningSession` in services/api). A payload past them is rejected + * with a 400, so clamping here is not cosmetic: an unclamped session is a + * record that can never be uploaded. + * + * They are reachable in normal use. A session's span is measured from when + * playback started to when it was flushed, and a tab left open over a holiday + * with the player paused produces exactly one such span. + */ +const MAX_SESSION_SPAN_MS = 7 * 24 * 60 * 60 * 1000; +const MAX_SESSION_METRIC_MS = MAX_SESSION_SPAN_MS * 4; + +function integerMilliseconds(value: number, ceiling = MAX_SESSION_METRIC_MS): number { + return Number.isFinite(value) ? Math.min(ceiling, Math.max(0, Math.round(value))) : 0; } function timestampMilliseconds(value: number): number { @@ -11,12 +24,15 @@ function timestampMilliseconds(value: number): number { export function normalizeListeningSessionForSync( session: LocalListeningSession ): LocalListeningSession { - const startedAt = timestampMilliseconds(session.started_at); + const endedAt = Math.max(timestampMilliseconds(session.started_at), timestampMilliseconds(session.ended_at)); + // Hold the span, not the end: the end is when listening actually stopped and + // is what every last-writer-wins comparison keys on. + const startedAt = Math.max(timestampMilliseconds(session.started_at), endedAt - MAX_SESSION_SPAN_MS); return { ...session, started_at: startedAt, - ended_at: Math.max(startedAt, timestampMilliseconds(session.ended_at)), - wall_clock_ms: integerMilliseconds(session.wall_clock_ms), + ended_at: endedAt, + wall_clock_ms: integerMilliseconds(session.wall_clock_ms, MAX_SESSION_SPAN_MS), audio_listened_ms: integerMilliseconds(session.audio_listened_ms), speed_saved_ms: integerMilliseconds(session.speed_saved_ms), silence_saved_ms: integerMilliseconds(session.silence_saved_ms), diff --git a/apps/web/src/routes/episode/[id]/+page.svelte b/apps/web/src/routes/episode/[id]/+page.svelte index 224dcda3..fff07488 100644 --- a/apps/web/src/routes/episode/[id]/+page.svelte +++ b/apps/web/src/routes/episode/[id]/+page.svelte @@ -214,22 +214,62 @@ handlePlay(); } - async function shareHandoff() { - if (!episode || !browser) return; + // Sharing an episode used to be a single button labelled "continue on another + // device" that quietly wrote to the clipboard on every browser without a + // native share sheet — a name that described neither the thing shared nor + // what pressing it did. It is a share control now: it opens, it shows the + // exact link it is about to hand over, and every way of handing it over is + // named. + let shareOpen = $state(false); + let shareFromPosition = $state(true); + + const shareUrl = $derived.by(() => { + if (!episode || !browser) return ''; const url = new URL(`/episode/${encodeURIComponent(episode.id)}`, location.origin); - url.searchParams.set('t', String(Math.floor(handoffPositionMs / 1000))); - const data = { title: episode.title, text: t('episode.handoffText'), url: url.toString() }; + const seconds = Math.floor(handoffPositionMs / 1000); + if (shareFromPosition && seconds > 0) url.searchParams.set('t', String(seconds)); + return url.toString(); + }); + + /** True where the browser can hand the link to the operating system. */ + const canShareNatively = $derived(browser && typeof navigator.share === 'function'); + + function toggleShare() { + shareOpen = !shareOpen; + } + + async function shareNatively() { + if (!episode || !browser || !shareUrl) return; + const data = { title: episode.title, text: t('episode.shareText'), url: shareUrl }; try { - if (navigator.share) await navigator.share(data); - else { - await navigator.clipboard.writeText(url.toString()); - toast.success(t('episode.handoffCopied')); - } + await navigator.share(data); + shareOpen = false; } catch (error: any) { - if (error?.name !== 'AbortError') toast.error(t('episode.handoffFailed')); + if (error?.name !== 'AbortError') toast.error(t('episode.shareFailed')); } } + async function copyShareLink() { + if (!shareUrl) return; + try { + await navigator.clipboard.writeText(shareUrl); + toast.success(t('episode.shareCopied')); + shareOpen = false; + } catch { + // A denied clipboard is not a dead end: the field below holds the link + // and the listener can still select it by hand. + toast.error(t('episode.shareFailed')); + } + } + + function shareByEmail() { + if (!episode || !shareUrl) return; + const subject = encodeURIComponent(episode.title); + const body = encodeURIComponent(`${t('episode.shareText')}\n\n${shareUrl}`); + window.location.href = `mailto:?subject=${subject}&body=${body}`; + shareOpen = false; + } + async function handleAddToQueue() { if (!episode) return; await player.addToQueue({ @@ -530,9 +570,15 @@ {t('episode.addBookmarkAt', { time: formatTimecode(handoffPositionMs) })} - {#if episode.chapters_url} {/if} + + {#if shareOpen} + + {/if} @@ -718,6 +795,57 @@ gap: 0.75rem; } + /* The share control opens in place rather than in a dialog: it is a short + list of ways to hand over one link, and a modal over the episode would be + heavier than the decision it asks for. */ + .share-panel { + margin-top: 0.9rem; + padding: 0.9rem; + border: 1px solid var(--border-ui); + border-radius: 12px; + background: var(--bg-elevated); + display: flex; + flex-direction: column; + gap: 0.75rem; + } + + .share-position { + display: flex; + align-items: center; + gap: 0.5rem; + font-size: 0.85rem; + font-weight: 600; + color: var(--text-secondary); + } + + .share-link { + display: flex; + flex-direction: column; + gap: 0.35rem; + } + + .share-link-label { + font-size: 0.78rem; + font-weight: 700; + color: var(--text-secondary); + } + + .share-link input { + width: 100%; + padding: 0.55rem 0.7rem; + border: 1px solid var(--border-ui); + border-radius: 8px; + background: var(--bg-primary); + color: var(--text-primary); + font-size: 0.85rem; + } + + .share-actions { + display: flex; + flex-wrap: wrap; + gap: 0.6rem; + } + /* Only what makes it the play button: the show's colour and its lift. The shape comes from the shared `.btn`, like every other action here. */ .btn-play { diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index d5135863..bd8c9280 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -42,6 +42,7 @@ KoalaCast enforces strict configuration rules to ensure self-hosters and securit | Feed Timeout | `FEED_REQUEST_TIMEOUT_MS` | No | Outbound feed request deadline | | Feed Response Limit | `FEED_MAX_RESPONSE_BYTES` | No | Maximum RSS response body size | | Stored Episodes per Feed | `FEED_MAX_STORED_EPISODES` | No | Recent metadata rows retained per podcast; user-state references are preserved | +| Feed Refresh Interval | `FEED_REFRESH_INTERVAL_MS` | No | Delay before a healthy notification-enabled feed is rechecked (clamped to 15 min – 24 h) | | Podcast Index Credentials | `PODCAST_INDEX_KEY`, `PODCAST_INDEX_SECRET` | No | Optional credentials for catalog search | | Initial Admin | `ADMIN_USERNAME`, `ADMIN_PASSWORD` | No | Create/promote an administrator at startup; password is used only for first creation | | Web Push VAPID Keys | `WEB_PUSH_VAPID_PUBLIC_KEY`, `WEB_PUSH_VAPID_PRIVATE_KEY` | No | Enables authenticated server-to-browser push delivery | diff --git a/docs/current-status.md b/docs/current-status.md index 7c837884..d0d20819 100644 --- a/docs/current-status.md +++ b/docs/current-status.md @@ -12,9 +12,9 @@ This document records shipped behavior. Proposed work belongs in | :--- | :--- | :--- | | **Web application** | Implemented | SvelteKit 5 static SPA, served by the Go application. Responsive three-column layout with resizable/collapsible side rails and a four-destination mobile navigation matching Android. | | **Discovery and search** | Implemented | iTunes charts/search, optional Podcast Index search, direct RSS addition, language filters, multi-select preferred/hidden genres, account-synced per-podcast hiding, clear/reset behavior, and subscription-aware Inbox. | -| **Playback** | Implemented | HTML audio + Media Session, compatible-browser Remote Playback, speed control, skip controls, silence trimming, volume boost, selectable audio visualisers, chapters, transcripts, queue, keyboard shortcuts, timestamp bookmarks, and handoff links. The timeline scrubs live with a hover readout and chapter markers, a jump of fifteen seconds or more offers the position it came from, the previous-track control steps back through the played history, the full-screen view carries a reorderable queue, and the transcript follows the playhead. The sleep timer counts listening time, not wall-clock time, so a pause does not spend it. Web Audio effects need CORS, so blocked enclosures are first retried against the host their redirect chain resolves to and only fall back to the audio relay when that host refuses too. | +| **Playback** | Implemented | HTML audio + Media Session, compatible-browser Remote Playback, speed control, skip controls, silence trimming, volume boost, selectable audio visualisers, chapters, transcripts, queue, keyboard shortcuts, timestamp bookmarks, and a share control that hands over a link carrying the current position (native share sheet where the browser has one, copy or email everywhere else). The timeline scrubs live with a hover readout and chapter markers, a jump of fifteen seconds or more offers the position it came from, the previous-track control steps back through the played history, the full-screen view carries a reorderable queue, and the transcript follows the playhead. The sleep timer counts listening time, not wall-clock time, so a pause does not spend it. Web Audio effects need CORS, so blocked enclosures are first retried against the host their redirect chain resolves to and only fall back to the audio relay when that host refuses too. | | **Local-first storage** | Implemented | Subscriptions with folders, queue plus reusable named queues, smart queues (saved rules evaluated over cached episodes), favorites, timestamp bookmarks, playback progress, listening sessions, and preferences work without an account in IndexedDB/LocalStorage. Deleting local data removes the downloaded audio and every stored preference, not only the database rows. | -| **Accounts and sync** | Implemented with documented boundaries | Username/password accounts, recovery codes, web sessions, Android device tokens, account export, account deletion, and transactional synchronized-data deletion that keeps the account. A generation/epoch blocks stale clients from restoring deleted data and makes web and Android clear their old local account copy before pushing. Incremental sync covers subscriptions, favorites, playback state, listening sessions, queue, podcast settings, and global settings. Unknown settings keys survive mixed-version clients, settings merge per field, and unreadable records are skipped and counted instead of wedging a pull. See [sync-protocol/specification.md](sync-protocol/specification.md). Named queues, folders, and timestamp bookmarks remain local. | +| **Accounts and sync** | Implemented with documented boundaries | Username/password accounts, recovery codes, web sessions, Android device tokens, account export, account deletion, and transactional synchronized-data deletion that keeps the account. A generation/epoch blocks stale clients from restoring deleted data and makes web and Android clear their old local account copy before pushing. Incremental sync covers subscriptions, favorites, playback state, listening sessions, queue, podcast settings, and global settings. Unknown settings keys survive mixed-version clients, settings merge per field, unreadable records are skipped and counted instead of wedging a pull, and a record the server refuses is isolated by halving the batch and reported instead of wedging every later push behind it. See [sync-protocol/specification.md](sync-protocol/specification.md). Named queues, folders, and timestamp bookmarks remain local. | | **Statistics** | Implemented | Personal listening duration, sessions, podcasts, speed and time-saved metrics. Signed-in users can separately opt into global aggregates, podcast rankings, and the listener leaderboard; participation defaults to off. | | **Themes and accessibility** | Implemented | System/light/dark modes, nine palettes (Fjord default; Eucalyptus retained), selectable start screen, configurable artwork privacy, download policies now honoured by both clients, scalable/resizable layout, reduced motion, focus treatment, tooltips, accessible names, and English/German UI. | | **SEO and sharing** | Implemented | Canonical/robots metadata, sitemap with Git-derived `lastmod`, WebSite/SoftwareApplication JSON-LD, `llms.txt`, `llms-full.txt`, and 1200×630 Open Graph/Twitter artwork. | @@ -33,8 +33,11 @@ hiding, playback, Room local-first library, resumable downloads, Inbox, profile statistics, accounts, device-token sync, OPML, global statistics, Android Auto/Wear browse support, a home-screen widget, chapters, Chromecast output transfer, dynamic artwork palettes, advanced download policies, timestamp -bookmarks, handoff links, named queues, and podcast folders. Destructive -actions sit behind a menu or a confirmation, the start screen is selectable, +bookmarks, episode sharing through the system share sheet, named queues, and +podcast folders. Pull-to-refresh reaches the account on Inbox, Library and the +statistics screens, and re-reads the feed on Discover and a podcast; opening the +app from the background syncs at once rather than waiting out the periodic tick. +Destructive actions sit behind a menu or a confirmation, the start screen is selectable, sign-in is offered during onboarding, and the player fits one screen with an optional amplitude visualiser (off by default) fed by a Media3 tap on the app's own decoded PCM. The sleep timer counts listening time rather than diff --git a/docs/sync-protocol/specification.md b/docs/sync-protocol/specification.md index 12f0fc86..a3ddc995 100644 --- a/docs/sync-protocol/specification.md +++ b/docs/sync-protocol/specification.md @@ -40,6 +40,26 @@ end-to-end encrypted. a reset can return resolved public podcast metadata, but cannot recreate the account's deleted subscription rows or sync metadata. +### Rejected operations are isolated, not retried forever + +A `400` is the server's verdict on one operation's *content*, and a client that +treats it as a failure of the whole batch never moves its push watermark: the +same unacceptable record goes back up on every tick and every other local +change stays on the device behind it. Both clients therefore halve a rejected +batch until the offending operation is alone, count and report that one, and +let the rest through. A `409` (generation mismatch), `401`, `429` or `5xx` is +about the request rather than its contents and still fails the run so it +retries. + +### Payload fields the server does not model + +Both clients denormalize display metadata into the operation payload — an +episode's title, artwork, podcast, enclosure and duration — so a peer can +render the record without a second fetch. The sync log stores each payload as +received and Pull returns it, which makes it the only place that metadata +lives. Normalizing an operation therefore merges the server's own fields back +into the client's object rather than replacing it; unknown keys round-trip. + ### Materialized entity types - `subscription` @@ -113,6 +133,15 @@ Playback events carry `event_type`, `episode_id`, `position_ms`, - Within a playback session, a higher `per_session_seq` supersedes an older operation. +### Listening-session bounds + +A session is rejected when it spans more than seven days, when a metric exceeds +seven days (`wall_clock_ms`) or twenty-eight days (the rest), or when it names +no episode or podcast. Both clients clamp to those bounds before pushing rather +than discovering them as a rejection: a player left paused across a holiday +produces exactly such a span, and the end timestamp — which every last-writer +comparison keys on — is held while the start is moved forward. + ### Retention and full-resync signal The background worker compacts old sync-log entries. A pull older than the diff --git a/services/api/internal/config/config.go b/services/api/internal/config/config.go index 8e9f09c6..1381cb38 100644 --- a/services/api/internal/config/config.go +++ b/services/api/internal/config/config.go @@ -31,6 +31,7 @@ type Config struct { FeedRequestTimeoutMS int FeedMaxResponseBytes int64 FeedMaxStoredEpisodes int + FeedRefreshIntervalMS int WebPushVAPIDPublicKey string WebPushVAPIDPrivateKey string WebPushVAPIDSubject string @@ -65,6 +66,7 @@ func LoadConfig() (*Config, error) { FeedRequestTimeoutMS: getEnvInt("FEED_REQUEST_TIMEOUT_MS", 15000), FeedMaxResponseBytes: int64(getEnvInt("FEED_MAX_RESPONSE_BYTES", 33554432)), FeedMaxStoredEpisodes: getEnvInt("FEED_MAX_STORED_EPISODES", 200), + FeedRefreshIntervalMS: getEnvInt("FEED_REFRESH_INTERVAL_MS", 3600000), WebPushVAPIDPublicKey: strings.TrimSpace(os.Getenv("WEB_PUSH_VAPID_PUBLIC_KEY")), WebPushVAPIDPrivateKey: strings.TrimSpace(os.Getenv("WEB_PUSH_VAPID_PRIVATE_KEY")), WebPushVAPIDSubject: getEnv("WEB_PUSH_VAPID_SUBJECT", getEnv("PUBLIC_BASE_URL", "http://localhost:3000")), @@ -157,6 +159,29 @@ func validVAPIDSubject(subject string) bool { } } +// EffectiveFeedRefreshIntervalMS bounds how long a successfully fetched feed +// waits before the background worker looks at it again. +// +// This only governs subscriptions that asked to be notified about new episodes; +// everything else refreshes on demand when a listener opens it. The old +// hard-coded twenty-four hours meant a daily show could be announced most of a +// day late, which is not a notification. Conditional requests make the shorter +// cadence cheap: an unchanged feed answers 304 with no body. +func EffectiveFeedRefreshIntervalMS(configured int) int { + const minimum = 15 * 60 * 1000 + const maximum = 24 * 60 * 60 * 1000 + if configured <= 0 { + return 60 * 60 * 1000 + } + if configured < minimum { + return minimum + } + if configured > maximum { + return maximum + } + return configured +} + func EffectiveFeedMaxStoredEpisodes(configured int) int { if configured <= 0 { return 200 diff --git a/services/api/internal/config/config_test.go b/services/api/internal/config/config_test.go index 5528f4ce..b3d817a6 100644 --- a/services/api/internal/config/config_test.go +++ b/services/api/internal/config/config_test.go @@ -143,3 +143,24 @@ func TestConfig_WebPushRequiresCompleteValidVAPIDConfiguration(t *testing.T) { t.Fatalf("expected HTTPS VAPID subject to succeed: %v", err) } } + +// A healthy feed's next scheduled fetch decides how late a "new episode" +// notification can be. Twenty-four hours was not a notification. +func TestEffectiveFeedRefreshIntervalMS(t *testing.T) { + hour := 60 * 60 * 1000 + cases := map[string]struct { + configured int + want int + }{ + "unset falls back to an hour": {0, hour}, + "negative falls back": {-5, hour}, + "below the floor is raised": {60_000, 15 * 60 * 1000}, + "a sane value is kept": {30 * 60 * 1000, 30 * 60 * 1000}, + "above the ceiling is capped": {72 * 60 * 60 * 1000, 24 * 60 * 60 * 1000}, + } + for name, tc := range cases { + if got := EffectiveFeedRefreshIntervalMS(tc.configured); got != tc.want { + t.Errorf("%s: EffectiveFeedRefreshIntervalMS(%d) = %d, want %d", name, tc.configured, got, tc.want) + } + } +} diff --git a/services/api/internal/push/service.go b/services/api/internal/push/service.go index 30a57b0d..52d31435 100644 --- a/services/api/internal/push/service.go +++ b/services/api/internal/push/service.go @@ -2,6 +2,8 @@ package push import ( "context" + "crypto/sha256" + "encoding/base64" "encoding/json" "fmt" "io" @@ -35,6 +37,19 @@ func (s *Service) Configured() bool { return s != nil && s.cfg.WebPushVAPIDPublicKey != "" && s.cfg.WebPushVAPIDPrivateKey != "" } +// pushTopic derives the RFC 8030 Topic header for a podcast. +// +// The header is capped at 32 characters from the URL-safe base64 alphabet, and +// push services reject the whole request when it is longer. "podcast-" plus a +// 36-character UUID is 44, so every new-episode notification was answered with +// a 400 and no browser ever received one. A hash keeps the collapsing behaviour +// the topic is for — a second notification for the same show replaces the first +// one still queued — inside the length the spec allows. +func pushTopic(podcastID string) string { + sum := sha256.Sum256([]byte("podcast:" + podcastID)) + return base64.RawURLEncoding.EncodeToString(sum[:])[:32] +} + func (s *Service) NotifyNewEpisodes( ctx context.Context, podcastID string, @@ -73,13 +88,27 @@ func (s *Service) NotifyNewEpisodes( s.logger.Warn("failed to select web push subscriptions", "podcast_id", podcastID, "error", err) return } - defer rows.Close() + // Drain the cursor before sending anything. Each delivery is a network round + // trip to a third-party push service, and the loop below also deletes expired + // registrations — holding a read cursor open across both, on the same SQLite + // database, is how a popular show's notification run blocks its own writes. + type target struct{ endpoint, p256dh, auth, locale string } + targets := make([]target, 0) for rows.Next() { - var endpoint, p256dh, auth, locale string - if err := rows.Scan(&endpoint, &p256dh, &auth, &locale); err != nil { + var item target + if err := rows.Scan(&item.endpoint, &item.p256dh, &item.auth, &item.locale); err != nil { continue } + targets = append(targets, item) + } + if err := rows.Err(); err != nil { + s.logger.Warn("failed to read web push subscriptions", "podcast_id", podcastID, "error", err) + } + rows.Close() + + for _, item := range targets { + endpoint, p256dh, auth, locale := item.endpoint, item.p256dh, item.auth, item.locale body := episodes[0].Title if len(episodes) > 1 { if locale == "de" { @@ -108,7 +137,7 @@ func (s *Service) NotifyNewEpisodes( VAPIDPrivateKey: s.cfg.WebPushVAPIDPrivateKey, TTL: 86400, Urgency: webpush.UrgencyNormal, - Topic: "podcast-" + podcastID, + Topic: pushTopic(podcastID), }) if sendErr != nil { s.logger.Warn("web push send failed", "podcast_id", podcastID, "error", sendErr) diff --git a/services/api/internal/push/topic_test.go b/services/api/internal/push/topic_test.go new file mode 100644 index 00000000..44b8183b --- /dev/null +++ b/services/api/internal/push/topic_test.go @@ -0,0 +1,44 @@ +package push + +import ( + "strings" + "testing" +) + +// RFC 8030 §5.4 caps the Topic header at 32 characters from the URL-safe base64 +// alphabet. Push services reject the entire request when it is longer, so a +// topic built by concatenating a UUID silently disabled every new-episode +// notification. +func TestPushTopicFitsRFC8030(t *testing.T) { + const urlSafeBase64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_" + + ids := []string{ + "dbb4aca4-77d5-4ddf-9e4a-ddde8a68ae29", + "", + strings.Repeat("x", 512), + } + for _, id := range ids { + topic := pushTopic(id) + if len(topic) != 32 { + t.Fatalf("topic for %q is %d characters, want 32: %q", id, len(topic), topic) + } + if strings.ContainsFunc(topic, func(r rune) bool { + return !strings.ContainsRune(urlSafeBase64, r) + }) { + t.Fatalf("topic for %q leaves the URL-safe base64 alphabet: %q", id, topic) + } + } +} + +// The topic exists so a queued notification for a show is replaced by a newer +// one instead of stacking. That only works if it is stable per podcast and +// distinct between podcasts. +func TestPushTopicIsStableAndDistinct(t *testing.T) { + first := pushTopic("pod-1") + if first != pushTopic("pod-1") { + t.Fatal("topic is not stable for the same podcast") + } + if first == pushTopic("pod-2") { + t.Fatal("two podcasts collapsed onto the same topic") + } +} diff --git a/services/api/internal/rss/ssrf.go b/services/api/internal/rss/ssrf.go index d47df7e9..19ecef4b 100644 --- a/services/api/internal/rss/ssrf.go +++ b/services/api/internal/rss/ssrf.go @@ -21,6 +21,8 @@ var blockedCIDRs = []string{ "192.168.0.0/16", // Private Class C "169.254.0.0/16", // Link Local / Cloud Metadata "100.64.0.0/10", // Carrier-Grade NAT + "192.0.0.0/24", // IETF Protocol Assignments (incl. NAT64 well-known prefix) + "198.18.0.0/15", // Benchmarking (RFC 2544) "192.0.2.0/24", // TEST-NET-1 "198.51.100.0/24", // TEST-NET-2 "203.0.113.0/24", // TEST-NET-3 @@ -32,6 +34,11 @@ var blockedCIDRs = []string{ "fe80::/10", // IPv6 Link Local "ff00::/8", // IPv6 Multicast "2001:db8::/32", // IPv6 Documentation + // NAT64 maps an IPv4 address into IPv6, so on a network that runs one + // `64:ff9b::7f00:1` is a route to 127.0.0.1 that To4() does not unwrap and + // none of the IPv4 rules above can see. + "64:ff9b::/96", + "64:ff9b:1::/48", } var blockedNets []*net.IPNet diff --git a/services/api/internal/server/handlers/auth.go b/services/api/internal/server/handlers/auth.go index 1b0671a4..3a19aa00 100644 --- a/services/api/internal/server/handlers/auth.go +++ b/services/api/internal/server/handlers/auth.go @@ -37,6 +37,13 @@ type RecoveryVerifyRequest struct { NewPassword string `json:"new_password"` } +// isUniqueConstraintError reports whether a write lost a race against a UNIQUE +// index rather than failing for an operational reason. The driver is matched by +// message because the SQLite driver in use does not export a typed error. +func isUniqueConstraintError(err error) bool { + return err != nil && strings.Contains(strings.ToUpper(err.Error()), "UNIQUE CONSTRAINT FAILED") +} + func randomToken() (string, error) { raw := make([]byte, 32) if _, err := rand.Read(raw); err != nil { @@ -150,6 +157,17 @@ func (h *AuthHandler) Register(w http.ResponseWriter, r *http.Request) { VALUES (?, ?, ?, ?, ?, ?, 0, ?, ?) `, userID, username, normalizedUsername, pwdHash, recoveryHash, role, nowMs, nowMs) if err != nil { + // The availability check above runs outside this transaction, so two + // simultaneous sign-ups for the same name both pass it and the unique + // index decides. That is the same "name is taken" answer as before, not a + // server fault, and telling the loser otherwise sends them to a bug report + // instead of to a different username. + if isUniqueConstraintError(err) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "username is already taken"}) + return + } http.Error(w, `{"error":"failed to create user account"}`, http.StatusInternalServerError) return } @@ -553,6 +571,10 @@ func (h *AuthHandler) VerifyRecoveryCode(w http.ResponseWriter, r *http.Request) var userID, storedRecoveryHash string err := h.DB.SQL.QueryRowContext(r.Context(), "SELECT id, recovery_code_hash FROM users WHERE normalized_username = ?", normalizedUsername).Scan(&userID, &storedRecoveryHash) if err == sql.ErrNoRows { + // Spend the same work as a real verification, exactly as Login does, so + // this endpoint does not become the cheap way to learn which usernames + // exist on an instance. + auth.DummyVerify(req.RecoveryCode) http.Error(w, `{"error":"invalid recovery code or username"}`, http.StatusUnauthorized) return } else if err != nil { diff --git a/services/api/internal/server/handlers/sync.go b/services/api/internal/server/handlers/sync.go index a58286b2..d3cfbc4a 100644 --- a/services/api/internal/server/handlers/sync.go +++ b/services/api/internal/server/handlers/sync.go @@ -757,7 +757,14 @@ func validateSyncOperation(op *SyncPushOperation) error { default: return fmt.Errorf("invalid playback_state event_type") } - encoded, err := json.Marshal(p) + // Re-encoding the parsed struct used to *replace* the payload, which + // silently dropped every key this server does not model — the episode + // title, artwork, podcast id, enclosure and duration that clients + // denormalize into the operation. The sync log stores what Pull hands + // back, so the other device reconstructed a progress row with no title + // and no artwork and overwrote a good local one with it. Only the + // normalized keys are written back; everything else round-trips. + encoded, err := mergeNormalizedPayload(op.Payload, p) if err != nil { return fmt.Errorf("invalid playback_state payload") } @@ -772,7 +779,10 @@ func validateSyncOperation(op *SyncPushOperation) error { } if p.ID == "" { p.ID = op.EntityID - encoded, _ := json.Marshal(p) + encoded, err := mergeNormalizedPayload(op.Payload, p) + if err != nil { + return fmt.Errorf("invalid listening_session payload") + } op.Payload = encoded } if p.ID != op.EntityID { @@ -820,6 +830,30 @@ func validateSyncOperation(op *SyncPushOperation) error { return nil } +// mergeNormalizedPayload writes the server-normalized fields of `normalized` +// back into the client's original JSON object, keeping every key the server +// does not model. Clients denormalize display metadata into sync payloads +// precisely so a peer can render the record without a second fetch; the sync +// log is the only place that metadata lives. +func mergeNormalizedPayload(original json.RawMessage, normalized any) (json.RawMessage, error) { + merged := map[string]json.RawMessage{} + if err := json.Unmarshal(original, &merged); err != nil { + return nil, err + } + overrides := map[string]json.RawMessage{} + encoded, err := json.Marshal(normalized) + if err != nil { + return nil, err + } + if err := json.Unmarshal(encoded, &overrides); err != nil { + return nil, err + } + for key, value := range overrides { + merged[key] = value + } + return json.Marshal(merged) +} + func validateObjectPayload(payload json.RawMessage, timestamp int64) error { if timestamp <= 0 { return fmt.Errorf("timestamp must be positive") diff --git a/services/api/internal/server/handlers/sync_playback_payload_test.go b/services/api/internal/server/handlers/sync_playback_payload_test.go new file mode 100644 index 00000000..c7a3b5bc --- /dev/null +++ b/services/api/internal/server/handlers/sync_playback_payload_test.go @@ -0,0 +1,133 @@ +package handlers + +import ( + "bytes" + "context" + "encoding/json" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/Shik3i/KoalaCast/services/api/internal/db" + customMiddleware "github.com/Shik3i/KoalaCast/services/api/internal/server/middleware" +) + +// TestSync_PlaybackPayloadKeepsDenormalizedMetadata pins the contract both +// clients depend on: a playback_state payload carries the episode's title, +// artwork, podcast and duration so the receiving device can render "continue +// listening" without a second fetch. Normalizing the operation must not throw +// those keys away — the sync log is the only place they exist. +func TestSync_PlaybackPayloadKeepsDenormalizedMetadata(t *testing.T) { + tempDir, err := os.MkdirTemp("", "koala_sync_pb_*") + if err != nil { + t.Fatalf("temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError})) + database, err := db.OpenDB(filepath.Join(tempDir, "test.db"), logger) + if err != nil { + t.Fatalf("OpenDB failed: %v", err) + } + defer database.Close() + + if _, err := database.SQL.Exec(` + INSERT INTO users (id, username, normalized_username, password_hash, recovery_code_hash, role, is_suspended, created_at, updated_at) + VALUES ('u1','User','user','x','y','user',0,0,0); + INSERT INTO podcasts (id, feed_url, title, created_at, updated_at) + VALUES ('pod-1','https://example.com/feed.xml','My Show',0,0); + INSERT INTO episodes (id, podcast_id, stable_identity_key, title, enclosure_url, created_at) + VALUES ('ep-1','pod-1','guid-1','Episode One','https://cdn/x.mp3',0) + `); err != nil { + t.Fatalf("seed: %v", err) + } + + h := &SyncHandler{DB: database} + authCtx := context.WithValue(context.Background(), customMiddleware.UserContextKey, &customMiddleware.AuthUser{ID: "u1", Role: "user"}) + + pushBody := `{ + "client_schema_version": 2, + "operations": [{ + "client_op_id": "p:ep-1:100", + "device_id": "dev-A", + "entity_type": "playback_state", + "action": "upsert", + "entity_id": "ep-1", + "payload": { + "episode_id":"ep-1", + "podcast_id":"pod-1", + "position_ms":42000, + "completed":false, + "progress_percent":12.5, + "last_played_at":100, + "title":"Episode One", + "podcast_title":"My Show", + "artwork_url":"https://cdn/x.jpg", + "enclosure_url":"https://cdn/x.mp3", + "duration_ms":360000, + "categories":["News"], + "event_type":"PROGRESS_TICK", + "playback_session_id":"sess-1", + "device_id":"dev-A", + "per_session_seq":3, + "client_timestamp":100 + }, + "client_timestamp": 100 + }] + }` + reqPush := httptest.NewRequest(http.MethodPost, "/api/v1/sync", bytes.NewBufferString(pushBody)).WithContext(authCtx) + recPush := httptest.NewRecorder() + h.Push(recPush, reqPush) + if recPush.Code != http.StatusOK { + t.Fatalf("push: expected 200, got %d: %s", recPush.Code, recPush.Body.String()) + } + + reqPull := httptest.NewRequest(http.MethodGet, "/api/v1/sync?since_cursor=0", nil).WithContext(authCtx) + recPull := httptest.NewRecorder() + h.Pull(recPull, reqPull) + if recPull.Code != http.StatusOK { + t.Fatalf("pull: expected 200, got %d", recPull.Code) + } + + var pull struct { + Changesets []struct { + EntityType string `json:"entity_type"` + Payload json.RawMessage `json:"payload"` + } `json:"changesets"` + } + if err := json.NewDecoder(recPull.Body).Decode(&pull); err != nil { + t.Fatalf("decode pull: %v", err) + } + if len(pull.Changesets) != 1 || pull.Changesets[0].EntityType != "playback_state" { + t.Fatalf("expected one playback changeset, got %+v", pull.Changesets) + } + + var payload struct { + PodcastID string `json:"podcast_id"` + Title string `json:"title"` + PodcastTitle string `json:"podcast_title"` + ArtworkURL string `json:"artwork_url"` + EnclosureURL string `json:"enclosure_url"` + DurationMS int64 `json:"duration_ms"` + LastPlayedAt int64 `json:"last_played_at"` + Categories []string `json:"categories"` + PositionMS int64 `json:"position_ms"` + EventType string `json:"event_type"` + PerSessionSeq int64 `json:"per_session_seq"` + } + if err := json.Unmarshal(pull.Changesets[0].Payload, &payload); err != nil { + t.Fatalf("payload not valid JSON: %v (raw=%s)", err, pull.Changesets[0].Payload) + } + if payload.PodcastID != "pod-1" || payload.Title != "Episode One" || + payload.PodcastTitle != "My Show" || payload.ArtworkURL != "https://cdn/x.jpg" || + payload.EnclosureURL != "https://cdn/x.mp3" || payload.DurationMS != 360000 || + payload.LastPlayedAt != 100 || len(payload.Categories) != 1 { + t.Fatalf("denormalized metadata was dropped: %+v", payload) + } + if payload.PositionMS != 42000 || payload.EventType != "PROGRESS_TICK" || payload.PerSessionSeq != 3 { + t.Fatalf("normalized fields did not survive: %+v", payload) + } +} diff --git a/services/api/internal/worker/worker.go b/services/api/internal/worker/worker.go index d8f42f2e..5acdb65e 100644 --- a/services/api/internal/worker/worker.go +++ b/services/api/internal/worker/worker.go @@ -139,6 +139,12 @@ func (w *FeedWorker) runLoop(ctx context.Context) { } } +// refreshIntervalMS is how long a healthy feed waits before the next scheduled +// fetch. Errors use their own exponential backoff in updateFeedError. +func (w *FeedWorker) refreshIntervalMS() int64 { + return int64(config.EffectiveFeedRefreshIntervalMS(w.cfg.FeedRefreshIntervalMS)) +} + func (w *FeedWorker) compactSyncLog(ctx context.Context) { if n, err := w.db.CompactSyncLog(ctx); err != nil { w.logger.Warn("sync_log compaction failed", "error", err) @@ -207,6 +213,11 @@ func (w *FeedWorker) RefreshScheduledFeeds(ctx context.Context) { toFetch = append(toFetch, item) } } + if err := rows.Err(); err != nil { + // A truncated batch is not an empty one. Saying so keeps a partial refresh + // from reading as "nothing was due". + w.logger.Error("failed to read feeds for refresh", "error", err) + } if len(toFetch) == 0 { return @@ -276,7 +287,7 @@ func (w *FeedWorker) RefreshSingleFeed(ctx context.Context, podcastID, feedURL, defer resp.Body.Close() if resp.StatusCode == http.StatusNotModified { - // 304 Not Modified -> Schedule next fetch with exponential backoff + // 304 Not Modified -> nothing changed; wait out the ordinary interval. if _, err := w.db.SQL.ExecContext(ctx, ` UPDATE podcasts SET last_fetch_attempt_at = ?, @@ -284,7 +295,7 @@ func (w *FeedWorker) RefreshSingleFeed(ctx context.Context, podcastID, feedURL, consecutive_error_count = 0, last_error_category = '' WHERE id = ? - `, nowMs, nowMs+86400000, podcastID); err != nil { + `, nowMs, nowMs+w.refreshIntervalMS(), podcastID); err != nil { return fmt.Errorf("record not-modified feed refresh: %w", err) } return nil @@ -346,7 +357,7 @@ func (w *FeedWorker) RefreshSingleFeed(ctx context.Context, podcastID, feedURL, WHERE id = ? `, parsedFeed.Title, parsedFeed.Description, parsedFeed.Author, parsedFeed.ArtworkURL, parsedFeed.Link, parsedFeed.Language, podcastExplicitInt, parsedFeed.Copyright, - nowMs, nowMs, nowMs+86400000, newETag, newLastModified, nowMs, podcastID) + nowMs, nowMs, nowMs+w.refreshIntervalMS(), newETag, newLastModified, nowMs, podcastID) if err != nil { return fmt.Errorf("failed to update podcast: %w", err) } From 998a4ca8fc3ef7bf0abd6b7a98826249766769f3 Mon Sep 17 00:00:00 2001 From: Timo <6156589+Shik3i@users.noreply.github.com> Date: Sat, 15 Aug 2026 12:31:56 +0200 Subject: [PATCH 2/2] Stop the image proxy asking CDNs for a format it cannot decode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proxy sent a browser's Accept header, AVIF first. Every CDN that negotiates on it — imgix, Cloudinary, Cloudflare Images, anything serving `auto=format` — honoured that and returned AVIF. This build registers JPEG, PNG, GIF and WebP decoders and no AVIF one, so the decode failed and the handler answered with its own placeholder at 200, complete with a Cache-Control header. Artwork that was never broken rendered as a grey rectangle, and the 200 made it invisible in the access log. Accept now lists only what the registered decoders can read. Confirmed against a real imgix cover: `image/avif` before, `image/jpeg` and no X-KoalaCast-Image-Fallback after, and the cover renders on device. Co-Authored-By: Claude Opus 5 --- .../api/internal/server/handlers/proxy.go | 15 +++++- .../internal/server/handlers/proxy_test.go | 47 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/services/api/internal/server/handlers/proxy.go b/services/api/internal/server/handlers/proxy.go index d51d34ae..fed3359a 100644 --- a/services/api/internal/server/handlers/proxy.go +++ b/services/api/internal/server/handlers/proxy.go @@ -135,6 +135,12 @@ const maxDecodedPixels = 40 * 1000 * 1000 // artwork arrived, forcing users to reload until a request happened to be fast. // Singleflight still coalesces identical requests while this bounded deadline // gives DNS, TLS and the first upstream response a realistic window. +// acceptedImageFormats mirrors the decoders registered by this package's +// imports (JPEG, PNG, GIF, WebP). Content negotiation is a promise about what +// the client can read; advertising a format with no decoder turns every +// negotiating CDN into a broken image. +const acceptedImageFormats = "image/webp,image/jpeg,image/png,image/gif;q=0.8,*/*;q=0.5" + const imageProxyTimeout = 8 * time.Second const maxAudioDownloadBytes = int64(2 * 1024 * 1024 * 1024) const maxAudioStreamDuration = 4 * time.Hour @@ -430,7 +436,14 @@ func (h *ProxyHandler) GetImageProxy(w http.ResponseWriter, r *http.Request) { return nil, fmt.Errorf("invalid url") } req.Header.Set("User-Agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36") - req.Header.Set("Accept", "image/avif,image/webp,image/apng,image/svg+xml,image/*,*/*;q=0.8") + // Ask only for what this process can actually decode. Copying a browser's + // Accept header meant advertising AVIF, and every CDN that negotiates on + // it — imgix, Cloudinary, Cloudflare Images, anything with `auto=format` — + // duly returned AVIF. Go has no AVIF decoder here, so the decode below + // failed and the handler answered with its own placeholder, at 200, for + // artwork that was never broken. SVG is excluded for the same reason it + // always was: it is not a raster format this resizer can handle. + req.Header.Set("Accept", acceptedImageFormats) resp, err := h.httpClient.Do(req) if err != nil { diff --git a/services/api/internal/server/handlers/proxy_test.go b/services/api/internal/server/handlers/proxy_test.go index df1e88e0..0406893b 100644 --- a/services/api/internal/server/handlers/proxy_test.go +++ b/services/api/internal/server/handlers/proxy_test.go @@ -368,3 +368,50 @@ func TestGetAudioResolveRejectsNonHTTPURL(t *testing.T) { t.Fatalf("expected 400 for a non-http scheme, got %d", rec.Code) } } + +// A CDN that negotiates on Accept gives back exactly what was asked for. The +// proxy used to send a browser's header, AVIF first, and then failed to decode +// the AVIF it had requested — answering with its own placeholder, at 200, for +// artwork that was perfectly fine. Every imgix/Cloudinary `auto=format` cover in +// the app was a grey rectangle because of it. +func TestProxyHandler_ImageAcceptOffersOnlyDecodableFormats(t *testing.T) { + for _, unsupported := range []string{"image/avif", "image/svg+xml", "image/heic", "image/jxl"} { + if strings.Contains(acceptedImageFormats, unsupported) { + t.Errorf("Accept advertises %s, which this build cannot decode: %q", unsupported, acceptedImageFormats) + } + } + for _, supported := range []string{"image/webp", "image/jpeg", "image/png"} { + if !strings.Contains(acceptedImageFormats, supported) { + t.Errorf("Accept omits %s, which this build can decode: %q", supported, acceptedImageFormats) + } + } +} + +// The header has to survive the trip, not just be well-formed: an upstream that +// varies on Accept must see it. +func TestProxyHandler_ImageRequestSendsNegotiatedAccept(t *testing.T) { + img := image.NewRGBA(image.Rect(0, 0, 10, 10)) + var seenAccept string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seenAccept = r.Header.Get("Accept") + w.Header().Set("Content-Type", "image/jpeg") + w.WriteHeader(http.StatusOK) + _ = jpeg.Encode(w, img, nil) + })) + defer ts.Close() + + proxy := NewProxyHandler(false) + proxy.httpClient = rss.NewSafeHTTPClient(rss.SafeTransportConfig{AllowLoopback: true}) + rec := httptest.NewRecorder() + proxy.GetImageProxy(rec, httptest.NewRequest(http.MethodGet, "/api/v1/proxy/image?url="+ts.URL+"&w=5", nil)) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", rec.Code) + } + if rec.Header().Get("X-KoalaCast-Image-Fallback") == "true" { + t.Fatal("a decodable upstream image was replaced by the placeholder") + } + if seenAccept != acceptedImageFormats { + t.Fatalf("upstream saw Accept %q, want %q", seenAccept, acceptedImageFormats) + } +}