Skip to content

Port upstream OSS fixes #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lostf1sh wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Port upstream OSS fixes #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions ...src/androidTest/java/com/lostf1sh/pixelplayeross/data/repository/PlaylistSongCountTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package com.lostf1sh.pixelplayeross.data.repository

import android.content.Context
import androidx.datastore.core.DataStore
import androidx.datastore.preferences.core.PreferenceDataStoreFactory
import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.preferencesDataStoreFile
import androidx.room.Room
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.lostf1sh.pixelplayeross.data.database.LocalPlaylistDao
import com.lostf1sh.pixelplayeross.data.database.PixelPlayerDatabase
import com.lostf1sh.pixelplayeross.data.preferences.PlaylistPreferencesRepository
import com.lostf1sh.pixelplayeross.data.preferences.UserPreferencesRepository
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
import kotlinx.serialization.json.Json
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
class PlaylistSongCountTest {

private lateinit var db: PixelPlayerDatabase
private lateinit var dao: LocalPlaylistDao
private lateinit var dataStore: DataStore<Preferences>
private lateinit var repo: PlaylistPreferencesRepository

@Before
fun setup() {
val context = ApplicationProvider.getApplicationContext<Context>()
db = Room.inMemoryDatabaseBuilder(context, PixelPlayerDatabase::class.java)
.addCallback(PixelPlayerDatabase.createRuntimeArtifactsCallback())
.allowMainThreadQueries()
.build()
dao = db.localPlaylistDao()
dataStore = PreferenceDataStoreFactory.create {
context.preferencesDataStoreFile("test_settings_${System.nanoTime()}")
}
val userPrefs = UserPreferencesRepository(dataStore, Json { ignoreUnknownKeys = true })
repo = PlaylistPreferencesRepository(dao, userPrefs)
}

@After
fun teardown() {
db.close()
}

private suspend fun countFor(playlistId: String): Int =
repo.userPlaylistsFlow.first().first { it.id == playlistId }.songIds.size

@Test
fun menuSongCount_reflectsAddAndRemove() = runTest {
val playlist = repo.createPlaylist(name = "J-Pop", songIds = listOf("10", "20", "30"))
assertEquals("initial count", 3, countFor(playlist.id))

repo.removeSongFromPlaylist(playlist.id, "20")
assertEquals("after removing one song", 2, countFor(playlist.id))

repo.removeSongFromPlaylist(playlist.id, "30")
assertEquals("after removing a second song", 1, countFor(playlist.id))

repo.addSongsToPlaylist(playlist.id, listOf("40"))
assertEquals("after adding one song", 2, countFor(playlist.id))
}

@Test
fun concurrentRemovals_doNotLoseUpdates() = runBlocking {
val playlist = repo.createPlaylist(
name = "Race",
songIds = listOf("1", "2", "3", "4", "5")
)
assertEquals(5, countFor(playlist.id))

coroutineScope {
listOf("1", "2", "3", "4").forEach { id ->
launch(Dispatchers.IO) { repo.removeSongFromPlaylist(playlist.id, id) }
}
}

assertEquals("All concurrent removals must persist", 1, countFor(playlist.id))
}

@Test
fun quickRemoveThenAdd_keepsCountAccurate() = runBlocking {
val playlist = repo.createPlaylist(
name = "J-Pop",
songIds = listOf("1", "2", "3", "4", "5", "6")
)
assertEquals(6, countFor(playlist.id))

coroutineScope {
launch(Dispatchers.IO) { repo.removeSongFromPlaylist(playlist.id, "2") }
launch(Dispatchers.IO) { repo.removeSongFromPlaylist(playlist.id, "4") }
}
assertEquals("count after removing two songs", 4, countFor(playlist.id))

repo.addSongsToPlaylist(playlist.id, listOf("7", "8"))
assertEquals("count after adding two songs", 6, countFor(playlist.id))
}
}
4 changes: 4 additions & 0 deletions app/src/main/java/com/lostf1sh/pixelplayeross/data/database/MusicDao.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1577,6 +1577,7 @@ interface MusicDao {
artist_id = :artistId,
artists_json = :artistsJson,
album_name = :album,
album_artist = :albumArtist,
genre = :genre,
track_number = :trackNumber,
disc_number = :discNumber,
Expand All @@ -1593,6 +1594,7 @@ interface MusicDao {
artistId: Long,
artistsJson: String?,
album: String,
albumArtist: String?,
genre: String?,
trackNumber: Int,
discNumber: Int?
Expand All @@ -1606,6 +1608,7 @@ interface MusicDao {
artistId: Long,
artistsJson: String?,
album: String,
albumArtist: String?,
genre: String?,
trackNumber: Int,
discNumber: Int?,
Expand All @@ -1623,6 +1626,7 @@ interface MusicDao {
artistId = artistId,
artistsJson = artistsJson,
album = album,
albumArtist = albumArtist,
genre = genre,
trackNumber = trackNumber,
discNumber = discNumber
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/java/com/lostf1sh/pixelplayeross/data/media/SongMetadataEditor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class SongMetadataEditor(
title: String,
artist: String,
album: String,
albumArtist: String?,
genre: String?,
trackNumber: Int,
discNumber: Int?
Expand Down Expand Up @@ -218,6 +219,7 @@ class SongMetadataEditor(
artistId = primaryArtistId,
artistsJson = artistsJson,
album = album,
albumArtist = albumArtist,
genre = genre,
trackNumber = trackNumber,
discNumber = discNumber,
Expand Down Expand Up @@ -444,6 +446,7 @@ class SongMetadataEditor(
title = newTitle,
artist = newArtist,
album = newAlbum,
albumArtist = newAlbumArtist,
genre = normalizedGenre,
trackNumber = newTrackNumber,
discNumber = newDiscNumber
Expand Down
75 changes: 47 additions & 28 deletions ...c/main/java/com/lostf1sh/pixelplayeross/data/preferences/PlaylistPreferencesRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class PlaylistPreferencesRepository @Inject constructor(
private val userPreferencesRepository: UserPreferencesRepository
) {
private val migrationMutex = Mutex()
// Serializes playlist read-modify-write edits. Concurrent quick taps can otherwise
// read the same snapshot and let the last writer silently drop earlier edits.
private val editMutex = Mutex()
@Volatile
private var migrationChecked = false

Expand Down Expand Up @@ -85,28 +88,38 @@ class PlaylistPreferencesRepository @Inject constructor(
}

suspend fun renamePlaylist(playlistId: String, newName: String) {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
val updated = existing.copy(
name = newName,
lastModified = System.currentTimeMillis()
)
localPlaylistDao.upsertPlaylist(updated.toEntity())
editMutex.withLock {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
val updated = existing.copy(
name = newName,
lastModified = System.currentTimeMillis()
)
localPlaylistDao.upsertPlaylist(updated.toEntity())
}
}

suspend fun updatePlaylist(playlist: Playlist) {
editMutex.withLock {
updatePlaylistLocked(playlist)
}
}

private suspend fun updatePlaylistLocked(playlist: Playlist) {
ensureMigratedIfNeeded()
val updated = playlist.copy(lastModified = System.currentTimeMillis())
localPlaylistDao.upsertPlaylist(updated.toEntity())
localPlaylistDao.replacePlaylistSongs(updated.id, updated.songIds)
}

suspend fun addSongsToPlaylist(playlistId: String, songIdsToAdd: List<String>) {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
if (existing.isSmartPlaylist) return
val merged = (existing.songIds + songIdsToAdd).distinct()
updatePlaylist(existing.copy(songIds = merged))
editMutex.withLock {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
if (existing.isSmartPlaylist) return
val merged = (existing.songIds + songIdsToAdd).distinct()
updatePlaylistLocked(existing.copy(songIds = merged))
}
}

suspend fun addOrRemoveSongFromPlaylists(songId: String, playlistIds: List<String>): MutableList<String> {
Expand All @@ -132,17 +145,21 @@ class PlaylistPreferencesRepository @Inject constructor(
}

suspend fun removeSongFromPlaylist(playlistId: String, songIdToRemove: String) {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
if (existing.isSmartPlaylist) return
updatePlaylist(existing.copy(songIds = existing.songIds.filterNot { it == songIdToRemove }))
editMutex.withLock {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
if (existing.isSmartPlaylist) return
updatePlaylistLocked(existing.copy(songIds = existing.songIds.filterNot { it == songIdToRemove }))
}
}

suspend fun reorderSongsInPlaylist(playlistId: String, newSongOrderIds: List<String>) {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
if (existing.isSmartPlaylist) return
updatePlaylist(existing.copy(songIds = newSongOrderIds))
editMutex.withLock {
ensureMigratedIfNeeded()
val existing = userPlaylistsFlow.first().find { it.id == playlistId } ?: return
if (existing.isSmartPlaylist) return
updatePlaylistLocked(existing.copy(songIds = newSongOrderIds))
}
}

suspend fun setPlaylistSongOrderMode(playlistId: String, modeValue: String) =
Expand Down Expand Up @@ -171,15 +188,17 @@ class PlaylistPreferencesRepository @Inject constructor(
}

suspend fun removeSongFromAllPlaylists(songId: String) {
ensureMigratedIfNeeded()
val playlists = userPlaylistsFlow.first()
playlists.forEach { playlist ->
if (songId in playlist.songIds) {
updatePlaylist(
playlist.copy(
songIds = playlist.songIds.filterNot { it == songId }
editMutex.withLock {
ensureMigratedIfNeeded()
val playlists = userPlaylistsFlow.first()
playlists.forEach { playlist ->
if (songId in playlist.songIds) {
updatePlaylistLocked(
playlist.copy(
songIds = playlist.songIds.filterNot { it == songId }
)
)
)
}
}
}
}
Expand Down
21 changes: 19 additions & 2 deletions app/src/main/java/com/lostf1sh/pixelplayeross/data/preferences/UserPreferencesRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ constructor(
// ReplayGain
val REPLAYGAIN_ENABLED = booleanPreferencesKey("replaygain_enabled")
val REPLAYGAIN_USE_ALBUM_GAIN = booleanPreferencesKey("replaygain_use_album_gain")
val PAUSE_ON_VOLUME_ZERO = booleanPreferencesKey("pause_on_volume_zero")
}

val appRebrandDialogShownFlow: Flow<Boolean> =
Expand Down Expand Up @@ -406,7 +407,7 @@ constructor(
val artistDelimitersFlow: Flow<List<String>> =
dataStore.data.map { preferences ->
val stored = preferences[PreferencesKeys.ARTIST_DELIMITERS]
if (stored != null) {
val delimiters = if (stored != null) {
try {
json.decodeFromString<List<String>>(stored)
} catch (e: Exception) {
Expand All @@ -415,6 +416,7 @@ constructor(
} else {
DEFAULT_ARTIST_DELIMITERS
}
normalizeLegacyDefaultArtistDelimiters(delimiters)
}

suspend fun setArtistDelimiters(delimiters: List<String>) {
Expand Down Expand Up @@ -805,6 +807,11 @@ constructor(
preferences[PreferencesKeys.REPLAYGAIN_USE_ALBUM_GAIN] ?: false
}

val pauseOnVolumeZeroFlow: Flow<Boolean> =
dataStore.data.map { preferences ->
preferences[PreferencesKeys.PAUSE_ON_VOLUME_ZERO] ?: false
}

suspend fun setReplayGainEnabled(enabled: Boolean) {
dataStore.edit { preferences ->
preferences[PreferencesKeys.REPLAYGAIN_ENABLED] = enabled
Expand All @@ -817,6 +824,12 @@ constructor(
}
}

suspend fun setPauseOnVolumeZero(enabled: Boolean) {
dataStore.edit { preferences ->
preferences[PreferencesKeys.PAUSE_ON_VOLUME_ZERO] = enabled
}
}

// ===== End ReplayGain =====

val allowedDirectoriesFlow: Flow<Set<String>> =
Expand Down Expand Up @@ -1242,12 +1255,16 @@ constructor(

companion object {
/** Default character delimiters for splitting multi-artist tags */
val DEFAULT_ARTIST_DELIMITERS = listOf("/", ";", ",", "+", "&")
val DEFAULT_ARTIST_DELIMITERS = listOf(";")
private val LEGACY_DEFAULT_ARTIST_DELIMITERS = listOf("/", ";", ",", "+", "&")
/** Default word-based delimiters (matched case-insensitively with whitespace boundaries) */
val DEFAULT_ARTIST_WORD_DELIMITERS = listOf("featuring", "feat.", "feat", "ft.", "ft", "vs.", "vs", "versus", "with", "prod.", "prod")
const val DEFAULT_ALBUM_ART_CACHE_LIMIT_MB = 200
}

private fun normalizeLegacyDefaultArtistDelimiters(delimiters: List<String>): List<String> =
if (delimiters == LEGACY_DEFAULT_ARTIST_DELIMITERS) DEFAULT_ARTIST_DELIMITERS else delimiters

val navBarCornerRadiusFlow: Flow<Int> =
dataStore.data.map { preferences ->
sanitizeNavBarCornerRadius(preferences[PreferencesKeys.NAV_BAR_CORNER_RADIUS] ?: 32)
Expand Down
Loading