Skip to content

Commit

Permalink
Add tests for changed
Browse files Browse the repository at this point in the history
- Add tests
- Guard AtomicFileWriter with a try-with-resources during save
- use Objects.requireNonNull in DefaultFileUpdateMontitor
- add comment
- some auto-formattings
- Fix typo in comment
  • Loading branch information
koppor committed Nov 21, 2019
1 parent 782953d commit b5ac16a
Show file tree
Hide file tree
Showing 9 changed files with 180 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public void listen(@SuppressWarnings("unused") AutosaveEvent event) {
try {
new SaveDatabaseAction(panel, Globals.prefs, Globals.entryTypesManager).save(SaveDatabaseAction.SaveDatabaseMode.SILENT);
} catch (Throwable e) {
LOGGER.error("Problem occured while saving.", e);
LOGGER.error("Problem occurred while saving.", e);
}
}
}
14 changes: 6 additions & 8 deletions src/main/java/org/jabref/gui/exporter/SaveDatabaseAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
/**
* Action for the "Save" and "Save as" operations called from BasePanel. This class is also used for save operations
* when closing a database or quitting the applications.
*
* <p>
* The save operation is loaded off of the GUI thread using {@link BackgroundTask}. Callers can query whether the
* operation was canceled, or whether it was successful.
*/
Expand All @@ -69,12 +69,10 @@ public SaveDatabaseAction(BasePanel panel, JabRefPreferences prefs, BibEntryType
}

private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding, SavePreferences.DatabaseSaveType saveType) throws SaveException {
try {
SavePreferences preferences = prefs.loadForSaveFromPreferences()
.withEncoding(encoding)
.withSaveType(saveType);

AtomicFileWriter fileWriter = new AtomicFileWriter(file, preferences.getEncoding(), preferences.makeBackup());
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);

if (selectedOnly) {
Expand Down Expand Up @@ -148,7 +146,7 @@ private boolean doSave() {

// Reset title of tab
frame.setTabTitle(panel, panel.getTabTitle(),
panel.getBibDatabaseContext().getDatabaseFile().get().getAbsolutePath());
panel.getBibDatabaseContext().getDatabasePath().get().toAbsolutePath().toString());
frame.setWindowTitle();
frame.updateAllTabTitles();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.nio.file.WatchEvent;
import java.nio.file.WatchKey;
import java.nio.file.WatchService;
import java.util.Objects;

import org.jabref.model.util.FileUpdateListener;
import org.jabref.model.util.FileUpdateMonitor;
Expand All @@ -19,7 +20,7 @@
/**
* This class monitors a set of files for changes. Upon detecting a change it notifies the registered {@link
* FileUpdateListener}s.
*
* <p>
* Implementation based on https://stackoverflow.com/questions/16251273/can-i-watch-for-single-file-change-with-watchservice-not-the-whole-directory
*/
public class DefaultFileUpdateMonitor implements Runnable, FileUpdateMonitor {
Expand Down Expand Up @@ -69,9 +70,7 @@ private void notifyAboutChange(Path path) {

@Override
public void addListenerForFile(Path file, FileUpdateListener listener) throws IOException {
if (watcher == null) {
throw new IllegalStateException("You need to start the file monitor before watching files");
}
Objects.requireNonNull(watcher, "You need to start the file monitor before watching files");

// We can't watch files directly, so monitor their parent directory for updates
Path directory = file.toAbsolutePath().getParent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ public void saveDatabase(BibDatabaseContext bibDatabaseContext) throws IOExcepti
*/
public void savePartOfDatabase(BibDatabaseContext bibDatabaseContext, List<BibEntry> entries) throws IOException {
Optional<String> sharedDatabaseIDOptional = bibDatabaseContext.getDatabase().getSharedDatabaseID();

if (sharedDatabaseIDOptional.isPresent()) {
// may throw an IOException. Thus, we do not use "ifPresent", but the "old" isPresent way
writeDatabaseID(sharedDatabaseIDOptional.get());
}

Expand Down
16 changes: 8 additions & 8 deletions src/main/java/org/jabref/model/database/BibDatabaseContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ public class BibDatabaseContext {
private final BibDatabase database;
private final Defaults defaults;
private MetaData metaData;

/**
* The file where this database was last saved to.
*/
private Optional<Path> file;

private DatabaseSynchronizer dbmsSynchronizer;
private CoarseChangeFilter dbmsListener;
private DatabaseLocation location;
Expand Down Expand Up @@ -115,7 +117,6 @@ public Optional<File> getDatabaseFile() {
}

/**
*
* @param file the database file
* @deprecated use {@link #setDatabaseFile(Path)}
*/
Expand Down Expand Up @@ -155,11 +156,11 @@ public boolean isBiblatexMode() {
public List<Path> getFileDirectoriesAsPaths(FilePreferences preferences) {
// Filter for empty string, as this would be expanded to the jar-directory with Paths.get()
return getFileDirectories(preferences).stream()
.filter(s -> !s.isEmpty())
.map(Paths::get)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.collect(Collectors.toList());
.filter(s -> !s.isEmpty())
.map(Paths::get)
.map(Path::toAbsolutePath)
.map(Path::normalize)
.collect(Collectors.toList());
}

/**
Expand Down Expand Up @@ -192,7 +193,7 @@ public Optional<Path> getFirstExistingFileDir(FilePreferences preferences) {
* <li>BIB file directory</li>
* </ol>
*
* @param field The field
* @param field The field
* @param preferences The fileDirectory preferences
* @return The default directory for this field type.
*/
Expand Down Expand Up @@ -293,5 +294,4 @@ public void convertToLocalDatabase() {
public List<BibEntry> getEntries() {
return database.getEntries();
}

}
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/model/entry/BibEntry.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public class BibEntry implements Cloneable {
/**
* Marks whether the complete serialization, which was read from file, should be used.
*
* Is set to false, if parts of the entry change. This causes the entry to be serialized based on the internal state (and not based on the old serialization)
* Is set to <code>true</code>, if parts of the entry changed. This causes the entry to be serialized based on the internal state (and not based on the old serialization)
*/
private boolean changed;

Expand Down
66 changes: 64 additions & 2 deletions src/test/java/org/jabref/gui/exporter/SaveDatabaseActionTest.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,34 @@
package org.jabref.gui.exporter;

import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Optional;

import org.jabref.gui.BasePanel;
import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.undo.CountingUndoManager;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.bibtex.FieldContentParserPreferences;
import org.jabref.logic.bibtex.LatexFieldFormatterPreferences;
import org.jabref.logic.exporter.SavePreferences;
import org.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.shared.DatabaseLocation;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.metadata.MetaData;
import org.jabref.preferences.JabRefPreferences;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doNothing;
Expand All @@ -27,8 +41,7 @@
class SaveDatabaseActionTest {

private static final String TEST_BIBTEX_LIBRARY_LOCATION = "C:\\Users\\John_Doe\\Jabref\\literature.bib";
private final Path file = Path.of(TEST_BIBTEX_LIBRARY_LOCATION);

private Path file = Path.of(TEST_BIBTEX_LIBRARY_LOCATION);
private DialogService dialogService = mock(DialogService.class);
private JabRefPreferences preferences = mock(JabRefPreferences.class);
private BasePanel basePanel = mock(BasePanel.class);
Expand Down Expand Up @@ -91,6 +104,55 @@ public void saveShouldShowSaveAsIfDatabaseNotSelected() {
verify(saveDatabaseAction, times(1)).saveAs(file);
}

private SaveDatabaseAction createSaveDatabaseActionForBibDatabase(BibDatabase database) throws IOException {
file = Files.createTempFile("JabRef", ".bib");
file.toFile().deleteOnExit();

LatexFieldFormatterPreferences latexFieldFormatterPreferences = mock(LatexFieldFormatterPreferences.class);
when(latexFieldFormatterPreferences.getFieldContentParserPreferences()).thenReturn(mock(FieldContentParserPreferences.class));
SavePreferences savePreferences = mock(SavePreferences.class);
// In case a "thenReturn" is modified, the whole mock has to be recreated
dbContext = mock(BibDatabaseContext.class);
basePanel = mock(BasePanel.class);
MetaData metaData = mock(MetaData.class);
when(savePreferences.withEncoding(any(Charset.class))).thenReturn(savePreferences);
when(savePreferences.withSaveType(any(SavePreferences.DatabaseSaveType.class))).thenReturn(savePreferences);
when(savePreferences.getEncoding()).thenReturn(Charset.forName("UTF-8"));
when(savePreferences.getLatexFieldFormatterPreferences()).thenReturn(latexFieldFormatterPreferences);
GlobalBibtexKeyPattern emptyGlobalBibtexKeyPattern = GlobalBibtexKeyPattern.fromPattern("");
when(savePreferences.getGlobalCiteKeyPattern()).thenReturn(emptyGlobalBibtexKeyPattern);
when(metaData.getCiteKeyPattern(any(GlobalBibtexKeyPattern.class))).thenReturn(emptyGlobalBibtexKeyPattern);
when(dbContext.getDatabasePath()).thenReturn(Optional.of(file));
when(dbContext.getLocation()).thenReturn(DatabaseLocation.LOCAL);
when(dbContext.getDatabase()).thenReturn(database);
when(dbContext.getMetaData()).thenReturn(metaData);
when(dbContext.getEntries()).thenReturn(database.getEntries());
when(preferences.getBoolean(JabRefPreferences.LOCAL_AUTO_SAVE)).thenReturn(false);
when(preferences.getDefaultEncoding()).thenReturn(Charset.forName("UTF-8"));
when(preferences.getFieldContentParserPreferences()).thenReturn(mock(FieldContentParserPreferences.class));
when(preferences.loadForSaveFromPreferences()).thenReturn(savePreferences);
when(basePanel.frame()).thenReturn(jabRefFrame);
when(basePanel.getBibDatabaseContext()).thenReturn(dbContext);
when(basePanel.getUndoManager()).thenReturn(mock(CountingUndoManager.class));
when(basePanel.getBibDatabaseContext()).thenReturn(dbContext);
saveDatabaseAction = new SaveDatabaseAction(basePanel, preferences, mock(BibEntryTypesManager.class));
return saveDatabaseAction;
}

@Test
public void saveKeepsChangedFlag() throws Exception {
BibEntry firstEntry = new BibEntry().withField(StandardField.AUTHOR, "first");
firstEntry.setChanged(true);
BibEntry secondEntry = new BibEntry().withField(StandardField.AUTHOR, "second");
secondEntry.setChanged(true);
BibDatabase database = new BibDatabase(List.of(firstEntry, secondEntry));

saveDatabaseAction = createSaveDatabaseActionForBibDatabase(database);
saveDatabaseAction.save();

assertThat(database.getEntries()).extracting(BibEntry::hasChanged).doesNotContain(false);
}

@Test
public void saveShouldNotSaveDatabaseIfPathNotSet() {
when(dbContext.getDatabasePath()).thenReturn(Optional.empty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Scanner;

Expand Down Expand Up @@ -538,7 +539,7 @@ void writeFileDirectories() throws Exception {

assertEquals(OS.NEWLINE + "@Comment{jabref-meta: fileDirectory:\\\\Literature\\\\;}" + OS.NEWLINE +
OS.NEWLINE + "@Comment{jabref-meta: fileDirectory-defaultOwner-user:D:\\\\Documents;}"
+ OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: fileDirectoryLatex-defaultOwner-user:D:\\\\Latex;}" + OS.NEWLINE, stringWriter.toString());
+ OS.NEWLINE + OS.NEWLINE + "@Comment{jabref-meta: fileDirectoryLatex-defaultOwner-user:D:\\\\Latex;}" + OS.NEWLINE, stringWriter.toString());
}

@Test
Expand Down Expand Up @@ -651,4 +652,87 @@ void roundtripWithContentSelectorsAndUmlauts() throws Exception {

assertEquals(fileContent, stringWriter.toString());
}

@Test
public void saveAlsoSavesSecondModification() throws Exception {
// @formatter:off
String bibtexEntry = OS.NEWLINE + "@Article{test," + OS.NEWLINE +
" Author = {Foo Bar}," + OS.NEWLINE +
" Journal = {International Journal of Something}," + OS.NEWLINE +
" Note = {some note}," + OS.NEWLINE +
" Number = {1}," + OS.NEWLINE +
"}";
// @formatter:on

// read in bibtex string
ImportFormatPreferences importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS);
ParserResult firstParse = new BibtexParser(importFormatPreferences, new DummyFileUpdateMonitor()).parse(new StringReader(bibtexEntry));
Collection<BibEntry> entries = firstParse.getDatabase().getEntries();
BibEntry entry = entries.iterator().next();

// modify entry
entry.setField(StandardField.AUTHOR, "BlaBla");

BibDatabaseContext context = new BibDatabaseContext(firstParse.getDatabase(), firstParse.getMetaData(),
new Defaults(BibDatabaseMode.BIBTEX));

databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries());

// modify entry a second time
entry.setField(StandardField.AUTHOR, "Test");

// write a second time
stringWriter = new StringWriter();
databaseWriter = new BibtexDatabaseWriter(stringWriter, preferences, entryTypesManager);
databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries());

assertEquals(OS.NEWLINE +
"@Article{test," + OS.NEWLINE +
" author = {Test}," + OS.NEWLINE +
" journal = {International Journal of Something}," + OS.NEWLINE +
" note = {some note}," + OS.NEWLINE +
" number = {1}," + OS.NEWLINE +
"}" + OS.NEWLINE +
"" + OS.NEWLINE +
"@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, stringWriter.toString());
}

@Test
public void saveReturnsToOriginalEntryWhenEntryIsUnchanged() throws Exception {
// @formatter:off
String bibtexEntry = OS.NEWLINE + "@Article{test," + OS.NEWLINE +
" Author = {Foo Bar}," + OS.NEWLINE +
" Journal = {International Journal of Something}," + OS.NEWLINE +
" Note = {some note}," + OS.NEWLINE +
" Number = {1}," + OS.NEWLINE +
"}";
// @formatter:on

// read in bibtex string
ImportFormatPreferences importFormatPreferences = mock(ImportFormatPreferences.class, Answers.RETURNS_DEEP_STUBS);
ParserResult firstParse = new BibtexParser(importFormatPreferences, new DummyFileUpdateMonitor()).parse(new StringReader(bibtexEntry));
Collection<BibEntry> entries = firstParse.getDatabase().getEntries();
BibEntry entry = entries.iterator().next();

// modify entry
entry.setField(StandardField.AUTHOR, "BlaBla");

BibDatabaseContext context = new BibDatabaseContext(firstParse.getDatabase(), firstParse.getMetaData(),
new Defaults(BibDatabaseMode.BIBTEX));

databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries());

// modify entry a second time
entry.setField(StandardField.AUTHOR, "Test");

entry.setChanged(false);

// write a second time
stringWriter = new StringWriter();
databaseWriter = new BibtexDatabaseWriter(stringWriter, preferences, entryTypesManager);
databaseWriter.savePartOfDatabase(context, firstParse.getDatabase().getEntries());

assertEquals(bibtexEntry + OS.NEWLINE +
"@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, stringWriter.toString());
}
}
11 changes: 11 additions & 0 deletions src/test/java/org/jabref/model/entry/BibEntryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ public void getFieldWorksWithBibFieldAsWell() throws Exception {
assertEquals(Optional.of("value"), entry.getField(new BibField(StandardField.AUTHOR, FieldPriority.IMPORTANT).getField()));
}

@Test
public void newBibEntryIsUnchanged() {
assertFalse(entry.hasChanged());
}

@Test
public void setFieldLeadsToAChangedEntry() throws Exception {
entry.setField(StandardField.AUTHOR, "value");
assertTrue(entry.hasChanged());
}

@Test
public void setFieldWorksWithBibFieldAsWell() throws Exception {
entry.setField(new BibField(StandardField.AUTHOR, FieldPriority.IMPORTANT).getField(), "value");
Expand Down

0 comments on commit b5ac16a

Please sign in to comment.