diff --git a/CHANGELOG.md b/CHANGELOG.md index aaef3c8e8ea..9158dd129d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We improved the performance when pasting and importing entries in an existing library. [#11843](https://github.com/JabRef/jabref/pull/11843) - When fulltext search is selected but indexing is deactivated, a dialog is now shown asking if the user wants to enable indexing now [#9491](https://github.com/JabRef/jabref/issues/9491) - We changed instances of 'Search Selected' to 'Search Pre-configured' in Web Search Preferences UI. [#11871](https://github.com/JabRef/jabref/pull/11871) +- We rewrote the [remote SQL database](https://docs.jabref.org/collaborative-work/sqldatabase) support. ⚠️database tables will be migrated. [#11879](https://github.com/JabRef/jabref/pull/11879) ### Fixed diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index b3647b4ae0e..c925a077f52 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -24,6 +24,7 @@ import org.jabref.model.entry.event.EntriesEventSource; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; +import org.jabref.model.entry.types.EntryType; import org.jabref.model.entry.types.EntryTypeFactory; import org.jabref.model.metadata.MetaData; @@ -129,19 +130,6 @@ public void setupSharedDatabase() throws SQLException { */ protected abstract void setUp() throws SQLException; - /** - * Escapes parts of SQL expressions such as a table name or a field name to match the conventions of the database - * system using the current dbmsType. - *

- * This method is package private, because of DBMSProcessorTest - * - * @param expression Table or field name - * @return Correctly escaped expression - */ - abstract String escape(String expression); - - abstract String escape_Table(String expression); - abstract Integer getCURRENT_VERSION_DB_STRUCT(); /** @@ -173,12 +161,11 @@ public void insertEntries(List bibEntries) { * @param bibEntries List of {@link BibEntry} to be inserted */ protected void insertIntoEntryTable(List bibEntries) { - StringBuilder insertIntoEntryQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("ENTRY")) - .append("(") - .append(escape("TYPE")) - .append(") VALUES(?)"); + if (bibEntries.isEmpty()) { + return; + } + + StringBuilder insertIntoEntryQuery = new StringBuilder().append("INSERT INTO entry (entrytype) VALUES (?)"); // Number of commas is bibEntries.size() - 1 insertIntoEntryQuery.append(", (?)".repeat(Math.max(0, (bibEntries.size() - 1)))); @@ -201,7 +188,7 @@ protected void insertIntoEntryTable(List bibEntries) { } } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } @@ -222,8 +209,7 @@ private List getNotYetExistingEntries(List bibEntries) { return bibEntries; } try { - String selectQuery = "SELECT * FROM " + - escape_Table("ENTRY"); + String selectQuery = "SELECT * FROM ENTRY"; try (ResultSet resultSet = connection.createStatement().executeQuery(selectQuery)) { while (resultSet.next()) { @@ -245,22 +231,19 @@ private List getNotYetExistingEntries(List bibEntries) { * @param bibEntries {@link BibEntry} to be inserted */ protected void insertIntoFieldTable(List bibEntries) { + if (bibEntries.isEmpty()) { + return; + } + try { // Inserting into FIELD table // Coerce to ArrayList in order to use List.get() - List> fields = bibEntries.stream().map(bibEntry -> 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_Table("FIELD")) - .append("(") - .append(escape("ENTRY_SHARED_ID")) - .append(", ") - .append(escape("NAME")) - .append(", ") - .append(escape("VALUE")) - .append(") VALUES(?, ?, ?)"); + .append("INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE) VALUES(?, ?, ?)"); int numFields = 0; for (List entryFields : fields) { numFields += entryFields.size(); @@ -283,6 +266,7 @@ protected void insertIntoFieldTable(List bibEntries) { fieldsCompleted += 1; } } + // TODO: This could grow too large for a single query preparedFieldStatement.executeUpdate(); } } catch (SQLException e) { @@ -317,18 +301,12 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL .getVersion()) || localBibEntry.equals(sharedBibEntry)) { insertOrUpdateFields(localBibEntry); - // updating entry type - String updateEntryTypeQuery = "UPDATE " + - escape_Table("ENTRY") + - " SET " + - escape("TYPE") + - " = ?, " + - escape("VERSION") + - " = " + - escape("VERSION") + - " + 1 WHERE " + - escape("SHARED_ID") + - " = ?"; + String updateEntryTypeQuery = """ + UPDATE entry + SET entrytype = ?, + version = version + 1 + WHERE shared_id = ? + """; try (PreparedStatement preparedUpdateEntryTypeStatement = connection.prepareStatement(updateEntryTypeQuery)) { preparedUpdateEntryTypeStatement.setString(1, localBibEntry.getType().getName()); @@ -355,13 +333,10 @@ private void removeSharedFieldsByDifference(BibEntry localBibEntry, BibEntry sha Set nullFields = new HashSet<>(sharedBibEntry.getFields()); nullFields.removeAll(localBibEntry.getFields()); for (Field nullField : nullFields) { - String deleteFieldQuery = "DELETE FROM " + - escape_Table("FIELD") + - " WHERE " + - escape("NAME") + - " = ? AND " + - escape("ENTRY_SHARED_ID") + - " = ?"; + String deleteFieldQuery = """ + DELETE FROM FIELD + WHERE NAME = ? AND ENTRY_SHARED_ID = ? + """; try (PreparedStatement preparedDeleteFieldStatement = connection .prepareStatement(deleteFieldQuery)) { @@ -385,13 +360,10 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { value = valueOptional.get(); } - String selectFieldQuery = "SELECT * FROM " + - escape_Table("FIELD") + - " WHERE " + - escape("NAME") + - " = ? AND " + - escape("ENTRY_SHARED_ID") + - " = ?"; + String selectFieldQuery = """ + SELECT * FROM FIELD + WHERE NAME = ? AND ENTRY_SHARED_ID = ? + """; try (PreparedStatement preparedSelectFieldStatement = connection .prepareStatement(selectFieldQuery)) { @@ -400,15 +372,11 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { try (ResultSet selectFieldResultSet = preparedSelectFieldStatement.executeQuery()) { if (selectFieldResultSet.next()) { // check if field already exists - String updateFieldQuery = "UPDATE " + - escape_Table("FIELD") + - " SET " + - escape("VALUE") + - " = ? WHERE " + - escape("NAME") + - " = ? AND " + - escape("ENTRY_SHARED_ID") + - " = ?"; + String updateFieldQuery = """ + UPDATE FIELD + SET VALUE = ? + WHERE NAME = ? AND ENTRY_SHARED_ID = ? + """; try (PreparedStatement preparedUpdateFieldStatement = connection .prepareStatement(updateFieldQuery)) { @@ -418,15 +386,10 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { preparedUpdateFieldStatement.executeUpdate(); } } else { - String insertFieldQuery = "INSERT INTO " + - escape_Table("FIELD") + - "(" + - escape("ENTRY_SHARED_ID") + - ", " + - escape("NAME") + - ", " + - escape("VALUE") + - ") VALUES(?, ?, ?)"; + String insertFieldQuery = """ + INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE) + VALUES (?, ?, ?) + """; try (PreparedStatement preparedFieldStatement = connection .prepareStatement(insertFieldQuery)) { @@ -451,12 +414,7 @@ public void removeEntries(List bibEntries) { if (bibEntries.isEmpty()) { return; } - StringBuilder query = new StringBuilder() - .append("DELETE FROM ") - .append(escape_Table("ENTRY")) - .append(" WHERE ") - .append(escape("SHARED_ID")) - .append(" IN ("); + StringBuilder query = new StringBuilder().append("DELETE FROM ENTRY WHERE SHARED_ID IN ("); query.append("?, ".repeat(bibEntries.size() - 1)); query.append("?)"); @@ -509,31 +467,19 @@ public List getSharedEntries(List sharedIDs) { List sharedEntries = new ArrayList<>(); - StringBuilder query = new StringBuilder(); - query.append("SELECT ") - .append(escape_Table("ENTRY")).append(".").append(escape("SHARED_ID")).append(", ") - .append(escape_Table("ENTRY")).append(".").append(escape("TYPE")).append(", ") - .append(escape_Table("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_Table("ENTRY")) - // Handle special case if entry does not have any fields (yet) - .append(" left outer join ") - .append(escape_Table("FIELD")) - .append(" F on ") - .append(escape_Table("ENTRY")).append(".").append(escape("SHARED_ID")) - .append(" = F.").append(escape("ENTRY_SHARED_ID")); + StringBuilder query = new StringBuilder() + .append("SELECT entry.shared_id, entry.entrytype, entry.version, ") + .append("F.entry_shared_id, F.name, F.value ") + .append("FROM entry ") + .append("LEFT OUTER JOIN field F ON entry.shared_id = F.entry_shared_id"); if (!sharedIDs.isEmpty()) { - query.append(" where ") - .append(escape("SHARED_ID")).append(" in (") + query.append(" WHERE entry.shared_id IN (") .append("?, ".repeat(sharedIDs.size() - 1)) .append("?)"); } - query.append(" order by ") - .append(escape("SHARED_ID")); + + query.append(" ORDER BY shared_id"); try (PreparedStatement preparedStatement = connection.prepareStatement(query.toString())) { for (int i = 0; i < sharedIDs.size(); i++) { @@ -547,12 +493,16 @@ public List getSharedEntries(List sharedIDs) { // We get a list of field values of bib entries "grouped" by bib entries // Thus, the first change in the shared id leads to a new BibEntry if (selectEntryResultSet.getInt("SHARED_ID") > lastId) { - bibEntry = new BibEntry(); - bibEntry.getSharedBibEntryData().setSharedID(selectEntryResultSet.getInt("SHARED_ID")); - bibEntry.setType(EntryTypeFactory.parse(selectEntryResultSet.getString("TYPE"))); - bibEntry.getSharedBibEntryData().setVersion(selectEntryResultSet.getInt("VERSION")); + int sharedId = selectEntryResultSet.getInt("shared_id"); + int version = selectEntryResultSet.getInt("version"); + EntryType entrytype = EntryTypeFactory.parse(selectEntryResultSet.getString("entrytype")); + + bibEntry = new BibEntry(entrytype); + bibEntry.getSharedBibEntryData().setSharedID(sharedId); + bibEntry.getSharedBibEntryData().setVersion(version); + sharedEntries.add(bibEntry); - lastId = selectEntryResultSet.getInt("SHARED_ID"); + lastId = sharedId; } // In all cases, we set the field value of the newly created BibEntry object @@ -563,8 +513,7 @@ public List getSharedEntries(List sharedIDs) { } } } catch (SQLException e) { - LOGGER.error("Executed >{}<", query); - LOGGER.error("SQL Error", e); + LOGGER.error("Executed >{}< and got an error", query, e); return Collections.emptyList(); } @@ -580,10 +529,10 @@ public List getSharedEntries() { */ public Map getSharedIDVersionMapping() { Map sharedIDVersionMapping = new HashMap<>(); - String selectEntryQuery = "SELECT * FROM " + - escape_Table("ENTRY") + - " ORDER BY " + - escape("SHARED_ID"); + String selectEntryQuery = """ + SELECT * FROM ENTRY + ORDER BY SHARED_ID + """; try (ResultSet selectEntryResultSet = connection.createStatement().executeQuery(selectEntryQuery)) { while (selectEntryResultSet.next()) { @@ -602,7 +551,7 @@ public Map getSharedIDVersionMapping() { public Map getSharedMetaData() { Map data = new HashMap<>(); - try (ResultSet resultSet = connection.createStatement().executeQuery("SELECT * FROM " + escape_Table("METADATA"))) { + try (ResultSet resultSet = connection.createStatement().executeQuery("SELECT * FROM METADATA")) { while (resultSet.next()) { data.put(resultSet.getString("KEY"), resultSet.getString("VALUE")); } @@ -619,21 +568,12 @@ public Map getSharedMetaData() { * @param data JabRef meta data as map */ public void setSharedMetaData(Map data) throws SQLException { - String insertOrUpdateQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("METADATA")) - .append(" (") - .append(escape("KEY")) - .append(", ") - .append(escape("VALUE")) - .append(") VALUES (?, ?) ") - .append("ON CONFLICT (") - .append(escape("KEY")) - .append(") DO UPDATE SET ") - .append(escape("VALUE")) - .append(" = EXCLUDED.") - .append(escape("VALUE")) - .toString(); + String insertOrUpdateQuery = """ + INSERT INTO METADATA (KEY, VALUE) + VALUES (?, ?) + ON CONFLICT (KEY) DO UPDATE + SET VALUE = EXCLUDED.VALUE + """; try (PreparedStatement statement = connection.prepareStatement(insertOrUpdateQuery)) { for (Map.Entry metaEntry : data.entrySet()) { diff --git a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java index 63b10440b9e..d3b880829d8 100644 --- a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java +++ b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java @@ -6,6 +6,7 @@ import java.sql.Statement; import java.util.List; import java.util.Map; +import java.util.StringJoiner; import org.jabref.logic.shared.listener.PostgresSQLNotificationListener; import org.jabref.logic.util.HeadlessExecutorService; @@ -22,7 +23,9 @@ public class PostgreSQLProcessor extends DBMSProcessor { private PostgresSQLNotificationListener listener; private int VERSION_DB_STRUCT_DEFAULT = -1; - private final int CURRENT_VERSION_DB_STRUCT = 1; + + // TODO: We need to migrate data - or ask the user to recreate + private final int CURRENT_VERSION_DB_STRUCT = 2; public PostgreSQLProcessor(DatabaseConnection connection) { super(connection); @@ -41,24 +44,37 @@ public void setUp() throws SQLException { VERSION_DB_STRUCT_DEFAULT = 0; } - connection.createStatement().executeUpdate("CREATE SCHEMA IF NOT EXISTS jabref"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS " + escape_Table("ENTRY") + " (" + - "\"SHARED_ID\" SERIAL PRIMARY KEY, " + - "\"TYPE\" VARCHAR, " + - "\"VERSION\" INTEGER DEFAULT 1)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS " + escape_Table("FIELD") + " (" + - "\"ENTRY_SHARED_ID\" INTEGER REFERENCES " + escape_Table("ENTRY") + "(\"SHARED_ID\") ON DELETE CASCADE, " + - "\"NAME\" VARCHAR, " + - "\"VALUE\" TEXT)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS " + escape_Table("METADATA") + " (" - + "\"KEY\" VARCHAR," - + "\"VALUE\" TEXT)"); + // TODO: Before a release, fix the names (and migrate data to the new names) + // Think of using Flyway or Liquibase instead of manual migration + // If changed, also adjust {@link org.jabref.logic.shared.TestManager.clearTables} + connection.createStatement().executeUpdate("CREATE SCHEMA IF NOT EXISTS \"jabref-alpha\""); + connection.createStatement().executeUpdate("SET search_path TO \"jabref-alpha\""); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS entry ( + shared_id SERIAL PRIMARY KEY, + entrytype VARCHAR, + version INTEGER DEFAULT 1 + ) + """); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS field ( + entry_shared_id INTEGER REFERENCES entry(shared_id) ON DELETE CASCADE, + name VARCHAR, + value TEXT + ) + """); + connection.createStatement().executeUpdate("CREATE INDEX idx_field_entry_shared_id ON FIELD (ENTRY_SHARED_ID);"); + connection.createStatement().executeUpdate("CREATE INDEX idx_field_name ON FIELD (NAME);"); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS metadata ( + key VARCHAR, + value TEXT + ) + """); + connection.createStatement().executeUpdate("CREATE UNIQUE INDEX idx_metadata_key ON METADATA (key);"); Map metadata = getSharedMetaData(); @@ -100,29 +116,34 @@ ON CONFLICT (KEY) // We can migrate data from old tables in new table if (VERSION_DB_STRUCT_DEFAULT == 0 && CURRENT_VERSION_DB_STRUCT == 1) { LOGGER.info("Migrating from VersionDBStructure == 0"); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("ENTRY") + " SELECT * FROM \"ENTRY\""); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("FIELD") + " SELECT * FROM \"FIELD\""); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("METADATA") + " SELECT * FROM \"METADATA\""); - connection.createStatement().execute("SELECT setval(\'jabref.\"ENTRY_SHARED_ID_seq\"\', (select max(\"SHARED_ID\") from jabref.\"ENTRY\"))"); + connection.createStatement().executeUpdate("INSERT INTO ENTRY SELECT * FROM \"ENTRY\""); + connection.createStatement().executeUpdate("INSERT INTO FIELD SELECT * FROM \"FIELD\""); + connection.createStatement().executeUpdate("INSERT INTO METADATA SELECT * FROM \"METADATA\""); + connection.createStatement().execute("SELECT setval(\'\"ENTRY_SHARED_ID_seq\"\', (select max(\"SHARED_ID\") from \"ENTRY\"))"); metadata = getSharedMetaData(); } metadata.put(MetaData.VERSION_DB_STRUCT, String.valueOf(CURRENT_VERSION_DB_STRUCT)); setSharedMetaData(metadata); } + + // TODO: implement migration of changes from version 1 to 2 + // - "TYPE" is now called entrytype (to be consistent with org.jabref.model.entry.field.InternalField.TYPE_HEADER) + // - table names and field names now lower case } @Override protected void insertIntoEntryTable(List bibEntries) { - StringBuilder insertIntoEntryQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("ENTRY")) - .append("(") - .append(escape("TYPE")) - .append(") VALUES(?)"); - // Number of commas is bibEntries.size() - 1 - insertIntoEntryQuery.append(", (?)".repeat(Math.max(0, bibEntries.size() - 1))); - try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(), + if (bibEntries.isEmpty()) { + return; + } + + StringJoiner insertIntoEntryQuery = new StringJoiner(", ", "INSERT INTO entry (entrytype) values ", ";"); + for (int i = 0; i < bibEntries.size(); i++) { + insertIntoEntryQuery.add("(?)"); + } + try (PreparedStatement preparedEntryStatement = connection.prepareStatement( + insertIntoEntryQuery.toString(), Statement.RETURN_GENERATED_KEYS)) { for (int i = 0; i < bibEntries.size(); i++) { preparedEntryStatement.setString(i + 1, bibEntries.get(i).getType().getName()); @@ -145,16 +166,6 @@ protected void insertIntoEntryTable(List bibEntries) { } } - @Override - String escape(String expression) { - return "\"" + expression + "\""; - } - - @Override - String escape_Table(String expression) { - return "jabref." + escape(expression); - } - @Override Integer getCURRENT_VERSION_DB_STRUCT() { return CURRENT_VERSION_DB_STRUCT; diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 706037d63d9..1b6a04ee9f9 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -77,14 +77,14 @@ void insertEntry() throws SQLException { Map actualFieldMap = new HashMap<>(); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(1, entryResultSet.getInt("SHARED_ID")); - assertEquals("inproceedings", entryResultSet.getString("TYPE")); + assertEquals("inproceedings", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertFalse(entryResultSet.next()); - try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) { + try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection)) { while (fieldResultSet.next()) { actualFieldMap.put(fieldResultSet.getString("NAME"), fieldResultSet.getString("VALUE")); } @@ -102,15 +102,15 @@ void insertEntryWithEmptyFields() throws SQLException { dbmsProcessor.insertEntry(expectedEntry); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(1, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertFalse(entryResultSet.next()); // Adding an empty entry should not create an entry in field table, only in entry table - try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) { + try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection)) { assertFalse(fieldResultSet.next()); } } @@ -206,7 +206,7 @@ void removeAllEntries() throws SQLException { dbmsProcessor.insertEntry(secondEntry); dbmsProcessor.removeEntries(entriesToRemove); - try (ResultSet resultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet resultSet = selectFrom("ENTRY", dbmsConnection)) { assertFalse(resultSet.next()); } } @@ -225,7 +225,7 @@ void removeSomeEntries() throws SQLException { dbmsProcessor.insertEntry(thirdEntry); dbmsProcessor.removeEntries(entriesToRemove); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(2, entryResultSet.getInt("SHARED_ID")); assertFalse(entryResultSet.next()); @@ -238,7 +238,7 @@ void removeSingleEntry() throws SQLException { dbmsProcessor.insertEntry(entryToRemove); dbmsProcessor.removeEntries(Collections.singletonList(entryToRemove)); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertFalse(entryResultSet.next()); } } @@ -252,7 +252,7 @@ void removeEntriesOnNullThrows() { void removeEmptyEntryList() throws SQLException { dbmsProcessor.removeEntries(Collections.emptyList()); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertFalse(entryResultSet.next()); } } @@ -288,9 +288,10 @@ void getNotExistingSharedEntry() { @Test void getSharedIDVersionMapping() throws OfflineLockException, SQLException { BibEntry firstEntry = getBibEntryExample(); - BibEntry secondEntry = getBibEntryExample(); - dbmsProcessor.insertEntry(firstEntry); + + // insert duplicate entry + BibEntry secondEntry = getBibEntryExample(); dbmsProcessor.insertEntry(secondEntry); dbmsProcessor.updateEntry(secondEntry); @@ -305,11 +306,10 @@ void getSharedIDVersionMapping() throws OfflineLockException, SQLException { @Test void getSharedMetaData() { - insertMetaData("databaseType", "bibtex;", dbmsConnection, dbmsProcessor); - insertMetaData("protectedFlag", "true;", dbmsConnection, dbmsProcessor); - insertMetaData("saveActions", "enabled;\nauthor[capitalize,html_to_latex]\ntitle[title_case]\n;", dbmsConnection, dbmsProcessor); - insertMetaData("saveOrderConfig", "specified;author;false;title;false;year;true;", dbmsConnection, dbmsProcessor); - insertMetaData("VersionDBStructure", "1", dbmsConnection, dbmsProcessor); + insertMetaData("databaseType", "bibtex;", dbmsConnection); + insertMetaData("protectedFlag", "true;", dbmsConnection); + insertMetaData("saveActions", "enabled;\nauthor[capitalize,html_to_latex]\ntitle[title_case]\n;", dbmsConnection); + insertMetaData("saveOrderConfig", "specified;author;false;title;false;year;true;", dbmsConnection); Map expectedMetaData = getMetaDataExample(); Map actualMetaData = dbmsProcessor.getSharedMetaData(); @@ -334,7 +334,7 @@ private static Map getMetaDataExample() { expectedMetaData.put("protectedFlag", "true;"); expectedMetaData.put("saveActions", "enabled;\nauthor[capitalize,html_to_latex]\ntitle[title_case]\n;"); expectedMetaData.put("saveOrderConfig", "specified;author;false;title;false;year;true;"); - expectedMetaData.put("VersionDBStructure", "1"); + expectedMetaData.put("VersionDBStructure", "2"); return expectedMetaData; } @@ -378,30 +378,30 @@ void insertMultipleEntries() throws SQLException { Map> actualFieldMap = new HashMap<>(); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(1, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(2, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(3, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(4, entryResultSet.getInt("SHARED_ID")); - assertEquals("thesis", entryResultSet.getString("TYPE")); + assertEquals("thesis", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(5, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertFalse(entryResultSet.next()); - try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) { + try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection)) { while (fieldResultSet.next()) { if (actualFieldMap.containsKey(fieldResultSet.getInt("ENTRY_SHARED_ID"))) { actualFieldMap.get(fieldResultSet.getInt("ENTRY_SHARED_ID")).put( @@ -424,9 +424,9 @@ void insertMultipleEntries() throws SQLException { assertEquals(expectedFieldMap, actualFieldMap); } - private ResultSet selectFrom(String table, DBMSConnection dbmsConnection, DBMSProcessor dbmsProcessor) { + private ResultSet selectFrom(String table, DBMSConnection dbmsConnection) { try { - return dbmsConnection.getConnection().createStatement().executeQuery("SELECT * FROM " + escape_Table(table, dbmsProcessor)); + return dbmsConnection.getConnection().createStatement().executeQuery("SELECT * FROM " + table); } catch (SQLException e) { fail(e.getMessage()); return null; @@ -434,24 +434,11 @@ private ResultSet selectFrom(String table, DBMSConnection dbmsConnection, DBMSPr } // Oracle does not support multiple tuple insertion in one INSERT INTO command. - // Therefore this function was defined to improve the readability and to keep the code short. - private void insertMetaData(String key, String value, DBMSConnection dbmsConnection, DBMSProcessor dbmsProcessor) { + // Therefore, this function was defined to improve the readability and to keep the code short. + private void insertMetaData(String key, String value, DBMSConnection dbmsConnection) { assertDoesNotThrow(() -> { - dbmsConnection.getConnection().createStatement().executeUpdate("INSERT INTO " + escape_Table("METADATA", dbmsProcessor) + "(" - + escape("KEY", dbmsProcessor) + ", " + escape("VALUE", dbmsProcessor) + ") VALUES(" - + escapeValue(key) + ", " + escapeValue(value) + ")"); + // TODO: maybe use a prepared statement here + dbmsConnection.getConnection().createStatement().executeUpdate("INSERT INTO metadata (\"key\", value) VALUES ('" + key + "', '" + value + "');"); }); } - - private static String escape(String expression, DBMSProcessor dbmsProcessor) { - return dbmsProcessor.escape(expression); - } - - private static String escape_Table(String expression, DBMSProcessor dbmsProcessor) { - return dbmsProcessor.escape_Table(expression); - } - - private static String escapeValue(String value) { - return "'" + value + "'"; - } } diff --git a/src/test/java/org/jabref/logic/shared/TestManager.java b/src/test/java/org/jabref/logic/shared/TestManager.java index b4007370c4b..17cf4ef3745 100644 --- a/src/test/java/org/jabref/logic/shared/TestManager.java +++ b/src/test/java/org/jabref/logic/shared/TestManager.java @@ -1,7 +1,6 @@ package org.jabref.logic.shared; import java.sql.SQLException; -import java.util.Objects; /** * This class provides helping methods for database tests. Furthermore, it determines database systems which are ready to @@ -9,10 +8,6 @@ */ public class TestManager { public static void clearTables(DBMSConnection dbmsConnection) throws SQLException { - Objects.requireNonNull(dbmsConnection); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP TABLE IF EXISTS jabref.\"FIELD\""); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP TABLE IF EXISTS jabref.\"ENTRY\""); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP TABLE IF EXISTS jabref.\"METADATA\""); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP SCHEMA IF EXISTS jabref"); + dbmsConnection.getConnection().createStatement().executeUpdate("DROP SCHEMA IF EXISTS \"jabref-alpha\" CASCADE"); } }