Changeset 18598 in josm

Timestamp:

2022-11-16T01:08:37+01:00 (2 years ago)

Author:

taylor.smock

Message:

FlatLaf uses properties files which occasionally have breaking changes

In the event that a properties file is misconfigured, FlatLaf will throw
an IllegalArgumentException, which causes JOSM to exit, which is not ideal,
since recovering is not intuitive.

This will roll back the LAF to the platform default, so people can start JOSM.
Which isn't ideal, but is better than not starting indefinitely.

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/MainApplication.java (modified) (3 diffs)
• test/unit/org/openstreetmap/josm/gui/MainApplicationTest.java (modified) (6 diffs)

trunk/src/org/openstreetmap/josm/gui/MainApplication.java

@@ -1123 +1123 @@
     }
 
+    /**
+     * Set up the UI manager
+     */
+    // We want to catch all exceptions here to reset LaF to defaults and report it.
+    @SuppressWarnings("squid:S2221")
     static void setupUIManager() {
         String defaultlaf = PlatformManager.getPlatform().getDefaultStyle();
@@ -1149 +1154 @@
                     LafPreference.LAF.put(defaultlaf);
                     Logging.trace(ex);
+                } catch (Exception ex) {
+                    // We do not want to silently exit if there is an exception.
+                    // Put the default laf in place so that the user can use JOSM.
+                    LafPreference.LAF.put(defaultlaf);
+                    BugReportExceptionHandler.handleException(ex);
                 }
             } else {
@@ -1160 +1170 @@
         } catch (InstantiationException | IllegalAccessException e) {
             Logging.error(e);
+        } catch (Exception e) {
+            // We do not want to silently exit if there is an exception.
+            // Put the default laf in place.
+            LafPreference.LAF.put(defaultlaf);
+            BugReportExceptionHandler.handleException(e);
         }
 

trunk/test/unit/org/openstreetmap/josm/gui/MainApplicationTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -15 +16 @@
 import java.io.PrintStream;
 import java.net.MalformedURLException;
+import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.jar.Attributes;
 
 import javax.swing.JComponent;
 import javax.swing.JPanel;
+import javax.swing.UIDefaults;
 import javax.swing.UIManager;
-
+import javax.swing.plaf.metal.MetalLookAndFeel;
+
+import mockit.Mock;
+import mockit.MockUp;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -38 +46 @@
 import org.openstreetmap.josm.gui.layer.GpxLayer;
 import org.openstreetmap.josm.gui.preferences.ToolbarPreferences;
+import org.openstreetmap.josm.gui.preferences.display.LafPreference;
+import org.openstreetmap.josm.plugins.PluginClassLoader;
 import org.openstreetmap.josm.plugins.PluginHandler;
 import org.openstreetmap.josm.plugins.PluginHandlerTestIT;
@@ -50 +60 @@
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import org.openstreetmap.josm.tools.bugreport.BugReportQueue;
 
 /**
@@ -196 +207 @@
     @Test
     void testSetupUIManager() {
+        TestUtils.assumeWorkingJMockit();
         assumeFalse(PlatformManager.isPlatformWindows() && "True".equals(System.getenv("APPVEYOR")));
-        MainApplication.setupUIManager();
+        assertDoesNotThrow(MainApplication::setupUIManager);
         assertEquals(Config.getPref().get("laf", PlatformManager.getPlatform().getDefaultStyle()),
                 UIManager.getLookAndFeel().getClass().getCanonicalName());
+        try {
+            LafPreference.LAF.put(BadLaf.class.getName());
+            new PluginHandlerMock();
+            AtomicReference<Throwable> exceptionAtomicReference = new AtomicReference<>();
+            BugReportQueue.getInstance().setBugReportHandler((e, index) -> {
+                exceptionAtomicReference.set(e.getCause());
+                return BugReportQueue.SuppressionMode.NONE;
+            });
+            assertDoesNotThrow(MainApplication::setupUIManager);
+
+            assertNotNull(exceptionAtomicReference.get());
+            assertTrue(exceptionAtomicReference.get() instanceof UnsupportedOperationException);
+            // The LAF only resets on restart, so don't bother checking that it switched back in UIManager
+            assertEquals(LafPreference.LAF.getDefaultValue(), LafPreference.LAF.get());
+        } finally {
+            BugReportQueue.getInstance().setBugReportHandler(BugReportQueue.FALLBACK_BUGREPORT_HANDLER);
+            // Make certain we reset the LAF
+            LafPreference.LAF.remove();
+            assertDoesNotThrow(MainApplication::setupUIManager);
+            assertEquals(Config.getPref().get("laf", PlatformManager.getPlatform().getDefaultStyle()),
+                    UIManager.getLookAndFeel().getClass().getCanonicalName());
+        }
     }
 
@@ -317 +351 @@
         TestUtils.superficialEnumCodeCoverage(DownloadParamType.class);
     }
+
+    /**
+     * This class exists to test a failure in non-default UI loading
+     */
+    public static class BadLaf extends MetalLookAndFeel {
+        @Override
+        public UIDefaults getDefaults() {
+            throw new UnsupportedOperationException("Test failure loading");
+        }
+    }
+
+    /**
+     * A mock class for returning a fake plugin class loader for {@link #testSetupUIManager()}
+     */
+    public static class PluginHandlerMock extends MockUp<PluginHandler> {
+        @Mock
+        public static Collection<PluginClassLoader> getPluginClassLoaders() {
+            return Collections.singleton(new PluginClassLoader(new URL[0], BadLaf.class.getClassLoader(), Collections.emptyList()));
+        }
+    }
 }