From e96ef7603b5dff436c0172cde47e2ec683d21aa1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Nov 2025 11:32:25 +0000 Subject: [PATCH 1/2] Fix playlist import race condition on large libraries (#127) This fixes a race condition where playlist imports would fail because the song database query returned stale cached data from a StateFlow before it had time to update after song insertion. The issue was particularly noticeable with large libraries (7000+ songs) where the StateFlow emission delay was more pronounced. Changes: - Add getSongsDirect() method to SongRepository interface that bypasses the StateFlow cache and queries the database directly - Implement getSongsDirect() in LocalSongRepository to query fresh data from the Room DAO - Update importPlaylists() to use getSongsDirect() instead of getSongs() to ensure fresh song data is available when matching playlist songs This ensures playlist imports can reliably match songs regardless of library size or timing of StateFlow updates. --- .../mediaprovider/MediaImporter.kt | 7 +++---- .../repository/songs/SongRepository.kt | 2 ++ .../local/repository/LocalSongRepository.kt | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt index 13338cf06..8454277b9 100644 --- a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt +++ b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt @@ -257,16 +257,15 @@ class MediaImporter( .firstOrNull() .orEmpty() + // Use getSongsDirect() to bypass StateFlow cache and ensure fresh data from database + // This fixes a race condition where the StateFlow may not have updated yet after song import val existingSongs = - songRepository.getSongs( + songRepository.getSongsDirect( SongQuery.All( includeExcluded = true, providerType = mediaProvider.type ) ) - .filterNotNull() - .firstOrNull() - .orEmpty() mediaProvider.findPlaylists(existingPlaylists, existingSongs).collect { event -> when (event) { diff --git a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt index 2a6e83129..901c849a8 100644 --- a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt +++ b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt @@ -8,6 +8,8 @@ import kotlinx.coroutines.flow.Flow interface SongRepository { fun getSongs(query: SongQuery): Flow?> + suspend fun getSongsDirect(query: SongQuery): List + suspend fun insert( songs: List, mediaProviderType: MediaProviderType diff --git a/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt b/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt index 7cd318d29..020ccdccb 100644 --- a/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt +++ b/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt @@ -1,6 +1,7 @@ package com.simplecityapps.localmediaprovider.local.repository import com.simplecityapps.localmediaprovider.local.data.room.dao.SongDataDao +import com.simplecityapps.localmediaprovider.local.data.room.dao.toSong import com.simplecityapps.localmediaprovider.local.data.room.entity.toSongData import com.simplecityapps.localmediaprovider.local.data.room.entity.toSongDataUpdate import com.simplecityapps.mediaprovider.repository.songs.SongRepository @@ -46,6 +47,24 @@ class LocalSongRepository( ?.sortedWith(query.sortOrder.comparator) } + override suspend fun getSongsDirect(query: SongQuery): List { + val songs = songDataDao.get().map { it.toSong() } + + var result = songs + + if (!query.includeExcluded) { + result = songs.filterNot { it.blacklisted } + } + + query.providerType?.let { providerType -> + result = songs.filter { song -> song.mediaProvider == providerType } + } + + return result + .filter(query.predicate) + .sortedWith(query.sortOrder.comparator) + } + override suspend fun insert( songs: List, mediaProviderType: MediaProviderType From 4cbe034f3982adbb2df9f247f6e2b967222aed54 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Nov 2025 05:44:41 +0000 Subject: [PATCH 2/2] Refactor: Replace getSongsDirect() with proper cache synchronization This improves upon the previous fix by removing the code smell of exposing getSongsDirect() in the repository interface. Instead of adding a parallel query method, we now ensure that insertUpdateAndDelete() properly synchronizes the StateFlow cache before returning. This maintains the clean reactive architecture while guaranteeing that subsequent reads will see the updated data. Changes: - Remove getSongsDirect() from SongRepository interface - Add cache synchronization to insertUpdateAndDelete() in LocalSongRepository by waiting for StateFlow to emit updated data - Revert importPlaylists() to use standard getSongs() flow This approach: - Maintains single responsibility and clean interfaces - Ensures write operations are truly complete before returning - Preserves the reactive Flow-based architecture - Follows the principle of least surprise --- .../mediaprovider/MediaImporter.kt | 7 +++-- .../repository/songs/SongRepository.kt | 2 -- .../local/repository/LocalSongRepository.kt | 30 +++++++------------ 3 files changed, 14 insertions(+), 25 deletions(-) diff --git a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt index 8454277b9..13338cf06 100644 --- a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt +++ b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/MediaImporter.kt @@ -257,15 +257,16 @@ class MediaImporter( .firstOrNull() .orEmpty() - // Use getSongsDirect() to bypass StateFlow cache and ensure fresh data from database - // This fixes a race condition where the StateFlow may not have updated yet after song import val existingSongs = - songRepository.getSongsDirect( + songRepository.getSongs( SongQuery.All( includeExcluded = true, providerType = mediaProvider.type ) ) + .filterNotNull() + .firstOrNull() + .orEmpty() mediaProvider.findPlaylists(existingPlaylists, existingSongs).collect { event -> when (event) { diff --git a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt index 901c849a8..2a6e83129 100644 --- a/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt +++ b/android/mediaprovider/core/src/main/java/com/simplecityapps/mediaprovider/repository/songs/SongRepository.kt @@ -8,8 +8,6 @@ import kotlinx.coroutines.flow.Flow interface SongRepository { fun getSongs(query: SongQuery): Flow?> - suspend fun getSongsDirect(query: SongQuery): List - suspend fun insert( songs: List, mediaProviderType: MediaProviderType diff --git a/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt b/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt index 020ccdccb..48305af80 100644 --- a/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt +++ b/android/mediaprovider/local/src/main/java/com/simplecityapps/localmediaprovider/local/repository/LocalSongRepository.kt @@ -1,7 +1,6 @@ package com.simplecityapps.localmediaprovider.local.repository import com.simplecityapps.localmediaprovider.local.data.room.dao.SongDataDao -import com.simplecityapps.localmediaprovider.local.data.room.dao.toSong import com.simplecityapps.localmediaprovider.local.data.room.entity.toSongData import com.simplecityapps.localmediaprovider.local.data.room.entity.toSongDataUpdate import com.simplecityapps.mediaprovider.repository.songs.SongRepository @@ -14,6 +13,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn @@ -47,24 +47,6 @@ class LocalSongRepository( ?.sortedWith(query.sortOrder.comparator) } - override suspend fun getSongsDirect(query: SongQuery): List { - val songs = songDataDao.get().map { it.toSong() } - - var result = songs - - if (!query.includeExcluded) { - result = songs.filterNot { it.blacklisted } - } - - query.providerType?.let { providerType -> - result = songs.filter { song -> song.mediaProvider == providerType } - } - - return result - .filter(query.predicate) - .sortedWith(query.sortOrder.comparator) - } - override suspend fun insert( songs: List, mediaProviderType: MediaProviderType @@ -92,7 +74,15 @@ class LocalSongRepository( updates: List, deletes: List, mediaProviderType: MediaProviderType - ): Triple = songDataDao.insertUpdateAndDelete(inserts.toSongData(mediaProviderType), updates.toSongDataUpdate(), deletes.toSongData(mediaProviderType)) + ): Triple { + val result = songDataDao.insertUpdateAndDelete(inserts.toSongData(mediaProviderType), updates.toSongDataUpdate(), deletes.toSongData(mediaProviderType)) + + // Wait for the StateFlow cache to synchronize with the database changes + // This ensures subsequent reads will see the updated data, preventing race conditions + songsRelay.first { it != null } + + return result + } override suspend fun incrementPlayCount(song: Song) { Timber.v("Incrementing play count for song: ${song.name}")