fix plugin manager not deleting old plugin version (#991)

* read plugin version & name from manifest instead of file name in plugin manager

* close JarFile correctly

Author: topi314

LavalinkServer/src/main/java/lavalink/server/bootstrap/PluginManager.kt

@@ -10,15 +10,16 @@ import java.io.InputStream
 import java.net.URL
 import java.net.URLClassLoader
 import java.nio.channels.Channels
-import java.nio.file.Files
 import java.util.*
 import java.util.jar.JarFile
-import java.util.regex.Pattern
-
 
 @SpringBootApplication
 class PluginManager(val config: PluginsConfig) {
 
+    companion object {
+        private val log: Logger = LoggerFactory.getLogger(PluginManager::class.java)
+    }
+
     final val pluginManifests: MutableList<PluginManifest> = mutableListOf()
     var classLoader: ClassLoader = PluginManager::class.java.classLoader
 
@@ -35,14 +36,13 @@ class PluginManager(val config: PluginsConfig) {
         val directory = File(config.pluginsDir)
         directory.mkdir()
 
-        data class PluginJar(val name: String, val version: String, val file: File)
+        data class PluginJar(val manifest: PluginManifest, val file: File)
 
-        val pattern = Pattern.compile("(.+)-(.+)\\.jar$")
-        val pluginJars = directory.listFiles()!!.mapNotNull { f ->
-            val matcher = pattern.matcher(f.name)
-            if (!matcher.find()) return@mapNotNull null
-            PluginJar(matcher.group(1), matcher.group(2), f)
-        }
+        val pluginJars = directory.listFiles()!!.filter { it.extension == "jar" }.map {
+            JarFile(it).use {jar ->
+                loadPluginManifests(jar).map { manifest -> PluginJar(manifest, it) }
+            }
+        }.flatten()
 
         data class Declaration(val group: String, val name: String, val version: String, val repository: String)
 
@@ -55,21 +55,22 @@ class PluginManager(val config: PluginsConfig) {
                 ?: if (declaration.snapshot) config.defaultPluginSnapshotRepository else config.defaultPluginRepository
             repository = if (repository.endsWith("/")) repository else "$repository/"
             Declaration(fragments[0], fragments[1], fragments[2], repository)
-        }
+        }.distinctBy { "${it.group}:${it.name}" }
 
         declarations.forEach declarationLoop@{ declaration ->
-            pluginJars.forEach { jar ->
-                if (declaration.name == jar.name) {
-                    if (declaration.version == jar.version) {
-                        // We already have this jar so don't redownload it
-                        return@declarationLoop
-                    }
-
-                    // Delete jar of different versions
-                    if (!jar.file.delete()) throw RuntimeException("Failed to delete ${jar.file.path}")
-                    log.info("Deleted ${jar.file.path}")
+            var hasVersion = false
+            pluginJars.forEach pluginLoop@{ jar ->
+                if (declaration.version == jar.manifest.version && !hasVersion) {
+                    hasVersion = true
+                    // We already have this jar so don't redownload it
+                    return@pluginLoop
                 }
+
+                // Delete jar of different versions
+                if (!jar.file.delete()) throw RuntimeException("Failed to delete ${jar.file.path}")
+                log.info("Deleted ${jar.file.path}")
             }
+            if (hasVersion) return@declarationLoop
 
             val url = declaration.run { "$repository${group.replace(".", "/")}/$name/$version/$name-$version.jar" }
             val file = File(directory, declaration.run { "$name-$version.jar" })
@@ -87,16 +88,19 @@ class PluginManager(val config: PluginsConfig) {
     private fun readClasspathManifests(): List<PluginManifest> {
         return PathMatchingResourcePatternResolver()
             .getResources("classpath*:lavalink-plugins/*.properties")
-            .map { r -> parsePluginManifest(r.inputStream) }
+            .map map@{ r ->
+                val manifest = parsePluginManifest(r.inputStream)
+                log.info("Found plugin '${manifest.name}' version ${manifest.version}")
+                return@map manifest
+            }
     }
 
     private fun loadJars(): List<PluginManifest> {
         val directory = File(config.pluginsDir)
         if (!directory.isDirectory) return emptyList()
         val jarsToLoad = mutableListOf<File>()
 
-        Files.list(File(config.pluginsDir).toPath()).forEach { path ->
-            val file = path.toFile()
+        directory.listFiles()?.forEach { file ->
             if (!file.isFile) return@forEach
             if (file.extension != "jar") return@forEach
             jarsToLoad.add(file)
@@ -111,7 +115,6 @@ class PluginManager(val config: PluginsConfig) {
         classLoader = cl
 
         val manifests = mutableListOf<PluginManifest>()
-
         jarsToLoad.forEach { file ->
             try {
                 manifests.addAll(loadJar(file, cl))
@@ -124,31 +127,43 @@ class PluginManager(val config: PluginsConfig) {
         return manifests
     }
 
-    private fun loadJar(file: File, cl: URLClassLoader): MutableList<PluginManifest> {
+    private fun loadJar(file: File, cl: URLClassLoader): List<PluginManifest> {
         var classCount = 0
         val jar = JarFile(file)
+        var manifests:  List<PluginManifest>
+
+        jar.use {
+            manifests = loadPluginManifests(jar)
+            if (manifests.isEmpty()) {
+                throw RuntimeException("No plugin manifest found in ${file.path}")
+            }
+            val allowedPaths = manifests.map { it.path.replace(".", "/") }
+
+            jar.entries().asIterator().forEach { entry ->
+                if (entry.isDirectory) return@forEach
+                if (!entry.name.endsWith(".class")) return@forEach
+                if (!allowedPaths.any { entry.name.startsWith(it) }) return@forEach
+                cl.loadClass(entry.name.dropLast(6).replace("/", "."))
+                classCount++
+            }
+        }
+
+        log.info("Loaded ${file.name} ($classCount classes)")
+        return manifests
+    }
+
+    private fun loadPluginManifests(jar: JarFile): List<PluginManifest> {
         val manifests = mutableListOf<PluginManifest>()
 
         jar.entries().asIterator().forEach { entry ->
             if (entry.isDirectory) return@forEach
             if (!entry.name.startsWith("lavalink-plugins/")) return@forEach
             if (!entry.name.endsWith(".properties")) return@forEach
-            manifests.add(parsePluginManifest(jar.getInputStream(entry)))
-        }
 
-        if (manifests.isEmpty()) {
-            throw RuntimeException("No plugin manifest found in ${file.path}")
+            val manifest = parsePluginManifest(jar.getInputStream(entry))
+            log.info("Found plugin '${manifest.name}' version ${manifest.version}")
+            manifests.add(manifest)
         }
-        val allowedPaths = manifests.map { it.path.replace(".", "/") }
-
-        jar.entries().asIterator().forEach { entry ->
-            if (entry.isDirectory) return@forEach
-            if (!entry.name.endsWith(".class")) return@forEach
-            if (!allowedPaths.any { entry.name.startsWith(it) }) return@forEach
-            cl.loadClass(entry.name.dropLast(6).replace("/", "."))
-            classCount++
-        }
-        log.info("Loaded ${file.name} ($classCount classes)")
         return manifests
     }
 
@@ -160,11 +175,6 @@ class PluginManager(val config: PluginsConfig) {
         val name = props.getProperty("name") ?: throw RuntimeException("Manifest is missing 'name'")
         val path = props.getProperty("path") ?: throw RuntimeException("Manifest is missing 'path'")
         val version = props.getProperty("version") ?: throw RuntimeException("Manifest is missing 'version'")
-        log.info("Found plugin '$name' version $version")
         return PluginManifest(name, path, version)
     }
-
-    companion object {
-        private val log: Logger = LoggerFactory.getLogger(PluginManager::class.java)
-    }
 }