Refactor BibDatabase Migrations

src/main/java/org/jabref/gui/importer/actions/CheckForNewEntryTypesAction.java

@@ -37,7 +37,6 @@ public boolean isActionNecessary(ParserResult parserResult) {
 
     @Override
     public void performAction(BasePanel panel, ParserResult parserResult) {
-
         BibDatabaseMode mode = getBibDatabaseModeFromParserResult(parserResult);
 
         List<EntryType> typesToStore = determineEntryTypesToSave(panel, getListOfUnknownAndUnequalCustomizations(parserResult), mode);

src/main/java/org/jabref/gui/importer/actions/MergeReviewIntoComment.java

@@ -0,0 +1,23 @@
+package org.jabref.gui.importer.actions;
+
+import org.jabref.gui.BasePanel;
+import org.jabref.logic.importer.ParserResult;
+import org.jabref.logic.migrations.MergeReviewIntoCommentMigration;
+
+public class MergeReviewIntoComment implements GUIPostOpenAction {
+
+    @Override
+    public boolean isActionNecessary(ParserResult parserResult) {
+        return MergeReviewIntoCommentMigration.needsMigration(parserResult);
+    }
+
+    @Override
+    public void performAction(BasePanel basePanel, ParserResult parserResult) {
+        MergeReviewIntoCommentMigration migration = new MergeReviewIntoCommentMigration();
+
+        migration.performMigration(parserResult);
+        if (new MergeReviewIntoCommentConfirmation(basePanel).askUserForMerge(MergeReviewIntoCommentMigration.collectConflicts(parserResult))) {
+            migration.performConflictingMigration(parserResult);
+        }
+    }
+}

src/main/java/org/jabref/gui/dialogs/MergeReviewIntoCommentUIManager.java → src/main/java/org/jabref/gui/importer/actions/MergeReviewIntoCommentConfirmation.java

@@ -1,26 +1,33 @@
-package org.jabref.gui.dialogs;
+package org.jabref.gui.importer.actions;
 
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
 import javax.swing.JOptionPane;
 
+import org.jabref.gui.BasePanel;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.entry.BibEntry;
 
-public class MergeReviewIntoCommentUIManager {
+public class MergeReviewIntoCommentConfirmation {
+
+    private final BasePanel panel;
+
+    public MergeReviewIntoCommentConfirmation(BasePanel panel) {
+        this.panel = panel;
+    }
 
     public boolean askUserForMerge(List<BibEntry> conflicts) {
-        List<String> bibKeys = conflicts.stream()
+        String bibKeys = conflicts.stream()
                 .map(BibEntry::getCiteKeyOptional)
                 .filter(Optional::isPresent)
                 .map(Optional::get)
-                .collect(Collectors.toList());
+                .collect(Collectors.joining(",\n"));
 
         int answer = JOptionPane.showConfirmDialog(
-                null,
-                String.join(",\n", bibKeys) + " " +
+                panel,
+                bibKeys + " " +
                         Localization.lang("has/have both a 'Comment' and a 'Review' field.") + "\n" +
                         Localization.lang("Since the 'Review' field was deprecated in JabRef 4.2, these two fields are about to be merged into the 'Comment' field.") + "\n" +
                         Localization.lang("The conflicting fields of these entries will be merged into the 'Comment' field."),

src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java

@@ -7,6 +7,7 @@
 import java.nio.file.attribute.FileTime;
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -50,22 +51,23 @@
 import org.slf4j.LoggerFactory;
 
 // The action concerned with opening an existing database.
-
 public class OpenDatabaseAction extends MnemonicAwareAction {
 
     public static final Logger LOGGER = LoggerFactory.getLogger(OpenDatabaseAction.class);
     // List of actions that may need to be called after opening the file. Such as
     // upgrade actions etc. that may depend on the JabRef version that wrote the file:
-    private static final List<GUIPostOpenAction> POST_OPEN_ACTIONS = new ArrayList<>();
-
-    static {
-        // Add the action for checking for new custom entry types loaded from the BIB file:
-        POST_OPEN_ACTIONS.add(new CheckForNewEntryTypesAction());
-        // Add the action for the new external file handling system in version 2.3:
-        POST_OPEN_ACTIONS.add(new FileLinksUpgradeWarning());
-        // Add the action for warning about and handling duplicate BibTeX keys:
-        POST_OPEN_ACTIONS.add(new HandleDuplicateWarnings());
-    }
+    private static final List<GUIPostOpenAction> POST_OPEN_ACTIONS = Arrays.asList(
+            // Migrations:
+            // Warning for migrating the Review into the Comment field
+            new MergeReviewIntoComment(),
+            // External file handling system in version 2.3:
+            new FileLinksUpgradeWarning(),
+
+            // Check for new custom entry types loaded from the BIB file:
+            new CheckForNewEntryTypesAction(),
+            // Warning about and handling duplicate BibTeX keys:
+            new HandleDuplicateWarnings());
+
 
     private final boolean showDialog;
     private final JabRefFrame frame;
@@ -99,7 +101,6 @@ public void actionPerformed(ActionEvent e) {
         List<Path> filesToOpen = new ArrayList<>();
 
         if (showDialog) {
-
             DialogService ds = new FXDialogService();
             FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
                     .addExtensionFilter(FileType.BIBTEX_DB)
@@ -146,7 +147,7 @@ public void openFiles(List<Path> filesToOpen, boolean raisePanel) {
         int removed = 0;
 
         // Check if any of the files are already open:
-        for (Iterator<Path> iterator = filesToOpen.iterator(); iterator.hasNext();) {
+        for (Iterator<Path> iterator = filesToOpen.iterator(); iterator.hasNext(); ) {
             Path file = iterator.next();
             for (int i = 0; i < frame.getTabbedPane().getTabCount(); i++) {
                 BasePanel basePanel = frame.getBasePanelAt(i);
@@ -224,7 +225,6 @@ private void openTheFile(Path file, boolean raisePanel) {
                             Localization.lang("Error"), JOptionPane.ERROR_MESSAGE);
                     return;
                 }
-
             }
 
             if (BackupManager.checkForBackupFile(fileToLoad)) {
@@ -284,5 +284,4 @@ private BasePanel addNewDatabase(ParserResult result, final Path file, boolean r
 
         return basePanel;
     }
-
 }

src/main/java/org/jabref/logic/importer/OpenDatabase.java

@@ -2,15 +2,14 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import org.jabref.logic.importer.fileformat.BibtexImporter;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.specialfields.SpecialFieldsUtils;
 import org.jabref.logic.util.io.FileBasedLock;
 import org.jabref.migrations.ConvertLegacyExplicitGroups;
-import org.jabref.migrations.MergeReviewIntoComment;
 import org.jabref.migrations.PostOpenMigration;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.util.FileUpdateMonitor;
@@ -29,7 +28,6 @@ private OpenDatabase() {
      * Load database (bib-file)
      *
      * @param name Name of the BIB-file to open
-     * @param fileMonitor
      * @return ParserResult which never is null
      */
     public static ParserResult loadDatabase(String name, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) {
@@ -88,8 +86,7 @@ public static ParserResult loadDatabase(File fileToOpen, ImportFormatPreferences
     }
 
     private static void performLoadDatabaseMigrations(ParserResult parserResult) {
-
-        List<PostOpenMigration> postOpenMigrations = Arrays.asList(new ConvertLegacyExplicitGroups(), new MergeReviewIntoComment());
+        List<PostOpenMigration> postOpenMigrations = Collections.singletonList(new ConvertLegacyExplicitGroups());
 
         for (PostOpenMigration migration : postOpenMigrations) {
             migration.performMigration(parserResult);

src/main/java/org/jabref/logic/migrations/MergeReviewIntoCommentMigration.java

@@ -0,0 +1,75 @@
+package org.jabref.logic.migrations;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+import org.jabref.logic.importer.ParserResult;
+import org.jabref.logic.l10n.Localization;
+import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.FieldName;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class MergeReviewIntoCommentMigration {
+    public static final Logger LOGGER = LoggerFactory.getLogger(MergeReviewIntoCommentMigration.class);
+
+    public static boolean needsMigration(ParserResult parserResult) {
+        return parserResult.getDatabase().getEntries().stream()
+                .anyMatch(bibEntry -> bibEntry.getField(FieldName.REVIEW).isPresent());
+    }
+
+    public void performMigration(ParserResult parserResult) {
+        /* This migration only handles the non-conflicting entries.
+         * For the other see this.performConflictingMigration().
+         */
+        List<BibEntry> entries = Objects.requireNonNull(parserResult).getDatabase().getEntries();
+
+        entries.stream()
+                .filter(MergeReviewIntoCommentMigration::hasReviewField)
+                .filter(entry -> !MergeReviewIntoCommentMigration.hasCommentField(entry))
+                .forEach(entry -> migrate(entry, parserResult));
+    }
+
+    public static List<BibEntry> collectConflicts(ParserResult parserResult) {
+        List<BibEntry> entries = Objects.requireNonNull(parserResult).getDatabase().getEntries();
+
+        return entries.stream()
+                .filter(MergeReviewIntoCommentMigration::hasReviewField)
+                .filter(MergeReviewIntoCommentMigration::hasCommentField)
+                .collect(Collectors.toList());
+    }
+
+    public void performConflictingMigration(ParserResult parserResult) {
+        collectConflicts(parserResult).forEach(entry -> migrate(entry, parserResult));
+    }
+
+    private String mergeCommentFieldIfPresent(BibEntry entry, String review) {
+        if (entry.getField(FieldName.COMMENT).isPresent()) {
+            LOGGER.info(String.format("Both Comment and Review fields are present in %s! Merging them into the comment field.", entry.getAuthorTitleYear(150)));
+            return String.format("%s\n%s:\n%s", entry.getField(FieldName.COMMENT).get().trim(), Localization.lang("Review"), review.trim());
+        }
+        return review;
+    }
+
+    private static boolean hasCommentField(BibEntry entry) {
+        return entry.getField(FieldName.COMMENT).isPresent();
+    }
+
+    private static boolean hasReviewField(BibEntry entry) {
+        return entry.getField(FieldName.REVIEW).isPresent();
+    }
+
+    private void migrate(BibEntry entry, ParserResult parserResult) {
+        if (hasReviewField(entry)) {
+            updateFields(entry, mergeCommentFieldIfPresent(entry, entry.getField(FieldName.REVIEW).get()));
+            parserResult.wasChangedOnMigration();
+        }
+    }
+
+    private void updateFields(BibEntry entry, String review) {
+        entry.setField(FieldName.COMMENT, review);
+        entry.clearField(FieldName.REVIEW);
+    }
+}

src/main/java/org/jabref/migrations/FileLinksUpgradeWarning.java

@@ -43,7 +43,7 @@
  */
 public class FileLinksUpgradeWarning implements GUIPostOpenAction {
 
-    private static final String[] FIELDS_TO_LOOK_FOR = new String[] {FieldName.PDF, FieldName.PS};
+    private static final String[] FIELDS_TO_LOOK_FOR = new String[]{FieldName.PDF, FieldName.PS};
 
     private boolean offerChangeSettings;
 
@@ -56,7 +56,6 @@ public class FileLinksUpgradeWarning implements GUIPostOpenAction {
      * GUIGlobals.FILE_FIELD.
      *
      * @param database The database to modify.
-     * @param fields   The fields to find links in.
      * @return A CompoundEdit specifying the undo operation for the whole operation.
      */
     private static NamedCompound upgradePdfPsToFile(BibDatabase database) {
@@ -78,7 +77,7 @@ private static NamedCompound upgradePdfPsToFile(BibDatabase database) {
     /**
      * This method should be performed if the major/minor versions recorded in the ParserResult
      * are less than or equal to 2.2.
-     * @param pr
+     *
      * @return true if the file was written by a jabref version <=2.2
      */
     @Override
@@ -91,18 +90,16 @@ public boolean isActionNecessary(ParserResult pr) {
         // If the "file" directory is not set, offer to migrate pdf/ps dir:
         offerSetFileDir = !Globals.prefs.hasKey(FieldName.FILE + FileDirectoryPreferences.DIR_SUFFIX)
                 && (Globals.prefs.hasKey(FieldName.PDF + FileDirectoryPreferences.DIR_SUFFIX)
-                        || Globals.prefs.hasKey(FieldName.PS + FileDirectoryPreferences.DIR_SUFFIX));
+                || Globals.prefs.hasKey(FieldName.PS + FileDirectoryPreferences.DIR_SUFFIX));
 
         // First check if this warning is disabled:
         return Globals.prefs.getBoolean(JabRefPreferences.SHOW_FILE_LINKS_UPGRADE_WARNING)
                 && isThereSomethingToBeDone();
     }
 
-    /**
+    /*
      * This method presents a dialog box explaining and offering to make the
      * changes. If the user confirms, the changes are performed.
-     * @param panel
-     * @param parserResult
      */
     @Override
     public void performAction(BasePanel panel, ParserResult parserResult) {
@@ -130,7 +127,7 @@ public void performAction(BasePanel panel, ParserResult parserResult) {
         int row = 1;
         formBuilder.add(new JLabel("<html>" + Localization.lang("This library uses outdated file links.") + "<br><br>"
                 + Localization
-                        .lang("JabRef no longer supports 'ps' or 'pdf' fields.<br>File links are now stored in the 'file' field and files are stored in an external file directory.<br>To make use of this feature, JabRef needs to upgrade file links.<br><br>")
+                .lang("JabRef no longer supports 'ps' or 'pdf' fields.<br>File links are now stored in the 'file' field and files are stored in an external file directory.<br>To make use of this feature, JabRef needs to upgrade file links.<br><br>")
                 + "<p>"
                 + Localization.lang("Do you want JabRef to do the following operations?") + "</html>")).xy(1, row);
 
@@ -191,10 +188,11 @@ private boolean isThereSomethingToBeDone() {
     /**
      * Check the database to find out whether any of a set of fields are used
      * for any of the entries.
+     *
      * @param database The BIB database.
-     * @param fields The set of fields to look for.
+     * @param fields   The set of fields to look for.
      * @return true if at least one of the given fields is set in at least one entry,
-     *  false otherwise.
+     * false otherwise.
      */
     private boolean linksFound(BibDatabase database, String[] fields) {
         for (BibEntry entry : database.getEntries()) {
@@ -209,12 +207,11 @@ private boolean linksFound(BibDatabase database, String[] fields) {
 
     /**
      * This method performs the actual changes.
-     * @param panel
-     * @param pr
+     *
      * @param fileDir The path to the file directory to set, or null if it should not be set.
      */
     private void makeChanges(BasePanel panel, ParserResult pr, boolean upgradePrefs,
-            boolean upgradeDatabase, String fileDir) {
+                             boolean upgradeDatabase, String fileDir) {
 
         if (upgradeDatabase) {
             // Update file links links in the database: