Cleanup nested functions on OfflineMusicService and make it return the correct MusicDirectory type

core/domain/src/main/kotlin/org/moire/ultrasonic/domain/MusicDirectory.kt

@@ -34,13 +34,13 @@ class MusicDirectory : ArrayList<MusicDirectory.Child>() {
 
     abstract class Child : Identifiable, GenericEntry() {
         abstract override var id: String
-        abstract val parent: String?
+        abstract var parent: String?
-        abstract val isDirectory: Boolean
+        abstract var isDirectory: Boolean
         abstract var album: String?
-        abstract val title: String?
+        abstract var title: String?
         abstract override val name: String?
         abstract val discNumber: Int?
-        abstract val coverArt: String?
+        abstract var coverArt: String?
         abstract val songCount: Long?
         abstract val created: Date?
         abstract var artist: String?
@@ -49,7 +49,7 @@ class MusicDirectory : ArrayList<MusicDirectory.Child>() {
         abstract val year: Int?
         abstract val genre: String?
         abstract var starred: Boolean
-        abstract val path: String?
+        abstract var path: String?
         abstract var closeness: Int
     }
 
@@ -115,12 +115,12 @@ class MusicDirectory : ArrayList<MusicDirectory.Child>() {
 
     data class Album(
         @PrimaryKey override var id: String,
-        override val parent: String? = null,
+        override var parent: String? = null,
         override var album: String? = null,
-        override val title: String? = null,
+        override var title: String? = null,
         override val name: String? = null,
         override val discNumber: Int = 0,
-        override val coverArt: String? = null,
+        override var coverArt: String? = null,
         override val songCount: Long? = null,
         override val created: Date? = null,
         override var artist: String? = null,
@@ -132,6 +132,6 @@ class MusicDirectory : ArrayList<MusicDirectory.Child>() {
         override var path: String? = null,
         override var closeness: Int = 0,
     ) : Child() {
-        override val isDirectory = true
+        override var isDirectory = true
     }
 }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/TrackCollectionFragment.kt

@@ -604,12 +604,12 @@ open class TrackCollectionFragment : MultiListFragment<MusicDirectory.Entry>() {
                 setTitle(name)
                 if (!isOffline() && Settings.shouldUseId3Tags) {
                     if (isAlbum) {
-                        listModel.getAlbum(refresh, id!!, name, parentId)
+                        listModel.getAlbum(refresh, id!!, name)
                     } else {
                         throw IllegalAccessException("Use AlbumFragment instead!")
                     }
                 } else {
-                    listModel.getMusicDirectory(refresh, id!!, name, parentId)
+                    listModel.getMusicDirectory(refresh, id!!, name)
                 }
             }
 

ultrasonic/src/main/kotlin/org/moire/ultrasonic/model/TrackCollectionModel.kt

@@ -30,8 +30,7 @@ class TrackCollectionModel(application: Application) : GenericListModel(applicat
     suspend fun getMusicDirectory(
         refresh: Boolean,
         id: String,
-        name: String?,
+        name: String?
-        parentId: String?
     ) {
         withContext(Dispatchers.IO) {
 
@@ -42,27 +41,7 @@ class TrackCollectionModel(application: Application) : GenericListModel(applicat
         }
     }
 
-    // Given a Music directory "songs" it recursively adds all children to "songs"
+    suspend fun getAlbum(refresh: Boolean, id: String, name: String?) {
-    @Suppress("unused")
-    private fun getSongsRecursively(
-        parent: MusicDirectory,
-        songs: MutableList<MusicDirectory.Entry>
-    ) {
-        val service = MusicServiceFactory.getMusicService()
-
-        for (song in parent.getTracks()) {
-            if (!song.isVideo && !song.isDirectory) {
-                songs.add(song)
-            }
-        }
-
-        for ((id1, _, _, title) in parent.getAlbums()) {
-            val root: MusicDirectory = service.getMusicDirectory(id1, title, false)
-            getSongsRecursively(root, songs)
-        }
-    }
-
-    suspend fun getAlbum(refresh: Boolean, id: String, name: String?, parentId: String?) {
 
         withContext(Dispatchers.IO) {
 

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/OfflineMusicService.kt

@@ -43,8 +43,6 @@ import org.moire.ultrasonic.util.FileUtil
 import org.moire.ultrasonic.util.Util
 import timber.log.Timber
 
-// TODO: There are quite a number of deeply nested and complicated functions in this class..
-// Simplify them :)
 @Suppress("TooManyFunctions")
 class OfflineMusicService : MusicService, KoinComponent {
     private val activeServerProvider: ActiveServerProvider by inject()
@@ -94,6 +92,9 @@ class OfflineMusicService : MusicService, KoinComponent {
         return indexes
     }
 
+    /*
+    * Especially when dealing with indexes, this method can return Albums, Entries or a mix of both!
+    */
     override fun getMusicDirectory(
         id: String,
         name: String?,
@@ -109,7 +110,11 @@ class OfflineMusicService : MusicService, KoinComponent {
             val filename = getName(file)
             if (filename != null && !seen.contains(filename)) {
                 seen.add(filename)
-                result.add(createEntry(file, filename))
+                if (file.isFile) {
+                    result.add(createEntry(file, filename))
+                } else {
+                    result.add(createAlbum(file, filename))
+                }
             }
         }
 
@@ -481,188 +486,204 @@ class OfflineMusicService : MusicService, KoinComponent {
         throw OfflineException("getPodcastsChannels isn't available in offline mode")
     }
 
-    companion object {
+    private fun getName(file: File): String? {
-        private val COMPILE = Pattern.compile(" ")
+        var name = file.name
-        private fun getName(file: File): String? {
+        if (file.isDirectory) {
-            var name = file.name
+            return name
-            if (file.isDirectory) {
-                return name
-            }
-            if (name.endsWith(".partial") || name.contains(".partial.") ||
-                name == Constants.ALBUM_ART_FILE
-            ) {
-                return null
-            }
-            name = name.replace(".complete", "")
-            return FileUtil.getBaseName(name)
         }
-
+        if (name.endsWith(".partial") || name.contains(".partial.") ||
-        @Suppress("TooGenericExceptionCaught", "ComplexMethod", "LongMethod", "NestedBlockDepth")
+            name == Constants.ALBUM_ART_FILE
-        private fun createEntry(file: File, name: String?): MusicDirectory.Child {
-            val entry = MusicDirectory.Entry(file.path)
-            entry.isDirectory = file.isDirectory
-            entry.parent = file.parent
-            entry.size = file.length()
-            val root = FileUtil.musicDirectory.path
-            entry.path = file.path.replaceFirst(
-                String.format(Locale.ROOT, "^%s/", root).toRegex(), ""
-            )
-            entry.title = name
-            if (file.isFile) {
-                var artist: String? = null
-                var album: String? = null
-                var title: String? = null
-                var track: String? = null
-                var disc: String? = null
-                var year: String? = null
-                var genre: String? = null
-                var duration: String? = null
-                var hasVideo: String? = null
-                try {
-                    val mmr = MediaMetadataRetriever()
-                    mmr.setDataSource(file.path)
-                    artist = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_ARTIST)
-                    album = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_ALBUM)
-                    title = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_TITLE)
-                    track = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_CD_TRACK_NUMBER)
-                    disc = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_DISC_NUMBER)
-                    year = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_YEAR)
-                    genre = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_GENRE)
-                    duration = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_DURATION)
-                    hasVideo = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_HAS_VIDEO)
-                    mmr.release()
-                } catch (ignored: Exception) {
-                }
-                entry.artist = artist ?: file.parentFile!!.parentFile!!.name
-                entry.album = album ?: file.parentFile!!.name
-                if (title != null) {
-                    entry.title = title
-                }
-                entry.isVideo = hasVideo != null
-                Timber.i("Offline Stuff: %s", track)
-                if (track != null) {
-                    var trackValue = 0
-                    try {
-                        val slashIndex = track.indexOf('/')
-                        if (slashIndex > 0) {
-                            track = track.substring(0, slashIndex)
-                        }
-                        trackValue = track.toInt()
-                    } catch (ex: Exception) {
-                        Timber.e(ex, "Offline Stuff")
-                    }
-                    Timber.i("Offline Stuff: Setting Track: %d", trackValue)
-                    entry.track = trackValue
-                }
-                if (disc != null) {
-                    var discValue = 0
-                    try {
-                        val slashIndex = disc.indexOf('/')
-                        if (slashIndex > 0) {
-                            disc = disc.substring(0, slashIndex)
-                        }
-                        discValue = disc.toInt()
-                    } catch (ignored: Exception) {
-                    }
-                    entry.discNumber = discValue
-                }
-                if (year != null) {
-                    var yearValue = 0
-                    try {
-                        yearValue = year.toInt()
-                    } catch (ignored: Exception) {
-                    }
-                    entry.year = yearValue
-                }
-                if (genre != null) {
-                    entry.genre = genre
-                }
-                if (duration != null) {
-                    var durationValue: Long = 0
-                    try {
-                        durationValue = duration.toLong()
-                        durationValue = TimeUnit.MILLISECONDS.toSeconds(durationValue)
-                    } catch (ignored: Exception) {
-                    }
-                    entry.setDuration(durationValue)
-                }
-            }
-            entry.suffix = FileUtil.getExtension(file.name.replace(".complete", ""))
-            val albumArt = FileUtil.getAlbumArtFile(entry)
-            if (albumArt.exists()) {
-                entry.coverArt = albumArt.path
-            }
-            return entry
-        }
-
-        @Suppress("NestedBlockDepth")
-        private fun recursiveAlbumSearch(
-            artistName: String,
-            file: File,
-            criteria: SearchCriteria,
-            albums: MutableList<MusicDirectory.Album>,
-            songs: MutableList<MusicDirectory.Entry>
         ) {
-            var closeness: Int
+            return null
-            for (albumFile in FileUtil.listMediaFiles(file)) {
+        }
-                if (albumFile.isDirectory) {
+        name = name.replace(".complete", "")
-                    val albumName = getName(albumFile)
+        return FileUtil.getBaseName(name)
-                    if (matchCriteria(criteria, albumName).also { closeness = it } > 0) {
+    }
-                        val album = createEntry(albumFile, albumName)
+
-                        album.artist = artistName
+    private fun createEntry(file: File, name: String?): MusicDirectory.Entry {
-                        album.closeness = closeness
+        val entry = MusicDirectory.Entry(file.path)
-                        albums.add(album as MusicDirectory.Album)
+        entry.populateWithDataFrom(file, name)
-                    }
+        return entry
-                    for (songFile in FileUtil.listMediaFiles(albumFile)) {
+    }
-                        val songName = getName(songFile)
+
-                        if (songFile.isDirectory) {
+    private fun createAlbum(file: File, name: String?): MusicDirectory.Album {
-                            recursiveAlbumSearch(artistName, songFile, criteria, albums, songs)
+        val album = MusicDirectory.Album(file.path)
-                        } else if (matchCriteria(criteria, songName).also { closeness = it } > 0) {
+        album.populateWithDataFrom(file, name)
-                            val song = createEntry(albumFile, songName)
+        return album
-                            song.artist = artistName
+    }
-                            song.album = albumName
+
-                            song.closeness = closeness
+
-                            songs.add(song as MusicDirectory.Entry)
+    /*
-                        }
+     * Extracts some basic data from a File object and applies it to an Album or Entry
-                    }
+     */
-                } else {
+    private fun MusicDirectory.Child.populateWithDataFrom(file: File, name: String?) {
-                    val songName = getName(albumFile)
+        isDirectory = file.isDirectory
-                    if (matchCriteria(criteria, songName).also { closeness = it } > 0) {
+        parent = file.parent
+        val root = FileUtil.musicDirectory.path
+        path = file.path.replaceFirst(
+            String.format(Locale.ROOT, "^%s/", root).toRegex(), ""
+        )
+        title = name
+
+        val albumArt = FileUtil.getAlbumArtFile(file)
+        if (albumArt.exists()) {
+            coverArt = albumArt.path
+        }
+    }
+
+
+    /*
+     * More extensive variant of Child.populateWithDataFrom(), which also parses the ID3 tags of
+     * a given track file.
+     */
+    private fun MusicDirectory.Entry.populateWithDataFrom(file: File, name: String?) {
+        (this as MusicDirectory.Child).populateWithDataFrom(file, name)
+
+        val meta = RawMetadata(null)
+
+        try {
+            val mmr = MediaMetadataRetriever()
+            mmr.setDataSource(file.path)
+            meta.artist = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_ARTIST)
+            meta.album = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_ALBUM)
+            meta.title = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_TITLE)
+            meta.track = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_CD_TRACK_NUMBER)
+            meta.disc = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_DISC_NUMBER)
+            meta.year = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_YEAR)
+            meta.genre = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_GENRE)
+            meta.duration = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_DURATION)
+            meta.hasVideo = mmr.extractMetadata(MediaMetadataRetriever.METADATA_KEY_HAS_VIDEO)
+            mmr.release()
+        } catch (ignored: Exception) {
+        }
+
+        artist = meta.artist ?: file.parentFile!!.parentFile!!.name
+        album = meta.album ?: file.parentFile!!.name
+        title = meta.title?: title
+        isVideo = meta.hasVideo != null
+        track = parseSlashedNumber(meta.track)
+        discNumber = parseSlashedNumber(meta.disc)
+        year = meta.year?.toIntOrNull()
+        genre = meta.genre
+        duration = parseDuration(meta.duration)
+        size = file.length()
+        suffix = FileUtil.getExtension(file.name.replace(".complete", ""))
+    }
+
+    /*
+     * Parses a number from a string in the format of 05/21,
+     * where the first number is the track number
+     * and the second the number of total tracks
+     */
+    private fun parseSlashedNumber(string: String?): Int? {
+        if (string == null) return null
+
+        val slashIndex = string.indexOf('/')
+        if (slashIndex > 0)
+            return string.substring(0, slashIndex).toIntOrNull()
+        else
+            return string.toIntOrNull()
+    }
+
+    /*
+     * Parses a duration from a String
+     */
+    private fun parseDuration(string: String?): Int? {
+        if (string == null) return null
+
+        val duration: Long? = string.toLongOrNull()
+
+        if (duration != null)
+            return TimeUnit.MILLISECONDS.toSeconds(duration).toInt()
+        else
+            return null
+    }
+
+    // TODO: Simplify this deeply nested and complicated function
+    @Suppress("NestedBlockDepth")
+    private fun recursiveAlbumSearch(
+        artistName: String,
+        file: File,
+        criteria: SearchCriteria,
+        albums: MutableList<MusicDirectory.Album>,
+        songs: MutableList<MusicDirectory.Entry>
+    ) {
+        var closeness: Int
+        for (albumFile in FileUtil.listMediaFiles(file)) {
+            if (albumFile.isDirectory) {
+                val albumName = getName(albumFile)
+                if (matchCriteria(criteria, albumName).also { closeness = it } > 0) {
+                    val album = createAlbum(albumFile, albumName)
+                    album.artist = artistName
+                    album.closeness = closeness
+                    albums.add(album)
+                }
+                for (songFile in FileUtil.listMediaFiles(albumFile)) {
+                    val songName = getName(songFile)
+                    if (songFile.isDirectory) {
+                        recursiveAlbumSearch(artistName, songFile, criteria, albums, songs)
+                    } else if (matchCriteria(criteria, songName).also { closeness = it } > 0) {
                         val song = createEntry(albumFile, songName)
                         song.artist = artistName
-                        song.album = songName
+                        song.album = albumName
                         song.closeness = closeness
-                        songs.add(song as MusicDirectory.Entry)
+                        songs.add(song)
                     }
                 }
-            }
+            } else {
-        }
+                val songName = getName(albumFile)
-
+                if (matchCriteria(criteria, songName).also { closeness = it } > 0) {
-        private fun matchCriteria(criteria: SearchCriteria, name: String?): Int {
+                    val song = createEntry(albumFile, songName)
-            val query = criteria.query.lowercase(Locale.ROOT)
+                    song.artist = artistName
-            val queryParts = COMPILE.split(query)
+                    song.album = songName
-            val nameParts = COMPILE.split(
+                    song.closeness = closeness
-                name!!.lowercase(Locale.ROOT)
+                    songs.add(song)
-            )
-            var closeness = 0
-            for (queryPart in queryParts) {
-                for (namePart in nameParts) {
-                    if (namePart == queryPart) {
-                        closeness++
-                    }
-                }
-            }
-            return closeness
-        }
-
-        private fun listFilesRecursively(parent: File, children: MutableList<File>) {
-            for (file in FileUtil.listMediaFiles(parent)) {
-                if (file.isFile) {
-                    children.add(file)
-                } else {
-                    listFilesRecursively(file, children)
                 }
             }
         }
     }
+
+    private fun matchCriteria(criteria: SearchCriteria, name: String?): Int {
+        val query = criteria.query.lowercase(Locale.ROOT)
+        val queryParts = COMPILE.split(query)
+        val nameParts = COMPILE.split(
+            name!!.lowercase(Locale.ROOT)
+        )
+        var closeness = 0
+        for (queryPart in queryParts) {
+            for (namePart in nameParts) {
+                if (namePart == queryPart) {
+                    closeness++
+                }
+            }
+        }
+        return closeness
+    }
+
+
+    private fun listFilesRecursively(parent: File, children: MutableList<File>) {
+        for (file in FileUtil.listMediaFiles(parent)) {
+            if (file.isFile) {
+                children.add(file)
+            } else {
+                listFilesRecursively(file, children)
+            }
+        }
+    }
+
+    data class RawMetadata(val id: String?) {
+        var artist: String? = null
+        var album: String? = null
+        var title: String? = null
+        var track: String? = null
+        var disc: String? = null
+        var year: String? = null
+        var genre: String? = null
+        var duration: String? = null
+        var hasVideo: String? = null
+    }
+
+    companion object {
+        private val COMPILE = Pattern.compile(" ")
+    }
 }