musikr: refactor graphing

The sub-elements are still very deeply coupled, but that's to be
expected.

What this enables is reasonable testing of the graphing system, given
that it's easily the most brittle part of musikr.

musikr/src/main/java/org/oxycblt/musikr/graph/AlbumGraph.kt

@@ -22,27 +22,27 @@ import org.oxycblt.musikr.tag.interpret.PreAlbum
 import org.oxycblt.musikr.util.unlikelyToBeNull
 
 internal class AlbumGraph(private val artistGraph: ArtistGraph) {
-    val vertices = mutableMapOf<PreAlbum, AlbumVertex>()
+    private val vertices = mutableMapOf<PreAlbum, AlbumVertex>()
 
-    fun add(preAlbum: PreAlbum): AlbumVertex {
+    fun link(vertex: SongVertex) {
         // Albums themselves have their own parent artists that also need to be
         // linked up.
-        val albumartistGraph = preAlbum.preArtists.map { preArtist -> artistGraph.add(preArtist) }
+        val preAlbum = vertex.preSong.preAlbum
-        val albumVertex = AlbumVertex(preAlbum, albumartistGraph.toMutableList())
+        val albumVertex =
-        // Album vertex is linked, now link artists back to album.
+            vertices.getOrPut(preAlbum) { AlbumVertex(preAlbum).also { artistGraph.link(it) } }
-        albumartistGraph.forEach { artistVertex -> artistVertex.albumGraph.add(albumVertex) }
+        vertex.albumVertex = albumVertex
-        return albumVertex
+        albumVertex.songVertices.add(vertex)
     }
 
-    fun simplify() {
+    fun solve(): Collection<AlbumVertex> {
         val albumClusters = vertices.values.groupBy { it.preAlbum.rawName?.lowercase() }
         for (cluster in albumClusters.values) {
             simplifyAlbumCluster(cluster)
         }
-
         // Remove any edges that wound up connecting to the same artist or genre
         // in the end after simplification.
-        vertices.values.forEach { it.artistGraph = it.artistGraph.distinct().toMutableList() }
+        vertices.values.forEach { it.artistVertices = it.artistVertices.distinct().toMutableList() }
+        return vertices.values
     }
 
     private fun simplifyAlbumCluster(cluster: Collection<AlbumVertex>) {
@@ -65,9 +65,9 @@ internal class AlbumGraph(private val artistGraph: ArtistGraph) {
                 val noMbidPreAlbum = it.preAlbum.copy(musicBrainzId = null)
                 val simpleMbidVertex =
                     vertices.getOrPut(noMbidPreAlbum) {
-                        AlbumVertex(noMbidPreAlbum, it.artistGraph.toMutableList())
+                        AlbumVertex(noMbidPreAlbum).apply { artistVertices = it.artistVertices }
                     }
-                meldAlbumGraph(it, simpleMbidVertex)
+                meldAlbumVertices(it, simpleMbidVertex)
                 simpleMbidVertex
             }
         simplifyAlbumClusterImpl(strippedCluster)
@@ -84,31 +84,33 @@ internal class AlbumGraph(private val artistGraph: ArtistGraph) {
         val dst = clusterSet.maxBy { it.songVertices.size }
         clusterSet.remove(dst)
         for (src in clusterSet) {
-            meldAlbumGraph(src, dst)
+            meldAlbumVertices(src, dst)
         }
     }
 
-    private fun meldAlbumGraph(src: AlbumVertex, dst: AlbumVertex) {
+    private fun meldAlbumVertices(src: AlbumVertex, dst: AlbumVertex) {
         if (src == dst) {
             // Same vertex, do nothing
             return
         }
         // Link all songs and artists from the irrelevant album to the relevant album.
         dst.songVertices.addAll(src.songVertices)
-        dst.artistGraph.addAll(src.artistGraph)
+        dst.artistVertices.addAll(src.artistVertices)
         // Update all songs and artists to point to the relevant album.
         src.songVertices.forEach { it.albumVertex = dst }
-        src.artistGraph.forEach {
+        src.artistVertices.forEach {
-            it.albumGraph.remove(src)
+            it.albumVertices.remove(src)
-            it.albumGraph.add(dst)
+            it.albumVertices.add(dst)
         }
         // Remove the irrelevant album from the graph.
         vertices.remove(src.preAlbum)
     }
 }
 
-internal class AlbumVertex(val preAlbum: PreAlbum, var artistGraph: MutableList<ArtistVertex>) :
+internal class AlbumVertex(
-    Vertex {
+    val preAlbum: PreAlbum,
+) : Vertex {
+    var artistVertices = mutableListOf<ArtistVertex>()
     val songVertices = mutableSetOf<SongVertex>()
     override var tag: Any? = null
 

musikr/src/main/java/org/oxycblt/musikr/graph/ArtistGraph.kt

@@ -21,17 +21,29 @@ package org.oxycblt.musikr.graph
 import org.oxycblt.musikr.tag.interpret.PreArtist
 import org.oxycblt.musikr.util.unlikelyToBeNull
 
-internal class ArtistGraph() {
+internal class ArtistGraph {
-    val vertices = mutableMapOf<PreArtist, ArtistVertex>()
+    private val vertices = mutableMapOf<PreArtist, ArtistVertex>()
 
-    fun add(preArtist: PreArtist): ArtistVertex =
+    fun link(songVertex: SongVertex) {
-        vertices.getOrPut(preArtist) { ArtistVertex(preArtist) }
+        val preArtists = songVertex.preSong.preArtists
+        val artistVertices = preArtists.map { vertices.getOrPut(it) { ArtistVertex(it) } }
+        songVertex.artistVertices.addAll(artistVertices)
+        artistVertices.forEach { it.songVertices.add(songVertex) }
+    }
 
-    fun simplify() {
+    fun link(albumVertex: AlbumVertex) {
+        val preArtists = albumVertex.preAlbum.preArtists
+        val artistVertices = preArtists.map { vertices.getOrPut(it) { ArtistVertex(it) } }
+        albumVertex.artistVertices = artistVertices.toMutableList()
+        artistVertices.forEach { it.albumVertices.add(albumVertex) }
+    }
+
+    fun solve(): Collection<ArtistVertex> {
         val artistClusters = vertices.values.groupBy { it.preArtist.rawName?.lowercase() }
         for (cluster in artistClusters.values) {
             simplifyArtistCluster(cluster)
         }
+        return vertices.values
     }
 
     private fun simplifyArtistCluster(cluster: Collection<ArtistVertex>) {
@@ -54,7 +66,7 @@ internal class ArtistGraph() {
                 val noMbidPreArtist = it.preArtist.copy(musicBrainzId = null)
                 val simpleMbidVertex =
                     vertices.getOrPut(noMbidPreArtist) { ArtistVertex(noMbidPreArtist) }
-                meldArtistGraph(it, simpleMbidVertex)
+                meldArtistVertices(it, simpleMbidVertex)
                 simpleMbidVertex
             }
         simplifyArtistClusterImpl(strippedCluster)
@@ -69,33 +81,33 @@ internal class ArtistGraph() {
         val relevantArtistVertex = clusterSet.maxBy { it.songVertices.size }
         clusterSet.remove(relevantArtistVertex)
         for (irrelevantArtistVertex in clusterSet) {
-            meldArtistGraph(irrelevantArtistVertex, relevantArtistVertex)
+            meldArtistVertices(irrelevantArtistVertex, relevantArtistVertex)
         }
     }
 
-    private fun meldArtistGraph(src: ArtistVertex, dst: ArtistVertex) {
+    private fun meldArtistVertices(src: ArtistVertex, dst: ArtistVertex) {
         if (src == dst) {
             // Same vertex, do nothing
             return
         }
         // Link all songs and albums from the irrelevant artist to the relevant artist.
         dst.songVertices.addAll(src.songVertices)
-        dst.albumGraph.addAll(src.albumGraph)
+        dst.albumVertices.addAll(src.albumVertices)
-        dst.genreGraph.addAll(src.genreGraph)
+        dst.genreVertices.addAll(src.genreVertices)
         // Update all songs, albums, and genres to point to the relevant artist.
         src.songVertices.forEach {
-            val index = it.artistGraph.indexOf(src)
+            val index = it.artistVertices.indexOf(src)
             check(index >= 0) { "Illegal state: directed edge between artist and song" }
-            it.artistGraph[index] = dst
+            it.artistVertices[index] = dst
         }
-        src.albumGraph.forEach {
+        src.albumVertices.forEach {
-            val index = it.artistGraph.indexOf(src)
+            val index = it.artistVertices.indexOf(src)
             check(index >= 0) { "Illegal state: directed edge between artist and album" }
-            it.artistGraph[index] = dst
+            it.artistVertices[index] = dst
         }
-        src.genreGraph.forEach {
+        src.genreVertices.forEach {
-            it.artistGraph.remove(src)
+            it.artistVertices.remove(src)
-            it.artistGraph.add(dst)
+            it.artistVertices.add(dst)
         }
 
         // Remove the irrelevant artist from the graph.
@@ -107,8 +119,8 @@ internal class ArtistVertex(
     val preArtist: PreArtist,
 ) : Vertex {
     val songVertices = mutableSetOf<SongVertex>()
-    val albumGraph = mutableSetOf<AlbumVertex>()
+    val albumVertices = mutableSetOf<AlbumVertex>()
-    val genreGraph = mutableSetOf<GenreVertex>()
+    val genreVertices = mutableSetOf<GenreVertex>()
     override var tag: Any? = null
 
     override fun toString() = "ArtistVertex(preArtist=$preArtist)"

musikr/src/main/java/org/oxycblt/musikr/graph/GenreGraph.kt

@@ -21,15 +21,30 @@ package org.oxycblt.musikr.graph
 import org.oxycblt.musikr.tag.interpret.PreGenre
 
 internal class GenreGraph {
-    val vertices = mutableMapOf<PreGenre, GenreVertex>()
+    private val vertices = mutableMapOf<PreGenre, GenreVertex>()
 
-    fun add(preGenre: PreGenre): GenreVertex = vertices.getOrPut(preGenre) { GenreVertex(preGenre) }
+    fun link(vertex: SongVertex) {
+        val preGenres = vertex.preSong.preGenres
+        val artistVertices = vertex.artistVertices
 
-    fun simplify() {
+        for (preGenre in preGenres) {
+            val genreVertex = vertices.getOrPut(preGenre) { GenreVertex(preGenre) }
+            vertex.genreVertices.add(genreVertex)
+            genreVertex.songVertices.add(vertex)
+
+            for (artistVertex in artistVertices) {
+                genreVertex.artistVertices.add(artistVertex)
+                artistVertex.genreVertices.add(genreVertex)
+            }
+        }
+    }
+
+    fun solve(): Collection<GenreVertex> {
         val genreClusters = vertices.values.groupBy { it.preGenre.rawName?.lowercase() }
         for (cluster in genreClusters.values) {
             simplifyGenreCluster(cluster)
         }
+        return vertices.values
     }
 
     private fun simplifyGenreCluster(cluster: Collection<GenreVertex>) {
@@ -43,27 +58,27 @@ internal class GenreGraph {
         val dst = clusterSet.maxBy { it.songVertices.size }
         clusterSet.remove(dst)
         for (src in clusterSet) {
-            meldGenreGraph(src, dst)
+            meldGenreVertices(src, dst)
         }
     }
 
-    private fun meldGenreGraph(src: GenreVertex, dst: GenreVertex) {
+    private fun meldGenreVertices(src: GenreVertex, dst: GenreVertex) {
         if (src == dst) {
             // Same vertex, do nothing
             return
         }
         // Link all songs and artists from the irrelevant genre to the relevant genre.
         dst.songVertices.addAll(src.songVertices)
-        dst.artistGraph.addAll(src.artistGraph)
+        dst.artistVertices.addAll(src.artistVertices)
         // Update all songs and artists to point to the relevant genre.
         src.songVertices.forEach {
-            val index = it.genreGraph.indexOf(src)
+            val index = it.genreVertices.indexOf(src)
             check(index >= 0) { "Illegal state: directed edge between genre and song" }
-            it.genreGraph[index] = dst
+            it.genreVertices[index] = dst
         }
-        src.artistGraph.forEach {
+        src.artistVertices.forEach {
-            it.genreGraph.remove(src)
+            it.genreVertices.remove(src)
-            it.genreGraph.add(dst)
+            it.genreVertices.add(dst)
         }
         // Remove the irrelevant genre from the graph.
         vertices.remove(src.preGenre)
@@ -72,7 +87,7 @@ internal class GenreGraph {
 
 internal class GenreVertex(val preGenre: PreGenre) : Vertex {
     val songVertices = mutableSetOf<SongVertex>()
-    val artistGraph = mutableSetOf<ArtistVertex>()
+    val artistVertices = mutableSetOf<ArtistVertex>()
     override var tag: Any? = null
 
     override fun toString() = "GenreVertex(preGenre=$preGenre)"

musikr/src/main/java/org/oxycblt/musikr/graph/MusicGraph.kt

@@ -22,11 +22,11 @@ import org.oxycblt.musikr.playlist.interpret.PrePlaylist
 import org.oxycblt.musikr.tag.interpret.PreSong
 
 internal data class MusicGraph(
-    val songVertex: List<SongVertex>,
+    val songVertices: Collection<SongVertex>,
-    val albumVertex: List<AlbumVertex>,
+    val albumVertices: Collection<AlbumVertex>,
-    val artistVertex: List<ArtistVertex>,
+    val artistVertices: Collection<ArtistVertex>,
-    val genreVertex: List<GenreVertex>,
+    val genreVertices: Collection<GenreVertex>,
-    val playlistVertex: Set<PlaylistVertex>
+    val playlistVertices: Collection<PlaylistVertex>
 ) {
     interface Builder {
         fun add(preSong: PreSong)
@@ -41,35 +41,32 @@ internal data class MusicGraph(
     }
 }
 
+internal interface Vertex {
+    val tag: Any?
+}
+
 private class MusicGraphBuilderImpl : MusicGraph.Builder {
     private val genreGraph = GenreGraph()
     private val artistGraph = ArtistGraph()
     private val albumGraph = AlbumGraph(artistGraph)
     private val playlistGraph = PlaylistGraph()
-    private val songVertices = SongGraph(albumGraph, artistGraph, genreGraph, playlistGraph)
+    private val songGraph = SongGraph(albumGraph, artistGraph, genreGraph, playlistGraph)
 
     override fun add(preSong: PreSong) {
-        songVertices.add(preSong)
+        songGraph.link(SongVertex(preSong))
     }
 
     override fun add(prePlaylist: PrePlaylist) {
-        playlistGraph.add(prePlaylist)
+        playlistGraph.link(PlaylistVertex(prePlaylist))
     }
 
     override fun build(): MusicGraph {
-        genreGraph.simplify()
+        val genreVertices = genreGraph.solve()
-        artistGraph.simplify()
+        val artistVertices = artistGraph.solve()
-        albumGraph.simplify()
+        val albumVertices = albumGraph.solve()
-        songVertices.simplify()
+        val songVertices = songGraph.solve()
-
+        val playlistVertices = playlistGraph.solve()
-        val graph =
+        return MusicGraph(
-            MusicGraph(
+            songVertices, albumVertices, artistVertices, genreVertices, playlistVertices)
-                songVertices.vertices.values.toList(),
-                albumGraph.vertices.values.toList(),
-                artistGraph.vertices.values.toList(),
-                genreGraph.vertices.values.toList(),
-                playlistGraph.vertices)
-
-        return graph
     }
 }

musikr/src/main/java/org/oxycblt/musikr/graph/PlaylistGraph.kt

@@ -18,14 +18,30 @@
  
 package org.oxycblt.musikr.graph
 
+import org.oxycblt.musikr.playlist.SongPointer
 import org.oxycblt.musikr.playlist.interpret.PrePlaylist
 
 internal class PlaylistGraph {
-    val vertices = mutableSetOf<PlaylistVertex>()
+    private val pointerMap = mutableMapOf<SongPointer, SongVertex>()
+    private val vertices = mutableSetOf<PlaylistVertex>()
 
-    fun add(prePlaylist: PrePlaylist) {
+    fun link(vertex: PlaylistVertex) {
-        vertices.add(PlaylistVertex(prePlaylist))
+        for ((pointer, songVertex) in pointerMap) {
+            vertex.pointerMap[pointer]?.forEach { index -> vertex.songVertices[index] = songVertex }
         }
+    }
+
+    fun link(vertex: SongVertex) {
+        val pointer = SongPointer.UID(vertex.preSong.uid)
+        pointerMap[pointer] = vertex
+        for (playlistVertex in vertices) {
+            playlistVertex.pointerMap[pointer]?.forEach { index ->
+                playlistVertex.songVertices[index] = vertex
+            }
+        }
+    }
+
+    fun solve(): Collection<PlaylistVertex> = vertices
 }
 
 internal class PlaylistVertex(val prePlaylist: PrePlaylist) {
@@ -33,7 +49,7 @@ internal class PlaylistVertex(val prePlaylist: PrePlaylist) {
     val pointerMap =
         prePlaylist.songPointers
             .withIndex()
-            .associateBy { it.value }
+            .groupBy { it.value }
-            .mapValuesTo(mutableMapOf()) { it.value.index }
+            .mapValuesTo(mutableMapOf()) { indexed -> indexed.value.map { it.index } }
     val tag: Any? = null
 }

musikr/src/main/java/org/oxycblt/musikr/graph/SongGraph.kt

@@ -19,7 +19,6 @@
 package org.oxycblt.musikr.graph
 
 import org.oxycblt.musikr.Music
-import org.oxycblt.musikr.playlist.SongPointer
 import org.oxycblt.musikr.tag.interpret.PreSong
 
 internal class SongGraph(
@@ -28,66 +27,38 @@ internal class SongGraph(
     private val genreGraph: GenreGraph,
     private val playlistGraph: PlaylistGraph
 ) {
-    val vertices = mutableMapOf<Music.UID, SongVertex>()
+    private val vertices = mutableMapOf<Music.UID, SongVertex>()
 
-    fun add(preSong: PreSong): SongVertex? {
+    fun link(vertex: SongVertex): Boolean {
+        val preSong = vertex.preSong
         val uid = preSong.uid
         if (vertices.containsKey(uid)) {
-            return null
+            return false
+        }
+        artistGraph.link(vertex)
+        genreGraph.link(vertex)
+        albumGraph.link(vertex)
+        playlistGraph.link(vertex)
+        vertices[uid] = vertex
+        return true
     }
 
-        val songGenreVertices = preSong.preGenres.map { preGenre -> genreGraph.add(preGenre) }
+    fun solve(): Collection<SongVertex> {
-
-        val songArtistVertices = preSong.preArtists.map { preArtist -> artistGraph.add(preArtist) }
-
-        val albumVertex = albumGraph.add(preSong.preAlbum)
-
-        val songVertex =
-            SongVertex(
-                preSong,
-                albumVertex,
-                songArtistVertices.toMutableList(),
-                songGenreVertices.toMutableList())
-
-        songVertex.artistGraph.forEach { artistVertex ->
-            artistVertex.songVertices.add(songVertex)
-            songGenreVertices.forEach { genreVertex ->
-                // Mutually link any new genres to the artist
-                artistVertex.genreGraph.add(genreVertex)
-                genreVertex.artistGraph.add(artistVertex)
-            }
-        }
-
-        songVertex.genreGraph.forEach { genreVertex -> genreVertex.songVertices.add(songVertex) }
-
-        vertices[uid] = songVertex
-
-        return songVertex
-    }
-
-    fun simplify() {
         vertices.entries.forEach { entry ->
             val vertex = entry.value
-            vertex.artistGraph = vertex.artistGraph.distinct().toMutableList()
+            vertex.artistVertices = vertex.artistVertices.distinct().toMutableList()
-            vertex.genreGraph = vertex.genreGraph.distinct().toMutableList()
+            vertex.genreVertices = vertex.genreVertices.distinct().toMutableList()
-
-            playlistGraph.vertices.forEach {
-                val pointer = SongPointer.UID(entry.key)
-                val index = it.pointerMap[pointer]
-                if (index != null) {
-                    it.songVertices[index] = vertex
-                }
-            }
         }
+        return vertices.values
     }
 }
 
 internal class SongVertex(
     val preSong: PreSong,
-    var albumVertex: AlbumVertex,
-    var artistGraph: MutableList<ArtistVertex>,
-    var genreGraph: MutableList<GenreVertex>
 ) : Vertex {
+    var albumVertex: AlbumVertex? = null
+    var artistVertices = mutableListOf<ArtistVertex>()
+    var genreVertices = mutableListOf<GenreVertex>()
     override var tag: Any? = null
 
     override fun toString() = "SongVertex(preSong=$preSong)"

musikr/src/main/java/org/oxycblt/musikr/model/LibraryFactory.kt

@@ -53,23 +53,23 @@ private class LibraryFactoryImpl() : LibraryFactory {
         playlistInterpreter: PlaylistInterpreter
     ): MutableLibrary {
         val songs =
-            graph.songVertex.mapTo(mutableSetOf()) { vertex ->
+            graph.songVertices.mapTo(mutableSetOf()) { vertex ->
                 SongImpl(SongVertexCore(vertex)).also { vertex.tag = it }
             }
         val albums =
-            graph.albumVertex.mapTo(mutableSetOf()) { vertex ->
+            graph.albumVertices.mapTo(mutableSetOf()) { vertex ->
                 AlbumImpl(AlbumVertexCore(vertex)).also { vertex.tag = it }
             }
         val artists =
-            graph.artistVertex.mapTo(mutableSetOf()) { vertex ->
+            graph.artistVertices.mapTo(mutableSetOf()) { vertex ->
                 ArtistImpl(ArtistVertexCore(vertex)).also { vertex.tag = it }
             }
         val genres =
-            graph.genreVertex.mapTo(mutableSetOf()) { vertex ->
+            graph.genreVertices.mapTo(mutableSetOf()) { vertex ->
                 GenreImpl(GenreVertexCore(vertex)).also { vertex.tag = it }
             }
         val playlists =
-            graph.playlistVertex.mapTo(mutableSetOf()) { vertex ->
+            graph.playlistVertices.mapTo(mutableSetOf()) { vertex ->
                 PlaylistImpl(PlaylistVertexCore(vertex))
             }
         return LibraryImpl(
@@ -121,8 +121,8 @@ private class LibraryFactoryImpl() : LibraryFactory {
     }
 
     private companion object {
-        private inline fun <reified T : Music> tag(vertex: Vertex): T {
+        private inline fun <reified T : Music> tag(vertex: Vertex?): T {
-            val tag = vertex.tag
+            val tag = vertex?.tag
             check(tag is T) { "Dead Vertex Detected: $vertex" }
             return tag
         }

musikr/src/main/java/org/oxycblt/musikr/pipeline/EvaluateStep.kt

@@ -26,6 +26,8 @@ import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.flowOn
 import kotlinx.coroutines.flow.merge
 import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import org.oxycblt.musikr.Interpretation
 import org.oxycblt.musikr.MutableLibrary
 import org.oxycblt.musikr.Storage
@@ -78,11 +80,16 @@ private class EvaluateStepImpl(
                 .flowOn(Dispatchers.Default)
                 .buffer(Channel.UNLIMITED)
         val graphBuilder = MusicGraph.builder()
+        // Concurrent addition of playlists and songs could easily
+        // break the grapher (remember, individual pipeline elements
+        // are generally unaware of the highly concurrent nature of
+        // the pipeline), prevent that with a mutex.
+        val graphLock = Mutex()
         val graphBuild =
             merge(
                 filterFlow.manager,
-                preSongs.onEach { graphBuilder.add(it) },
+                preSongs.onEach { graphLock.withLock { graphBuilder.add(it) } },
-                prePlaylists.onEach { graphBuilder.add(it) })
+                prePlaylists.onEach { graphLock.withLock { graphBuilder.add(it) } })
         graphBuild.collect()
         logger.d("starting graph build")
         val graph = graphBuilder.build()