Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor SaveAction #6117

Merged
merged 3 commits into from
Mar 16, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/cli/ArgumentProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/exporter/SaveAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void execute() {

switch (saveMethod) {
case SAVE:
saveDatabaseAction.save();
saveDatabaseAction.save(frame.getCurrentBasePanel().getBibDatabaseContext());
break;
case SAVE_AS:
saveDatabaseAction.saveAs();
Expand Down
11 changes: 4 additions & 7 deletions src/main/java/org/jabref/gui/exporter/SaveAllAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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."));
Expand Down
90 changes: 55 additions & 35 deletions src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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();
}
Expand All @@ -161,32 +163,36 @@ private boolean doSave() {
}
}

public boolean save() {
return save(SaveDatabaseMode.NORMAL);
public boolean save(BibDatabaseContext bibDatabaseContext) {
stefan-kolb marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
11 changes: 1 addition & 10 deletions src/main/java/org/jabref/model/database/BibDatabaseContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
29 changes: 8 additions & 21 deletions src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down