Skip to content

Commit

Permalink
Optimize data fetching (#4520)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@Ali96kz @tobiasdiez
Ali96kz and tobiasdiez committed Mar 11, 2019
1 parent fa984a7 commit b906e69
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 75 deletions.
131 changes: 65 additions & 66 deletions src/main/java/org/jabref/logic/shared/DBMSProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -52,16 +54,16 @@ protected DBMSProcessor(DatabaseConnection dbmsConnection) {
* @throws SQLException
*/
public boolean checkBaseIntegrity() throws SQLException {
return checkTableAvailibility("ENTRY", "FIELD", "METADATA");
return checkTableAvailability("ENTRY", "FIELD", "METADATA");
}

/**
* Determines whether the database is using an pre-3.6 structure.
*
* @return <code>true</code> if the structure is old, else <code>false</code>.
*/
public boolean checkForPre3Dot6Intergrity() throws SQLException {
return checkTableAvailibility(
public boolean checkForPare3Dot6Integrity() throws SQLException {
return checkTableAvailability(
"ENTRIES",
"ENTRY_GROUP",
"ENTRY_TYPES",
Expand All @@ -77,7 +79,7 @@ public boolean checkForPre3Dot6Intergrity() throws SQLException {
* @param tableNames Table names to be checked
* @return <code>true</code> if <b>all</b> given tables are present, else <code>false</code>.
*/
private boolean checkTableAvailibility(String... tableNames) throws SQLException {
private boolean checkTableAvailability(String... tableNames) throws SQLException {
List<String> requiredTables = new ArrayList<>();
for (String name : tableNames) {
requiredTables.add(name.toUpperCase(Locale.ENGLISH));
Expand Down Expand Up @@ -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());
Expand All @@ -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()) {
Expand Down Expand Up @@ -420,63 +421,57 @@ public void removeEntry(BibEntry bibEntry) {
* @return instance of {@link BibEntry}
*/
public Optional<BibEntry> getSharedEntry(int sharedID) {
List<BibEntry> sharedEntries = getSharedEntryList(sharedID);
List<BibEntry> sharedEntries = getSharedEntries(Collections.singletonList(sharedID));
if (!sharedEntries.isEmpty()) {
return Optional.of(sharedEntries.get(0));
}
return Optional.empty();
}

public List<BibEntry> 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<BibEntry> getSharedEntryList(int sharedID) {
public List<BibEntry> getSharedEntries(List<Integer> sharedIDs) {
List<BibEntry> 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);
Expand All @@ -485,6 +480,10 @@ private List<BibEntry> getSharedEntryList(int sharedID) {
return sharedEntries;
}

public List<BibEntry> getSharedEntries() {
return getSharedEntries(Collections.emptyList());
}

/**
* Retrieves a mapping between the columns SHARED_ID and VERSION.
*/
Expand Down
20 changes: 11 additions & 9 deletions src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -175,7 +176,7 @@ public void synchronizeLocalDatabase() {

// remove old entries locally
removeNotSharedEntries(localEntries, idVersionMap.keySet());

List<Integer> entriesToDrag = new ArrayList<>();
// compare versions and update local entry if needed
for (Map.Entry<Integer, Integer> idVersionEntry : idVersionMap.entrySet()) {
boolean match = false;
Expand Down Expand Up @@ -205,12 +206,13 @@ public void synchronizeLocalDatabase() {
}
}
if (!match) {
Optional<BibEntry> 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);
}
}

/**
Expand Down Expand Up @@ -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 <code>true</code> if the connection is valid, else <code>false</code>.
* @return <code>true</code> if the connection is valid, else <code>false</code>.
*/
public boolean checkCurrentConnection() {
try {
Expand Down
18 changes: 18 additions & 0 deletions src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<BibEntry> 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 {
Expand Down

0 comments on commit b906e69

Please sign in to comment.