OpenOffice Refactor subtasks

jabgui/src/main/java/org/jabref/gui/openoffice/OOBibBaseConnect.java

@@ -218,16 +218,7 @@ public boolean isDocumentConnectionMissing() {
         return false;
     }
 
-    /// Either return a valid XTextDocument or throw NoDocumentException.
-    public XTextDocument getXTextDocumentOrThrow()
-            throws
-            NoDocumentException {
-        if (isDocumentConnectionMissing()) {
-            throw new NoDocumentException("Not connected to document");
-        }
-        return this.xTextDocument;
-    }
-
+    /// Returns either a valid XTextDocument in OOResult or a NoDocumentException error
     public OOResult<XTextDocument, OOError> getXTextDocument() {
         if (isDocumentConnectionMissing()) {
             return OOResult.error(OOError.from(new NoDocumentException()));

jabgui/src/main/java/org/jabref/gui/openoffice/OpenOfficePanel.java

@@ -225,7 +225,8 @@ private void initPanel() {
                 ooBase.guiActionSelectDocument(false);
             } catch (WrappedTargetException
                      | NoSuchElementException ex) {
-                throw new RuntimeException(ex);
+                LOGGER.warn("Unable to select document to work on", ex);
+                OOError.fromMisc(ex).setTitle("Unable to select document to work on").showErrorDialog(dialogService);
             }
         });
 
@@ -348,23 +349,23 @@ private void exportEntries() {
     }
 
     private List<BibDatabase> getBaseList() {
-        List<BibDatabase> databases = new ArrayList<>();
         if (openOfficePreferences.getUseAllDatabases()) {
-            for (BibDatabaseContext database : stateManager.getOpenDatabases()) {
-                databases.add(database.getDatabase());
-            }
-        } else {
-            databases.add(stateManager.getActiveDatabase()
-                                      .map(BibDatabaseContext::getDatabase)
-                                      .orElse(new BibDatabase()));
+            return new ArrayList<>(stateManager.getOpenDatabases().stream()
+                                               .map(BibDatabaseContext::getDatabase)
+                                               .toList());
         }
-
-        return databases;
+        return new ArrayList<>(List.of(
+                stateManager.getActiveDatabase()
+                            .map(BibDatabaseContext::getDatabase)
+                            .orElseGet(BibDatabase::new)));
     }
 
     private void connectAutomatically() {
         DetectOpenOfficeInstallation officeInstallation = new DetectOpenOfficeInstallation(openOfficePreferences, dialogService);
 
+        final String errorTitle = Localization.lang("Autodetection failed");
+        final String progressMessage = Localization.lang("Autodetecting paths...");
+
         if (officeInstallation.isExecutablePathDefined()) {
             connect();
         } else {
@@ -386,9 +387,9 @@ protected List<Path> call() {
                 }
             });
 
-            taskConnectIfInstalled.setOnFailed(_ -> dialogService.showErrorDialogAndWait(Localization.lang("Autodetection failed"), Localization.lang("Autodetection failed"), taskConnectIfInstalled.getException()));
+            taskConnectIfInstalled.setOnFailed(_ -> dialogService.showErrorDialogAndWait(errorTitle, errorTitle, taskConnectIfInstalled.getException()));
 
-            dialogService.showProgressDialog(Localization.lang("Autodetecting paths..."), Localization.lang("Autodetecting paths..."), taskConnectIfInstalled);
+            dialogService.showProgressDialog(progressMessage, progressMessage, taskConnectIfInstalled);
             taskExecutor.execute(taskConnectIfInstalled);
         }
     }
@@ -397,6 +398,9 @@ private void connectManually() {
         DirectoryDialogConfiguration fileDialogConfiguration = new DirectoryDialogConfiguration.Builder().withInitialDirectory(System.getProperty("user.home")).build();
         Optional<Path> selectedPath = dialogService.showDirectorySelectionDialog(fileDialogConfiguration);
 
+        final String errorTitle = Localization.lang("Could not connect to running OpenOffice/LibreOffice.");
+        final String extendedErrorTitle = Localization.lang("If connecting manually, please verify program and library paths.");
+
         DetectOpenOfficeInstallation officeInstallation = new DetectOpenOfficeInstallation(openOfficePreferences, dialogService);
 
         if (selectedPath.isPresent()) {
@@ -406,12 +410,12 @@ private void connectManually() {
                               if (value) {
                                   connect();
                               } else {
-                                  dialogService.showErrorDialogAndWait(Localization.lang("Could not connect to running OpenOffice/LibreOffice."), Localization.lang("If connecting manually, please verify program and library paths."));
+                                  dialogService.showErrorDialogAndWait(errorTitle, extendedErrorTitle);
                               }
                           })
                           .executeWith(taskExecutor);
         } else {
-            dialogService.showErrorDialogAndWait(Localization.lang("Could not connect to running OpenOffice/LibreOffice."), Localization.lang("If connecting manually, please verify program and library paths."));
+            dialogService.showErrorDialogAndWait(errorTitle, extendedErrorTitle);
         }
     }
 
@@ -440,6 +444,11 @@ private void updateButtonAvailability() {
     }
 
     private void connect() {
+        final String connectionError = Localization.lang("Could not connect to running OpenOffice/LibreOffice.");
+        final String autodetectionFailedError = Localization.lang("Autodetection failed");
+        final String progressMessage = Localization.lang("Autodetecting paths...");
+        final String loggerMessage = "Could not connect to running OpenOffice/LibreOffice";
+
         Task<OOBibBase> connectTask = new Task<>() {
             @Override
             protected OOBibBase call() throws BootstrapException, CreationException, IOException, InterruptedException {
@@ -457,7 +466,9 @@ protected OOBibBase call() throws BootstrapException, CreationException, IOExcep
                 ooBase.guiActionSelectDocument(true);
             } catch (WrappedTargetException
                      | NoSuchElementException e) {
-                throw new RuntimeException(e);
+                LOGGER.warn("Unable to connect to document", e);
+                OOError.fromMisc(e).showErrorDialog(dialogService);
+                return;
             }
 
             // Enable actions that depend on a connection
@@ -469,16 +480,16 @@ protected OOBibBase call() throws BootstrapException, CreationException, IOExcep
             LOGGER.error("autodetect failed", ex);
             switch (ex) {
                 case UnsatisfiedLinkError unsatisfiedLinkError -> {
-                    LOGGER.warn("Could not connect to running OpenOffice/LibreOffice", unsatisfiedLinkError);
+                    LOGGER.warn(loggerMessage, unsatisfiedLinkError);
 
                     dialogService.showErrorDialogAndWait(Localization.lang("Unable to connect. One possible reason is that JabRef "
                             + "and OpenOffice/LibreOffice are not both running in either 32 bit mode or 64 bit mode."));
                 }
                 case IOException ioException -> {
-                    LOGGER.warn("Could not connect to running OpenOffice/LibreOffice", ioException);
+                    LOGGER.warn(loggerMessage, ioException);
 
-                    dialogService.showErrorDialogAndWait(Localization.lang("Could not connect to running OpenOffice/LibreOffice."),
-                            Localization.lang("Could not connect to running OpenOffice/LibreOffice.")
+                    dialogService.showErrorDialogAndWait(connectionError,
+                            connectionError
                                     + "\n"
                                     + Localization.lang("Make sure you have installed OpenOffice/LibreOffice with Java support.") + "\n"
                                     + Localization.lang("If connecting manually, please verify program and library paths.") + "\n" + "\n" + Localization.lang("Error message:"),
@@ -490,11 +501,11 @@ protected OOBibBase call() throws BootstrapException, CreationException, IOExcep
                 }
                 case null,
                      default ->
-                        dialogService.showErrorDialogAndWait(Localization.lang("Autodetection failed"), Localization.lang("Autodetection failed"), ex);
+                        dialogService.showErrorDialogAndWait(autodetectionFailedError, autodetectionFailedError, ex);
             }
         });
 
-        dialogService.showProgressDialog(Localization.lang("Autodetecting paths..."), Localization.lang("Autodetecting paths..."), connectTask);
+        dialogService.showProgressDialog(progressMessage, progressMessage, connectTask);
         taskExecutor.execute(connectTask);
     }
 
@@ -588,15 +599,9 @@ private void pushEntries(CitationType citationType, boolean addPageInfo) {
     /// @return true if all entries have citation keys, if it so may be after generating them
     private boolean checkThatEntriesHaveKeys(List<BibEntry> entries) {
         // Check if there are empty keys
-        boolean emptyKeys = false;
-        for (BibEntry entry : entries) {
-            if (entry.getCitationKey().isEmpty()) {
-                // Found one, no need to look further for now
-                emptyKeys = true;
-                break;
-            }
-        }
-
+        // Found one, no need to look further for now
+        boolean emptyKeys = entries.stream()
+                                   .anyMatch(entry -> entry.getCitationKey().isEmpty());
         // If no empty keys, return true
         if (!emptyKeys) {
             return true;

jablib/src/main/java/org/jabref/logic/openoffice/action/EditMerge.java

@@ -141,6 +141,8 @@ private static boolean checkAddToGroup(ScanState state, CitationGroup group, XTe
             return false;
         }
 
+        final String loggerMessage = "MergeCitationGroups: cursorBetween.end != currentGroupCursor.end";
+
         Objects.requireNonNull(state.currentGroupCursor);
         Objects.requireNonNull(state.cursorBetween);
         Objects.requireNonNull(state.prev);
@@ -187,10 +189,20 @@ private static boolean checkAddToGroup(ScanState state, CitationGroup group, XTe
 
         // assume: currentGroupCursor.getEnd() == cursorBetween.getEnd()
         if (UnoTextRange.compareEnds(state.cursorBetween, state.currentGroupCursor) != 0) {
-            LOGGER.warn("MergeCitationGroups: cursorBetween.end != currentGroupCursor.end");
-            throw new IllegalStateException("MergeCitationGroups failed");
+            LOGGER.warn(loggerMessage);
+            return false;
         }
 
+        return checkCouldExpand(state, currentRange, loggerMessage);
+    }
+
+    /// Helper method for checkAddToGroup. Tries to expand state.currentGroupCursor and state.cursorBetween by going right to reach rangeStart.
+    ///
+    /// @param state         The CitationGroup to test.
+    /// @param currentRange  The XTextRange corresponding to group.
+    /// @param loggerMessage The failure message for the LOGGER.
+    /// @return false if cannot expand, true if can.
+    private static boolean checkCouldExpand(ScanState state, XTextRange currentRange, String loggerMessage) {
         /*
          * Try to expand state.currentGroupCursor and state.cursorBetween by going right to reach
          * rangeStart.
@@ -219,11 +231,10 @@ private static boolean checkAddToGroup(ScanState state, CitationGroup group, XTe
 
             // These two should move in sync:
             if (UnoTextRange.compareEnds(state.cursorBetween, state.currentGroupCursor) != 0) {
-                LOGGER.warn("MergeCitationGroups: cursorBetween.end != currentGroupCursor.end (during expand)");
-                throw new IllegalStateException("MergeCitationGroups failed");
+                LOGGER.warn(loggerMessage + " (during expand)");
+                return false;
             }
         }
-
         return couldExpand;
     }
 
@@ -255,7 +266,7 @@ private static void addToCurrentGroup(ScanState state, CitationGroup group, XTex
 
         if (UnoTextRange.compareEnds(state.cursorBetween, state.currentGroupCursor) != 0) {
             LOGGER.warn("MergeCitationGroups: cursorBetween.end != currentGroupCursor.end");
-            throw new IllegalStateException("MergeCitationGroups failed");
+            return;
         }
 
         /* Store data about last entry in currentGroup */

jablib/src/main/java/org/jabref/logic/openoffice/frontend/OOFrontend.java

@@ -325,7 +325,7 @@ public List<RangeForOverlapCheck<CitationGroupId>> viewCursorRanges(XTextDocumen
     ///
     /// Note: Here we directly communicate to the document, not through the backend. This is because mapping ranges to footnote marks does not depend on how do we mark or structure those ranges.
     public List<RangeForOverlapCheck<CitationGroupId>>
-    footnoteMarkRanges(XTextDocument doc, List<RangeForOverlapCheck<CitationGroupId>> citationRanges) {
+    footnoteMarkRanges(List<RangeForOverlapCheck<CitationGroupId>> citationRanges) {
         // We partition by XText and use a single range from
         // each partition to get at the corresponding footnotemark range.
 
@@ -402,7 +402,7 @@ static String rangeOverlapsToMessage(List<RangeOverlap<RangeForOverlapCheck<Cita
         // ranges.addAll(userRanges);
         ranges.addAll(bibliographyRanges(doc));
         ranges.addAll(citationRanges);
-        ranges.addAll(footnoteMarkRanges(doc, citationRanges));
+        ranges.addAll(footnoteMarkRanges(citationRanges));
 
         List<RangeOverlap<RangeForOverlapCheck<CitationGroupId>>> overlaps =
                 RangeOverlapBetween.findFirst(doc,
@@ -432,7 +432,7 @@ public OOVoidResult<JabRefException> checkRangeOverlaps(XTextDocument doc,
         ranges.addAll(userRanges);
         ranges.addAll(bibliographyRanges(doc));
         ranges.addAll(citationRanges);
-        ranges.addAll(footnoteMarkRanges(doc, citationRanges));
+        ranges.addAll(footnoteMarkRanges(citationRanges));
 
         List<RangeOverlap<RangeForOverlapCheck<CitationGroupId>>> overlaps =
                 RangeOverlapWithin.findOverlappingRanges(doc, ranges, requireSeparation, reportAtMost);