Merge pull request #403 from Oliver-Loeffler/issue-402

When loading of FXML file fails, in some cases the error is not shown to the user

app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java

@@ -639,7 +639,7 @@ private void performOpenFiles(List<File> fxmlFiles,
         assert fxmlFiles != null;
         assert fxmlFiles.isEmpty() == false;
 
-        final Map<File, IOException> exceptions = new HashMap<>();
+        final Map<File, Exception> exceptions = new HashMap<>();
         for (File fxmlFile : fxmlFiles) {
             try {
                 final DocumentWindowController dwc
@@ -660,7 +660,7 @@ private void performOpenFiles(List<File> fxmlFiles,
                     hostWindow.loadFromFile(fxmlFile);
                     hostWindow.openWindow();
                 }
-            } catch (IOException xx) {
+            } catch (Exception xx) {
                 exceptions.put(fxmlFile, xx);
             }
         }

kit/src/main/java/com/oracle/javafx/scenebuilder/kit/fxom/FXOMLoader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 Gluon and/or its affiliates.
+ * Copyright (c) 2017, 2022, Gluon and/or its affiliates.
  * Copyright (c) 2012, 2014, Oracle and/or its affiliates.
  * All rights reserved. Use is subject to license terms.
  *
@@ -48,6 +48,8 @@
 import java.io.InputStream;
 import java.nio.charset.Charset;
 import java.util.Collections;
+import java.util.Objects;
+import java.util.function.Consumer;
 
 
 /**
@@ -57,6 +59,7 @@
 class FXOMLoader implements LoadListener {
 
     private final FXOMDocument document;
+    private final Consumer<Exception> knownErrorsHandler;
     private TransientNode currentTransientNode;
     private GlueCursor glueCursor;
 
@@ -65,9 +68,14 @@ class FXOMLoader implements LoadListener {
      */
 
     public FXOMLoader(FXOMDocument document) {
+        this(document, FXOMLoader::showErrorDialog);
+    }
+
+    FXOMLoader(FXOMDocument document, Consumer<Exception> knownErrorsHandler) {
         assert document != null;
         assert document.getGlue().getRootElement() != null;
         this.document = document;
+        this.knownErrorsHandler = Objects.requireNonNull(knownErrorsHandler);
     }
 
     public void load(String fxmlText) throws java.io.IOException {
@@ -95,14 +103,31 @@ public void load(String fxmlText) throws java.io.IOException {
             is.reset();
             setSceneGraphRoot(fxmlLoader.load(is));
         } catch (RuntimeException | IOException x) {
-            if (x.getCause().getClass() == XMLStreamException.class) {
-                handleUnsupportedCharset(x);
-            } else
-                throw new IOException(x);
+            handleFxmlLoadingError(x);
+        }
+    }
+
+    private void handleFxmlLoadingError(Exception x) throws IOException {
+        if (x.getCause() != null) {
+            handleKnownCauses(x);
+        } else {
+            handleUnknownAndMissingCauses(x);
+        }
+    }
+
+    private void handleUnknownAndMissingCauses(Exception x) throws IOException {
+        throw new IOException(x);
+    }
+
+    private void handleKnownCauses(Exception x) throws IOException {
+        if (x.getCause().getClass() == XMLStreamException.class) {
+            knownErrorsHandler.accept(x);
+        } else {                    
+            handleUnknownAndMissingCauses(x);
         }
     }
 
-    private void handleUnsupportedCharset(Exception x) {
+    private static void showErrorDialog(Exception x) {
         final ErrorDialog errorDialog = new ErrorDialog(null);
         errorDialog.setMessage(I18N.getString("alert.open.failure.charset.not.found"));
         errorDialog.setDetails(I18N.getString("alert.open.failure.charset.not.found.details"));

kit/src/test/java/com/oracle/javafx/scenebuilder/kit/fxom/FXOMDocumentTest.java

@@ -0,0 +1,180 @@
+/*
+ * Copyright (c) 2022, Gluon and/or its affiliates.
+ * All rights reserved. Use is subject to license terms.
+ *
+ * This file is available and licensed under the following license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the distribution.
+ *  - Neither the name of Oracle Corporation and Gluon nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package com.oracle.javafx.scenebuilder.kit.fxom;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.concurrent.Callable;
+import java.util.concurrent.FutureTask;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.xml.sax.SAXParseException;
+
+import com.oracle.javafx.scenebuilder.kit.JfxInitializer;
+
+import javafx.application.Platform;
+import javafx.fxml.FXMLLoader;
+
+public class FXOMDocumentTest {
+
+    @Before
+    public void init() {
+        JfxInitializer.initialize();
+    }
+
+    @Test
+    public void that_IOException_is_thrown_in_case_FXMLLoader_error() throws Exception {
+        URL resource = getClass().getResource("BrokenByUserData.fxml");
+        String fxmlText = FXOMDocument.readContentFromURL(resource);
+        Throwable t = assertThrows(IOException.class, () -> new FXOMDocument(fxmlText, resource, null, null));
+        String message = t.getMessage();
+        assertTrue(message.startsWith("javafx.fxml.LoadException:"));
+    }
+
+    @Test
+    public void that_illegal_null_value_for_fxmlText_raises_AssertionError() throws Exception {
+        URL resource = getClass().getResource("BrokenByUserData.fxml");
+        String fxmlText = null;
+        assertThrows(AssertionError.class, () -> new FXOMDocument(fxmlText, resource, null, null));
+    }
+
+    @Test
+    public void that_exception_in_case_of_broken_XML_is_captured() throws Exception {
+        URL resource = getClass().getResource("IncompleteXml.fxml");
+        String fxmlText = FXOMDocument.readContentFromURL(resource);
+        Throwable t = assertThrows(IOException.class, () -> new FXOMDocument(fxmlText, resource, null, null));
+        String message = t.getMessage();
+        assertTrue(message.startsWith("org.xml.sax.SAXParseException;"));
+
+        Throwable cause = t.getCause();
+        assertTrue(cause instanceof SAXParseException);
+    }
+
+    @Test
+    public void that_no_exception_is_created_with_empty_FXML() throws Exception {
+        URL resource = getClass().getResource("Empty.fxml");
+        String fxmlText = "";
+        boolean normalizeFxom = false;
+        FXOMDocument classUnderTest = new FXOMDocument(fxmlText, resource, null, null, normalizeFxom);
+        assertNotNull(classUnderTest);
+    }
+
+    @Test
+    public void that_FXOMDocument_is_created_for_valid_FXML() throws Exception {
+        URL validResource = getClass().getResource("ValidFxml.fxml");
+        String validFxmlText = FXOMDocument.readContentFromURL(validResource);
+        FXOMDocument classUnderTest = new FXOMDocument(validFxmlText, validResource, null, null);
+        assertNotNull(classUnderTest);
+    }
+
+    @Test
+    public void that_wildcard_imports_are_built_on_demand() throws Exception {
+        URL validResource = getClass().getResource("PublicStaticImport.fxml");
+        String validFxmlText = FXOMDocument.readContentFromURL(validResource);
+        FXOMDocument classUnderTest = new FXOMDocument(validFxmlText, validResource, null, null);
+        boolean withWildCardImports = true;
+
+        String javaFxVersion = FXMLLoader.JAVAFX_VERSION;
+        String generatedFxmlText = classUnderTest.getFxmlText(withWildCardImports);
+        String expectedFxmlText = 
+                  "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n\n"
+                + "<?import javafx.scene.effect.*?>\n"
+                + "<?import javafx.scene.layout.*?>\n"
+                + "<?import javafx.scene.text.*?>\n\n"
+                + "<StackPane xmlns=\"http://javafx.com/javafx/"+javaFxVersion+"\" xmlns:fx=\"http://javafx.com/fxml/1\">\n"
+                + "   <children>\n"
+                + "      <Text stroke=\"BLACK\" text=\"Some simple text\">\n"
+                + "         <effect>\n"
+                + "            <Lighting diffuseConstant=\"2.0\" specularConstant=\"0.9\" specularExponent=\"10.5\" surfaceScale=\"9.3\">\n"
+                + "               <light>\n"
+                + "                  <Light.Distant />\n"
+                + "               </light>\n"
+                + "            </Lighting>\n"
+                + "         </effect>\n"
+                + "      </Text>\n"
+                + "   </children>\n"
+                + "</StackPane>\n";
+        assertEquals(expectedFxmlText, generatedFxmlText);
+    }
+
+    @Test
+    public void that_generated_FXML_text_is_empty_for_empty_FXOMDocument() throws Exception {
+        FXOMDocument classUnderTest = new FXOMDocument();
+        boolean withWildCardImports = false;
+        String generatedFxmlText = classUnderTest.getFxmlText(withWildCardImports);
+
+        assertEquals("", generatedFxmlText);
+    }
+
+    @Test
+    public void that_fxml_with_defines_loads_without_error_without_normalization() throws Exception {
+        URL resource = getClass().getResource("DynamicScreenSize.fxml");
+        String validFxmlText = FXOMDocument.readContentFromURL(resource);
+
+        FXOMDocument classUnderTest = waitFor(() -> new FXOMDocument(validFxmlText, resource, null, null, false));
+        String beforeNormalization = classUnderTest.getFxmlText(false);
+        assertFalse(beforeNormalization.isBlank());
+    }
+
+    @Test
+    public void that_missing_imports_during_defines_resolution_cause_exception() throws Exception {
+        URL resource = getClass().getResource("DynamicScreenSize.fxml");
+        String validFxmlText = FXOMDocument.readContentFromURL(resource);
+
+        Throwable t = assertThrows(Throwable.class,
+                       () -> waitFor(() -> new FXOMDocument(validFxmlText, resource, null, null, true /* normalization enabled */)));
+
+        if (t.getCause() != null) {
+            t = t.getCause();
+        }
+
+        assertEquals(IllegalStateException.class, t.getClass());
+        assertTrue(t.getMessage().contains("Bug in FXOMRefresher"));
+    }
+
+    private <T> T waitFor(Callable<T> callable) throws Exception {
+        FutureTask<T> task = new FutureTask<T>(callable);
+        if (Platform.isFxApplicationThread()) {
+            return callable.call();
+        } else {
+            Platform.runLater(()->task.run());
+            return task.get();
+        }
+    }
+}

kit/src/test/java/com/oracle/javafx/scenebuilder/kit/fxom/FXOMLoaderTest.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2022, Gluon and/or its affiliates.
+ * All rights reserved. Use is subject to license terms.
+ *
+ * This file is available and licensed under the following license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the distribution.
+ *  - Neither the name of Oracle Corporation and Gluon nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package com.oracle.javafx.scenebuilder.kit.fxom;
+
+import static org.junit.Assert.assertTrue;
+
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Consumer;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import com.oracle.javafx.scenebuilder.kit.JfxInitializer;
+
+public class FXOMLoaderTest {
+
+    private FXOMLoader classUnderTest;
+
+    @Before
+    public void init() {
+        JfxInitializer.initialize();
+    }
+
+    @Test
+    public void that_LoadException_caused_by_XMLStreamException_is_handled() throws Exception {
+        String invalidXmlText = FXOMDocument.readContentFromURL(getClass().getResource("IncompleteXml.fxml"));
+        URL validResource = getClass().getResource("CompleteFxml.fxml");
+        String validFxmlText = FXOMDocument.readContentFromURL(validResource);
+        FXOMDocument document = new FXOMDocument(validFxmlText, validResource, null, null);
+
+        // When there are exceptions, than the error handler should store these here
+        Map<Class<?>, Throwable> handledErrors = new HashMap<>();
+
+        // In Scene Builder, the error is displayed in an error dialog.
+        // For testing, this custom error handler replaces the dialog.
+        Consumer<Exception> errorHandler = ex -> {
+            handledErrors.put(ex.getClass(), ex);
+            if (ex.getCause() != null) {
+                handledErrors.put(ex.getCause().getClass(), ex.getCause());
+            }
+        };
+
+        classUnderTest = new FXOMLoader(document, errorHandler);
+        classUnderTest.load(invalidXmlText);
+
+        assertTrue(handledErrors.containsKey(javax.xml.stream.XMLStreamException.class));
+        assertTrue(handledErrors.containsKey(javafx.fxml.LoadException.class));
+    }
+}

kit/src/test/resources/com/oracle/javafx/scenebuilder/kit/fxom/BrokenByUserData.fxml

@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<?import javafx.geometry.Insets?>
+<?import javafx.scene.control.Button?>
+<?import javafx.scene.control.TableColumn?>
+<?import javafx.scene.control.TableView?>
+<?import javafx.scene.layout.HBox?>
+<?import javafx.scene.layout.VBox?>
+
+<VBox prefWidth="940.0" xmlns="http://javafx.com/javafx/11.0.1" xmlns:fx="http://javafx.com/fxml/1" fx:controller="com.example.controller">
+    <userData>
+        <fx:reference source="controller"/>
+    </userData>
+   <children>
+      <HBox spacing="7.0">
+         <children>
+            <Button fx:id="importButton" mnemonicParsing="false" onAction="#onImport" text="Import methods" />
+            <Button fx:id="exportButton" mnemonicParsing="false" onAction="#onExport" text="Export selection" />
+            <Button fx:id="analysisButton" mnemonicParsing="false" onAction="#onAnalysis" text="Use method for analysis" />
+            <Button fx:id="deleteButton" mnemonicParsing="false" onAction="#onDelete" text="Delete selected methods" />
+         </children>
+         <opaqueInsets>
+            <Insets />
+         </opaqueInsets>
+         <padding>
+            <Insets bottom="7.0" left="7.0" right="7.0" top="7.0" />
+         </padding>
+      </HBox>
+      <TableView fx:id="tableView" minWidth="700.0" VBox.vgrow="ALWAYS">
+        <columns>
+          <TableColumn fx:id="kitId" editable="false" prefWidth="178.0" text="Kit ID" />
+          <TableColumn fx:id="methodId" editable="false" prefWidth="164.0" text="Method ID" />
+          <TableColumn fx:id="revision" editable="false" prefWidth="148.0" text="Revision" />
+        </columns>
+      </TableView>
+   </children>
+</VBox>