gate scrobbling behind a setting and funnel server writes through a sink

Scrobbling can be turned off in Settings (on by default). Now-playing, scrobbles and stars all go through a ServerActionSink so a persistent offline outbox can replace the network implementation later without touching the callers.
2026-11-10 15:11:58 +01:00 · 2026-10-04 00:30:04 +00:00
parent 29e5029b56
commit 0379102e24
13 changed files with 202 additions and 65 deletions
@@ -105,6 +105,14 @@ class UserPreferences(

    private val syncConcurrencyKey = intPreferencesKey("sync_concurrency")

+    fun scrobbling(): Flow<Boolean> = store.data.map { prefs -> prefs[scrobblingKey] ?: true }
+
+    suspend fun setScrobbling(enabled: Boolean) {
+        store.edit { prefs -> prefs[scrobblingKey] = enabled }
+    }
+
+    private val scrobblingKey = booleanPreferencesKey("scrobbling")
+
    suspend fun lastSeed(): ArtworkSeedValue? {
        val prefs = store.data.first()
        val cacheKey = prefs[lastSeedKey] ?: return null
@@ -15,6 +15,7 @@ import com.subtracks.data.prefs.AlbumSort
 import com.subtracks.data.prefs.ArtistSort
 import com.subtracks.data.prefs.PlaylistSort
 import com.subtracks.data.prefs.StarredFilter
+import com.subtracks.data.source.ServerActionSink
 import com.subtracks.data.source.StarType
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineScope
@@ -31,6 +32,7 @@ import kotlinx.coroutines.withContext
 class LibraryRepository(
    private val db: SubtracksDatabase,
    private val sourceRepository: SourceRepository,
+    private val serverActions: ServerActionSink,
    private val showMessage: (String) -> Unit = {},
 ) {
    val activeSourceId: Flow<Long?> = sourceRepository.activeSourceId()
@@ -239,14 +241,13 @@ class LibraryRepository(
        starred: Boolean,
    ): Result<Unit> {
        val sourceId = sourceRepository.activeSourceIdOnce()
-        val source = sourceRepository.activeMusicSource()
-        if (sourceId == null || source == null) {
+        if (sourceId == null) {
            return Result.failure(IllegalStateException("No active server"))
        }
        val previous = starredValue(sourceId, type, id)
        updateStarred(sourceId, type, id, if (starred) System.currentTimeMillis() else null)
        return try {
-            source.setStar(type, id, starred)
+            serverActions.setStar(type, id, starred)
            Result.success(Unit)
        } catch (cancellation: CancellationException) {
            throw cancellation
@@ -0,0 +1,28 @@
+package com.subtracks.data.repo
+
+import com.subtracks.data.source.ServerActionSink
+import com.subtracks.data.source.StarType
+
+class NetworkServerActionSink(
+    private val sources: SourceRepository,
+) : ServerActionSink {
+    override suspend fun nowPlaying(songId: String) {
+        sources.activeMusicSource()?.scrobble(songId, submission = false)
+    }
+
+    override suspend fun scrobble(
+        songId: String,
+        time: Long,
+    ) {
+        sources.activeMusicSource()?.scrobble(songId, submission = true, time = time)
+    }
+
+    override suspend fun setStar(
+        type: StarType,
+        id: String,
+        starred: Boolean,
+    ) {
+        val source = sources.activeMusicSource() ?: throw IllegalStateException("No active server")
+        source.setStar(type, id, starred)
+    }
+}
@@ -0,0 +1,21 @@
+package com.subtracks.data.source
+
+/**
+ * Writes the app pushes back to the server. Today they go straight over the
+ * network, but funneling them through one seam leaves room to persist them and
+ * replay later while the server is unreachable.
+ */
+interface ServerActionSink {
+    suspend fun nowPlaying(songId: String)
+
+    suspend fun scrobble(
+        songId: String,
+        time: Long,
+    )
+
+    suspend fun setStar(
+        type: StarType,
+        id: String,
+        starred: Boolean,
+    )
+}
@@ -13,13 +13,16 @@ import com.subtracks.data.download.DownloadNotifier
 import com.subtracks.data.download.OkHttpArtworkFetcher
 import com.subtracks.data.download.SystemDownloadEngine
 import com.subtracks.data.net.networkMode
+import com.subtracks.data.prefs.UserPreferences
 import com.subtracks.data.prefs.createUserPreferences
 import com.subtracks.data.repo.ArtworkSeedRepository
 import com.subtracks.data.repo.ArtworkSeedStore
 import com.subtracks.data.repo.DownloadRepository
 import com.subtracks.data.repo.LibraryRepository
+import com.subtracks.data.repo.NetworkServerActionSink
 import com.subtracks.data.repo.QueueRepository
 import com.subtracks.data.repo.SourceRepository
+import com.subtracks.data.source.ServerActionSink
 import com.subtracks.data.sync.SyncManager
 import com.subtracks.playback.MediaSessionConnection
 import com.subtracks.playback.PlaybackController
@@ -51,7 +54,8 @@ fun appModule(
    single { http }
    single { createUserPreferences(context) }
    single { SourceRepository(get(), get(), get(), get(), networkMode = networkMode(context.applicationContext), showMessage = toast) }
-    single { LibraryRepository(get(), get(), toast) }
+    single<ServerActionSink> { NetworkServerActionSink(get()) }
+    single { LibraryRepository(get(), get(), get(), toast) }
    single<ArtworkSeedStore> { ArtworkSeedRepository(get()) }
    single { SyncManager(get(), get(), get()) }
    single { QueueRepository(get()) }
@@ -74,7 +78,7 @@ fun appModule(
    single { PlaybackController(get(), get(), get(), get(), showMessage = toast) }
    single(createdAtStart = true) {
        val playback = get<PlaybackController>()
-        Scrobbler(source = { get<SourceRepository>().activeMusicSource() }).also {
+        Scrobbler(sink = get(), enabled = get<UserPreferences>().scrobbling()).also {
            it.attach(playback.state, playback.positionMs)
        }
    }
@@ -1,11 +1,12 @@
 package com.subtracks.playback

-import com.subtracks.data.source.MusicSource
+import com.subtracks.data.source.ServerActionSink
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
+import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.launch
@@ -34,6 +35,14 @@ class ScrobblePolicy(
    private var nowPlayingSent = false
    private var submitted = false

+    fun reset() {
+        songId = null
+        playedMs = 0
+        lastPositionMs = 0
+        nowPlayingSent = false
+        submitted = false
+    }
+
    fun advance(
        item: QueueItem?,
        isPlaying: Boolean,
@@ -80,7 +89,8 @@ class ScrobblePolicy(
 }

 class Scrobbler(
-    private val source: suspend () -> MusicSource?,
+    private val sink: ServerActionSink,
+    private val enabled: Flow<Boolean>,
    private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default),
    private val policy: ScrobblePolicy = ScrobblePolicy(),
 ) {
@@ -89,28 +99,25 @@ class Scrobbler(
        positionMs: StateFlow<Long>,
    ) {
        scope.launch {
-            combine(state, positionMs) { current, position -> current to position }.collect { (current, position) ->
-                val event = policy.advance(current.item, current.isPlaying, position) ?: return@collect
-                submit(event)
-            }
+            combine(state, positionMs, enabled) { current, position, on -> Triple(current, position, on) }
+                .collect { (current, position, on) ->
+                    if (!on) {
+                        policy.reset()
+                        return@collect
+                    }
+                    val event = policy.advance(current.item, current.isPlaying, position) ?: return@collect
+                    submit(event)
+                }
        }
    }

    fun close() = scope.cancel()

    private suspend fun submit(event: Scrobble) {
-        val source =
-            try {
-                source()
-            } catch (cancellation: CancellationException) {
-                throw cancellation
-            } catch (_: Exception) {
-                null
-            } ?: return
        try {
            when (event) {
-                is Scrobble.NowPlaying -> source.scrobble(event.songId, submission = false)
-                is Scrobble.Submission -> source.scrobble(event.songId, submission = true, time = event.time)
+                is Scrobble.NowPlaying -> sink.nowPlaying(event.songId)
+                is Scrobble.Submission -> sink.scrobble(event.songId, event.time)
            }
        } catch (cancellation: CancellationException) {
            throw cancellation
@@ -73,6 +73,7 @@ fun SettingsRoute(
    val syncConcurrency by viewModel.syncConcurrency.collectAsStateWithLifecycle()
    val downloadQuality by viewModel.downloadQuality.collectAsStateWithLifecycle()
    val downloadOverMetered by viewModel.downloadOverMetered.collectAsStateWithLifecycle()
+    val scrobbling by viewModel.scrobbling.collectAsStateWithLifecycle()
    SettingsScreen(
        sources = sources,
        activeSourceId = activeSourceId,
@@ -81,6 +82,7 @@ fun SettingsRoute(
        syncConcurrency = syncConcurrency,
        downloadQuality = downloadQuality,
        downloadOverMetered = downloadOverMetered,
+        scrobbling = scrobbling,
        onSelectSource = viewModel::selectSource,
        onEditServer = onEditServer,
        onWifiQualityChange = viewModel::setWifiQuality,
@@ -88,6 +90,7 @@ fun SettingsRoute(
        onSyncConcurrencyChange = viewModel::setSyncConcurrency,
        onDownloadQualityChange = viewModel::setDownloadQuality,
        onDownloadOverMeteredChange = viewModel::setDownloadOverMetered,
+        onScrobblingChange = viewModel::setScrobbling,
        onAddServer = onAddServer,
        onOpenDownloads = onOpenDownloads,
        onBack = onBack,
@@ -104,6 +107,7 @@ fun SettingsScreen(
    syncConcurrency: Int,
    downloadQuality: StreamQuality,
    downloadOverMetered: Boolean,
+    scrobbling: Boolean,
    onSelectSource: (Long) -> Unit,
    onEditServer: (Long) -> Unit,
    onWifiQualityChange: (StreamQuality) -> Unit,
@@ -111,6 +115,7 @@ fun SettingsScreen(
    onSyncConcurrencyChange: (Int) -> Unit,
    onDownloadQualityChange: (StreamQuality) -> Unit,
    onDownloadOverMeteredChange: (Boolean) -> Unit,
+    onScrobblingChange: (Boolean) -> Unit,
    onAddServer: () -> Unit,
    onOpenDownloads: () -> Unit,
    onBack: () -> Unit,
@@ -197,6 +202,27 @@ fun SettingsScreen(
                    colors = ListItemDefaults.colors(containerColor = Color.Transparent),
                )
            }
+            item { SectionHeader("Playback") }
+            item {
+                ListItem(
+                    headlineContent = { Text("Scrobble to server") },
+                    supportingContent = { Text("Send now playing and play counts") },
+                    trailingContent = {
+                        Switch(
+                            checked = scrobbling,
+                            onCheckedChange = onScrobblingChange,
+                            colors =
+                                SwitchDefaults.colors(
+                                    uncheckedThumbColor = MaterialTheme.colorScheme.onSurfaceVariant,
+                                    uncheckedTrackColor = MaterialTheme.colorScheme.surfaceVariant,
+                                    uncheckedBorderColor = MaterialTheme.colorScheme.outline,
+                                ),
+                        )
+                    },
+                    modifier = Modifier.clickable { onScrobblingChange(!scrobbling) },
+                    colors = ListItemDefaults.colors(containerColor = Color.Transparent),
+                )
+            }
            item { SectionHeader("Downloads") }
            item {
                ListItem(
@@ -48,6 +48,11 @@ class SettingsViewModel(
            .syncConcurrency()
            .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), DEFAULT_FETCH_CONCURRENCY)

+    val scrobbling: StateFlow<Boolean> =
+        userPreferences
+            .scrobbling()
+            .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), true)
+
    fun selectSource(id: Long) {
        viewModelScope.launch { sourceRepository.selectSource(id) }
    }
@@ -71,4 +76,8 @@ class SettingsViewModel(
    fun setSyncConcurrency(value: Int) {
        viewModelScope.launch { userPreferences.setSyncConcurrency(value) }
    }
+
+    fun setScrobbling(enabled: Boolean) {
+        viewModelScope.launch { userPreferences.setScrobbling(enabled) }
+    }
 }
@@ -99,6 +99,21 @@ class UserPreferencesTest {
            file.delete()
        }

+    @Test
+    fun scrobblingDefaultsOnAndRoundTrips() =
+        runTest {
+            val file = File.createTempFile("user-prefs", ".preferences_pb").apply { delete() }
+            val store = PreferenceDataStoreFactory.create(scope = backgroundScope) { file }
+            val prefs = UserPreferences(store)
+
+            assertEquals(true, prefs.scrobbling().first())
+
+            prefs.setScrobbling(false)
+            assertEquals(false, prefs.scrobbling().first())
+
+            file.delete()
+        }
+
    @Test
    fun lastSeedRoundTripsWithAndWithoutASecondary() =
        runTest {
@@ -45,7 +45,7 @@ class LibraryRepositoryTest {
                .setDriver(BundledSQLiteDriver())
                .build()
        sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art")))
-        repository = LibraryRepository(db, sourceRepository, showMessage = { messages += it })
+        repository = LibraryRepository(db, sourceRepository, NetworkServerActionSink(sourceRepository), showMessage = { messages += it })
    }

    @After
@@ -1,18 +1,13 @@
 package com.subtracks.playback

-import com.subtracks.data.model.Album
-import com.subtracks.data.model.Artist
-import com.subtracks.data.model.Playlist
-import com.subtracks.data.model.PlaylistSong
-import com.subtracks.data.model.Song
-import com.subtracks.data.source.MusicSource
+import com.subtracks.data.source.ServerActionSink
 import com.subtracks.data.source.StarType
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.SupervisorJob
-import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.emptyFlow
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
@@ -20,56 +15,52 @@ import org.junit.Test

@OptIn(ExperimentalCoroutinesApi::class)
 class ScrobblerTest {
-    private data class Call(
-        val songId: String,
-        val submission: Boolean,
-        val time: Long?,
-    )
+    private sealed interface Call {
+        data class NowPlaying(
+            val songId: String,
+        ) : Call

-    private class RecordingSource : MusicSource {
-        override val id: Long = 1
+        data class Scrobble(
+            val songId: String,
+            val time: Long,
+        ) : Call
+    }
+
+    private class RecordingSink : ServerActionSink {
        val calls = mutableListOf<Call>()

-        override suspend fun ping() = Unit
+        override suspend fun nowPlaying(songId: String) {
+            calls += Call.NowPlaying(songId)
+        }
+
+        override suspend fun scrobble(
+            songId: String,
+            time: Long,
+        ) {
+            calls += Call.Scrobble(songId, time)
+        }

        override suspend fun setStar(
            type: StarType,
            id: String,
            starred: Boolean,
        ) = Unit
-
-        override suspend fun scrobble(
-            songId: String,
-            submission: Boolean,
-            time: Long?,
-        ) {
-            calls += Call(songId, submission, time)
-        }
-
-        override fun artists(): Flow<List<Artist>> = emptyFlow()
-
-        override fun albums(): Flow<List<Album>> = emptyFlow()
-
-        override fun songs(): Flow<List<Song>> = emptyFlow()
-
-        override fun playlists(): Flow<List<Playlist>> = emptyFlow()
-
-        override fun playlistSongs(playlistIds: List<String>): Flow<List<PlaylistSong>> = emptyFlow()
    }

+    private fun TestScope.testScope() = CoroutineScope(UnconfinedTestDispatcher(testScheduler) + SupervisorJob())
+
    @Test
    fun sendsNowPlayingThenScrobbleOnce() =
        runTest {
-            val source = RecordingSource()
-            val scope = CoroutineScope(UnconfinedTestDispatcher(testScheduler) + SupervisorJob())
-            val scrobbler = Scrobbler(source = { source }, scope = scope, policy = ScrobblePolicy(now = { 1_000L }))
+            val sink = RecordingSink()
+            val scrobbler = Scrobbler(sink = sink, enabled = flowOf(true), scope = testScope(), policy = ScrobblePolicy(now = { 1_000L }))
            val state = MutableStateFlow(PlaybackState())
            val position = MutableStateFlow(0L)
            scrobbler.attach(state, position)
            val item = QueueItem("s1", "Title", "Artist", "Album", null, durationMs = 30_000)

            state.value = PlaybackState(item = item, isPlaying = true)
-            assertEquals(listOf(Call("s1", submission = false, time = null)), source.calls)
+            assertEquals(listOf(Call.NowPlaying("s1")), sink.calls)

            var tick = 500L
            while (tick <= 15_000L) {
@@ -79,13 +70,36 @@ class ScrobblerTest {

            assertEquals(
                listOf(
-                    Call("s1", submission = false, time = null),
-                    Call("s1", submission = true, time = 1_000L),
+                    Call.NowPlaying("s1"),
+                    Call.Scrobble("s1", 1_000L),
                ),
-                source.calls,
+                sink.calls,
            )

            position.value = 25_000L
-            assertEquals(2, source.calls.size)
+            assertEquals(2, sink.calls.size)
+        }
+
+    @Test
+    fun disabledSendsNothing() =
+        runTest {
+            val sink = RecordingSink()
+            val enabled = MutableStateFlow(false)
+            val scrobbler = Scrobbler(sink = sink, enabled = enabled, scope = testScope(), policy = ScrobblePolicy(now = { 1_000L }))
+            val state = MutableStateFlow(PlaybackState())
+            val position = MutableStateFlow(0L)
+            scrobbler.attach(state, position)
+            val item = QueueItem("s1", "Title", "Artist", "Album", null, durationMs = 30_000)
+
+            state.value = PlaybackState(item = item, isPlaying = true)
+            var tick = 500L
+            while (tick <= 20_000L) {
+                position.value = tick
+                tick += 500
+            }
+            assertEquals(emptyList<Call>(), sink.calls)
+
+            enabled.value = true
+            assertEquals(listOf(Call.NowPlaying("s1")), sink.calls)
        }
 }
@@ -345,6 +345,7 @@ class AppScreenshotTest {
                    syncConcurrency = 4,
                    downloadQuality = StreamQuality(128, "opus"),
                    downloadOverMetered = false,
+                    scrobbling = true,
                    onSelectSource = {},
                    onEditServer = {},
                    onWifiQualityChange = {},
@@ -352,6 +353,7 @@ class AppScreenshotTest {
                    onSyncConcurrencyChange = {},
                    onDownloadQualityChange = {},
                    onDownloadOverMeteredChange = {},
+                    onScrobblingChange = {},
                    onAddServer = {},
                    onOpenDownloads = {},
                    onBack = {},
@@ -92,7 +92,9 @@ Stream URLs come from `SourceRepository.streamUri`, which asks the active `Subso

 The window is also the queue the media notification and Android Auto can browse — a deliberate trade-off, since the alternative is holding an entire large library in memory. The queue view resolves positions lazily too: it loads the queue around the cursor in chunks and extends by a chunk when you scroll near either end, so it never materializes the whole queue. Dragging uses `sh.calvin.reorderable`, which floats the lifted row and reorders the loaded list live (animating with `animateItem`), so editing is what a drag needs: the view reorders its loaded list while dragging and commits one move on drop. `queue_entries` is rewritten to edit: removing a position splits its entry into up to two ranges, and moving one turns the moved track into an explicit song entry inserted at the target index, so an edit costs a couple of rows rather than a copy of the queue. When the edit falls inside the player's window the controller mirrors it with `removeMediaItem`/`moveMediaItem` so playback is not interrupted; removing the playing track, or a move that crosses the window boundary, reloads the window around the cursor. The controller keeps the pre-edit entries and cursor, so the queue view can offer a one-step undo. The up-next block is fully editable in the same view (drag reorder and delete), while the context is read-only while shuffled. Timed lyrics and gapless format preferences are not wired yet.

-Scrobbling is a `Scrobbler` that collects the controller's `state` and `positionMs` and drives the Subsonic `scrobble` endpoint: a now-playing notification (`submission=false`) when a track starts playing, and one submission (`submission=true`, with the playback start time) when the track passes half its duration or four minutes, whichever comes first. Tracks under 30 seconds are never submitted, and position jumps are treated as seeks rather than listened time. The policy is a pure state machine with no Android dependencies, so the threshold logic is unit-tested directly.
+Scrobbling is a `Scrobbler` that collects the controller's `state` and `positionMs` and drives the Subsonic `scrobble` endpoint: a now-playing notification (`submission=false`) when a track starts playing, and one submission (`submission=true`, with the playback start time) when the track passes half its duration or four minutes, whichever comes first. That split is the endpoint's own `submission` flag, and the threshold is the Last.fm rule the server forwards to; servers such as Navidrome trust the client and apply no threshold themselves. Tracks under 30 seconds are never submitted, position jumps are treated as seeks rather than listened time, and the policy is a pure state machine with no Android dependencies. A Settings toggle (`UserPreferences.scrobbling`, on by default) gates both calls.
+
+Server writes — now-playing, scrobbles and stars — go through a `ServerActionSink`. The only implementation today is `NetworkServerActionSink`, which resolves the active source and calls it; a persistent, ordered outbox can replace it later so plays and stars taken offline are queued and replayed when the server is reachable. Now-playing is deliberately not durable (Last.fm says failed now-playing requests must not be retried), while submissions and stars carry everything a queue would need (the song or entity id and, for a play, the time it was listened).

 The media notification carries a star button for the playing track. `PlaybackService` adds a `com.subtracks.STAR` session command and a `CommandButton` to the `MediaSession`, and drives its filled/unfilled icon from the playing song's Room row. That path — `MediaSession.Callback`, `SessionCommand`, `CommandButton`, `SessionResult`/`SessionError` and `MediaSession.setMediaButtonPreferences` — is annotated `@UnstableApi` in Media3 1.11.1 and is therefore opted into explicitly: when bumping Media3, re-check those signatures and the notification's button resolution. The stable `setCustomLayout` is not a substitute here, because the notification manager only refreshes on `onMediaButtonPreferencesChanged`, so the icon would not update mid-playback.