From ed7bb495ca939c25005a6367ef44f6db18daf686 Mon Sep 17 00:00:00 2001
From: Naoki Takezoe <takezoe@gmail.com>
Date: Mon, 11 Jun 2018 13:36:53 +0900
Subject: [PATCH] Enhance plugin install / uninstall process

---
 .../controller/SystemSettingsController.scala | 19 ++++----
 .../core/plugin/PluginRegistry.scala          | 44 +++++++++----------
 .../gitbucket/core/admin/plugins.scala.html   | 33 +++++++-------
 3 files changed, 49 insertions(+), 47 deletions(-)

diff --git a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala
index dd9edc9ea..df4e712bd 100644
--- a/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala
+++ b/src/main/scala/gitbucket/core/controller/SystemSettingsController.scala
@@ -364,17 +364,15 @@ trait SystemSettingsControllerBase extends AccountManagementControllerBase {
     redirect("/admin/plugins")
   })
 
-  post("/admin/plugins/:pluginId/:version/_uninstall")(adminOnly {
+  post("/admin/plugins/:pluginId/_uninstall")(adminOnly { // TODO Is version unnecessary?
     val pluginId = params("pluginId")
-    val version = params("version")
-    PluginRegistry()
-      .getPlugins()
-      .collect { case plugin if (plugin.pluginId == pluginId && plugin.pluginVersion == version) => plugin }
-      .foreach { _ =>
-        PluginRegistry
-          .uninstall(pluginId, request.getServletContext, loadSystemSettings(), request2Session(request).conn)
-        flash += "info" -> s"${pluginId} was uninstalled."
-      }
+
+    if (PluginRegistry().getPlugins().exists(_.pluginId == pluginId)) {
+      PluginRegistry
+        .uninstall(pluginId, request.getServletContext, loadSystemSettings(), request2Session(request).conn)
+      flash += "info" -> s"${pluginId} was uninstalled."
+    }
+
     redirect("/admin/plugins")
   })
 
@@ -389,6 +387,7 @@ trait SystemSettingsControllerBase extends AccountManagementControllerBase {
         case (meta, version) =>
           version.foreach { version =>
             PluginRegistry.install(
+              pluginId,
               new java.net.URL(version.url),
               request.getServletContext,
               loadSystemSettings(),
diff --git a/src/main/scala/gitbucket/core/plugin/PluginRegistry.scala b/src/main/scala/gitbucket/core/plugin/PluginRegistry.scala
index 857e501b4..239889142 100644
--- a/src/main/scala/gitbucket/core/plugin/PluginRegistry.scala
+++ b/src/main/scala/gitbucket/core/plugin/PluginRegistry.scala
@@ -225,37 +225,37 @@ object PluginRegistry {
    */
   def uninstall(pluginId: String, context: ServletContext, settings: SystemSettings, conn: java.sql.Connection): Unit =
     synchronized {
-      instance
-        .getPlugins()
-        .collect { case plugin if plugin.pluginId == pluginId => plugin }
-        .foreach { plugin =>
-//      try {
-//        plugin.pluginClass.uninstall(instance, context, settings)
-//      } catch {
-//        case e: Exception =>
-//          logger.error(s"Error during uninstalling plugin: ${plugin.pluginJar.getName}", e)
-//      }
-          shutdown(context, settings)
+      shutdown(context, settings)
 
-          new File(PluginHome)
-            .listFiles((_: File, name: String) => {
-              name.startsWith(s"gitbucket-${pluginId}-plugin") && name.endsWith(".jar")
-            })
-            .foreach { file =>
-              file.delete()
-            }
+      new File(PluginHome)
+        .listFiles((_: File, name: String) => {
+          name.startsWith(s"gitbucket-${pluginId}-plugin") && name.endsWith(".jar")
+        })
+        .foreach(_.delete())
 
-          instance = new PluginRegistry()
-          initialize(context, settings, conn)
-        }
+      instance = new PluginRegistry()
+      initialize(context, settings, conn)
     }
 
   /**
    * Install a plugin from a specified jar file.
    */
-  def install(url: java.net.URL, context: ServletContext, settings: SystemSettings, conn: java.sql.Connection): Unit =
+  def install(
+    pluginId: String,
+    url: java.net.URL,
+    context: ServletContext,
+    settings: SystemSettings,
+    conn: java.sql.Connection
+  ): Unit =
     synchronized {
       shutdown(context, settings)
+
+      new File(PluginHome)
+        .listFiles((_: File, name: String) => {
+          name.startsWith(s"gitbucket-${pluginId}-plugin") && name.endsWith(".jar")
+        })
+        .foreach(_.delete())
+
       val in = url.openStream()
       FileUtils.copyToFile(in, new File(PluginHome, new File(url.getFile).getName))
       instance = new PluginRegistry()
diff --git a/src/main/twirl/gitbucket/core/admin/plugins.scala.html b/src/main/twirl/gitbucket/core/admin/plugins.scala.html
index d97d8930d..0970f8286 100644
--- a/src/main/twirl/gitbucket/core/admin/plugins.scala.html
+++ b/src/main/twirl/gitbucket/core/admin/plugins.scala.html
@@ -16,21 +16,19 @@
       @plugins.map { case (plugin, enabled, updatableVersion) =>
         <div class="panel panel-default">
           <div class="panel-heading strong" id="@plugin.pluginId">
-            @if(enabled){
-              @if(updatableVersion.isEmpty){
-                <form action="@{context.path}/admin/plugins/@{plugin.pluginId}/@{plugin.pluginVersion}/_uninstall" method="POST" class="pull-right uninstall-form">
-                  <input type="submit" value="Uninstall" class="btn btn-danger btn-sm" style="position: relative; top: -5px; left: 10px;" data-name="@plugin.pluginName">
-                </form>
+            <form method="POST" class="pull-right">
+              @if(enabled){
+                @if(updatableVersion.nonEmpty){
+                  <input type="submit" value="Update" class="btn btn-success btn-sm update-plugin" style="position: relative; top: -5px; left: 10px;"
+                  data-name="@plugin.pluginName" formaction="@{context.path}/admin/plugins/@{plugin.pluginId}/@{updatableVersion}/_install">
+                }
+                <input type="submit" value="Uninstall" class="btn btn-danger btn-sm uninstall-plugin" style="position: relative; top: -5px; left: 10px;"
+                       data-name="@plugin.pluginName" formaction="@{context.path}/admin/plugins/@{plugin.pluginId}/_uninstall">
               } else {
-                <form action="@{context.path}/admin/plugins/@{plugin.pluginId}/@{updatableVersion}/_install" method="POST" class="pull-right install-form">
-                  <input type="submit" value="Update" class="btn btn-primary btn-sm" style="position: relative; top: -5px; left: 10px;" data-name="@plugin.pluginName">
-                </form>
+                <input type="submit" value="Install" class="btn btn-success btn-sm install-plugin" style="position: relative; top: -5px; left: 10px;"
+                       data-name="@plugin.pluginName" formaction="@{context.path}/admin/plugins/@{plugin.pluginId}/@{plugin.pluginVersion}/_install">
               }
-            } else {
-              <form action="@{context.path}/admin/plugins/@{plugin.pluginId}/@{plugin.pluginVersion}/_install" method="POST" class="pull-right install-form">
-                <input type="submit" value="Install" class="btn btn-success btn-sm" style="position: relative; top: -5px; left: 10px;" data-name="@plugin.pluginName">
-              </form>
-            }
+            </form>
             @plugin.pluginName
           </div>
           <div class="panel-body">
@@ -60,14 +58,19 @@
 }
 <script>
   $(function(){
-    $('.uninstall-form').click(function(e){
+    $('.uninstall-plugin').click(function(e){
       var name = $(e.target).data('name');
       return confirm('Uninstall ' + name + '. Are you sure?');
     });
 
-    $('.install-form').click(function(e){
+    $('.install-plugin').click(function(e){
       var name = $(e.target).data('name');
       return confirm('Install ' + name + '. Are you sure?');
     });
+
+    $('.update-plugin').click(function(e){
+      var name = $(e.target).data('name');
+      return confirm('Update ' + name + '. Are you sure?');
+    });
   });
 </script>