Added warning and informative logging for cases of removing files fro…

…m recent items and for cases where errors occured during FXML loading.
gluonhq · Jan 3, 2023 · f11a8a1 · f11a8a1
1 parent 8d14862
commit f11a8a1
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 18 deletions.
diff --git a/app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java b/app/src/main/java/com/oracle/javafx/scenebuilder/app/SceneBuilderApp.java
@@ -99,6 +99,8 @@
  */
 public class SceneBuilderApp extends Application implements AppPlatform.AppNotificationHandler {
 
+    private static final Logger LOGGER = Logger.getLogger(SceneBuilderApp.class.getName());
+
     public enum ApplicationControlAction {
         ABOUT,
         CHECK_UPDATES,
@@ -497,11 +499,24 @@ private void createEmptyDocumentWindow() {
         newWindow.updateWithDefaultContent();
     }
 
+    /**
+     * By default all necessary actions to open a single file or a group of files take place in this method.
+     * If it is required to perform certain actions after successfully loading all files, please use {@code handleOpenFilesAction(List<String> files, Runnable onSuccess)} instead.
+     * 
+     * All error handling takes place here within, there is no way yet to access exceptional results and to work with them.
+     */
     @Override
     public void handleOpenFilesAction(List<String> files) {
         handleOpenFilesAction(files, null);
     }
 
+    /**
+     * As file loading errors are handled within this method (all exceptions are handled within), it can be helpful to be able to run a certain action after successful file loading (e.g. closing a certain stage).
+     * For this case this method offers the argument {@code Runnable onSuccess} which will be executed after successful file open activity. The {@code Runnable onSuccess} is only ran once, despite how many files have been loaded.
+     *  
+     * @param files List of Strings denoting file paths to be opened
+     * @param onSuccess {@link Runnable} to be executed after all files have been opened successfully
+     */
     public void handleOpenFilesAction(List<String> files, Runnable onSuccess) {
         assert files != null;
         assert files.isEmpty() == false;
@@ -720,9 +735,11 @@ private FileOpenResult performOpenFiles(List<File> fxmlFiles) {
         assert fxmlFiles != null;
         assert fxmlFiles.isEmpty() == false;
 
+        LOGGER.log(Level.FINE, "Opening {0} files...", fxmlFiles.size());
         final Map<File, Exception> exceptions = new HashMap<>();
         final List<File> openedFiles = new ArrayList<>();
         for (File fxmlFile : fxmlFiles) {
+            LOGGER.log(Level.FINE, "Attempting to open file {0}", fxmlFile);
             try {
                 final DocumentWindowController dwc
                         = lookupDocumentWindowControllers(fxmlFile.toURI().toURL());
@@ -735,8 +752,10 @@ private FileOpenResult performOpenFiles(List<File> fxmlFiles) {
                     hostWindow.loadFromFile(fxmlFile);
                     hostWindow.openWindow();
                     openedFiles.add(fxmlFile);
+                    LOGGER.log(Level.INFO, "Successfully opened file {0}", fxmlFile);
                 }
             } catch (Exception xx) {
+                LOGGER.log(Level.WARNING, "Failed to open file: %s".formatted(fxmlFile), xx);
                 exceptions.put(fxmlFile, xx);
             }
         }
@@ -746,7 +765,11 @@ private FileOpenResult performOpenFiles(List<File> fxmlFiles) {
             final PreferencesController pc = PreferencesController.getSingleton();
             pc.getRecordGlobal().addRecentItems(openedFiles);
         }
-
+        if (exceptions.size() > 0) {
+            LOGGER.log(Level.WARNING, "Failed to open {0} of {1} files!", new Object[] {exceptions.size(), fxmlFiles.size()});
+        } else {
+            LOGGER.log(Level.FINE, "Successfully opened all files.");
+        }
         return new FileOpenResult(fxmlFiles, exceptions);
     }
 
@@ -1124,9 +1147,6 @@ public static void applyToAllDocumentWindows(Consumer<DocumentWindowController>
      * a map per file.
      */
     record FileOpenResult(List<File> filesToOpen, Map<File,Exception> errors) {
-        public boolean isSuccess() {
-            return errors.isEmpty();
-        }
         public boolean hasErrors() {
             return !errors.isEmpty();
         }

diff --git a/.../java/com/oracle/javafx/scenebuilder/app/welcomedialog/WelcomeDialogWindowController.java b/.../java/com/oracle/javafx/scenebuilder/app/welcomedialog/WelcomeDialogWindowController.java
@@ -38,6 +38,8 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 import com.oracle.javafx.scenebuilder.app.SceneBuilderApp;
@@ -67,6 +69,8 @@
 
 public class WelcomeDialogWindowController extends TemplatesBaseWindowController {
 
+    private static final Logger LOGGER = Logger.getLogger(WelcomeDialogWindowController.class.getName());
+
     @FXML
     private BorderPane contentPane;
 
@@ -207,7 +211,6 @@ private void openDocument() {
                 new FileChooser.ExtensionFilter(I18N.getString("file.filter.label.fxml"), "*.fxml")
         );
         fileChooser.setInitialDirectory(EditorController.getNextInitialDirectory());
-
         List<File> fxmlFiles = fileChooser.showOpenMultipleDialog(getStage());
 
         // no file was selected, so nothing to do
@@ -225,7 +228,6 @@ private void openDocument() {
     protected static AlertDialog questionMissingFilesCleanup(Stage stage, List<String> missingFiles) {
         String withPath = missingFiles.stream()
                                       .collect(Collectors.joining(System.lineSeparator()));
-
         AlertDialog question = new AlertDialog(stage);
         StringBuilder shortMessage = new StringBuilder();
         if (missingFiles.size() > 1) {
@@ -292,8 +294,9 @@ private void openFilesAndHideStage(List<String> files) {
      * @param fileLoader Determines how files are loaded.
      */
     void handleOpen(List<String> filePaths, 
-                              Consumer<List<String>> missingFilesHandler,
-                              Consumer<List<String>> fileLoader) {
+                    Consumer<List<String>> missingFilesHandler,
+                    Consumer<List<String>> fileLoader) {
+
         if (filePaths.isEmpty()) {
             return;
         }
@@ -303,7 +306,7 @@ void handleOpen(List<String> filePaths,
 
         List<String> missingFiles = candidates.getOrDefault(Boolean.FALSE, new ArrayList<>());
         missingFilesHandler.accept(missingFiles);
-        
+
         List<String> paths = candidates.getOrDefault(Boolean.TRUE, new ArrayList<>())
                                        .stream()
                                        .toList();
@@ -315,6 +318,7 @@ void handleOpen(List<String> filePaths,
     }
 
     private void removeMissingFilesFromPrefs(List<String> missingFiles) {
+        missingFiles.forEach(fxmlFileName->LOGGER.log(Level.INFO, "Removing missing file from recent items: {0}", fxmlFileName));
         PreferencesRecordGlobal preferencesRecordGlobal = PreferencesController.getSingleton().getRecordGlobal();
         preferencesRecordGlobal.removeRecentItems(missingFiles);
     }
@@ -330,6 +334,7 @@ private void showMasker(Runnable onEndAction) {
             if (isFinished) {
                 Platform.runLater(() -> {
                     onEndAction.run();
+                    // restore state in case welcome dialog is opened again
                     contentPane.setDisable(false);
                     masker.setVisible(false);
                 });

diff --git a/app/src/test/java/com/oracle/javafx/scenebuilder/app/JfxInitializer.java b/app/src/test/java/com/oracle/javafx/scenebuilder/app/JfxInitializer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Gluon and/or its affiliates.
+ * Copyright (c) 2021, 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:

diff --git a/kit/pom.xml b/kit/pom.xml
@@ -91,14 +91,6 @@
             <version>1.0.4</version>
             <scope>runtime</scope>
         </dependency>
-
-        <!-- Test -->
-        <dependency>
-            <groupId>org.assertj</groupId>
-            <artifactId>assertj-core</artifactId>
-            <version>3.19.0</version>
-            <scope>test</scope>
-        </dependency>
     </dependencies>
 
     <build>

diff --git a/pom.xml b/pom.xml
@@ -70,6 +70,12 @@
             <artifactId>junit-jupiter-engine</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <version>3.19.0</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>