Skip to content

Commit

Permalink
Small refactorings.
Browse files Browse the repository at this point in the history 
- Use PreparedStatements in trivial methods
- Move escape functionality to DBMSType
  • Loading branch information
@obraliar
obraliar committed Aug 4, 2016
1 parent 9c06495 commit c17631b
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 122 deletions.
189 changes: 98 additions & 91 deletions src/main/java/net/sf/jabref/shared/DBMSProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,11 @@ public void insertEntry(BibEntry bibEntry) {

// Inserting into ENTRY table
StringBuilder insertIntoEntryQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape("ENTRY"))
.append("(")
.append(escape("TYPE"))
.append(") VALUES(?)");
.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 = dbmsHelper.prepareStatement(insertIntoEntryQuery.toString(),
Expand All @@ -195,15 +195,15 @@ public void insertEntry(BibEntry bibEntry) {
// Inserting into FIELD table
for (String fieldName : bibEntry.getFieldNames()) {
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(?, ?, ?)");
.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 = dbmsHelper.prepareStatement(insertFieldQuery.toString())) {
// columnIndex starts with 1
Expand Down Expand Up @@ -246,17 +246,18 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, Sha

// updating entry type
StringBuilder updateEntryTypeQuery = new StringBuilder()
.append("UPDATE ")
.append(escape("ENTRY"))
.append(" SET ")
.append(escape("TYPE"))
.append(" = ?, ")
.append(escape("VERSION"))
.append(" = ")
.append(escape("VERSION"))
.append(" + 1 WHERE ")
.append(escape("SHARED_ID"))
.append(" = ?");
.append("UPDATE ")
.append(escape("ENTRY"))
.append(" SET ")
.append(escape("TYPE"))
.append(" = ?, ")
.append(escape("VERSION"))
.append(" = ")
.append(escape("VERSION"))
.append(" + 1 WHERE ")
.append(escape("SHARED_ID"))
.append(" = ?");

try (PreparedStatement preparedUpdateEntryTypeStatement = dbmsHelper.prepareStatement(updateEntryTypeQuery.toString())) {
preparedUpdateEntryTypeStatement.setString(1, localBibEntry.getType());
preparedUpdateEntryTypeStatement.setInt(2, localBibEntry.getSharedID());
Expand Down Expand Up @@ -284,13 +285,13 @@ private void removeSharedFieldsByDifference(BibEntry localBibEntry, BibEntry sha
nullFields.removeAll(localBibEntry.getFieldNames());
for (String nullField : nullFields) {
StringBuilder deleteFieldQuery = new StringBuilder()
.append("DELETE FROM ")
.append(escape("FIELD"))
.append(" WHERE ")
.append(escape("NAME"))
.append(" = ? AND ")
.append(escape("ENTRY_SHARED_ID"))
.append(" = ?");
.append("DELETE FROM ")
.append(escape("FIELD"))
.append(" WHERE ")
.append(escape("NAME"))
.append(" = ? AND ")
.append(escape("ENTRY_SHARED_ID"))
.append(" = ?");

try (PreparedStatement preparedDeleteFieldStatement = dbmsHelper.prepareStatement(deleteFieldQuery.toString())) {
preparedDeleteFieldStatement.setString(1, nullField);
Expand All @@ -312,8 +313,13 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException {
value = valueOptional.get();
}

StringBuilder selectFieldQuery = new StringBuilder().append("SELECT * FROM ").append(escape("FIELD"))
.append(" WHERE ").append(escape("NAME")).append(" = ? AND ").append(escape("ENTRY_SHARED_ID"))
StringBuilder selectFieldQuery = new StringBuilder()
.append("SELECT * FROM ")
.append(escape("FIELD"))
.append(" WHERE ")
.append(escape("NAME"))
.append(" = ? AND ")
.append(escape("ENTRY_SHARED_ID"))
.append(" = ?");

try (PreparedStatement preparedSelectFieldStatement = dbmsHelper
Expand All @@ -323,9 +329,16 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException {

try (ResultSet selectFieldResultSet = preparedSelectFieldStatement.executeQuery()) {
if (selectFieldResultSet.next()) { // check if field already exists
StringBuilder updateFieldQuery = new StringBuilder().append("UPDATE ").append(escape("FIELD"))
.append(" SET ").append(escape("VALUE")).append(" = ? WHERE ").append(escape("NAME"))
.append(" = ? AND ").append(escape("ENTRY_SHARED_ID")).append(" = ?");
StringBuilder updateFieldQuery = new StringBuilder()
.append("UPDATE ")
.append(escape("FIELD"))
.append(" SET ")
.append(escape("VALUE"))
.append(" = ? WHERE ")
.append(escape("NAME"))
.append(" = ? AND ")
.append(escape("ENTRY_SHARED_ID"))
.append(" = ?");

try (PreparedStatement preparedUpdateFieldStatement = dbmsHelper
.prepareStatement(updateFieldQuery.toString())) {
Expand All @@ -335,9 +348,15 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException {
preparedUpdateFieldStatement.executeUpdate();
}
} else {
StringBuilder insertFieldQuery = new StringBuilder().append("INSERT INTO ")
.append(escape("FIELD")).append("(").append(escape("ENTRY_SHARED_ID")).append(", ")
.append(escape("NAME")).append(", ").append(escape("VALUE"))
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 = dbmsHelper
Expand All @@ -359,11 +378,11 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException {
*/
public void removeEntry(BibEntry bibEntry) {
StringBuilder query = new StringBuilder()
.append("DELETE FROM ")
.append(escape("ENTRY"))
.append(" WHERE ")
.append(escape("SHARED_ID"))
.append(" = ?");
.append("DELETE FROM ")
.append(escape("ENTRY"))
.append(" WHERE ")
.append(escape("SHARED_ID"))
.append(" = ?");

try (PreparedStatement preparedStatement = dbmsHelper.prepareStatement(query.toString())) {
preparedStatement.setInt(1, bibEntry.getSharedID());
Expand Down Expand Up @@ -420,17 +439,19 @@ private List<BibEntry> getSharedEntryList(int sharedID) {
bibEntry.setVersion(selectEntryResultSet.getInt("VERSION"));

StringBuilder selectFieldQuery = new StringBuilder()
.append("SELECT * FROM ")
.append(escape("FIELD"))
.append(" WHERE ")
.append(escape("ENTRY_SHARED_ID"))
.append(" = ")
.append(selectEntryResultSet.getInt("SHARED_ID"));
.append("SELECT * FROM ")
.append(escape("FIELD"))
.append(" WHERE ")
.append(escape("ENTRY_SHARED_ID"))
.append(" = ?");

try (ResultSet selectFieldResultSet = dbmsHelper.query(selectFieldQuery.toString())) {
while (selectFieldResultSet.next()) {
bibEntry.setField(selectFieldResultSet.getString("NAME"),
Optional.ofNullable(selectFieldResultSet.getString("VALUE")), EntryEventSource.SHARED);
try (PreparedStatement preparedSelectFieldStatement = dbmsHelper.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);
}
}
}
sharedEntries.add(bibEntry);
Expand All @@ -448,10 +469,10 @@ private List<BibEntry> getSharedEntryList(int sharedID) {
public Map<Integer, Integer> getSharedIDVersionMapping() {
Map<Integer, Integer> sharedIDVersionMapping = new HashMap<>();
StringBuilder selectEntryQuery = new StringBuilder()
.append("SELECT * FROM ")
.append(escape("ENTRY"))
.append(" ORDER BY ")
.append(escape("SHARED_ID"));
.append("SELECT * FROM ")
.append(escape("ENTRY"))
.append(" ORDER BY ")
.append(escape("SHARED_ID"));

try (ResultSet selectEntryResultSet = dbmsHelper.query(selectEntryQuery.toString())) {
while (selectEntryResultSet.next()) {
Expand Down Expand Up @@ -491,13 +512,13 @@ public void setSharedMetaData(Map<String, String> data) {
for (Map.Entry<String, String> metaEntry : data.entrySet()) {

StringBuilder query = new StringBuilder()
.append("INSERT INTO ")
.append(escape("METADATA"))
.append("(")
.append(escape("KEY"))
.append(", ")
.append(escape("VALUE"))
.append(") VALUES(?, ?)");
.append("INSERT INTO ")
.append(escape("METADATA"))
.append("(")
.append(escape("KEY"))
.append(", ")
.append(escape("VALUE"))
.append(") VALUES(?, ?)");

try (PreparedStatement preparedStatement = dbmsHelper.prepareStatement(query.toString())) {
preparedStatement.setString(1, metaEntry.getKey());
Expand All @@ -515,40 +536,26 @@ public void setSharedMetaData(Map<String, String> data) {
*/
private ResultSet selectFromEntryTable(int sharedID) throws SQLException {
StringBuilder query = new StringBuilder()
.append("SELECT * FROM ")
.append(escape("ENTRY"))
.append(" WHERE ")
.append(escape("SHARED_ID"))
.append(" = ")
.append(sharedID);

return dbmsHelper.query(query.toString());
}
.append("SELECT * FROM ")
.append(escape("ENTRY"))
.append(" WHERE ")
.append(escape("SHARED_ID"))
.append(" = ?");

/**
* Escapes parts of SQL expressions like table or field name to match the conventions
* of the database system.
* @param expression Table or field name
* @param type Type of database system
* @return Correctly escape expression
*/
public static String escape(String expression, DBMSType type) {
if (type == DBMSType.MYSQL) {
return "`" + expression + "`";
} else if ((type == DBMSType.ORACLE) || (type == DBMSType.POSTGRESQL)) {
return "\"" + expression + "\"";
try (PreparedStatement preparedStatement = dbmsHelper.prepareStatement(query.toString())) {
preparedStatement.setInt(1, sharedID);
return preparedStatement.executeQuery();
}
return expression;
}

/**
* Escapes parts of SQL expressions like table or field name to match the conventions
* of the database system using the current dbmsType.
* @param expression Table or field name
* @return Correctly escape expression
* @return Correctly escaped expression
*/
public String escape(String expression) {
return escape(expression, dbmsType);
private String escape(String expression) {
return dbmsType.escape(expression);
}

public void setDBType(DBMSType dbmsType) {
Expand Down
32 changes: 22 additions & 10 deletions src/main/java/net/sf/jabref/shared/DBMSType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package net.sf.jabref.shared;

import java.util.EnumSet;
import java.util.Optional;

/**
Expand All @@ -36,10 +35,10 @@ public enum DBMSType {
"org.postgresql.Driver",
"jdbc:postgresql://%s:%d/%s", 5432);

private String type;
private String driverPath;
private String urlPattern;
private int defaultPort;
private final String type;
private final String driverPath;
private final String urlPattern;
private final int defaultPort;


private DBMSType(String type, String driverPath, String urlPattern, int defaultPort) {
Expand Down Expand Up @@ -76,13 +75,26 @@ public int getDefaultPort() {
}

public static Optional<DBMSType> fromString(String typeName) {
for (DBMSType dbmsType : EnumSet.allOf(DBMSType.class)) {
if (typeName.equals(dbmsType.toString())) {
return Optional.ofNullable(dbmsType);
}
try {
return Optional.of(Enum.valueOf(DBMSType.class, typeName.toUpperCase()));
} catch (IllegalArgumentException exception) {
return Optional.empty();
}
return Optional.empty();
}

/**
* Escapes parts of SQL expressions like table or field name to match the conventions
* of the current database system type.
* @param expression Table or field name
* @return Correctly escaped expression
*/
public String escape(String expression) {
if (this == DBMSType.MYSQL) {
return "`" + expression + "`";
} else if ((this == DBMSType.ORACLE) || (this == DBMSType.POSTGRESQL)) {
return "\"" + expression + "\"";
}
return expression;
}

}
2 changes: 1 addition & 1 deletion src/test/java/net/sf/jabref/shared/DBMSHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void clear() {
}

public String escape(String expression) {
return DBMSProcessor.escape(expression, dbmsType);
return dbmsType.escape(expression);
}

}
16 changes: 1 addition & 15 deletions src/test/java/net/sf/jabref/shared/DBMSProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,20 +268,6 @@ public void testSetSharedMetaData() {
Assert.assertEquals(expectedMetaData, actualMetaData);
}

@Test
public void testEscape() {

if (dbmsType == DBMSType.MYSQL) {
Assert.assertEquals("`TABLE`", dbmsProcessor.escape("TABLE"));
Assert.assertEquals("`TABLE`", DBMSProcessor.escape("TABLE", dbmsType));
} else if ((dbmsType == DBMSType.ORACLE) || (dbmsType == DBMSType.POSTGRESQL)) {
Assert.assertEquals("\"TABLE\"", dbmsProcessor.escape("TABLE"));
Assert.assertEquals("\"TABLE\"", DBMSProcessor.escape("TABLE", dbmsType));
}

Assert.assertEquals("TABLE", DBMSProcessor.escape("TABLE", null));
}

private Map<String, String> getMetaDataExample() {
Map<String, String> expectedMetaData = new HashMap<>();

Expand Down Expand Up @@ -335,7 +321,7 @@ private void insertMetaData(String key, String value) {
}

private String escape(String expression) {
return dbmsProcessor.escape(expression);
return dbmsType.escape(expression);
}

private String escapeValue(String value) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/net/sf/jabref/shared/DBMSSynchronizerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ private BibEntry getBibEntryExample(int index) {
}

private String escape(String expression) {
return dbmsProcessor.escape(expression);
return dbmsType.escape(expression);
}

@After
Expand Down
Loading

0 comments on commit c17631b

Please sign in to comment.