From a5ea4af5c41b473e67b4b5b862c10d39a8049bf0 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Thu, 5 Jan 2023 20:08:12 -0700 Subject: [PATCH] music: finish parsing tests Finish ParsingUtil tests. --- .../java/org/oxycblt/auxio/music/Music.kt | 19 ++++--- .../music/extractor/MetadataExtractor.kt | 7 ++- .../auxio/music/parsing/ParsingUtil.kt | 50 ++++++----------- .../org/oxycblt/auxio/music/system/Indexer.kt | 3 +- .../org/oxycblt/auxio/settings/Settings.kt | 7 +-- .../auxio/music/parsing/ParsingUtilTest.kt | 56 ++++++++++++++++++- 6 files changed, 92 insertions(+), 50 deletions(-) diff --git a/app/src/main/java/org/oxycblt/auxio/music/Music.kt b/app/src/main/java/org/oxycblt/auxio/music/Music.kt index 8d76972c5..ad24c9b64 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/Music.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/Music.kt @@ -381,9 +381,10 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { val album: Album get() = unlikelyToBeNull(_album) - private val artistMusicBrainzIds = raw.artistMusicBrainzIds.parseMultiValue(settings) - private val artistNames = raw.artistNames.parseMultiValue(settings) - private val artistSortNames = raw.artistSortNames.parseMultiValue(settings) + private val artistMusicBrainzIds = + raw.artistMusicBrainzIds.parseMultiValue(settings.musicSeparators) + private val artistNames = raw.artistNames.parseMultiValue(settings.musicSeparators) + private val artistSortNames = raw.artistSortNames.parseMultiValue(settings.musicSeparators) private val rawArtists = artistNames.mapIndexed { i, name -> Artist.Raw( @@ -392,9 +393,11 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { artistSortNames.getOrNull(i)) } - private val albumArtistMusicBrainzIds = raw.albumArtistMusicBrainzIds.parseMultiValue(settings) - private val albumArtistNames = raw.albumArtistNames.parseMultiValue(settings) - private val albumArtistSortNames = raw.albumArtistSortNames.parseMultiValue(settings) + private val albumArtistMusicBrainzIds = + raw.albumArtistMusicBrainzIds.parseMultiValue(settings.musicSeparators) + private val albumArtistNames = raw.albumArtistNames.parseMultiValue(settings.musicSeparators) + private val albumArtistSortNames = + raw.albumArtistSortNames.parseMultiValue(settings.musicSeparators) private val rawAlbumArtists = albumArtistNames.mapIndexed { i, name -> Artist.Raw( @@ -462,7 +465,7 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { musicBrainzId = raw.albumMusicBrainzId?.toUuidOrNull(), name = requireNotNull(raw.albumName) { "Invalid raw: No album name" }, sortName = raw.albumSortName, - type = Album.Type.parse(raw.albumTypes.parseMultiValue(settings)), + type = Album.Type.parse(raw.albumTypes.parseMultiValue(settings.musicSeparators)), rawArtists = rawAlbumArtists.ifEmpty { rawArtists }.ifEmpty { listOf(Artist.Raw(null, null)) }) @@ -481,7 +484,7 @@ class Song constructor(raw: Raw, settings: Settings) : Music() { */ val _rawGenres = raw.genreNames - .parseId3GenreNames(settings) + .parseId3GenreNames(settings.musicSeparators) .map { Genre.Raw(it) } .ifEmpty { listOf(Genre.Raw()) } diff --git a/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt b/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt index f9399e409..7e353cb91 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/extractor/MetadataExtractor.kt @@ -21,6 +21,7 @@ import android.content.Context import androidx.core.text.isDigitsOnly import com.google.android.exoplayer2.MediaItem import com.google.android.exoplayer2.MetadataRetriever +import kotlinx.coroutines.flow.flow import org.oxycblt.auxio.music.Date import org.oxycblt.auxio.music.Song import org.oxycblt.auxio.music.parsing.parseId3v2Position @@ -61,12 +62,12 @@ class MetadataExtractor( fun finalize(rawSongs: List) = mediaStoreExtractor.finalize(rawSongs) /** - * Parse all [Song.Raw] instances queued by the sub-extractors. This will first delegate to the - * sub-extractors before parsing the metadata itself. + * Returns a flow that parses all [Song.Raw] instances queued by the sub-extractors. This will + * first delegate to the sub-extractors before parsing the metadata itself. * @param emit A listener that will be invoked with every new [Song.Raw] instance when they are * successfully loaded. */ - suspend fun parse(emit: suspend (Song.Raw) -> Unit) { + fun extract() = flow { while (true) { val raw = Song.Raw() when (mediaStoreExtractor.populate(raw)) { diff --git a/app/src/main/java/org/oxycblt/auxio/music/parsing/ParsingUtil.kt b/app/src/main/java/org/oxycblt/auxio/music/parsing/ParsingUtil.kt index 95f193971..fe7543062 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/parsing/ParsingUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/parsing/ParsingUtil.kt @@ -17,7 +17,6 @@ package org.oxycblt.auxio.music.parsing -import org.oxycblt.auxio.settings.Settings import org.oxycblt.auxio.util.nonZeroOrNull /// --- GENERIC PARSING --- @@ -26,12 +25,12 @@ import org.oxycblt.auxio.util.nonZeroOrNull * Parse a multi-value tag based on the user configuration. If the value is already composed of more * than one value, nothing is done. Otherwise, this function will attempt to split it based on the * user's separator preferences. - * @param settings [Settings] required to obtain user separator configuration. + * @param separators A string of characters to split by. Can be empty. * @return A new list of one or more [String]s. */ -fun List.parseMultiValue(settings: Settings) = +fun List.parseMultiValue(separators: String) = if (size == 1) { - first().maybeParseBySeparators(settings) + first().maybeParseBySeparators(separators) } else { // Nothing to do. this @@ -83,7 +82,7 @@ inline fun String.splitEscaped(selector: (Char) -> Boolean): List { /** * Fix trailing whitespace or blank contents in a [String]. - * @return A string with trailing whitespace remove,d or null if the [String] was all whitespace or + * @return A string with trailing whitespace removed or null if the [String] was all whitespace or * empty. */ fun String.correctWhitespace() = trim().ifBlank { null } @@ -96,14 +95,15 @@ fun List.correctWhitespace() = mapNotNull { it.correctWhitespace() } /** * Attempt to parse a string by the user's separator preferences. - * @param settings [Settings] required to obtain user separator configuration. - * @return A list of one or more [String]s that were split up by the user-defined separators. + * @param separators A string of characters to split by. Can be empty. + * @return A list of one or more [String]s that were split up by the given separators. */ -private fun String.maybeParseBySeparators(settings: Settings): List { - // Get the separators the user desires. If null, there's nothing to do. - val separators = settings.musicSeparators ?: return listOf(this) - return splitEscaped { separators.contains(it) }.correctWhitespace() -} +private fun String.maybeParseBySeparators(separators: String) = + if (separators.isNotEmpty()) { + splitEscaped { separators.contains(it) }.correctWhitespace() + } else { + listOf(this) + } /// --- ID3v2 PARSING --- @@ -119,29 +119,20 @@ fun String.parseId3v2Position() = split('/', limit = 2)[0].toIntOrNull()?.nonZer * Parse a multi-value genre name using ID3 rules. This will convert any ID3v1 integer * representations of genre fields into their named counterparts, and split up singular ID3v2-style * integer genre fields into one or more genres. - * @param settings [Settings] required to obtain user separator configuration. - * @return A list of one or more genre names.. + * @param separators A string of characters to split by. Can be empty. + * @return A list of one or more genre names. */ -fun List.parseId3GenreNames(settings: Settings) = +fun List.parseId3GenreNames(separators: String) = if (size == 1) { - first().parseId3MultiValueGenre(settings) + first().parseId3MultiValueGenre(separators) } else { // Nothing to split, just map any ID3v1 genres to their name counterparts. map { it.parseId3v1Genre() ?: it } } -/** - * Parse a single ID3v1/ID3v2 integer genre field into their named representations. - * @return A list of one or more genre names. - */ -private fun String.parseId3MultiValueGenre(settings: Settings) = - parseId3v1Genre()?.let { listOf(it) } ?: parseId3v2Genre() ?: maybeParseBySeparators(settings) +private fun String.parseId3MultiValueGenre(separators: String) = + parseId3v1Genre()?.let { listOf(it) } ?: parseId3v2Genre() ?: maybeParseBySeparators(separators) -/** - * Parse an ID3v1 integer genre field. - * @return A named genre if the field is a valid integer, "Cover" or "Remix" if the field is - * "CR"/"RX" respectively, and nothing if the field is not a valid ID3v1 integer genre. - */ private fun String.parseId3v1Genre(): String? { // ID3v1 genres are a plain integer value without formatting, so in that case // try to index the genre table with such. @@ -164,11 +155,6 @@ private fun String.parseId3v1Genre(): String? { */ private val ID3V2_GENRE_RE = Regex("((?:\\((\\d+|RX|CR)\\))*)(.+)?") -/** - * Parse an ID3v2 integer genre field, which has support for multiple genre values and combined - * named/integer genres. - * @return A list of one or more genres, or null if the field is not a valid ID3v2 integer genre. - */ private fun String.parseId3v2Genre(): List? { val groups = (ID3V2_GENRE_RE.matchEntire(this) ?: return null).groupValues val genres = mutableSetOf() diff --git a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt index 1557ec4a8..adb25242a 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/system/Indexer.kt @@ -24,6 +24,7 @@ import android.os.Build import androidx.core.content.ContextCompat import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.withContext import kotlinx.coroutines.yield import org.oxycblt.auxio.BuildConfig @@ -264,7 +265,7 @@ class Indexer private constructor() { // Note: We use a set here so we can eliminate song duplicates. val songs = mutableSetOf() val rawSongs = mutableListOf() - metadataExtractor.parse { rawSong -> + metadataExtractor.extract().collect { rawSong -> songs.add(Song(rawSong, settings)) rawSongs.add(rawSong) diff --git a/app/src/main/java/org/oxycblt/auxio/settings/Settings.kt b/app/src/main/java/org/oxycblt/auxio/settings/Settings.kt index aabb07823..1ce8fac29 100644 --- a/app/src/main/java/org/oxycblt/auxio/settings/Settings.kt +++ b/app/src/main/java/org/oxycblt/auxio/settings/Settings.kt @@ -325,14 +325,13 @@ class Settings(private val context: Context) { * A string of characters representing the desired separator characters to denote multi-value * tags. */ - var musicSeparators: String? + var musicSeparators: String // Differ from convention and store a string of separator characters instead of an int // code. This makes it easier to use in Regexes and makes it more extendable. - get() = - inner.getString(context.getString(R.string.set_key_separators), null)?.ifEmpty { null } + get() = inner.getString(context.getString(R.string.set_key_separators), "") ?: "" set(value) { inner.edit { - putString(context.getString(R.string.set_key_separators), value?.ifEmpty { null }) + putString(context.getString(R.string.set_key_separators), value) apply() } } diff --git a/app/src/test/java/org/oxycblt/auxio/music/parsing/ParsingUtilTest.kt b/app/src/test/java/org/oxycblt/auxio/music/parsing/ParsingUtilTest.kt index 8ff8e1491..64e7463d4 100644 --- a/app/src/test/java/org/oxycblt/auxio/music/parsing/ParsingUtilTest.kt +++ b/app/src/test/java/org/oxycblt/auxio/music/parsing/ParsingUtilTest.kt @@ -21,7 +21,21 @@ import org.junit.Assert.assertEquals import org.junit.Test class ParsingUtilTest { - // TODO: Incomplete + @Test + fun parseMultiValue_single() { + assertEquals(listOf("a", "b", "c"), listOf("a,b,c").parseMultiValue(",")) + } + + @Test + fun parseMultiValue_many() { + assertEquals(listOf("a", "b", "c"), listOf("a", "b", "c").parseMultiValue(",")) + } + + @Test + fun parseMultiValue_several() { + assertEquals( + listOf("a", "b", "c", "d", "e", "f"), listOf("a,b;c/d+e&f").parseMultiValue(",;/+&")) + } @Test fun splitEscaped_correct() { @@ -67,7 +81,7 @@ class ParsingUtilTest { } @Test - fun correctWhitespace_listOopsAllWhitespacE() { + fun correctWhitespace_listOopsAllWhitespace() { assertEquals( listOf("tcp phagocyte"), listOf(" ", "", " tcp phagocyte").correctWhitespace()) } @@ -86,4 +100,42 @@ class ParsingUtilTest { fun parseId3v2Position_wack() { assertEquals(16, "16/".parseId3v2Position()) } + + @Test + fun parseId3v2Genre_multi() { + assertEquals( + listOf("Post-Rock", "Shoegaze", "Glitch"), + listOf("Post-Rock", "Shoegaze", "Glitch").parseId3GenreNames(",")) + } + + @Test + fun parseId3v2Genre_multiId3v1() { + assertEquals( + listOf("Post-Rock", "Shoegaze", "Glitch"), + listOf("176", "178", "Glitch").parseId3GenreNames(",")) + } + + @Test + fun parseId3v2Genre_wackId3() { + assertEquals(listOf("2941"), listOf("2941").parseId3GenreNames(",")) + } + + @Test + fun parseId3v2Genre_singleId3v23() { + assertEquals( + listOf("Post-Rock", "Shoegaze", "Remix", "Cover", "Glitch"), + listOf("(176)(178)(RX)(CR)Glitch").parseId3GenreNames(",")) + } + + @Test + fun parseId3v2Genre_singleSeparated() { + assertEquals( + listOf("Post-Rock", "Shoegaze", "Glitch"), + listOf("Post-Rock, Shoegaze, Glitch").parseId3GenreNames(",")) + } + + @Test + fun parsId3v2Genre_singleId3v1() { + assertEquals(listOf("Post-Rock"), listOf("176").parseId3GenreNames(",")) + } }