1. Don Brown
  2. atlassian-plugins

Commits

Don Brown  committed d0329ba

PLUG-843 Ensure Plugin.uninstall() is called

* Uninstallable plugins should have Plugin.uninstall() on plugin uninstallation
* Switch up logic so PluginLoader.removePlugin is called for uninstallable plugins
* This should fix uninstalled bundled plugins leaking memory

  • Participants
  • Parent commits 7bc6d2b
  • Branches uninstall-bundled

Comments (0)

Files changed (4)

File atlassian-plugins-core/src/main/java/com/atlassian/plugin/Plugin.java

View file
     boolean isUninstallable();
 
     /**
-     * Should the plugin file be deleted on unistall?
+     * Should the plugin file be deleted on uninstall?
      *
-     * @return {@code true} if this plugin file should be deleted on unistall.
+     * @return {@code true} if this plugin file should be deleted on uninstall.
      */
     boolean isDeleteable();
 

File atlassian-plugins-core/src/main/java/com/atlassian/plugin/loaders/ScanningPluginLoader.java

View file
             throw new PluginException("Cannot remove an enabled plugin");
         }
 
-        if (!plugin.isUninstallable())
+        final DeploymentUnit deploymentUnit = findMatchingDeploymentUnit(plugin);
+        plugin.uninstall();
+
+        if (plugin.isDeleteable())
         {
-            throw new PluginException("Cannot remove an uninstallable plugin: [" + plugin.getName() + "]");
+            deleteDeploymentUnit(deploymentUnit);
         }
 
-        final DeploymentUnit deploymentUnit = findMatchingDeploymentUnit(plugin);
-        plugin.uninstall();
 
+        plugins.remove(deploymentUnit);
+        log.info("Removed plugin " + plugin.getKey());
+    }
+
+    private void deleteDeploymentUnit(DeploymentUnit deploymentUnit)
+    {
         try
         {
             // Loop over to see if there are any other deployment units with the same filename. This will happen
         {
             throw new PluginException(e);
         }
-
-        plugins.remove(deploymentUnit);
-        log.info("Removed plugin " + plugin.getKey());
     }
 
     private DeploymentUnit findMatchingDeploymentUnit(final Plugin plugin) throws PluginException

File atlassian-plugins-core/src/main/java/com/atlassian/plugin/manager/DefaultPluginManager.java

View file
 
     private void removePluginFromLoader(final Plugin plugin) throws PluginException
     {
-        if (plugin.isDeleteable())
+        if (plugin.isUninstallable())
         {
             final PluginLoader pluginLoader = pluginToPluginLoader.get(plugin);
             pluginLoader.removePlugin(plugin);

File atlassian-plugins-osgi/src/test/java/com/atlassian/plugin/osgi/TestPluginUninstall.java

View file
+package com.atlassian.plugin.osgi;
+
+import com.atlassian.plugin.DefaultModuleDescriptorFactory;
+import com.atlassian.plugin.JarPluginArtifact;
+import com.atlassian.plugin.Plugin;
+import com.atlassian.plugin.hostcontainer.DefaultHostContainer;
+import com.atlassian.plugin.osgi.util.OsgiHeaderUtil;
+import com.atlassian.plugin.test.PluginJarBuilder;
+import org.osgi.framework.Bundle;
+
+import java.io.File;
+
+/**
+ * Created by IntelliJ IDEA.
+ * User: mrdon
+ * Date: 24/01/12
+ * Time: 10:04 PM
+ * To change this template use File | Settings | File Templates.
+ */
+public class TestPluginUninstall extends PluginInContainerTestBase
+{
+    public void testUninstallBundled() throws Exception
+    {
+        File jar = new PluginJarBuilder("test-uninstall")
+                .addPluginInformation("test-uninstall", "test", "1")
+                .build();
+        initBundlingPluginManager(new DefaultModuleDescriptorFactory(new DefaultHostContainer()), jar);
+        Bundle pluginBundle = null;
+        for (Bundle bundle : osgiContainerManager.getBundles())
+        {
+            if ("test-uninstall".equals(OsgiHeaderUtil.getPluginKey(bundle)))
+            {
+                pluginBundle = bundle;
+                break;
+            }
+        }
+        assertNotNull(pluginBundle);
+
+        pluginManager.uninstall(pluginManager.getPlugin("test-uninstall"));
+        assertEquals(Bundle.UNINSTALLED, pluginBundle.getState());
+    }
+}