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

Jump to bottom

Optimize data fetching #4520

Merged
merged 16 commits into from
Mar 11, 2019
Merged

Optimize data fetching #4520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3c31a3d
optimize data fetching from database
Nov 28, 2018
e97e346
delete time measure code
Nov 28, 2018
041b5e5
revert code style
Nov 28, 2018
825fc9d
fix checkstyle
Nov 28, 2018
7e73d2b
put columns name instead of *
Nov 28, 2018
534c188
revert checkstyle
Nov 28, 2018
84e0f5f
change for to stream
Dec 4, 2018
67a84f3
refactor variable name
Dec 4, 2018
1cae782
create simple test
Dec 5, 2018
58c91aa
fix test
Dec 5, 2018
b10bc03
Merge branch 'master' into optimize-data-fetching
Dec 5, 2018
3ced52b
rename method name
Dec 30, 2018
7dccfe5
refactor
Feb 16, 2019
e3ebbdb
create test
Feb 16, 2019
18cf256
Fix formatting
tobiasdiez Feb 16, 2019
ee4db9b
Remove obsolete method by slight refactoring
tobiasdiez Mar 11, 2019
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
131 changes: 65 additions & 66 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 @@ -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(";");

tobiasdiez marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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());
tobiasdiez marked this conversation as resolved.
Show resolved Hide resolved
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.
Copy link
Member

Choose a reason for hiding this comment

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

The original formatting looked right to me.

* 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
Open in desktop
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