diff --git a/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt b/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt index f2930d3ec..488c98126 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/MusicSettings.kt @@ -43,7 +43,7 @@ interface MusicSettings : Settings { /** Whether to be actively watching for changes in the music library. */ val shouldBeObserving: Boolean /** A [String] of characters representing the desired characters to denote multi-value tags. */ - var multiValueSeparators: String + var separators: String /** Whether to enable more advanced sorting by articles and numbers. */ val intelligentSorting: Boolean @@ -85,7 +85,7 @@ class MusicSettingsImpl @Inject constructor(@ApplicationContext context: Context override val shouldBeObserving: Boolean get() = sharedPreferences.getBoolean(getString(R.string.set_key_observing), false) - override var multiValueSeparators: String + override var separators: String // Differ from convention and store a string of separator characters instead of an int // code. This makes it easier to use and more extendable. get() = sharedPreferences.getString(getString(R.string.set_key_separators), "") ?: "" diff --git a/app/src/main/java/org/oxycblt/auxio/music/device/DeviceLibrary.kt b/app/src/main/java/org/oxycblt/auxio/music/device/DeviceLibrary.kt index cd75ba578..eae4265ff 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/device/DeviceLibrary.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/device/DeviceLibrary.kt @@ -32,6 +32,7 @@ import org.oxycblt.auxio.music.MusicSettings import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.fs.contentResolverSafe import org.oxycblt.auxio.music.fs.useQuery +import org.oxycblt.auxio.music.metadata.Separators import org.oxycblt.auxio.util.logW import org.oxycblt.auxio.util.unlikelyToBeNull @@ -118,6 +119,8 @@ class DeviceLibraryFactoryImpl @Inject constructor(private val musicSettings: Mu rawSongs: Channel, processedSongs: Channel ): DeviceLibraryImpl { + val separators = Separators.from(musicSettings.separators) + val songGrouping = mutableMapOf() val albumGrouping = mutableMapOf>() val artistGrouping = mutableMapOf>() @@ -127,7 +130,7 @@ class DeviceLibraryFactoryImpl @Inject constructor(private val musicSettings: Mu // All music information is grouped as it is indexed by other components. for (rawSong in rawSongs) { - val song = SongImpl(rawSong, musicSettings) + val song = SongImpl(rawSong, musicSettings, separators) // At times the indexer produces duplicate songs, try to filter these. Comparing by // UID is sufficient for something like this, and also prevents collisions from // causing severe issues elsewhere. diff --git a/app/src/main/java/org/oxycblt/auxio/music/device/DeviceMusicImpl.kt b/app/src/main/java/org/oxycblt/auxio/music/device/DeviceMusicImpl.kt index 1d2ce2a26..b8b20f06b 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/device/DeviceMusicImpl.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/device/DeviceMusicImpl.kt @@ -36,8 +36,8 @@ import org.oxycblt.auxio.music.info.Date import org.oxycblt.auxio.music.info.Disc import org.oxycblt.auxio.music.info.Name import org.oxycblt.auxio.music.info.ReleaseType +import org.oxycblt.auxio.music.metadata.Separators import org.oxycblt.auxio.music.metadata.parseId3GenreNames -import org.oxycblt.auxio.music.metadata.parseMultiValue import org.oxycblt.auxio.playback.replaygain.ReplayGainAdjustment import org.oxycblt.auxio.util.positiveOrNull import org.oxycblt.auxio.util.toUuidOrNull @@ -48,10 +48,12 @@ import org.oxycblt.auxio.util.update * Library-backed implementation of [Song]. * * @param rawSong The [RawSong] to derive the member data from. + * @param separators The [Separators] to parse multi-value tags with. * @param musicSettings [MusicSettings] to for user parsing configuration. * @author Alexander Capehart (OxygenCobalt) */ -class SongImpl(private val rawSong: RawSong, musicSettings: MusicSettings) : Song { +class SongImpl(private val rawSong: RawSong, musicSettings: MusicSettings, separators: Separators) : + Song { override val uid = // Attempt to use a MusicBrainz ID first before falling back to a hashed UID. rawSong.musicBrainzId?.toUuidOrNull()?.let { Music.UID.musicBrainz(MusicType.SONGS, it) } @@ -95,42 +97,11 @@ class SongImpl(private val rawSong: RawSong, musicSettings: MusicSettings) : Son track = rawSong.replayGainTrackAdjustment, album = rawSong.replayGainAlbumAdjustment) override val dateAdded = requireNotNull(rawSong.dateAdded) { "Invalid raw: No date added" } + private var _album: AlbumImpl? = null override val album: Album get() = unlikelyToBeNull(_album) - private val hashCode = 31 * uid.hashCode() + rawSong.hashCode() - - override fun hashCode() = hashCode - - override fun equals(other: Any?) = - other is SongImpl && uid == other.uid && rawSong == other.rawSong - - override fun toString() = "Song(uid=$uid, name=$name)" - - private val artistMusicBrainzIds = rawSong.artistMusicBrainzIds.parseMultiValue(musicSettings) - private val artistNames = rawSong.artistNames.parseMultiValue(musicSettings) - private val artistSortNames = rawSong.artistSortNames.parseMultiValue(musicSettings) - private val rawIndividualArtists = - artistNames.mapIndexed { i, name -> - RawArtist( - artistMusicBrainzIds.getOrNull(i)?.toUuidOrNull(), - name, - artistSortNames.getOrNull(i)) - } - - private val albumArtistMusicBrainzIds = - rawSong.albumArtistMusicBrainzIds.parseMultiValue(musicSettings) - private val albumArtistNames = rawSong.albumArtistNames.parseMultiValue(musicSettings) - private val albumArtistSortNames = rawSong.albumArtistSortNames.parseMultiValue(musicSettings) - private val rawAlbumArtists = - albumArtistNames.mapIndexed { i, name -> - RawArtist( - albumArtistMusicBrainzIds.getOrNull(i)?.toUuidOrNull(), - name, - albumArtistSortNames.getOrNull(i)) - } - private val _artists = mutableListOf() override val artists: List get() = _artists @@ -139,44 +110,87 @@ class SongImpl(private val rawSong: RawSong, musicSettings: MusicSettings) : Son override val genres: List get() = _genres + private val hashCode = 31 * uid.hashCode() + rawSong.hashCode() + + override fun hashCode() = hashCode + + // TODO: I cant compare by raw information actually, as it also means that any settings + // configuration will be lost as well. + override fun equals(other: Any?) = + other is SongImpl && uid == other.uid && rawSong == other.rawSong + + override fun toString() = "Song(uid=$uid, name=$name)" + /** * The [RawAlbum] instances collated by the [Song]. This can be used to group [Song]s into an * [Album]. */ - val rawAlbum = - RawAlbum( - mediaStoreId = requireNotNull(rawSong.albumMediaStoreId) { "Invalid raw: No album id" }, - musicBrainzId = rawSong.albumMusicBrainzId?.toUuidOrNull(), - name = requireNotNull(rawSong.albumName) { "Invalid raw: No album name" }, - sortName = rawSong.albumSortName, - releaseType = ReleaseType.parse(rawSong.releaseTypes.parseMultiValue(musicSettings)), - rawArtists = - rawAlbumArtists - .ifEmpty { rawIndividualArtists } - .distinctBy { it.key } - .ifEmpty { listOf(RawArtist(null, null)) }) + val rawAlbum: RawAlbum /** * The [RawArtist] instances collated by the [Song]. The artists of the song take priority, * followed by the album artists. If there are no artists, this field will be a single "unknown" * [RawArtist]. This can be used to group up [Song]s into an [Artist]. */ - val rawArtists = - rawIndividualArtists - .ifEmpty { rawAlbumArtists } - .distinctBy { it.key } - .ifEmpty { listOf(RawArtist()) } + val rawArtists: List /** * The [RawGenre] instances collated by the [Song]. This can be used to group up [Song]s into a * [Genre]. ID3v2 Genre names are automatically converted to their resolved names. */ - val rawGenres = - rawSong.genreNames - .parseId3GenreNames(musicSettings) - .map { RawGenre(it) } - .distinctBy { it.key } - .ifEmpty { listOf(RawGenre()) } + val rawGenres: List + + init { + val artistMusicBrainzIds = separators.split(rawSong.artistMusicBrainzIds) + val artistNames = separators.split(rawSong.artistNames) + val artistSortNames = separators.split(rawSong.artistSortNames) + val rawIndividualArtists = + artistNames + .mapIndexedTo(mutableSetOf()) { i, name -> + RawArtist( + artistMusicBrainzIds.getOrNull(i)?.toUuidOrNull(), + name, + artistSortNames.getOrNull(i)) + } + .toList() + + val albumArtistMusicBrainzIds = separators.split(rawSong.albumArtistMusicBrainzIds) + val albumArtistNames = separators.split(rawSong.albumArtistNames) + val albumArtistSortNames = separators.split(rawSong.albumArtistSortNames) + val rawAlbumArtists = + albumArtistNames + .mapIndexedTo(mutableSetOf()) { i, name -> + RawArtist( + albumArtistMusicBrainzIds.getOrNull(i)?.toUuidOrNull(), + name, + albumArtistSortNames.getOrNull(i)) + } + .toList() + + rawAlbum = + RawAlbum( + mediaStoreId = + requireNotNull(rawSong.albumMediaStoreId) { "Invalid raw: No album id" }, + musicBrainzId = rawSong.albumMusicBrainzId?.toUuidOrNull(), + name = requireNotNull(rawSong.albumName) { "Invalid raw: No album name" }, + sortName = rawSong.albumSortName, + releaseType = ReleaseType.parse(separators.split(rawSong.releaseTypes)), + rawArtists = + rawAlbumArtists + .ifEmpty { rawIndividualArtists } + .ifEmpty { listOf(RawArtist()) }) + + rawArtists = + rawIndividualArtists.ifEmpty { rawAlbumArtists }.ifEmpty { listOf(RawArtist()) } + + val genreNames = + (rawSong.genreNames.parseId3GenreNames() ?: separators.split(rawSong.genreNames)) + rawGenres = + genreNames + .mapTo(mutableSetOf()) { RawGenre(it) } + .toList() + .ifEmpty { listOf(RawGenre()) } + } /** * Links this [Song] with a parent [Album]. diff --git a/app/src/main/java/org/oxycblt/auxio/music/metadata/Separators.kt b/app/src/main/java/org/oxycblt/auxio/music/metadata/Separators.kt new file mode 100644 index 000000000..5142c6905 --- /dev/null +++ b/app/src/main/java/org/oxycblt/auxio/music/metadata/Separators.kt @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2023 Auxio Project + * Separators.kt is part of Auxio. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.oxycblt.auxio.music.metadata + +/** + * Defines the user-specified parsing of multi-value tags. This should be used to parse any tags + * that may be delimited with a separator character. + * + * @author Alexander Capehart (OxygenCobalt) + */ +interface Separators { + /** + * Parse a separated value from one or more strings. If the value is already composed of more + * than one value, nothing is done. Otherwise, it will attempt to split it based on the user's + * separator preferences. + * + * @return A new list of one or more [String]s parsed by the separator configuration + */ + fun split(strings: List): List + + companion object { + const val COMMA = ',' + const val SEMICOLON = ';' + const val SLASH = '/' + const val PLUS = '+' + const val AND = '&' + + fun from(selector: String) = + if (selector.isNotEmpty()) CharSeparators(selector.toSet()) else NoSeparators + } +} + +private data class CharSeparators(private val chars: Set) : Separators { + override fun split(strings: List) = + if (strings.size == 1) splitImpl(strings.first()) else strings + + private fun splitImpl(string: String) = + string.splitEscaped { chars.contains(it) }.correctWhitespace() +} + +private object NoSeparators : Separators { + override fun split(strings: List) = strings +} diff --git a/app/src/main/java/org/oxycblt/auxio/music/metadata/SeparatorsDialog.kt b/app/src/main/java/org/oxycblt/auxio/music/metadata/SeparatorsDialog.kt index d74b9ba53..31195c408 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/metadata/SeparatorsDialog.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/metadata/SeparatorsDialog.kt @@ -52,7 +52,7 @@ class SeparatorsDialog : ViewBindingMaterialDialogFragment - musicSettings.multiValueSeparators = getCurrentSeparators() + musicSettings.separators = getCurrentSeparators() } } @@ -68,8 +68,7 @@ class SeparatorsDialog : ViewBindingMaterialDialogFragment binding.separatorComma.isChecked = true @@ -102,14 +101,6 @@ class SeparatorsDialog : ViewBindingMaterialDialogFragment.parseMultiValue(settings: MusicSettings) = - if (size == 1) { - first().maybeParseBySeparators(settings) - } else { - // Nothing to do. - this - } - // TODO: Remove the escaping checks, it's too expensive to do this for every single tag. /** @@ -101,17 +84,6 @@ fun String.correctWhitespace() = trim().ifBlank { null } */ fun List.correctWhitespace() = mapNotNull { it.correctWhitespace() } -/** - * Attempt to parse a string by the user's separator preferences. - * - * @param settings [MusicSettings] required to obtain user separator configuration. - * @return A list of one or more [String]s that were split up by the user-defined separators. - */ -private fun String.maybeParseBySeparators(settings: MusicSettings): List { - if (settings.multiValueSeparators.isEmpty()) return listOf(this) - return splitEscaped { settings.multiValueSeparators.contains(it) }.correctWhitespace() -} - /// --- ID3v2 PARSING --- /** @@ -165,12 +137,12 @@ fun transformPositionField(pos: Int?, total: Int?) = * representations of genre fields into their named counterparts, and split up singular ID3v2-style * integer genre fields into one or more genres. * - * @param settings [MusicSettings] required to obtain user separator configuration. - * @return A list of one or more genre names.. + * @return A list of one or more genre names, or null if this multi-value list has no valid + * formatting. */ -fun List.parseId3GenreNames(settings: MusicSettings) = +fun List.parseId3GenreNames() = if (size == 1) { - first().parseId3MultiValueGenre(settings) + first().parseId3MultiValueGenre() } else { // Nothing to split, just map any ID3v1 genres to their name counterparts. map { it.parseId3v1Genre() ?: it } @@ -179,11 +151,10 @@ fun List.parseId3GenreNames(settings: MusicSettings) = /** * Parse a single ID3v1/ID3v2 integer genre field into their named representations. * - * @param settings [MusicSettings] required to obtain user separator configuration. - * @return A list of one or more genre names. + * @return list of one or more genre names, or null if this is not in ID3v2 format. */ -private fun String.parseId3MultiValueGenre(settings: MusicSettings) = - parseId3v1Genre()?.let { listOf(it) } ?: parseId3v2Genre() ?: maybeParseBySeparators(settings) +private fun String.parseId3MultiValueGenre() = + parseId3v1Genre()?.let { listOf(it) } ?: parseId3v2Genre() /** * Parse an ID3v1 integer genre field. diff --git a/app/src/test/java/org/oxycblt/auxio/music/info/NameTest.kt b/app/src/test/java/org/oxycblt/auxio/music/info/NameTest.kt index 9ede93b5e..f23f76f16 100644 --- a/app/src/test/java/org/oxycblt/auxio/music/info/NameTest.kt +++ b/app/src/test/java/org/oxycblt/auxio/music/info/NameTest.kt @@ -27,7 +27,7 @@ import org.oxycblt.auxio.music.MusicSettings class NameTest { private fun mockIntelligentSorting(enabled: Boolean) = - mockk().apply { every { intelligentSorting } returns enabled } + mockk() { every { intelligentSorting } returns enabled } @Test fun name_from_simple_withoutPunct() { diff --git a/app/src/test/java/org/oxycblt/auxio/music/metadata/SeparatorsTest.kt b/app/src/test/java/org/oxycblt/auxio/music/metadata/SeparatorsTest.kt new file mode 100644 index 000000000..440f044e3 --- /dev/null +++ b/app/src/test/java/org/oxycblt/auxio/music/metadata/SeparatorsTest.kt @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2023 Auxio Project + * SeparatorsTest.kt is part of Auxio. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.oxycblt.auxio.music.metadata + +import org.junit.Assert.assertEquals +import org.junit.Test + +class SeparatorsTest { + @Test + fun separators_split_withString_withSingleChar() { + assertEquals(listOf("a", "b", "c"), Separators.from(",").split(listOf("a,b,c"))) + } + + @Test + fun separators_split_withMultiple_withSingleChar() { + assertEquals(listOf("a,b", "c", "d"), Separators.from(",").split(listOf("a,b", "c", "d"))) + } + + @Test + fun separators_split_withString_withMultipleChar() { + assertEquals( + listOf("a", "b", "c", "d", "e", "f"), + Separators.from(",;/+&").split(listOf("a,b;c/d+e&f"))) + } + + @Test + fun separators_split_withList_withMultipleChar() { + assertEquals( + listOf("a,b;c/d", "e&f"), Separators.from(",;/+&").split(listOf("a,b;c/d", "e&f"))) + } +} diff --git a/app/src/test/java/org/oxycblt/auxio/music/metadata/TagUtilTest.kt b/app/src/test/java/org/oxycblt/auxio/music/metadata/TagUtilTest.kt index 5f6c67c89..7c900d42c 100644 --- a/app/src/test/java/org/oxycblt/auxio/music/metadata/TagUtilTest.kt +++ b/app/src/test/java/org/oxycblt/auxio/music/metadata/TagUtilTest.kt @@ -18,34 +18,10 @@ package org.oxycblt.auxio.music.metadata -import io.mockk.every -import io.mockk.mockk import org.junit.Assert.assertEquals import org.junit.Test -import org.oxycblt.auxio.music.MusicSettings class TagUtilTest { - private fun mockSeparators(separators: String) = - mockk().apply { every { multiValueSeparators } returns separators } - - @Test - fun parseMultiValue_single() { - assertEquals(listOf("a", "b", "c"), listOf("a,b,c").parseMultiValue(mockSeparators(","))) - } - - @Test - fun parseMultiValue_many() { - assertEquals( - listOf("a", "b", "c"), listOf("a", "b", "c").parseMultiValue(mockSeparators(","))) - } - - @Test - fun parseMultiValue_several() { - assertEquals( - listOf("a", "b", "c", "d", "e", "f"), - listOf("a,b;c/d+e&f").parseMultiValue(mockSeparators(",;/+&"))) - } - @Test fun splitEscaped_correct() { assertEquals(listOf("a", "b", "c"), "a,b,c".splitEscaped { it == ',' }) @@ -136,37 +112,30 @@ class TagUtilTest { fun parseId3v2Genre_multi() { assertEquals( listOf("Post-Rock", "Shoegaze", "Glitch"), - listOf("Post-Rock", "Shoegaze", "Glitch").parseId3GenreNames(mockSeparators(","))) + listOf("Post-Rock", "Shoegaze", "Glitch").parseId3GenreNames()) } @Test fun parseId3v2Genre_multiId3v1() { assertEquals( listOf("Post-Rock", "Shoegaze", "Glitch"), - listOf("176", "178", "Glitch").parseId3GenreNames(mockSeparators(","))) + listOf("176", "178", "Glitch").parseId3GenreNames()) } @Test fun parseId3v2Genre_wackId3() { - assertEquals(listOf("2941"), listOf("2941").parseId3GenreNames(mockSeparators(","))) + assertEquals(null, listOf("2941").parseId3GenreNames()) } @Test fun parseId3v2Genre_singleId3v23() { assertEquals( listOf("Post-Rock", "Shoegaze", "Remix", "Cover", "Glitch"), - listOf("(176)(178)(RX)(CR)Glitch").parseId3GenreNames(mockSeparators(","))) - } - - @Test - fun parseId3v2Genre_singleSeparated() { - assertEquals( - listOf("Post-Rock", "Shoegaze", "Glitch"), - listOf("Post-Rock, Shoegaze, Glitch").parseId3GenreNames(mockSeparators(","))) + listOf("(176)(178)(RX)(CR)Glitch").parseId3GenreNames()) } @Test fun parsId3v2Genre_singleId3v1() { - assertEquals(listOf("Post-Rock"), listOf("176").parseId3GenreNames(mockSeparators(","))) + assertEquals(listOf("Post-Rock"), listOf("176").parseId3GenreNames()) } }