Shuttle2 icon indicating copy to clipboard operation
Shuttle2 copied to clipboard

Fix playlist import race condition on large libraries (#127)Import playlists issue

Open timusus opened this issue 3 months ago • 0 comments

Summary

Fixes #127 - Playlist imports were failing on large libraries due to a race condition between database writes and StateFlow cache updates.

This PR implements a clean architectural solution by ensuring write operations fully complete (including cache synchronization) before returning.

Root Cause

When importing large music libraries (7000+ songs):

  1. Songs were inserted into the Room database successfully
  2. The StateFlow cache in LocalSongRepository updated asynchronously via Room's InvalidationTracker
  3. Playlist import immediately queried songs using .firstOrNull() on the StateFlow
  4. The StateFlow returned stale/empty cached data before the invalidation completed
  5. Playlists couldn't match any songs, resulting in empty or failed imports

Solution

Made write operations synchronous with cache updates:

  • Modified insertUpdateAndDelete() in LocalSongRepository to wait for the StateFlow cache to synchronize before returning
  • Uses songsRelay.first { it != null } to suspend until the cache reflects database changes
  • Maintains clean reactive architecture

Why this approach:

  • ✅ Maintains single responsibility - repositories manage their own consistency
  • ✅ No code smell - doesn't expose implementation details in interfaces
  • ✅ Preserves reactive Flow-based architecture for UI
  • ✅ Follows principle of least surprise - writes complete before returning
  • ✅ No timing dependencies or arbitrary delays

Testing

The fix ensures that:

  • After insertUpdateAndDelete() returns, the StateFlow cache is synchronized
  • Subsequent queries via getSongs().firstOrNull() return fresh data
  • Playlist imports can reliably match songs regardless of library size

Changes

LocalSongRepository.kt:71-84

  • Added cache synchronization to insertUpdateAndDelete()
  • Waits for StateFlow emission before returning

No changes to:

  • SongRepository interface (maintains clean abstraction)
  • MediaImporter logic (no workarounds needed)

timusus avatar Nov 16 '25 06:11 timusus