fix plugin manager not deleting old plugin version (lavalink-devs#991)

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

* close JarFile correctly

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)
- }
 }