Index: src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java =================================================================== --- src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java (revision 15505) +++ src/org/openstreetmap/josm/gui/preferences/plugin/PluginPreference.java (working copy) @@ -55,6 +55,7 @@ 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,8 +325,12 @@ List l = new LinkedList<>(model.getSelectedPluginNames()); Collections.sort(l); Config.getPref().putList("plugins", l); - if (!model.getNewlyDeactivatedPlugins().isEmpty()) - return true; + List 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; Index: src/org/openstreetmap/josm/plugins/PluginHandler.java =================================================================== --- src/org/openstreetmap/josm/plugins/PluginHandler.java (revision 15505) +++ src/org/openstreetmap/josm/plugins/PluginHandler.java (working copy) @@ -68,6 +68,7 @@ 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; @@ -1168,7 +1169,6 @@ pluginsToDownload, tr("Update plugins") ); - try { pluginDownloadTask.run(); } catch (RuntimeException e) { // NOPMD @@ -1332,6 +1332,18 @@ 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); } @@ -1643,4 +1655,28 @@ return cbDontShowAgain.isSelected(); } } + + /** + * Remove deactivated plugins, returning true if JOSM should restart + * + * @param deactivatedPlugins The plugins to deactivate + * + * @return true if there was a plugin that requires a restart + */ + public static boolean removePlugins(List deactivatedPlugins) { + List 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); + new ArrayList<>(pluginList).stream().filter(proxy -> noRestart.contains(proxy.getPlugin())) + .forEach(pluginList::remove); + restartNeeded = deactivatedPlugins.size() != noRestart.size(); + } catch (Exception e) { + Logging.error(e); + restartNeeded = true; + } + return restartNeeded; + } } Index: test/unit/org/openstreetmap/josm/plugins/PluginHandlerTestIT.java =================================================================== --- test/unit/org/openstreetmap/josm/plugins/PluginHandlerTestIT.java (revision 15505) +++ test/unit/org/openstreetmap/josm/plugins/PluginHandlerTestIT.java (working copy) @@ -7,6 +7,8 @@ import java.awt.GraphicsEnvironment; import java.awt.HeadlessException; +import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; @@ -13,13 +15,16 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; import org.apache.commons.lang3.exception.ExceptionUtils; -import org.junit.Rule; +import org.junit.BeforeClass; +import org.junit.ClassRule; import org.junit.Test; +import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.data.Preferences; import org.openstreetmap.josm.data.gpx.GpxData; import org.openstreetmap.josm.data.osm.DataSet; @@ -30,6 +35,7 @@ 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; @@ -39,14 +45,26 @@ */ public class PluginHandlerTestIT { + private static List errorsToIgnore = new ArrayList<>(); /** * Setup test. */ - @Rule + @ClassRule @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public JOSMTestRules test = new JOSMTestRules().main().projection().preferences().https().timeout(10*60*1000); + public static JOSMTestRules test = new JOSMTestRules().main().projection().preferences().https() + .timeout(10 * 60 * 1000); /** + * Setup test + * + * @throws IOException in case of I/O error + */ + @BeforeClass + public static void beforeClass() throws IOException { + errorsToIgnore.addAll(TestUtils.getIgnoredErrorMessages(PluginHandlerTestIT.class)); + } + + /** * Test that available plugins rules can be loaded. */ @Test @@ -74,22 +92,66 @@ testPlugin(MainApplication.getLayerManager()::removeLayer, layer, layerExceptions, loadedPlugins); } + Map noRestartExceptions = new HashMap<>(); + testCompletelyRestartlessPlugins(loadedPlugins, noRestartExceptions); + debugPrint(invalidManifestEntries); debugPrint(loadingExceptions); debugPrint(layerExceptions); + debugPrint(noRestartExceptions); + + invalidManifestEntries = filterKnownErrors(invalidManifestEntries); + loadingExceptions = filterKnownErrors(loadingExceptions); + layerExceptions = filterKnownErrors(layerExceptions); + noRestartExceptions = filterKnownErrors(noRestartExceptions); + String msg = Arrays.toString(invalidManifestEntries.entrySet().toArray()) + '\n' + Arrays.toString(loadingExceptions.entrySet().toArray()) + '\n' + - Arrays.toString(layerExceptions.entrySet().toArray()); + Arrays.toString(layerExceptions.entrySet().toArray()) + '\n' + + Arrays.toString(noRestartExceptions.entrySet().toArray()); assertTrue(msg, invalidManifestEntries.isEmpty() && loadingExceptions.isEmpty() && layerExceptions.isEmpty()); } + private static void testCompletelyRestartlessPlugins(List loadedPlugins, + Map noRestartExceptions) { + try { + List restartable = loadedPlugins.parallelStream() + .filter(info -> PluginHandler.getPlugin(info.name) instanceof Destroyable) + .collect(Collectors.toList()); + // ensure good plugin behavior with regards to Destroyable (i.e., they can be + // removed and readded) + for (int i = 0; i < 2; i++) { + assertFalse(PluginHandler.removePlugins(restartable)); + assertTrue(restartable.stream().noneMatch(info -> PluginHandler.getPlugins().contains(info))); + loadPlugins(restartable); + } + + assertTrue(PluginHandler.removePlugins(loadedPlugins)); + assertTrue(restartable.parallelStream().noneMatch(info -> PluginHandler.getPlugins().contains(info))); + } catch (Exception | LinkageError t) { + Throwable root = ExceptionUtils.getRootCause(t); + root.printStackTrace(); + noRestartExceptions.put(findFaultyPlugin(loadedPlugins, root), root); + } + } + + private static Map filterKnownErrors(Map errorMap) { + return errorMap.entrySet().parallelStream() + .filter(entry -> !errorsToIgnore.contains(convertEntryToString(entry))) + .collect(Collectors.toMap(Entry::getKey, Entry::getValue)); + } + private static void debugPrint(Map invalidManifestEntries) { System.out.println(invalidManifestEntries.entrySet() .stream() - .map(e -> e.getKey() + "=\"" + e.getValue() + "\"") + .map(e -> convertEntryToString(e)) .collect(Collectors.joining(", "))); } + private static String convertEntryToString(Entry entry) { + return entry.getKey() + "=\"" + entry.getValue() + "\""; + } + /** * Downloads and loads all JOSM plugins. */ @@ -133,6 +195,10 @@ // Download plugins downloadPlugins(plugins); + loadPlugins(plugins); + } + + static void loadPlugins(List plugins) { // Load early plugins PluginHandler.loadEarlyPlugins(null, plugins, null);