Fix entry gets deleted after aux import (#6746)

* Fix entry gets deleted after aux import Set changed flag on clone also for Misc entry type, because otherwise it equals the default entry type and no change is triggered which results in the entry not beeing written to the database on save Fixes #6405 Simplify gui code * add changelog * fix checkstyle shit * copy serialization on clone * Update CHANGELOG.md Co-authored-by: Tobias Diez <tobiasdiez@gmx.de> * Cosmetic change * Fix clone * Mark each entry changed to trigger a "proper" write by JabRef Co-authored-by: Tobias Diez <tobiasdiez@gmx.de> Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
JabRef · Aug 18, 2020 · ec53712 · ec53712
1 parent 3462288
commit ec53712
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -97,6 +97,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue with the creation of a group of cited entries. Now the file path to an aux file gets validated. [#6585](https://github.com/JabRef/jabref/issues/6585)
 - We fixed an issue on Linux systems where the application would crash upon inotify failure. Now, the user is prompted with a warning, and given the choice to continue the session. [#6073](https://github.com/JabRef/jabref/issues/6073)
 - We moved the search modifier buttons into the search bar, as they were not accessible, if autocompletion was disabled. [#6625](https://github.com/JabRef/jabref/issues/6625)
+- We fixed an issue where entries with the entry type Misc from an imported aux file would not be saved correctly to the bib file on disk [#6405](https://github.com/JabRef/jabref/issues/6405)
 - We fixed an issue where percent sign ('%') was not formatted properly by the HTML formatter [#6753](https://github.com/JabRef/jabref/issues/6753)
 
 ### Removed

diff --git a/src/main/java/org/jabref/gui/JabRefFrame.java b/src/main/java/org/jabref/gui/JabRefFrame.java
@@ -420,7 +420,7 @@ public boolean quit() {
                     Localization.lang("Waiting for background tasks to finish. Quit anyway?"),
                     stateManager
             );
-            if (!(shouldClose.isPresent() && shouldClose.get() == ButtonType.YES)) {
+            if (!(shouldClose.isPresent() && (shouldClose.get() == ButtonType.YES))) {
                 return false;
             }
         }

diff --git a/src/main/java/org/jabref/gui/auximport/FromAuxDialog.java b/src/main/java/org/jabref/gui/auximport/FromAuxDialog.java
@@ -2,19 +2,18 @@
 
 import java.nio.file.Path;
 
+import javax.inject.Inject;
+
 import javafx.fxml.FXML;
 import javafx.scene.control.Button;
 import javafx.scene.control.ButtonType;
 import javafx.scene.control.ListView;
 import javafx.scene.control.TextArea;
 import javafx.scene.control.TextField;
 
-import org.jabref.Globals;
 import org.jabref.gui.BasePanel;
-import org.jabref.gui.BasePanelPreferences;
 import org.jabref.gui.DialogService;
 import org.jabref.gui.JabRefFrame;
-import org.jabref.gui.externalfiletype.ExternalFileTypes;
 import org.jabref.gui.util.BaseDialog;
 import org.jabref.gui.util.FileDialogConfiguration;
 import org.jabref.logic.auxparser.DefaultAuxParser;
@@ -24,7 +23,7 @@
 import org.jabref.model.auxparser.AuxParserResult;
 import org.jabref.model.database.BibDatabase;
 import org.jabref.model.database.BibDatabaseContext;
-import org.jabref.preferences.JabRefPreferences;
+import org.jabref.preferences.PreferencesService;
 
 import com.airhacks.afterburner.views.ViewLoader;
 
@@ -33,20 +32,19 @@
  */
 public class FromAuxDialog extends BaseDialog<Void> {
 
-    private final DialogService dialogService;
     private final BasePanel basePanel;
     @FXML private ButtonType generateButtonType;
     private final Button generateButton;
     @FXML private TextField auxFileField;
     @FXML private ListView<String> notFoundList;
-
-    private AuxParserResult auxParserResult;
     @FXML private TextArea statusInfos;
+    private AuxParserResult auxParserResult;
+
+    @Inject private PreferencesService preferences;
+    @Inject private DialogService dialogService;
 
     public FromAuxDialog(JabRefFrame frame) {
         basePanel = frame.getCurrentBasePanel();
-        dialogService = frame.getDialogService();
-
         this.setTitle(Localization.lang("AUX file import"));
 
         ViewLoader.view(this)
@@ -58,8 +56,8 @@ public FromAuxDialog(JabRefFrame frame) {
         generateButton.defaultButtonProperty().bind(generateButton.disableProperty().not());
         setResultConverter(button -> {
             if (button == generateButtonType) {
-                BasePanel bp = new BasePanel(frame, BasePanelPreferences.from(Globals.prefs), new BibDatabaseContext(auxParserResult.getGeneratedBibDatabase()), ExternalFileTypes.getInstance());
-                frame.addTab(bp, true);
+                BibDatabaseContext context = new BibDatabaseContext(auxParserResult.getGeneratedBibDatabase());
+                frame.addTab(context, true);
             }
             return null;
         });
@@ -93,9 +91,9 @@ private void parseActionPerformed() {
     @FXML
     private void browseButtonClicked() {
         FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
-                .addExtensionFilter(StandardFileType.AUX)
-                .withDefaultExtension(StandardFileType.AUX)
-                .withInitialDirectory(Globals.prefs.get(JabRefPreferences.WORKING_DIRECTORY)).build();
+                                                                                               .addExtensionFilter(StandardFileType.AUX)
+                                                                                               .withDefaultExtension(StandardFileType.AUX)
+                                                                                               .withInitialDirectory(preferences.getWorkingDir()).build();
         dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(file -> auxFileField.setText(file.toAbsolutePath().toString()));
     }
 }
diff --git a/src/main/java/org/jabref/logic/auxparser/DefaultAuxParser.java b/src/main/java/org/jabref/logic/auxparser/DefaultAuxParser.java
@@ -178,12 +178,15 @@ private void resolveCrossReferences(List<BibEntry> entries, AuxParserResult resu
      * Insert a clone of each given entry. The clones are each given a new unique ID.
      *
      * @param entries Entries to be cloned
-     * @param result AUX file
+     * @param result the parser result (representing the AUX file)
      */
     private void insertEntries(List<BibEntry> entries, AuxParserResult result) {
         List<BibEntry> clonedEntries = new ArrayList<>();
         for (BibEntry entry : entries) {
-            clonedEntries.add((BibEntry) entry.clone());
+            BibEntry bibEntryToAdd = (BibEntry) entry.clone();
+            // ensure proper "rendering" of the BibTeX code
+            bibEntryToAdd.setChanged(true);
+            clonedEntries.add(bibEntryToAdd);
         }
         result.getGeneratedBibDatabase().insertEntries(clonedEntries);
     }

diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java
@@ -72,13 +72,23 @@ public class BibEntry implements Cloneable {
     private final MultiKeyMap<StandardField, Character, KeywordList> fieldsAsKeywords = new MultiKeyMap<>(StandardField.class);
 
     private final EventBus eventBus = new EventBus();
+
     private String id;
+
     private final ObjectProperty<EntryType> type = new SimpleObjectProperty<>(DEFAULT_TYPE);
 
     private ObservableMap<Field, String> fields = FXCollections.observableMap(new ConcurrentHashMap<>());
-    private String parsedSerialization = "";
+
+    /**
+     * The part before the start of the entry
+     */
     private String commentsBeforeEntry = "";
 
+    /**
+     * Stores the text "rendering" of the entry as read by the BibTeX reader. Includes comments.
+     */
+    private String parsedSerialization = "";
+
     /**
      * Marks whether the complete serialization, which was read from file, should be used.
      * <p>
@@ -355,7 +365,8 @@ public Optional<FieldChange> setType(EntryType type) {
     }
 
     /**
-     * Sets this entry's type.
+     * Sets this entry's type and sets the changed flag to true <br>
+     * If the new entry type equals the old entry type no changed flag is set.
      */
     public Optional<FieldChange> setType(EntryType newType, EntriesEventSource eventSource) {
         Objects.requireNonNull(newType);
@@ -608,6 +619,9 @@ public boolean allFieldsPresent(Collection<OrFields> fields, BibDatabase databas
     public Object clone() {
         BibEntry clone = new BibEntry(type.getValue());
         clone.fields = FXCollections.observableMap(new ConcurrentHashMap<>(fields));
+        clone.commentsBeforeEntry = new String(commentsBeforeEntry);
+        clone.parsedSerialization = new String(parsedSerialization);
+        clone.changed = changed;
         return clone;
     }
 

diff --git a/src/test/java/org/jabref/logic/auxparser/AuxParserTest.java b/src/test/java/org/jabref/logic/auxparser/AuxParserTest.java
@@ -6,6 +6,7 @@
 import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
+import java.util.List;
 import java.util.Optional;
 
 import org.jabref.logic.importer.ImportFormatPreferences;
@@ -14,6 +15,7 @@
 import org.jabref.model.auxparser.AuxParser;
 import org.jabref.model.auxparser.AuxParserResult;
 import org.jabref.model.database.BibDatabase;
+import org.jabref.model.entry.BibEntry;
 import org.jabref.model.util.DummyFileUpdateMonitor;
 
 import org.junit.jupiter.api.AfterEach;
@@ -52,7 +54,10 @@ void testNormal() throws URISyntaxException, IOException {
             assertTrue(auxResult.getGeneratedBibDatabase().hasEntries());
             assertEquals(0, auxResult.getUnresolvedKeysCount());
             BibDatabase newDB = auxResult.getGeneratedBibDatabase();
-            assertEquals(2, newDB.getEntries().size());
+            List<BibEntry> newEntries = newDB.getEntries();
+            assertEquals(2, newEntries.size());
+            assertTrue(newEntries.get(0).hasChanged());
+            assertTrue(newEntries.get(1).hasChanged());
             assertEquals(2, auxResult.getResolvedKeysCount());
             assertEquals(2, auxResult.getFoundKeysInAux());
             assertEquals(auxResult.getFoundKeysInAux() + auxResult.getCrossRefEntriesCount(),

diff --git a/src/test/java/org/jabref/logic/auxparser/DefaultAuxParserTest.java b/src/test/java/org/jabref/logic/auxparser/DefaultAuxParserTest.java
@@ -0,0 +1,8 @@
+package org.jabref.logic.auxparser;
+
+/**
+ * Please see {@link AuxParserTest} for the test cases
+ */
+class DefaultAuxParserTest {
+
+}
diff --git a/src/test/java/org/jabref/logic/cleanup/BibtexBiblatexRoundtripTest.java b/src/test/java/org/jabref/logic/cleanup/BibtexBiblatexRoundtripTest.java
@@ -16,50 +16,52 @@ class BibtexBiblatexRoundtripTest {
 
     @BeforeEach
     void setUp() {
-        bibtex = new BibEntry(StandardEntryType.Article);
-        bibtex.setField(StandardField.AUTHOR, "Frame, J. S. and Robinson, G. de B. and Thrall, R. M.");
-        bibtex.setField(StandardField.TITLE, "The hook graphs of the symmetric groups");
-        bibtex.setField(StandardField.JOURNAL, "Canadian J. Math.");
-        bibtex.setField(new UnknownField("fjournal"), "Canadian Journal of Mathematics. Journal Canadien de Math\\'ematiques");
-        bibtex.setField(StandardField.VOLUME, "6");
-        bibtex.setField(StandardField.YEAR, "1954");
-        bibtex.setField(StandardField.PAGES, "316--324");
-        bibtex.setField(StandardField.ISSN, "0008-414X");
-        bibtex.setField(new UnknownField("mrclass"), "20.0X");
-        bibtex.setField(StandardField.MR_NUMBER, "0062127");
-        bibtex.setField(new UnknownField("mrreviewer"), "D. E. Littlewood");
+        bibtex = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.AUTHOR, "Frame, J. S. and Robinson, G. de B. and Thrall, R. M.")
+                .withField(StandardField.TITLE, "The hook graphs of the symmetric groups")
+                .withField(StandardField.JOURNAL, "Canadian J. Math.")
+                .withField(new UnknownField("fjournal"), "Canadian Journal of Mathematics. Journal Canadien de Math\\'ematiques")
+                .withField(StandardField.VOLUME, "6")
+                .withField(StandardField.YEAR, "1954")
+                .withField(StandardField.PAGES, "316--324")
+                .withField(StandardField.ISSN, "0008-414X")
+                .withField(new UnknownField("mrclass"), "20.0X")
+                .withField(StandardField.MR_NUMBER, "0062127")
+                .withField(new UnknownField("mrreviewer"), "D. E. Littlewood");
 
-        biblatex = new BibEntry(StandardEntryType.Article);
-        biblatex.setField(StandardField.AUTHOR, "Frame, J. S. and Robinson, G. de B. and Thrall, R. M.");
-        biblatex.setField(StandardField.TITLE, "The hook graphs of the symmetric groups");
-        biblatex.setField(StandardField.JOURNALTITLE, "Canadian J. Math.");
-        biblatex.setField(new UnknownField("fjournal"), "Canadian Journal of Mathematics. Journal Canadien de Math\\'ematiques");
-        biblatex.setField(StandardField.VOLUME, "6");
-        biblatex.setField(StandardField.DATE, "1954");
-        biblatex.setField(StandardField.PAGES, "316--324");
-        biblatex.setField(StandardField.ISSN, "0008-414X");
-        biblatex.setField(new UnknownField("mrclass"), "20.0X");
-        biblatex.setField(StandardField.MR_NUMBER, "0062127");
-        biblatex.setField(new UnknownField("mrreviewer"), "D. E. Littlewood");
+        biblatex = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.AUTHOR, "Frame, J. S. and Robinson, G. de B. and Thrall, R. M.")
+                .withField(StandardField.TITLE, "The hook graphs of the symmetric groups")
+                .withField(StandardField.JOURNALTITLE, "Canadian J. Math.")
+                .withField(new UnknownField("fjournal"), "Canadian Journal of Mathematics. Journal Canadien de Math\\'ematiques")
+                .withField(StandardField.VOLUME, "6")
+                .withField(StandardField.DATE, "1954")
+                .withField(StandardField.PAGES, "316--324")
+                .withField(StandardField.ISSN, "0008-414X")
+                .withField(new UnknownField("mrclass"), "20.0X")
+                .withField(StandardField.MR_NUMBER, "0062127")
+                .withField(new UnknownField("mrreviewer"), "D. E. Littlewood");
     }
 
     @Test
     void roundTripBibtexToBiblatexIsIdentity() {
         BibEntry clone = (BibEntry) bibtex.clone();
+
         new ConvertToBiblatexCleanup().cleanup(clone);
         assertEquals(biblatex, clone);
-        new ConvertToBibtexCleanup().cleanup(clone);
 
+        new ConvertToBibtexCleanup().cleanup(clone);
         assertEquals(bibtex, clone);
     }
 
     @Test
     void roundTripBiblatexToBibtexIsIdentity() {
         BibEntry clone = (BibEntry) biblatex.clone();
+
         new ConvertToBibtexCleanup().cleanup(clone);
         assertEquals(bibtex, clone);
-        new ConvertToBiblatexCleanup().cleanup(clone);
 
+        new ConvertToBiblatexCleanup().cleanup(clone);
         assertEquals(biblatex, clone);
     }
 }
diff --git a/src/test/java/org/jabref/model/entry/BibEntryTest.java b/src/test/java/org/jabref/model/entry/BibEntryTest.java
@@ -97,6 +97,19 @@ void clonedBibEntryHasUniqueID() throws Exception {
         assertNotEquals(entry.getId(), entryClone.getId());
     }
 
+    @Test
+    void clonedBibEntryWithMiscTypeHasOriginalChangedFlag() throws Exception {
+        BibEntry entryClone = (BibEntry) entry.clone();
+        assertFalse(entryClone.hasChanged());
+    }
+
+    @Test
+    void clonedBibEntryWithBookTypeAndOneFieldHasOriginalChangedFlag() throws Exception {
+        entry = new BibEntry(StandardEntryType.Book).withField(StandardField.AUTHOR, "value");
+        BibEntry entryClone = (BibEntry) entry.clone();
+        assertFalse(entryClone.hasChanged());
+    }
+
     @Test
     void setAndGetAreConsistentForMonth() throws Exception {
         entry.setField(StandardField.MONTH, "may");