Add tests for "changed" flag (#5640)

- 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

.gitattributes

@@ -1,13 +1,19 @@
 # unix  line endings at unix files
 gradlew text eol=lf
 *.sh text eol=lf
+
+# windows line endings at windows files
 *.bat text eol=crlf
 
+# required for proper releasing
 AUTHORS text eol=lf
 
-# ensure that line endings of *.java and *.properties are normalized
-*.properties text
+# ensure that line endings of *.java, and *.properties are normalized
 *.java text
+*.properties text
+
+# .bib files have to be written using OS specific line endings to enable our tests working
+*.bib text !eol
 
 # disable after a release
 # CHANGELOG.md merge=union

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

@@ -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);
         }
     }
 }

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

@@ -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.
  */
@@ -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) {
@@ -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();
             }

src/main/java/org/jabref/gui/util/DefaultFileUpdateMonitor.java

@@ -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;
@@ -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 {
@@ -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();

src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java

@@ -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());
         }
 

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

@@ -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;
@@ -115,7 +117,6 @@ public Optional<File> getDatabaseFile() {
     }
 
     /**
-     *
      * @param file the database file
      * @deprecated use {@link #setDatabaseFile(Path)}
      */
@@ -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());
     }
 
     /**
@@ -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.
      */
@@ -293,5 +294,4 @@ public void convertToLocalDatabase() {
     public List<BibEntry> getEntries() {
         return database.getEntries();
     }
-
 }

src/main/java/org/jabref/model/entry/BibEntry.java

@@ -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;
 

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

@@ -1,20 +1,36 @@
 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.Collections;
+import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 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.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;
@@ -27,8 +43,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);
@@ -91,6 +106,58 @@ 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();
+
+        assertEquals(database
+                        .getEntries().stream()
+                        .map(entry -> entry.hasChanged()).filter(changed -> false).collect(Collectors.toList()),
+                Collections.emptyList());
+    }
+
     @Test
     public void saveShouldNotSaveDatabaseIfPathNotSet() {
         when(dbContext.getDatabasePath()).thenReturn(Optional.empty());