diff --git a/CHANGELOG.md b/CHANGELOG.md index cfbb06fed9a..1ba8762b633 100644 --- 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 index 539ac974e61..ec364110a91 100644 --- 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 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 bibEntries) { // Coerce to ArrayList in order to use List.get() List> 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 bibEntries) { for (List 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 getSharedEntries(List 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 getSharedEntries(List 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 index 97732a6ca5d..abc597cb186 100644 --- 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 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 index 36534eb7da2..11fa69ad521 100644 --- 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(); }