Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch DBMSProcessor fields insertion #5812

Merged
merged 4 commits into from
Jan 7, 2020
Merged

Batch DBMSProcessor fields insertion #5812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1348c2f
Initial changes to DBMSProcessor for insert entry functionality
abepolk Jan 2, 2020
7e4e76f
Added unit test for inserting multiple entries, with multiple of BibE…
abepolk Jan 2, 2020
8cf40b4
Remove StandardOut
abepolk Jan 2, 2020
931ccad
Merge branch 'master' into batch_DBMSProcessor_fields
abepolk Jan 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions src/main/java/org/jabref/logic/shared/DBMSProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,25 +213,30 @@ private boolean checkForBibEntryExistence(BibEntry bibEntry) {
private void insertIntoFieldTable(BibEntry bibEntry) {
try {
// Inserting into FIELD table
for (Field field : bibEntry.getFields()) {
StringBuilder insertFieldQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape("FIELD"))
.append("(")
.append(escape("ENTRY_SHARED_ID"))
.append(", ")
.append(escape("NAME"))
.append(", ")
.append(escape("VALUE"))
.append(") VALUES(?, ?, ?)");

try (PreparedStatement preparedFieldStatement = connection.prepareStatement(insertFieldQuery.toString())) {
// Coerce to ArrayList in order to use List.get()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, this should be "Convert" instead of "Coerce"?

List<Field> fields = new ArrayList<>(bibEntry.getFields());
StringBuilder insertFieldQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape("FIELD"))
.append("(")
.append(escape("ENTRY_SHARED_ID"))
.append(", ")
.append(escape("NAME"))
.append(", ")
.append(escape("VALUE"))
.append(") VALUES(?, ?, ?)");
// Number of commas is fields.size() - 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use the repeat() thing there, too?

for (int i = 0; i < fields.size() - 1; i++) {
insertFieldQuery.append(", (?, ?, ?)");
}
try (PreparedStatement preparedFieldStatement = connection.prepareStatement(insertFieldQuery.toString())) {
for (int i = 0; i < fields.size(); i++) {
// columnIndex starts with 1
preparedFieldStatement.setInt(1, bibEntry.getSharedBibEntryData().getSharedID());
preparedFieldStatement.setString(2, field.getName());
preparedFieldStatement.setString(3, bibEntry.getField(field).get());
preparedFieldStatement.executeUpdate();
preparedFieldStatement.setInt((3 * i) + 1, bibEntry.getSharedBibEntryData().getSharedID());
preparedFieldStatement.setString((3 * i) + 2, fields.get(i).getName());
preparedFieldStatement.setString((3 * i) + 3, bibEntry.getField(fields.get(i)).get());
}
preparedFieldStatement.executeUpdate();
}
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
Expand Down
77 changes: 74 additions & 3 deletions src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,59 @@ void testInsertEntry() throws SQLException {
assertEquals(expectedFieldMap, actualFieldMap);
}

@Test
void testInsertMultipleEntries() throws SQLException {
BibEntry firstEntry = getBibEntryExample();
String firstId = firstEntry.getId();
BibEntry secondEntry = getBibEntryExample2();
String secondId = secondEntry.getId();
BibEntry thirdEntry = getBibEntryExample3();
String thirdId = thirdEntry.getId();

// This must eventually be changed to insertEntries() once that method is implemented
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"eventually" is a false friend. Please translate it to "possibly" or "maybe"

dbmsProcessor.insertEntry(firstEntry);
dbmsProcessor.insertEntry(secondEntry);
dbmsProcessor.insertEntry(thirdEntry);

Map<Integer, Map<String, String>> actualFieldMap = new HashMap<>();

try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) {
assertTrue(entryResultSet.next());
assertEquals(1, entryResultSet.getInt("SHARED_ID"));
assertEquals("inproceedings", entryResultSet.getString("TYPE"));
assertEquals(1, entryResultSet.getInt("VERSION"));
assertTrue(entryResultSet.next());
assertEquals(2, entryResultSet.getInt("SHARED_ID"));
assertEquals("inproceedings", entryResultSet.getString("TYPE"));
assertEquals(1, entryResultSet.getInt("VERSION"));
assertTrue(entryResultSet.next());
assertEquals(3, entryResultSet.getInt("SHARED_ID"));
assertFalse(entryResultSet.next());

try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) {
while (fieldResultSet.next()) {
if (actualFieldMap.keySet().contains(fieldResultSet.getInt("ENTRY_SHARED_ID"))) {
actualFieldMap.get(fieldResultSet.getInt("ENTRY_SHARED_ID")).put(
fieldResultSet.getString("NAME"), fieldResultSet.getString("VALUE"));
} else {
int sharedId = fieldResultSet.getInt("ENTRY_SHARED_ID");
actualFieldMap.put(sharedId,
new HashMap<>());
actualFieldMap.get(sharedId).put(fieldResultSet.getString("NAME"),
fieldResultSet.getString("VALUE"));
}
}
}
}
List<BibEntry> entries = Arrays.asList(firstEntry, secondEntry, thirdEntry);
Map<Integer, Map<String, String>> expectedFieldMap = entries.stream()
.collect(Collectors.toMap(bibEntry -> bibEntry.getSharedBibEntryData().getSharedID(),
(bibEntry) -> bibEntry.getFieldMap().entrySet().stream()
.collect(Collectors.toMap((entry) -> entry.getKey().getName(), Map.Entry::getValue))));

assertEquals(expectedFieldMap, actualFieldMap);
}

@Test
void testUpdateEntry() throws Exception {
BibEntry expectedEntry = getBibEntryExample();
Expand Down Expand Up @@ -156,7 +209,7 @@ void testUpdateEqualEntry() throws OfflineLockException, SQLException {
@Test
void testRemoveAllEntries() throws SQLException {
BibEntry firstEntry = getBibEntryExample();
BibEntry secondEntry = getBibEntryExample();
BibEntry secondEntry = getBibEntryExample2();
List<BibEntry> entriesToRemove = Arrays.asList(firstEntry, secondEntry);
dbmsProcessor.insertEntry(firstEntry);
dbmsProcessor.insertEntry(secondEntry);
Expand All @@ -170,8 +223,8 @@ void testRemoveAllEntries() throws SQLException {
@Test
void testRemoveSomeEntries() throws SQLException {
BibEntry firstEntry = getBibEntryExample();
BibEntry secondEntry = getBibEntryExample();
BibEntry thirdEntry = getBibEntryExample();
BibEntry secondEntry = getBibEntryExample2();
BibEntry thirdEntry = getBibEntryExample3();

// Remove the first and third entries - the second should remain (SHARED_ID will be 2)

Expand Down Expand Up @@ -311,6 +364,24 @@ private static BibEntry getBibEntryExample() {
.withCiteKey("nanoproc1994");
}

private static BibEntry getBibEntryExample2() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please give the method a more meaningful name. For instance getShelahBibEntry.

return new BibEntry(StandardEntryType.InProceedings)
.withField(StandardField.AUTHOR, "Shelah, Saharon and Ziegler, Martin")
.withField(StandardField.TITLE, "Algebraically closed groups of large cardinality")
.withField(StandardField.JOURNAL, "The Journal of Symbolic Logic")
.withField(StandardField.YEAR, "1979")
.withCiteKey("algegrou1979");
}

private static BibEntry getBibEntryExample3() {
return new BibEntry(StandardEntryType.InProceedings)
.withField(StandardField.AUTHOR, "Hodges, Wilfrid and Shelah, Saharon")
.withField(StandardField.TITLE, "Infinite games and reduced products")
.withField(StandardField.JOURNAL, "Annals of Mathematical Logic")
.withField(StandardField.YEAR, "1981")
.withCiteKey("infigame1981");
}

private ResultSet selectFrom(String table, DBMSConnection dbmsConnection, DBMSProcessor dbmsProcessor) {
try {
return dbmsConnection.getConnection().createStatement().executeQuery("SELECT * FROM " + escape(table, dbmsProcessor));
Expand Down