[RFC PATCH] Allow plugins to implement Destroyable if they want to allow restartless updates/removals

[taylor.smock]

Use case: there is a plugin update, but the user doesn't want to restart JOSM (maybe they have a lot of unsaved work that they cannot save right now, e.g. they have an incomplete rework of a complicated intersection).
Use case: A plugin crashes, but it implements Destroyable and there is an update for that plugin.

I don't know if I went about the implementation of this the best way, but it works for my plugin (with some modifications on the plugin side).

[taylor.smock]

Initial implementation (no tests)

[Don-vip]

Looks nice! Do you plan to write some tests?

[taylor.smock]

If I didn't have to change the implementation, yes. I haven't looked at the tests for the PluginHandler yet, so I don't have a timeframe on that.

I'll probably modify the patch so that I'm not calling the destroyables inside the PluginPreference#ok class (e.g., PluginHandler.removePlugins(PluginList)).

[Don-vip]

OK. Just one note, the collect is probably unneeded, you can call the destroy method on the stream directly.

[taylor.smock]

Move calls for plugin destroy functions out of PluginPreference.java

[taylor.smock]

Catch exceptions thrown by plugins with a destroy method and indicate that a restart should occur

[taylor.smock]

Replying to Don-vip:

OK. Just one note, the collect is probably unneeded, you can call the destroy method on the stream directly.

I was thinking of using the list to check if there were any plugins that did not have a destroy method, and return true if that were the case.

I did that in the last two patches, although the last patch is a bit more hardened against a plugin throwing an exception during destroy (which shouldn't happen, but bugs).

[Don-vip]

Please log the exception. We should know when a plugin fails to destroy itself. Also we should get a better variable name for returnBoolean.

[taylor.smock]

Change returnBoolean to restartNeeded, log error when a plugin can't destroy itself, and add initial test (currently fails -- there is a plugin that implements destroyable already, but I don't know if that is the cause of the failure).

[taylor.smock]

Fix test (still fails due to kenzi3d issues, but it isn't the destroyable part of the test). Also remove destroyed plugins from the pluginList in PluginHandler.

[taylor.smock]

The only two plugins that currently implement Destroyable are MapWithAI and Tagging Preset Tester, although the latter "implements" it through extending JosmAction (so not intentionally).

[Don-vip]

If you have trouble with some specific plugins, you can update PluginHandlerTestIT so that it reads ignored lines from IntegrationTestIgnores.

[taylor.smock]

Add methods to ignore known failing plugins (see https://josm.openstreetmap.de/wiki/IntegrationTestIgnores?version=228 -- kenzi3d was consistently failing on my local machine)

[Don-vip]

In 15508/josm:

fix #18277 - Allow plugins to implement Destroyable if they want to allow restartless updates/removals (patch by taylor.smock)