From 93196eeb819024079e94dd1eea3b22af993723cf Mon Sep 17 00:00:00 2001 From: Abraham Polk Date: Wed, 19 Feb 2020 18:57:05 -0500 Subject: [PATCH] [WIP] Initial work on DBMSProcessor batch entry insertion into ENTRY table (#5814) * Initial work on DBMSProcessor entry insertion into ENTRY table * Change syntax for Oracle multi-row insert SQL statement * Run tests also when source files changed * Add to comment about Oracle * Assume ResultSet is in order for setting shared IDs * Add insertEntry for DBMSProcessor tests and fix PostgresSQLProcessor * Fix SQL typo * Separate table drops in Oracle tests * Remove CI tests that were added in branch * Work on unit test for DBMSProcessor insertEntries * Fix bug in DBMSProcessorTest and simplify DBMSProcessor.FilterForBibEntryExistence * Remove Oracle connection bug with wrong port * Add Oracle insertIntoEntryTable * Oracle connection fix - taken from fix_fields_sql branch * Fix typo bug * Clean up code * Remove commented blocks * Remove comment about needing a test that probably isn't necessary * Manually merge fix_fields_sql OracleProcessor (just add method) * Emphasize todo * setSharedID into OracleProcessor entry table method * Add shared id to preparedEntryStatement * Make Oracle insertIntoEntryTable iterative - pasted from master - not yet tested * Add fields to fields table in parallel * Reset test trace length * Fix checkStyle * Revert port setting Co-authored-by: Tobias Diez --- .../jabref/logic/shared/DBMSProcessor.java | 141 +++++++++++------- .../jabref/logic/shared/DBMSSynchronizer.java | 5 +- .../jabref/logic/shared/OracleProcessor.java | 59 ++++++-- .../logic/shared/PostgreSQLProcessor.java | 35 +++-- .../logic/shared/DBMSProcessorTest.java | 36 +++-- .../jabref/logic/shared/TestConnector.java | 2 +- 6 files changed, 182 insertions(+), 96 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 0e6d9550e5c..9ef40561391 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -16,12 +16,14 @@ import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import org.jabref.logic.shared.exception.OfflineLockException; import org.jabref.model.database.shared.DBMSType; import org.jabref.model.database.shared.DatabaseConnection; import org.jabref.model.database.shared.DatabaseConnectionProperties; import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.SharedBibEntryData; import org.jabref.model.entry.event.EntriesEventSource; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; @@ -131,40 +133,61 @@ public void setupSharedDatabase() throws SQLException { abstract String escape(String expression); /** - * Inserts the given bibEntry into shared database. + * For use in test only. Inserts the BibEntry into the shared database. * - * @param bibEntry {@link BibEntry} to be inserted + * @param bibEntry {@link BibEntry} to be inserted. + * */ + public void insertEntry(BibEntry bibEntry) { + insertEntries(Collections.singletonList(bibEntry)); + } + + /** + * Inserts the List of BibEntry into the shared database. + * + * @param bibEntries List of {@link BibEntry} to be inserted */ - public void insertEntry(BibEntry bibEntry) { - if (!checkForBibEntryExistence(bibEntry)) { - insertIntoEntryTable(bibEntry); - insertIntoFieldTable(bibEntry); + public void insertEntries(List bibEntries) { + List notYetExistingEntries = getNotYetExistingEntries(bibEntries); + if (notYetExistingEntries.isEmpty()) { + return; } + insertIntoEntryTable(notYetExistingEntries); + insertIntoFieldTable(notYetExistingEntries); } /** - * Inserts the given bibEntry into ENTRY table. + * Inserts the given List of BibEntry into the ENTRY table. * - * @param bibEntry {@link BibEntry} to be inserted + * @param bibEntries List of {@link BibEntry} to be inserted */ - protected void insertIntoEntryTable(BibEntry bibEntry) { - // This is the only method to get generated keys which is accepted by MySQL, PostgreSQL and Oracle. - String insertIntoEntryQuery = - "INSERT INTO " + - escape("ENTRY") + - "(" + - escape("TYPE") + - ") VALUES(?)"; - - try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery, - new String[]{"SHARED_ID"})) { + protected void insertIntoEntryTable(List bibEntries) { + StringBuilder insertIntoEntryQuery = new StringBuilder() + .append("INSERT INTO ") + .append(escape("ENTRY")) + .append("(") + .append(escape("TYPE")) + .append(") VALUES(?)"); + // Number of commas is bibEntries.size() - 1 + for (int i = 0; i < bibEntries.size() - 1; i++) { + insertIntoEntryQuery.append(", (?)"); + } - preparedEntryStatement.setString(1, bibEntry.getType().getName()); + try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(), + new String[]{"SHARED_ID"})) { + for (int i = 0; i < bibEntries.size(); i++) { + preparedEntryStatement.setString(i + 1, bibEntries.get(i).getType().getName()); + } preparedEntryStatement.executeUpdate(); try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) { + // The following assumes that we get the generated keys in the order the entries were inserted + // This should be the case + for (BibEntry bibEntry : bibEntries) { + generatedKeys.next(); + bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); + } if (generatedKeys.next()) { - bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); // set generated ID locally + LOGGER.error("Error: Some shared IDs left unassigned"); } } } catch (SQLException e) { @@ -173,48 +196,52 @@ protected void insertIntoEntryTable(BibEntry bibEntry) { } /** - * Checks whether the given bibEntry already exists on shared database. + * Filters a list of BibEntry to and returns those which do not exist in the database * - * @param bibEntry {@link BibEntry} to be checked + * @param bibEntries {@link BibEntry} to be checked * @return true if existent, else false */ - private boolean checkForBibEntryExistence(BibEntry bibEntry) { + private List getNotYetExistingEntries(List bibEntries) { + + List remoteIds = new ArrayList<>(); + List localIds = bibEntries.stream() + .map(BibEntry::getSharedBibEntryData) + .map(SharedBibEntryData::getSharedID) + .filter((id) -> id != -1) + .collect(Collectors.toList()); + if (localIds.isEmpty()) { + return bibEntries; + } try { - // Check if already exists - int sharedID = bibEntry.getSharedBibEntryData().getSharedID(); - if (sharedID != -1) { - String selectQuery = - "SELECT * FROM " + - escape("ENTRY") + - " WHERE " + - escape("SHARED_ID") + - " = ?"; - - try (PreparedStatement preparedSelectStatement = connection.prepareStatement(selectQuery)) { - preparedSelectStatement.setInt(1, sharedID); - try (ResultSet resultSet = preparedSelectStatement.executeQuery()) { - if (resultSet.next()) { - return true; - } - } + StringBuilder selectQuery = new StringBuilder() + .append("SELECT * FROM ") + .append(escape("ENTRY")); + + try (ResultSet resultSet = connection.createStatement().executeQuery(selectQuery.toString())) { + while (resultSet.next()) { + int id = resultSet.getInt("SHARED_ID"); + remoteIds.add(id); } } } catch (SQLException e) { LOGGER.error("SQL Error: ", e); } - return false; - } + return bibEntries.stream().filter((entry) -> + !remoteIds.contains(entry.getSharedBibEntryData().getSharedID())) + .collect(Collectors.toList()); + } /** - * Inserts the given bibEntry into FIELD table. + * Inserts the given list of BibEntry into FIELD table. * - * @param bibEntry {@link BibEntry} to be inserted + * @param bibEntries {@link BibEntry} to be inserted */ - protected void insertIntoFieldTable(BibEntry bibEntry) { + protected void insertIntoFieldTable(List bibEntries) { try { // Inserting into FIELD table // Coerce to ArrayList in order to use List.get() - List fields = new ArrayList<>(bibEntry.getFields()); + List> fields = bibEntries.stream().map(bibEntry -> new ArrayList<>(bibEntry.getFields())) + .collect(Collectors.toList()); StringBuilder insertFieldQuery = new StringBuilder() .append("INSERT INTO ") .append(escape("FIELD")) @@ -225,16 +252,24 @@ protected void insertIntoFieldTable(BibEntry bibEntry) { .append(", ") .append(escape("VALUE")) .append(") VALUES(?, ?, ?)"); + int numFields = 0; + for (List entryFields : fields) { + numFields += entryFields.size(); + } // Number of commas is fields.size() - 1 - for (int i = 0; i < fields.size() - 1; i++) { + for (int i = 0; i < numFields - 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((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()); + int fieldsCompleted = 0; + for (int entryIndex = 0; entryIndex < fields.size(); entryIndex++) { + for (int entryFieldsIndex = 0; entryFieldsIndex < fields.get(entryIndex).size(); entryFieldsIndex++) { + // columnIndex starts with 1 + preparedFieldStatement.setInt((3 * fieldsCompleted) + 1, bibEntries.get(entryIndex).getSharedBibEntryData().getSharedID()); + preparedFieldStatement.setString((3 * fieldsCompleted) + 2, fields.get(entryIndex).get(entryFieldsIndex).getName()); + preparedFieldStatement.setString((3 * fieldsCompleted) + 3, bibEntries.get(entryIndex).getField(fields.get(entryIndex).get(entryFieldsIndex)).get()); + fieldsCompleted += 1; + } } preparedFieldStatement.executeUpdate(); } diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 72b9fc069f2..6499d78a179 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -82,12 +82,9 @@ public void listen(EntriesAddedEvent event) { if (isEventSourceAccepted(event) && checkCurrentConnection()) { synchronizeLocalMetaData(); synchronizeLocalDatabase(); // Pull changes for the case that there were some - List entries = event.getBibEntries(); - for (BibEntry entry : entries) { - dbmsProcessor.insertEntry(entry); + dbmsProcessor.insertEntries(event.getBibEntries()); } } - } /** * Listening method. Updates an existing shared {@link BibEntry}. diff --git a/src/main/java/org/jabref/logic/shared/OracleProcessor.java b/src/main/java/org/jabref/logic/shared/OracleProcessor.java index 6e4b820bcf4..f4fd183a1d3 100644 --- a/src/main/java/org/jabref/logic/shared/OracleProcessor.java +++ b/src/main/java/org/jabref/logic/shared/OracleProcessor.java @@ -1,11 +1,13 @@ package org.jabref.logic.shared; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; import java.util.List; import java.util.Properties; +import java.util.stream.Collectors; import org.jabref.logic.shared.listener.OracleNotificationListener; import org.jabref.model.database.shared.DatabaseConnection; @@ -103,14 +105,48 @@ public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) { } @Override - protected void insertIntoFieldTable(BibEntry bibEntry) { + protected void insertIntoEntryTable(List entries) { + try { + for (BibEntry entry : entries) { + String insertIntoEntryQuery = + "INSERT INTO " + + escape("ENTRY") + + "(" + + escape("TYPE") + + ") VALUES(?)"; + + try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery, + new String[]{"SHARED_ID"})) { + + preparedEntryStatement.setString(1, entry.getType().getName()); + preparedEntryStatement.executeUpdate(); + + try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) { + if (generatedKeys.next()) { + entry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); // set generated ID locally + } + } + } + } + } catch (SQLException e) { + LOGGER.error("SQL Error: ", e); + } + } + + @Override + protected void insertIntoFieldTable(List bibEntries) { try { // Inserting into FIELD table // Coerce to ArrayList in order to use List.get() - List fields = new ArrayList<>(bibEntry.getFields()); + List> fields = bibEntries.stream().map(entry -> new ArrayList<>(entry.getFields())) + .collect(Collectors.toList()); StringBuilder insertFieldQuery = new StringBuilder() .append("INSERT ALL"); - for (Field field : fields) { + int numFields = 0; + for (List entryFields : fields) { + numFields += entryFields.size(); + } + for (int i = 0; i < numFields; i++) { insertFieldQuery.append(" INTO ") .append(escape("FIELD")) .append(" (") @@ -123,14 +159,17 @@ protected void insertIntoFieldTable(BibEntry bibEntry) { } insertFieldQuery.append(" SELECT * FROM DUAL"); try (PreparedStatement preparedFieldStatement = connection.prepareStatement(insertFieldQuery.toString())) { - for (int i = 0; i < fields.size(); i++) { - // columnIndex starts with 1 - 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()); + int fieldsCompleted = 0; + for (int entryIndex = 0; entryIndex < fields.size(); entryIndex++) { + for (int entryFieldsIndex = 0; entryFieldsIndex < fields.get(entryIndex).size(); entryFieldsIndex++) { + // columnIndex starts with 1 + preparedFieldStatement.setInt((3 * fieldsCompleted) + 1, bibEntries.get(entryIndex).getSharedBibEntryData().getSharedID()); + preparedFieldStatement.setString((3 * fieldsCompleted) + 2, fields.get(entryIndex).get(entryFieldsIndex).getName()); + preparedFieldStatement.setString((3 * fieldsCompleted) + 3, bibEntries.get(entryIndex).getField(fields.get(entryIndex).get(entryFieldsIndex)).get()); + fieldsCompleted += 1; + } } - preparedFieldStatement.executeUpdate(); - } + preparedFieldStatement.executeUpdate(); } } catch (SQLException e) { LOGGER.error("SQL Error: ", e); } diff --git a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java index 26cb1adfd01..ad53d54fd3b 100644 --- a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java +++ b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java @@ -4,6 +4,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.util.List; import org.jabref.JabRefExecutorService; import org.jabref.logic.shared.listener.PostgresSQLNotificationListener; @@ -49,25 +50,33 @@ public void setUp() throws SQLException { } @Override - protected void insertIntoEntryTable(BibEntry bibEntry) { - // Inserting into ENTRY table + protected void insertIntoEntryTable(List bibEntries) { StringBuilder insertIntoEntryQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape("ENTRY")) - .append("(") - .append(escape("TYPE")) - .append(") VALUES(?)"); - - // This is the only method to get generated keys which is accepted by MySQL, PostgreSQL and Oracle. + .append("INSERT INTO ") + .append(escape("ENTRY")) + .append("(") + .append(escape("TYPE")) + .append(") VALUES(?)"); + // Number of commas is bibEntries.size() - 1 + for (int i = 0; i < bibEntries.size() - 1; i++) { + insertIntoEntryQuery.append(", (?)"); + } try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(), - Statement.RETURN_GENERATED_KEYS)) { - - preparedEntryStatement.setString(1, bibEntry.getType().getName()); + Statement.RETURN_GENERATED_KEYS)) { + for (int i = 0; i < bibEntries.size(); i++) { + preparedEntryStatement.setString(i + 1, bibEntries.get(i).getType().getName()); + } preparedEntryStatement.executeUpdate(); try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) { + // The following assumes that we get the generated keys in the order the entries were inserted + // This should be the case + for (BibEntry bibEntry : bibEntries) { + generatedKeys.next(); + bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); + } if (generatedKeys.next()) { - bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); // set generated ID locally + LOGGER.error("Error: Some shared IDs left unassigned"); } } } catch (SQLException e) { diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 754e601e0b2..fe7c59b4263 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -2,6 +2,7 @@ import java.sql.ResultSet; import java.sql.SQLException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -70,7 +71,7 @@ void testInsertEntry() throws SQLException { dbmsProcessor.insertEntry(expectedEntry); - BibEntry emptyEntry = new BibEntry(); + BibEntry emptyEntry = getBibEntryExample(); emptyEntry.getSharedBibEntryData().setSharedID(1); dbmsProcessor.insertEntry(emptyEntry); // does not insert, due to same sharedID. @@ -97,31 +98,37 @@ void testInsertEntry() throws SQLException { @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 - dbmsProcessor.insertEntry(firstEntry); - dbmsProcessor.insertEntry(secondEntry); - dbmsProcessor.insertEntry(thirdEntry); + List entries = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + entries.add(new BibEntry(StandardEntryType.Article).withField(StandardField.JOURNAL, "journal " + i) + .withField(StandardField.ISSUE, Integer.toString(i))); + } + entries.get(3).setType(StandardEntryType.Thesis); + dbmsProcessor.insertEntries(entries); Map> 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("article", entryResultSet.getString("TYPE")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(2, entryResultSet.getInt("SHARED_ID")); - assertEquals("inproceedings", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("TYPE")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(3, entryResultSet.getInt("SHARED_ID")); + assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals(1, entryResultSet.getInt("VERSION")); + assertTrue(entryResultSet.next()); + assertEquals(4, entryResultSet.getInt("SHARED_ID")); + assertEquals("thesis", entryResultSet.getString("TYPE")); + assertEquals(1, entryResultSet.getInt("VERSION")); + assertTrue(entryResultSet.next()); + assertEquals(5, entryResultSet.getInt("SHARED_ID")); + assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals(1, entryResultSet.getInt("VERSION")); assertFalse(entryResultSet.next()); try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) { @@ -139,7 +146,6 @@ void testInsertMultipleEntries() throws SQLException { } } } - List entries = Arrays.asList(firstEntry, secondEntry, thirdEntry); Map> expectedFieldMap = entries.stream() .collect(Collectors.toMap(bibEntry -> bibEntry.getSharedBibEntryData().getSharedID(), (bibEntry) -> bibEntry.getFieldMap().entrySet().stream() diff --git a/src/test/java/org/jabref/logic/shared/TestConnector.java b/src/test/java/org/jabref/logic/shared/TestConnector.java index 73d7d9b9a8a..f48bb98e422 100644 --- a/src/test/java/org/jabref/logic/shared/TestConnector.java +++ b/src/test/java/org/jabref/logic/shared/TestConnector.java @@ -24,7 +24,7 @@ public static DBMSConnectionProperties getTestConnectionProperties(DBMSType dbms case POSTGRESQL: return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("localhost").setPort(dbmsType.getDefaultPort()).setDatabase("postgres").setUser("postgres").setPassword("postgres").setUseSSL(false).createDBMSConnectionProperties(); case ORACLE: - return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("localhost").setPort(dbmsType.getDefaultPort()).setDatabase("jabref").setUser("jabref").setPassword("jabref").setUseSSL(false).setPort(32118).createDBMSConnectionProperties(); + return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("localhost").setPort(32118).setDatabase("jabref").setUser("jabref").setPassword("jabref").setUseSSL(false).createDBMSConnectionProperties(); default: return new DBMSConnectionPropertiesBuilder().createDBMSConnectionProperties(); }