Skip to content

Commit b364396

Browse files
kopporstefan-kolb
andcommitted
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 <[email protected]>
1 parent 209d336 commit b364396

File tree

14 files changed

+81
-87
lines changed

14 files changed

+81
-87
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ private void exportFile(List<ParserResult> loaded, String[] data) {
431431
theFile = theFile.getAbsoluteFile();
432432
}
433433
BibDatabaseContext databaseContext = pr.getDatabaseContext();
434-
databaseContext.setDatabaseFile(theFile);
434+
databaseContext.setDatabasePath(theFile.toPath());
435435
Globals.prefs.fileDirForDatabase = databaseContext
436436
.getFileDirectories(Globals.prefs.getFilePreferences());
437437
System.out.println(Localization.lang("Exporting") + ": " + data[0]);

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,8 +1137,7 @@ private boolean confirmClose(BasePanel panel) {
11371137
// The user wants to save.
11381138
try {
11391139
SaveDatabaseAction saveAction = new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager);
1140-
if (saveAction.save()) {
1141-
// Saved, now exit.
1140+
if (saveAction.save(panel.getBibDatabaseContext())) {
11421141
return true;
11431142
}
11441143
// The action was either canceled or unsuccessful.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public AutosaveUIManager(BasePanel panel) {
2626
@Subscribe
2727
public void listen(@SuppressWarnings("unused") AutosaveEvent event) {
2828
try {
29-
new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(SaveDatabaseAction.SaveDatabaseMode.SILENT);
29+
new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(panel.getBibDatabaseContext(), SaveDatabaseAction.SaveDatabaseMode.SILENT);
3030
} catch (Throwable e) {
3131
LOGGER.error("Problem occurred while saving.", e);
3232
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public void execute() {
3737

3838
switch (saveMethod) {
3939
case SAVE:
40-
saveDatabaseAction.save();
40+
saveDatabaseAction.save(frame.getCurrentBasePanel().getBibDatabaseContext());
4141
break;
4242
case SAVE_AS:
4343
saveDatabaseAction.saveAs();

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,10 @@ public void execute() {
2323

2424
for (BasePanel panel : frame.getBasePanelList()) {
2525
SaveDatabaseAction saveDatabaseAction = new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager);
26-
if (panel.getBibDatabaseContext().getDatabasePath().isEmpty()) {
27-
//It will ask a path before saving.
28-
saveDatabaseAction.saveAs();
29-
} else {
30-
saveDatabaseAction.save();
31-
// TODO: can we find out whether the save was actually done or not?
32-
}
26+
boolean saveResult = saveDatabaseAction.save(panel.getBibDatabaseContext());
27+
if (!saveResult) {
28+
dialogService.notify(Localization.lang("Could not save file."));
29+
}
3330
}
3431

3532
dialogService.notify(Localization.lang("Save all finished."));

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

Lines changed: 55 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,12 @@ private void saveWithDifferentEncoding(Path file, boolean selectedOnly, Charset
122122
}
123123
}
124124

125-
private boolean doSave() {
125+
public boolean save(Path targetPath, SaveDatabaseMode mode) {
126+
if (mode == SaveDatabaseMode.NORMAL) {
127+
panel.frame().getDialogService().notify(Localization.lang("Saving library") + "...");
128+
}
129+
126130
panel.setSaving(true);
127-
Path targetPath = panel.getBibDatabaseContext().getDatabasePath().get();
128131
try {
129132
Charset encoding = panel.getBibDatabaseContext()
130133
.getMetaData()
@@ -144,9 +147,8 @@ private boolean doSave() {
144147
panel.setNonUndoableChange(false);
145148
panel.setBaseChanged(false);
146149

147-
// Reset title of tab
148150
frame.setTabTitle(panel, panel.getTabTitle(),
149-
panel.getBibDatabaseContext().getDatabasePath().get().toAbsolutePath().toString());
151+
targetPath.toAbsolutePath().toString());
150152
frame.setWindowTitle();
151153
frame.updateAllTabTitles();
152154
}
@@ -161,32 +163,36 @@ private boolean doSave() {
161163
}
162164
}
163165

164-
public boolean save() {
165-
return save(SaveDatabaseMode.NORMAL);
166+
public boolean save(BibDatabaseContext bibDatabaseContext) {
167+
return save(bibDatabaseContext, SaveDatabaseMode.NORMAL);
166168
}
167169

168-
public boolean save(SaveDatabaseMode mode) {
169-
if (panel.getBibDatabaseContext().getDatabasePath().isPresent()) {
170-
if (mode == SaveDatabaseMode.NORMAL) {
171-
panel.frame().getDialogService().notify(Localization.lang("Saving library") + "...");
172-
}
173-
return doSave();
174-
} else {
175-
Optional<Path> savePath = getSavePath();
176-
if (savePath.isPresent()) {
177-
saveAs(savePath.get());
178-
return true;
170+
public boolean save(BibDatabaseContext bibDatabaseContext, SaveDatabaseMode mode) {
171+
Optional<Path> databasePath = bibDatabaseContext.getDatabasePath();
172+
if (!databasePath.isPresent()) {
173+
Optional<Path> savePath = askForSavePath();
174+
if (!savePath.isPresent()) {
175+
return false;
179176
}
177+
return saveAs(savePath.get(), mode);
180178
}
181179

182-
return false;
180+
return save(databasePath.get(), mode);
183181
}
184182

183+
/**
184+
* Asks the user for the path and saves afterwards
185+
*/
185186
public void saveAs() {
186-
getSavePath().ifPresent(this::saveAs);
187+
askForSavePath().ifPresent(this::saveAs);
187188
}
188189

189-
private Optional<Path> getSavePath() {
190+
/**
191+
* Asks the user for the path to save to. Stores the directory to the preferences, which is used next time when opening the dialog.
192+
*
193+
* @return the path set by the user
194+
*/
195+
public Optional<Path> askForSavePath() {
190196
FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
191197
.addExtensionFilter(StandardFileType.BIBTEX_DB)
192198
.withDefaultExtension(StandardFileType.BIBTEX_DB)
@@ -197,14 +203,22 @@ private Optional<Path> getSavePath() {
197203
return selectedPath;
198204
}
199205

200-
public void saveAs(Path file) {
206+
public boolean saveAs(Path file) {
207+
return this.saveAs(file, SaveDatabaseMode.NORMAL);
208+
}
209+
210+
/**
211+
* @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.
212+
* @return true on successful save
213+
*/
214+
public boolean saveAs(Path file, SaveDatabaseMode mode) {
201215
BibDatabaseContext context = panel.getBibDatabaseContext();
202216

203217
// Close AutosaveManager and BackupManager for original library
204218
Optional<Path> databasePath = context.getDatabasePath();
205219
if (databasePath.isPresent()) {
206220
final Path oldFile = databasePath.get();
207-
context.setDatabaseFile(oldFile.toFile());
221+
context.setDatabasePath(oldFile);
208222
AutosaveManager.shutdown(context);
209223
BackupManager.shutdown(context);
210224
}
@@ -215,22 +229,28 @@ public void saveAs(Path file) {
215229
new SharedDatabasePreferences(context.getDatabase().generateSharedDatabaseID())
216230
.putAllDBMSConnectionProperties(context.getDBMSSynchronizer().getConnectionProperties());
217231
}
218-
context.setDatabaseFile(file);
219232

220-
// Save
221-
save();
233+
boolean saveResult = save(file, mode);
222234

223-
// Reinstall AutosaveManager and BackupManager
224-
panel.resetChangeMonitorAndChangePane();
225-
if (readyForAutosave(context)) {
226-
AutosaveManager autosaver = AutosaveManager.start(context);
227-
autosaver.registerListener(new AutosaveUIManager(panel));
228-
}
229-
if (readyForBackup(context)) {
230-
BackupManager.start(context, entryTypesManager, prefs);
235+
if (saveResult) {
236+
// we managed to successfully save the file
237+
// thus, we can store the store the path into the context
238+
context.setDatabasePath(file);
239+
240+
// Reinstall AutosaveManager and BackupManager for the new file name
241+
panel.resetChangeMonitorAndChangePane();
242+
if (readyForAutosave(context)) {
243+
AutosaveManager autosaver = AutosaveManager.start(context);
244+
autosaver.registerListener(new AutosaveUIManager(panel));
245+
}
246+
if (readyForBackup(context)) {
247+
BackupManager.start(context, entryTypesManager, prefs);
248+
}
249+
250+
frame.getFileHistory().newFile(file);
231251
}
232252

233-
context.getDatabasePath().ifPresent(presentFile -> frame.getFileHistory().newFile(presentFile));
253+
return saveResult;
234254
}
235255

236256
private boolean readyForAutosave(BibDatabaseContext context) {
@@ -246,7 +266,7 @@ private boolean readyForBackup(BibDatabaseContext context) {
246266
}
247267

248268
public void saveSelectedAsPlain() {
249-
getSavePath().ifPresent(path -> {
269+
askForSavePath().ifPresent(path -> {
250270
try {
251271
saveDatabase(path, true, prefs.getDefaultEncoding(), SavePreferences.DatabaseSaveType.PLAIN_BIBTEX);
252272
frame.getFileHistory().newFile(path);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public void openSharedDatabaseFromParserResult(ParserResult parserResult)
173173
bibDatabaseContext.convertToSharedDatabase(synchronizer);
174174

175175
bibDatabaseContext.getDatabase().setSharedDatabaseID(sharedDatabaseID);
176-
bibDatabaseContext.setDatabaseFile(parserResult.getDatabaseContext().getDatabasePath().orElse(null));
176+
bibDatabaseContext.setDatabasePath(parserResult.getDatabaseContext().getDatabasePath().orElse(null));
177177

178178
dbmsSynchronizer = bibDatabaseContext.getDBMSSynchronizer();
179179
dbmsSynchronizer.openSharedDatabase(new DBMSConnection(dbmsConnectionProperties));

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,7 @@ public Optional<File> getDatabaseFile() {
8585
return file.map(Path::toFile);
8686
}
8787

88-
/**
89-
* @param file the database file
90-
* @deprecated use {@link #setDatabaseFile(Path)}
91-
*/
92-
@Deprecated
93-
public void setDatabaseFile(File file) {
94-
this.file = Optional.ofNullable(file).map(File::toPath);
95-
}
96-
97-
public void setDatabaseFile(Path file) {
88+
public void setDatabasePath(Path file) {
9889
this.file = Optional.ofNullable(file);
9990
}
10091

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

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
import static org.junit.jupiter.api.Assertions.assertEquals;
3535
import static org.junit.jupiter.api.Assertions.assertFalse;
3636
import static org.mockito.Mockito.any;
37-
import static org.mockito.Mockito.doNothing;
37+
import static org.mockito.Mockito.doReturn;
3838
import static org.mockito.Mockito.mock;
3939
import static org.mockito.Mockito.spy;
4040
import static org.mockito.Mockito.times;
@@ -65,7 +65,7 @@ public void setUp() {
6565
public void saveAsShouldSetWorkingDirectory() {
6666
when(preferences.get(JabRefPreferences.WORKING_DIRECTORY)).thenReturn(TEST_BIBTEX_LIBRARY_LOCATION);
6767
when(dialogService.showFileSaveDialog(any(FileDialogConfiguration.class))).thenReturn(Optional.of(file));
68-
doNothing().when(saveDatabaseAction).saveAs(any());
68+
doReturn(true).when(saveDatabaseAction).saveAs(any());
6969

7070
saveDatabaseAction.saveAs();
7171

@@ -76,35 +76,24 @@ public void saveAsShouldSetWorkingDirectory() {
7676
public void saveAsShouldNotSetWorkingDirectoryIfNotSelected() {
7777
when(preferences.get(JabRefPreferences.WORKING_DIRECTORY)).thenReturn(TEST_BIBTEX_LIBRARY_LOCATION);
7878
when(dialogService.showFileSaveDialog(any(FileDialogConfiguration.class))).thenReturn(Optional.empty());
79-
doNothing().when(saveDatabaseAction).saveAs(any());
79+
doReturn(false).when(saveDatabaseAction).saveAs(any());
8080

8181
saveDatabaseAction.saveAs();
8282

8383
verify(preferences, times(0)).setWorkingDir(file.getParent());
8484
}
8585

86-
@Test
87-
public void saveAsShouldSetNewDatabasePathIntoContext() {
88-
when(dbContext.getDatabasePath()).thenReturn(Optional.empty());
89-
when(dbContext.getLocation()).thenReturn(DatabaseLocation.LOCAL);
90-
when(preferences.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE)).thenReturn(false);
91-
92-
saveDatabaseAction.saveAs(file);
93-
94-
verify(dbContext, times(1)).setDatabaseFile(file);
95-
}
96-
9786
@Test
9887
public void saveShouldShowSaveAsIfDatabaseNotSelected() {
9988
when(dbContext.getDatabasePath()).thenReturn(Optional.empty());
10089
when(dbContext.getLocation()).thenReturn(DatabaseLocation.LOCAL);
10190
when(preferences.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE)).thenReturn(false);
10291
when(dialogService.showFileSaveDialog(any())).thenReturn(Optional.of(file));
103-
doNothing().when(saveDatabaseAction).saveAs(file);
92+
doReturn(true).when(saveDatabaseAction).saveAs(any(), any());
10493

105-
saveDatabaseAction.save();
94+
saveDatabaseAction.save(dbContext);
10695

107-
verify(saveDatabaseAction, times(1)).saveAs(file);
96+
verify(saveDatabaseAction, times(1)).saveAs(file, SaveDatabaseAction.SaveDatabaseMode.NORMAL);
10897
}
10998

11099
private SaveDatabaseAction createSaveDatabaseActionForBibDatabase(BibDatabase database) throws IOException {
@@ -151,7 +140,7 @@ public void saveKeepsChangedFlag() throws Exception {
151140
BibDatabase database = new BibDatabase(List.of(firstEntry, secondEntry));
152141

153142
saveDatabaseAction = createSaveDatabaseActionForBibDatabase(database);
154-
saveDatabaseAction.save();
143+
saveDatabaseAction.save(dbContext);
155144

156145
assertEquals(database
157146
.getEntries().stream()
@@ -162,9 +151,7 @@ public void saveKeepsChangedFlag() throws Exception {
162151
@Test
163152
public void saveShouldNotSaveDatabaseIfPathNotSet() {
164153
when(dbContext.getDatabasePath()).thenReturn(Optional.empty());
165-
166-
boolean result = saveDatabaseAction.save();
167-
154+
boolean result = saveDatabaseAction.save(dbContext);
168155
assertFalse(result);
169156
}
170157
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ void setUp(@TempDir Path bibFolder) throws IOException {
6060
metaData.setDefaultFileDirectory(pdfFolder.getAbsolutePath());
6161
BibDatabaseContext context = new BibDatabaseContext(new BibDatabase(), metaData);
6262
Files.createFile(bibFolder.resolve("test.bib"));
63-
context.setDatabaseFile(bibFolder.resolve("test.bib").toFile());
63+
context.setDatabasePath(bibFolder.resolve("test.bib"));
6464

6565
FilePreferences fileDirPrefs = mock(FilePreferences.class, Answers.RETURNS_SMART_NULLS);
6666
//Biblocation as Primary overwrites all other dirs

0 commit comments

Comments
 (0)