Improve music loading system

Heavily improve the music loading system, hopefully fixing some edge-cases that people have been reporting.

app/src/main/java/org/oxycblt/auxio/MainActivity.kt

@@ -52,7 +52,6 @@ class MainActivity : AppCompatActivity() {
         // Since the activity is set to singleInstance [Given that there's only MainActivity]
         // We have to manually push the intent whenever we get one so that the fragments
         // can catch any file intents
-        // FIXME: Centralize the file intent code in MainActivity, if thats even possible
         setIntent(intent)
     }
 

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

@@ -45,34 +45,20 @@ data class Song(
     private var mGenre: Genre? = null
 
     val genre: Genre? get() = mGenre
-    val album: Album get() {
+    val album: Album get() = requireNotNull(mAlbum)
-        val album = mAlbum
 
-        if (album != null) {
+    fun linkAlbum(album: Album) {
-            return album
-        } else {
-            error("Song $name must have an album")
-        }
-    }
-
-    /**
-     * Apply a genre to a song.
-     */
-    fun applyGenre(genre: Genre) {
-        if (mGenre == null) {
-            mGenre = genre
-        }
-    }
-
-    /**
-     * Apply an album to a song.
-     */
-    fun applyAlbum(album: Album) {
         if (mAlbum == null) {
             mAlbum = album
         }
     }
 
+    fun linkGenre(genre: Genre) {
+        if (mGenre == null) {
+            mGenre = genre
+        }
+    }
+
     val seconds = duration / 1000
     val formattedDuration: String = seconds.toDuration()
 }
@@ -95,37 +81,23 @@ data class Album(
     val year: Int = 0
 ) : Parent() {
     private var mArtist: Artist? = null
-    val artist: Artist get() {
+    val artist: Artist get() = requireNotNull(mArtist)
-        val artist = mArtist
-
-        if (artist != null) {
-            return artist
-        } else {
-            error("Album $name must have an artist")
-        }
-    }
 
     private val mSongs = mutableListOf<Song>()
     val songs: List<Song> get() = mSongs
 
-    val totalDuration: String by lazy {
+    val totalDuration: String get() = songs.sumOf { it.seconds }.toDuration()
-        var seconds: Long = 0
-        songs.forEach {
-            seconds += it.seconds
-        }
-        seconds.toDuration()
-    }
 
-    fun applySongs(songs: List<Song>) {
+    fun linkArtist(artist: Artist) {
-        songs.forEach {
-            it.applyAlbum(this)
-            mSongs.add(it)
-        }
-    }
-
-    fun applyArtist(artist: Artist) {
         mArtist = artist
     }
+
+    fun linkSongs(songs: List<Song>) {
+        for (song in songs) {
+            song.linkAlbum(this)
+            mSongs.add(song)
+        }
+    }
 }
 
 /**
@@ -141,25 +113,17 @@ data class Artist(
     val albums: List<Album>
 ) : Parent() {
     init {
-        albums.forEach {
+        albums.forEach { album ->
-            it.applyArtist(this)
+            album.linkArtist(this)
         }
     }
 
     val genre: Genre? by lazy {
-        val groupedGenres = songs.groupBy { it.genre }
+        songs.map { it.genre }.maxByOrNull { it?.songs?.size ?: 0 }
-
-        groupedGenres.keys.maxByOrNull { key ->
-            groupedGenres[key]?.size ?: 0
-        }
     }
 
     val songs: List<Song> by lazy {
-        val songs = mutableListOf<Song>()
+        albums.flatMap { it.songs }
-        albums.forEach {
-            songs.addAll(it.songs)
-        }
-        songs
     }
 }
 
@@ -184,17 +148,12 @@ data class Genre(
         }
     }
 
-    val totalDuration: String by lazy {
+    val totalDuration: String get() =
-        var seconds: Long = 0
+        songs.sumOf { it.seconds }.toDuration()
-        songs.forEach {
-            seconds += it.seconds
-        }
-        seconds.toDuration()
-    }
 
-    fun addSong(song: Song) {
+    fun linkSong(song: Song) {
         mSongs.add(song)
-        song.applyGenre(this)
+        song.linkGenre(this)
     }
 }
 

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

@@ -7,8 +7,10 @@ import android.provider.OpenableColumns
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import org.oxycblt.auxio.logD
+import org.oxycblt.auxio.logE
+import org.oxycblt.auxio.music.processing.MusicLinker
 import org.oxycblt.auxio.music.processing.MusicLoader
-import org.oxycblt.auxio.music.processing.MusicSorter
+import java.lang.Exception
 
 /**
  * The main storage for music items. Use [MusicStore.getInstance] to get the single instance of it.
@@ -50,28 +52,35 @@ class MusicStore private constructor() {
 
             val start = System.currentTimeMillis()
 
-            val loader = MusicLoader(app)
+            try {
-            val response = loader.loadMusic()
+                val loader = MusicLoader(app)
+                loader.loadMusic()
 
-            if (response == Response.SUCCESS) {
+                if (loader.songs.isEmpty()) {
-                // If the loading succeeds, then sort the songs and update the value
+                    return@withContext Response.NO_MUSIC
-                val sorter = MusicSorter(loader.songs, loader.albums)
+                }
 
-                sorter.sort()
+                val linker = MusicLinker(app, loader.songs, loader.albums, loader.genres)
+                linker.link()
 
-                mSongs = sorter.songs.toList()
+                mSongs = linker.songs.toList()
-                mAlbums = sorter.albums.toList()
+                mAlbums = linker.albums.toList()
-                mArtists = sorter.artists.toList()
+                mArtists = linker.artists.toList()
-                mGenres = loader.genres.toList()
+                mGenres = linker.genres.toList()
-
-                val elapsed = System.currentTimeMillis() - start
-
-                this@MusicStore.logD("Music load completed successfully in ${elapsed}ms.")
 
                 loaded = true
+
+                this@MusicStore.logD(
+                    "Music load completed successfully in ${System.currentTimeMillis() - start}ms."
+                )
+            } catch (e: Exception) {
+                logE("Something went horribly wrong.")
+                logE(e.stackTraceToString())
+
+                return@withContext Response.FAILED
             }
 
-            response
+            return@withContext Response.SUCCESS
         }
     }
 

app/src/main/java/org/oxycblt/auxio/music/processing/MusicLinker.kt

@@ -0,0 +1,116 @@
+package org.oxycblt.auxio.music.processing
+
+import android.content.Context
+import android.provider.MediaStore.Audio.Genres
+import org.oxycblt.auxio.R
+import org.oxycblt.auxio.logD
+import org.oxycblt.auxio.music.Album
+import org.oxycblt.auxio.music.Artist
+import org.oxycblt.auxio.music.Genre
+import org.oxycblt.auxio.music.Song
+
+/**
+ * Object that links music data to one-another,
+ */
+class MusicLinker(
+    private val context: Context,
+    val songs: MutableList<Song>,
+    val albums: MutableList<Album>,
+    val genres: MutableList<Genre>
+) {
+    private val resolver = context.contentResolver
+    val artists = mutableListOf<Artist>()
+
+    fun link() {
+        linkAlbums()
+        linkArtists()
+        linkGenres()
+    }
+
+    private fun linkAlbums() {
+        logD("Linking albums")
+
+        // Group up songs by their album ids and then link them with their albums
+        val songsByAlbum = songs.groupBy { it.albumId }
+        val unknownAlbum = Album(
+            name = context.getString(R.string.placeholder_album),
+            artistName = context.getString(R.string.placeholder_artist)
+        )
+
+        songsByAlbum.forEach { entry ->
+            (albums.find { it.id == entry.key } ?: unknownAlbum).linkSongs(entry.value)
+        }
+
+        // If something goes horribly wrong and somehow songs are still not linked up by the
+        // album id, just throw them into an unknown album.
+        if (unknownAlbum.songs.isNotEmpty()) {
+            albums.add(unknownAlbum)
+        }
+    }
+
+    private fun linkArtists() {
+        logD("Linking artists")
+
+        // Group albums up by their artist name, should not result in any null-artist issues
+        val albumsByArtist = albums.groupBy { it.artistName }
+
+        albumsByArtist.forEach { entry ->
+            artists.add(
+                Artist(
+                    id = (artists.size + Int.MIN_VALUE).toLong(),
+                    name = entry.key, albums = entry.value
+                )
+            )
+        }
+
+        logD("Albums successfully linked into ${artists.size} artists")
+    }
+
+    private fun linkGenres() {
+        logD("Linking genres")
+
+        /*
+         * Okay, I'm going to go on a bit of a tangent here, but why the hell do I have do this?
+         * In an ideal world I should just be able to write MediaStore.Media.Audio.GENRE in the
+         * original song projection and then have it fetch the genre from the database, but no,
+         * why would ANYONE do that? Instead, I have to manually PROJECT EACH GENRE, get their
+         * song ids, and then waste CPU cycles REPEATEDLY ITERATING through the songs list
+         * to LINK SONG WITH THEIR GENRE. I bet the google dev who built this busted system
+         * feels REALLY happy about the promotion they likely got from rushing out another
+         * android API that quickly rots from the basic act of existing, because now this quirk
+         * is immortalized and has to be replicated to be backwards compatible! Thanks for nothing!
+         */
+        genres.forEach { genre ->
+            val songCursor = resolver.query(
+                Genres.Members.getContentUri("external", genre.id),
+                arrayOf(Genres.Members._ID),
+                null, null, null
+            )
+
+            songCursor?.use { cursor ->
+                val idIndex = cursor.getColumnIndexOrThrow(Genres.Members._ID)
+
+                while (cursor.moveToNext()) {
+                    val id = cursor.getLong(idIndex)
+
+                    songs.find { it.id == id }?.let { song ->
+                        genre.linkSong(song)
+                    }
+                }
+            }
+        }
+
+        // Any songs without genres will be thrown into an unknown genre
+        val songsWithoutGenres = songs.filter { it.genre == null }
+
+        if (songsWithoutGenres.isNotEmpty()) {
+            val unknownGenre = Genre(name = context.getString(R.string.placeholder_genre))
+
+            songsWithoutGenres.forEach { song ->
+                unknownGenre.linkSong(song)
+            }
+
+            genres.add(unknownGenre)
+        }
+    }
+}

app/src/main/java/org/oxycblt/auxio/music/processing/MusicLoader.kt

@@ -9,10 +9,9 @@ import android.provider.MediaStore.Audio.Media
 import androidx.core.database.getStringOrNull
 import org.oxycblt.auxio.R
 import org.oxycblt.auxio.logD
-import org.oxycblt.auxio.logE
 import org.oxycblt.auxio.music.Album
+import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Genre
-import org.oxycblt.auxio.music.MusicStore
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.music.toAlbumArtURI
 
@@ -22,30 +21,16 @@ import org.oxycblt.auxio.music.toAlbumArtURI
  */
 class MusicLoader(private val app: Application) {
     var genres = mutableListOf<Genre>()
+    var artists = mutableListOf<Artist>()
     var albums = mutableListOf<Album>()
     var songs = mutableListOf<Song>()
 
     private val resolver = app.contentResolver
 
-    fun loadMusic(): MusicStore.Response {
+    fun loadMusic() {
-        try {
+        loadGenres()
-            loadGenres()
+        loadAlbums()
-            loadAlbums()
+        loadSongs()
-            loadSongs()
-        } catch (error: Exception) {
-            val trace = error.stackTraceToString()
-
-            logE("Something went horribly wrong.")
-            logE(trace)
-
-            return MusicStore.Response.FAILED
-        }
-
-        if (songs.isEmpty()) {
-            return MusicStore.Response.NO_MUSIC
-        }
-
-        return MusicStore.Response.SUCCESS
     }
 
     private fun loadGenres() {
@@ -174,46 +159,6 @@ class MusicLoader(private val app: Application) {
             it.name to it.albumId to it.track to it.duration
         }.toMutableList()
 
-        // Then try to associate any genres with their respective songs
-        // This is stupidly inefficient, but I don't have another choice really.
-        // Blame the android devs for deciding to design MediaStore this way.
-        for (genre in genres) {
-            val songGenreCursor = resolver.query(
-                Genres.Members.getContentUri("external", genre.id),
-                arrayOf(Genres.Members._ID),
-                null, null, null
-            )
-
-            songGenreCursor?.use { cursor ->
-                val idIndex = cursor.getColumnIndexOrThrow(Genres.Members._ID)
-
-                while (cursor.moveToNext()) {
-                    val songId = cursor.getLong(idIndex)
-
-                    songs.find { it.id == songId }?.let {
-                        genre.addSong(it)
-                    }
-                }
-            }
-        }
-
-        /*
-        // Fix that will group songs w/o genres into an unknown genre
-        // Currently disabled until it would actually benefit someone, otherwise its just
-        // a performance deadweight.
-        val songsWithoutGenres = songs.filter { it.genre == null }
-
-        if (songsWithoutGenres.isNotEmpty()) {
-            val unknownGenre = Genre(name = app.getString(R.string.placeholder_genre))
-
-            songsWithoutGenres.forEach {
-                unknownGenre.addSong(it)
-            }
-
-            genres.add(unknownGenre)
-        }
-         */
-
         logD("Song search finished with ${songs.size} found")
     }
 }

app/src/main/java/org/oxycblt/auxio/music/processing/MusicSorter.kt

@@ -1,44 +0,0 @@
-package org.oxycblt.auxio.music.processing
-
-import org.oxycblt.auxio.music.Album
-import org.oxycblt.auxio.music.Artist
-import org.oxycblt.auxio.music.Song
-
-/**
- * Object responsible for creating [Artist]s from [Album]s and generally sorting everything.
- */
-class MusicSorter(
-    val songs: MutableList<Song>,
-    val albums: MutableList<Album>,
-) {
-    val artists = mutableListOf<Artist>()
-
-    fun sort() {
-        albums.forEach {
-            groupSongsIntoAlbum(it)
-        }
-
-        createArtistsFromAlbums(albums)
-    }
-
-    private fun groupSongsIntoAlbum(album: Album) {
-        album.applySongs(songs.filter { it.albumId == album.id })
-    }
-
-    private fun createArtistsFromAlbums(albums: List<Album>) {
-        val groupedAlbums = albums.groupBy { it.artistName }
-
-        groupedAlbums.forEach {
-            artists.add(
-                // Min value is deliberately used to prevent conflicts with the MediaStore
-                // album & artist IDs. Shouldnt conflict with other negative IDs unless there
-                // are ~2.147 billion artists.
-                Artist(
-                    id = (artists.size + Int.MIN_VALUE).toLong(),
-                    name = it.key,
-                    albums = it.value
-                )
-            )
-        }
-    }
-}