Add tests for month issue and strings (and some minor code impr…

src/main/java/org/jabref/logic/shared/DBMSProcessor.java

@@ -473,7 +473,10 @@ public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {
                     lastId = selectEntryResultSet.getInt("SHARED_ID");
                 }
 
-                bibEntry.setField(FieldFactory.parseField(selectEntryResultSet.getString("NAME")), Optional.ofNullable(selectEntryResultSet.getString("VALUE")), EntryEventSource.SHARED);
+                String value = selectEntryResultSet.getString("VALUE");
+                if (value != null) {
+                    bibEntry.setField(FieldFactory.parseField(selectEntryResultSet.getString("NAME")), value, EntryEventSource.SHARED);
+                }
             }
         } catch (SQLException e) {
             LOGGER.error("SQL Error", e);

src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java

@@ -30,7 +30,6 @@
 import org.jabref.model.entry.event.EntryEvent;
 import org.jabref.model.entry.event.EntryEventSource;
 import org.jabref.model.entry.event.FieldChangedEvent;
-import org.jabref.model.entry.field.Field;
 import org.jabref.model.metadata.MetaData;
 import org.jabref.model.metadata.event.MetaDataChangedEvent;
 import org.jabref.model.util.FileUpdateMonitor;
@@ -41,8 +40,8 @@
 import org.slf4j.LoggerFactory;
 
 /**
- * Synchronizes the shared or local databases with their opposite side.
- * Local changes are pushed by {@link EntryEvent} using Google's Guava EventBus.
+ * Synchronizes the shared or local databases with their opposite side. Local changes are pushed by {@link EntryEvent}
+ * using Google's Guava EventBus.
  */
 public class DBMSSynchronizer implements DatabaseSynchronizer {
 
@@ -135,11 +134,10 @@ public void listen(MetaDataChangedEvent event) {
     }
 
     /**
-     * Sets the table structure of shared database if needed and pulls all shared entries
-     * to the new local database.
+     * Sets the table structure of shared database if needed and pulls all shared entries to the new local database.
      *
-     * @throws DatabaseNotSupportedException if the version of shared database does not match
-     *          the version of current shared database support ({@link DBMSProcessor}).
+     * @throws DatabaseNotSupportedException if the version of shared database does not match the version of current
+     *                                       shared database support ({@link DBMSProcessor}).
      */
     public void initializeDatabases() throws DatabaseNotSupportedException {
         try {
@@ -163,8 +161,8 @@ public void initializeDatabases() throws DatabaseNotSupportedException {
     }
 
     /**
-     * Synchronizes the local database with shared one.
-     * Possible update types are removal, update or insert of a {@link BibEntry}.
+     * Synchronizes the local database with shared one. Possible update types are removal, update or insert of a {@link
+     * BibEntry}.
      */
     @Override
     public void synchronizeLocalDatabase() {
@@ -191,17 +189,17 @@ public void synchronizeLocalDatabase() {
                             localEntry.setType(sharedEntry.get().getType(), EntryEventSource.SHARED);
                             localEntry.getSharedBibEntryData()
                                       .setVersion(sharedEntry.get().getSharedBibEntryData().getVersion());
-                            for (Field field : sharedEntry.get().getFields()) {
-                                localEntry.setField(field, sharedEntry.get().getField(field), EntryEventSource.SHARED);
-                            }
-
-                            Set<Field> redundantLocalEntryFields = localEntry.getFields();
-                            redundantLocalEntryFields.removeAll(sharedEntry.get().getFields());
-
-                            // remove not existing fields
-                            for (Field redundantField : redundantLocalEntryFields) {
-                                localEntry.clearField(redundantField, EntryEventSource.SHARED);
-                            }
+                            // copy remote values to local entry
+                            sharedEntry.get().getFieldMap().forEach(
+                                    (field, value) -> localEntry.setField(field, value, EntryEventSource.SHARED)
+                            );
+
+                            // locally remove not existing fields
+                            localEntry.getFields().stream()
+                                      .filter(field -> !sharedEntry.get().hasField(field))
+                                      .forEach(
+                                              field -> localEntry.clearField(field, EntryEventSource.SHARED)
+                                      );
                         }
                     }
                 }
@@ -220,7 +218,7 @@ public void synchronizeLocalDatabase() {
      * Removes all local entries which are not present on shared database.
      *
      * @param localEntries List of {@link BibEntry} the entries should be removed from
-     * @param sharedIDs Set of all IDs which are present on shared database
+     * @param sharedIDs    Set of all IDs which are present on shared database
      */
     private void removeNotSharedEntries(List<BibEntry> localEntries, Set<Integer> sharedIDs) {
         for (int i = 0; i < localEntries.size(); i++) {
@@ -325,8 +323,8 @@ public void pullChanges() {
     }
 
     /**
-     * Checks whether the current SQL connection is valid.
-     * In case that the connection is not valid a new {@link ConnectionLostEvent} is going to be sent.
+     * Checks whether the current SQL connection is valid. In case that the connection is not valid a new {@link
+     * ConnectionLostEvent} is going to be sent.
      *
      * @return <code>true</code> if the connection is valid, else <code>false</code>.
      */
@@ -337,7 +335,6 @@ public boolean checkCurrentConnection() {
                 eventBus.post(new ConnectionLostEvent(bibDatabaseContext));
             }
             return isValid;
-
         } catch (SQLException e) {
             LOGGER.error("SQL Error:", e);
             return false;
@@ -348,7 +345,8 @@ public boolean checkCurrentConnection() {
      * Checks whether the {@link EntryEventSource} of an {@link EntryEvent} is crucial for this class.
      *
      * @param event An {@link EntryEvent}
-     * @return <code>true</code> if the event is able to trigger operations in {@link DBMSSynchronizer}, else <code>false</code>
+     * @return <code>true</code> if the event is able to trigger operations in {@link DBMSSynchronizer}, else
+     * <code>false</code>
      */
     public boolean isEventSourceAccepted(EntryEvent event) {
         EntryEventSource eventSource = event.getEntryEventSource();

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

@@ -42,6 +42,11 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * Represents a BibTex / BibLaTeX entry.
+ *
+ * In case you search for a builder as described in Item 2 of the book "Effective Java", you won't find one. Please use the methods {@link #withCiteKey(String)} and {@link #withField(Field, String)}.
+ */
 public class BibEntry implements Cloneable {
 
     public static final EntryType DEFAULT_TYPE = StandardEntryType.Misc;
@@ -63,6 +68,7 @@ public class BibEntry implements Cloneable {
     private ObservableMap<Field, String> fields = FXCollections.observableMap(new ConcurrentHashMap<>());
     private String parsedSerialization = "";
     private String commentsBeforeEntry = "";
+
     /**
      * Marks whether the complete serialization, which was read from file, should be used.
      *
@@ -75,7 +81,6 @@ public class BibEntry implements Cloneable {
      */
     public BibEntry() {
         this(IdGenerator.next(), DEFAULT_TYPE);
-
     }
 
     /**
@@ -266,8 +271,9 @@ public String getId() {
     }
 
     /**
-     * Sets this entry's ID, provided the database containing it
-     * doesn't veto the change.
+     * Sets this entry's identifier (ID). It is used internally  to distinguish different BibTeX entries. It is <emph>not</emph> the BibTeX key. The BibTexKey is the {@link InternalField.KEY_FIELD}.
+     *
+     * The entry is also updated in the shared database - provided the database containing it doesn't veto the change.
      *
      * @param id The ID to be used
      */
@@ -523,13 +529,6 @@ public Optional<FieldChange> setField(Field field, String value, EntryEventSourc
         return Optional.of(change);
     }
 
-    public Optional<FieldChange> setField(Field field, Optional<String> value, EntryEventSource eventSource) {
-        if (value.isPresent()) {
-            return setField(field, value.get(), eventSource);
-        }
-        return Optional.empty();
-    }
-
     /**
      * Set a field, and notify listeners about the change.
      *

src/main/java/org/jabref/model/entry/field/InternalField.java

@@ -10,16 +10,17 @@
  * JabRef internal fields
  */
 public enum InternalField implements Field {
-    INTERNAL_ALL_FIELD("all"),
-    INTERNAL_ALL_TEXT_FIELDS_FIELD("all-text-fields"),
-    MARKED_INTERNAL("__markedentry"),
     OWNER("owner"),
     TIMESTAMP("timestamp", FieldProperty.DATE),
     GROUPS("groups"),
     KEY_FIELD("bibtexkey"),
     TYPE_HEADER("entrytype"),
     OBSOLETE_TYPE_HEADER("bibtextype"),
-    BIBTEX_STRING("__string"),
+    MARKED_INTERNAL("__markedentry"), // used in old versions of JabRef. Currently used for conversion only
+    // all field names starting with "Jabref-internal-" are not appearing in .bib files
+    BIBTEX_STRING("__string"), // marker that the content is just a BibTeX string
+    INTERNAL_ALL_FIELD("all"), // virtual field to denote "all fields". Used in the meta data serialiization for save actions.
+    INTERNAL_ALL_TEXT_FIELDS_FIELD("all-text-fields"), // virtual field to denote "all text fields". Used in the meta data serialiization for save actions.
     INTERNAL_ID_FIELD("JabRef-internal-id");
 
     private final String name;

src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java

@@ -108,30 +108,30 @@ void writeEntryWithFile() throws Exception {
 
     @Test
     void writeEntryWithOrField() throws Exception {
-            StringWriter stringWriter = new StringWriter();
-
-            BibEntry entry = new BibEntry(StandardEntryType.InBook);
-            //set an required OR field (author/editor)
-            entry.setField(StandardField.EDITOR, "Foo Bar");
-            entry.setField(StandardField.JOURNAL, "International Journal of Something");
-            //set an optional field
-            entry.setField(StandardField.NUMBER, "1");
-            entry.setField(StandardField.NOTE, "some note");
-
-            writer.write(entry, stringWriter, BibDatabaseMode.BIBTEX);
-
-            String actual = stringWriter.toString();
-
-            // @formatter:off
-            String expected = OS.NEWLINE + "@InBook{," + OS.NEWLINE +
-                    "  editor  = {Foo Bar}," + OS.NEWLINE +
-                    "  note    = {some note}," + OS.NEWLINE +
-                    "  number  = {1}," + OS.NEWLINE +
-                    "  journal = {International Journal of Something}," + OS.NEWLINE +
-                    "}" + OS.NEWLINE;
-            // @formatter:on
-
-            assertEquals(expected, actual);
+        StringWriter stringWriter = new StringWriter();
+
+        BibEntry entry = new BibEntry(StandardEntryType.InBook);
+        //set an required OR field (author/editor)
+        entry.setField(StandardField.EDITOR, "Foo Bar");
+        entry.setField(StandardField.JOURNAL, "International Journal of Something");
+        //set an optional field
+        entry.setField(StandardField.NUMBER, "1");
+        entry.setField(StandardField.NOTE, "some note");
+
+        writer.write(entry, stringWriter, BibDatabaseMode.BIBTEX);
+
+        String actual = stringWriter.toString();
+
+        // @formatter:off
+        String expected = OS.NEWLINE + "@InBook{," + OS.NEWLINE +
+                "  editor  = {Foo Bar}," + OS.NEWLINE +
+                "  note    = {some note}," + OS.NEWLINE +
+                "  number  = {1}," + OS.NEWLINE +
+                "  journal = {International Journal of Something}," + OS.NEWLINE +
+                "}" + OS.NEWLINE;
+        // @formatter:on
+
+        assertEquals(expected, actual);
     }
 
     @Test
@@ -426,6 +426,36 @@ void monthFieldSpecialSyntax() throws IOException {
         assertEquals(bibtexEntry, actual);
     }
 
+    @Test
+    void constantMonthApril() throws Exception {
+        BibEntry entry = new BibEntry(StandardEntryType.Misc)
+                .withField(StandardField.MONTH, "#apr#");
+
+        StringWriter stringWriter = new StringWriter();
+        writer.write(entry, stringWriter, BibDatabaseMode.BIBTEX);
+
+        assertEquals(OS.NEWLINE +
+                        "@Misc{," + OS.NEWLINE +
+                        "  month = apr," + OS.NEWLINE +
+                        "}" + OS.NEWLINE,
+                stringWriter.toString());
+    }
+
+    @Test
+    void monthApril() throws Exception {
+        BibEntry entry = new BibEntry(StandardEntryType.Misc)
+                .withField(StandardField.MONTH, "apr");
+
+        StringWriter stringWriter = new StringWriter();
+        writer.write(entry, stringWriter, BibDatabaseMode.BIBTEX);
+
+        assertEquals(OS.NEWLINE +
+                        "@Misc{," + OS.NEWLINE +
+                        "  month = {apr}," + OS.NEWLINE +
+                        "}" + OS.NEWLINE,
+                stringWriter.toString());
+    }
+
     @Test
     void addFieldWithLongerLength() throws IOException {
         // @formatter:off

src/test/java/org/jabref/logic/exporter/BibtexDatabaseWriterTest.java

@@ -289,7 +289,24 @@ void writeEntryWithCustomizedTypeAlsoWritesTypeDeclaration() throws Exception {
     }
 
     @Test
-    void roundtrip() throws Exception {
+    void roundtripWithArticleMonths() throws Exception {
+        Path testBibtexFile = Paths.get("src/test/resources/testbib/articleWithMonths.bib");
+        Charset encoding = StandardCharsets.UTF_8;
+        ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding));
+
+        when(preferences.getEncoding()).thenReturn(encoding);
+        when(preferences.isSaveInOriginalOrder()).thenReturn(true);
+        BibDatabaseContext context = new BibDatabaseContext(result.getDatabase(), result.getMetaData(),
+                new Defaults(BibDatabaseMode.BIBTEX));
+
+        databaseWriter.savePartOfDatabase(context, result.getDatabase().getEntries());
+        try (Scanner scanner = new Scanner(testBibtexFile, encoding.name())) {
+            assertEquals(scanner.useDelimiter("\\A").next(), stringWriter.toString());
+        }
+    }
+
+    @Test
+    void roundtripWithComplexBib() throws Exception {
         Path testBibtexFile = Paths.get("src/test/resources/testbib/complex.bib");
         Charset encoding = StandardCharsets.UTF_8;
         ParserResult result = new BibtexParser(importFormatPreferences, fileMonitor).parse(Importer.getReader(testBibtexFile, encoding));