Ticket #18277: 18277.3.patch

src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java

@@ -55 +55 @@
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.gui.widgets.FilterField;
 import org.openstreetmap.josm.plugins.PluginDownloadTask;
+import org.openstreetmap.josm.plugins.PluginHandler;
 import org.openstreetmap.josm.plugins.PluginInformation;
 import org.openstreetmap.josm.plugins.ReadLocalPluginInformationTask;
 import org.openstreetmap.josm.plugins.ReadRemotePluginInformationTask;
@@ -324 +325 @@
             List<String> l = new LinkedList<>(model.getSelectedPluginNames());
             Collections.sort(l);
             Config.getPref().putList("plugins", l);
-            if (!model.getNewlyDeactivatedPlugins().isEmpty())
-                return true;
+            List<PluginInformation> deactivatedPlugins = model.getNewlyDeactivatedPlugins();
+            if (!deactivatedPlugins.isEmpty()) {
+                boolean requiresRestart = PluginHandler.removePlugins(deactivatedPlugins);
+                if (requiresRestart)
+                    return requiresRestart;
+            }
             for (PluginInformation pi : model.getNewlyActivatedPlugins()) {
                 if (!pi.canloadatruntime)
                     return true;

src/org/openstreetmap/josm/plugins/PluginHandler.java

@@ -68 +68 @@
 import org.openstreetmap.josm.io.OfflineAccessException;
 import org.openstreetmap.josm.io.OnlineResource;
 import org.openstreetmap.josm.spi.preferences.Config;
+import org.openstreetmap.josm.tools.Destroyable;
 import org.openstreetmap.josm.tools.GBC;
 import org.openstreetmap.josm.tools.I18n;
 import org.openstreetmap.josm.tools.ImageProvider;
@@ -1167 +1168 @@
                         pluginsToDownload,
                         tr("Update plugins")
                 );
-
                 try {
                     pluginDownloadTask.run();
                 } catch (RuntimeException e) { // NOPMD
@@ -1331 +1331 @@
                     URL oldPluginURL = updatedPlugin.toURI().toURL();
                     pluginsToLoad.stream().filter(x -> x.libraries.contains(oldPluginURL)).forEach(
                             x -> Collections.replaceAll(x.libraries, oldPluginURL, newPluginURL));
+
+                    // Attempt to update loaded plugin (must implement Destroyable)
+                    PluginInformation tInfo = pluginsToLoad.parallelStream()
+                            .filter(x -> x.libraries.contains(newPluginURL)).findAny().orElse(null);
+                    if (tInfo != null) {
+                        Object tUpdatedPlugin = getPlugin(tInfo.name);
+                        if (tUpdatedPlugin instanceof Destroyable) {
+                            ((Destroyable) tUpdatedPlugin).destroy();
+                            PluginHandler.loadPlugins(getInfoPanel(), Collections.singleton(tInfo),
+                                    NullProgressMonitor.INSTANCE);
+                        }
+                    }
                 } catch (MalformedURLException e) {
                     Logging.warn(e);
                 }
@@ -1642 +1654 @@
             return cbDontShowAgain.isSelected();
         }
     }
+
+    /**
+     * Remove deactivated plugins, returning true if JOSM should restart
+     *
+     * @param deactivatedPlugins
+     * @return true if there was a plugin that requires a restart
+     */
+    public static boolean removePlugins(List<PluginInformation> deactivatedPlugins) {
+        List<Destroyable> noRestart = deactivatedPlugins.parallelStream()
+                .map(info -> PluginHandler.getPlugin(info.name)).filter(Destroyable.class::isInstance)
+                .map(Destroyable.class::cast).collect(Collectors.toList());
+        boolean restartNeeded;
+        try {
+            noRestart.forEach(Destroyable::destroy);
+            restartNeeded = deactivatedPlugins.size() != noRestart.size();
+        } catch (Exception e) {
+            Logging.error(e);
+            restartNeeded = true;
+        }
+        return restartNeeded;
+    }
 }

test/unit/org/openstreetmap/josm/plugins/PluginHandlerTestIT.java

@@ -13 +13 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
@@ -30 +31 @@
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
+import org.openstreetmap.josm.tools.Destroyable;
 import org.openstreetmap.josm.tools.Utils;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -74 +76 @@
             testPlugin(MainApplication.getLayerManager()::removeLayer, layer, layerExceptions, loadedPlugins);
         }
 
+        testCompletelyRestartlessPlugins(loadedPlugins);
+
         debugPrint(invalidManifestEntries);
         debugPrint(loadingExceptions);
         debugPrint(layerExceptions);
@@ -83 +87 @@
         assertTrue(msg, invalidManifestEntries.isEmpty() && loadingExceptions.isEmpty() && layerExceptions.isEmpty());
     }
 
+    private static void testCompletelyRestartlessPlugins(List<PluginInformation> loadedPlugins) {
+        List<PluginInformation> restartable = loadedPlugins.parallelStream()
+                .filter(info -> PluginHandler.getPlugin(info.name) instanceof Destroyable).collect(Collectors.toList());
+        // ensure good plugin behavior with regards to Destroyable
+        for (int i = 0; i < 2; i++) {
+            assertFalse(PluginHandler.removePlugins(restartable));
+            Optional<PluginInformation> optional = restartable.stream()
+                    .filter(info -> PluginHandler.getPlugins().contains(info)).findAny();
+            assertTrue(restartable.stream().noneMatch(info -> PluginHandler.getPlugins().contains(info)));
+            PluginHandler.loadEarlyPlugins(null, restartable, null);
+            PluginHandler.loadLatePlugins(null, restartable, null);
+        }
+
+        assertTrue(PluginHandler.removePlugins(loadedPlugins));
+        assertTrue(restartable.parallelStream().noneMatch(info -> PluginHandler.getPlugins().contains(info)));
+    }
+
     private static void debugPrint(Map<String, ?> invalidManifestEntries) {
         System.out.println(invalidManifestEntries.entrySet()
                 .stream()