diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 244d0986f30..4cec490d8d0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -199,18 +199,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: @@ -227,24 +218,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 049d6112641..c002f68d158 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We improved the user comments field visibility so that it remains displayed if it contains text. Additionally, users can now easily toggle the field on or off via buttons unless disabled in preferences. [#11021](https://github.com/JabRef/jabref/issues/11021) - The LibreOffice integration for CSL styles is now more performant. [#12472](https://github.com/JabRef/jabref/pull/12472) - The "automatically sync bibliography when citing" feature of the LibreOffice integration is now disabled by default (can be enabled in settings). [#12472](https://github.com/JabRef/jabref/pull/12472) +- 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) - For the Citation key generator patterns, we reverted how `[authorsAlpha]` would behave to the original pattern and renamed the LNI-based pattern introduced in V6.0-alpha to `[authorsAlphaLNI]`. [#12499](https://github.com/JabRef/jabref/pull/12499) - We keep the list of recent files if one files could not be found. [#12517](https://github.com/JabRef/jabref/pull/12517) - During the import process, the labels indicating individual paragraphs within an abstract returned by PubMed/Medline XML are preserved. [#12527](https://github.com/JabRef/jabref/issues/12527) @@ -84,6 +85,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv ### Removed - "Web of Science" [journal abbreviation list](https://docs.jabref.org/advanced/journalabbreviations) was removed. [abbrv.jabref.org#176](https://github.com/JabRef/abbrv.jabref.org/issues/176) +- We removed support for MySQL/MariaDB and Oracle. ## [6.0-alpha] – 2024-12-23 diff --git a/build.gradle b/build.gradle index 5336d5325c4..6cfee205194 100644 --- a/build.gradle +++ b/build.gradle @@ -211,18 +211,10 @@ dependencies { implementation 'com.fasterxml:aalto-xml:1.3.3' - implementation group: 'org.mariadb.jdbc', name: 'mariadb-java-client', version: '2.7.12' - implementation 'org.postgresql:postgresql:42.7.5' // Support unix socket connection types implementation 'com.kohlschutter.junixsocket:junixsocket-core:2.10.1' - implementation 'com.kohlschutter.junixsocket:junixsocket-mysql:2.10.1' - - 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 @@ -820,25 +812,9 @@ 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' uses 'org.postgresql.shaded.com.ongres.stringprep.Profile' - 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-jabdrive.md b/docs/code-howtos/remote-storage-jabdrive.md index 1d31cfe88d6..ada503326cb 100644 --- a/docs/code-howtos/remote-storage-jabdrive.md +++ b/docs/code-howtos/remote-storage-jabdrive.md @@ -32,13 +32,14 @@ Longer explanations are put below at "The 'pull-merge-push cycle'". In order to support synchronization, additional metadata is kept for each item: -- `ID`: An unique identifier for the entry (will be a UUID). -- `Revision`: The revision is a "generation Id" being increasing positive integer. - This is based on [Multiversion concurrency control (MVCC)](http://en.wikipedia.org/wiki/Multiversion_concurrency_control), where an increasing identifier ("time stamp") is used. -- `hash`: This is the hash of the item (i.e., of all the data except for `Revision` and `hash`). +- `ID`: An unique identifier for the entry (will be a [CUID2](https://github.com/paralleldrive/cuid2?tab=readme-ov-file#cuid2)). +- `Version`: The version is a "generation Id" being increasing positive integer. + This is based on [Multiversion concurrency control (MVCC)](http://en.wikipedia.org/wiki/Multiversion_concurrency_control), where an increasing identifier ("time stamp") is used. See also [Optimistic Offline Lock](https://patterns-kompakt.de/patterns/persistenz/optimisticofflinelock). + Note that older versions called this "Revision". However, neither the page of MVCC nor the page of Optimistic Offline Lock use the term "Revision" (they do use "Version" though) +- `hash`: This is the hash of the item (i.e., of all the data except for `Version` and `hash`). - (Client only) `dirty`: Marks whether the user changed the entry. -`ID` and `Revision` are handled in [`org.jabref.model.entry.SharedBibEntryData`](https://github.com/JabRef/jabref/blob/main/src/main/java/org/jabref/model/entry/SharedBibEntryData.java). +`ID` and `Version` are handled in [`org.jabref.model.entry.SharedBibEntryData`](https://github.com/JabRef/jabref/blob/main/src/main/java/org/jabref/model/entry/SharedBibEntryData.java). {: .note-title } > Dirty flags @@ -53,15 +54,15 @@ In order to support synchronization, additional metadata is kept for each item: ### Global time clock -The idea is that the server tracks a global (logical) monotone increasing "time clock" tracking the existing revisions. -Each entry has its own revision, increased "locally". -The "global revision id" keeps track of the global synchronization state. +The idea is that the server tracks a global (logical) monotone increasing "time clock" tracking the existing versions. +Each entry has its own version, increased "locally". +The "global version id" keeps track of the global synchronization state. One can view it as aggregation on the synchronization state of all entries. -Similar to the revision concept of Subversion. +Similar to the version concept of Subversion. ### Tombstones -Deleted items are persisted as [tombstones](https://docs.couchbase.com/sync-gateway/current/managing-tombstones.html), which contain the metadata `ID` and `Revision` only. +Deleted items are persisted as [tombstones](https://docs.couchbase.com/sync-gateway/current/managing-tombstones.html), which contain the metadata `ID` and `version` only. Tombstones ensure that all synchronizing devices can identify that a previously existing entry has been deleted. On the client, a tombstone is created whenever an entry is deleted. Moreover, the client keeps a list of all entries in the library so that external deletions can be recognized when loading the library into memory. @@ -93,9 +94,9 @@ We assume that the server has some view on the library and the client has a view {: .note-title } > Straight-forward synchronization > -> When the client connects to the server, one option for synchronization is to ask the server for all up-to-date entries and then using the `Revision` information to merge with the local data. +> When the client connects to the server, one option for synchronization is to ask the server for all up-to-date entries and then using the `Version` information to merge with the local data. > However, this is highly inefficient as the whole database has to be sent over the wire. -> A small improvement is gained by first asking only for tuples of `ID` and `Revision`, and only pull the complete entry if the local data is outdated or in conflict. +> A small improvement is gained by first asking only for tuples of `ID` and `Version`, and only pull the complete entry if the local data is outdated or in conflict. > However, this still requires to send quite a bit of data. > Instead, we will use the following refinement. @@ -103,7 +104,7 @@ We assume that the server has some view on the library and the client has a view The client pulls on first connect or when requested to pull. The client asks the server for a list of documents that changed since the last checkpoint. (Creating a checkpoint is explained further below.) -The server responses with a batched list of these entries together with their `Revision` information. +The server responses with a batched list of these entries together with their `Version` information. These entries could also be tombstones. Each batch includes also a checkpoint `To` that has the meaning "all changes to this point in time are included in the current batch". @@ -115,15 +116,15 @@ This is more an implementation detail than a conceptual difference. The pulled data from the server needs to be merged with the local view of the data. The data is merged on a per-entry basis. -Based on the "generation ID" (`Revision`) of server and client, following cases can occur: +Based on the "generation ID" (`Version`) of server and client, following cases can occur: -1. The server's `Revision` is higher than the client's `Revision`: Two cases need to be distinguished: +1. The server's `Version` is higher than the client's `Version`: Two cases need to be distinguished: 1. The client's entry is dirty. That means, the user has edited the entry in the meantime. Then the user is shown a message to resolve the conflict (see "Conflict Handling" below) 2. The client's entry is clean. That means, the user has not edited the entry in the meantime. - In this case, the client's entry is replaced by the server's one (including the revision). -2. The server's `Revision` is equal to the client's `Revision`: Both entries are up-to-date and nothing has to be done. This case may happen if the library is synchronized by other means. -3. The server's `Revision` is lower than the client's `Revision`: This should never be the case, as revisions are only increased on the server. Show error message to user. + In this case, the client's entry is replaced by the server's one (including the Version). +2. The server's `Version` is equal to the client's `Version`: Both entries are up-to-date and nothing has to be done. This case may happen if the library is synchronized by other means. +3. The server's `Version` is lower than the client's `Version`: This should never be the case, as Version are only increased on the server. Show error message to user. If the entry returned by the server is a tombstone, then: @@ -133,8 +134,8 @@ If the entry returned by the server is a tombstone, then: #### Conflict Handling -If the user chooses to overwrite the local entry with the server entry, then the entry's `Revision` is updated as well, and it is no longer marked as dirty. -Otherwise, its `Revision` is updated to the one provided by the server, but it is still marked as dirty. +If the user chooses to overwrite the local entry with the server entry, then the entry's `Version` is updated as well, and it is no longer marked as dirty. +Otherwise, its `Version` is updated to the one provided by the server, but it is still marked as dirty. This will enable pushing of the entry to the server during the "Push Phase". After the merging is done, the client sets its local checkpoint to the value of `To`. @@ -143,17 +144,16 @@ After the merging is done, the client sets its local checkpoint to the value of The client sends the following information back to the server: -- The list of entries that are marked dirty (along with their `Revision` data). +- The list of entries that are marked dirty (along with their `Version` data). - The list of entries that are new, i.e., that do not have an `ID` yet. - The list of tombstones, i.e., entries that have been deleted. -The server accepts only changes if the provided `Revision` coincides with the `Revision` stored on the server. +The server accepts only changes if the provided `Version` coincides with the `Version` stored on the server. If this is not the case, then the entry has been modified on the server since the last pull operation, and then the user needs to go through a new pull-merge-push cycle. During the push operation, the user is not allowed to locally edit these entries that are currently pushed. After the push operation, all entries accepted by the server are marked clean. -Moreover, the server will generate a new revision number for each accepted entry, which will then be stored locally. -Entries rejected (as conflicts) by the server stay dirty and their `Revision` remains unchanged. +Moreover, the server will generate a new `Version` dirty and their `Version` remains unchanged. ### Start the "pull-merge-push cycle" again @@ -199,7 +199,7 @@ If the user decides in step 3 to save their changes, then in step 5 JabRef would ### Sync after successful sync of client changes 1. JabRef modifies local data: `{id: 1, value: 0, _rev=1, _dirty=false}` to `{id: 1, value: 1, _rev=1, _dirty=true}`. - `id` is `ID` from above, `value` summarizes all fields of the entry, `_rev` is `Revision` from above, and `_dirty` the dirty flag. + `id` is `ID` from above, `value` summarizes all fields of the entry, `_rev` is `Version` from above, and `_dirty` the dirty flag. 2. JabRef pulls server changes. Suppose there are none. 3. Consequently, Merge is not necessary. JabRef sets checkpoint to `T = 1`. 4. JabRef pushes its changes to the server. @@ -212,7 +212,7 @@ This is not quite optimal since the last pull response contains the full data of *Possible future improvements:* -- First pull only the `IDs` and `Revisions` of the server-side changes, and then filter out the ones we already have locally before querying the complete entry. +- First pull only the `IDs` and `Versions` of the server-side changes, and then filter out the ones we already have locally before querying the complete entry. Downside is that this solution always needs one more request (per change batch) and it is not clear if this outweighs the costs of sending the full entry. - The server can remember where a change came from and then not send these changes back to that client. Especially if the server's generation Id increased by one due to the update, this is straight-forward. @@ -224,37 +224,37 @@ The identifier needs to be unique at the very least across the library and shoul Both features cannot be ensured for BibTeX keys. Note this is similar to the `shared_id` in the case of the SQL synchronization. -### Why do we need revisions? Are `updatedAt` timeflags not enough? +### Why do we need versions? Are `updatedAt` timeflags not enough? -The revision functions as "generation Id" known from [Lamport clocks](https://en.wikipedia.org/wiki/Lamport_timestamp) and common in synchronization. +The versions functions as "generation Id" known from [Lamport clocks](https://en.wikipedia.org/wiki/Lamport_timestamp) and common in synchronization. For instance, the [Optimistic Offline Lock](https://martinfowler.com/eaaCatalog/optimisticOfflineLock.html) also uses these kinds of clocks. A "generation Id" is essentially a clock local to the entry that ticks whenever the entry is synced with the server. As for us there is only one server, strictly speaking, it would suffice to use the global server time for this. -Moreover, for the sync algorithm, the client would only need to store the revision/server time during the pull-merge-push cycle (to make sure that during this time the entry is not modified again on the server). +Moreover, for the sync algorithm, the client would only need to store the version/server time during the pull-merge-push cycle (to make sure that during this time the entry is not modified again on the server). Nevertheless, the generation Id is only a tiny data blob, and it gives a bit of additional security/consistency during the merge operation, so we keep it around all the time. ### Why do we need an entry hash? The hash is only used on the client to determine whether an entry has been changed outside of JabRef. -#### Why don't we need to keep the whole revision history as it is done in CouchDB? +#### Why don't we need to keep the whole version history as it is done in CouchDB? -The revision history is used by CouchDB to find a common ancestor of two given revisions. +The versions history is used by CouchDB to find a common ancestor of two given versions. This is needed since CouchDB provides main-main sync. -However, in our setting, we have a central server and thus the last synced revision is *the* common ancestor for both the new server and client revision. +However, in our setting, we have a central server and thus the last synced versions is *the* common ancestor for both the new server and client versions. -### Why is a dirty flag enough on the client? Why don't we need local revisions? +### Why is a dirty flag enough on the client? Why don't we need local versionss? -In CouchDB, every client has their own history of revisions. +In CouchDB, every client has their own history of versionss. This is needed to have a deterministic conflict resolution that can run on both the server and client side independently. -In this setting, it is important to determine which revision is older, which is then declared to be the winner. +In this setting, it is important to determine which versions is older, which is then declared to be the winner. However, we do not need an automatic conflict resolution: Whenever there is a conflict, the user is asked to resolve it. For this it is not important to know how many times (and when) the user changed the entry locally. It suffices to know that it changed at some point from the last synced version. -Local revision histories could be helpful in scenarios such as the following: +Local version histories could be helpful in scenarios such as the following: 1. Device A is offline, and the user changes an entry. 2. The user sends this changed entry to Device B (say, via git). @@ -262,8 +262,8 @@ Local revision histories could be helpful in scenarios such as the following: 4. The user syncs Device B with the server. 5. The user syncs Device A with the server. -Without local revisions, it is not possible for Device A to figure out that the entry from the server logically evolved from its own local version. -Instead, it shows a conflict message since the entry changed locally (step 1) and there is a newer revision on the server (from step 4). +Without local versions, it is not possible for Device A to figure out that the entry from the server logically evolved from its own local version. +Instead, it shows a conflict message since the entry changed locally (step 1) and there is a newer version on the server (from step 4). ## More Readings diff --git a/docs/code-howtos/remote-storage-sql.md b/docs/code-howtos/remote-storage-sql.md index 267d05129fe..26e7ce18d3e 100644 --- a/docs/code-howtos/remote-storage-sql.md +++ b/docs/code-howtos/remote-storage-sql.md @@ -7,16 +7,42 @@ grand_parent: Code Howtos For user documentation, see . +## Involved classes + +* `org.jabref.logic.shared.listener.PostgresSQLNotificationListener`: handles and routes notifications from the PostgreSQL database to the `DBMSSynchronizer`. + +## Flow of calls + +The idea is to "publish" the change event with data both locally and remotely. +The change event should contain the new value, which can be directly applied remotely. +The change event should contain the old value to enable sanity checks while applying the change. + +```mermaid +sequenceDiagram + BibEntry (A)->>DBMSSynchronizer (A): "FieldChangedEvent" +``` + ## Handling large shared databases -Synchronization times may get long when working with a large database containing several thousand entries. Therefore, synchronization only happens if several conditions are fulfilled: +Synchronization times may get long when working with a large database containing several thousand entries. +Therefore, we use PostgreSQL's `LISTEN` and `NOTIFY` commands to inform the client about changes in the database on an entry level. + +Background reading: . + +## 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 +87,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 ccb2f386829..8bb00f85e69 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -98,11 +98,8 @@ // region: SQL databases requires embedded.postgres; - requires org.tukaani.xz; - requires ojdbc10; requires org.postgresql.jdbc; - requires org.mariadb.jdbc; - uses org.mariadb.jdbc.credential.CredentialPlugin; + requires org.tukaani.xz; // 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 49c84c384c8..c26224ceda2 100644 --- a/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -76,7 +76,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/autosaveandbackup/AutosaveManager.java b/src/main/java/org/jabref/gui/autosaveandbackup/AutosaveManager.java index 72402b18dd5..5aaf831a829 100644 --- a/src/main/java/org/jabref/gui/autosaveandbackup/AutosaveManager.java +++ b/src/main/java/org/jabref/gui/autosaveandbackup/AutosaveManager.java @@ -56,7 +56,7 @@ private AutosaveManager(BibDatabaseContext bibDatabaseContext) { @Subscribe public void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) { - if (!event.isFilteredOut()) { + if (!event.isFiltered()) { this.needsSave = true; } } diff --git a/src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java b/src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java index 677e2090a68..f99eb142b57 100644 --- a/src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java @@ -333,7 +333,7 @@ private void logIfCritical(Path backupPath, IOException e) { @Subscribe public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) { - if (!event.isFilteredOut()) { + if (!event.isFiltered()) { this.needsBackup = true; } } diff --git a/src/main/java/org/jabref/gui/frame/MainMenu.java b/src/main/java/org/jabref/gui/frame/MainMenu.java index 70964048e56..40375cd7403 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 787bc6c3f33..3a137088e4b 100644 --- a/src/main/java/org/jabref/gui/keyboard/KeyBinding.java +++ b/src/main/java/org/jabref/gui/keyboard/KeyBinding.java @@ -91,7 +91,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/gui/shared/SharedDatabaseUIManager.java b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java index a1e73bcb96a..6a18f030aab 100644 --- a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java +++ b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java @@ -130,7 +130,7 @@ public void listen(UpdateRefusedEvent updateRefusedEvent) { Optional mergedEntry = dialogService.showCustomDialogAndWait(dialog).map(EntriesMergeResult::mergedEntry); mergedEntry.ifPresent(mergedBibEntry -> { - mergedBibEntry.getSharedBibEntryData().setSharedID(sharedBibEntry.getSharedBibEntryData().getSharedID()); + mergedBibEntry.getSharedBibEntryData().setSharedId(sharedBibEntry.getSharedBibEntryData().getSharedIdAsString()); mergedBibEntry.getSharedBibEntryData().setVersion(sharedBibEntry.getSharedBibEntryData().getVersion()); dbmsSynchronizer.synchronizeSharedEntry(mergedBibEntry); diff --git a/src/main/java/org/jabref/http/server/LibraryResource.java b/src/main/java/org/jabref/http/server/LibraryResource.java index 8e4b3f7ef18..4fbf560468a 100644 --- a/src/main/java/org/jabref/http/server/LibraryResource.java +++ b/src/main/java/org/jabref/http/server/LibraryResource.java @@ -45,7 +45,7 @@ public String getJson(@PathParam("id") String id) { ParserResult parserResult = getParserResult(id); BibEntryTypesManager entryTypesManager = Injector.instantiateModelOrService(BibEntryTypesManager.class); List list = parserResult.getDatabase().getEntries().stream() - .peek(bibEntry -> bibEntry.getSharedBibEntryData().setSharedID(Objects.hash(bibEntry))) + .peek(bibEntry -> bibEntry.getSharedBibEntryData().setSharedId(Objects.hash(bibEntry))) .map(entry -> new BibEntryDTO(entry, parserResult.getDatabaseContext().getMode(), preferences.getFieldPreferences(), entryTypesManager)) .toList(); return gson.toJson(list); diff --git a/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java b/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java index f9755996beb..db9e0c6485a 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..859ae4a3d52 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, String databaseName) { + this.connection = connection; + this.properties = new DBMSConnectionProperties(databaseName); + } + 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/DBMSConnectionProperties.java b/src/main/java/org/jabref/logic/shared/DBMSConnectionProperties.java index acfe8f9250b..10658233b7f 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSConnectionProperties.java +++ b/src/main/java/org/jabref/logic/shared/DBMSConnectionProperties.java @@ -9,6 +9,7 @@ import org.jabref.logic.shared.prefs.SharedDatabasePreferences; import org.jabref.logic.shared.security.Password; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,14 +27,19 @@ public class DBMSConnectionProperties implements DatabaseConnectionProperties { private String user; private String password; private boolean allowPublicKeyRetrieval; - private final boolean useSSL; + private boolean useSSL; private String serverTimezone = ""; private String jdbcUrl = ""; - private final boolean expertMode; + private boolean expertMode; // Not needed for connection, but stored for future login private String keyStore; + @VisibleForTesting + public DBMSConnectionProperties(String databaseName) { + this.database = databaseName; + } + /** * Gets all required data from {@link SharedDatabasePreferences} and sets them if present. */ diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index d7ac69ee6f9..e008513e55f 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -5,6 +5,7 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; +import java.sql.Statement; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -15,28 +16,33 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.UUID; +import java.util.StringJoiner; import java.util.stream.Collectors; import org.jabref.logic.shared.exception.OfflineLockException; +import org.jabref.logic.shared.notifications.NotificationListener; +import org.jabref.logic.util.HeadlessExecutorService; import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.SharedBibEntryData; 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; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; +import io.github.thibaultmeyer.cuid.CUID; +import org.postgresql.PGConnection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Processes all incoming or outgoing bib data to external SQL Database and manages its structure. */ -public abstract class DBMSProcessor { +public class DBMSProcessor { - public static final String PROCESSOR_ID = UUID.randomUUID().toString(); + public static final String PROCESSOR_ID = CUID.randomCUID2(8).toString(); protected static final Logger LOGGER = LoggerFactory.getLogger(DBMSProcessor.class); @@ -44,6 +50,13 @@ public abstract class DBMSProcessor { protected DatabaseConnectionProperties connectionProperties; + private NotificationListener listener; + + private int VERSION_DB_STRUCT_DEFAULT = -1; + + // TODO: We need to migrate data - or ask the user to recreate, #Liquibase + private int CURRENT_VERSION_DB_STRUCT = 2; + protected DBMSProcessor(DatabaseConnection dbmsConnection) { this.connection = dbmsConnection.getConnection(); this.connectionProperties = dbmsConnection.getProperties(); @@ -57,18 +70,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; } @@ -132,63 +140,138 @@ public void setupSharedDatabase() throws SQLException { * * @throws SQLException in case of error */ - protected abstract void setUp() throws SQLException; + 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; + } - /** - * 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); + // TODO: Before a release, fix the names (and migrate data to the new names) + // Think of using Flyway or Liquibase instead of manual migration + // Liquibase: + // - https://contribute.liquibase.com/extensions-integrations/directory/integration-docs/gradle + // - https://forum.liquibase.org/t/adding-liquibase-to-an-existing-project/6076 + // 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\""); + + // TODO: entrytype should be moved to table "field" (org.jabref.model.entry.field.InternalField.TYPE_HEADER) + 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);"); - abstract String escape_Table(String expression); + Map metadata = getSharedMetaData(); - abstract Integer getCURRENT_VERSION_DB_STRUCT(); + if (metadata.get(MetaData.VERSION_DB_STRUCT) != null) { + try { + // 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] is not an Integer."); + } + } else { + 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 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 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 + } + + int getCURRENT_VERSION_DB_STRUCT() { + return CURRENT_VERSION_DB_STRUCT; + } /** * For use in test only. Inserts the BibEntry into the shared database. * * @param bibEntry {@link BibEntry} to be inserted. */ + @VisibleForTesting public void insertEntry(BibEntry bibEntry) { insertEntries(Collections.singletonList(bibEntry)); } - /** - * Inserts the List of BibEntry into the shared database. - * - * @param bibEntries List of {@link BibEntry} to be inserted - */ public void insertEntries(List bibEntries) { - List notYetExistingEntries = getNotYetExistingEntries(bibEntries); - if (notYetExistingEntries.isEmpty()) { - return; - } - insertIntoEntryTable(notYetExistingEntries); - insertIntoFieldTable(notYetExistingEntries); + assert bibEntries.stream().filter(bibEntry -> bibEntry.getSharedBibEntryData().getSharedIdAsInt() != -1).findAny().isEmpty(); + insertIntoEntryTable(bibEntries); + insertIntoFieldTable(bibEntries); } - /** - * Inserts the given List of BibEntry into the ENTRY table. - * - * @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(?)"); - // Number of commas is bibEntries.size() - 1 - insertIntoEntryQuery.append(", (?)".repeat(Math.max(0, (bibEntries.size() - 1)))); - - try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(), - new String[]{"SHARED_ID"})) { + 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()); } @@ -199,99 +282,65 @@ protected void insertIntoEntryTable(List bibEntries) { // This should be the case for (BibEntry bibEntry : bibEntries) { generatedKeys.next(); - bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); + bibEntry.getSharedBibEntryData().setSharedId(generatedKeys.getInt(1)); } if (generatedKeys.next()) { - LOGGER.error("Error: Some shared IDs left unassigned"); - } - } - } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); - } - } - - /** - * Filters a list of BibEntry to and returns those which do not exist in the database - * - * @param bibEntries {@link BibEntry} to be checked - * @return true if existent, else false - */ - private List getNotYetExistingEntries(List bibEntries) { - List remoteIds = new ArrayList<>(); - List localIds = bibEntries.stream() - .map(BibEntry::getSharedBibEntryData) - .map(SharedBibEntryData::getSharedID) - .filter(id -> id != -1) - .toList(); - if (localIds.isEmpty()) { - return bibEntries; - } - try { - String selectQuery = "SELECT * FROM " + - escape_Table("ENTRY"); - - try (ResultSet resultSet = connection.createStatement().executeQuery(selectQuery)) { - while (resultSet.next()) { - int id = resultSet.getInt("SHARED_ID"); - remoteIds.add(id); + LOGGER.error("Some shared IDs left unassigned"); } } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error during entry insertion", e); } - return bibEntries.stream().filter(entry -> - !remoteIds.contains(entry.getSharedBibEntryData().getSharedID())) - .collect(Collectors.toList()); } /** * Inserts the given list of BibEntry into FIELD table. + * These entries do not yet exist in the remote database. * * @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(); } if (numFields == 0) { - return; // Prevent SQL Exception + // Nothing to insert + return; } // Number of commas is fields.size() - 1 - insertFieldQuery.append(", (?, ?, ?)".repeat(Math.max(0, (numFields - 1)))); + insertFieldQuery.append(", (?, ?, ?)".repeat(numFields - 1)); 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.setInt((3 * fieldsCompleted) + 1, bibEntries.get(entryIndex).getSharedBibEntryData().getSharedIdAsInt()); 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; } } + // 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,10 +351,11 @@ 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 { - Optional sharedEntryOptional = getSharedEntry(localBibEntry.getSharedBibEntryData().getSharedID()); + Optional sharedEntryOptional = getSharedEntry(localBibEntry.getSharedBibEntryData().getSharedIdAsInt()); if (sharedEntryOptional.isEmpty()) { return; @@ -321,22 +371,16 @@ 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()); - preparedUpdateEntryTypeStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedUpdateEntryTypeStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedIdAsInt()); preparedUpdateEntryTypeStatement.executeUpdate(); } @@ -345,7 +389,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,18 +403,15 @@ 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)) { preparedDeleteFieldStatement.setString(1, nullField.getName()); - preparedDeleteFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedDeleteFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedIdAsInt()); preparedDeleteFieldStatement.executeUpdate(); } } @@ -389,52 +430,40 @@ 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 name FROM FIELD + WHERE NAME = ? AND ENTRY_SHARED_ID = ? + """; try (PreparedStatement preparedSelectFieldStatement = connection .prepareStatement(selectFieldQuery)) { preparedSelectFieldStatement.setString(1, field.getName()); - preparedSelectFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedSelectFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedIdAsInt()); 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)) { preparedUpdateFieldStatement.setString(1, value); preparedUpdateFieldStatement.setString(2, field.getName()); - preparedUpdateFieldStatement.setInt(3, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedUpdateFieldStatement.setInt(3, localBibEntry.getSharedBibEntryData().getSharedIdAsInt()); 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)) { - preparedFieldStatement.setInt(1, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedFieldStatement.setInt(1, localBibEntry.getSharedBibEntryData().getSharedIdAsInt()); preparedFieldStatement.setString(2, field.getName()); preparedFieldStatement.setString(3, value); preparedFieldStatement.executeUpdate(); @@ -455,18 +484,13 @@ 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 ("); - query.append("?, ".repeat(bibEntries.size() - 1)); - query.append("?)"); + String query = "DELETE FROM ENTRY WHERE SHARED_ID IN (" + + "?, ".repeat(bibEntries.size() - 1) + + "?)"; - try (PreparedStatement preparedStatement = connection.prepareStatement(query.toString())) { + try (PreparedStatement preparedStatement = connection.prepareStatement(query)) { for (int j = 0; j < bibEntries.size(); j++) { - preparedStatement.setInt(j + 1, bibEntries.get(j).getSharedBibEntryData().getSharedID()); + preparedStatement.setInt(j + 1, bibEntries.get(j).getSharedBibEntryData().getSharedIdAsInt()); } preparedStatement.executeUpdate(); } catch (SQLException e) { @@ -479,7 +503,7 @@ public void removeEntries(List bibEntries) { * @return instance of {@link BibEntry} */ public Optional getSharedEntry(int sharedID) { - List sharedEntries = getSharedEntries(Collections.singletonList(sharedID)); + List sharedEntries = getSharedEntries(List.of(sharedID)); if (sharedEntries.isEmpty()) { return Optional.empty(); } else { @@ -513,31 +537,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.version, entry.entrytype, ") + .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 +563,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 +583,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,14 +599,12 @@ 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 shared_id, version FROM entry"; try (ResultSet selectEntryResultSet = connection.createStatement().executeQuery(selectEntryQuery)) { while (selectEntryResultSet.next()) { - sharedIDVersionMapping.put(selectEntryResultSet.getInt("SHARED_ID"), selectEntryResultSet.getInt("VERSION")); + sharedIDVersionMapping.put( + selectEntryResultSet.getInt("shared_id"), + selectEntryResultSet.getInt("version")); } } catch (SQLException e) { LOGGER.error("SQL Error", e); @@ -606,7 +619,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,60 +636,22 @@ 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(); } } } - /** - * 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. - } - public DatabaseConnectionProperties getDBMSConnectionProperties() { return this.connectionProperties; } @@ -686,22 +661,31 @@ public DatabaseConnectionProperties getDBMSConnectionProperties() { * * @param dbmsSynchronizer {@link DBMSSynchronizer} which handles the notification. */ - public void startNotificationListener(@SuppressWarnings("unused") DBMSSynchronizer dbmsSynchronizer) { - // nothing to do + public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) { + // Disable cleanup output of ThreadedHousekeeper + // Logger.getLogger(ThreadedHousekeeper.class.getName()).setLevel(Level.SEVERE); + try { + connection.createStatement().execute("LISTEN jabrefLiveUpdate"); + // Do not use `new PostgresSQLNotificationListener(...)` as the object has to exist continuously! + // Otherwise, the listener is going to be deleted by Java's garbage collector. + PGConnection pgConnection = connection.unwrap(PGConnection.class); + listener = new NotificationListener(dbmsSynchronizer, pgConnection); + HeadlessExecutorService.INSTANCE.execute(listener); + } catch (SQLException e) { + LOGGER.error("SQL Error during starting the notification listener", e); + } } /** * Terminates the notification listener. Needs to be implemented if LiveUpdate is supported by the DBMS */ public void stopNotificationListener() { - // nothing to do - } - - /** - * Notifies all clients ({@link DBMSSynchronizer}) which are connected to the same DBMS. Needs to be implemented if - * LiveUpdate is supported by the DBMS - */ - public void notifyClients() { - // nothing to do + try { + listener.stop(); + connection.close(); + } catch (SQLException e) { + LOGGER.error("SQL Error during stopping the notification listener", e); + } } } + diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index d388de0b884..d7601d67e67 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; @@ -20,6 +19,7 @@ import org.jabref.logic.shared.event.SharedEntriesNotPresentEvent; import org.jabref.logic.shared.event.UpdateRefusedEvent; import org.jabref.logic.shared.exception.OfflineLockException; +import org.jabref.logic.shared.notifications.Notifier; import org.jabref.model.database.BibDatabase; import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.database.event.EntriesAddedEvent; @@ -34,6 +34,7 @@ import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; +import org.postgresql.PGConnection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,17 +47,24 @@ public class DBMSSynchronizer implements DatabaseSynchronizer { private static final Logger LOGGER = LoggerFactory.getLogger(DBMSSynchronizer.class); private DBMSProcessor dbmsProcessor; + private Connection currentConnection; + private Notifier notifier; private String dbName; - private final BibDatabaseContext bibDatabaseContext; + private MetaData metaData; + private final BibDatabaseContext bibDatabaseContext; private final BibDatabase bibDatabase; private final EventBus eventBus; - private Connection currentConnection; private final Character keywordSeparator; private final GlobalCitationKeyPatterns globalCiteKeyPattern; + + @SuppressWarnings("unused") + // will be required later for save actions 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 +77,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 +88,30 @@ 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(); + // TODO: Rewrite 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) { - 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); + if (event.isFiltered() || !isEventSourceAccepted(event)) { + return; } + + notifier.notifyAboutChangedField(event); } /** @@ -130,9 +136,7 @@ public void listen(EntriesRemovedEvent event) { public void listen(MetaDataChangedEvent event) { if (checkCurrentConnection()) { synchronizeSharedMetaData(event.getMetaData(), globalCiteKeyPattern); - synchronizeLocalDatabase(); - applyMetaData(); - dbmsProcessor.notifyClients(); + // TODO: applyMetaData(); } } @@ -156,7 +160,7 @@ public void initializeDatabases() throws DatabaseNotSupportedException { dbmsProcessor.setupSharedDatabase(); } } catch (SQLException e) { - LOGGER.error("Could not check intergrity", e); + LOGGER.error("Could not check integrity", e); throw new IllegalStateException(e); } @@ -185,7 +189,7 @@ public void synchronizeLocalDatabase() { for (Map.Entry idVersionEntry : idVersionMap.entrySet()) { boolean remoteEntryMatchingOneLocalEntryFound = false; for (BibEntry localEntry : localEntries) { - if (idVersionEntry.getKey().equals(localEntry.getSharedBibEntryData().getSharedID())) { + if (idVersionEntry.getKey().equals(localEntry.getSharedBibEntryData().getSharedIdAsInt())) { remoteEntryMatchingOneLocalEntryFound = true; if (idVersionEntry.getValue() > localEntry.getSharedBibEntryData().getVersion()) { Optional sharedEntry = dbmsProcessor.getSharedEntry(idVersionEntry.getKey()); @@ -229,7 +233,7 @@ public void synchronizeLocalDatabase() { private void removeNotSharedEntries(List localEntries, Set sharedIDs) { List entriesToRemove = localEntries.stream() - .filter(localEntry -> !sharedIDs.contains(localEntry.getSharedBibEntryData().getSharedID())) + .filter(localEntry -> !sharedIDs.contains(localEntry.getSharedBibEntryData().getSharedIdAsInt())) .collect(Collectors.toList()); if (!entriesToRemove.isEmpty()) { eventBus.post(new SharedEntriesNotPresentEvent(entriesToRemove)); @@ -247,7 +251,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 +289,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 +305,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 +336,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 +351,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(); } /** @@ -384,7 +393,13 @@ public boolean isEventSourceAccepted(EntriesEvent event) { public void openSharedDatabase(DatabaseConnection connection) throws DatabaseNotSupportedException { this.dbName = connection.getProperties().getDatabase(); this.currentConnection = connection.getConnection(); - this.dbmsProcessor = DBMSProcessor.getProcessorInstance(connection); + try { + this.notifier = new Notifier(currentConnection.unwrap(PGConnection.class)); + } catch (SQLException e) { + LOGGER.error("Could not get Postgres driver", e); + throw new DatabaseNotSupportedException(); + } + this.dbmsProcessor = new DBMSProcessor(connection); initializeDatabases(); } @@ -396,7 +411,7 @@ public void closeSharedDatabase() { dbmsProcessor.stopNotificationListener(); currentConnection.close(); } catch (SQLException e) { - LOGGER.error("SQL Error:", e); + LOGGER.error("SQL Error", e); } } @@ -409,10 +424,6 @@ public String getDBName() { return dbName; } - public DBMSProcessor getDBProcessor() { - return dbmsProcessor; - } - @Override public DatabaseConnectionProperties getConnectionProperties() { return dbmsProcessor.getDBMSConnectionProperties(); diff --git a/src/main/java/org/jabref/logic/shared/DBMSType.java b/src/main/java/org/jabref/logic/shared/DBMSType.java index ebb49730b9e..03127ca338b 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/DatabaseSynchronizer.java b/src/main/java/org/jabref/logic/shared/DatabaseSynchronizer.java index 1d30d31bfc6..49d46d976a9 100644 --- a/src/main/java/org/jabref/logic/shared/DatabaseSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DatabaseSynchronizer.java @@ -6,13 +6,13 @@ public interface DatabaseSynchronizer { String getDBName(); - void pullChanges(); + void openSharedDatabase(DatabaseConnection connection) throws DatabaseNotSupportedException; void closeSharedDatabase(); - void registerListener(Object listener); + void pullChanges(); - void openSharedDatabase(DatabaseConnection connection) throws DatabaseNotSupportedException; + void registerListener(Object listener); void synchronizeSharedEntry(BibEntry bibEntry); 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 deleted file mode 100644 index 21b04d09c6a..00000000000 --- a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java +++ /dev/null @@ -1,175 +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.List; -import java.util.Map; - -import org.jabref.logic.shared.listener.PostgresSQLNotificationListener; -import org.jabref.logic.util.HeadlessExecutorService; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.metadata.MetaData; - -import org.postgresql.PGConnection; - -/** - * Processes all incoming or outgoing bib data to PostgreSQL database and manages its structure. - */ -public class PostgreSQLProcessor extends DBMSProcessor { - - private PostgresSQLNotificationListener listener; - - private int VERSION_DB_STRUCT_DEFAULT = -1; - private final int CURRENT_VERSION_DB_STRUCT = 1; - - public PostgreSQLProcessor(DatabaseConnection connection) { - super(connection); - } - - /** - * Creates and sets up the needed tables and columns according to the database type. - * - * @throws SQLException in case of error - */ - @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)"); - - Map metadata = getSharedMetaData(); - - if (metadata.get(MetaData.VERSION_DB_STRUCT) != null) { - try { - // 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!"); - } - } 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 (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\"))"); - metadata = getSharedMetaData(); - } - - metadata.put(MetaData.VERSION_DB_STRUCT, String.valueOf(CURRENT_VERSION_DB_STRUCT)); - setSharedMetaData(metadata); - } - } - - @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(), - Statement.RETURN_GENERATED_KEYS)) { - for (int i = 0; i < bibEntries.size(); i++) { - preparedEntryStatement.setString(i + 1, bibEntries.get(i).getType().getName()); - } - preparedEntryStatement.executeUpdate(); - - try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) { - // The following assumes that we get the generated keys in the order the entries were inserted - // This should be the case - for (BibEntry bibEntry : bibEntries) { - generatedKeys.next(); - bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); - } - if (generatedKeys.next()) { - LOGGER.error("Some shared IDs left unassigned"); - } - } - } catch (SQLException e) { - LOGGER.error("SQL Error during entry insertion", e); - } - } - - @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; - } - - @Override - public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) { - // Disable cleanup output of ThreadedHousekeeper - // Logger.getLogger(ThreadedHousekeeper.class.getName()).setLevel(Level.SEVERE); - try { - connection.createStatement().execute("LISTEN jabrefLiveUpdate"); - // Do not use `new PostgresSQLNotificationListener(...)` as the object has to exist continuously! - // Otherwise, the listener is going to be deleted by Java's garbage collector. - PGConnection pgConnection = connection.unwrap(PGConnection.class); - listener = new PostgresSQLNotificationListener(dbmsSynchronizer, pgConnection); - HeadlessExecutorService.INSTANCE.execute(listener); - } catch (SQLException e) { - LOGGER.error("SQL Error during starting the notification listener", e); - } - } - - @Override - public void stopNotificationListener() { - try { - listener.stop(); - connection.close(); - } catch (SQLException e) { - LOGGER.error("SQL Error during stopping the notification listener", e); - } - } - - @Override - public void notifyClients() { - try { - connection.createStatement().execute("NOTIFY jabrefLiveUpdate, '" + PROCESSOR_ID + "';"); - } catch (SQLException e) { - LOGGER.error("SQL Error during client notification", e); - } - } -} 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/notifications/FieldChange.java b/src/main/java/org/jabref/logic/shared/notifications/FieldChange.java new file mode 100644 index 00000000000..d8c2ff308f7 --- /dev/null +++ b/src/main/java/org/jabref/logic/shared/notifications/FieldChange.java @@ -0,0 +1,8 @@ +package org.jabref.logic.shared.notifications; + +public record FieldChange( + String sourceProcessorId, + String bibEntryId, + String field, + String oldValue, + String newValue) {} diff --git a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java b/src/main/java/org/jabref/logic/shared/notifications/NotificationListener.java similarity index 67% rename from src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java rename to src/main/java/org/jabref/logic/shared/notifications/NotificationListener.java index 0c6db429622..3cbbfe3543a 100644 --- a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java +++ b/src/main/java/org/jabref/logic/shared/notifications/NotificationListener.java @@ -1,4 +1,4 @@ -package org.jabref.logic.shared.listener; +package org.jabref.logic.shared.notifications; import java.sql.SQLException; @@ -13,15 +13,15 @@ /** * A listener for PostgreSQL database notifications. */ -public class PostgresSQLNotificationListener implements Runnable { +public class NotificationListener implements Runnable { - private static final Logger LOGGER = LoggerFactory.getLogger(PostgresSQLNotificationListener.class); + private static final Logger LOGGER = LoggerFactory.getLogger(NotificationListener.class); private final DBMSSynchronizer dbmsSynchronizer; private final PGConnection pgConnection; private volatile boolean stop; - public PostgresSQLNotificationListener(DBMSSynchronizer dbmsSynchronizer, PGConnection pgConnection) { + public NotificationListener(DBMSSynchronizer dbmsSynchronizer, PGConnection pgConnection) { this.dbmsSynchronizer = dbmsSynchronizer; this.pgConnection = pgConnection; } @@ -30,22 +30,21 @@ public PostgresSQLNotificationListener(DBMSSynchronizer dbmsSynchronizer, PGConn public void run() { stop = false; try { - // noinspection InfiniteLoopStatement - while (!stop) { - PGNotification[] notifications = pgConnection.getNotifications(); + while (!stop && !Thread.currentThread().isInterrupted()) { + // Wait for 12 seconds for notifications. Result will be null if no notifications arrive + PGNotification[] notifications = pgConnection.getNotifications(12_000); if (notifications != null) { for (PGNotification notification : notifications) { if (!DBMSProcessor.PROCESSOR_ID.equals(notification.getName())) { + // Only process notifications that are not sent by this processor + notification.getParameter(); dbmsSynchronizer.pullChanges(); } } } - - // Wait a while before checking again for new notifications - Thread.sleep(500); } - } catch (SQLException | InterruptedException exception) { + } catch (SQLException exception) { LOGGER.error("Error while listening for updates to PostgresSQL", exception); } } diff --git a/src/main/java/org/jabref/logic/shared/notifications/Notifier.java b/src/main/java/org/jabref/logic/shared/notifications/Notifier.java new file mode 100644 index 00000000000..8281a604c12 --- /dev/null +++ b/src/main/java/org/jabref/logic/shared/notifications/Notifier.java @@ -0,0 +1,28 @@ +package org.jabref.logic.shared.notifications; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; + +import org.jabref.model.entry.event.FieldChangedEvent; +import org.postgresql.PGConnection; + +/** + * TODO: for sizes > 8000 bytes, use a table for exchange + */ +public class Notifier { + + private final PGConnection pgConnection; + private final Gson gson = new GsonBuilder().create(); + + public Notifier(PGConnection pgConnection) { + this.pgConnection = pgConnection; + } + + public void notifyAboutChangedField(FieldChangedEvent event) { + // FIXME: 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). + // connection.createStatement().execute("NOTIFY jabrefLiveUpdate, '" + PROCESSOR_ID + "';"); + } +} diff --git a/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java b/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java index 9160fba7938..a0a781c058d 100644 --- a/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java +++ b/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java @@ -42,10 +42,11 @@ public synchronized void listen(BibDatabaseContextChangedEvent event) { boolean isChangedEntry = lastEntryChanged.filter(e -> !e.equals(fieldChange.getBibEntry())).isPresent(); boolean isEditChanged = !isNewEdit && (isChangedField || isChangedEntry); // Only deltas of 1 when typing in manually, major change means pasting something (more than one character) - boolean isMajorChange = fieldChange.getMajorCharacterChange() > 1; + boolean isMajorChange = fieldChange.charactersChangedCount() > 1; - fieldChange.setFilteredOut(!(isEditChanged || isMajorChange)); + fieldChange.setFiltered(!(isEditChanged || isMajorChange)); // Post each FieldChangedEvent - even the ones being marked as "filtered" + // Explanation at https://github.com/JabRef/jabref/pull/6868. - especially necessary for BackupManager and AutoSaveManager eventBus.post(fieldChange); lastFieldChanged = Optional.of(fieldChange.getField()); diff --git a/src/main/java/org/jabref/model/database/BibDatabase.java b/src/main/java/org/jabref/model/database/BibDatabase.java index 911b5bed2a0..810bd7324d5 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)); } /** @@ -187,12 +184,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) { @@ -229,7 +226,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)); } } @@ -263,11 +260,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); } /** @@ -277,7 +274,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); } @@ -285,7 +282,7 @@ public void setStrings(List stringsToAdd) { * Removes the string with the given id. */ public void removeString(String id) { - bibtexStrings.remove(id); + stringToBibtexString.remove(id); } /** @@ -293,7 +290,7 @@ public void removeString(String id) { * These are in no sorted order. */ public Set getStringKeySet() { - return bibtexStrings.keySet(); + return stringToBibtexString.keySet(); } /** @@ -301,14 +298,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); } /** @@ -322,14 +319,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(); } /** @@ -345,7 +342,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)); } /** @@ -375,7 +372,7 @@ public List getUsedStrings(Collection entries) { } } - return allUsedIds.stream().map(bibtexStrings::get).toList(); + return allUsedIds.stream().map(stringToBibtexString::get).toList(); } /** @@ -437,7 +434,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 @@ -645,7 +642,7 @@ public int indexOf(BibEntry bibEntry) { } public BibEntry getEntryById(String id) { - return entriesId.get(id); + return entryIdToBibEntry.get(id); } @Override @@ -657,7 +654,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) @@ -666,6 +663,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/BibDatabaseContextChangedEvent.java b/src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java index 9db98e10dbf..73739b2d084 100644 --- a/src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java +++ b/src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java @@ -9,25 +9,16 @@ * because all three inherit from this class. */ public abstract class BibDatabaseContextChangedEvent { - // If the event has been filtered out - private boolean filteredOut; - - public BibDatabaseContextChangedEvent() { - this(false); - } - - public BibDatabaseContextChangedEvent(boolean filteredOut) { - this.filteredOut = filteredOut; - } + private boolean filtered = false; /** * Check if this event can be filtered out to be synchronized with a database at a later time. */ - public boolean isFilteredOut() { - return filteredOut; + public boolean isFiltered() { + return filtered; } - public void setFilteredOut(boolean filtered) { - this.filteredOut = filtered; + public void setFiltered(boolean filtered) { + this.filtered = filtered; } } 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 74f8840e189..abc5983e337 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -99,7 +99,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; /** @@ -379,10 +383,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; } @@ -453,7 +454,7 @@ public Optional setType(EntryType newType, EntriesEventSource event this.type.setValue(newType); FieldChange change = new FieldChange(this, InternalField.TYPE_HEADER, oldType.getName(), newType.getName()); - eventBus.post(new FieldChangedEvent(change, eventSource)); + eventBus.post(new FieldChangedEvent(eventSource, change)); return Optional.of(change); } @@ -643,7 +644,7 @@ public Optional setField(Field field, String value, EntriesEventSou if (isNewField) { eventBus.post(new FieldAddedOrRemovedEvent(change, eventSource)); } else { - eventBus.post(new FieldChangedEvent(change, eventSource)); + eventBus.post(new FieldChangedEvent(eventSource, change)); } return Optional.of(change); } @@ -919,7 +920,7 @@ public SharedBibEntryData getSharedBibEntryData() { } public BibEntry withSharedBibEntryData(int sharedId, int version) { - sharedBibEntryData.setSharedID(sharedId); + sharedBibEntryData.setSharedId(sharedId); sharedBibEntryData.setVersion(version); return this; } diff --git a/src/main/java/org/jabref/model/entry/CanonicalBibEntry.java b/src/main/java/org/jabref/model/entry/CanonicalBibEntry.java index 1a15918ac9a..3fee0d89ffb 100644 --- a/src/main/java/org/jabref/model/entry/CanonicalBibEntry.java +++ b/src/main/java/org/jabref/model/entry/CanonicalBibEntry.java @@ -61,7 +61,7 @@ public static String getCanonicalRepresentation(BibEntry entry) { sj.add(line); } - sj.add(" _jabref_shared = {sharedId: %d, version: %d}".formatted(entry.getSharedBibEntryData().getSharedID(), entry.getSharedBibEntryData().getVersion())); + sj.add(" _jabref_shared = {sharedId: %d, version: %d}".formatted(entry.getSharedBibEntryData().getSharedIdAsInt(), entry.getSharedBibEntryData().getVersion())); sb.append(sj); diff --git a/src/main/java/org/jabref/model/entry/SharedBibEntryData.java b/src/main/java/org/jabref/model/entry/SharedBibEntryData.java index 69c3c6bf976..4d9c7637f04 100644 --- a/src/main/java/org/jabref/model/entry/SharedBibEntryData.java +++ b/src/main/java/org/jabref/model/entry/SharedBibEntryData.java @@ -1,6 +1,9 @@ package org.jabref.model.entry; +import java.util.Objects; + import com.google.common.base.MoreObjects; +import org.jspecify.annotations.NonNull; /** * Stores all information needed to manage entries on a shared (SQL) database. @@ -11,23 +14,46 @@ public class SharedBibEntryData implements Comparable { // It has to be unique on remote DBS for all connected JabRef instances. // The old id above does not satisfy this requirement. // This is "ID" in JabDrive sync - private int sharedID; + private String sharedIdAsString; + + private int sharedIdAsInt; // Needed for version controlling if used on shared database // This is "Revision" in JabDrive sync private int version; public SharedBibEntryData() { - this.sharedID = -1; + this.sharedIdAsString = ""; + this.sharedIdAsInt = -1; this.version = 1; } - public int getSharedID() { - return sharedID; + /** + * @return Empty string if no sharedId is set yet + */ + public String getSharedIdAsString() { + return sharedIdAsString; + } + + /** + * @return -1 if no sharedId is set yet + */ + public int getSharedIdAsInt() { + return sharedIdAsInt; + } + + public void setSharedId(@NonNull String sharedIdAsString) { + this.sharedIdAsString = sharedIdAsString; + try { + this.sharedIdAsInt = Integer.parseInt(sharedIdAsString); + } catch (NumberFormatException e) { + this.sharedIdAsInt = Objects.hash(sharedIdAsString); + } } - public void setSharedID(int sharedID) { - this.sharedID = sharedID; + public void setSharedId(@NonNull int sharedId) { + this.sharedIdAsString = String.valueOf(sharedId); + this.sharedIdAsInt = sharedId; } public int getVersion() { @@ -41,17 +67,17 @@ public void setVersion(int version) { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("sharedID", sharedID) + .add("sharedId", sharedIdAsString) .add("version", version) .toString(); } @Override public int compareTo(SharedBibEntryData o) { - if (this.sharedID == o.sharedID) { + if (this.sharedIdAsString == o.sharedIdAsString) { return Integer.compare(this.version, o.version); } else { - return Integer.compare(this.sharedID, o.sharedID); + return this.sharedIdAsString.compareTo(o.sharedIdAsString); } } } diff --git a/src/main/java/org/jabref/model/entry/event/EntryChangedEvent.java b/src/main/java/org/jabref/model/entry/event/EntryChangedEvent.java index a16819b9ab2..ee3838ca7dc 100644 --- a/src/main/java/org/jabref/model/entry/event/EntryChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/EntryChangedEvent.java @@ -1,6 +1,6 @@ package org.jabref.model.entry.event; -import java.util.Collections; +import java.util.List; import org.jabref.model.entry.BibEntry; @@ -11,18 +11,18 @@ public class EntryChangedEvent extends EntriesEvent { /** - * @param bibEntry BibEntry object the changes were applied on. + * @param bibEntry where the changes were applied on. */ public EntryChangedEvent(BibEntry bibEntry) { - super(Collections.singletonList(bibEntry)); + super(List.of(bibEntry)); } /** - * @param bibEntry BibEntry object the changes were applied on. - * @param location Location affected by this event + * @param bibEntry where the changes were applied on. + * @param source Source of this event */ - public EntryChangedEvent(BibEntry bibEntry, EntriesEventSource location) { - super(Collections.singletonList(bibEntry), location); + public EntryChangedEvent(BibEntry bibEntry, EntriesEventSource source) { + super(List.of(bibEntry), source); } public BibEntry getBibEntry() { diff --git a/src/main/java/org/jabref/model/entry/event/FieldAddedOrRemovedEvent.java b/src/main/java/org/jabref/model/entry/event/FieldAddedOrRemovedEvent.java index 63306f6582d..37ea3107ded 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldAddedOrRemovedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldAddedOrRemovedEvent.java @@ -5,6 +5,6 @@ public class FieldAddedOrRemovedEvent extends FieldChangedEvent { public FieldAddedOrRemovedEvent(FieldChange fieldChange, EntriesEventSource location) { - super(fieldChange, location); + super(location, fieldChange); } } 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 d452f84649b..b42b2087ca1 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -12,24 +12,23 @@ public class FieldChangedEvent extends EntryChangedEvent { private final Field field; - private final String newValue; private final String oldValue; - private int majorCharacterChange = 0; + private final String newValue; + private int charactersChangedCount = 0; /** + * @param source source of 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) { - super(bibEntry, location); + public FieldChangedEvent(EntriesEventSource source, BibEntry bibEntry, Field field, String oldValue, String newValue) { + super(bibEntry, source); this.field = field; - this.newValue = newValue; this.oldValue = oldValue; - this.majorCharacterChange = computeMajorCharacterChange(oldValue, newValue); + this.newValue = newValue; + this.charactersChangedCount = computeMajorCharacterChange(oldValue, newValue); } /** @@ -37,30 +36,27 @@ 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; this.oldValue = oldValue; - this.majorCharacterChange = computeMajorCharacterChange(oldValue, newValue); + this.charactersChangedCount = computeMajorCharacterChange(oldValue, newValue); } - /** - * @param location Location affected by this event - */ - public FieldChangedEvent(FieldChange fieldChange, EntriesEventSource location) { - super(fieldChange.getEntry(), location); + public FieldChangedEvent(EntriesEventSource source, FieldChange fieldChange) { + super(fieldChange.getEntry(), source); this.field = fieldChange.getField(); this.newValue = fieldChange.getNewValue(); this.oldValue = fieldChange.getOldValue(); - this.majorCharacterChange = computeMajorCharacterChange(oldValue, newValue); + this.charactersChangedCount = computeMajorCharacterChange(oldValue, newValue); } public FieldChangedEvent(FieldChange fieldChange) { - this(fieldChange, EntriesEventSource.LOCAL); + this(EntriesEventSource.LOCAL, fieldChange); } - private int computeMajorCharacterChange(String oldValue, String newValue) { + private static int computeMajorCharacterChange(String oldValue, String newValue) { // Objects.equals first checks '==' if (Objects.equals(oldValue, newValue)) { return 0; @@ -79,15 +75,15 @@ public Field getField() { return field; } - public String getNewValue() { - return newValue; - } - public String getOldValue() { return oldValue; } - public int getMajorCharacterChange() { - return majorCharacterChange; + public String getNewValue() { + return newValue; + } + + public int charactersChangedCount() { + return charactersChangedCount; } } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index a7737a9a456..f5a925f20a9 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -703,7 +703,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..a76af43a168 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, "postgres"); } - 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..56c8d896dab 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 = new DBMSProcessor(dbmsConnection); TestManager.clearTables(this.dbmsConnection); dbmsProcessor.setupSharedDatabase(); } @AfterEach - void closeDbmsConnection() throws SQLException { - this.dbmsConnection.getConnection().close(); + void closeDbmsConnection() throws Exception { + connectorTest.close(); } @Test @@ -72,19 +72,18 @@ void insertEntry() throws SQLException { dbmsProcessor.insertEntry(expectedEntry); BibEntry emptyEntry = getBibEntryExample(); - emptyEntry.getSharedBibEntryData().setSharedID(1); - dbmsProcessor.insertEntry(emptyEntry); // does not insert, due to same sharedID. + emptyEntry.getSharedBibEntryData().setSharedId(1); 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 +101,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()); } } @@ -136,7 +135,7 @@ void updateEntry() throws Exception { expectedEntry.clearField(StandardField.BOOKTITLE); dbmsProcessor.updateEntry(expectedEntry); - Optional actualEntry = dbmsProcessor.getSharedEntry(expectedEntry.getSharedBibEntryData().getSharedID()); + Optional actualEntry = dbmsProcessor.getSharedEntry(expectedEntry.getSharedBibEntryData().getSharedIdAsInt()); assertEquals(Optional.of(expectedEntry), actualEntry); } @@ -150,7 +149,7 @@ void updateEmptyEntry() throws Exception { // Update field should now find the entry dbmsProcessor.updateEntry(expectedEntry); - Optional actualEntry = dbmsProcessor.getSharedEntry(expectedEntry.getSharedBibEntryData().getSharedID()); + Optional actualEntry = dbmsProcessor.getSharedEntry(expectedEntry.getSharedBibEntryData().getSharedIdAsInt()); assertEquals(Optional.of(expectedEntry), actualEntry); } @@ -192,7 +191,7 @@ void updateEqualEntry() throws OfflineLockException, SQLException { dbmsProcessor.updateEntry(expectedBibEntry); Optional actualBibEntryOptional = dbmsProcessor - .getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedID()); + .getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedIdAsInt()); assertEquals(Optional.of(expectedBibEntry), actualBibEntryOptional); } @@ -206,7 +205,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 +224,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 +237,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 +251,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()); } } @@ -263,6 +262,9 @@ void getSharedEntries() { dbmsProcessor.insertEntry(bibEntry); + bibEntry = getBibEntryExampleWithEmptyFields(); + bibEntry.getSharedBibEntryData().setSharedId(1); + List actualEntries = dbmsProcessor.getSharedEntries(); assertEquals(List.of(bibEntry), actualEntries); @@ -270,13 +272,16 @@ void getSharedEntries() { @Test void getSharedEntry() { - BibEntry expectedBibEntry = getBibEntryExampleWithEmptyFields(); + BibEntry bibEntry = getBibEntryExampleWithEmptyFields(); - dbmsProcessor.insertEntry(expectedBibEntry); + dbmsProcessor.insertEntry(bibEntry); - Optional actualBibEntryOptional = dbmsProcessor.getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedID()); + bibEntry = getBibEntryExampleWithEmptyFields(); + bibEntry.getSharedBibEntryData().setSharedId(1); - assertEquals(Optional.of(expectedBibEntry), actualBibEntryOptional); + Optional actualBibEntryOptional = dbmsProcessor.getSharedEntry(bibEntry.getSharedBibEntryData().getSharedIdAsInt()); + + assertEquals(Optional.of(bibEntry), actualBibEntryOptional); } @Test @@ -286,17 +291,18 @@ void getNotExistingSharedEntry() { } @Test - void getSharedIDVersionMapping() throws OfflineLockException, SQLException { + 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); Map expectedIDVersionMap = new HashMap<>(); - expectedIDVersionMap.put(firstEntry.getSharedBibEntryData().getSharedID(), 1); - expectedIDVersionMap.put(secondEntry.getSharedBibEntryData().getSharedID(), 2); + expectedIDVersionMap.put(firstEntry.getSharedBibEntryData().getSharedIdAsInt(), 1); + expectedIDVersionMap.put(secondEntry.getSharedBibEntryData().getSharedIdAsInt(), 2); Map actualIDVersionMap = dbmsProcessor.getSharedIDVersionMapping(); @@ -305,11 +311,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 +339,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; } @@ -344,7 +349,6 @@ private static BibEntry getBibEntryExampleWithEmptyFields() { .withField(StandardField.AUTHOR, "Author") .withField(StandardField.TITLE, "") .withField(StandardField.YEAR, ""); - bibEntry.getSharedBibEntryData().setSharedID(1); return bibEntry; } @@ -378,55 +382,50 @@ 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( - fieldResultSet.getString("NAME"), fieldResultSet.getString("VALUE")); - } else { - int sharedId = fieldResultSet.getInt("ENTRY_SHARED_ID"); - actualFieldMap.put(sharedId, - new HashMap<>()); - actualFieldMap.get(sharedId).put(fieldResultSet.getString("NAME"), - fieldResultSet.getString("VALUE")); - } + int sharedId = fieldResultSet.getInt("ENTRY_SHARED_ID"); + actualFieldMap.putIfAbsent(sharedId, new HashMap<>()); + actualFieldMap.get(sharedId).put( + fieldResultSet.getString("NAME"), + fieldResultSet.getString("VALUE")); } } } Map> expectedFieldMap = entries.stream() - .collect(Collectors.toMap(bibEntry -> bibEntry.getSharedBibEntryData().getSharedID(), + .collect(Collectors.toMap(bibEntry -> bibEntry.getSharedBibEntryData().getSharedIdAsInt(), bibEntry -> bibEntry.getFieldMap().entrySet().stream() .collect(Collectors.toMap(entry -> entry.getKey().getName(), Map.Entry::getValue)))); 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 +433,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..d57fd842e48 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,21 +48,21 @@ 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) .withField(StandardField.AUTHOR, "Wirthlin, Michael J" + index) .withField(StandardField.TITLE, "The nano processor" + index); - bibEntry.getSharedBibEntryData().setSharedID(index); + bibEntry.getSharedBibEntryData().setSharedId(index); return bibEntry; } @BeforeEach void setup() throws Exception { - this.dbmsType = TestManager.getDBMSTypeTestParameter(); - this.dbmsConnection = ConnectorTest.getTestDBMSConnection(dbmsType); - this.dbmsProcessor = DBMSProcessor.getProcessorInstance(this.dbmsConnection); + this.connectorTest = new ConnectorTest(); + this.dbmsConnection = connectorTest.getTestDBMSConnection(); + this.dbmsProcessor = new DBMSProcessor(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"); @@ -203,7 +207,7 @@ void synchronizeLocalDatabaseWithEntryUpdate() throws Exception { modifiedBibEntry.setType(StandardEntryType.Article); dbmsProcessor.updateEntry(modifiedBibEntry); - assertEquals(1, modifiedBibEntry.getSharedBibEntryData().getSharedID()); + assertEquals(1, modifiedBibEntry.getSharedBibEntryData().getSharedIdAsInt()); dbmsSynchronizer.synchronizeLocalDatabase(); assertEquals(List.of(modifiedBibEntry), bibDatabase.getEntries()); 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 f4d09b06904..7c8a1f682a7 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"); } }