From b906e69449d70857d8a46b2139032f47fa2a06b3 Mon Sep 17 00:00:00 2001 From: Ali Date: Tue, 12 Mar 2019 00:35:49 +0300 Subject: [PATCH] Optimize data fetching (#4520) * optimize data fetching from database * delete time measure code * revert code style * fix checkstyle * put columns name instead of * * revert checkstyle * change for to stream * refactor variable name * create simple test * fix test * rename method name * refactor * create test * Fix formatting * Remove obsolete method by slight refactoring Co-authored-by: Tobias Diez --- .../jabref/logic/shared/DBMSProcessor.java | 131 +++++++++--------- .../jabref/logic/shared/DBMSSynchronizer.java | 20 +-- .../logic/shared/DBMSProcessorTest.java | 18 +++ 3 files changed, 94 insertions(+), 75 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 9ebf92f0d33..59d3deed123 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -6,6 +6,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -14,6 +15,7 @@ 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; @@ -52,7 +54,7 @@ protected DBMSProcessor(DatabaseConnection dbmsConnection) { * @throws SQLException */ public boolean checkBaseIntegrity() throws SQLException { - return checkTableAvailibility("ENTRY", "FIELD", "METADATA"); + return checkTableAvailability("ENTRY", "FIELD", "METADATA"); } /** @@ -60,8 +62,8 @@ public boolean checkBaseIntegrity() throws SQLException { * * @return true if the structure is old, else false. */ - public boolean checkForPre3Dot6Intergrity() throws SQLException { - return checkTableAvailibility( + public boolean checkForPare3Dot6Integrity() throws SQLException { + return checkTableAvailability( "ENTRIES", "ENTRY_GROUP", "ENTRY_TYPES", @@ -77,7 +79,7 @@ public boolean checkForPre3Dot6Intergrity() throws SQLException { * @param tableNames Table names to be checked * @return true if all given tables are present, else false. */ - private boolean checkTableAvailibility(String... tableNames) throws SQLException { + private boolean checkTableAvailability(String... tableNames) throws SQLException { List requiredTables = new ArrayList<>(); for (String name : tableNames) { requiredTables.add(name.toUpperCase(Locale.ENGLISH)); @@ -147,16 +149,15 @@ public void insertEntry(BibEntry bibEntry) { * @param bibEntry {@link BibEntry} to be inserted */ protected void insertIntoEntryTable(BibEntry bibEntry) { - // Inserting into ENTRY table - 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. - try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(), + String insertIntoEntryQuery = + "INSERT INTO " + + escape("ENTRY") + + "(" + + escape("TYPE") + + ") VALUES(?)"; + + try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery, new String[] {"SHARED_ID"})) { preparedEntryStatement.setString(1, bibEntry.getType()); @@ -182,14 +183,14 @@ private boolean checkForBibEntryExistence(BibEntry bibEntry) { // Check if already exists int sharedID = bibEntry.getSharedBibEntryData().getSharedID(); if (sharedID != -1) { - StringBuilder selectQuery = new StringBuilder() - .append("SELECT * FROM ") - .append(escape("ENTRY")) - .append(" WHERE ") - .append(escape("SHARED_ID")) - .append(" = ?"); - - try (PreparedStatement preparedSelectStatement = connection.prepareStatement(selectQuery.toString())) { + 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()) { @@ -420,63 +421,57 @@ public void removeEntry(BibEntry bibEntry) { * @return instance of {@link BibEntry} */ public Optional getSharedEntry(int sharedID) { - List sharedEntries = getSharedEntryList(sharedID); + List sharedEntries = getSharedEntries(Collections.singletonList(sharedID)); if (!sharedEntries.isEmpty()) { return Optional.of(sharedEntries.get(0)); } return Optional.empty(); } - public List getSharedEntries() { - return getSharedEntryList(0); - } - - /** - * @param sharedID Entry ID. If 0, all entries are going to be fetched. - * @return List of {@link BibEntry} instances - */ - private List getSharedEntryList(int sharedID) { + public List getSharedEntries(List sharedIDs) { List sharedEntries = new ArrayList<>(); - StringBuilder selectEntryQuery = new StringBuilder(); - selectEntryQuery.append("SELECT * FROM "); - selectEntryQuery.append(escape("ENTRY")); - - if (sharedID != 0) { - selectEntryQuery.append(" WHERE "); - selectEntryQuery.append(escape("SHARED_ID")); - selectEntryQuery.append(" = "); - selectEntryQuery.append(sharedID); + StringBuilder query = new StringBuilder(); + query.append("SELECT ") + .append(escape("ENTRY")).append(".").append(escape("SHARED_ID")).append(", ") + .append(escape("ENTRY")).append(".").append(escape("TYPE")).append(", ") + .append(escape("ENTRY")).append(".").append(escape("VERSION")).append(", ") + .append("F.").append(escape("ENTRY_SHARED_ID")).append(", ") + .append("F.").append(escape("NAME")).append(", ") + .append("F.").append(escape("VALUE")) + .append(" FROM ") + .append(escape("ENTRY")) + .append(" inner join ") + .append(escape("FIELD")) + .append(" F on ") + .append(escape("ENTRY")).append(".").append(escape("SHARED_ID")) + .append(" = F.").append(escape("ENTRY_SHARED_ID")); + + if (!sharedIDs.isEmpty()) { + String idListAsString = sharedIDs.stream().map(String::valueOf).collect(Collectors.joining(", ")); + query.append(" where ") + .append(escape("SHARED_ID")).append(" in (") + .append(idListAsString) + .append(")"); } + query.append(" order by ") + .append(escape("SHARED_ID")) + .append(";"); - selectEntryQuery.append(" ORDER BY "); - selectEntryQuery.append(escape("SHARED_ID")); - - try (ResultSet selectEntryResultSet = connection.createStatement().executeQuery(selectEntryQuery.toString())) { + try (ResultSet selectEntryResultSet = connection.createStatement().executeQuery(query.toString())) { + BibEntry bibEntry = null; + int lastId = -1; while (selectEntryResultSet.next()) { - BibEntry bibEntry = new BibEntry(); - // setting the base attributes once - bibEntry.getSharedBibEntryData().setSharedID(selectEntryResultSet.getInt("SHARED_ID")); - bibEntry.setType(selectEntryResultSet.getString("TYPE")); - bibEntry.getSharedBibEntryData().setVersion(selectEntryResultSet.getInt("VERSION")); - - StringBuilder selectFieldQuery = new StringBuilder() - .append("SELECT * FROM ") - .append(escape("FIELD")) - .append(" WHERE ") - .append(escape("ENTRY_SHARED_ID")) - .append(" = ?"); - - try (PreparedStatement preparedSelectFieldStatement = connection.prepareStatement(selectFieldQuery.toString())) { - preparedSelectFieldStatement.setInt(1, selectEntryResultSet.getInt("SHARED_ID")); - try (ResultSet selectFieldResultSet = preparedSelectFieldStatement.executeQuery()) { - while (selectFieldResultSet.next()) { - bibEntry.setField(selectFieldResultSet.getString("NAME"), - Optional.ofNullable(selectFieldResultSet.getString("VALUE")), EntryEventSource.SHARED); - } - } + if (selectEntryResultSet.getInt("SHARED_ID") > lastId) { + bibEntry = new BibEntry(); + bibEntry.getSharedBibEntryData().setSharedID(selectEntryResultSet.getInt("SHARED_ID")); + bibEntry.setType(selectEntryResultSet.getString("TYPE")); + bibEntry.getSharedBibEntryData().setVersion(selectEntryResultSet.getInt("VERSION")); + sharedEntries.add(bibEntry); + lastId = selectEntryResultSet.getInt("SHARED_ID"); } - sharedEntries.add(bibEntry); + + bibEntry.setField(selectEntryResultSet.getString("NAME"), Optional.ofNullable(selectEntryResultSet.getString("VALUE")), EntryEventSource.SHARED); } } catch (SQLException e) { LOGGER.error("SQL Error", e); @@ -485,6 +480,10 @@ private List getSharedEntryList(int sharedID) { return sharedEntries; } + public List getSharedEntries() { + return getSharedEntries(Collections.emptyList()); + } + /** * Retrieves a mapping between the columns SHARED_ID and VERSION. */ diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 64ede9203ba..94edfe1c93d 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -2,6 +2,7 @@ import java.sql.Connection; import java.sql.SQLException; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Objects; @@ -147,7 +148,7 @@ public void initializeDatabases() throws DatabaseNotSupportedException { // This check should only be performed once on initial database setup. // Calling dbmsProcessor.setupSharedDatabase() lets dbmsProcessor.checkBaseIntegrity() be true. - if (dbmsProcessor.checkForPre3Dot6Intergrity()) { + if (dbmsProcessor.checkForPare3Dot6Integrity()) { throw new DatabaseNotSupportedException(); } } @@ -175,7 +176,7 @@ public void synchronizeLocalDatabase() { // remove old entries locally removeNotSharedEntries(localEntries, idVersionMap.keySet()); - + List entriesToDrag = new ArrayList<>(); // compare versions and update local entry if needed for (Map.Entry idVersionEntry : idVersionMap.entrySet()) { boolean match = false; @@ -205,12 +206,13 @@ public void synchronizeLocalDatabase() { } } if (!match) { - Optional bibEntry = dbmsProcessor.getSharedEntry(idVersionEntry.getKey()); - if (bibEntry.isPresent()) { - bibDatabase.insertEntry(bibEntry.get(), EntryEventSource.SHARED); - } + entriesToDrag.add(idVersionEntry.getKey()); } } + + for (BibEntry bibEntry : dbmsProcessor.getSharedEntries(entriesToDrag)) { + bibDatabase.insertEntry(bibEntry, EntryEventSource.SHARED); + } } /** @@ -322,10 +324,10 @@ 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 true if the connection is valid, else false. + * @return true if the connection is valid, else false. */ public boolean checkCurrentConnection() { try { diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 1d84949752b..088ff3c9988 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -113,6 +113,24 @@ void testUpdateEntry(DBMSType dbmsType, DBMSConnection dbmsConnection, DBMSProce assertEquals(expectedEntry, actualEntryOptional.get()); } + @ParameterizedTest + @MethodSource("getTestingDatabaseSystems") + void testGetEntriesByIdList(DBMSType dbmsType, DBMSConnection dbmsConnection, DBMSProcessor dbmsProcessor) throws OfflineLockException, SQLException { + dbmsProcessor.setupSharedDatabase(); + BibEntry firstEntry = getBibEntryExample(); + firstEntry.setId("1"); + BibEntry secondEntry = getBibEntryExample(); + secondEntry.setId("2"); + + dbmsProcessor.insertEntry(firstEntry); + dbmsProcessor.insertEntry(secondEntry); + + List sharedEntriesByIdList = dbmsProcessor.getSharedEntries(Arrays.asList(1, 2)); + + assertEquals(firstEntry.getId(), sharedEntriesByIdList.get(0).getId()); + assertEquals(secondEntry.getId(), sharedEntriesByIdList.get(1).getId()); + } + @ParameterizedTest @MethodSource("getTestingDatabaseSystems") void testUpdateNewerEntry(DBMSType dbmsType, DBMSConnection dbmsConnection, DBMSProcessor dbmsProcessor) throws OfflineLockException, SQLException {