From b3643969d8d8ed408e3df44102f8ce2c906ce2aa Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 14 Mar 2020 01:15:05 +0100 Subject: [PATCH 1/3] 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 --- .../org/jabref/cli/ArgumentProcessor.java | 2 +- src/main/java/org/jabref/gui/JabRefFrame.java | 3 +- .../jabref/gui/dialogs/AutosaveUIManager.java | 2 +- .../org/jabref/gui/exporter/SaveAction.java | 2 +- .../jabref/gui/exporter/SaveAllAction.java | 11 +-- .../gui/exporter/SaveDatabaseAction.java | 90 +++++++++++-------- .../gui/shared/SharedDatabaseUIManager.java | 2 +- .../model/database/BibDatabaseContext.java | 11 +-- .../gui/exporter/SaveDatabaseActionTest.java | 29 ++---- .../logic/cleanup/CleanupWorkerTest.java | 2 +- .../logic/cleanup/MoveFilesCleanupTest.java | 2 +- .../logic/cleanup/RenamePdfCleanupTest.java | 2 +- .../logic/integrity/IntegrityCheckTest.java | 2 +- .../database/BibDatabaseContextTest.java | 8 +- 14 files changed, 81 insertions(+), 87 deletions(-) diff --git a/src/main/java/org/jabref/cli/ArgumentProcessor.java b/src/main/java/org/jabref/cli/ArgumentProcessor.java index c9d01d6784d..16f0ede5aea 100644 --- a/src/main/java/org/jabref/cli/ArgumentProcessor.java +++ b/src/main/java/org/jabref/cli/ArgumentProcessor.java @@ -431,7 +431,7 @@ private void exportFile(List 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]); diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 355fcd9d066..8f65312114e 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/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. diff --git a/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java b/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java index c9268c40dc1..612a4d6092a 100644 --- a/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java +++ b/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); } diff --git a/src/main/java/org/jabref/gui/exporter/SaveAction.java b/src/main/java/org/jabref/gui/exporter/SaveAction.java index 4684cc17954..acbb8a2e602 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveAction.java +++ b/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(); diff --git a/src/main/java/org/jabref/gui/exporter/SaveAllAction.java b/src/main/java/org/jabref/gui/exporter/SaveAllAction.java index c1d74faf00e..0b72650d806 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveAllAction.java +++ b/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.")); diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index 0a8f5b3ce28..b2ea69c642c 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/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 savePath = getSavePath(); - if (savePath.isPresent()) { - saveAs(savePath.get()); - return true; + public boolean save(BibDatabaseContext bibDatabaseContext, SaveDatabaseMode mode) { + Optional databasePath = bibDatabaseContext.getDatabasePath(); + if (!databasePath.isPresent()) { + Optional 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 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 askForSavePath() { FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder() .addExtensionFilter(StandardFileType.BIBTEX_DB) .withDefaultExtension(StandardFileType.BIBTEX_DB) @@ -197,14 +203,22 @@ private Optional 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 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); diff --git a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java index 9931096eb66..56fd9dd74ba 100644 --- a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java +++ b/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)); diff --git a/src/main/java/org/jabref/model/database/BibDatabaseContext.java b/src/main/java/org/jabref/model/database/BibDatabaseContext.java index 9cff25928fc..d2e10dec3c2 100644 --- a/src/main/java/org/jabref/model/database/BibDatabaseContext.java +++ b/src/main/java/org/jabref/model/database/BibDatabaseContext.java @@ -85,16 +85,7 @@ public Optional 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); } diff --git a/src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java b/src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java index 6afed9d0c01..534823b4531 100644 --- a/src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java +++ b/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); } } diff --git a/src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java b/src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java index 0e719bb2b3e..7cdf6d2dc56 100644 --- a/src/test/java/org/jabref/logic/cleanup/CleanupWorkerTest.java +++ b/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 diff --git a/src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java b/src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java index f17c694528a..523dfb5a049 100644 --- a/src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java +++ b/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"); diff --git a/src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java b/src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java index ad01d555be2..d60b82a72c3 100644 --- a/src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java +++ b/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"); diff --git a/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java b/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java index e166869806d..2e096753fed 100644 --- a/src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java +++ b/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); } diff --git a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java b/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java index 6b0a6eae40b..0c60cf2bbc1 100644 --- a/src/test/java/org/jabref/model/database/BibDatabaseContextTest.java +++ b/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 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 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 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 fileDirectories = dbContext.getFileDirectories(StandardField.FILE, fileDirPrefs); assertEquals(Collections.singletonList(currentWorkingDir.resolve(file.getParent()).toString()), fileDirectories); From 3f9922e62e3a294bd19928227776f327b66158a3 Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Sun, 15 Mar 2020 18:56:50 +0100 Subject: [PATCH 2/3] More refactorings --- src/main/java/org/jabref/gui/JabRefFrame.java | 6 +- ...eUIManager.java => AutosaveUiManager.java} | 13 +- .../org/jabref/gui/exporter/SaveAction.java | 5 +- .../jabref/gui/exporter/SaveAllAction.java | 6 +- .../gui/exporter/SaveDatabaseAction.java | 302 +++++++++--------- 5 files changed, 166 insertions(+), 166 deletions(-) rename src/main/java/org/jabref/gui/dialogs/{AutosaveUIManager.java => AutosaveUiManager.java} (72%) diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java index 8f65312114e..f904f291fb5 100644 --- a/src/main/java/org/jabref/gui/JabRefFrame.java +++ b/src/main/java/org/jabref/gui/JabRefFrame.java @@ -57,7 +57,7 @@ import org.jabref.gui.copyfiles.CopyFilesAction; import org.jabref.gui.customentrytypes.CustomizeEntryAction; import org.jabref.gui.customizefields.SetupGeneralFieldsAction; -import org.jabref.gui.dialogs.AutosaveUIManager; +import org.jabref.gui.dialogs.AutosaveUiManager; import org.jabref.gui.documentviewer.ShowDocumentViewerAction; import org.jabref.gui.duplicationFinder.DuplicateSearch; import org.jabref.gui.edit.CopyMoreAction; @@ -1018,7 +1018,7 @@ public void addTab(BasePanel basePanel, boolean raisePanel) { if (readyForAutosave(context)) { AutosaveManager autosaver = AutosaveManager.start(context); - autosaver.registerListener(new AutosaveUIManager(basePanel)); + autosaver.registerListener(new AutosaveUiManager(basePanel)); } BackupManager.start(context, Globals.entryTypesManager, prefs); @@ -1137,7 +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(panel.getBibDatabaseContext())) { + if (saveAction.save()) { return true; } // The action was either canceled or unsuccessful. diff --git a/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java b/src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java similarity index 72% rename from src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java rename to src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java index 612a4d6092a..a1d6da59753 100644 --- a/src/main/java/org/jabref/gui/dialogs/AutosaveUIManager.java +++ b/src/main/java/org/jabref/gui/dialogs/AutosaveUiManager.java @@ -10,23 +10,22 @@ import org.slf4j.LoggerFactory; /** - * This class has an abstract UI role as it listens for an {@link AutosaveEvent} - * and saves the bib file associated with the given {@link BasePanel}. + * This class has an abstract UI role as it listens for an {@link AutosaveEvent} and saves the bib file associated with + * the given {@link BasePanel}. */ -public class AutosaveUIManager { +public class AutosaveUiManager { + private static final Logger LOGGER = LoggerFactory.getLogger(AutosaveUiManager.class); - private static final Logger LOGGER = LoggerFactory.getLogger(AutosaveUIManager.class); private final BasePanel panel; - - public AutosaveUIManager(BasePanel panel) { + public AutosaveUiManager(BasePanel panel) { this.panel = panel; } @Subscribe public void listen(@SuppressWarnings("unused") AutosaveEvent event) { try { - new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(panel.getBibDatabaseContext(), SaveDatabaseAction.SaveDatabaseMode.SILENT); + new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(SaveDatabaseAction.SaveDatabaseMode.SILENT); } catch (Throwable e) { LOGGER.error("Problem occurred while saving.", e); } diff --git a/src/main/java/org/jabref/gui/exporter/SaveAction.java b/src/main/java/org/jabref/gui/exporter/SaveAction.java index acbb8a2e602..3c2a4f4b824 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveAction.java @@ -11,7 +11,7 @@ */ public class SaveAction extends SimpleCommand { - public enum SaveMethod { SAVE, SAVE_AS, SAVE_SELECTED } + public enum SaveMethod {SAVE, SAVE_AS, SAVE_SELECTED} private final SaveMethod saveMethod; private final JabRefFrame frame; @@ -20,7 +20,6 @@ public SaveAction(SaveMethod saveMethod, JabRefFrame frame, StateManager stateMa this.saveMethod = saveMethod; this.frame = frame; - if (saveMethod == SaveMethod.SAVE_SELECTED) { this.executable.bind(ActionHelper.needsEntriesSelected(stateManager)); } else { @@ -37,7 +36,7 @@ public void execute() { switch (saveMethod) { case SAVE: - saveDatabaseAction.save(frame.getCurrentBasePanel().getBibDatabaseContext()); + saveDatabaseAction.save(); break; case SAVE_AS: saveDatabaseAction.saveAs(); diff --git a/src/main/java/org/jabref/gui/exporter/SaveAllAction.java b/src/main/java/org/jabref/gui/exporter/SaveAllAction.java index 0b72650d806..0e9770c034f 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveAllAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveAllAction.java @@ -23,10 +23,10 @@ public void execute() { for (BasePanel panel : frame.getBasePanelList()) { SaveDatabaseAction saveDatabaseAction = new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager); - boolean saveResult = saveDatabaseAction.save(panel.getBibDatabaseContext()); + boolean saveResult = saveDatabaseAction.save(); if (!saveResult) { - dialogService.notify(Localization.lang("Could not save file.")); - } + dialogService.notify(Localization.lang("Could not save file.")); + } } dialogService.notify(Localization.lang("Save all finished.")); diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index b2ea69c642c..41d1588eee9 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -17,7 +17,7 @@ import org.jabref.gui.BasePanel; import org.jabref.gui.DialogService; import org.jabref.gui.JabRefFrame; -import org.jabref.gui.dialogs.AutosaveUIManager; +import org.jabref.gui.dialogs.AutosaveUiManager; import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.FileDialogConfiguration; import org.jabref.logic.autosaveandbackup.AutosaveManager; @@ -47,84 +47,138 @@ * operation was canceled, or whether it was successful. */ public class SaveDatabaseAction { - - public enum SaveDatabaseMode { - SILENT, NORMAL - } - private static final Logger LOGGER = LoggerFactory.getLogger(SaveDatabaseAction.class); private final BasePanel panel; private final JabRefFrame frame; private final DialogService dialogService; - private final JabRefPreferences prefs; + private final JabRefPreferences preferences; private final BibEntryTypesManager entryTypesManager; - public SaveDatabaseAction(BasePanel panel, JabRefPreferences prefs, BibEntryTypesManager entryTypesManager) { + public enum SaveDatabaseMode { + SILENT, NORMAL + } + + public SaveDatabaseAction(BasePanel panel, JabRefPreferences preferences, BibEntryTypesManager entryTypesManager) { this.panel = panel; this.frame = panel.frame(); this.dialogService = frame.getDialogService(); - this.prefs = prefs; + this.preferences = preferences; this.entryTypesManager = entryTypesManager; } - private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, SavePreferences.DatabaseSaveType saveType) throws SaveException { - SavePreferences preferences = prefs.loadForSaveFromPreferences() - .withEncoding(encoding) - .withSaveType(saveType); - try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding(), preferences.makeBackup())) { - BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter, preferences, entryTypesManager); + public boolean save() { + return save(panel.getBibDatabaseContext(), SaveDatabaseMode.NORMAL); + } - if (selectedOnly) { - databaseWriter.savePartOfDatabase(panel.getBibDatabaseContext(), panel.getSelectedEntries()); - } else { - databaseWriter.saveDatabase(panel.getBibDatabaseContext()); + public boolean save(SaveDatabaseMode mode) { + return save(panel.getBibDatabaseContext(), mode); + } + + /** + * Asks the user for the path and saves afterwards + */ + public void saveAs() { + askForSavePath().ifPresent(this::saveAs); + } + + public boolean saveAs(Path file) { + return this.saveAs(file, SaveDatabaseMode.NORMAL); + } + + public void saveSelectedAsPlain() { + askForSavePath().ifPresent(path -> { + try { + saveDatabase(path, true, preferences.getDefaultEncoding(), SavePreferences.DatabaseSaveType.PLAIN_BIBTEX); + frame.getFileHistory().newFile(path); + dialogService.notify(Localization.lang("Saved selected to '%0'.", path.toString())); + } catch (SaveException ex) { + LOGGER.error("A problem occurred when trying to save the file", ex); + dialogService.showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); } + }); + } - panel.registerUndoableChanges(databaseWriter.getSaveActionsFieldChanges()); + /** + * @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 + */ + private boolean saveAs(Path file, SaveDatabaseMode mode) { + BibDatabaseContext context = panel.getBibDatabaseContext(); - if (fileWriter.hasEncodingProblems()) { - saveWithDifferentEncoding(file, selectedOnly, preferences.getEncoding(), fileWriter.getEncodingProblems(), saveType); + // Close AutosaveManager and BackupManager for original library + Optional databasePath = context.getDatabasePath(); + if (databasePath.isPresent()) { + final Path oldFile = databasePath.get(); + context.setDatabasePath(oldFile); + AutosaveManager.shutdown(context); + BackupManager.shutdown(context); + } + + // Set new location + if (context.getLocation() == DatabaseLocation.SHARED) { + // Save all properties dependent on the ID. This makes it possible to restore them. + new SharedDatabasePreferences(context.getDatabase().generateSharedDatabaseID()) + .putAllDBMSConnectionProperties(context.getDBMSSynchronizer().getConnectionProperties()); + } + + boolean saveResult = save(file, mode); + + 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)); } - } catch (UnsupportedCharsetException ex) { - throw new SaveException(Localization.lang("Character encoding '%0' is not supported.", encoding.displayName()), ex); - } catch (IOException ex) { - throw new SaveException("Problems saving:", ex); + if (readyForBackup(context)) { + BackupManager.start(context, entryTypesManager, preferences); + } + + frame.getFileHistory().newFile(file); } - return true; + return saveResult; } - private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset encoding, Set encodingProblems, SavePreferences.DatabaseSaveType saveType) throws SaveException { - DialogPane pane = new DialogPane(); - VBox vbox = new VBox(); - vbox.getChildren().addAll( - new Text(Localization.lang("The chosen encoding '%0' could not encode the following characters:", encoding.displayName())), - new Text(encodingProblems.stream().map(Object::toString).collect(Collectors.joining("."))), - new Text(Localization.lang("What do you want to do?")) - ); - pane.setContent(vbox); - - ButtonType tryDifferentEncoding = new ButtonType(Localization.lang("Try different encoding"), ButtonBar.ButtonData.OTHER); - ButtonType ignore = new ButtonType(Localization.lang("Ignore"), ButtonBar.ButtonData.APPLY); - boolean saveWithDifferentEncoding = frame.getDialogService() - .showCustomDialogAndWait(Localization.lang("Save library"), pane, ignore, tryDifferentEncoding) - .filter(buttonType -> buttonType.equals(tryDifferentEncoding)) - .isPresent(); - if (saveWithDifferentEncoding) { - Optional newEncoding = frame.getDialogService().showChoiceDialogAndWait(Localization.lang("Save library"), Localization.lang("Select new encoding"), Localization.lang("Save library"), encoding, Encodings.getCharsets()); - if (newEncoding.isPresent()) { - // Make sure to remember which encoding we used. - panel.getBibDatabaseContext().getMetaData().setEncoding(newEncoding.get(), ChangePropagation.DO_NOT_POST_EVENT); + /** + * 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 + */ + private Optional askForSavePath() { + FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder() + .addExtensionFilter(StandardFileType.BIBTEX_DB) + .withDefaultExtension(StandardFileType.BIBTEX_DB) + .withInitialDirectory(preferences.get(JabRefPreferences.WORKING_DIRECTORY)) + .build(); + Optional selectedPath = dialogService.showFileSaveDialog(fileDialogConfiguration); + selectedPath.ifPresent(path -> preferences.setWorkingDir(path.getParent())); + return selectedPath; + } - saveDatabase(file, selectedOnly, newEncoding.get(), saveType); + private boolean save(BibDatabaseContext bibDatabaseContext, SaveDatabaseMode mode) { + Optional databasePath = bibDatabaseContext.getDatabasePath(); + if (!databasePath.isPresent()) { + Optional savePath = askForSavePath(); + if (!savePath.isPresent()) { + return false; } + return saveAs(savePath.get(), mode); } + + return save(databasePath.get(), mode); } - public boolean save(Path targetPath, SaveDatabaseMode mode) { + private boolean save(Path targetPath, SaveDatabaseMode mode) { if (mode == SaveDatabaseMode.NORMAL) { - panel.frame().getDialogService().notify(Localization.lang("Saving library") + "..."); + dialogService.notify(String.format("%s...", Localization.lang("Saving library"))); } panel.setSaving(true); @@ -132,7 +186,7 @@ public boolean save(Path targetPath, SaveDatabaseMode mode) { Charset encoding = panel.getBibDatabaseContext() .getMetaData() .getEncoding() - .orElse(prefs.getDefaultEncoding()); + .orElse(preferences.getDefaultEncoding()); // Make sure to remember which encoding we used. panel.getBibDatabaseContext().getMetaData().setEncoding(encoding, ChangePropagation.DO_NOT_POST_EVENT); @@ -141,21 +195,18 @@ public boolean save(Path targetPath, SaveDatabaseMode mode) { if (success) { panel.getUndoManager().markUnchanged(); - // (Only) after a successful save the following - // statement marks that the base is unchanged - // since last save: + // After a successful save the following statement marks that the base is unchanged since last save panel.setNonUndoableChange(false); panel.setBaseChanged(false); - frame.setTabTitle(panel, panel.getTabTitle(), - targetPath.toAbsolutePath().toString()); + frame.setTabTitle(panel, panel.getTabTitle(), targetPath.toAbsolutePath().toString()); frame.setWindowTitle(); frame.updateAllTabTitles(); } return success; } catch (SaveException ex) { - LOGGER.error("A problem occurred when trying to save the file " + targetPath, ex); - frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); + LOGGER.error(String.format("A problem occurred when trying to save the file %s", targetPath), ex); + dialogService.showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); return false; } finally { // release panel from save status @@ -163,100 +214,64 @@ public boolean save(Path targetPath, SaveDatabaseMode mode) { } } - public boolean save(BibDatabaseContext bibDatabaseContext) { - return save(bibDatabaseContext, SaveDatabaseMode.NORMAL); - } + private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, SavePreferences.DatabaseSaveType saveType) throws SaveException { + SavePreferences preferences = this.preferences.loadForSaveFromPreferences() + .withEncoding(encoding) + .withSaveType(saveType); + try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding(), preferences.makeBackup())) { + BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(fileWriter, preferences, entryTypesManager); - public boolean save(BibDatabaseContext bibDatabaseContext, SaveDatabaseMode mode) { - Optional databasePath = bibDatabaseContext.getDatabasePath(); - if (!databasePath.isPresent()) { - Optional savePath = askForSavePath(); - if (!savePath.isPresent()) { - return false; + if (selectedOnly) { + databaseWriter.savePartOfDatabase(panel.getBibDatabaseContext(), panel.getSelectedEntries()); + } else { + databaseWriter.saveDatabase(panel.getBibDatabaseContext()); } - return saveAs(savePath.get(), mode); - } - - return save(databasePath.get(), mode); - } - - /** - * Asks the user for the path and saves afterwards - */ - public void saveAs() { - askForSavePath().ifPresent(this::saveAs); - } - /** - * 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 askForSavePath() { - FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder() - .addExtensionFilter(StandardFileType.BIBTEX_DB) - .withDefaultExtension(StandardFileType.BIBTEX_DB) - .withInitialDirectory(prefs.get(JabRefPreferences.WORKING_DIRECTORY)) - .build(); - Optional selectedPath = dialogService.showFileSaveDialog(fileDialogConfiguration); - selectedPath.ifPresent(path -> prefs.setWorkingDir(path.getParent())); - return selectedPath; - } - - 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(); + panel.registerUndoableChanges(databaseWriter.getSaveActionsFieldChanges()); - // Close AutosaveManager and BackupManager for original library - Optional databasePath = context.getDatabasePath(); - if (databasePath.isPresent()) { - final Path oldFile = databasePath.get(); - context.setDatabasePath(oldFile); - AutosaveManager.shutdown(context); - BackupManager.shutdown(context); + if (fileWriter.hasEncodingProblems()) { + saveWithDifferentEncoding(file, selectedOnly, preferences.getEncoding(), fileWriter.getEncodingProblems(), saveType); + } + } catch (UnsupportedCharsetException ex) { + throw new SaveException(Localization.lang("Character encoding '%0' is not supported.", encoding.displayName()), ex); + } catch (IOException ex) { + throw new SaveException("Problems saving:", ex); } - // Set new location - if (context.getLocation() == DatabaseLocation.SHARED) { - // Save all properties dependent on the ID. This makes it possible to restore them. - new SharedDatabasePreferences(context.getDatabase().generateSharedDatabaseID()) - .putAllDBMSConnectionProperties(context.getDBMSSynchronizer().getConnectionProperties()); - } + return true; + } - boolean saveResult = save(file, mode); + private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset encoding, Set encodingProblems, SavePreferences.DatabaseSaveType saveType) throws SaveException { + DialogPane pane = new DialogPane(); + VBox vbox = new VBox(); + vbox.getChildren().addAll( + new Text(Localization.lang("The chosen encoding '%0' could not encode the following characters:", encoding.displayName())), + new Text(encodingProblems.stream().map(Object::toString).collect(Collectors.joining("."))), + new Text(Localization.lang("What do you want to do?")) + ); + pane.setContent(vbox); - if (saveResult) { - // we managed to successfully save the file - // thus, we can store the store the path into the context - context.setDatabasePath(file); + ButtonType tryDifferentEncoding = new ButtonType(Localization.lang("Try different encoding"), ButtonBar.ButtonData.OTHER); + ButtonType ignore = new ButtonType(Localization.lang("Ignore"), ButtonBar.ButtonData.APPLY); + boolean saveWithDifferentEncoding = dialogService + .showCustomDialogAndWait(Localization.lang("Save library"), pane, ignore, tryDifferentEncoding) + .filter(buttonType -> buttonType.equals(tryDifferentEncoding)) + .isPresent(); + if (saveWithDifferentEncoding) { + Optional newEncoding = dialogService.showChoiceDialogAndWait(Localization.lang("Save library"), Localization.lang("Select new encoding"), Localization.lang("Save library"), encoding, Encodings.getCharsets()); + if (newEncoding.isPresent()) { + // Make sure to remember which encoding we used. + panel.getBibDatabaseContext().getMetaData().setEncoding(newEncoding.get(), ChangePropagation.DO_NOT_POST_EVENT); - // 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); + saveDatabase(file, selectedOnly, newEncoding.get(), saveType); } - - frame.getFileHistory().newFile(file); } - - return saveResult; } private boolean readyForAutosave(BibDatabaseContext context) { return ((context.getLocation() == DatabaseLocation.SHARED) || ((context.getLocation() == DatabaseLocation.LOCAL) - && prefs.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE))) + && preferences.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE))) && context.getDatabasePath().isPresent(); } @@ -264,17 +279,4 @@ private boolean readyForAutosave(BibDatabaseContext context) { private boolean readyForBackup(BibDatabaseContext context) { return (context.getLocation() == DatabaseLocation.LOCAL) && context.getDatabasePath().isPresent(); } - - public void saveSelectedAsPlain() { - askForSavePath().ifPresent(path -> { - try { - saveDatabase(path, true, prefs.getDefaultEncoding(), SavePreferences.DatabaseSaveType.PLAIN_BIBTEX); - frame.getFileHistory().newFile(path); - frame.getDialogService().notify(Localization.lang("Saved selected to '%0'.", path.toString())); - } catch (SaveException ex) { - LOGGER.error("A problem occurred when trying to save the file", ex); - frame.getDialogService().showErrorDialogAndWait(Localization.lang("Save library"), Localization.lang("Could not save file."), ex); - } - }); - } } From f52bd03a5e72ae7284767ee27f3174ba84ceaec6 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 16 Mar 2020 07:45:25 +0100 Subject: [PATCH 3/3] Fix title refresh --- src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java index 41d1588eee9..4cdf82354fb 100644 --- a/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java +++ b/src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java @@ -129,6 +129,7 @@ private boolean saveAs(Path file, SaveDatabaseMode mode) { // we managed to successfully save the file // thus, we can store the store the path into the context context.setDatabasePath(file); + frame.refreshTitleAndTabs(); // Reinstall AutosaveManager and BackupManager for the new file name panel.resetChangeMonitorAndChangePane();