Refactor SaveAction

- Get rid of "doSave()"
- Add notification for non-successful save in case of "SaveAll"
- Remove deprecated "setDatabaseFile" (as we are Java8 with Path)

Co-authored-by: Stefan Kolb <stefan-kolb@web.de>

src/main/java/org/jabref/cli/ArgumentProcessor.java

@@ -431,7 +431,7 @@ private void exportFile(List<ParserResult> loaded, String[] data) {
                 theFile = theFile.getAbsoluteFile();
             }
             BibDatabaseContext databaseContext = pr.getDatabaseContext();
-            databaseContext.setDatabaseFile(theFile);
+            databaseContext.setDatabasePath(theFile.toPath());
             Globals.prefs.fileDirForDatabase = databaseContext
                     .getFileDirectories(Globals.prefs.getFilePreferences());
             System.out.println(Localization.lang("Exporting") + ": " + data[0]);

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -1137,8 +1137,7 @@ private boolean confirmClose(BasePanel panel) {
             // The user wants to save.
             try {
                 SaveDatabaseAction saveAction = new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager);
-                if (saveAction.save()) {
-                    // Saved, now exit.
+                if (saveAction.save(panel.getBibDatabaseContext())) {
                     return true;
                 }
                 // The action was either canceled or unsuccessful.

src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java

@@ -26,7 +26,7 @@ public AutosaveUIManager(BasePanel panel) {
     @Subscribe
     public void listen(@SuppressWarnings("unused") AutosaveEvent event) {
         try {
-            new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(SaveDatabaseAction.SaveDatabaseMode.SILENT);
+            new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(panel.getBibDatabaseContext(), SaveDatabaseAction.SaveDatabaseMode.SILENT);
         } catch (Throwable e) {
             LOGGER.error("Problem occurred while saving.", e);
         }

src/main/java/org/jabref/gui/exporter/SaveAction.java

@@ -37,7 +37,7 @@ public void execute() {
 
         switch (saveMethod) {
             case SAVE:
-                saveDatabaseAction.save();
+                saveDatabaseAction.save(frame.getCurrentBasePanel().getBibDatabaseContext());
                 break;
             case SAVE_AS:
                 saveDatabaseAction.saveAs();

src/main/java/org/jabref/gui/exporter/SaveAllAction.java

@@ -23,13 +23,10 @@ public void execute() {
 
         for (BasePanel panel : frame.getBasePanelList()) {
             SaveDatabaseAction saveDatabaseAction = new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager);
-            if (panel.getBibDatabaseContext().getDatabasePath().isEmpty()) {
-                //It will ask a path before saving.
-                saveDatabaseAction.saveAs();
-            } else {
-                saveDatabaseAction.save();
-                // TODO: can we find out whether the save was actually done or not?
-            }
+            boolean saveResult = saveDatabaseAction.save(panel.getBibDatabaseContext());
+            if (!saveResult) {
+                 dialogService.notify(Localization.lang("Could not save file."));
+             }
         }
 
         dialogService.notify(Localization.lang("Save all finished."));

src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java

@@ -122,9 +122,12 @@ private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset
         }
     }
 
-    private boolean doSave() {
+    public boolean save(Path targetPath, SaveDatabaseMode mode) {
+        if (mode == SaveDatabaseMode.NORMAL) {
+            panel.frame().getDialogService().notify(Localization.lang("Saving library") + "...");
+        }
+
         panel.setSaving(true);
-        Path targetPath = panel.getBibDatabaseContext().getDatabasePath().get();
         try {
             Charset encoding = panel.getBibDatabaseContext()
                                     .getMetaData()
@@ -144,9 +147,8 @@ private boolean doSave() {
                 panel.setNonUndoableChange(false);
                 panel.setBaseChanged(false);
 
-                // Reset title of tab
                 frame.setTabTitle(panel, panel.getTabTitle(),
-                        panel.getBibDatabaseContext().getDatabasePath().get().toAbsolutePath().toString());
+                        targetPath.toAbsolutePath().toString());
                 frame.setWindowTitle();
                 frame.updateAllTabTitles();
             }
@@ -161,32 +163,36 @@ private boolean doSave() {
         }
     }
 
-    public boolean save() {
-        return save(SaveDatabaseMode.NORMAL);
+    public boolean save(BibDatabaseContext bibDatabaseContext) {
+        return save(bibDatabaseContext, SaveDatabaseMode.NORMAL);
     }
 
-    public boolean save(SaveDatabaseMode mode) {
-        if (panel.getBibDatabaseContext().getDatabasePath().isPresent()) {
-            if (mode == SaveDatabaseMode.NORMAL) {
-                panel.frame().getDialogService().notify(Localization.lang("Saving library") + "...");
-            }
-            return doSave();
-        } else {
-            Optional<Path> savePath = getSavePath();
-            if (savePath.isPresent()) {
-                saveAs(savePath.get());
-                return true;
+    public boolean save(BibDatabaseContext bibDatabaseContext, SaveDatabaseMode mode) {
+        Optional<Path> databasePath = bibDatabaseContext.getDatabasePath();
+        if (!databasePath.isPresent()) {
+            Optional<Path> savePath = askForSavePath();
+            if (!savePath.isPresent()) {
+                return false;
             }
+            return saveAs(savePath.get(), mode);
         }
 
-        return false;
+        return save(databasePath.get(), mode);
     }
 
+    /**
+     * Asks the user for the path and saves afterwards
+     */
     public void saveAs() {
-        getSavePath().ifPresent(this::saveAs);
+        askForSavePath().ifPresent(this::saveAs);
     }
 
-    private Optional<Path> getSavePath() {
+    /**
+     * Asks the user for the path to save to. Stores the directory to the preferences, which is used next time when opening the dialog.
+     *
+     * @return the path set by the user
+     */
+    public Optional<Path> askForSavePath() {
         FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
                 .addExtensionFilter(StandardFileType.BIBTEX_DB)
                 .withDefaultExtension(StandardFileType.BIBTEX_DB)
@@ -197,14 +203,22 @@ private Optional<Path> getSavePath() {
         return selectedPath;
     }
 
-    public void saveAs(Path file) {
+    public boolean saveAs(Path file) {
+        return this.saveAs(file, SaveDatabaseMode.NORMAL);
+    }
+
+    /**
+     * @param file the new file name to save the data base to. This is stored in the database context of the panel upon successful save.
+     * @return true on successful save
+     */
+    public boolean saveAs(Path file, SaveDatabaseMode mode) {
         BibDatabaseContext context = panel.getBibDatabaseContext();
 
         // Close AutosaveManager and BackupManager for original library
         Optional<Path> databasePath = context.getDatabasePath();
         if (databasePath.isPresent()) {
             final Path oldFile = databasePath.get();
-            context.setDatabaseFile(oldFile.toFile());
+            context.setDatabasePath(oldFile);
             AutosaveManager.shutdown(context);
             BackupManager.shutdown(context);
         }
@@ -215,22 +229,28 @@ public void saveAs(Path file) {
             new SharedDatabasePreferences(context.getDatabase().generateSharedDatabaseID())
                     .putAllDBMSConnectionProperties(context.getDBMSSynchronizer().getConnectionProperties());
         }
-        context.setDatabaseFile(file);
 
-        // Save
-        save();
+        boolean saveResult = save(file, mode);
 
-        // Reinstall AutosaveManager and BackupManager
-        panel.resetChangeMonitorAndChangePane();
-        if (readyForAutosave(context)) {
-            AutosaveManager autosaver = AutosaveManager.start(context);
-            autosaver.registerListener(new AutosaveUIManager(panel));
-        }
-        if (readyForBackup(context)) {
-            BackupManager.start(context, entryTypesManager, prefs);
+        if (saveResult) {
+            // we managed to successfully save the file
+            // thus, we can store the store the path into the context
+            context.setDatabasePath(file);
+
+            // Reinstall AutosaveManager and BackupManager for the new file name
+            panel.resetChangeMonitorAndChangePane();
+            if (readyForAutosave(context)) {
+                AutosaveManager autosaver = AutosaveManager.start(context);
+                autosaver.registerListener(new AutosaveUIManager(panel));
+            }
+            if (readyForBackup(context)) {
+                BackupManager.start(context, entryTypesManager, prefs);
+            }
+
+            frame.getFileHistory().newFile(file);
         }
 
-        context.getDatabasePath().ifPresent(presentFile -> frame.getFileHistory().newFile(presentFile));
+        return saveResult;
     }
 
     private boolean readyForAutosave(BibDatabaseContext context) {
@@ -246,7 +266,7 @@ private boolean readyForBackup(BibDatabaseContext context) {
     }
 
     public void saveSelectedAsPlain() {
-        getSavePath().ifPresent(path -> {
+        askForSavePath().ifPresent(path -> {
             try {
                 saveDatabase(path, true, prefs.getDefaultEncoding(), SavePreferences.DatabaseSaveType.PLAIN_BIBTEX);
                 frame.getFileHistory().newFile(path);

src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java

@@ -173,7 +173,7 @@ public void openSharedDatabaseFromParserResult(ParserResult parserResult)
         bibDatabaseContext.convertToSharedDatabase(synchronizer);
 
         bibDatabaseContext.getDatabase().setSharedDatabaseID(sharedDatabaseID);
-        bibDatabaseContext.setDatabaseFile(parserResult.getDatabaseContext().getDatabasePath().orElse(null));
+        bibDatabaseContext.setDatabasePath(parserResult.getDatabaseContext().getDatabasePath().orElse(null));
 
         dbmsSynchronizer = bibDatabaseContext.getDBMSSynchronizer();
         dbmsSynchronizer.openSharedDatabase(new DBMSConnection(dbmsConnectionProperties));

src/main/java/org/jabref/model/database/BibDatabaseContext.java

@@ -85,16 +85,7 @@ public Optional<File> getDatabaseFile() {
         return file.map(Path::toFile);
     }
 
-    /**
-     * @param file the database file
-     * @deprecated use {@link #setDatabaseFile(Path)}
-     */
-    @Deprecated
-    public void setDatabaseFile(File file) {
-        this.file = Optional.ofNullable(file).map(File::toPath);
-    }
-
-    public void setDatabaseFile(Path file) {
+    public void setDatabasePath(Path file) {
         this.file = Optional.ofNullable(file);
     }
 

src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java

@@ -34,7 +34,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
@@ -65,7 +65,7 @@ public void setUp() {
     public void saveAsShouldSetWorkingDirectory() {
         when(preferences.get(JabRefPreferences.WORKING_DIRECTORY)).thenReturn(TEST_BIBTEX_LIBRARY_LOCATION);
         when(dialogService.showFileSaveDialog(any(FileDialogConfiguration.class))).thenReturn(Optional.of(file));
-        doNothing().when(saveDatabaseAction).saveAs(any());
+        doReturn(true).when(saveDatabaseAction).saveAs(any());
 
         saveDatabaseAction.saveAs();
 
@@ -76,35 +76,24 @@ public void saveAsShouldSetWorkingDirectory() {
     public void saveAsShouldNotSetWorkingDirectoryIfNotSelected() {
         when(preferences.get(JabRefPreferences.WORKING_DIRECTORY)).thenReturn(TEST_BIBTEX_LIBRARY_LOCATION);
         when(dialogService.showFileSaveDialog(any(FileDialogConfiguration.class))).thenReturn(Optional.empty());
-        doNothing().when(saveDatabaseAction).saveAs(any());
+        doReturn(false).when(saveDatabaseAction).saveAs(any());
 
         saveDatabaseAction.saveAs();
 
         verify(preferences, times(0)).setWorkingDir(file.getParent());
     }
 
-    @Test
-    public void saveAsShouldSetNewDatabasePathIntoContext() {
-        when(dbContext.getDatabasePath()).thenReturn(Optional.empty());
-        when(dbContext.getLocation()).thenReturn(DatabaseLocation.LOCAL);
-        when(preferences.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE)).thenReturn(false);
-
-        saveDatabaseAction.saveAs(file);
-
-        verify(dbContext, times(1)).setDatabaseFile(file);
-    }
-
     @Test
     public void saveShouldShowSaveAsIfDatabaseNotSelected() {
         when(dbContext.getDatabasePath()).thenReturn(Optional.empty());
         when(dbContext.getLocation()).thenReturn(DatabaseLocation.LOCAL);
         when(preferences.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE)).thenReturn(false);
         when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(file));
-        doNothing().when(saveDatabaseAction).saveAs(file);
+        doReturn(true).when(saveDatabaseAction).saveAs(any(), any());
 
-        saveDatabaseAction.save();
+        saveDatabaseAction.save(dbContext);
 
-        verify(saveDatabaseAction, times(1)).saveAs(file);
+        verify(saveDatabaseAction, times(1)).saveAs(file, SaveDatabaseAction.SaveDatabaseMode.NORMAL);
     }
 
     private SaveDatabaseAction createSaveDatabaseActionForBibDatabase(BibDatabase database) throws IOException {
@@ -151,7 +140,7 @@ public void saveKeepsChangedFlag() throws Exception {
         BibDatabase database = new BibDatabase(List.of(firstEntry, secondEntry));
 
         saveDatabaseAction = createSaveDatabaseActionForBibDatabase(database);
-        saveDatabaseAction.save();
+        saveDatabaseAction.save(dbContext);
 
         assertEquals(database
                         .getEntries().stream()
@@ -162,9 +151,7 @@ public void saveKeepsChangedFlag() throws Exception {
     @Test
     public void saveShouldNotSaveDatabaseIfPathNotSet() {
         when(dbContext.getDatabasePath()).thenReturn(Optional.empty());
-
-        boolean result = saveDatabaseAction.save();
-
+        boolean result = saveDatabaseAction.save(dbContext);
         assertFalse(result);
     }
 }

src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java

@@ -60,7 +60,7 @@ void setUp(@TempDir Path bibFolder) throws IOException {
         metaData.setDefaultFileDirectory(pdfFolder.getAbsolutePath());
         BibDatabaseContext context = new BibDatabaseContext(new BibDatabase(), metaData);
         Files.createFile(bibFolder.resolve("test.bib"));
-        context.setDatabaseFile(bibFolder.resolve("test.bib").toFile());
+        context.setDatabasePath(bibFolder.resolve("test.bib"));
 
         FilePreferences fileDirPrefs = mock(FilePreferences.class, Answers.RETURNS_SMART_NULLS);
         //Biblocation as Primary overwrites all other dirs

src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java

@@ -49,7 +49,7 @@ void setUp(@TempDir Path bibFolder) throws IOException {
         metaData.setDefaultFileDirectory(defaultFileFolder.toAbsolutePath().toString());
         BibDatabaseContext databaseContext = new BibDatabaseContext(new BibDatabase(), metaData);
         Files.createFile(bibFolder.resolve("test.bib"));
-        databaseContext.setDatabaseFile(bibFolder.resolve("test.bib"));
+        databaseContext.setDatabasePath(bibFolder.resolve("test.bib"));
 
         entry = new BibEntry();
         entry.setCiteKey("Toot");

src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java

@@ -35,7 +35,7 @@ void setUp(@TempDir Path testFolder) {
         Path path = testFolder.resolve("test.bib");
         MetaData metaData = new MetaData();
         BibDatabaseContext context = new BibDatabaseContext(new BibDatabase(), metaData);
-        context.setDatabaseFile(path);
+        context.setDatabasePath(path);
 
         entry = new BibEntry();
         entry.setCiteKey("Toot");

src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java

@@ -232,7 +232,7 @@ void fileCheckFindsFilesRelativeToBibFile(@TempDir Path testFolder) throws IOExc
         Files.createFile(pdfFile);
 
         BibDatabaseContext databaseContext = createContext(StandardField.FILE, ":file.pdf:PDF");
-        databaseContext.setDatabaseFile(bibFile);
+        databaseContext.setDatabasePath(bibFile);
 
         assertCorrect(databaseContext);
     }

src/test/java/org/jabref/model/database/BibDatabaseContextTest.java

@@ -34,7 +34,7 @@ public void setUp() {
     @Test
     public void getFileDirectoriesWithEmptyDbParent() {
         BibDatabaseContext dbContext = new BibDatabaseContext();
-        dbContext.setDatabaseFile(Paths.get("biblio.bib").toFile());
+        dbContext.setDatabasePath(Paths.get("biblio.bib"));
         List<String> fileDirectories = dbContext.getFileDirectories(StandardField.FILE, fileDirPrefs);
         assertEquals(Collections.singletonList(currentWorkingDir.toString()),
                 fileDirectories);
@@ -45,7 +45,7 @@ public void getFileDirectoriesWithRelativeDbParent() {
         Path file = Paths.get("relative/subdir").resolve("biblio.bib");
 
         BibDatabaseContext dbContext = new BibDatabaseContext();
-        dbContext.setDatabaseFile(file.toFile());
+        dbContext.setDatabasePath(file);
         List<String> fileDirectories = dbContext.getFileDirectories(StandardField.FILE, fileDirPrefs);
         assertEquals(Collections.singletonList(currentWorkingDir.resolve(file.getParent()).toString()),
                 fileDirectories);
@@ -56,7 +56,7 @@ public void getFileDirectoriesWithRelativeDottedDbParent() {
         Path file = Paths.get("./relative/subdir").resolve("biblio.bib");
 
         BibDatabaseContext dbContext = new BibDatabaseContext();
-        dbContext.setDatabaseFile(file.toFile());
+        dbContext.setDatabasePath(file);
         List<String> fileDirectories = dbContext.getFileDirectories(StandardField.FILE, fileDirPrefs);
         assertEquals(Collections.singletonList(currentWorkingDir.resolve(file.getParent()).toString()),
                 fileDirectories);
@@ -67,7 +67,7 @@ public void getFileDirectoriesWithAbsoluteDbParent() {
         Path file = Paths.get("/absolute/subdir").resolve("biblio.bib");
 
         BibDatabaseContext dbContext = new BibDatabaseContext();
-        dbContext.setDatabaseFile(file.toFile());
+        dbContext.setDatabasePath(file);
         List<String> fileDirectories = dbContext.getFileDirectories(StandardField.FILE, fileDirPrefs);
         assertEquals(Collections.singletonList(currentWorkingDir.resolve(file.getParent()).toString()),
                 fileDirectories);