diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ec81fc21db6..77d4528c10b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -198,18 +198,9 @@ jobs: databasetests: name: Database tests runs-on: ubuntu-latest - services: - postgres: - image: postgres:13-alpine - env: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: postgres - POSTGRES_DB: postgres - ports: - - 5432:5432 - # needed because the postgres container does not provide a healthcheck - options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 steps: + - name: Shutdown Ubuntu MySQL + run: sudo service mysql stop # Shutdown the Default MySQL to save memory, "sudo" is necessary, please do not remove it - name: Checkout source uses: actions/checkout@v4 with: @@ -228,24 +219,6 @@ jobs: run: ./gradlew databaseTest --rerun-tasks env: CI: "true" - DBMS: "postgresql" - - name: Shutdown Ubuntu MySQL - run: sudo service mysql stop # Shutdown the Default MySQL, "sudo" is necessary, please not remove it - - name: Start custom MySQL - uses: mirromutth/mysql-action@v1.1 - with: - host port: 3800 - container port: 3307 - character set server: 'utf8' - collation server: 'utf8_general_ci' - mysql version: '8.0' - mysql database: 'jabref' - mysql root password: 'root' - - name: Run tests on MySQL - run: ./gradlew databaseTest --rerun-tasks - env: - CI: "true" - DBMS: "mysql" guitests: name: GUI tests diff --git a/CHANGELOG.md b/CHANGELOG.md index f1bfa3434b5..371843ab92a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,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) - We added a new CSS style class `main-table` for the main table. [#11881](https://github.com/JabRef/jabref/pull/11881) ### Fixed @@ -93,7 +94,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We removed support for case-sensitive and exact search. [#11542](https://github.com/JabRef/jabref/pull/11542) - We removed the description of search strings. [#11542](https://github.com/JabRef/jabref/pull/11542) - We removed support for importing using the SilverPlatterImporter (`Record INSPEC`). [#11576](https://github.com/JabRef/jabref/pull/11576) - +- We removed support for MySQL/MariaDB and Oracle. diff --git a/build.gradle b/build.gradle index ff57188b2c0..07416f2cd1b 100644 --- a/build.gradle +++ b/build.gradle @@ -205,18 +205,10 @@ dependencies { implementation 'com.fasterxml:aalto-xml:1.3.3' - implementation group: 'org.mariadb.jdbc', name: 'mariadb-java-client', version: '2.7.9' - implementation 'org.postgresql:postgresql:42.7.4' // Support unix socket connection types implementation 'com.kohlschutter.junixsocket:junixsocket-core:2.10.1' - implementation 'com.kohlschutter.junixsocket:junixsocket-mysql:2.10.0' - - implementation ('com.oracle.ojdbc:ojdbc10:19.3.0.0') { - // causing module issues - exclude module: 'oraclepki' - } implementation ('com.google.guava:guava:33.1.0-jre') { // TODO: Remove this as soon as https://github.com/google/guava/issues/2960 is fixed @@ -370,6 +362,9 @@ dependencies { // Even if "compileOnly" is used, IntelliJ always adds to module-info.java. To avoid issues during committing, we use "implementation" instead of "compileOnly" implementation 'io.github.adr:e-adr:2.0.0-SNAPSHOT' + implementation 'io.zonky.test:embedded-postgres:2.0.7' + implementation enforcedPlatform('io.zonky.test.postgres:embedded-postgres-binaries-bom:17.0.0') + testImplementation 'io.github.classgraph:classgraph:4.8.176' testImplementation 'org.junit.jupiter:junit-jupiter:5.11.0' testImplementation 'org.junit.platform:junit-platform-launcher:1.10.3' @@ -803,24 +798,8 @@ jlink { uses 'kong.unirest.core.json.JsonEngine' uses 'org.eclipse.jgit.lib.Signer' uses 'org.eclipse.jgit.transport.SshSessionFactory' - uses 'org.mariadb.jdbc.LocalInfileInterceptor' - uses 'org.mariadb.jdbc.authentication.AuthenticationPlugin' - uses 'org.mariadb.jdbc.credential.CredentialPlugin' - uses 'org.mariadb.jdbc.tls.TlsSocketPlugin' - provides 'org.mariadb.jdbc.tls.TlsSocketPlugin' with 'org.mariadb.jdbc.internal.protocol.tls.DefaultTlsSocketPlugin' provides 'java.sql.Driver' with 'org.postgresql.Driver' - provides 'org.mariadb.jdbc.authentication.AuthenticationPlugin' with 'org.mariadb.jdbc.internal.com.send.authentication.CachingSha2PasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.ClearPasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.Ed25519PasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.NativePasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.OldPasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.SendGssApiAuthPacket', - 'org.mariadb.jdbc.internal.com.send.authentication.SendPamAuthPacket', - 'org.mariadb.jdbc.internal.com.send.authentication.Sha256PasswordPlugin' - provides 'org.mariadb.jdbc.credential.CredentialPlugin' with 'org.mariadb.jdbc.credential.aws.AwsIamCredentialPlugin', - 'org.mariadb.jdbc.credential.env.EnvCredentialPlugin', - 'org.mariadb.jdbc.credential.system.PropertiesCredentialPlugin' provides 'java.security.Provider' with 'org.bouncycastle.jce.provider.BouncyCastleProvider', 'org.bouncycastle.pqc.jcajce.provider.BouncyCastlePQCProvider' provides 'kong.unirest.core.json.JsonEngine' with 'kong.unirest.modules.gson.GsonEngine'; diff --git a/docs/code-howtos/remote-storage-sql.md b/docs/code-howtos/remote-storage-sql.md index 267d05129fe..5a28cbc5cc4 100644 --- a/docs/code-howtos/remote-storage-sql.md +++ b/docs/code-howtos/remote-storage-sql.md @@ -9,14 +9,25 @@ For user documentation, see . + +## Handling synchronization of "micro-edits" + +It causes too much load both on the server and at all subscribed clients to synchronize every single letter change. +Therefore, synchronization only happens if several conditions are fulfilled: * Edit to another field. * Major changes have been made (pasting or deleting more than one character). Class `org.jabref.logic.util.CoarseChangeFilter.java` checks both conditions. -Remaining changes that have not been synchronized yet are saved at closing the database rendering additional closing time. Saving is realized in `org.jabref.logic.shared.DBMSSynchronizer.java`. Following methods account for synchronization modes: +Remaining changes that have not been synchronized yet are saved at closing the database rendering additional closing time. +Saving is realized in `org.jabref.logic.shared.DBMSSynchronizer.java`. + +Following methods account for synchronization modes: * `pullChanges` synchronizes the database unconditionally. * `pullLastEntryChanges` synchronizes only if there are remaining entry changes. It is invoked when closing the shared database (`closeSharedDatabase`). @@ -61,3 +72,8 @@ PostgreSQL supports to register listeners on the database on changes. The listening is implemented at [`org.jabref.logic.shared.listener.PostgresSQLNotificationListener`](https://github.com/JabRef/jabref/blob/main/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java#L16). It "just" fetches updates from the server when a change occurred there. Thus, the changes are not actively pushed from the server, but still need to be fetched by the client. + +## Tests + +Tests are executed using [Zonky Embedded Postgres](https://github.com/zonkyio/embedded-postgres). +This installs and runs a PostgreSQL server and frees the developer from the need to install a PostgreSQL server on the local machine. diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 852af1ba396..db91051b266 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -97,10 +97,7 @@ // endregion // region: SQL databases - requires ojdbc10; requires org.postgresql.jdbc; - requires org.mariadb.jdbc; - uses org.mariadb.jdbc.credential.CredentialPlugin; // endregion // region: Apache Commons and other (similar) helper libraries diff --git a/src/main/java/org/jabref/gui/actions/StandardActions.java b/src/main/java/org/jabref/gui/actions/StandardActions.java index 7f44fb8810f..2859f552475 100644 --- a/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -75,7 +75,6 @@ public enum StandardActions implements Action { REMOTE_DB(Localization.lang("Shared database"), IconTheme.JabRefIcons.REMOTE_DATABASE), EXPORT_SELECTED(Localization.lang("Export selected entries"), KeyBinding.EXPORT_SELECTED), CONNECT_TO_SHARED_DB(Localization.lang("Connect to shared database"), IconTheme.JabRefIcons.CONNECT_DB), - PULL_CHANGES_FROM_SHARED_DB(Localization.lang("Pull changes from shared database"), KeyBinding.PULL_CHANGES_FROM_SHARED_DATABASE), CLOSE_LIBRARY(Localization.lang("Close library"), Localization.lang("Close the current library"), IconTheme.JabRefIcons.CLOSE, KeyBinding.CLOSE_DATABASE), CLOSE_OTHER_LIBRARIES(Localization.lang("Close others"), Localization.lang("Close other libraries"), IconTheme.JabRefIcons.CLOSE), CLOSE_ALL_LIBRARIES(Localization.lang("Close all"), Localization.lang("Close all libraries"), IconTheme.JabRefIcons.CLOSE), diff --git a/src/main/java/org/jabref/gui/frame/MainMenu.java b/src/main/java/org/jabref/gui/frame/MainMenu.java index 96ab72d70d4..3b7e00615c2 100644 --- a/src/main/java/org/jabref/gui/frame/MainMenu.java +++ b/src/main/java/org/jabref/gui/frame/MainMenu.java @@ -60,7 +60,6 @@ import org.jabref.gui.push.PushToApplicationCommand; import org.jabref.gui.search.RebuildFulltextSearchIndexAction; import org.jabref.gui.shared.ConnectToSharedDatabaseCommand; -import org.jabref.gui.shared.PullChangesFromSharedAction; import org.jabref.gui.sidepane.SidePane; import org.jabref.gui.sidepane.SidePaneType; import org.jabref.gui.slr.EditExistingStudyAction; @@ -170,8 +169,7 @@ private void createMenu() { new SeparatorMenuItem(), factory.createSubMenu(StandardActions.REMOTE_DB, - factory.createMenuItem(StandardActions.CONNECT_TO_SHARED_DB, new ConnectToSharedDatabaseCommand(frame, dialogService)), - factory.createMenuItem(StandardActions.PULL_CHANGES_FROM_SHARED_DB, new PullChangesFromSharedAction(stateManager))), + factory.createMenuItem(StandardActions.CONNECT_TO_SHARED_DB, new ConnectToSharedDatabaseCommand(frame, dialogService))), new SeparatorMenuItem(), diff --git a/src/main/java/org/jabref/gui/keyboard/KeyBinding.java b/src/main/java/org/jabref/gui/keyboard/KeyBinding.java index d0cb6100338..541f39a658e 100644 --- a/src/main/java/org/jabref/gui/keyboard/KeyBinding.java +++ b/src/main/java/org/jabref/gui/keyboard/KeyBinding.java @@ -90,7 +90,6 @@ public enum KeyBinding { SHOW_PREFS("Preferences", Localization.lang("Preferences"), "ctrl+,", KeyBindingCategory.FILE), OPEN_URL_OR_DOI("Open URL or DOI", Localization.lang("Open URL or DOI"), "F3", KeyBindingCategory.TOOLS), PASTE("Paste", Localization.lang("Paste"), "ctrl+V", KeyBindingCategory.EDIT), - PULL_CHANGES_FROM_SHARED_DATABASE("Pull changes from shared database", Localization.lang("Pull changes from shared database"), "ctrl+shift+R", KeyBindingCategory.FILE), PREVIOUS_PREVIEW_LAYOUT("Previous preview layout", Localization.lang("Previous preview layout"), "shift+F9", KeyBindingCategory.VIEW), PREVIOUS_LIBRARY("Previous library", Localization.lang("Previous library"), "ctrl+PAGE_UP", KeyBindingCategory.VIEW), SCROLL_TO_NEXT_MATCH_CATEGORY("Scroll to next match category", Localization.lang("Scroll to next match category"), "right", KeyBindingCategory.VIEW), diff --git a/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java b/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java index dc4993e67ab..0b469a3cecf 100644 --- a/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java +++ b/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java @@ -18,7 +18,6 @@ public Map getKeyBindings() { final Map keyBindings = new HashMap<>(); // Clear conflicting default presets - keyBindings.put(KeyBinding.PULL_CHANGES_FROM_SHARED_DATABASE, ""); keyBindings.put(KeyBinding.COPY_PREVIEW, ""); // Add new entry presets diff --git a/src/main/java/org/jabref/gui/shared/PullChangesFromSharedAction.java b/src/main/java/org/jabref/gui/shared/PullChangesFromSharedAction.java deleted file mode 100644 index d19090cf367..00000000000 --- a/src/main/java/org/jabref/gui/shared/PullChangesFromSharedAction.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.jabref.gui.shared; - -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.ActionHelper; -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.logic.shared.DatabaseSynchronizer; - -public class PullChangesFromSharedAction extends SimpleCommand { - - private final StateManager stateManager; - - public PullChangesFromSharedAction(StateManager stateManager) { - this.stateManager = stateManager; - - this.executable.bind(ActionHelper.needsDatabase(stateManager).and(ActionHelper.needsSharedDatabase(stateManager))); - } - - public void execute() { - stateManager.getActiveDatabase().ifPresent(databaseContext -> { - DatabaseSynchronizer dbmsSynchronizer = databaseContext.getDBMSSynchronizer(); - dbmsSynchronizer.pullChanges(); - }); - } -} diff --git a/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java b/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java index 4b8f625ea15..931b19bc2b7 100644 --- a/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java +++ b/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java @@ -31,7 +31,7 @@ private MetaDataSerializer() { } /** - * Writes all data in the format <key, serialized data>. + * Writes all data in the format {@code }. */ public static Map getSerializedStringMap(MetaData metaData, GlobalCitationKeyPatterns globalCiteKeyPatterns) { diff --git a/src/main/java/org/jabref/logic/shared/DBMSConnection.java b/src/main/java/org/jabref/logic/shared/DBMSConnection.java index 2d1438f6136..3d4a755ddf5 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSConnection.java +++ b/src/main/java/org/jabref/logic/shared/DBMSConnection.java @@ -9,6 +9,7 @@ import org.jabref.logic.l10n.Localization; import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -19,6 +20,12 @@ public class DBMSConnection implements DatabaseConnection { private final Connection connection; private final DBMSConnectionProperties properties; + @VisibleForTesting + public DBMSConnection(Connection connection) { + this.connection = connection; + this.properties = null; + } + public DBMSConnection(DBMSConnectionProperties connectionProperties) throws SQLException, InvalidDBMSConnectionPropertiesException { if (!connectionProperties.isValid()) { throw new InvalidDBMSConnectionPropertiesException(); @@ -38,7 +45,7 @@ public DBMSConnection(DBMSConnectionProperties connectionProperties) throws SQLE } } catch (SQLException e) { // Some systems like PostgreSQL retrieves 0 to every exception. - // Therefore a stable error determination is not possible. + // Therefore, a stable error determination is not possible. LOGGER.error("Could not connect to database: {} - Error code: {}", e.getMessage(), e.getErrorCode(), e); throw e; } diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index d7ac69ee6f9..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; @@ -57,18 +58,13 @@ protected DBMSProcessor(DatabaseConnection dbmsConnection) { */ public boolean checkBaseIntegrity() throws SQLException { boolean databasePassesIntegrityCheck = false; - DBMSType type = this.connectionProperties.getType(); Map metadata = getSharedMetaData(); - if (type == DBMSType.POSTGRESQL || type == DBMSType.MYSQL) { - String metadataVersion = metadata.get(MetaData.VERSION_DB_STRUCT); - if (metadataVersion != null) { - int VERSION_DB_STRUCT = Integer.parseInt(metadata.getOrDefault(MetaData.VERSION_DB_STRUCT, "").replace(";", "")); - if (VERSION_DB_STRUCT == getCURRENT_VERSION_DB_STRUCT()) { - databasePassesIntegrityCheck = true; - } + String metadataVersion = metadata.get(MetaData.VERSION_DB_STRUCT); + if (metadataVersion != null) { + int VERSION_DB_STRUCT = Integer.parseInt(metadata.getOrDefault(MetaData.VERSION_DB_STRUCT, "").replace(";", "")); + if (VERSION_DB_STRUCT == getCURRENT_VERSION_DB_STRUCT()) { + databasePassesIntegrityCheck = true; } - } else { - databasePassesIntegrityCheck = checkTableAvailability("ENTRY", "FIELD", "METADATA"); } return databasePassesIntegrityCheck; } @@ -134,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(); /** @@ -178,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)))); @@ -206,7 +188,7 @@ protected void insertIntoEntryTable(List bibEntries) { } } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } @@ -227,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()) { @@ -250,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(); @@ -288,10 +266,11 @@ protected void insertIntoFieldTable(List bibEntries) { fieldsCompleted += 1; } } + // TODO: This could grow too large for a single query preparedFieldStatement.executeUpdate(); } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } @@ -302,6 +281,7 @@ protected void insertIntoFieldTable(List bibEntries) { * @throws SQLException in case of error */ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQLException { + // FIXME: either two connections (one with auto commit and one without) or better auto commit state - this line here can lead to issues if autocommit is required in a parallel thread connection.setAutoCommit(false); // disable auto commit due to transaction try { @@ -321,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()); @@ -345,7 +319,7 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL throw new OfflineLockException(localBibEntry, sharedBibEntry); } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); connection.rollback(); // undo changes made in current transaction } finally { connection.setAutoCommit(true); // enable auto commit mode again @@ -359,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)) { @@ -389,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)) { @@ -404,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)) { @@ -422,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)) { @@ -455,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("?)"); @@ -513,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++) { @@ -551,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 @@ -567,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(); } @@ -584,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()) { @@ -606,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")); } @@ -623,41 +568,18 @@ public Map getSharedMetaData() { * @param data JabRef meta data as map */ public void setSharedMetaData(Map data) throws SQLException { - StringBuilder updateQuery = new StringBuilder() - .append("UPDATE ") - .append(escape_Table("METADATA")) - .append(" SET ") - .append(escape("VALUE")) - .append(" = ? ") - .append(" WHERE ") - .append(escape("KEY")) - .append(" = ?"); - - StringBuilder insertQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("METADATA")) - .append("(") - .append(escape("KEY")) - .append(", ") - .append(escape("VALUE")) - .append(") VALUES(?, ?)"); - - for (Map.Entry metaEntry : data.entrySet()) { - try (PreparedStatement updateStatement = connection.prepareStatement(updateQuery.toString())) { - updateStatement.setString(2, metaEntry.getKey()); - updateStatement.setString(1, metaEntry.getValue()); - if (updateStatement.executeUpdate() == 0) { - // No rows updated -> insert data - try (PreparedStatement insertStatement = connection.prepareStatement(insertQuery.toString())) { - insertStatement.setString(1, metaEntry.getKey()); - insertStatement.setString(2, metaEntry.getValue()); - insertStatement.executeUpdate(); - } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); - } - } - } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + 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()) { + statement.setString(1, metaEntry.getKey()); + statement.setString(2, metaEntry.getValue()); + statement.executeUpdate(); } } } @@ -666,15 +588,7 @@ public void setSharedMetaData(Map data) throws SQLException { * Returns a new instance of the abstract type {@link DBMSProcessor} */ public static DBMSProcessor getProcessorInstance(DatabaseConnection connection) { - DBMSType type = connection.getProperties().getType(); - if (type == DBMSType.MYSQL) { - return new MySQLProcessor(connection); - } else if (type == DBMSType.POSTGRESQL) { - return new PostgreSQLProcessor(connection); - } else if (type == DBMSType.ORACLE) { - return new OracleProcessor(connection); - } - return null; // can never happen except new types were added without updating this method. + return new PostgreSQLProcessor(connection); } public DatabaseConnectionProperties getDBMSConnectionProperties() { diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index d388de0b884..10aa9576b97 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -12,7 +12,6 @@ import org.jabref.logic.bibtex.FieldPreferences; import org.jabref.logic.citationkeypattern.GlobalCitationKeyPatterns; -import org.jabref.logic.exporter.BibDatabaseWriter; import org.jabref.logic.exporter.MetaDataSerializer; import org.jabref.logic.importer.ParseException; import org.jabref.logic.importer.util.MetaDataParser; @@ -56,7 +55,7 @@ public class DBMSSynchronizer implements DatabaseSynchronizer { private final GlobalCitationKeyPatterns globalCiteKeyPattern; private final FieldPreferences fieldPreferences; private final FileUpdateMonitor fileMonitor; - private Optional lastEntryChanged; + private Optional lastEntryChanged_REMOVEME; public DBMSSynchronizer(BibDatabaseContext bibDatabaseContext, Character keywordSeparator, FieldPreferences fieldPreferences, @@ -69,7 +68,7 @@ public DBMSSynchronizer(BibDatabaseContext bibDatabaseContext, Character keyword this.eventBus = new EventBus(); this.keywordSeparator = keywordSeparator; this.globalCiteKeyPattern = Objects.requireNonNull(globalCiteKeyPattern); - this.lastEntryChanged = Optional.empty(); + this.lastEntryChanged_REMOVEME = Optional.empty(); } /** @@ -80,32 +79,36 @@ public void listen(EntriesAddedEvent event) { // While synchronizing the local database (see synchronizeLocalDatabase() below), some EntriesEvents may be posted. // In this case DBSynchronizer should not try to insert the bibEntry entry again (but it would not harm). if (isEventSourceAccepted(event) && checkCurrentConnection()) { + // TODO: Make use of org.jabref.model.metadata.event.MetaDataChangedEvent (and also do Added/Removed there) synchronizeLocalMetaData(); + pullWithLastEntry(); synchronizeLocalDatabase(); dbmsProcessor.insertEntries(event.getBibEntries()); // Reset last changed entry because it just has already been synchronized -> Why necessary? - lastEntryChanged = Optional.empty(); + lastEntryChanged_REMOVEME = Optional.empty(); } } /** * Listening method. Updates an existing shared {@link BibEntry}. + * + * In JabRef (UI), the field is modified */ @Subscribe public void listen(FieldChangedEvent event) { + if (event.isFilteredOut() || !isEventSourceAccepted(event) || !checkCurrentConnection()) { + return; + } + BibEntry bibEntry = event.getBibEntry(); // While synchronizing the local database (see synchronizeLocalDatabase() below), some EntriesEvents may be posted. // In this case DBSynchronizer should not try to update the bibEntry entry again (but it would not harm). - if (isPresentLocalBibEntry(bibEntry) && isEventSourceAccepted(event) && checkCurrentConnection() && !event.isFilteredOut()) { + synchronizeLocalMetaData(); pullWithLastEntry(); synchronizeSharedEntry(bibEntry); synchronizeLocalDatabase(); // Pull changes for the case that there were some - } else { - // Set new BibEntry that has been changed last - lastEntryChanged = Optional.of(bibEntry); - } } /** @@ -130,9 +133,7 @@ public void listen(EntriesRemovedEvent event) { public void listen(MetaDataChangedEvent event) { if (checkCurrentConnection()) { synchronizeSharedMetaData(event.getMetaData(), globalCiteKeyPattern); - synchronizeLocalDatabase(); - applyMetaData(); - dbmsProcessor.notifyClients(); + // TODO: applyMetaData(); } } @@ -247,7 +248,9 @@ public void synchronizeSharedEntry(BibEntry bibEntry) { return; } try { - BibDatabaseWriter.applySaveActions(bibEntry, metaData, fieldPreferences); // perform possibly existing save actions + // TODO: Either reenable (compare with fix https://github.com/JabRef/jabref/pull/11282) or write user documentation that "cleanup actions" should be used or introduce SQL databse cleanup (stored procedure, ...) + // BibDatabaseWriter.applySaveActions(bibEntry, metaData, fieldPreferences); // perform possibly existing save actions + dbmsProcessor.updateEntry(bibEntry); } catch (OfflineLockException exception) { eventBus.post(new UpdateRefusedEvent(bibDatabaseContext, exception.getLocalBibEntry(), exception.getSharedBibEntry())); @@ -283,8 +286,9 @@ private void synchronizeSharedMetaData(MetaData data, GlobalCitationKeyPatterns } try { dbmsProcessor.setSharedMetaData(MetaDataSerializer.getSerializedStringMap(data, globalCiteKeyPattern)); + // TODO: synchronize with server - currently, only data is written to the server } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } @@ -298,13 +302,15 @@ public void applyMetaData() { for (BibEntry bibEntry : bibDatabase.getEntries()) { try { // synchronize only if changes were present - if (!BibDatabaseWriter.applySaveActions(bibEntry, metaData, fieldPreferences).isEmpty()) { + // TODO: apply save actions + // if (!BibDatabaseWriter.applySaveActions(bibEntry, metaData, fieldPreferences).isEmpty()) { + if (false) { dbmsProcessor.updateEntry(bibEntry); } } catch (OfflineLockException exception) { eventBus.post(new UpdateRefusedEvent(bibDatabaseContext, exception.getLocalBibEntry(), exception.getSharedBibEntry())); } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } } @@ -327,7 +333,7 @@ public void pullChanges() { * Synchronizes local BibEntries only if last entry changes still remain */ public void pullLastEntryChanges() { - if (lastEntryChanged.isPresent()) { + if (lastEntryChanged_REMOVEME.isPresent()) { if (!checkCurrentConnection()) { return; } @@ -342,10 +348,10 @@ public void pullLastEntryChanges() { * Synchronizes local BibEntries and pulls remaining last entry changes */ private void pullWithLastEntry() { - if (lastEntryChanged.isPresent() && isPresentLocalBibEntry(lastEntryChanged.get())) { - synchronizeSharedEntry(lastEntryChanged.get()); + if (lastEntryChanged_REMOVEME.isPresent() && isPresentLocalBibEntry(lastEntryChanged_REMOVEME.get())) { + synchronizeSharedEntry(lastEntryChanged_REMOVEME.get()); } - lastEntryChanged = Optional.empty(); + lastEntryChanged_REMOVEME = Optional.empty(); } /** @@ -396,7 +402,7 @@ public void closeSharedDatabase() { dbmsProcessor.stopNotificationListener(); currentConnection.close(); } catch (SQLException e) { - LOGGER.error("SQL Error:", e); + LOGGER.error("SQL Error", e); } } diff --git a/src/main/java/org/jabref/logic/shared/DBMSType.java b/src/main/java/org/jabref/logic/shared/DBMSType.java index 8a3717abaf0..6851e0daf6f 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSType.java +++ b/src/main/java/org/jabref/logic/shared/DBMSType.java @@ -7,9 +7,7 @@ * Enumerates all supported database systems (DBMS) by JabRef. */ public enum DBMSType { - POSTGRESQL("PostgreSQL", "org.postgresql.Driver", "jdbc:postgresql://%s:%d/%s", 5432), - MYSQL("MySQL", "org.mariadb.jdbc.Driver", "jdbc:mariadb://%s:%d/%s", 3306), - ORACLE("Oracle", "oracle.jdbc.driver.OracleDriver", "jdbc:oracle:thin:@%s:%d/%s", 1521); + POSTGRESQL("PostgreSQL", "org.postgresql.Driver", "jdbc:postgresql://%s:%d/%s", 5432); private final String type; private final String driverPath; diff --git a/src/main/java/org/jabref/logic/shared/MySQLProcessor.java b/src/main/java/org/jabref/logic/shared/MySQLProcessor.java deleted file mode 100644 index 906fbaf50e8..00000000000 --- a/src/main/java/org/jabref/logic/shared/MySQLProcessor.java +++ /dev/null @@ -1,86 +0,0 @@ -package org.jabref.logic.shared; - -import java.sql.SQLException; -import java.util.Map; - -import org.jabref.model.metadata.MetaData; - -/** - * Processes all incoming or outgoing bib data to MySQL Database and manages its structure. - */ -public class MySQLProcessor extends DBMSProcessor { - - private Integer VERSION_DB_STRUCT_DEFAULT = -1; - private Integer CURRENT_VERSION_DB_STRUCT = 1; - - public MySQLProcessor(DatabaseConnection connection) { - super(connection); - } - - /** - * Creates and sets up the needed tables and columns according to the database type. - * - * @throws SQLException - */ - @Override - public void setUp() throws SQLException { - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS `JABREF_ENTRY` (" + - "`SHARED_ID` INT(11) NOT NULL PRIMARY KEY AUTO_INCREMENT, " + - "`TYPE` VARCHAR(255) NOT NULL, " + - "`VERSION` INT(11) DEFAULT 1)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS `JABREF_FIELD` (" + - "`ENTRY_SHARED_ID` INT(11) NOT NULL, " + - "`NAME` VARCHAR(255) NOT NULL, " + - "`VALUE` TEXT DEFAULT NULL, " + - "FOREIGN KEY (`ENTRY_SHARED_ID`) REFERENCES `JABREF_ENTRY`(`SHARED_ID`) ON DELETE CASCADE)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS `JABREF_METADATA` (" + - "`KEY` varchar(255) NOT NULL," + - "`VALUE` text NOT NULL)"); - - Map metadata = getSharedMetaData(); - - if (metadata.get(MetaData.VERSION_DB_STRUCT) != null) { - try { - VERSION_DB_STRUCT_DEFAULT = Integer.valueOf(metadata.get(MetaData.VERSION_DB_STRUCT)); - } catch (Exception e) { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Integer!"); - } - } else { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Exist!"); - } - - if (VERSION_DB_STRUCT_DEFAULT < CURRENT_VERSION_DB_STRUCT) { - // We can to migrate from old table in new table - if (CURRENT_VERSION_DB_STRUCT == 1 && checkTableAvailability("ENTRY", "FIELD", "METADATA")) { - 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`"); - metadata = getSharedMetaData(); - } - - metadata.put(MetaData.VERSION_DB_STRUCT, CURRENT_VERSION_DB_STRUCT.toString()); - setSharedMetaData(metadata); - } - } - - @Override - String escape(String expression) { - return "`" + expression + "`"; - } - - @Override - String escape_Table(String expression) { - return escape("JABREF_" + expression); - } - - @Override - Integer getCURRENT_VERSION_DB_STRUCT() { - return CURRENT_VERSION_DB_STRUCT; - } -} diff --git a/src/main/java/org/jabref/logic/shared/OracleProcessor.java b/src/main/java/org/jabref/logic/shared/OracleProcessor.java deleted file mode 100644 index 3221a211808..00000000000 --- a/src/main/java/org/jabref/logic/shared/OracleProcessor.java +++ /dev/null @@ -1,221 +0,0 @@ -package org.jabref.logic.shared; - -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.stream.Collectors; - -import org.jabref.logic.shared.listener.OracleNotificationListener; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.field.Field; -import org.jabref.model.metadata.MetaData; - -import oracle.jdbc.OracleConnection; -import oracle.jdbc.OracleStatement; -import oracle.jdbc.dcn.DatabaseChangeRegistration; - -/** - * Processes all incoming or outgoing bib data to Oracle database and manages its structure. - */ -public class OracleProcessor extends DBMSProcessor { - - private OracleConnection oracleConnection; - - private OracleNotificationListener listener; - - private DatabaseChangeRegistration databaseChangeRegistration; - - private Integer VERSION_DB_STRUCT_DEFAULT = -1; - private Integer CURRENT_VERSION_DB_STRUCT = 0; - - public OracleProcessor(DatabaseConnection connection) { - super(connection); - } - - /** - * Creates and sets up the needed tables and columns according to the database type. - * - * @throws SQLException - */ - @Override - public void setUp() throws SQLException { - connection.createStatement().executeUpdate( - "CREATE TABLE \"ENTRY\" (" + - "\"SHARED_ID\" NUMBER NOT NULL, " + - "\"TYPE\" VARCHAR2(255) NULL, " + - "\"VERSION\" NUMBER DEFAULT 1, " + - "CONSTRAINT \"ENTRY_PK\" PRIMARY KEY (\"SHARED_ID\"))"); - - connection.createStatement().executeUpdate("CREATE SEQUENCE \"ENTRY_SEQ\""); - - connection.createStatement().executeUpdate("CREATE TRIGGER \"ENTRY_T\" BEFORE INSERT ON \"ENTRY\" " + - "FOR EACH ROW BEGIN SELECT \"ENTRY_SEQ\".NEXTVAL INTO :NEW.shared_id FROM DUAL; END;"); - - connection.createStatement().executeUpdate( - "CREATE TABLE \"FIELD\" (" + - "\"ENTRY_SHARED_ID\" NUMBER NOT NULL, " + - "\"NAME\" VARCHAR2(255) NOT NULL, " + - "\"VALUE\" CLOB NULL, " + - "CONSTRAINT \"ENTRY_SHARED_ID_FK\" FOREIGN KEY (\"ENTRY_SHARED_ID\") " + - "REFERENCES \"ENTRY\"(\"SHARED_ID\") ON DELETE CASCADE)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE \"METADATA\" (" + - "\"KEY\" VARCHAR2(255) NULL," + - "\"VALUE\" CLOB NOT NULL)"); - - Map metadata = getSharedMetaData(); - - if (metadata.get(MetaData.VERSION_DB_STRUCT) != null) { - try { - VERSION_DB_STRUCT_DEFAULT = Integer.valueOf(metadata.get(MetaData.VERSION_DB_STRUCT)); - } catch (Exception e) { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Integer!"); - } - } else { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Exist!"); - } - - if (VERSION_DB_STRUCT_DEFAULT < CURRENT_VERSION_DB_STRUCT) { - // We can to migrate from old table in new table - metadata.put(MetaData.VERSION_DB_STRUCT, CURRENT_VERSION_DB_STRUCT.toString()); - setSharedMetaData(metadata); - } - } - - @Override - String escape(String expression) { - return expression; - } - - @Override - String escape_Table(String expression) { - return escape(expression); - } - - @Override - Integer getCURRENT_VERSION_DB_STRUCT() { - return CURRENT_VERSION_DB_STRUCT; - } - - @Override - public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) { - this.listener = new OracleNotificationListener(dbmsSynchronizer); - - try { - oracleConnection = (OracleConnection) connection; - - Properties properties = new Properties(); - properties.setProperty(OracleConnection.DCN_NOTIFY_ROWIDS, "true"); - properties.setProperty(OracleConnection.DCN_QUERY_CHANGE_NOTIFICATION, "true"); - - databaseChangeRegistration = oracleConnection.registerDatabaseChangeNotification(properties); - databaseChangeRegistration.addListener(listener); - - try (Statement statement = oracleConnection.createStatement()) { - ((OracleStatement) statement).setDatabaseChangeRegistration(databaseChangeRegistration); - StringBuilder selectQuery = new StringBuilder() - .append("SELECT 1 FROM ") - .append(escape_Table("ENTRY")) - .append(", ") - .append(escape_Table("METADATA")); - // this execution registers all tables mentioned in selectQuery - statement.executeQuery(selectQuery.toString()); - } - } catch (SQLException e) { - LOGGER.error("SQL Error during starting the notification listener", e); - } - } - - @Override - protected void insertIntoEntryTable(List entries) { - try { - for (BibEntry entry : entries) { - String insertIntoEntryQuery = - "INSERT INTO " + - escape_Table("ENTRY") + - "(" + - escape("TYPE") + - ") VALUES(?)"; - - try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery, - new String[]{"SHARED_ID"})) { - - preparedEntryStatement.setString(1, entry.getType().getName()); - preparedEntryStatement.executeUpdate(); - - try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) { - if (generatedKeys.next()) { - entry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); // set generated ID locally - } - } - } - } - } catch (SQLException e) { - LOGGER.error("SQL Error during entry insertion", e); - } - } - - @Override - protected void insertIntoFieldTable(List bibEntries) { - try { - // Inserting into FIELD table - // Coerce to ArrayList in order to use List.get() - List> fields = bibEntries.stream().map(entry -> new ArrayList<>(entry.getFields())) - .collect(Collectors.toList()); - StringBuilder insertFieldQuery = new StringBuilder() - .append("INSERT ALL"); - int numFields = 0; - for (List entryFields : fields) { - numFields += entryFields.size(); - } - for (int i = 0; i < numFields; i++) { - insertFieldQuery.append(" INTO ") - .append(escape_Table("FIELD")) - .append(" (") - .append(escape("ENTRY_SHARED_ID")) - .append(", ") - .append(escape("NAME")) - .append(", ") - .append(escape("VALUE")) - .append(") VALUES (?, ?, ?)"); - } - insertFieldQuery.append(" SELECT * FROM DUAL"); - try (PreparedStatement preparedFieldStatement = connection.prepareStatement(insertFieldQuery.toString())) { - int fieldsCompleted = 0; - for (int entryIndex = 0; entryIndex < fields.size(); entryIndex++) { - for (int entryFieldsIndex = 0; entryFieldsIndex < fields.get(entryIndex).size(); entryFieldsIndex++) { - // columnIndex starts with 1 - preparedFieldStatement.setInt((3 * fieldsCompleted) + 1, bibEntries.get(entryIndex).getSharedBibEntryData().getSharedID()); - preparedFieldStatement.setString((3 * fieldsCompleted) + 2, fields.get(entryIndex).get(entryFieldsIndex).getName()); - preparedFieldStatement.setString((3 * fieldsCompleted) + 3, bibEntries.get(entryIndex).getField(fields.get(entryIndex).get(entryFieldsIndex)).get()); - fieldsCompleted += 1; - } - } - preparedFieldStatement.executeUpdate(); - } - } catch (SQLException e) { - LOGGER.error("SQL Error during field insertion", e); - } - } - - @Override - public void stopNotificationListener() { - try { - oracleConnection.unregisterDatabaseChangeNotification(databaseChangeRegistration); - oracleConnection.close(); - } catch (SQLException e) { - LOGGER.error("SQL Error during stopping the notification listener", e); - } - } - - @Override - public void notifyClients() { - // Do nothing because Oracle triggers notifications automatically. - } -} diff --git a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java index 21b04d09c6a..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); @@ -35,31 +38,43 @@ public PostgreSQLProcessor(DatabaseConnection connection) { */ @Override public void setUp() throws SQLException { - if (CURRENT_VERSION_DB_STRUCT == 1 && checkTableAvailability("ENTRY", "FIELD", "METADATA")) { // checkTableAvailability does not distinguish if same table name exists in different schemas // VERSION_DB_STRUCT_DEFAULT must be forced 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(); @@ -68,39 +83,67 @@ public void setUp() throws SQLException { // replace semicolon so we can parse it VERSION_DB_STRUCT_DEFAULT = Integer.parseInt(metadata.get(MetaData.VERSION_DB_STRUCT).replace(";", "")); } catch (Exception e) { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Integer!"); + LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] is not an Integer."); } } else { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Exist!"); + LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] does not exist."); } + String upsertMetadata = """ + CREATE OR REPLACE FUNCTION upsert_metadata(key TEXT, value TEXT) RETURNS VOID AS $$ + DECLARE + existing_value TEXT; + BEGIN + -- Check if the key already exists and get its current value + SELECT VALUE INTO existing_value FROM METADATA WHERE KEY = key; + + -- Perform the upsert + INSERT INTO METADATA (KEY, VALUE) + VALUES (key, value) + ON CONFLICT (KEY) + DO UPDATE SET VALUE = EXCLUDED.VALUE; + + -- Notify only if the value has changed + IF existing_value IS DISTINCT FROM value THEN + PERFORM pg_notify('metadata_update', json_build_object('key', key, 'value', value)::TEXT); + END IF; + END; + $$ LANGUAGE plpgsql; + """; + connection.createStatement().executeUpdate(upsertMetadata); + if (VERSION_DB_STRUCT_DEFAULT < CURRENT_VERSION_DB_STRUCT) { - // We can to migrate from old table in new table + // 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()); @@ -123,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/main/java/org/jabref/logic/shared/listener/OracleNotificationListener.java b/src/main/java/org/jabref/logic/shared/listener/OracleNotificationListener.java deleted file mode 100644 index 51aec6ddaaa..00000000000 --- a/src/main/java/org/jabref/logic/shared/listener/OracleNotificationListener.java +++ /dev/null @@ -1,23 +0,0 @@ -package org.jabref.logic.shared.listener; - -import org.jabref.logic.shared.DBMSSynchronizer; - -import oracle.jdbc.dcn.DatabaseChangeEvent; -import oracle.jdbc.dcn.DatabaseChangeListener; - -/** - * A listener for Oracle database notifications. - */ -public class OracleNotificationListener implements DatabaseChangeListener { - - private final DBMSSynchronizer dbmsSynchronizer; - - public OracleNotificationListener(DBMSSynchronizer dbmsSynchronizer) { - this.dbmsSynchronizer = dbmsSynchronizer; - } - - @Override - public void onDatabaseChangeNotification(DatabaseChangeEvent event) { - dbmsSynchronizer.pullChanges(); - } -} diff --git a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java b/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java index 96b8b7ccaf6..59ef9258874 100644 --- a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java +++ b/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java @@ -30,13 +30,13 @@ public PostgresSQLNotificationListener(DBMSSynchronizer dbmsSynchronizer, PGConn public void run() { stop = false; try { - // noinspection InfiniteLoopStatement - while (!stop) { - PGNotification notifications[] = pgConnection.getNotifications(); + while (!stop && !Thread.currentThread().isInterrupted()) { + PGNotification[] notifications = pgConnection.getNotifications(); if (notifications != null) { for (PGNotification notification : notifications) { if (!notification.getName().equals(DBMSProcessor.PROCESSOR_ID)) { + // Only process notifications that are not sent by this processor dbmsSynchronizer.pullChanges(); } } diff --git a/src/main/java/org/jabref/model/database/BibDatabase.java b/src/main/java/org/jabref/model/database/BibDatabase.java index a5d1d461851..2bd7c84b86c 100644 --- a/src/main/java/org/jabref/model/database/BibDatabase.java +++ b/src/main/java/org/jabref/model/database/BibDatabase.java @@ -49,14 +49,11 @@ public class BibDatabase { private static final Logger LOGGER = LoggerFactory.getLogger(BibDatabase.class); private static final Pattern RESOLVE_CONTENT_PATTERN = Pattern.compile(".*#[^#]+#.*"); - /** - * State attributes - */ private final ObservableList entries = FXCollections.synchronizedObservableList(FXCollections.observableArrayList(BibEntry::getObservables)); - // BibEntryId to BibEntry - private final Map entriesId = new HashMap<>(); - private Map bibtexStrings = new ConcurrentHashMap<>(); + private final Map entryIdToBibEntry = new HashMap<>(); + + private Map stringToBibtexString = new ConcurrentHashMap<>(); // Not included in equals, because it is not relevant for the content of the database private final EventBus eventBus = new EventBus(); @@ -81,7 +78,7 @@ public BibDatabase(List entries) { } public BibDatabase() { - this.registerListener(new KeyChangeListener(this)); + this.registerListener(new CitationKeyListener(this)); } /** @@ -203,12 +200,12 @@ public synchronized void insertEntries(List newEntries, EntriesEventSo entry.registerListener(this); } if (newEntries.isEmpty()) { - eventBus.post(new EntriesAddedEvent(newEntries, eventSource)); + LOGGER.warn("No entries to insert"); } else { - eventBus.post(new EntriesAddedEvent(newEntries, newEntries.getFirst(), eventSource)); + eventBus.post(new EntriesAddedEvent(newEntries, eventSource)); } entries.addAll(newEntries); - newEntries.forEach(entry -> entriesId.put(entry.getId(), entry)); + newEntries.forEach(entry -> entryIdToBibEntry.put(entry.getId(), entry)); } public synchronized void removeEntry(BibEntry bibEntry) { @@ -245,7 +242,7 @@ public synchronized void removeEntries(List toBeDeleted, EntriesEventS } boolean anyRemoved = entries.removeIf(entry -> ids.contains(entry.getId())); if (anyRemoved) { - toBeDeleted.forEach(entry -> entriesId.remove(entry.getId())); + toBeDeleted.forEach(entry -> entryIdToBibEntry.remove(entry.getId())); eventBus.post(new EntriesRemovedEvent(toBeDeleted, eventSource)); } } @@ -279,11 +276,11 @@ public synchronized void addString(BibtexString string) throws KeyCollisionExcep throw new KeyCollisionException("A string with that label already exists", id); } - if (bibtexStrings.containsKey(id)) { + if (stringToBibtexString.containsKey(id)) { throw new KeyCollisionException("Duplicate BibTeX string id.", id); } - bibtexStrings.put(id, string); + stringToBibtexString.put(id, string); } /** @@ -293,7 +290,7 @@ public synchronized void addString(BibtexString string) throws KeyCollisionExcep * @param stringsToAdd The collection of strings to set */ public void setStrings(List stringsToAdd) { - bibtexStrings = new ConcurrentHashMap<>(); + stringToBibtexString = new ConcurrentHashMap<>(); stringsToAdd.forEach(this::addString); } @@ -301,7 +298,7 @@ public void setStrings(List stringsToAdd) { * Removes the string with the given id. */ public void removeString(String id) { - bibtexStrings.remove(id); + stringToBibtexString.remove(id); } /** @@ -309,7 +306,7 @@ public void removeString(String id) { * These are in no sorted order. */ public Set getStringKeySet() { - return bibtexStrings.keySet(); + return stringToBibtexString.keySet(); } /** @@ -317,14 +314,14 @@ public Set getStringKeySet() { * These are in no particular order. */ public Collection getStringValues() { - return bibtexStrings.values(); + return stringToBibtexString.values(); } /** * Returns the string with the given id. */ public BibtexString getString(String id) { - return bibtexStrings.get(id); + return stringToBibtexString.get(id); } /** @@ -338,14 +335,14 @@ public Optional getStringByName(String name) { * Returns the number of strings. */ public int getStringCount() { - return bibtexStrings.size(); + return stringToBibtexString.size(); } /** * Check if there are strings. */ public boolean hasNoStrings() { - return bibtexStrings.isEmpty(); + return stringToBibtexString.isEmpty(); } /** @@ -361,7 +358,7 @@ public void copyPreamble(BibDatabase database) { * Returns true if a string with the given label already exists. */ public synchronized boolean hasStringByName(String label) { - return bibtexStrings.values().stream().anyMatch(value -> value.getName().equals(label)); + return stringToBibtexString.values().stream().anyMatch(value -> value.getName().equals(label)); } /** @@ -391,7 +388,7 @@ public List getUsedStrings(Collection entries) { } } - return allUsedIds.stream().map(bibtexStrings::get).toList(); + return allUsedIds.stream().map(stringToBibtexString::get).toList(); } /** @@ -453,7 +450,7 @@ private String resolveString(String label, Set usedIds, Set allU Objects.requireNonNull(usedIds); Objects.requireNonNull(allUsedIds); - for (BibtexString string : bibtexStrings.values()) { + for (BibtexString string : stringToBibtexString.values()) { if (string.getName().equalsIgnoreCase(label)) { // First check if this string label has been resolved // earlier in this recursion. If so, we have a @@ -657,7 +654,7 @@ public int indexOf(BibEntry bibEntry) { } public BibEntry getEntryById(String id) { - return entriesId.get(id); + return entryIdToBibEntry.get(id); } @Override @@ -669,7 +666,7 @@ public boolean equals(Object o) { return false; } return Objects.equals(entries, that.entries) - && Objects.equals(bibtexStrings, that.bibtexStrings) + && Objects.equals(stringToBibtexString, that.stringToBibtexString) && Objects.equals(preamble, that.preamble) && Objects.equals(epilog, that.epilog) && Objects.equals(sharedDatabaseID, that.sharedDatabaseID) @@ -678,6 +675,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(entries, bibtexStrings, preamble, epilog, sharedDatabaseID, newLineSeparator); + return Objects.hash(entries, stringToBibtexString, preamble, epilog, sharedDatabaseID, newLineSeparator); } } diff --git a/src/main/java/org/jabref/model/database/KeyChangeListener.java b/src/main/java/org/jabref/model/database/CitationKeyListener.java similarity index 69% rename from src/main/java/org/jabref/model/database/KeyChangeListener.java rename to src/main/java/org/jabref/model/database/CitationKeyListener.java index 6dc601971f1..e5607525b8c 100644 --- a/src/main/java/org/jabref/model/database/KeyChangeListener.java +++ b/src/main/java/org/jabref/model/database/CitationKeyListener.java @@ -3,7 +3,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Optional; import org.jabref.model.database.event.EntriesRemovedEvent; import org.jabref.model.entry.BibEntry; @@ -13,34 +12,34 @@ import org.jabref.model.entry.field.InternalField; import com.google.common.eventbus.Subscribe; +import org.jspecify.annotations.Nullable; -public class KeyChangeListener { +/** + * Updates references of citation keys if the citation key of an entry is changed. + */ +public class CitationKeyListener { private final BibDatabase database; - public KeyChangeListener(BibDatabase database) { + public CitationKeyListener(BibDatabase database) { this.database = database; } @Subscribe public void listen(FieldChangedEvent event) { if (event.getField().equals(InternalField.KEY_FIELD)) { - String newKey = event.getNewValue(); - String oldKey = event.getOldValue(); - updateEntryLinks(newKey, oldKey); + updateEntryLinks(event.getOldValue(), event.getNewValue()); } } @Subscribe public void listen(EntriesRemovedEvent event) { - List entries = event.getBibEntries(); - for (BibEntry entry : entries) { - Optional citeKey = entry.getCitationKey(); - citeKey.ifPresent(oldkey -> updateEntryLinks(null, oldkey)); - } + event.getBibEntries().stream() + .forEach(entry -> entry.getCitationKey() + .ifPresent(oldkey -> updateEntryLinks(oldkey, null))); } - private void updateEntryLinks(String newKey, String oldKey) { + private void updateEntryLinks(String oldKey, @Nullable String newKey) { for (BibEntry entry : database.getEntries()) { entry.getFields(field -> field.getProperties().contains(FieldProperty.SINGLE_ENTRY_LINK)) .forEach(field -> { @@ -50,13 +49,16 @@ private void updateEntryLinks(String newKey, String oldKey) { entry.getFields(field -> field.getProperties().contains(FieldProperty.MULTIPLE_ENTRY_LINK)) .forEach(field -> { String fieldContent = entry.getField(field).orElseThrow(); - replaceKeyInMultiplesKeyField(newKey, oldKey, entry, field, fieldContent); + replaceKeyInMultiplesKeyField(entry, field, fieldContent, oldKey, newKey); }); } } - private void replaceKeyInMultiplesKeyField(String newKey, String oldKey, BibEntry entry, Field field, String fieldContent) { - List keys = new ArrayList<>(Arrays.asList(fieldContent.split(","))); + /** + * @param newKey The new key. If null, the key is removed. + */ + private void replaceKeyInMultiplesKeyField(BibEntry entry, Field field, String fieldContent, String oldKey, @Nullable String newKey) { + List keys = new ArrayList<>(Arrays.asList(fieldContent.split(BibEntry.ENTRY_LINK_SEPARATOR))); int index = keys.indexOf(oldKey); if (index != -1) { if (newKey == null) { diff --git a/src/main/java/org/jabref/model/database/event/EntriesAddedEvent.java b/src/main/java/org/jabref/model/database/event/EntriesAddedEvent.java index 35e51ee5ed1..824ce4046b9 100644 --- a/src/main/java/org/jabref/model/database/event/EntriesAddedEvent.java +++ b/src/main/java/org/jabref/model/database/event/EntriesAddedEvent.java @@ -12,27 +12,15 @@ */ public class EntriesAddedEvent extends EntriesEvent { - // firstEntry used by listeners that used to listen to AllInsertsFinishedEvent - // final? private final BibEntry firstEntry; - /** - * @param bibEntries the entries which are being added - * @param firstEntry the first entry being added - */ - - public EntriesAddedEvent(List bibEntries, BibEntry firstEntry, EntriesEventSource location) { - super(bibEntries, location); - this.firstEntry = firstEntry; - } - - /** - * @param bibEntries List of BibEntry objects which are being added. - * @param location Location affected by this event - */ public EntriesAddedEvent(List bibEntries, EntriesEventSource location) { super(bibEntries, location); - this.firstEntry = null; + + // The event makes only sense if there is at least one entry + assert !bibEntries.isEmpty(); + + this.firstEntry = bibEntries.getFirst(); } public BibEntry getFirstEntry() { diff --git a/src/main/java/org/jabref/model/database/event/EntriesRemovedEvent.java b/src/main/java/org/jabref/model/database/event/EntriesRemovedEvent.java index e252132f82a..8061fcadcdc 100644 --- a/src/main/java/org/jabref/model/database/event/EntriesRemovedEvent.java +++ b/src/main/java/org/jabref/model/database/event/EntriesRemovedEvent.java @@ -7,23 +7,11 @@ import org.jabref.model.entry.event.EntriesEventSource; /** - * EntriesRemovedEvent is fired when at least one BibEntry is being removed + * EntriesRemovedEvent is fired when at least one {@link BibEntry} is being removed * from the database. */ public class EntriesRemovedEvent extends EntriesEvent { - - /** - * @param bibEntries List of BibEntry objects which are being removed. - */ - public EntriesRemovedEvent(List bibEntries) { - super(bibEntries); - } - - /** - * @param bibEntries List of BibEntry objects which are being removed. - * @param location Location affected by this event - */ public EntriesRemovedEvent(List bibEntries, EntriesEventSource location) { super(bibEntries, location); } diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index f0d2d43689e..8cab3df558a 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -96,7 +96,11 @@ public class BibEntry implements Cloneable { public static final EntryType DEFAULT_TYPE = StandardEntryType.Misc; + + public static final String ENTRY_LINK_SEPARATOR = ","; + private static final Logger LOGGER = LoggerFactory.getLogger(BibEntry.class); + private final SharedBibEntryData sharedBibEntryData; /** @@ -373,10 +377,7 @@ public String getId() { @VisibleForTesting public void setId(String id) { Objects.requireNonNull(id, "Every BibEntry must have an ID"); - - String oldId = this.id; - - eventBus.post(new FieldChangedEvent(this, InternalField.INTERNAL_ID_FIELD, id, oldId)); + eventBus.post(new FieldChangedEvent(this, InternalField.INTERNAL_ID_FIELD, this.id, id)); this.id = id; changed = true; } diff --git a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java index c7d6255a813..51d0d6606e2 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -15,18 +15,17 @@ public class FieldChangedEvent extends EntryChangedEvent { private int majorCharacterChange = 0; /** + * @param location Location affected by this event * @param bibEntry Affected BibEntry object * @param field Name of field which has been changed * @param oldValue old field value * @param newValue new field value - * @param location Location affected by this event */ - public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String oldValue, - EntriesEventSource location) { + public FieldChangedEvent(EntriesEventSource location, BibEntry bibEntry, Field field, String oldValue, String newValue) { super(bibEntry, location); this.field = field; - this.newValue = newValue; this.oldValue = oldValue; + this.newValue = newValue; this.majorCharacterChange = computeMajorCharacterChange(oldValue, newValue); } @@ -35,7 +34,7 @@ public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String * @param field Name of field which has been changed * @param newValue new field value */ - public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String oldValue) { + public FieldChangedEvent(BibEntry bibEntry, Field field, String oldValue, String newValue) { super(bibEntry); this.field = field; this.newValue = newValue; @@ -58,7 +57,7 @@ public FieldChangedEvent(FieldChange fieldChange) { this(fieldChange, EntriesEventSource.LOCAL); } - private int computeMajorCharacterChange(String oldValue, String newValue) { + private static int computeMajorCharacterChange(String oldValue, String newValue) { // We do == because of performance reasons if (oldValue == newValue) { return 0; diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 454a38cf4a6..8e4befbde7b 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -699,7 +699,6 @@ Selected\ Layouts\ can\ not\ be\ empty=Selected Layouts can not be empty Reset\ default\ preview\ style=Reset default preview style Previous\ entry=Previous entry Problem\ with\ parsing\ entry=Problem with parsing entry -Pull\ changes\ from\ shared\ database=Pull changes from shared database Pushed\ citations\ to\ %0=Pushed citations to %0 diff --git a/src/test/java/org/jabref/logic/shared/ConnectorTest.java b/src/test/java/org/jabref/logic/shared/ConnectorTest.java index fabcf68b1b1..d87d407798f 100644 --- a/src/test/java/org/jabref/logic/shared/ConnectorTest.java +++ b/src/test/java/org/jabref/logic/shared/ConnectorTest.java @@ -1,31 +1,43 @@ package org.jabref.logic.shared; +import java.io.IOException; +import java.sql.Connection; +import java.sql.DriverManager; import java.sql.SQLException; -import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; import org.jabref.testutils.category.DatabaseTest; +import io.zonky.test.db.postgres.embedded.EmbeddedPostgres; + /** * Stores the credentials for the test systems */ @DatabaseTest -public class ConnectorTest { +public class ConnectorTest implements AutoCloseable { + + private EmbeddedPostgres postgres; + private Connection connection; - public static DBMSConnection getTestDBMSConnection(DBMSType dbmsType) throws SQLException, InvalidDBMSConnectionPropertiesException { - DBMSConnectionProperties properties = getTestConnectionProperties(dbmsType); - return new DBMSConnection(properties); + /** + * Fires up a new postgres + */ + public DBMSConnection getTestDBMSConnection() throws SQLException, IOException { + postgres = EmbeddedPostgres.builder().start(); + String url = postgres.getJdbcUrl("postgres", "postgres"); + connection = DriverManager.getConnection(url, "postgres", "postgres"); + return new DBMSConnection(connection); } - public static DBMSConnectionProperties getTestConnectionProperties(DBMSType dbmsType) { - switch (dbmsType) { - case MYSQL: - return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("127.0.0.1").setPort(3800).setDatabase("jabref").setUser("root").setPassword("root").setUseSSL(false).setAllowPublicKeyRetrieval(true).createDBMSConnectionProperties(); - case POSTGRESQL: - return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("localhost").setPort(dbmsType.getDefaultPort()).setDatabase("postgres").setUser("postgres").setPassword("postgres").setUseSSL(false).createDBMSConnectionProperties(); - case ORACLE: - return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("localhost").setPort(32118).setDatabase("jabref").setUser("jabref").setPassword("jabref").setUseSSL(false).createDBMSConnectionProperties(); - default: - return new DBMSConnectionPropertiesBuilder().createDBMSConnectionProperties(); + /** + * Closes the connection and shuts down postgres + */ + @Override + public void close() throws Exception { + if (connection != null) { + connection.close(); + } + if (postgres != null) { + postgres.close(); } } } diff --git a/src/test/java/org/jabref/logic/shared/DBMSConnectionPropertiesTest.java b/src/test/java/org/jabref/logic/shared/DBMSConnectionPropertiesTest.java deleted file mode 100644 index 5f7e6e983b8..00000000000 --- a/src/test/java/org/jabref/logic/shared/DBMSConnectionPropertiesTest.java +++ /dev/null @@ -1,20 +0,0 @@ -package org.jabref.logic.shared; - -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -class DBMSConnectionPropertiesTest { - - @Test - void urlForMySqlDoesNotIncludeSslConfig() { - DBMSConnectionProperties connectionProperties = new DBMSConnectionPropertiesBuilder().setType(DBMSType.MYSQL).setHost("localhost").setPort(3108).setDatabase("jabref").setUser("user").setPassword("password").setUseSSL(false).setAllowPublicKeyRetrieval(true).setServerTimezone("").createDBMSConnectionProperties(); - assertEquals("jdbc:mariadb://localhost:3108/jabref", connectionProperties.getUrl()); - } - - @Test - void urlForOracle() { - DBMSConnectionProperties connectionProperties = new DBMSConnectionPropertiesBuilder().setType(DBMSType.ORACLE).setHost("localhost").setPort(3108).setDatabase("jabref").setUser("user").setPassword("password").setUseSSL(false).setServerTimezone("").createDBMSConnectionProperties(); - assertEquals("jdbc:oracle:thin:@localhost:3108/jabref", connectionProperties.getUrl()); - } -} diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 8f0b4625daa..1b6a04ee9f9 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -38,20 +38,20 @@ class DBMSProcessorTest { private DBMSConnection dbmsConnection; private DBMSProcessor dbmsProcessor; - private DBMSType dbmsType; + private ConnectorTest connectorTest; @BeforeEach void setup() throws Exception { - this.dbmsType = TestManager.getDBMSTypeTestParameter(); - this.dbmsConnection = ConnectorTest.getTestDBMSConnection(dbmsType); - this.dbmsProcessor = DBMSProcessor.getProcessorInstance(ConnectorTest.getTestDBMSConnection(dbmsType)); + this.connectorTest = new ConnectorTest(); + this.dbmsConnection = connectorTest.getTestDBMSConnection(); + this.dbmsProcessor = DBMSProcessor.getProcessorInstance(dbmsConnection); TestManager.clearTables(this.dbmsConnection); dbmsProcessor.setupSharedDatabase(); } @AfterEach - void closeDbmsConnection() throws SQLException { - this.dbmsConnection.getConnection().close(); + void closeDbmsConnection() throws Exception { + connectorTest.close(); } @Test @@ -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/DBMSSynchronizerTest.java b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java index 5983b345ade..0a300ace8c3 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java @@ -36,6 +36,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +/** + * More tests are located at {@link org.jabref.logic.shared.SynchronizationSimulatorTest} and {@link org.jabref.logic.shared.DBMSProcessorTest}. + */ @DatabaseTest @Execution(ExecutionMode.SAME_THREAD) class DBMSSynchronizerTest { @@ -45,7 +48,7 @@ class DBMSSynchronizerTest { private final GlobalCitationKeyPatterns pattern = GlobalCitationKeyPatterns.fromPattern("[auth][year]"); private DBMSConnection dbmsConnection; private DBMSProcessor dbmsProcessor; - private DBMSType dbmsType; + private ConnectorTest connectorTest; private BibEntry createExampleBibEntry(int index) { BibEntry bibEntry = new BibEntry(StandardEntryType.Book) @@ -57,8 +60,8 @@ private BibEntry createExampleBibEntry(int index) { @BeforeEach void setup() throws Exception { - this.dbmsType = TestManager.getDBMSTypeTestParameter(); - this.dbmsConnection = ConnectorTest.getTestDBMSConnection(dbmsType); + this.connectorTest = new ConnectorTest(); + this.dbmsConnection = connectorTest.getTestDBMSConnection(); this.dbmsProcessor = DBMSProcessor.getProcessorInstance(this.dbmsConnection); TestManager.clearTables(this.dbmsConnection); this.dbmsProcessor.setupSharedDatabase(); @@ -76,8 +79,8 @@ void setup() throws Exception { } @AfterEach - void clear() { - dbmsSynchronizer.closeSharedDatabase(); + void closeDbmsConnection() throws Exception { + connectorTest.close(); } @Test @@ -100,6 +103,7 @@ void twoLocalFieldChangesAreSynchronizedCorrectly() throws Exception { expectedEntry.registerListener(dbmsSynchronizer); bibDatabase.insertEntry(expectedEntry); + expectedEntry.setField(StandardField.AUTHOR, "Brad L and Gilson"); expectedEntry.setField(StandardField.TITLE, "The micro multiplexer"); diff --git a/src/test/java/org/jabref/logic/shared/DBMSTypeTest.java b/src/test/java/org/jabref/logic/shared/DBMSTypeTest.java index 04abaf58939..5ef22cd4dec 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSTypeTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSTypeTest.java @@ -12,26 +12,11 @@ @DatabaseTest class DBMSTypeTest { - @Test - void toStringCorrectForMysql() { - assertEquals("MySQL", DBMSType.MYSQL.toString()); - } - - @Test - void toStringCorrectForOracle() { - assertEquals("Oracle", DBMSType.ORACLE.toString()); - } - @Test void toStringCorrectForPostgres() { assertEquals("PostgreSQL", DBMSType.POSTGRESQL.toString()); } - @Test - void fromStringWorksForMySQL() { - assertEquals(Optional.of(DBMSType.MYSQL), DBMSType.fromString("MySQL")); - } - @Test void fromStringWorksForPostgreSQL() { assertEquals(Optional.of(DBMSType.POSTGRESQL), DBMSType.fromString("PostgreSQL")); @@ -52,31 +37,11 @@ void fromStringWorksForUnkownString() { assertEquals(Optional.empty(), DBMSType.fromString("unknown")); } - @Test - void driverClassForMysqlIsCorrect() { - assertEquals("org.mariadb.jdbc.Driver", DBMSType.MYSQL.getDriverClassPath()); - } - - @Test - void driverClassForOracleIsCorrect() { - assertEquals("oracle.jdbc.driver.OracleDriver", DBMSType.ORACLE.getDriverClassPath()); - } - @Test void driverClassForPostgresIsCorrect() { assertEquals("org.postgresql.Driver", DBMSType.POSTGRESQL.getDriverClassPath()); } - @Test - void fromStringForMysqlReturnsCorrectValue() { - assertEquals(DBMSType.MYSQL, DBMSType.fromString("MySQL").get()); - } - - @Test - void fromStringForOracleRturnsCorrectValue() { - assertEquals(DBMSType.ORACLE, DBMSType.fromString("Oracle").get()); - } - @Test void fromStringForPostgresReturnsCorrectValue() { assertEquals(DBMSType.POSTGRESQL, DBMSType.fromString("PostgreSQL").get()); @@ -87,31 +52,11 @@ void fromStringFromInvalidStringReturnsOptionalEmpty() { assertFalse(DBMSType.fromString("XXX").isPresent()); } - @Test - void getUrlForMysqlHasCorrectFormat() { - assertEquals("jdbc:mariadb://localhost:3306/xe", DBMSType.MYSQL.getUrl("localhost", 3306, "xe")); - } - - @Test - void getUrlForOracleHasCorrectFormat() { - assertEquals("jdbc:oracle:thin:@localhost:1521/xe", DBMSType.ORACLE.getUrl("localhost", 1521, "xe")); - } - @Test void getUrlForPostgresHasCorrectFormat() { assertEquals("jdbc:postgresql://localhost:5432/xe", DBMSType.POSTGRESQL.getUrl("localhost", 5432, "xe")); } - @Test - void getDefaultPortForMysqlHasCorrectValue() { - assertEquals(3306, DBMSType.MYSQL.getDefaultPort()); - } - - @Test - void getDefaultPortForOracleHasCorrectValue() { - assertEquals(1521, DBMSType.ORACLE.getDefaultPort()); - } - @Test void getDefaultPortForPostgresHasCorrectValue() { assertEquals(5432, DBMSType.POSTGRESQL.getDefaultPort()); diff --git a/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java b/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java index 2558cbdb85f..33dfcdb3407 100644 --- a/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java +++ b/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java @@ -1,6 +1,5 @@ package org.jabref.logic.shared; -import java.sql.SQLException; import java.util.List; import javafx.collections.FXCollections; @@ -37,6 +36,8 @@ class SynchronizationSimulatorTest { private BibDatabaseContext clientContextB; private SynchronizationEventListenerTest eventListenerB; // used to monitor occurring events private final GlobalCitationKeyPatterns pattern = GlobalCitationKeyPatterns.fromPattern("[auth][year]"); + private ConnectorTest connectorTestA; + private ConnectorTest connectorTestB; private BibEntry getBibEntryExample(int index) { return new BibEntry(StandardEntryType.InProceedings) @@ -49,7 +50,8 @@ private BibEntry getBibEntryExample(int index) { @BeforeEach void setup() throws Exception { - DBMSConnection dbmsConnection = ConnectorTest.getTestDBMSConnection(TestManager.getDBMSTypeTestParameter()); + this.connectorTestA = new ConnectorTest(); + DBMSConnection dbmsConnection = connectorTestA.getTestDBMSConnection(); TestManager.clearTables(dbmsConnection); FieldPreferences fieldPreferences = mock(FieldPreferences.class); @@ -60,19 +62,22 @@ void setup() throws Exception { clientContextA.convertToSharedDatabase(synchronizerA); clientContextA.getDBMSSynchronizer().openSharedDatabase(dbmsConnection); + this.connectorTestB = new ConnectorTest(); clientContextB = new BibDatabaseContext(); DBMSSynchronizer synchronizerB = new DBMSSynchronizer(clientContextB, ',', fieldPreferences, pattern, new DummyFileUpdateMonitor()); clientContextB.convertToSharedDatabase(synchronizerB); // use a second connection, because this is another client (typically on another machine) - clientContextB.getDBMSSynchronizer().openSharedDatabase(ConnectorTest.getTestDBMSConnection(TestManager.getDBMSTypeTestParameter())); + clientContextB.getDBMSSynchronizer().openSharedDatabase(connectorTestB.getTestDBMSConnection()); eventListenerB = new SynchronizationEventListenerTest(); clientContextB.getDBMSSynchronizer().registerListener(eventListenerB); } @AfterEach - void clear() throws SQLException { + void clear() throws Exception { clientContextA.getDBMSSynchronizer().closeSharedDatabase(); clientContextB.getDBMSSynchronizer().closeSharedDatabase(); + connectorTestA.close(); + connectorTestB.close(); } @Test diff --git a/src/test/java/org/jabref/logic/shared/TestManager.java b/src/test/java/org/jabref/logic/shared/TestManager.java index 954bbccbb5c..17cf4ef3745 100644 --- a/src/test/java/org/jabref/logic/shared/TestManager.java +++ b/src/test/java/org/jabref/logic/shared/TestManager.java @@ -1,52 +1,13 @@ 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 * be used for tests. */ public class TestManager { - - /** - * Determine the DBMSType to test from the environment variable "DMBS". In case that variable is not set, use "PostgreSQL" as default - */ - public static DBMSType getDBMSTypeTestParameter() { - return DBMSType.fromString(System.getenv("DBMS")).orElse(DBMSType.POSTGRESQL); - } - public static void clearTables(DBMSConnection dbmsConnection) throws SQLException { - Objects.requireNonNull(dbmsConnection); - DBMSType dbmsType = dbmsConnection.getProperties().getType(); - - if (dbmsType == DBMSType.MYSQL) { - 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`"); - } else if (dbmsType == DBMSType.POSTGRESQL) { - 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"); - } else if (dbmsType == DBMSType.ORACLE) { - dbmsConnection.getConnection().createStatement() - .executeUpdate("BEGIN\n" - + "EXECUTE IMMEDIATE 'DROP TABLE \"FIELD\"';\n" + "EXCEPTION\n" + "WHEN OTHERS THEN\n" - + "IF SQLCODE != -942 THEN\n" + "RAISE;\n" + "END IF;\n" + "END;\n"); - dbmsConnection.getConnection().createStatement() - .executeUpdate("BEGIN\n" - + "EXECUTE IMMEDIATE 'DROP TABLE \"ENTRY\"';\n" + "EXCEPTION\n" + "WHEN OTHERS THEN\n" - + "IF SQLCODE != -942 THEN\n" + "RAISE;\n" + "END IF;\n" + "END;\n"); - dbmsConnection.getConnection().createStatement() - .executeUpdate("BEGIN\n" - + "EXECUTE IMMEDIATE 'DROP TABLE \"METADATA\"';\n" + "EXCEPTION\n" + "WHEN OTHERS THEN\n" - + "IF SQLCODE != -942 THEN\n" + "RAISE;\n" + "END IF;\n" + "END;\n"); - dbmsConnection.getConnection().createStatement() - // Sequence does not exist has a different error code than table does not exist - .executeUpdate("BEGIN\n" - + "EXECUTE IMMEDIATE 'DROP SEQUENCE \"ENTRY_SEQ\"';\n" + "EXCEPTION\n" + "WHEN OTHERS THEN\n" - + "IF SQLCODE != -2289 THEN\n" + "RAISE;\n" + "END IF;\n" + "END;\n"); - } + dbmsConnection.getConnection().createStatement().executeUpdate("DROP SCHEMA IF EXISTS \"jabref-alpha\" CASCADE"); } }