Fix order of fields in bib file (#6750)

tobiasdiez · web-flow · commit 9adfd5bb0d59 · 2020-08-19T09:43:57.000+02:00
diff --git a/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java b/src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java
@@ -3,10 +3,13 @@
 import java.io.IOException;
 import java.io.StringWriter;
 import java.io.Writer;
+import java.util.Collection;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Optional;
+import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
 import java.util.function.Predicate;
@@ -21,7 +24,6 @@
 import org.jabref.model.entry.field.BibField;
 import org.jabref.model.entry.field.Field;
 import org.jabref.model.entry.field.InternalField;
-import org.jabref.model.entry.field.OrFields;
 import org.jabref.model.strings.StringUtil;
 
 public class BibEntryWriter {
@@ -91,30 +93,44 @@ public void writeWithoutPrependedNewlines(BibEntry entry, Writer out, BibDatabas
      */
     private void writeRequiredFieldsFirstRemainingFieldsSecond(BibEntry entry, Writer out,
                                                                BibDatabaseMode bibDatabaseMode) throws IOException {
-        // Write header with type and bibtex-key.
+        // Write header with type and bibtex-key
         TypedBibEntry typedEntry = new TypedBibEntry(entry, bibDatabaseMode);
         out.write('@' + typedEntry.getTypeForDisplay() + '{');
 
         writeKeyField(entry, out);
 
-        TreeSet<Field> written = new TreeSet<>(Comparator.comparing(Field::getName));
+        Set<Field> written = new HashSet<>();
         written.add(InternalField.KEY_FIELD);
         int indentation = getLengthOfLongestFieldName(entry);
 
         Optional<BibEntryType> type = entryTypesManager.enrich(entry.getType(), bibDatabaseMode);
         if (type.isPresent()) {
-            // Write required fields first.
-            for (OrFields value : type.get().getRequiredFields()) {
-                for (Field field : value) {
-                    writeField(entry, out, field, indentation);
-                    written.add(field);
-                }
+            // Write required fields first
+            List<Field> requiredFields = type.get()
+                                             .getRequiredFields()
+                                             .stream()
+                                             .flatMap(Collection::stream)
+                                             .sorted(Comparator.comparing(Field::getName))
+                                             .collect(Collectors.toList());
+
+            for (Field field : requiredFields) {
+                writeField(entry, out, field, indentation);
             }
-            // Then optional fields.
-            for (BibField field : type.get().getOptionalFields()) {
-                writeField(entry, out, field.getField(), indentation);
-                written.add(field.getField());
+
+            // Then optional fields
+            List<Field> optionalFields = type.get()
+                                             .getOptionalFields()
+                                             .stream()
+                                             .map(BibField::getField)
+                                             .sorted(Comparator.comparing(Field::getName))
+                                             .collect(Collectors.toList());
+
+            for (Field field : optionalFields) {
+                writeField(entry, out, field, indentation);
             }
+
+            written.addAll(requiredFields);
+            written.addAll(optionalFields);
         }
         // Then write remaining fields in alphabetic order.
         SortedSet<Field> remainingFields = entry.getFields()
diff --git a/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java b/src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java
@@ -22,7 +22,6 @@
 import org.jabref.model.util.FileUpdateMonitor;
 
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.mockito.Answers;
 
@@ -64,8 +63,8 @@ void testSerialization() throws IOException {
         String expected = OS.NEWLINE + "@Article{," + OS.NEWLINE +
                 "  author  = {Foo Bar}," + OS.NEWLINE +
                 "  journal = {International Journal of Something}," + OS.NEWLINE +
-                "  number  = {1}," + OS.NEWLINE +
                 "  note    = {some note}," + OS.NEWLINE +
+                "  number  = {1}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
         // @formatter:on
 
@@ -126,8 +125,8 @@ void writeEntryWithOrField() throws Exception {
         // @formatter:off
         String expected = OS.NEWLINE + "@InBook{," + OS.NEWLINE +
                 "  editor  = {Foo Bar}," + OS.NEWLINE +
-                "  number  = {1}," + OS.NEWLINE +
                 "  note    = {some note}," + OS.NEWLINE +
+                "  number  = {1}," + OS.NEWLINE +
                 "  journal = {International Journal of Something}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
         // @formatter:on
@@ -156,8 +155,8 @@ void writeEntryWithOrFieldBothFieldsPresent() throws Exception {
         String expected = OS.NEWLINE + "@InBook{," + OS.NEWLINE +
                 "  author  = {Foo Thor}," + OS.NEWLINE +
                 "  editor  = {Edi Bar}," + OS.NEWLINE +
-                "  number  = {1}," + OS.NEWLINE +
                 "  note    = {some note}," + OS.NEWLINE +
+                "  number  = {1}," + OS.NEWLINE +
                 "  journal = {International Journal of Something}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
         // @formatter:on
@@ -260,8 +259,8 @@ void roundTripWithModification() throws IOException {
         String expected = OS.NEWLINE + "@Article{test," + OS.NEWLINE +
                 "  author  = {BlaBla}," + OS.NEWLINE +
                 "  journal = {International Journal of Something}," + OS.NEWLINE +
-                "  number  = {1}," + OS.NEWLINE +
                 "  note    = {some note}," + OS.NEWLINE +
+                "  number  = {1}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
         // @formatter:on
         assertEquals(expected, actual);
@@ -296,8 +295,8 @@ void roundTripWithCamelCasingInTheOriginalEntryAndResultInLowerCase() throws IOE
         String expected = OS.NEWLINE + "@Article{test," + OS.NEWLINE +
                 "  author       = {BlaBla}," + OS.NEWLINE +
                 "  journal      = {International Journal of Something}," + OS.NEWLINE +
-                "  number       = {1}," + OS.NEWLINE +
                 "  note         = {some note}," + OS.NEWLINE +
+                "  number       = {1}," + OS.NEWLINE +
                 "  howpublished = {asdf}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
         // @formatter:on
@@ -332,8 +331,8 @@ void testEntryTypeChange() throws IOException {
         // @formatter:off
         String expectedNewEntry = OS.NEWLINE + "@InProceedings{test," + OS.NEWLINE +
                 "  author       = {BlaBla}," + OS.NEWLINE +
-                "  number       = {1}," + OS.NEWLINE +
                 "  note         = {some note}," + OS.NEWLINE +
+                "  number       = {1}," + OS.NEWLINE +
                 "  howpublished = {asdf}," + OS.NEWLINE +
                 "  journal      = {International Journal of Something}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
@@ -489,8 +488,8 @@ void addFieldWithLongerLength() throws IOException {
         String expected = OS.NEWLINE + "@Article{test," + OS.NEWLINE +
                 "  author       = {BlaBla}," + OS.NEWLINE +
                 "  journal      = {International Journal of Something}," + OS.NEWLINE +
-                "  number       = {1}," + OS.NEWLINE +
                 "  note         = {some note}," + OS.NEWLINE +
+                "  number       = {1}," + OS.NEWLINE +
                 "  howpublished = {asdf}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
         // @formatter:on
@@ -580,15 +579,14 @@ void roundTripWithPrecedingCommentAndModificationTest() throws IOException {
                 "@Article{test," + OS.NEWLINE +
                 "  author  = {John Doe}," + OS.NEWLINE +
                 "  journal = {International Journal of Something}," + OS.NEWLINE +
-                "  number  = {1}," + OS.NEWLINE +
                 "  note    = {some note}," + OS.NEWLINE +
+                "  number  = {1}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
         // @formatter:on
 
         assertEquals(expected, actual);
     }
 
-    @Disabled("We don't have alphabetic serialization anylonger TODO")
     @Test
     void alphabeticSerialization() throws IOException {
         StringWriter stringWriter = new StringWriter();
@@ -616,8 +614,8 @@ void alphabeticSerialization() throws IOException {
                 "  date         = {2019-10-16}," + OS.NEWLINE +
                 "  journaltitle = {International Journal of Something}," + OS.NEWLINE +
                 "  title        = {Title}," + OS.NEWLINE +
-                "  number       = {1}," + OS.NEWLINE +
                 "  note         = {some note}," + OS.NEWLINE +
+                "  number       = {1}," + OS.NEWLINE +
                 "  chapter      = {chapter}," + OS.NEWLINE +
                 "  year         = {2019}," + OS.NEWLINE +
                 "}" + OS.NEWLINE;
diff --git a/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java b/src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java
@@ -694,7 +694,7 @@ void roundtripWithContentSelectorsAndUmlauts() throws Exception {
     }
 
     @Test
-    public void saveAlsoSavesSecondModification() throws Exception {
+    void saveAlsoSavesSecondModification() throws Exception {
         // @formatter:off
         String bibtexEntry = OS.NEWLINE + "@Article{test," + OS.NEWLINE +
                 "  Author                   = {Foo Bar}," + OS.NEWLINE +
@@ -730,21 +730,21 @@ public void saveAlsoSavesSecondModification() throws Exception {
                 "@Article{test," + OS.NEWLINE +
                 "  author  = {Test}," + OS.NEWLINE +
                 "  journal = {International Journal of Something}," + OS.NEWLINE +
-                "  number  = {1}," + 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 saveReturnsToOriginalEntryWhenEntryIsFlaggedUnchanged() throws Exception {
+    void saveReturnsToOriginalEntryWhenEntryIsFlaggedUnchanged() 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 +
+                "  Note                     = {some note}," + OS.NEWLINE +
                 "}";
         // @formatter:on
 
@@ -770,7 +770,7 @@ public void saveReturnsToOriginalEntryWhenEntryIsFlaggedUnchanged() throws Excep
     }
 
     @Test
-    public void saveReturnsToOriginalEntryWhenEntryIsFlaggedUnchangedEvenInThePrecenseOfSavedModifications() throws Exception {
+    void saveReturnsToOriginalEntryWhenEntryIsFlaggedUnchangedEvenInThePrecenseOfSavedModifications() throws Exception {
         // @formatter:off
         String bibtexEntry = OS.NEWLINE + "@Article{test," + OS.NEWLINE +
                 "  Author                   = {Foo Bar}," + OS.NEWLINE +
diff --git a/src/test/resources/testbib/bibWithUserCommentAndEntryChange.bib b/src/test/resources/testbib/bibWithUserCommentAndEntryChange.bib
@@ -32,11 +32,11 @@ @Comment this
 
 @InProceedings{1137631,
   author     = {Mr. Author},
-  title      = {Extending XP practices to support security requirements engineering},
   booktitle  = {SESS '06: Proceedings of the 2006 international workshop on Software engineering for secure systems},
+  title      = {Extending XP practices to support security requirements engineering},
   year       = {2006},
-  pages      = {11--18},
   address    = {New York, NY, USA},
+  pages      = {11--18},
   publisher  = {ACM},
   bdsk-url-1 = {http://doi.acm.org/10.1145/1137627.1137631},
   doi        = {http://doi.acm.org/10.1145/1137627.1137631},
