Fix newly added entry not synced to db (#7178)

* Fix newly added entry not synced to db Newly added entries have empty fields; don't update the field table to prevent SQL Exception Fix shared entry not found by id use left outer join for this * fix checkstyle * fix wording * add tests for fix * adjust test
JabRef · Dec 14, 2020 · f356f9e · f356f9e
1 parent aa2b530
commit f356f9e
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -78,6 +78,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We fixed an issue where the "Document Viewer" did not show the first page of the opened pdf document and did not show the correct total number of pages [#7108](https://github.com/JabRef/jabref/issues/7108)
 - We fixed an issue where the context menu was not updated after a file link was changed. [#5777](https://github.com/JabRef/jabref/issues/5777)
 - We fixed an issue where the password for a shared SQL database was not remembered [#6869](https://github.com/JabRef/jabref/issues/6869)
+- We fixed an issue where newly added entires were not synced to a shared SQL database [#7176](https://github.com/JabRef/jabref/issues/7176)
 
 ### Removed
 

diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java
@@ -165,7 +165,7 @@ protected void insertIntoEntryTable(List<BibEntry> bibEntries) {
                 .append(escape("TYPE"))
                 .append(") VALUES(?)");
         // Number of commas is bibEntries.size() - 1
-        for (int i = 0; i < bibEntries.size() - 1; i++) {
+        for (int i = 0; i < (bibEntries.size() - 1); i++) {
             insertIntoEntryQuery.append(", (?)");
         }
 
@@ -239,6 +239,7 @@ protected void insertIntoFieldTable(List<BibEntry> bibEntries) {
             // Coerce to ArrayList in order to use List.get()
             List<List<Field>> fields = bibEntries.stream().map(bibEntry -> new ArrayList<>(bibEntry.getFields()))
                                                  .collect(Collectors.toList());
+
             StringBuilder insertFieldQuery = new StringBuilder()
                     .append("INSERT INTO ")
                     .append(escape("FIELD"))
@@ -253,8 +254,13 @@ protected void insertIntoFieldTable(List<BibEntry> bibEntries) {
             for (List<Field> entryFields : fields) {
                 numFields += entryFields.size();
             }
+
+            if (numFields == 0) {
+                return; // Prevent SQL Exception
+            }
+
             // Number of commas is fields.size() - 1
-            for (int i = 0; i < numFields - 1; i++) {
+            for (int i = 0; i < (numFields - 1); i++) {
                 insertFieldQuery.append(", (?, ?, ?)");
             }
             try (PreparedStatement preparedFieldStatement = connection.prepareStatement(insertFieldQuery.toString())) {
@@ -493,7 +499,8 @@ public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {
              .append("F.").append(escape("VALUE"))
              .append(" FROM ")
              .append(escape("ENTRY"))
-             .append(" inner join ")
+             // Handle special case if entry does not have any fields (yet)
+             .append(" left outer join ")
              .append(escape("FIELD"))
              .append(" F on ")
              .append(escape("ENTRY")).append(".").append(escape("SHARED_ID"))
@@ -508,41 +515,37 @@ public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {
         query.append(" order by ")
              .append(escape("SHARED_ID"));
 
-        PreparedStatement preparedStatement;
-        try {
-            preparedStatement = connection.prepareStatement(query.toString());
+        try (PreparedStatement preparedStatement = connection.prepareStatement(query.toString())) {
             for (int i = 0; i < sharedIDs.size(); i++) {
                 preparedStatement.setInt(i + 1, sharedIDs.get(i));
             }
-        } catch (SQLException e) {
-            LOGGER.debug("Executed >{}<", query.toString());
-            LOGGER.error("SQL Error", e);
-            return Collections.emptyList();
-        }
-        try (ResultSet selectEntryResultSet = preparedStatement.executeQuery()) {
-            BibEntry bibEntry = null;
-            int lastId = -1;
-            while (selectEntryResultSet.next()) {
-                // We get a list of field values of bib entries "grouped" by bib entries
-                // Thus, the first change in the shared id leads to a new BibEntry
-                if (selectEntryResultSet.getInt("SHARED_ID") > lastId) {
-                    bibEntry = new BibEntry();
-                    bibEntry.getSharedBibEntryData().setSharedID(selectEntryResultSet.getInt("SHARED_ID"));
-                    bibEntry.setType(EntryTypeFactory.parse(selectEntryResultSet.getString("TYPE")));
-                    bibEntry.getSharedBibEntryData().setVersion(selectEntryResultSet.getInt("VERSION"));
-                    sharedEntries.add(bibEntry);
-                    lastId = selectEntryResultSet.getInt("SHARED_ID");
-                }
 
-                // In all cases, we set the field value of the newly created BibEntry object
-                String value = selectEntryResultSet.getString("VALUE");
-                if (value != null) {
-                    bibEntry.setField(FieldFactory.parseField(selectEntryResultSet.getString("NAME")), value, EntriesEventSource.SHARED);
+            try (ResultSet selectEntryResultSet = preparedStatement.executeQuery()) {
+                BibEntry bibEntry = null;
+                int lastId = -1;
+                while (selectEntryResultSet.next()) {
+                    // We get a list of field values of bib entries "grouped" by bib entries
+                    // Thus, the first change in the shared id leads to a new BibEntry
+                    if (selectEntryResultSet.getInt("SHARED_ID") > lastId) {
+                        bibEntry = new BibEntry();
+                        bibEntry.getSharedBibEntryData().setSharedID(selectEntryResultSet.getInt("SHARED_ID"));
+                        bibEntry.setType(EntryTypeFactory.parse(selectEntryResultSet.getString("TYPE")));
+                        bibEntry.getSharedBibEntryData().setVersion(selectEntryResultSet.getInt("VERSION"));
+                        sharedEntries.add(bibEntry);
+                        lastId = selectEntryResultSet.getInt("SHARED_ID");
+                    }
+
+                    // In all cases, we set the field value of the newly created BibEntry object
+                    String value = selectEntryResultSet.getString("VALUE");
+                    if (value != null) {
+                        bibEntry.setField(FieldFactory.parseField(selectEntryResultSet.getString("NAME")), value, EntriesEventSource.SHARED);
+                    }
                 }
             }
         } catch (SQLException e) {
             LOGGER.error("Executed >{}<", query.toString());
             LOGGER.error("SQL Error", e);
+            return Collections.emptyList();
         }
 
         return sharedEntries;

diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java
@@ -95,6 +95,26 @@ void testInsertEntry() throws SQLException {
         assertEquals(expectedFieldMap, actualFieldMap);
     }
 
+    @Test
+    void testInsertEntryWithEmptyFields() throws SQLException {
+        BibEntry expectedEntry = new BibEntry(StandardEntryType.Article);
+
+        dbmsProcessor.insertEntry(expectedEntry);
+
+        try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) {
+            assertTrue(entryResultSet.next());
+            assertEquals(1, entryResultSet.getInt("SHARED_ID"));
+            assertEquals("article", entryResultSet.getString("TYPE"));
+            assertEquals(1, entryResultSet.getInt("VERSION"));
+            assertFalse(entryResultSet.next());
+
+            // Adding an empty entry should not create an entry in field table, only in entry table
+            try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) {
+                assertFalse(fieldResultSet.next());
+            }
+        }
+    }
+
     private static BibEntry getBibEntryExample() {
         return new BibEntry(StandardEntryType.InProceedings)
                 .withField(StandardField.AUTHOR, "Wirthlin, Michael J and Hutchings, Brad L and Gilson, Kent L")
@@ -119,6 +139,20 @@ void testUpdateEntry() throws Exception {
         assertEquals(Optional.of(expectedEntry), actualEntry);
     }
 
+    @Test
+    void testUpdateEmptyEntry() throws Exception {
+        BibEntry expectedEntry = new BibEntry(StandardEntryType.Article);
+        dbmsProcessor.insertEntry(expectedEntry);
+
+        expectedEntry.setField(StandardField.AUTHOR, "Michael J and Hutchings");
+        expectedEntry.setField(new UnknownField("customField"), "custom value");
+        // Update field should now find the entry
+        dbmsProcessor.updateEntry(expectedEntry);
+
+        Optional<BibEntry> actualEntry = dbmsProcessor.getSharedEntry(expectedEntry.getSharedBibEntryData().getSharedID());
+        assertEquals(Optional.of(expectedEntry), actualEntry);
+    }
+
     @Test
     void testGetEntriesByIdList() throws Exception {
         BibEntry firstEntry = getBibEntryExample();

diff --git a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java
@@ -1,6 +1,5 @@
 package org.jabref.logic.shared;
 
-import java.sql.SQLException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -69,7 +68,7 @@ public void setup() throws Exception {
     }
 
     @AfterEach
-    public void clear() throws SQLException {
+    public void clear() {
         dbmsSynchronizer.closeSharedDatabase();
     }