From 6556859dbbfe1ced505e70cb6e3692d239d7b915 Mon Sep 17 00:00:00 2001
From: Piotr Gawron <piotr.gawron@uni.lu>
Date: Tue, 13 Aug 2019 19:15:15 +0200
Subject: [PATCH] when plugin crash on upload it's removed from the system
 properly

---
 CHANGELOG                                        |  2 ++
 frontend-js/src/main/js/plugin/Plugin.js         |  4 ++++
 frontend-js/src/main/js/plugin/PluginManager.js  |  5 +++--
 .../src/test/js/plugin/PluginManager-test.js     | 16 ++++++++++++----
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 5b62fee111..8afa070c88 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,6 +8,8 @@ minerva (14.0.0~alpha.1) unstable; urgency=low
   * Bug fix: some project couldn't be accessed due to problem with migration of 
     reaction with unknown boolean operator (#880)
   * Bug fix: problem with unloading plugin is properly handled (#884)
+  * Bug fix: upload of invalid plugin doesn't add it to plugin tab and list of 
+    loaded plugins (#885)
 
  -- Piotr Gawron <piotr.gawron@uni.lu>  Mon, 12 Aug 2019 10:00:00 +0200
 
diff --git a/frontend-js/src/main/js/plugin/Plugin.js b/frontend-js/src/main/js/plugin/Plugin.js
index 89b722618b..1db70c2911 100644
--- a/frontend-js/src/main/js/plugin/Plugin.js
+++ b/frontend-js/src/main/js/plugin/Plugin.js
@@ -107,6 +107,10 @@ Plugin.prototype.getMinervaPluginProxy = function () {
  * @returns {string}
  */
 Plugin.prototype.getPluginId = function () {
+  if (this.getMinervaPluginProxy() === undefined) {
+    logger.warn("Plugin is not loaded properly");
+    return undefined;
+  }
   return this.getMinervaPluginProxy().pluginId;
 };
 
diff --git a/frontend-js/src/main/js/plugin/PluginManager.js b/frontend-js/src/main/js/plugin/PluginManager.js
index ff458420f6..be4a4bafc9 100644
--- a/frontend-js/src/main/js/plugin/PluginManager.js
+++ b/frontend-js/src/main/js/plugin/PluginManager.js
@@ -94,6 +94,7 @@ PluginManager.prototype.addPlugin = function (options) {
         map: self.getMap(),
         serverConnector: self.getServerConnector()
       });
+      self._plugins.push(plugin);
     } else {
       plugin = new Plugin({
         url: options.url,
@@ -105,11 +106,11 @@ PluginManager.prototype.addPlugin = function (options) {
       plugin.addListener("onUnload", function () {
         self.getGuiUtils().removeTab(self, element);
       });
+      self._plugins.push(plugin);
       if (!self.isValidUrl(options.url)) {
         return Promise.reject(new InvalidArgumentError("url: '" + options.url + "' is invalid"));
       }
     }
-    self._plugins.push(plugin);
     return plugin.load();
   }).then(function () {
     $("a[href='#" + element.parentNode.id + "']", $(self.getElement()))[0].innerHTML = plugin.getName();
@@ -140,7 +141,7 @@ PluginManager.prototype.addPlugin = function (options) {
     GuiConnector.setUrlParam("plugins", self._getLoadedPluginsAsHashList());
     return plugin;
   }).catch(function (e) {
-    return plugin.unload().then(function () {
+    return self.removePlugin(plugin).then(function () {
       GuiConnector.alert("Problem with loading plugin:<br/>" + e.message);
     });
   });
diff --git a/frontend-js/src/test/js/plugin/PluginManager-test.js b/frontend-js/src/test/js/plugin/PluginManager-test.js
index 4ac0fac0eb..ef6f02df82 100644
--- a/frontend-js/src/test/js/plugin/PluginManager-test.js
+++ b/frontend-js/src/test/js/plugin/PluginManager-test.js
@@ -88,10 +88,11 @@ describe('PluginManager', function () {
 
     it('with error', function () {
       var manager = createPluginManager();
-      return manager.addPlugin({url: "./invalid.js"}).then(function (plugin) {
+      return manager.addPlugin({url: "./invalid.js"}).then(function () {
         assert.notOk("Expected error");
       }).catch(function (e) {
-        assert.ok(e.message.indexOf("Problem with loading plugin") >= 0)
+        assert.ok(e.message.indexOf("Problem with loading plugin") >= 0);
+        assert.equal(0, manager.getPlugins().length);
       });
     });
     it('with invalid url', function () {
@@ -99,7 +100,7 @@ describe('PluginManager', function () {
       manager.isValidUrl = function () {
         return false;
       };
-      return manager.addPlugin({url: "xx"}).then(function (plugin) {
+      return manager.addPlugin({url: "xx"}).then(function () {
         assert.notOk("Expected error");
       }).catch(function (e) {
         assert.ok(e.message.indexOf("Problem with loading plugin") >= 0)
@@ -146,7 +147,14 @@ describe('PluginManager', function () {
 
     it('removing non existing plugin', function () {
       var manager = createPluginManager();
-      var plugin = new Plugin({url: "testFiles/plugin/empty.js"});
+      var plugin = new Plugin({
+        url: "testFiles/plugin/empty.js",
+        map: manager.getMap(),
+        configuration: manager.getConfiguration(),
+        serverConnector: manager.getServerConnector(),
+        element: testDiv
+      });
+
       return manager.removePlugin(plugin).then(function () {
         assert.notOk("Error expected");
       }, function (error) {
-- 
GitLab