music: decouple settings somewhat

Try to decouple the stateful music settings object from the stateless
internals of the music loader. This should make unit testing far
easier.

app/src/main/java/org/oxycblt/auxio/music/MusicRepository.kt

@@ -36,6 +36,8 @@ import org.oxycblt.auxio.music.cache.CacheRepository
 import org.oxycblt.auxio.music.device.DeviceLibrary
 import org.oxycblt.auxio.music.device.RawSong
 import org.oxycblt.auxio.music.fs.MediaStoreExtractor
+import org.oxycblt.auxio.music.info.Name
+import org.oxycblt.auxio.music.metadata.Separators
 import org.oxycblt.auxio.music.metadata.TagExtractor
 import org.oxycblt.auxio.music.user.MutableUserLibrary
 import org.oxycblt.auxio.music.user.UserLibrary
@@ -223,7 +225,8 @@ constructor(
     private val mediaStoreExtractor: MediaStoreExtractor,
     private val tagExtractor: TagExtractor,
     private val deviceLibraryFactory: DeviceLibrary.Factory,
-    private val userLibraryFactory: UserLibrary.Factory
+    private val userLibraryFactory: UserLibrary.Factory,
+    private val musicSettings: MusicSettings
 ) : MusicRepository {
     private val updateListeners = mutableListOf<MusicRepository.UpdateListener>()
     private val indexingListeners = mutableListOf<MusicRepository.IndexingListener>()
@@ -356,6 +359,8 @@ constructor(
     }
 
     private suspend fun indexImpl(worker: MusicRepository.IndexingWorker, withCache: Boolean) {
+        // TODO: Find a way to break up this monster of a method, preferably as another class.
+
         val start = System.currentTimeMillis()
         // Make sure we have permissions before going forward. Theoretically this would be better
         // done at the UI level, but that intertwines logic and display too much.
@@ -365,6 +370,17 @@ constructor(
             throw NoAudioPermissionException()
         }
 
+        // Obtain configuration information
+        val constraints =
+            MediaStoreExtractor.Constraints(musicSettings.excludeNonMusic, musicSettings.musicDirs)
+        val separators = Separators.from(musicSettings.separators)
+        val nameFactory =
+            if (musicSettings.intelligentSorting) {
+                Name.Known.IntelligentFactory
+            } else {
+                Name.Known.SimpleFactory
+            }
+
         // Begin with querying MediaStore and the music cache. The former is needed for Auxio
         // to figure out what songs are (probably) on the device, and the latter will be needed
         // for discovery (described later). These have no shared state, so they are done in
@@ -376,7 +392,7 @@ constructor(
             worker.scope.async {
                 val query =
                     try {
-                        mediaStoreExtractor.query()
+                        mediaStoreExtractor.query(constraints)
                     } catch (e: Exception) {
                         // Normally, errors in an async call immediately bubble up to the Looper
                         // and crash the app. Thus, we have to wrap any error into a Result
@@ -445,7 +461,8 @@ constructor(
             worker.scope.async(Dispatchers.Default) {
                 val deviceLibrary =
                     try {
-                        deviceLibraryFactory.create(completeSongs, processedSongs)
+                        deviceLibraryFactory.create(
+                            completeSongs, processedSongs, separators, nameFactory)
                     } catch (e: Exception) {
                         processedSongs.close(e)
                         return@async Result.failure(e)
@@ -518,7 +535,7 @@ constructor(
         logD("Awaiting DeviceLibrary creation")
         val deviceLibrary = deviceLibraryJob.await().getOrThrow()
         logD("Starting UserLibrary creation")
-        val userLibrary = userLibraryFactory.create(rawPlaylists, deviceLibrary)
+        val userLibrary = userLibraryFactory.create(rawPlaylists, deviceLibrary, nameFactory)
 
         // Loading process is functionally done, indicate such
         logD(

app/src/main/java/org/oxycblt/auxio/music/device/DeviceLibrary.kt

@@ -28,7 +28,6 @@ import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Genre
 import org.oxycblt.auxio.music.Music
 import org.oxycblt.auxio.music.MusicRepository
-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
@@ -110,19 +109,19 @@ interface DeviceLibrary {
         suspend fun create(
             rawSongs: Channel<RawSong>,
             processedSongs: Channel<RawSong>,
+            separators: Separators,
+            nameFactory: Name.Known.Factory
         ): DeviceLibraryImpl
     }
 }
 
-class DeviceLibraryFactoryImpl @Inject constructor(private val musicSettings: MusicSettings) :
-    DeviceLibrary.Factory {
+class DeviceLibraryFactoryImpl @Inject constructor() : DeviceLibrary.Factory {
     override suspend fun create(
         rawSongs: Channel<RawSong>,
-        processedSongs: Channel<RawSong>
+        processedSongs: Channel<RawSong>,
+        separators: Separators,
+        nameFactory: Name.Known.Factory
     ): DeviceLibraryImpl {
-        val nameFactory = Name.Known.Factory.from(musicSettings)
-        val separators = Separators.from(musicSettings)
-
         val songGrouping = mutableMapOf<Music.UID, SongImpl>()
         val albumGrouping = mutableMapOf<RawAlbum.Key, Grouping<RawAlbum, SongImpl>>()
         val artistGrouping = mutableMapOf<RawArtist.Key, Grouping<RawArtist, Music>>()

app/src/main/java/org/oxycblt/auxio/music/fs/FsModule.kt

@@ -24,12 +24,11 @@ import dagger.Provides
 import dagger.hilt.InstallIn
 import dagger.hilt.android.qualifiers.ApplicationContext
 import dagger.hilt.components.SingletonComponent
-import org.oxycblt.auxio.music.MusicSettings
 
 @Module
 @InstallIn(SingletonComponent::class)
 class FsModule {
     @Provides
-    fun mediaStoreExtractor(@ApplicationContext context: Context, musicSettings: MusicSettings) =
-        MediaStoreExtractor.from(context, musicSettings)
+    fun mediaStoreExtractor(@ApplicationContext context: Context) =
+        MediaStoreExtractor.from(context)
 }

app/src/main/java/org/oxycblt/auxio/music/fs/MediaStoreExtractor.kt

@@ -29,7 +29,6 @@ import androidx.core.database.getStringOrNull
 import java.io.File
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.yield
-import org.oxycblt.auxio.music.MusicSettings
 import org.oxycblt.auxio.music.cache.Cache
 import org.oxycblt.auxio.music.device.RawSong
 import org.oxycblt.auxio.music.info.Date
@@ -50,9 +49,11 @@ interface MediaStoreExtractor {
     /**
      * Query the media database.
      *
+     * @param constraints Configuration parameter to restrict what music should be ignored when
+     *   querying.
      * @return A new [Query] returned from the media database.
      */
-    suspend fun query(): Query
+    suspend fun query(constraints: Constraints): Query
 
     /**
      * Consume the [Cursor] loaded after [query].
@@ -84,46 +85,44 @@ interface MediaStoreExtractor {
         fun populateTags(rawSong: RawSong)
     }
 
+    data class Constraints(val excludeNonMusic: Boolean, val musicDirs: MusicDirectories)
+
     companion object {
         /**
          * Create a framework-backed instance.
          *
          * @param context [Context] required.
-         * @param musicSettings [MusicSettings] required.
          * @return A new [MediaStoreExtractor] that will work best on the device's API level.
          */
-        fun from(context: Context, musicSettings: MusicSettings): MediaStoreExtractor =
+        fun from(context: Context): MediaStoreExtractor =
             when {
-                Build.VERSION.SDK_INT >= Build.VERSION_CODES.R ->
-                    Api30MediaStoreExtractor(context, musicSettings)
-                Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q ->
-                    Api29MediaStoreExtractor(context, musicSettings)
-                else -> Api21MediaStoreExtractor(context, musicSettings)
+                Build.VERSION.SDK_INT >= Build.VERSION_CODES.R -> Api30MediaStoreExtractor(context)
+                Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q -> Api29MediaStoreExtractor(context)
+                else -> Api21MediaStoreExtractor(context)
             }
     }
 }
 
-private abstract class BaseMediaStoreExtractor(
-    protected val context: Context,
-    private val musicSettings: MusicSettings
-) : MediaStoreExtractor {
-    final override suspend fun query(): MediaStoreExtractor.Query {
+private abstract class BaseMediaStoreExtractor(protected val context: Context) :
+    MediaStoreExtractor {
+    final override suspend fun query(
+        constraints: MediaStoreExtractor.Constraints
+    ): MediaStoreExtractor.Query {
         val start = System.currentTimeMillis()
 
         val args = mutableListOf<String>()
         var selector = BASE_SELECTOR
 
         // Filter out audio that is not music, if enabled.
-        if (musicSettings.excludeNonMusic) {
+        if (constraints.excludeNonMusic) {
             logD("Excluding non-music")
             selector += " AND ${MediaStore.Audio.AudioColumns.IS_MUSIC}=1"
         }
 
         // Set up the projection to follow the music directory configuration.
-        val dirs = musicSettings.musicDirs
-        if (dirs.dirs.isNotEmpty()) {
+        if (constraints.musicDirs.dirs.isNotEmpty()) {
             selector += " AND "
-            if (!dirs.shouldInclude) {
+            if (!constraints.musicDirs.shouldInclude) {
                 logD("Excluding directories in selector")
                 // Without a NOT, the query will be restricted to the specified paths, resulting
                 // in the "Include" mode. With a NOT, the specified paths will not be included,
@@ -134,10 +133,10 @@ private abstract class BaseMediaStoreExtractor(
 
             // Specifying the paths to filter is version-specific, delegate to the concrete
             // implementations.
-            for (i in dirs.dirs.indices) {
-                if (addDirToSelector(dirs.dirs[i], args)) {
+            for (i in constraints.musicDirs.dirs.indices) {
+                if (addDirToSelector(constraints.musicDirs.dirs[i], args)) {
                     selector +=
-                        if (i < dirs.dirs.lastIndex) {
+                        if (i < constraints.musicDirs.dirs.lastIndex) {
                             "$dirSelectorTemplate OR "
                         } else {
                             dirSelectorTemplate
@@ -362,8 +361,7 @@ private abstract class BaseMediaStoreExtractor(
 // Note: The separation between version-specific backends may not be the cleanest. To preserve
 // speed, we only want to add redundancy on known issues, not with possible issues.
 
-private class Api21MediaStoreExtractor(context: Context, musicSettings: MusicSettings) :
-    BaseMediaStoreExtractor(context, musicSettings) {
+private class Api21MediaStoreExtractor(context: Context) : BaseMediaStoreExtractor(context) {
     override val projection: Array<String>
         get() =
             super.projection +
@@ -447,10 +445,8 @@ private class Api21MediaStoreExtractor(context: Context, musicSettings: MusicSet
  * @author Alexander Capehart (OxygenCobalt)
  */
 @RequiresApi(Build.VERSION_CODES.Q)
-private abstract class BaseApi29MediaStoreExtractor(
-    context: Context,
-    musicSettings: MusicSettings
-) : BaseMediaStoreExtractor(context, musicSettings) {
+private abstract class BaseApi29MediaStoreExtractor(context: Context) :
+    BaseMediaStoreExtractor(context) {
     override val projection: Array<String>
         get() =
             super.projection +
@@ -512,8 +508,7 @@ private abstract class BaseApi29MediaStoreExtractor(
  * @author Alexander Capehart (OxygenCobalt)
  */
 @RequiresApi(Build.VERSION_CODES.Q)
-private class Api29MediaStoreExtractor(context: Context, musicSettings: MusicSettings) :
-    BaseApi29MediaStoreExtractor(context, musicSettings) {
+private class Api29MediaStoreExtractor(context: Context) : BaseApi29MediaStoreExtractor(context) {
 
     override val projection: Array<String>
         get() = super.projection + arrayOf(MediaStore.Audio.AudioColumns.TRACK)
@@ -553,8 +548,7 @@ private class Api29MediaStoreExtractor(context: Context, musicSettings: MusicSet
  * @author Alexander Capehart (OxygenCobalt)
  */
 @RequiresApi(Build.VERSION_CODES.R)
-private class Api30MediaStoreExtractor(context: Context, musicSettings: MusicSettings) :
-    BaseApi29MediaStoreExtractor(context, musicSettings) {
+private class Api30MediaStoreExtractor(context: Context) : BaseApi29MediaStoreExtractor(context) {
     override val projection: Array<String>
         get() =
             super.projection +

app/src/main/java/org/oxycblt/auxio/music/info/Name.kt

@@ -23,12 +23,11 @@ import androidx.annotation.StringRes
 import androidx.annotation.VisibleForTesting
 import java.text.CollationKey
 import java.text.Collator
-import org.oxycblt.auxio.music.MusicSettings
 
 /**
  * The name of a music item.
  *
- * This class automatically implements
+ * This class automatically implements advanced sorting heuristics for music naming,
  *
  * @author Alexander Capehart
  */
@@ -80,30 +79,24 @@ sealed interface Name : Comparable<Name> {
                 is Unknown -> 1
             }
 
-        interface Factory {
+        sealed interface Factory {
             /**
              * Create a new instance of [Name.Known]
              *
              * @param raw The raw name obtained from the music item
              * @param sort The raw sort name obtained from the music item
              */
             fun parse(raw: String, sort: String?): Known
+        }
 
-            companion object {
-                /**
-                 * Creates a new instance from the **current state** of the given [MusicSettings]'s
-                 * user-defined name configuration.
-                 *
-                 * @param settings The [MusicSettings] to use.
-                 * @return A [Factory] instance reflecting the configuration state.
-                 */
-                fun from(settings: MusicSettings) =
-                    if (settings.intelligentSorting) {
-                        IntelligentKnownName.Factory
-                    } else {
-                        SimpleKnownName.Factory
-                    }
-            }
+        /** Produces a simple [Known] with basic sorting heuristics that are locale-independent. */
+        data object SimpleFactory : Factory {
+            override fun parse(raw: String, sort: String?) = SimpleKnownName(raw, sort)
+        }
+
+        /** Produces an intelligent [Known] with advanced, but more fragile heuristics. */
+        data object IntelligentFactory : Factory {
+            override fun parse(raw: String, sort: String?) = IntelligentKnownName(raw, sort)
         }
     }
 
@@ -137,7 +130,6 @@ private val punctRegex by lazy { Regex("[\\p{Punct}+]") }
  *
  * @author Alexander Capehart (OxygenCobalt)
  */
-@VisibleForTesting
 data class SimpleKnownName(override val raw: String, override val sort: String?) : Name.Known() {
     override val sortTokens = listOf(parseToken(sort ?: raw))
 
@@ -148,18 +140,13 @@ data class SimpleKnownName(override val raw: String, override val sort: String?)
         // Always use lexicographic mode since we aren't parsing any numeric components
         return SortToken(collationKey, SortToken.Type.LEXICOGRAPHIC)
     }
-
-    data object Factory : Name.Known.Factory {
-        override fun parse(raw: String, sort: String?) = SimpleKnownName(raw, sort)
-    }
 }
 
 /**
  * [Name.Known] implementation that adds advanced sorting behavior at the cost of localization.
  *
  * @author Alexander Capehart (OxygenCobalt)
  */
-@VisibleForTesting
 data class IntelligentKnownName(override val raw: String, override val sort: String?) :
     Name.Known() {
     override val sortTokens = parseTokens(sort ?: raw)
@@ -208,10 +195,6 @@ data class IntelligentKnownName(override val raw: String, override val sort: Str
         }
     }
 
-    data object Factory : Name.Known.Factory {
-        override fun parse(raw: String, sort: String?) = IntelligentKnownName(raw, sort)
-    }
-
     companion object {
         private val TOKEN_REGEX by lazy { Regex("(\\d+)|(\\D+)") }
     }

app/src/main/java/org/oxycblt/auxio/music/metadata/Separators.kt

@@ -18,9 +18,6 @@
 
 package org.oxycblt.auxio.music.metadata
 
-import androidx.annotation.VisibleForTesting
-import org.oxycblt.auxio.music.MusicSettings
-
 /**
  * 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.
@@ -45,15 +42,12 @@ interface Separators {
         const val AND = '&'
 
         /**
-         * Creates a new instance from the **current state** of the given [MusicSettings]'s
-         * user-defined separator configuration.
+         * Creates a new instance from a string of separator characters to use.
          *
-         * @param settings The [MusicSettings] to use.
-         * @return A new [Separators] instance reflecting the configuration state.
+         * @param chars The separator characters to use. Each character in the string will be
+         *   checked for when splitting a string list.
+         * @return A new [Separators] instance reflecting the separators.
          */
-        fun from(settings: MusicSettings) = from(settings.separators)
-
-        @VisibleForTesting
         fun from(chars: String) =
             if (chars.isNotEmpty()) {
                 CharSeparators(chars.toSet())