From 443765f67ec35fd829db793cf11084bc64948aa5 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 22:22:09 +0200 Subject: [PATCH 01/47] Refactor method signature Co-authored-by: Christoph --- src/main/java/org/jabref/model/entry/BibEntry.java | 5 +---- .../org/jabref/model/entry/event/FieldChangedEvent.java | 7 +++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index f0d2d43689e..50977987027 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -373,10 +373,7 @@ public String getId() { @VisibleForTesting public void setId(String id) { Objects.requireNonNull(id, "Every BibEntry must have an ID"); - - String oldId = this.id; - - eventBus.post(new FieldChangedEvent(this, InternalField.INTERNAL_ID_FIELD, id, oldId)); + eventBus.post(new FieldChangedEvent(this, InternalField.INTERNAL_ID_FIELD, this.id, id)); this.id = id; changed = true; } diff --git a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java index c7d6255a813..3f1684e98bc 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -15,14 +15,13 @@ public class FieldChangedEvent extends EntryChangedEvent { private int majorCharacterChange = 0; /** + * @param location Location affected by this event * @param bibEntry Affected BibEntry object * @param field Name of field which has been changed * @param oldValue old field value * @param newValue new field value - * @param location Location affected by this event */ - public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String oldValue, - EntriesEventSource location) { + public FieldChangedEvent(EntriesEventSource location, BibEntry bibEntry, Field field, String oldValue, String newValue) { super(bibEntry, location); this.field = field; this.newValue = newValue; @@ -35,7 +34,7 @@ public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String * @param field Name of field which has been changed * @param newValue new field value */ - public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String oldValue) { + public FieldChangedEvent(BibEntry bibEntry, Field field, String oldValue, String newValue) { super(bibEntry); this.field = field; this.newValue = newValue; From f3921c25066504e68890bf1fc489a474cc03bfca Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 22:29:36 +0200 Subject: [PATCH 02/47] Rename maps Co-authored-by: Christoph --- .../jabref/model/database/BibDatabase.java | 47 +++++++++---------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/jabref/model/database/BibDatabase.java b/src/main/java/org/jabref/model/database/BibDatabase.java index a5d1d461851..4d53deaf992 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(); @@ -203,12 +200,12 @@ public synchronized void insertEntries(List newEntries, EntriesEventSo entry.registerListener(this); } if (newEntries.isEmpty()) { - eventBus.post(new EntriesAddedEvent(newEntries, eventSource)); + LOGGER.warn("No entries to insert"); } else { - eventBus.post(new EntriesAddedEvent(newEntries, newEntries.getFirst(), eventSource)); + eventBus.post(new EntriesAddedEvent(newEntries, eventSource)); } entries.addAll(newEntries); - newEntries.forEach(entry -> entriesId.put(entry.getId(), entry)); + newEntries.forEach(entry -> entryIdToBibEntry.put(entry.getId(), entry)); } public synchronized void removeEntry(BibEntry bibEntry) { @@ -245,7 +242,7 @@ public synchronized void removeEntries(List toBeDeleted, EntriesEventS } boolean anyRemoved = entries.removeIf(entry -> ids.contains(entry.getId())); if (anyRemoved) { - toBeDeleted.forEach(entry -> entriesId.remove(entry.getId())); + toBeDeleted.forEach(entry -> entryIdToBibEntry.remove(entry.getId())); eventBus.post(new EntriesRemovedEvent(toBeDeleted, eventSource)); } } @@ -279,11 +276,11 @@ public synchronized void addString(BibtexString string) throws KeyCollisionExcep throw new KeyCollisionException("A string with that label already exists", id); } - if (bibtexStrings.containsKey(id)) { + if (stringToBibtexString.containsKey(id)) { throw new KeyCollisionException("Duplicate BibTeX string id.", id); } - bibtexStrings.put(id, string); + stringToBibtexString.put(id, string); } /** @@ -293,7 +290,7 @@ public synchronized void addString(BibtexString string) throws KeyCollisionExcep * @param stringsToAdd The collection of strings to set */ public void setStrings(List stringsToAdd) { - bibtexStrings = new ConcurrentHashMap<>(); + stringToBibtexString = new ConcurrentHashMap<>(); stringsToAdd.forEach(this::addString); } @@ -301,7 +298,7 @@ public void setStrings(List stringsToAdd) { * Removes the string with the given id. */ public void removeString(String id) { - bibtexStrings.remove(id); + stringToBibtexString.remove(id); } /** @@ -309,7 +306,7 @@ public void removeString(String id) { * These are in no sorted order. */ public Set getStringKeySet() { - return bibtexStrings.keySet(); + return stringToBibtexString.keySet(); } /** @@ -317,14 +314,14 @@ public Set getStringKeySet() { * These are in no particular order. */ public Collection getStringValues() { - return bibtexStrings.values(); + return stringToBibtexString.values(); } /** * Returns the string with the given id. */ public BibtexString getString(String id) { - return bibtexStrings.get(id); + return stringToBibtexString.get(id); } /** @@ -338,14 +335,14 @@ public Optional getStringByName(String name) { * Returns the number of strings. */ public int getStringCount() { - return bibtexStrings.size(); + return stringToBibtexString.size(); } /** * Check if there are strings. */ public boolean hasNoStrings() { - return bibtexStrings.isEmpty(); + return stringToBibtexString.isEmpty(); } /** @@ -361,7 +358,7 @@ public void copyPreamble(BibDatabase database) { * Returns true if a string with the given label already exists. */ public synchronized boolean hasStringByName(String label) { - return bibtexStrings.values().stream().anyMatch(value -> value.getName().equals(label)); + return stringToBibtexString.values().stream().anyMatch(value -> value.getName().equals(label)); } /** @@ -391,7 +388,7 @@ public List getUsedStrings(Collection entries) { } } - return allUsedIds.stream().map(bibtexStrings::get).toList(); + return allUsedIds.stream().map(stringToBibtexString::get).toList(); } /** @@ -453,7 +450,7 @@ private String resolveString(String label, Set usedIds, Set allU Objects.requireNonNull(usedIds); Objects.requireNonNull(allUsedIds); - for (BibtexString string : bibtexStrings.values()) { + for (BibtexString string : stringToBibtexString.values()) { if (string.getName().equalsIgnoreCase(label)) { // First check if this string label has been resolved // earlier in this recursion. If so, we have a @@ -657,7 +654,7 @@ public int indexOf(BibEntry bibEntry) { } public BibEntry getEntryById(String id) { - return entriesId.get(id); + return entryIdToBibEntry.get(id); } @Override @@ -669,7 +666,7 @@ public boolean equals(Object o) { return false; } return Objects.equals(entries, that.entries) - && Objects.equals(bibtexStrings, that.bibtexStrings) + && Objects.equals(stringToBibtexString, that.stringToBibtexString) && Objects.equals(preamble, that.preamble) && Objects.equals(epilog, that.epilog) && Objects.equals(sharedDatabaseID, that.sharedDatabaseID) @@ -678,6 +675,6 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hash(entries, bibtexStrings, preamble, epilog, sharedDatabaseID, newLineSeparator); + return Objects.hash(entries, stringToBibtexString, preamble, epilog, sharedDatabaseID, newLineSeparator); } } From 92a2287ef2c6747d55449db8e45cd296151fd262 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 22:29:49 +0200 Subject: [PATCH 03/47] Cleanup EntriesAddedEvent Co-authored-by: Christoph --- .../database/event/EntriesAddedEvent.java | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) 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() { From f0465a6e0ab139848ba8b9e8e746f0cbe6eca925 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 22:30:56 +0200 Subject: [PATCH 04/47] Clean up EntriesRemovedEvent Co-authored-by: Christoph --- .../model/database/event/EntriesRemovedEvent.java | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) 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); } From 95fe7f6b1a0c764955768b6ddf29626778451e9c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 22:41:24 +0200 Subject: [PATCH 05/47] Disable save actions for SQL Co-authored-by: Christoph --- src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index d388de0b884..5afff4c10b1 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -247,7 +247,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())); From 279da5be3ce3426a61a178c592e8aa2ebbcba75c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 22:41:31 +0200 Subject: [PATCH 06/47] Fix logger string --- src/main/java/org/jabref/logic/shared/DBMSProcessor.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index d7ac69ee6f9..82b66641104 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -291,7 +291,7 @@ protected void insertIntoFieldTable(List bibEntries) { preparedFieldStatement.executeUpdate(); } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } @@ -302,6 +302,7 @@ protected void insertIntoFieldTable(List bibEntries) { * @throws SQLException in case of error */ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQLException { + // FIXME: either two connections (one with auto commit and one without) or better auto commit state - this line here can lead to issues if autocommit is required in a parallel thread connection.setAutoCommit(false); // disable auto commit due to transaction try { @@ -345,7 +346,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 From e19bc1ac01e1c05a68630b85d9c08d509ea45156 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 22:41:39 +0200 Subject: [PATCH 07/47] Add TODO Co-authored-by: Christoph --- src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 5afff4c10b1..ae69c716ccf 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -80,7 +80,9 @@ public void listen(EntriesAddedEvent event) { // While synchronizing the local database (see synchronizeLocalDatabase() below), some EntriesEvents may be posted. // In this case DBSynchronizer should not try to insert the bibEntry entry again (but it would not harm). if (isEventSourceAccepted(event) && checkCurrentConnection()) { + // TODO: Make use of org.jabref.model.metadata.event.MetaDataChangedEvent (and also do Added/Removed there) synchronizeLocalMetaData(); + pullWithLastEntry(); synchronizeLocalDatabase(); dbmsProcessor.insertEntries(event.getBibEntries()); From 90e067d0e56e21efd348a61b0c07e6b7e7f30c15 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 23:00:57 +0200 Subject: [PATCH 08/47] Remove MySQL and Oracle dependencies Co-authored-by: Christoph --- .github/workflows/tests.yml | 19 +- CHANGELOG.md | 2 +- build.gradle | 24 -- .../jabref/logic/shared/DBMSProcessor.java | 24 +- .../org/jabref/logic/shared/DBMSType.java | 4 +- .../jabref/logic/shared/MySQLProcessor.java | 86 ------- .../jabref/logic/shared/OracleProcessor.java | 221 ------------------ .../jabref/logic/shared/ConnectorTest.java | 11 +- .../shared/DBMSConnectionPropertiesTest.java | 20 -- .../org/jabref/logic/shared/DBMSTypeTest.java | 55 ----- .../org/jabref/logic/shared/TestManager.java | 34 +-- 11 files changed, 15 insertions(+), 485 deletions(-) delete mode 100644 src/main/java/org/jabref/logic/shared/MySQLProcessor.java delete mode 100644 src/main/java/org/jabref/logic/shared/OracleProcessor.java delete mode 100644 src/test/java/org/jabref/logic/shared/DBMSConnectionPropertiesTest.java diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ec81fc21db6..007acec06b9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -210,6 +210,8 @@ jobs: # 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: @@ -229,23 +231,6 @@ jobs: 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 52105585c0f..aaef3c8e8ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,7 +89,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We removed support for case-sensitive and exact search. [#11542](https://github.com/JabRef/jabref/pull/11542) - We removed the description of search strings. [#11542](https://github.com/JabRef/jabref/pull/11542) - We removed support for importing using the SilverPlatterImporter (`Record INSPEC`). [#11576](https://github.com/JabRef/jabref/pull/11576) - +- We removed support for MySQL/MariaDB and Oracle. diff --git a/build.gradle b/build.gradle index ff57188b2c0..bea8da0c1c1 100644 --- a/build.gradle +++ b/build.gradle @@ -205,18 +205,10 @@ dependencies { implementation 'com.fasterxml:aalto-xml:1.3.3' - implementation group: 'org.mariadb.jdbc', name: 'mariadb-java-client', version: '2.7.9' - implementation 'org.postgresql:postgresql:42.7.4' // Support unix socket connection types implementation 'com.kohlschutter.junixsocket:junixsocket-core:2.10.1' - implementation 'com.kohlschutter.junixsocket:junixsocket-mysql:2.10.0' - - implementation ('com.oracle.ojdbc:ojdbc10:19.3.0.0') { - // causing module issues - exclude module: 'oraclepki' - } implementation ('com.google.guava:guava:33.1.0-jre') { // TODO: Remove this as soon as https://github.com/google/guava/issues/2960 is fixed @@ -803,24 +795,8 @@ jlink { uses 'kong.unirest.core.json.JsonEngine' uses 'org.eclipse.jgit.lib.Signer' uses 'org.eclipse.jgit.transport.SshSessionFactory' - uses 'org.mariadb.jdbc.LocalInfileInterceptor' - uses 'org.mariadb.jdbc.authentication.AuthenticationPlugin' - uses 'org.mariadb.jdbc.credential.CredentialPlugin' - uses 'org.mariadb.jdbc.tls.TlsSocketPlugin' - provides 'org.mariadb.jdbc.tls.TlsSocketPlugin' with 'org.mariadb.jdbc.internal.protocol.tls.DefaultTlsSocketPlugin' provides 'java.sql.Driver' with 'org.postgresql.Driver' - provides 'org.mariadb.jdbc.authentication.AuthenticationPlugin' with 'org.mariadb.jdbc.internal.com.send.authentication.CachingSha2PasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.ClearPasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.Ed25519PasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.NativePasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.OldPasswordPlugin', - 'org.mariadb.jdbc.internal.com.send.authentication.SendGssApiAuthPacket', - 'org.mariadb.jdbc.internal.com.send.authentication.SendPamAuthPacket', - 'org.mariadb.jdbc.internal.com.send.authentication.Sha256PasswordPlugin' - provides 'org.mariadb.jdbc.credential.CredentialPlugin' with 'org.mariadb.jdbc.credential.aws.AwsIamCredentialPlugin', - 'org.mariadb.jdbc.credential.env.EnvCredentialPlugin', - 'org.mariadb.jdbc.credential.system.PropertiesCredentialPlugin' provides 'java.security.Provider' with 'org.bouncycastle.jce.provider.BouncyCastleProvider', 'org.bouncycastle.pqc.jcajce.provider.BouncyCastlePQCProvider' provides 'kong.unirest.core.json.JsonEngine' with 'kong.unirest.modules.gson.GsonEngine'; diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 82b66641104..30b4236fcaa 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -59,16 +59,12 @@ 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; } @@ -667,15 +663,7 @@ public void setSharedMetaData(Map data) throws SQLException { * Returns a new instance of the abstract type {@link DBMSProcessor} */ public static DBMSProcessor getProcessorInstance(DatabaseConnection connection) { - DBMSType type = connection.getProperties().getType(); - if (type == DBMSType.MYSQL) { - return new MySQLProcessor(connection); - } else if (type == DBMSType.POSTGRESQL) { - return new PostgreSQLProcessor(connection); - } else if (type == DBMSType.ORACLE) { - return new OracleProcessor(connection); - } - return null; // can never happen except new types were added without updating this method. + return new PostgreSQLProcessor(connection); } public DatabaseConnectionProperties getDBMSConnectionProperties() { diff --git a/src/main/java/org/jabref/logic/shared/DBMSType.java b/src/main/java/org/jabref/logic/shared/DBMSType.java index 8a3717abaf0..6851e0daf6f 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSType.java +++ b/src/main/java/org/jabref/logic/shared/DBMSType.java @@ -7,9 +7,7 @@ * Enumerates all supported database systems (DBMS) by JabRef. */ public enum DBMSType { - POSTGRESQL("PostgreSQL", "org.postgresql.Driver", "jdbc:postgresql://%s:%d/%s", 5432), - MYSQL("MySQL", "org.mariadb.jdbc.Driver", "jdbc:mariadb://%s:%d/%s", 3306), - ORACLE("Oracle", "oracle.jdbc.driver.OracleDriver", "jdbc:oracle:thin:@%s:%d/%s", 1521); + POSTGRESQL("PostgreSQL", "org.postgresql.Driver", "jdbc:postgresql://%s:%d/%s", 5432); private final String type; private final String driverPath; diff --git a/src/main/java/org/jabref/logic/shared/MySQLProcessor.java b/src/main/java/org/jabref/logic/shared/MySQLProcessor.java deleted file mode 100644 index 906fbaf50e8..00000000000 --- a/src/main/java/org/jabref/logic/shared/MySQLProcessor.java +++ /dev/null @@ -1,86 +0,0 @@ -package org.jabref.logic.shared; - -import java.sql.SQLException; -import java.util.Map; - -import org.jabref.model.metadata.MetaData; - -/** - * Processes all incoming or outgoing bib data to MySQL Database and manages its structure. - */ -public class MySQLProcessor extends DBMSProcessor { - - private Integer VERSION_DB_STRUCT_DEFAULT = -1; - private Integer CURRENT_VERSION_DB_STRUCT = 1; - - public MySQLProcessor(DatabaseConnection connection) { - super(connection); - } - - /** - * Creates and sets up the needed tables and columns according to the database type. - * - * @throws SQLException - */ - @Override - public void setUp() throws SQLException { - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS `JABREF_ENTRY` (" + - "`SHARED_ID` INT(11) NOT NULL PRIMARY KEY AUTO_INCREMENT, " + - "`TYPE` VARCHAR(255) NOT NULL, " + - "`VERSION` INT(11) DEFAULT 1)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS `JABREF_FIELD` (" + - "`ENTRY_SHARED_ID` INT(11) NOT NULL, " + - "`NAME` VARCHAR(255) NOT NULL, " + - "`VALUE` TEXT DEFAULT NULL, " + - "FOREIGN KEY (`ENTRY_SHARED_ID`) REFERENCES `JABREF_ENTRY`(`SHARED_ID`) ON DELETE CASCADE)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS `JABREF_METADATA` (" + - "`KEY` varchar(255) NOT NULL," + - "`VALUE` text NOT NULL)"); - - Map metadata = getSharedMetaData(); - - if (metadata.get(MetaData.VERSION_DB_STRUCT) != null) { - try { - VERSION_DB_STRUCT_DEFAULT = Integer.valueOf(metadata.get(MetaData.VERSION_DB_STRUCT)); - } catch (Exception e) { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Integer!"); - } - } else { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Exist!"); - } - - if (VERSION_DB_STRUCT_DEFAULT < CURRENT_VERSION_DB_STRUCT) { - // We can to migrate from old table in new table - if (CURRENT_VERSION_DB_STRUCT == 1 && checkTableAvailability("ENTRY", "FIELD", "METADATA")) { - LOGGER.info("Migrating from VersionDBStructure == 0"); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("ENTRY") + " SELECT * FROM `ENTRY`"); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("FIELD") + " SELECT * FROM `FIELD`"); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("METADATA") + " SELECT * FROM `METADATA`"); - metadata = getSharedMetaData(); - } - - metadata.put(MetaData.VERSION_DB_STRUCT, CURRENT_VERSION_DB_STRUCT.toString()); - setSharedMetaData(metadata); - } - } - - @Override - String escape(String expression) { - return "`" + expression + "`"; - } - - @Override - String escape_Table(String expression) { - return escape("JABREF_" + expression); - } - - @Override - Integer getCURRENT_VERSION_DB_STRUCT() { - return CURRENT_VERSION_DB_STRUCT; - } -} diff --git a/src/main/java/org/jabref/logic/shared/OracleProcessor.java b/src/main/java/org/jabref/logic/shared/OracleProcessor.java deleted file mode 100644 index 3221a211808..00000000000 --- a/src/main/java/org/jabref/logic/shared/OracleProcessor.java +++ /dev/null @@ -1,221 +0,0 @@ -package org.jabref.logic.shared; - -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.sql.Statement; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.stream.Collectors; - -import org.jabref.logic.shared.listener.OracleNotificationListener; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.field.Field; -import org.jabref.model.metadata.MetaData; - -import oracle.jdbc.OracleConnection; -import oracle.jdbc.OracleStatement; -import oracle.jdbc.dcn.DatabaseChangeRegistration; - -/** - * Processes all incoming or outgoing bib data to Oracle database and manages its structure. - */ -public class OracleProcessor extends DBMSProcessor { - - private OracleConnection oracleConnection; - - private OracleNotificationListener listener; - - private DatabaseChangeRegistration databaseChangeRegistration; - - private Integer VERSION_DB_STRUCT_DEFAULT = -1; - private Integer CURRENT_VERSION_DB_STRUCT = 0; - - public OracleProcessor(DatabaseConnection connection) { - super(connection); - } - - /** - * Creates and sets up the needed tables and columns according to the database type. - * - * @throws SQLException - */ - @Override - public void setUp() throws SQLException { - connection.createStatement().executeUpdate( - "CREATE TABLE \"ENTRY\" (" + - "\"SHARED_ID\" NUMBER NOT NULL, " + - "\"TYPE\" VARCHAR2(255) NULL, " + - "\"VERSION\" NUMBER DEFAULT 1, " + - "CONSTRAINT \"ENTRY_PK\" PRIMARY KEY (\"SHARED_ID\"))"); - - connection.createStatement().executeUpdate("CREATE SEQUENCE \"ENTRY_SEQ\""); - - connection.createStatement().executeUpdate("CREATE TRIGGER \"ENTRY_T\" BEFORE INSERT ON \"ENTRY\" " + - "FOR EACH ROW BEGIN SELECT \"ENTRY_SEQ\".NEXTVAL INTO :NEW.shared_id FROM DUAL; END;"); - - connection.createStatement().executeUpdate( - "CREATE TABLE \"FIELD\" (" + - "\"ENTRY_SHARED_ID\" NUMBER NOT NULL, " + - "\"NAME\" VARCHAR2(255) NOT NULL, " + - "\"VALUE\" CLOB NULL, " + - "CONSTRAINT \"ENTRY_SHARED_ID_FK\" FOREIGN KEY (\"ENTRY_SHARED_ID\") " + - "REFERENCES \"ENTRY\"(\"SHARED_ID\") ON DELETE CASCADE)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE \"METADATA\" (" + - "\"KEY\" VARCHAR2(255) NULL," + - "\"VALUE\" CLOB NOT NULL)"); - - Map metadata = getSharedMetaData(); - - if (metadata.get(MetaData.VERSION_DB_STRUCT) != null) { - try { - VERSION_DB_STRUCT_DEFAULT = Integer.valueOf(metadata.get(MetaData.VERSION_DB_STRUCT)); - } catch (Exception e) { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Integer!"); - } - } else { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Exist!"); - } - - if (VERSION_DB_STRUCT_DEFAULT < CURRENT_VERSION_DB_STRUCT) { - // We can to migrate from old table in new table - metadata.put(MetaData.VERSION_DB_STRUCT, CURRENT_VERSION_DB_STRUCT.toString()); - setSharedMetaData(metadata); - } - } - - @Override - String escape(String expression) { - return expression; - } - - @Override - String escape_Table(String expression) { - return escape(expression); - } - - @Override - Integer getCURRENT_VERSION_DB_STRUCT() { - return CURRENT_VERSION_DB_STRUCT; - } - - @Override - public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) { - this.listener = new OracleNotificationListener(dbmsSynchronizer); - - try { - oracleConnection = (OracleConnection) connection; - - Properties properties = new Properties(); - properties.setProperty(OracleConnection.DCN_NOTIFY_ROWIDS, "true"); - properties.setProperty(OracleConnection.DCN_QUERY_CHANGE_NOTIFICATION, "true"); - - databaseChangeRegistration = oracleConnection.registerDatabaseChangeNotification(properties); - databaseChangeRegistration.addListener(listener); - - try (Statement statement = oracleConnection.createStatement()) { - ((OracleStatement) statement).setDatabaseChangeRegistration(databaseChangeRegistration); - StringBuilder selectQuery = new StringBuilder() - .append("SELECT 1 FROM ") - .append(escape_Table("ENTRY")) - .append(", ") - .append(escape_Table("METADATA")); - // this execution registers all tables mentioned in selectQuery - statement.executeQuery(selectQuery.toString()); - } - } catch (SQLException e) { - LOGGER.error("SQL Error during starting the notification listener", e); - } - } - - @Override - protected void insertIntoEntryTable(List entries) { - try { - for (BibEntry entry : entries) { - String insertIntoEntryQuery = - "INSERT INTO " + - escape_Table("ENTRY") + - "(" + - escape("TYPE") + - ") VALUES(?)"; - - try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery, - new String[]{"SHARED_ID"})) { - - preparedEntryStatement.setString(1, entry.getType().getName()); - preparedEntryStatement.executeUpdate(); - - try (ResultSet generatedKeys = preparedEntryStatement.getGeneratedKeys()) { - if (generatedKeys.next()) { - entry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); // set generated ID locally - } - } - } - } - } catch (SQLException e) { - LOGGER.error("SQL Error during entry insertion", e); - } - } - - @Override - protected void insertIntoFieldTable(List bibEntries) { - try { - // Inserting into FIELD table - // Coerce to ArrayList in order to use List.get() - List> fields = bibEntries.stream().map(entry -> new ArrayList<>(entry.getFields())) - .collect(Collectors.toList()); - StringBuilder insertFieldQuery = new StringBuilder() - .append("INSERT ALL"); - int numFields = 0; - for (List entryFields : fields) { - numFields += entryFields.size(); - } - for (int i = 0; i < numFields; i++) { - insertFieldQuery.append(" INTO ") - .append(escape_Table("FIELD")) - .append(" (") - .append(escape("ENTRY_SHARED_ID")) - .append(", ") - .append(escape("NAME")) - .append(", ") - .append(escape("VALUE")) - .append(") VALUES (?, ?, ?)"); - } - insertFieldQuery.append(" SELECT * FROM DUAL"); - try (PreparedStatement preparedFieldStatement = connection.prepareStatement(insertFieldQuery.toString())) { - int fieldsCompleted = 0; - for (int entryIndex = 0; entryIndex < fields.size(); entryIndex++) { - for (int entryFieldsIndex = 0; entryFieldsIndex < fields.get(entryIndex).size(); entryFieldsIndex++) { - // columnIndex starts with 1 - preparedFieldStatement.setInt((3 * fieldsCompleted) + 1, bibEntries.get(entryIndex).getSharedBibEntryData().getSharedID()); - preparedFieldStatement.setString((3 * fieldsCompleted) + 2, fields.get(entryIndex).get(entryFieldsIndex).getName()); - preparedFieldStatement.setString((3 * fieldsCompleted) + 3, bibEntries.get(entryIndex).getField(fields.get(entryIndex).get(entryFieldsIndex)).get()); - fieldsCompleted += 1; - } - } - preparedFieldStatement.executeUpdate(); - } - } catch (SQLException e) { - LOGGER.error("SQL Error during field insertion", e); - } - } - - @Override - public void stopNotificationListener() { - try { - oracleConnection.unregisterDatabaseChangeNotification(databaseChangeRegistration); - oracleConnection.close(); - } catch (SQLException e) { - LOGGER.error("SQL Error during stopping the notification listener", e); - } - } - - @Override - public void notifyClients() { - // Do nothing because Oracle triggers notifications automatically. - } -} diff --git a/src/test/java/org/jabref/logic/shared/ConnectorTest.java b/src/test/java/org/jabref/logic/shared/ConnectorTest.java index fabcf68b1b1..c1542264dab 100644 --- a/src/test/java/org/jabref/logic/shared/ConnectorTest.java +++ b/src/test/java/org/jabref/logic/shared/ConnectorTest.java @@ -17,15 +17,6 @@ public static DBMSConnection getTestDBMSConnection(DBMSType dbmsType) throws SQL } 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(); - } + return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("localhost").setPort(dbmsType.getDefaultPort()).setDatabase("postgres").setUser("postgres").setPassword("postgres").setUseSSL(false).createDBMSConnectionProperties(); } } 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/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/TestManager.java b/src/test/java/org/jabref/logic/shared/TestManager.java index 954bbccbb5c..8c110ab9b5b 100644 --- a/src/test/java/org/jabref/logic/shared/TestManager.java +++ b/src/test/java/org/jabref/logic/shared/TestManager.java @@ -18,35 +18,9 @@ public static DBMSType getDBMSTypeTestParameter() { 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 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"); } } From 35d64112bf7e1ef0a19955428bd1de105cfb22bf Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 23:04:06 +0200 Subject: [PATCH 09/47] Remove PullChangesFromSharedAction Co-authored-by: Christoph --- .../jabref/gui/actions/StandardActions.java | 1 - .../java/org/jabref/gui/frame/MainMenu.java | 4 +--- .../org/jabref/gui/keyboard/KeyBinding.java | 1 - .../presets/NewEntryBindingPreset.java | 1 - .../shared/PullChangesFromSharedAction.java | 24 ------------------- src/main/resources/l10n/JabRef_en.properties | 1 - 6 files changed, 1 insertion(+), 31 deletions(-) delete mode 100644 src/main/java/org/jabref/gui/shared/PullChangesFromSharedAction.java diff --git a/src/main/java/org/jabref/gui/actions/StandardActions.java b/src/main/java/org/jabref/gui/actions/StandardActions.java index 7f44fb8810f..2859f552475 100644 --- a/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -75,7 +75,6 @@ public enum StandardActions implements Action { REMOTE_DB(Localization.lang("Shared database"), IconTheme.JabRefIcons.REMOTE_DATABASE), EXPORT_SELECTED(Localization.lang("Export selected entries"), KeyBinding.EXPORT_SELECTED), CONNECT_TO_SHARED_DB(Localization.lang("Connect to shared database"), IconTheme.JabRefIcons.CONNECT_DB), - PULL_CHANGES_FROM_SHARED_DB(Localization.lang("Pull changes from shared database"), KeyBinding.PULL_CHANGES_FROM_SHARED_DATABASE), CLOSE_LIBRARY(Localization.lang("Close library"), Localization.lang("Close the current library"), IconTheme.JabRefIcons.CLOSE, KeyBinding.CLOSE_DATABASE), CLOSE_OTHER_LIBRARIES(Localization.lang("Close others"), Localization.lang("Close other libraries"), IconTheme.JabRefIcons.CLOSE), CLOSE_ALL_LIBRARIES(Localization.lang("Close all"), Localization.lang("Close all libraries"), IconTheme.JabRefIcons.CLOSE), diff --git a/src/main/java/org/jabref/gui/frame/MainMenu.java b/src/main/java/org/jabref/gui/frame/MainMenu.java index 7500cf72e03..42d285a9e5b 100644 --- a/src/main/java/org/jabref/gui/frame/MainMenu.java +++ b/src/main/java/org/jabref/gui/frame/MainMenu.java @@ -62,7 +62,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; @@ -171,8 +170,7 @@ private void createMenu() { new SeparatorMenuItem(), factory.createSubMenu(StandardActions.REMOTE_DB, - factory.createMenuItem(StandardActions.CONNECT_TO_SHARED_DB, new ConnectToSharedDatabaseCommand(frame, dialogService)), - factory.createMenuItem(StandardActions.PULL_CHANGES_FROM_SHARED_DB, new PullChangesFromSharedAction(stateManager))), + factory.createMenuItem(StandardActions.CONNECT_TO_SHARED_DB, new ConnectToSharedDatabaseCommand(frame, dialogService))), new SeparatorMenuItem(), diff --git a/src/main/java/org/jabref/gui/keyboard/KeyBinding.java b/src/main/java/org/jabref/gui/keyboard/KeyBinding.java index d0cb6100338..541f39a658e 100644 --- a/src/main/java/org/jabref/gui/keyboard/KeyBinding.java +++ b/src/main/java/org/jabref/gui/keyboard/KeyBinding.java @@ -90,7 +90,6 @@ public enum KeyBinding { SHOW_PREFS("Preferences", Localization.lang("Preferences"), "ctrl+,", KeyBindingCategory.FILE), OPEN_URL_OR_DOI("Open URL or DOI", Localization.lang("Open URL or DOI"), "F3", KeyBindingCategory.TOOLS), PASTE("Paste", Localization.lang("Paste"), "ctrl+V", KeyBindingCategory.EDIT), - PULL_CHANGES_FROM_SHARED_DATABASE("Pull changes from shared database", Localization.lang("Pull changes from shared database"), "ctrl+shift+R", KeyBindingCategory.FILE), PREVIOUS_PREVIEW_LAYOUT("Previous preview layout", Localization.lang("Previous preview layout"), "shift+F9", KeyBindingCategory.VIEW), PREVIOUS_LIBRARY("Previous library", Localization.lang("Previous library"), "ctrl+PAGE_UP", KeyBindingCategory.VIEW), SCROLL_TO_NEXT_MATCH_CATEGORY("Scroll to next match category", Localization.lang("Scroll to next match category"), "right", KeyBindingCategory.VIEW), diff --git a/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java b/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java index dc4993e67ab..0b469a3cecf 100644 --- a/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java +++ b/src/main/java/org/jabref/gui/preferences/keybindings/presets/NewEntryBindingPreset.java @@ -18,7 +18,6 @@ public Map getKeyBindings() { final Map keyBindings = new HashMap<>(); // Clear conflicting default presets - keyBindings.put(KeyBinding.PULL_CHANGES_FROM_SHARED_DATABASE, ""); keyBindings.put(KeyBinding.COPY_PREVIEW, ""); // Add new entry presets diff --git a/src/main/java/org/jabref/gui/shared/PullChangesFromSharedAction.java b/src/main/java/org/jabref/gui/shared/PullChangesFromSharedAction.java deleted file mode 100644 index d19090cf367..00000000000 --- a/src/main/java/org/jabref/gui/shared/PullChangesFromSharedAction.java +++ /dev/null @@ -1,24 +0,0 @@ -package org.jabref.gui.shared; - -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.ActionHelper; -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.logic.shared.DatabaseSynchronizer; - -public class PullChangesFromSharedAction extends SimpleCommand { - - private final StateManager stateManager; - - public PullChangesFromSharedAction(StateManager stateManager) { - this.stateManager = stateManager; - - this.executable.bind(ActionHelper.needsDatabase(stateManager).and(ActionHelper.needsSharedDatabase(stateManager))); - } - - public void execute() { - stateManager.getActiveDatabase().ifPresent(databaseContext -> { - DatabaseSynchronizer dbmsSynchronizer = databaseContext.getDBMSSynchronizer(); - dbmsSynchronizer.pullChanges(); - }); - } -} diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 454a38cf4a6..8e4befbde7b 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -699,7 +699,6 @@ Selected\ Layouts\ can\ not\ be\ empty=Selected Layouts can not be empty Reset\ default\ preview\ style=Reset default preview style Previous\ entry=Previous entry Problem\ with\ parsing\ entry=Problem with parsing entry -Pull\ changes\ from\ shared\ database=Pull changes from shared database Pushed\ citations\ to\ %0=Pushed citations to %0 From f4f40c68c238328b1a1b8b6ec0a4550c0de8694a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 3 Oct 2024 23:22:49 +0200 Subject: [PATCH 10/47] Improve doc and code comments Co-authored-by: Christoph --- docs/code-howtos/remote-storage-sql.md | 15 +++++++++++++-- .../jabref/logic/shared/PostgreSQLProcessor.java | 1 - .../listener/PostgresSQLNotificationListener.java | 6 +++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/docs/code-howtos/remote-storage-sql.md b/docs/code-howtos/remote-storage-sql.md index 267d05129fe..cf0772ad9d9 100644 --- a/docs/code-howtos/remote-storage-sql.md +++ b/docs/code-howtos/remote-storage-sql.md @@ -9,14 +9,25 @@ For user documentation, see . + +## Handling synchronization of "micro-edits" + +It causes too much load both on the server and at all subscribed clients to synchronize every single letter change. +Therefore, synchronization only happens if several conditions are fulfilled: * Edit to another field. * Major changes have been made (pasting or deleting more than one character). Class `org.jabref.logic.util.CoarseChangeFilter.java` checks both conditions. -Remaining changes that have not been synchronized yet are saved at closing the database rendering additional closing time. Saving is realized in `org.jabref.logic.shared.DBMSSynchronizer.java`. Following methods account for synchronization modes: +Remaining changes that have not been synchronized yet are saved at closing the database rendering additional closing time. +Saving is realized in `org.jabref.logic.shared.DBMSSynchronizer.java`. + +Following methods account for synchronization modes: * `pullChanges` synchronizes the database unconditionally. * `pullLastEntryChanges` synchronizes only if there are remaining entry changes. It is invoked when closing the shared database (`closeSharedDatabase`). diff --git a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java index 21b04d09c6a..5143dbc882a 100644 --- a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java +++ b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java @@ -35,7 +35,6 @@ public PostgreSQLProcessor(DatabaseConnection connection) { */ @Override public void setUp() throws SQLException { - if (CURRENT_VERSION_DB_STRUCT == 1 && checkTableAvailability("ENTRY", "FIELD", "METADATA")) { // checkTableAvailability does not distinguish if same table name exists in different schemas // VERSION_DB_STRUCT_DEFAULT must be forced diff --git a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java b/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java index 96b8b7ccaf6..59ef9258874 100644 --- a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java +++ b/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java @@ -30,13 +30,13 @@ public PostgresSQLNotificationListener(DBMSSynchronizer dbmsSynchronizer, PGConn public void run() { stop = false; try { - // noinspection InfiniteLoopStatement - while (!stop) { - PGNotification notifications[] = pgConnection.getNotifications(); + while (!stop && !Thread.currentThread().isInterrupted()) { + PGNotification[] notifications = pgConnection.getNotifications(); if (notifications != null) { for (PGNotification notification : notifications) { if (!notification.getName().equals(DBMSProcessor.PROCESSOR_ID)) { + // Only process notifications that are not sent by this processor dbmsSynchronizer.pullChanges(); } } From 7e459befac2fb07808a26d25d11a5988daef7298 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 14:03:13 +0200 Subject: [PATCH 11/47] Mark variable to be removed --- .../jabref/logic/shared/DBMSSynchronizer.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index ae69c716ccf..795c8d99517 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -56,7 +56,7 @@ public class DBMSSynchronizer implements DatabaseSynchronizer { private final GlobalCitationKeyPatterns globalCiteKeyPattern; private final FieldPreferences fieldPreferences; private final FileUpdateMonitor fileMonitor; - private Optional lastEntryChanged; + private Optional lastEntryChanged_REMOVEME; public DBMSSynchronizer(BibDatabaseContext bibDatabaseContext, Character keywordSeparator, FieldPreferences fieldPreferences, @@ -69,7 +69,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(); } /** @@ -87,27 +87,29 @@ public void listen(EntriesAddedEvent event) { synchronizeLocalDatabase(); dbmsProcessor.insertEntries(event.getBibEntries()); // Reset last changed entry because it just has already been synchronized -> Why necessary? - lastEntryChanged = Optional.empty(); + lastEntryChanged_REMOVEME = Optional.empty(); } } /** * Listening method. Updates an existing shared {@link BibEntry}. + * + * In JabRef (UI), the field is modified */ @Subscribe public void listen(FieldChangedEvent event) { + if (event.isFilteredOut() || !isEventSourceAccepted(event) || !checkCurrentConnection()) { + return; + } + BibEntry bibEntry = event.getBibEntry(); // While synchronizing the local database (see synchronizeLocalDatabase() below), some EntriesEvents may be posted. // In this case DBSynchronizer should not try to update the bibEntry entry again (but it would not harm). - if (isPresentLocalBibEntry(bibEntry) && isEventSourceAccepted(event) && checkCurrentConnection() && !event.isFilteredOut()) { + synchronizeLocalMetaData(); pullWithLastEntry(); synchronizeSharedEntry(bibEntry); synchronizeLocalDatabase(); // Pull changes for the case that there were some - } else { - // Set new BibEntry that has been changed last - lastEntryChanged = Optional.of(bibEntry); - } } /** @@ -331,7 +333,7 @@ public void pullChanges() { * Synchronizes local BibEntries only if last entry changes still remain */ public void pullLastEntryChanges() { - if (lastEntryChanged.isPresent()) { + if (lastEntryChanged_REMOVEME.isPresent()) { if (!checkCurrentConnection()) { return; } @@ -346,10 +348,10 @@ public void pullLastEntryChanges() { * Synchronizes local BibEntries and pulls remaining last entry changes */ private void pullWithLastEntry() { - if (lastEntryChanged.isPresent() && isPresentLocalBibEntry(lastEntryChanged.get())) { - synchronizeSharedEntry(lastEntryChanged.get()); + if (lastEntryChanged_REMOVEME.isPresent() && isPresentLocalBibEntry(lastEntryChanged_REMOVEME.get())) { + synchronizeSharedEntry(lastEntryChanged_REMOVEME.get()); } - lastEntryChanged = Optional.empty(); + lastEntryChanged_REMOVEME = Optional.empty(); } /** From f90115101ec3fc93f41e4e55bdb04af8d99a0354 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 14:27:29 +0200 Subject: [PATCH 12/47] Fix class name --- src/main/java/org/jabref/model/database/BibDatabase.java | 2 +- .../{KeyChangeListener.java => CitationKeyListener.java} | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) rename src/main/java/org/jabref/model/database/{KeyChangeListener.java => CitationKeyListener.java} (93%) diff --git a/src/main/java/org/jabref/model/database/BibDatabase.java b/src/main/java/org/jabref/model/database/BibDatabase.java index 4d53deaf992..2bd7c84b86c 100644 --- a/src/main/java/org/jabref/model/database/BibDatabase.java +++ b/src/main/java/org/jabref/model/database/BibDatabase.java @@ -78,7 +78,7 @@ public BibDatabase(List entries) { } public BibDatabase() { - this.registerListener(new KeyChangeListener(this)); + this.registerListener(new CitationKeyListener(this)); } /** diff --git a/src/main/java/org/jabref/model/database/KeyChangeListener.java b/src/main/java/org/jabref/model/database/CitationKeyListener.java similarity index 93% rename from src/main/java/org/jabref/model/database/KeyChangeListener.java rename to src/main/java/org/jabref/model/database/CitationKeyListener.java index 6dc601971f1..5ac99511c2f 100644 --- a/src/main/java/org/jabref/model/database/KeyChangeListener.java +++ b/src/main/java/org/jabref/model/database/CitationKeyListener.java @@ -14,11 +14,14 @@ import com.google.common.eventbus.Subscribe; -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; } From f6a2fe5b13edbbf6c05a7a6c9f0b7bb68217b117 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 14:32:32 +0200 Subject: [PATCH 13/47] Introduce BibEntry.ENTRY_LINK_SEPARATOR --- .../java/org/jabref/model/database/CitationKeyListener.java | 2 +- src/main/java/org/jabref/model/entry/BibEntry.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/model/database/CitationKeyListener.java b/src/main/java/org/jabref/model/database/CitationKeyListener.java index 5ac99511c2f..51e2c975316 100644 --- a/src/main/java/org/jabref/model/database/CitationKeyListener.java +++ b/src/main/java/org/jabref/model/database/CitationKeyListener.java @@ -59,7 +59,7 @@ private void updateEntryLinks(String newKey, String oldKey) { } private void replaceKeyInMultiplesKeyField(String newKey, String oldKey, BibEntry entry, Field field, String fieldContent) { - List keys = new ArrayList<>(Arrays.asList(fieldContent.split(","))); + 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/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index 50977987027..8cab3df558a 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -96,7 +96,11 @@ public class BibEntry implements Cloneable { public static final EntryType DEFAULT_TYPE = StandardEntryType.Misc; + + public static final String ENTRY_LINK_SEPARATOR = ","; + private static final Logger LOGGER = LoggerFactory.getLogger(BibEntry.class); + private final SharedBibEntryData sharedBibEntryData; /** From 8a11cb9a5ca693524f19cd1c8743185f6966c21f Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 14:32:45 +0200 Subject: [PATCH 14/47] Cleanup code in CitationKeyListener --- .../model/database/CitationKeyListener.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/model/database/CitationKeyListener.java b/src/main/java/org/jabref/model/database/CitationKeyListener.java index 51e2c975316..afc5a9d22ce 100644 --- a/src/main/java/org/jabref/model/database/CitationKeyListener.java +++ b/src/main/java/org/jabref/model/database/CitationKeyListener.java @@ -13,6 +13,7 @@ import org.jabref.model.entry.field.InternalField; import com.google.common.eventbus.Subscribe; +import org.jspecify.annotations.Nullable; /** * Updates references of citation keys if the citation key of an entry is changed. @@ -28,9 +29,7 @@ public CitationKeyListener(BibDatabase 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()); } } @@ -39,11 +38,11 @@ public void listen(EntriesRemovedEvent event) { List entries = event.getBibEntries(); for (BibEntry entry : entries) { Optional citeKey = entry.getCitationKey(); - citeKey.ifPresent(oldkey -> updateEntryLinks(null, oldkey)); + citeKey.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 -> { @@ -53,12 +52,15 @@ 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) { + /** + * @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) { From fd958b6d065c2c410991b1a130d9711ca01f59d9 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 14:34:10 +0200 Subject: [PATCH 15/47] More Java 8 --- .../org/jabref/model/database/CitationKeyListener.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/model/database/CitationKeyListener.java b/src/main/java/org/jabref/model/database/CitationKeyListener.java index afc5a9d22ce..e5607525b8c 100644 --- a/src/main/java/org/jabref/model/database/CitationKeyListener.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; @@ -35,11 +34,9 @@ public void listen(FieldChangedEvent event) { @Subscribe public void listen(EntriesRemovedEvent event) { - List entries = event.getBibEntries(); - for (BibEntry entry : entries) { - Optional citeKey = entry.getCitationKey(); - citeKey.ifPresent(oldkey -> updateEntryLinks(oldkey, null)); - } + event.getBibEntries().stream() + .forEach(entry -> entry.getCitationKey() + .ifPresent(oldkey -> updateEntryLinks(oldkey, null))); } private void updateEntryLinks(String oldKey, @Nullable String newKey) { From fd7697bde38c5b0e2ffffe0478712ed860ae3637 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 15:22:03 +0200 Subject: [PATCH 16/47] Minor code updates --- .../java/org/jabref/logic/exporter/MetaDataSerializer.java | 2 +- src/main/java/org/jabref/logic/shared/DBMSProcessor.java | 4 ++-- src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java | 2 +- .../java/org/jabref/model/entry/event/FieldChangedEvent.java | 4 ++-- .../java/org/jabref/logic/shared/DBMSSynchronizerTest.java | 4 ++++ 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java b/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java index 4b8f625ea15..931b19bc2b7 100644 --- a/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java +++ b/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java @@ -31,7 +31,7 @@ private MetaDataSerializer() { } /** - * Writes all data in the format <key, serialized data>. + * Writes all data in the format {@code }. */ public static Map getSerializedStringMap(MetaData metaData, GlobalCitationKeyPatterns globalCiteKeyPatterns) { diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 30b4236fcaa..4a98fe1204b 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -650,11 +650,11 @@ public void setSharedMetaData(Map data) throws SQLException { insertStatement.setString(2, metaEntry.getValue()); insertStatement.executeUpdate(); } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } } diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 795c8d99517..06fb93bd67c 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -290,7 +290,7 @@ private void synchronizeSharedMetaData(MetaData data, GlobalCitationKeyPatterns try { dbmsProcessor.setSharedMetaData(MetaDataSerializer.getSerializedStringMap(data, globalCiteKeyPattern)); } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } 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 3f1684e98bc..51d0d6606e2 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -24,8 +24,8 @@ public class FieldChangedEvent extends EntryChangedEvent { public FieldChangedEvent(EntriesEventSource location, BibEntry bibEntry, Field field, String oldValue, String newValue) { super(bibEntry, location); this.field = field; - this.newValue = newValue; this.oldValue = oldValue; + this.newValue = newValue; this.majorCharacterChange = computeMajorCharacterChange(oldValue, newValue); } @@ -57,7 +57,7 @@ public FieldChangedEvent(FieldChange fieldChange) { this(fieldChange, EntriesEventSource.LOCAL); } - private int computeMajorCharacterChange(String oldValue, String newValue) { + private static int computeMajorCharacterChange(String oldValue, String newValue) { // We do == because of performance reasons if (oldValue == newValue) { return 0; diff --git a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java index 5983b345ade..0463ea15b32 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 { @@ -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"); From 770cef0f9004b613101ed17c11649328de66ef60 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 15:22:27 +0200 Subject: [PATCH 17/47] Begin: separate metadata sync with all-entry-sync --- .../org/jabref/logic/shared/DBMSSynchronizer.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 06fb93bd67c..74b92519a89 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; @@ -134,9 +133,7 @@ public void listen(EntriesRemovedEvent event) { public void listen(MetaDataChangedEvent event) { if (checkCurrentConnection()) { synchronizeSharedMetaData(event.getMetaData(), globalCiteKeyPattern); - synchronizeLocalDatabase(); - applyMetaData(); - dbmsProcessor.notifyClients(); + // TODO: applyMetaData(); } } @@ -289,6 +286,7 @@ 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); } @@ -304,13 +302,15 @@ public void applyMetaData() { for (BibEntry bibEntry : bibDatabase.getEntries()) { try { // synchronize only if changes were present - if (!BibDatabaseWriter.applySaveActions(bibEntry, metaData, fieldPreferences).isEmpty()) { + // TODO: apply save actions + // if (!BibDatabaseWriter.applySaveActions(bibEntry, metaData, fieldPreferences).isEmpty()) { + if (false) { dbmsProcessor.updateEntry(bibEntry); } } catch (OfflineLockException exception) { eventBus.post(new UpdateRefusedEvent(bibDatabaseContext, exception.getLocalBibEntry(), exception.getSharedBibEntry())); } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } } From 2c8faa74d8cc45d0c63c76bdb048afa0ace68bab Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 15:34:55 +0200 Subject: [PATCH 18/47] Use "ON CONFLICT" of PostgreSQL --- .../jabref/logic/shared/DBMSProcessor.java | 48 +++++++------------ 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 4a98fe1204b..c723bac7ab2 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -620,42 +620,30 @@ 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() + String insertOrUpdateQuery = new StringBuilder() .append("INSERT INTO ") .append(escape_Table("METADATA")) - .append("(") + .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); + .append(") VALUES (?, ?) ") + .append("ON CONFLICT (") + .append(escape("KEY")) + .append(") DO UPDATE SET ") + .append(escape("VALUE")) + .append(" = EXCLUDED.") + .append(escape("VALUE")) + .toString(); + + try (PreparedStatement statement = connection.prepareStatement(insertOrUpdateQuery)) { + for (Map.Entry metaEntry : data.entrySet()) { + statement.setString(1, metaEntry.getKey()); + statement.setString(2, metaEntry.getValue()); + statement.executeUpdate(); } + } catch (SQLException e) { + LOGGER.error("SQL Error", e); } } From dcf605f53a767591d46140da566472c604465632 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 16:47:21 +0200 Subject: [PATCH 19/47] Remove non-used get --- src/main/java/org/jabref/logic/shared/DBMSProcessor.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index c723bac7ab2..f3ede7427af 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -57,7 +57,6 @@ protected DBMSProcessor(DatabaseConnection dbmsConnection) { */ public boolean checkBaseIntegrity() throws SQLException { boolean databasePassesIntegrityCheck = false; - DBMSType type = this.connectionProperties.getType(); Map metadata = getSharedMetaData(); String metadataVersion = metadata.get(MetaData.VERSION_DB_STRUCT); if (metadataVersion != null) { From 55bb387bd70e578ef92e11d18738e83c8284d948 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 16:47:48 +0200 Subject: [PATCH 20/47] Fix language --- .../jabref/logic/shared/DBMSConnection.java | 2 +- .../logic/shared/PostgreSQLProcessor.java | 29 +++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSConnection.java b/src/main/java/org/jabref/logic/shared/DBMSConnection.java index 2d1438f6136..8ec68583592 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSConnection.java +++ b/src/main/java/org/jabref/logic/shared/DBMSConnection.java @@ -38,7 +38,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/PostgreSQLProcessor.java b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java index 5143dbc882a..63b10440b9e 100644 --- a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java +++ b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java @@ -67,14 +67,37 @@ public void setUp() throws SQLException { // replace semicolon so we can parse it VERSION_DB_STRUCT_DEFAULT = Integer.parseInt(metadata.get(MetaData.VERSION_DB_STRUCT).replace(";", "")); } catch (Exception e) { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Integer!"); + LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] is not an Integer."); } } else { - LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] not Exist!"); + LOGGER.warn("[VERSION_DB_STRUCT_DEFAULT] does not exist."); } + String upsertMetadata = """ + CREATE OR REPLACE FUNCTION upsert_metadata(key TEXT, value TEXT) RETURNS VOID AS $$ + DECLARE + existing_value TEXT; + BEGIN + -- Check if the key already exists and get its current value + SELECT VALUE INTO existing_value FROM METADATA WHERE KEY = key; + + -- Perform the upsert + INSERT INTO METADATA (KEY, VALUE) + VALUES (key, value) + ON CONFLICT (KEY) + DO UPDATE SET VALUE = EXCLUDED.VALUE; + + -- Notify only if the value has changed + IF existing_value IS DISTINCT FROM value THEN + PERFORM pg_notify('metadata_update', json_build_object('key', key, 'value', value)::TEXT); + END IF; + END; + $$ LANGUAGE plpgsql; + """; + connection.createStatement().executeUpdate(upsertMetadata); + if (VERSION_DB_STRUCT_DEFAULT < CURRENT_VERSION_DB_STRUCT) { - // We can to migrate from old table in new table + // We can migrate data from old tables in new table if (VERSION_DB_STRUCT_DEFAULT == 0 && CURRENT_VERSION_DB_STRUCT == 1) { LOGGER.info("Migrating from VersionDBStructure == 0"); connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("ENTRY") + " SELECT * FROM \"ENTRY\""); From fd3c184bb1e4a1141b3e741b3a3d0f3669f48c41 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 16:51:11 +0200 Subject: [PATCH 21/47] Fix module-info.java --- src/main/java/module-info.java | 3 --- .../listener/OracleNotificationListener.java | 23 ------------------- 2 files changed, 26 deletions(-) delete mode 100644 src/main/java/org/jabref/logic/shared/listener/OracleNotificationListener.java diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index cd22dd41ad4..0dea708924e 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -97,10 +97,7 @@ // endregion // region: SQL databases - requires ojdbc10; requires org.postgresql.jdbc; - requires org.mariadb.jdbc; - uses org.mariadb.jdbc.credential.CredentialPlugin; // endregion // region: Apache Commons and other (similar) helper libraries diff --git a/src/main/java/org/jabref/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(); - } -} From 5c863dce657fa22aac1ccaf8f55604525292dfd4 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 17:25:13 +0200 Subject: [PATCH 22/47] Switch from "external" Postgres to embedded Postgres --- .github/workflows/tests.yml | 12 ------- build.gradle | 3 ++ docs/code-howtos/remote-storage-sql.md | 5 +++ .../jabref/logic/shared/DBMSConnection.java | 7 ++++ .../jabref/logic/shared/DBMSProcessor.java | 2 -- .../jabref/logic/shared/DBMSSynchronizer.java | 2 +- .../jabref/logic/shared/ConnectorTest.java | 35 +++++++++++++++---- .../logic/shared/DBMSProcessorTest.java | 12 +++---- .../logic/shared/DBMSSynchronizerTest.java | 10 +++--- .../shared/SynchronizationSimulatorTest.java | 13 ++++--- .../org/jabref/logic/shared/TestManager.java | 8 ----- 11 files changed, 64 insertions(+), 45 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 007acec06b9..77d4528c10b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -198,17 +198,6 @@ 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 @@ -230,7 +219,6 @@ jobs: run: ./gradlew databaseTest --rerun-tasks env: CI: "true" - DBMS: "postgresql" guitests: name: GUI tests diff --git a/build.gradle b/build.gradle index bea8da0c1c1..07416f2cd1b 100644 --- a/build.gradle +++ b/build.gradle @@ -362,6 +362,9 @@ dependencies { // Even if "compileOnly" is used, IntelliJ always adds to module-info.java. To avoid issues during committing, we use "implementation" instead of "compileOnly" implementation 'io.github.adr:e-adr:2.0.0-SNAPSHOT' + implementation 'io.zonky.test:embedded-postgres:2.0.7' + implementation enforcedPlatform('io.zonky.test.postgres:embedded-postgres-binaries-bom:17.0.0') + testImplementation 'io.github.classgraph:classgraph:4.8.176' testImplementation 'org.junit.jupiter:junit-jupiter:5.11.0' testImplementation 'org.junit.platform:junit-platform-launcher:1.10.3' diff --git a/docs/code-howtos/remote-storage-sql.md b/docs/code-howtos/remote-storage-sql.md index cf0772ad9d9..5a28cbc5cc4 100644 --- a/docs/code-howtos/remote-storage-sql.md +++ b/docs/code-howtos/remote-storage-sql.md @@ -72,3 +72,8 @@ PostgreSQL supports to register listeners on the database on changes. The listening is implemented at [`org.jabref.logic.shared.listener.PostgresSQLNotificationListener`](https://github.com/JabRef/jabref/blob/main/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java#L16). It "just" fetches updates from the server when a change occurred there. Thus, the changes are not actively pushed from the server, but still need to be fetched by the client. + +## Tests + +Tests are executed using [Zonky Embedded Postgres](https://github.com/zonkyio/embedded-postgres). +This installs and runs a PostgreSQL server and frees the developer from the need to install a PostgreSQL server on the local machine. diff --git a/src/main/java/org/jabref/logic/shared/DBMSConnection.java b/src/main/java/org/jabref/logic/shared/DBMSConnection.java index 8ec68583592..3d4a755ddf5 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSConnection.java +++ b/src/main/java/org/jabref/logic/shared/DBMSConnection.java @@ -9,6 +9,7 @@ import org.jabref.logic.l10n.Localization; import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; +import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -19,6 +20,12 @@ public class DBMSConnection implements DatabaseConnection { private final Connection connection; private final DBMSConnectionProperties properties; + @VisibleForTesting + public DBMSConnection(Connection connection) { + this.connection = connection; + this.properties = null; + } + public DBMSConnection(DBMSConnectionProperties connectionProperties) throws SQLException, InvalidDBMSConnectionPropertiesException { if (!connectionProperties.isValid()) { throw new InvalidDBMSConnectionPropertiesException(); diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index f3ede7427af..b3647b4ae0e 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -641,8 +641,6 @@ public void setSharedMetaData(Map data) throws SQLException { statement.setString(2, metaEntry.getValue()); statement.executeUpdate(); } - } catch (SQLException e) { - LOGGER.error("SQL Error", e); } } diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 74b92519a89..10aa9576b97 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -402,7 +402,7 @@ public void closeSharedDatabase() { dbmsProcessor.stopNotificationListener(); currentConnection.close(); } catch (SQLException e) { - LOGGER.error("SQL Error:", e); + LOGGER.error("SQL Error", e); } } diff --git a/src/test/java/org/jabref/logic/shared/ConnectorTest.java b/src/test/java/org/jabref/logic/shared/ConnectorTest.java index c1542264dab..d87d407798f 100644 --- a/src/test/java/org/jabref/logic/shared/ConnectorTest.java +++ b/src/test/java/org/jabref/logic/shared/ConnectorTest.java @@ -1,22 +1,43 @@ package org.jabref.logic.shared; +import java.io.IOException; +import java.sql.Connection; +import java.sql.DriverManager; import java.sql.SQLException; -import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException; import org.jabref.testutils.category.DatabaseTest; +import io.zonky.test.db.postgres.embedded.EmbeddedPostgres; + /** * Stores the credentials for the test systems */ @DatabaseTest -public class ConnectorTest { +public class ConnectorTest implements AutoCloseable { + + private EmbeddedPostgres postgres; + private Connection connection; - public static DBMSConnection getTestDBMSConnection(DBMSType dbmsType) throws SQLException, InvalidDBMSConnectionPropertiesException { - DBMSConnectionProperties properties = getTestConnectionProperties(dbmsType); - return new DBMSConnection(properties); + /** + * Fires up a new postgres + */ + public DBMSConnection getTestDBMSConnection() throws SQLException, IOException { + postgres = EmbeddedPostgres.builder().start(); + String url = postgres.getJdbcUrl("postgres", "postgres"); + connection = DriverManager.getConnection(url, "postgres", "postgres"); + return new DBMSConnection(connection); } - public static DBMSConnectionProperties getTestConnectionProperties(DBMSType dbmsType) { - return new DBMSConnectionPropertiesBuilder().setType(dbmsType).setHost("localhost").setPort(dbmsType.getDefaultPort()).setDatabase("postgres").setUser("postgres").setPassword("postgres").setUseSSL(false).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/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 8f0b4625daa..706037d63d9 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -38,20 +38,20 @@ class DBMSProcessorTest { private DBMSConnection dbmsConnection; private DBMSProcessor dbmsProcessor; - private DBMSType dbmsType; + private ConnectorTest connectorTest; @BeforeEach void setup() throws Exception { - this.dbmsType = TestManager.getDBMSTypeTestParameter(); - this.dbmsConnection = ConnectorTest.getTestDBMSConnection(dbmsType); - this.dbmsProcessor = DBMSProcessor.getProcessorInstance(ConnectorTest.getTestDBMSConnection(dbmsType)); + this.connectorTest = new ConnectorTest(); + this.dbmsConnection = connectorTest.getTestDBMSConnection(); + this.dbmsProcessor = DBMSProcessor.getProcessorInstance(dbmsConnection); TestManager.clearTables(this.dbmsConnection); dbmsProcessor.setupSharedDatabase(); } @AfterEach - void closeDbmsConnection() throws SQLException { - this.dbmsConnection.getConnection().close(); + void closeDbmsConnection() throws Exception { + connectorTest.close(); } @Test diff --git a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java index 0463ea15b32..0a300ace8c3 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java @@ -48,7 +48,7 @@ class DBMSSynchronizerTest { private final GlobalCitationKeyPatterns pattern = GlobalCitationKeyPatterns.fromPattern("[auth][year]"); private DBMSConnection dbmsConnection; private DBMSProcessor dbmsProcessor; - private DBMSType dbmsType; + private ConnectorTest connectorTest; private BibEntry createExampleBibEntry(int index) { BibEntry bibEntry = new BibEntry(StandardEntryType.Book) @@ -60,8 +60,8 @@ private BibEntry createExampleBibEntry(int index) { @BeforeEach void setup() throws Exception { - this.dbmsType = TestManager.getDBMSTypeTestParameter(); - this.dbmsConnection = ConnectorTest.getTestDBMSConnection(dbmsType); + this.connectorTest = new ConnectorTest(); + this.dbmsConnection = connectorTest.getTestDBMSConnection(); this.dbmsProcessor = DBMSProcessor.getProcessorInstance(this.dbmsConnection); TestManager.clearTables(this.dbmsConnection); this.dbmsProcessor.setupSharedDatabase(); @@ -79,8 +79,8 @@ void setup() throws Exception { } @AfterEach - void clear() { - dbmsSynchronizer.closeSharedDatabase(); + void closeDbmsConnection() throws Exception { + connectorTest.close(); } @Test diff --git a/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java b/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java index 2558cbdb85f..33dfcdb3407 100644 --- a/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java +++ b/src/test/java/org/jabref/logic/shared/SynchronizationSimulatorTest.java @@ -1,6 +1,5 @@ package org.jabref.logic.shared; -import java.sql.SQLException; import java.util.List; import javafx.collections.FXCollections; @@ -37,6 +36,8 @@ class SynchronizationSimulatorTest { private BibDatabaseContext clientContextB; private SynchronizationEventListenerTest eventListenerB; // used to monitor occurring events private final GlobalCitationKeyPatterns pattern = GlobalCitationKeyPatterns.fromPattern("[auth][year]"); + private ConnectorTest connectorTestA; + private ConnectorTest connectorTestB; private BibEntry getBibEntryExample(int index) { return new BibEntry(StandardEntryType.InProceedings) @@ -49,7 +50,8 @@ private BibEntry getBibEntryExample(int index) { @BeforeEach void setup() throws Exception { - DBMSConnection dbmsConnection = ConnectorTest.getTestDBMSConnection(TestManager.getDBMSTypeTestParameter()); + this.connectorTestA = new ConnectorTest(); + DBMSConnection dbmsConnection = connectorTestA.getTestDBMSConnection(); TestManager.clearTables(dbmsConnection); FieldPreferences fieldPreferences = mock(FieldPreferences.class); @@ -60,19 +62,22 @@ void setup() throws Exception { clientContextA.convertToSharedDatabase(synchronizerA); clientContextA.getDBMSSynchronizer().openSharedDatabase(dbmsConnection); + this.connectorTestB = new ConnectorTest(); clientContextB = new BibDatabaseContext(); DBMSSynchronizer synchronizerB = new DBMSSynchronizer(clientContextB, ',', fieldPreferences, pattern, new DummyFileUpdateMonitor()); clientContextB.convertToSharedDatabase(synchronizerB); // use a second connection, because this is another client (typically on another machine) - clientContextB.getDBMSSynchronizer().openSharedDatabase(ConnectorTest.getTestDBMSConnection(TestManager.getDBMSTypeTestParameter())); + clientContextB.getDBMSSynchronizer().openSharedDatabase(connectorTestB.getTestDBMSConnection()); eventListenerB = new SynchronizationEventListenerTest(); clientContextB.getDBMSSynchronizer().registerListener(eventListenerB); } @AfterEach - void clear() throws SQLException { + void clear() throws Exception { clientContextA.getDBMSSynchronizer().closeSharedDatabase(); clientContextB.getDBMSSynchronizer().closeSharedDatabase(); + connectorTestA.close(); + connectorTestB.close(); } @Test diff --git a/src/test/java/org/jabref/logic/shared/TestManager.java b/src/test/java/org/jabref/logic/shared/TestManager.java index 8c110ab9b5b..b4007370c4b 100644 --- a/src/test/java/org/jabref/logic/shared/TestManager.java +++ b/src/test/java/org/jabref/logic/shared/TestManager.java @@ -8,14 +8,6 @@ * 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); dbmsConnection.getConnection().createStatement().executeUpdate("DROP TABLE IF EXISTS jabref.\"FIELD\""); From 7e0ba3eb78bf24b1eba04e5e6d5b204ef05ff2b8 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 23:18:38 +0200 Subject: [PATCH 23/47] Remove escape and escape_Table (and add indexes) - try to simplify SQL statements - create indexes --- CHANGELOG.md | 1 + .../jabref/logic/shared/DBMSProcessor.java | 198 ++++++------------ .../logic/shared/PostgreSQLProcessor.java | 95 +++++---- .../logic/shared/DBMSProcessorTest.java | 75 +++---- .../org/jabref/logic/shared/TestManager.java | 7 +- 5 files changed, 155 insertions(+), 221 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaef3c8e8ea..9158dd129d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv - We improved the performance when pasting and importing entries in an existing library. [#11843](https://github.com/JabRef/jabref/pull/11843) - When fulltext search is selected but indexing is deactivated, a dialog is now shown asking if the user wants to enable indexing now [#9491](https://github.com/JabRef/jabref/issues/9491) - We changed instances of 'Search Selected' to 'Search Pre-configured' in Web Search Preferences UI. [#11871](https://github.com/JabRef/jabref/pull/11871) +- We rewrote the [remote SQL database](https://docs.jabref.org/collaborative-work/sqldatabase) support. ⚠️database tables will be migrated. [#11879](https://github.com/JabRef/jabref/pull/11879) ### Fixed diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index b3647b4ae0e..c925a077f52 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -24,6 +24,7 @@ import org.jabref.model.entry.event.EntriesEventSource; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.field.FieldFactory; +import org.jabref.model.entry.types.EntryType; import org.jabref.model.entry.types.EntryTypeFactory; import org.jabref.model.metadata.MetaData; @@ -129,19 +130,6 @@ public void setupSharedDatabase() throws SQLException { */ protected abstract void setUp() throws SQLException; - /** - * Escapes parts of SQL expressions such as a table name or a field name to match the conventions of the database - * system using the current dbmsType. - *

- * This method is package private, because of DBMSProcessorTest - * - * @param expression Table or field name - * @return Correctly escaped expression - */ - abstract String escape(String expression); - - abstract String escape_Table(String expression); - abstract Integer getCURRENT_VERSION_DB_STRUCT(); /** @@ -173,12 +161,11 @@ public void insertEntries(List bibEntries) { * @param bibEntries List of {@link BibEntry} to be inserted */ protected void insertIntoEntryTable(List bibEntries) { - StringBuilder insertIntoEntryQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("ENTRY")) - .append("(") - .append(escape("TYPE")) - .append(") VALUES(?)"); + if (bibEntries.isEmpty()) { + return; + } + + StringBuilder insertIntoEntryQuery = new StringBuilder().append("INSERT INTO entry (entrytype) VALUES (?)"); // Number of commas is bibEntries.size() - 1 insertIntoEntryQuery.append(", (?)".repeat(Math.max(0, (bibEntries.size() - 1)))); @@ -201,7 +188,7 @@ protected void insertIntoEntryTable(List bibEntries) { } } } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); + LOGGER.error("SQL Error", e); } } @@ -222,8 +209,7 @@ private List getNotYetExistingEntries(List bibEntries) { return bibEntries; } try { - String selectQuery = "SELECT * FROM " + - escape_Table("ENTRY"); + String selectQuery = "SELECT * FROM ENTRY"; try (ResultSet resultSet = connection.createStatement().executeQuery(selectQuery)) { while (resultSet.next()) { @@ -245,22 +231,19 @@ private List getNotYetExistingEntries(List bibEntries) { * @param bibEntries {@link BibEntry} to be inserted */ protected void insertIntoFieldTable(List bibEntries) { + if (bibEntries.isEmpty()) { + return; + } + try { // Inserting into FIELD table // Coerce to ArrayList in order to use List.get() - List> fields = bibEntries.stream().map(bibEntry -> new ArrayList<>(bibEntry.getFields())) + List> fields = bibEntries.stream() + .map(bibEntry -> new ArrayList<>(bibEntry.getFields())) .collect(Collectors.toList()); StringBuilder insertFieldQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("FIELD")) - .append("(") - .append(escape("ENTRY_SHARED_ID")) - .append(", ") - .append(escape("NAME")) - .append(", ") - .append(escape("VALUE")) - .append(") VALUES(?, ?, ?)"); + .append("INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE) VALUES(?, ?, ?)"); int numFields = 0; for (List entryFields : fields) { numFields += entryFields.size(); @@ -283,6 +266,7 @@ protected void insertIntoFieldTable(List bibEntries) { fieldsCompleted += 1; } } + // TODO: This could grow too large for a single query preparedFieldStatement.executeUpdate(); } } catch (SQLException e) { @@ -317,18 +301,12 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL .getVersion()) || localBibEntry.equals(sharedBibEntry)) { insertOrUpdateFields(localBibEntry); - // updating entry type - String updateEntryTypeQuery = "UPDATE " + - escape_Table("ENTRY") + - " SET " + - escape("TYPE") + - " = ?, " + - escape("VERSION") + - " = " + - escape("VERSION") + - " + 1 WHERE " + - escape("SHARED_ID") + - " = ?"; + String updateEntryTypeQuery = """ + UPDATE entry + SET entrytype = ?, + version = version + 1 + WHERE shared_id = ? + """; try (PreparedStatement preparedUpdateEntryTypeStatement = connection.prepareStatement(updateEntryTypeQuery)) { preparedUpdateEntryTypeStatement.setString(1, localBibEntry.getType().getName()); @@ -355,13 +333,10 @@ private void removeSharedFieldsByDifference(BibEntry localBibEntry, BibEntry sha Set nullFields = new HashSet<>(sharedBibEntry.getFields()); nullFields.removeAll(localBibEntry.getFields()); for (Field nullField : nullFields) { - String deleteFieldQuery = "DELETE FROM " + - escape_Table("FIELD") + - " WHERE " + - escape("NAME") + - " = ? AND " + - escape("ENTRY_SHARED_ID") + - " = ?"; + String deleteFieldQuery = """ + DELETE FROM FIELD + WHERE NAME = ? AND ENTRY_SHARED_ID = ? + """; try (PreparedStatement preparedDeleteFieldStatement = connection .prepareStatement(deleteFieldQuery)) { @@ -385,13 +360,10 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { value = valueOptional.get(); } - String selectFieldQuery = "SELECT * FROM " + - escape_Table("FIELD") + - " WHERE " + - escape("NAME") + - " = ? AND " + - escape("ENTRY_SHARED_ID") + - " = ?"; + String selectFieldQuery = """ + SELECT * FROM FIELD + WHERE NAME = ? AND ENTRY_SHARED_ID = ? + """; try (PreparedStatement preparedSelectFieldStatement = connection .prepareStatement(selectFieldQuery)) { @@ -400,15 +372,11 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { try (ResultSet selectFieldResultSet = preparedSelectFieldStatement.executeQuery()) { if (selectFieldResultSet.next()) { // check if field already exists - String updateFieldQuery = "UPDATE " + - escape_Table("FIELD") + - " SET " + - escape("VALUE") + - " = ? WHERE " + - escape("NAME") + - " = ? AND " + - escape("ENTRY_SHARED_ID") + - " = ?"; + String updateFieldQuery = """ + UPDATE FIELD + SET VALUE = ? + WHERE NAME = ? AND ENTRY_SHARED_ID = ? + """; try (PreparedStatement preparedUpdateFieldStatement = connection .prepareStatement(updateFieldQuery)) { @@ -418,15 +386,10 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { preparedUpdateFieldStatement.executeUpdate(); } } else { - String insertFieldQuery = "INSERT INTO " + - escape_Table("FIELD") + - "(" + - escape("ENTRY_SHARED_ID") + - ", " + - escape("NAME") + - ", " + - escape("VALUE") + - ") VALUES(?, ?, ?)"; + String insertFieldQuery = """ + INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE) + VALUES (?, ?, ?) + """; try (PreparedStatement preparedFieldStatement = connection .prepareStatement(insertFieldQuery)) { @@ -451,12 +414,7 @@ public void removeEntries(List bibEntries) { if (bibEntries.isEmpty()) { return; } - StringBuilder query = new StringBuilder() - .append("DELETE FROM ") - .append(escape_Table("ENTRY")) - .append(" WHERE ") - .append(escape("SHARED_ID")) - .append(" IN ("); + StringBuilder query = new StringBuilder().append("DELETE FROM ENTRY WHERE SHARED_ID IN ("); query.append("?, ".repeat(bibEntries.size() - 1)); query.append("?)"); @@ -509,31 +467,19 @@ public List getSharedEntries(List sharedIDs) { List sharedEntries = new ArrayList<>(); - StringBuilder query = new StringBuilder(); - query.append("SELECT ") - .append(escape_Table("ENTRY")).append(".").append(escape("SHARED_ID")).append(", ") - .append(escape_Table("ENTRY")).append(".").append(escape("TYPE")).append(", ") - .append(escape_Table("ENTRY")).append(".").append(escape("VERSION")).append(", ") - .append("F.").append(escape("ENTRY_SHARED_ID")).append(", ") - .append("F.").append(escape("NAME")).append(", ") - .append("F.").append(escape("VALUE")) - .append(" FROM ") - .append(escape_Table("ENTRY")) - // Handle special case if entry does not have any fields (yet) - .append(" left outer join ") - .append(escape_Table("FIELD")) - .append(" F on ") - .append(escape_Table("ENTRY")).append(".").append(escape("SHARED_ID")) - .append(" = F.").append(escape("ENTRY_SHARED_ID")); + StringBuilder query = new StringBuilder() + .append("SELECT entry.shared_id, entry.entrytype, entry.version, ") + .append("F.entry_shared_id, F.name, F.value ") + .append("FROM entry ") + .append("LEFT OUTER JOIN field F ON entry.shared_id = F.entry_shared_id"); if (!sharedIDs.isEmpty()) { - query.append(" where ") - .append(escape("SHARED_ID")).append(" in (") + query.append(" WHERE entry.shared_id IN (") .append("?, ".repeat(sharedIDs.size() - 1)) .append("?)"); } - query.append(" order by ") - .append(escape("SHARED_ID")); + + query.append(" ORDER BY shared_id"); try (PreparedStatement preparedStatement = connection.prepareStatement(query.toString())) { for (int i = 0; i < sharedIDs.size(); i++) { @@ -547,12 +493,16 @@ public List getSharedEntries(List sharedIDs) { // We get a list of field values of bib entries "grouped" by bib entries // Thus, the first change in the shared id leads to a new BibEntry if (selectEntryResultSet.getInt("SHARED_ID") > lastId) { - bibEntry = new BibEntry(); - bibEntry.getSharedBibEntryData().setSharedID(selectEntryResultSet.getInt("SHARED_ID")); - bibEntry.setType(EntryTypeFactory.parse(selectEntryResultSet.getString("TYPE"))); - bibEntry.getSharedBibEntryData().setVersion(selectEntryResultSet.getInt("VERSION")); + int sharedId = selectEntryResultSet.getInt("shared_id"); + int version = selectEntryResultSet.getInt("version"); + EntryType entrytype = EntryTypeFactory.parse(selectEntryResultSet.getString("entrytype")); + + bibEntry = new BibEntry(entrytype); + bibEntry.getSharedBibEntryData().setSharedID(sharedId); + bibEntry.getSharedBibEntryData().setVersion(version); + sharedEntries.add(bibEntry); - lastId = selectEntryResultSet.getInt("SHARED_ID"); + lastId = sharedId; } // In all cases, we set the field value of the newly created BibEntry object @@ -563,8 +513,7 @@ public List getSharedEntries(List sharedIDs) { } } } catch (SQLException e) { - LOGGER.error("Executed >{}<", query); - LOGGER.error("SQL Error", e); + LOGGER.error("Executed >{}< and got an error", query, e); return Collections.emptyList(); } @@ -580,10 +529,10 @@ public List getSharedEntries() { */ public Map getSharedIDVersionMapping() { Map sharedIDVersionMapping = new HashMap<>(); - String selectEntryQuery = "SELECT * FROM " + - escape_Table("ENTRY") + - " ORDER BY " + - escape("SHARED_ID"); + String selectEntryQuery = """ + SELECT * FROM ENTRY + ORDER BY SHARED_ID + """; try (ResultSet selectEntryResultSet = connection.createStatement().executeQuery(selectEntryQuery)) { while (selectEntryResultSet.next()) { @@ -602,7 +551,7 @@ public Map getSharedIDVersionMapping() { public Map getSharedMetaData() { Map data = new HashMap<>(); - try (ResultSet resultSet = connection.createStatement().executeQuery("SELECT * FROM " + escape_Table("METADATA"))) { + try (ResultSet resultSet = connection.createStatement().executeQuery("SELECT * FROM METADATA")) { while (resultSet.next()) { data.put(resultSet.getString("KEY"), resultSet.getString("VALUE")); } @@ -619,21 +568,12 @@ public Map getSharedMetaData() { * @param data JabRef meta data as map */ public void setSharedMetaData(Map data) throws SQLException { - String insertOrUpdateQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("METADATA")) - .append(" (") - .append(escape("KEY")) - .append(", ") - .append(escape("VALUE")) - .append(") VALUES (?, ?) ") - .append("ON CONFLICT (") - .append(escape("KEY")) - .append(") DO UPDATE SET ") - .append(escape("VALUE")) - .append(" = EXCLUDED.") - .append(escape("VALUE")) - .toString(); + String insertOrUpdateQuery = """ + INSERT INTO METADATA (KEY, VALUE) + VALUES (?, ?) + ON CONFLICT (KEY) DO UPDATE + SET VALUE = EXCLUDED.VALUE + """; try (PreparedStatement statement = connection.prepareStatement(insertOrUpdateQuery)) { for (Map.Entry metaEntry : data.entrySet()) { diff --git a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java index 63b10440b9e..d3b880829d8 100644 --- a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java +++ b/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java @@ -6,6 +6,7 @@ import java.sql.Statement; import java.util.List; import java.util.Map; +import java.util.StringJoiner; import org.jabref.logic.shared.listener.PostgresSQLNotificationListener; import org.jabref.logic.util.HeadlessExecutorService; @@ -22,7 +23,9 @@ public class PostgreSQLProcessor extends DBMSProcessor { private PostgresSQLNotificationListener listener; private int VERSION_DB_STRUCT_DEFAULT = -1; - private final int CURRENT_VERSION_DB_STRUCT = 1; + + // TODO: We need to migrate data - or ask the user to recreate + private final int CURRENT_VERSION_DB_STRUCT = 2; public PostgreSQLProcessor(DatabaseConnection connection) { super(connection); @@ -41,24 +44,37 @@ public void setUp() throws SQLException { VERSION_DB_STRUCT_DEFAULT = 0; } - connection.createStatement().executeUpdate("CREATE SCHEMA IF NOT EXISTS jabref"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS " + escape_Table("ENTRY") + " (" + - "\"SHARED_ID\" SERIAL PRIMARY KEY, " + - "\"TYPE\" VARCHAR, " + - "\"VERSION\" INTEGER DEFAULT 1)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS " + escape_Table("FIELD") + " (" + - "\"ENTRY_SHARED_ID\" INTEGER REFERENCES " + escape_Table("ENTRY") + "(\"SHARED_ID\") ON DELETE CASCADE, " + - "\"NAME\" VARCHAR, " + - "\"VALUE\" TEXT)"); - - connection.createStatement().executeUpdate( - "CREATE TABLE IF NOT EXISTS " + escape_Table("METADATA") + " (" - + "\"KEY\" VARCHAR," - + "\"VALUE\" TEXT)"); + // TODO: Before a release, fix the names (and migrate data to the new names) + // Think of using Flyway or Liquibase instead of manual migration + // If changed, also adjust {@link org.jabref.logic.shared.TestManager.clearTables} + connection.createStatement().executeUpdate("CREATE SCHEMA IF NOT EXISTS \"jabref-alpha\""); + connection.createStatement().executeUpdate("SET search_path TO \"jabref-alpha\""); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS entry ( + shared_id SERIAL PRIMARY KEY, + entrytype VARCHAR, + version INTEGER DEFAULT 1 + ) + """); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS field ( + entry_shared_id INTEGER REFERENCES entry(shared_id) ON DELETE CASCADE, + name VARCHAR, + value TEXT + ) + """); + connection.createStatement().executeUpdate("CREATE INDEX idx_field_entry_shared_id ON FIELD (ENTRY_SHARED_ID);"); + connection.createStatement().executeUpdate("CREATE INDEX idx_field_name ON FIELD (NAME);"); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS metadata ( + key VARCHAR, + value TEXT + ) + """); + connection.createStatement().executeUpdate("CREATE UNIQUE INDEX idx_metadata_key ON METADATA (key);"); Map metadata = getSharedMetaData(); @@ -100,29 +116,34 @@ ON CONFLICT (KEY) // We can migrate data from old tables in new table if (VERSION_DB_STRUCT_DEFAULT == 0 && CURRENT_VERSION_DB_STRUCT == 1) { LOGGER.info("Migrating from VersionDBStructure == 0"); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("ENTRY") + " SELECT * FROM \"ENTRY\""); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("FIELD") + " SELECT * FROM \"FIELD\""); - connection.createStatement().executeUpdate("INSERT INTO " + escape_Table("METADATA") + " SELECT * FROM \"METADATA\""); - connection.createStatement().execute("SELECT setval(\'jabref.\"ENTRY_SHARED_ID_seq\"\', (select max(\"SHARED_ID\") from jabref.\"ENTRY\"))"); + connection.createStatement().executeUpdate("INSERT INTO ENTRY SELECT * FROM \"ENTRY\""); + connection.createStatement().executeUpdate("INSERT INTO FIELD SELECT * FROM \"FIELD\""); + connection.createStatement().executeUpdate("INSERT INTO METADATA SELECT * FROM \"METADATA\""); + connection.createStatement().execute("SELECT setval(\'\"ENTRY_SHARED_ID_seq\"\', (select max(\"SHARED_ID\") from \"ENTRY\"))"); metadata = getSharedMetaData(); } metadata.put(MetaData.VERSION_DB_STRUCT, String.valueOf(CURRENT_VERSION_DB_STRUCT)); setSharedMetaData(metadata); } + + // TODO: implement migration of changes from version 1 to 2 + // - "TYPE" is now called entrytype (to be consistent with org.jabref.model.entry.field.InternalField.TYPE_HEADER) + // - table names and field names now lower case } @Override protected void insertIntoEntryTable(List bibEntries) { - StringBuilder insertIntoEntryQuery = new StringBuilder() - .append("INSERT INTO ") - .append(escape_Table("ENTRY")) - .append("(") - .append(escape("TYPE")) - .append(") VALUES(?)"); - // Number of commas is bibEntries.size() - 1 - insertIntoEntryQuery.append(", (?)".repeat(Math.max(0, bibEntries.size() - 1))); - try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(), + if (bibEntries.isEmpty()) { + return; + } + + StringJoiner insertIntoEntryQuery = new StringJoiner(", ", "INSERT INTO entry (entrytype) values ", ";"); + for (int i = 0; i < bibEntries.size(); i++) { + insertIntoEntryQuery.add("(?)"); + } + try (PreparedStatement preparedEntryStatement = connection.prepareStatement( + insertIntoEntryQuery.toString(), Statement.RETURN_GENERATED_KEYS)) { for (int i = 0; i < bibEntries.size(); i++) { preparedEntryStatement.setString(i + 1, bibEntries.get(i).getType().getName()); @@ -145,16 +166,6 @@ protected void insertIntoEntryTable(List bibEntries) { } } - @Override - String escape(String expression) { - return "\"" + expression + "\""; - } - - @Override - String escape_Table(String expression) { - return "jabref." + escape(expression); - } - @Override Integer getCURRENT_VERSION_DB_STRUCT() { return CURRENT_VERSION_DB_STRUCT; diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 706037d63d9..1b6a04ee9f9 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -77,14 +77,14 @@ void insertEntry() throws SQLException { Map actualFieldMap = new HashMap<>(); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(1, entryResultSet.getInt("SHARED_ID")); - assertEquals("inproceedings", entryResultSet.getString("TYPE")); + assertEquals("inproceedings", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertFalse(entryResultSet.next()); - try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) { + try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection)) { while (fieldResultSet.next()) { actualFieldMap.put(fieldResultSet.getString("NAME"), fieldResultSet.getString("VALUE")); } @@ -102,15 +102,15 @@ void insertEntryWithEmptyFields() throws SQLException { dbmsProcessor.insertEntry(expectedEntry); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(1, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertFalse(entryResultSet.next()); // Adding an empty entry should not create an entry in field table, only in entry table - try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) { + try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection)) { assertFalse(fieldResultSet.next()); } } @@ -206,7 +206,7 @@ void removeAllEntries() throws SQLException { dbmsProcessor.insertEntry(secondEntry); dbmsProcessor.removeEntries(entriesToRemove); - try (ResultSet resultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet resultSet = selectFrom("ENTRY", dbmsConnection)) { assertFalse(resultSet.next()); } } @@ -225,7 +225,7 @@ void removeSomeEntries() throws SQLException { dbmsProcessor.insertEntry(thirdEntry); dbmsProcessor.removeEntries(entriesToRemove); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(2, entryResultSet.getInt("SHARED_ID")); assertFalse(entryResultSet.next()); @@ -238,7 +238,7 @@ void removeSingleEntry() throws SQLException { dbmsProcessor.insertEntry(entryToRemove); dbmsProcessor.removeEntries(Collections.singletonList(entryToRemove)); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertFalse(entryResultSet.next()); } } @@ -252,7 +252,7 @@ void removeEntriesOnNullThrows() { void removeEmptyEntryList() throws SQLException { dbmsProcessor.removeEntries(Collections.emptyList()); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertFalse(entryResultSet.next()); } } @@ -288,9 +288,10 @@ void getNotExistingSharedEntry() { @Test void getSharedIDVersionMapping() throws OfflineLockException, SQLException { BibEntry firstEntry = getBibEntryExample(); - BibEntry secondEntry = getBibEntryExample(); - dbmsProcessor.insertEntry(firstEntry); + + // insert duplicate entry + BibEntry secondEntry = getBibEntryExample(); dbmsProcessor.insertEntry(secondEntry); dbmsProcessor.updateEntry(secondEntry); @@ -305,11 +306,10 @@ void getSharedIDVersionMapping() throws OfflineLockException, SQLException { @Test void getSharedMetaData() { - insertMetaData("databaseType", "bibtex;", dbmsConnection, dbmsProcessor); - insertMetaData("protectedFlag", "true;", dbmsConnection, dbmsProcessor); - insertMetaData("saveActions", "enabled;\nauthor[capitalize,html_to_latex]\ntitle[title_case]\n;", dbmsConnection, dbmsProcessor); - insertMetaData("saveOrderConfig", "specified;author;false;title;false;year;true;", dbmsConnection, dbmsProcessor); - insertMetaData("VersionDBStructure", "1", dbmsConnection, dbmsProcessor); + insertMetaData("databaseType", "bibtex;", dbmsConnection); + insertMetaData("protectedFlag", "true;", dbmsConnection); + insertMetaData("saveActions", "enabled;\nauthor[capitalize,html_to_latex]\ntitle[title_case]\n;", dbmsConnection); + insertMetaData("saveOrderConfig", "specified;author;false;title;false;year;true;", dbmsConnection); Map expectedMetaData = getMetaDataExample(); Map actualMetaData = dbmsProcessor.getSharedMetaData(); @@ -334,7 +334,7 @@ private static Map getMetaDataExample() { expectedMetaData.put("protectedFlag", "true;"); expectedMetaData.put("saveActions", "enabled;\nauthor[capitalize,html_to_latex]\ntitle[title_case]\n;"); expectedMetaData.put("saveOrderConfig", "specified;author;false;title;false;year;true;"); - expectedMetaData.put("VersionDBStructure", "1"); + expectedMetaData.put("VersionDBStructure", "2"); return expectedMetaData; } @@ -378,30 +378,30 @@ void insertMultipleEntries() throws SQLException { Map> actualFieldMap = new HashMap<>(); - try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection, dbmsProcessor)) { + try (ResultSet entryResultSet = selectFrom("ENTRY", dbmsConnection)) { assertTrue(entryResultSet.next()); assertEquals(1, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(2, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(3, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(4, entryResultSet.getInt("SHARED_ID")); - assertEquals("thesis", entryResultSet.getString("TYPE")); + assertEquals("thesis", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertTrue(entryResultSet.next()); assertEquals(5, entryResultSet.getInt("SHARED_ID")); - assertEquals("article", entryResultSet.getString("TYPE")); + assertEquals("article", entryResultSet.getString("entrytype")); assertEquals(1, entryResultSet.getInt("VERSION")); assertFalse(entryResultSet.next()); - try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection, dbmsProcessor)) { + try (ResultSet fieldResultSet = selectFrom("FIELD", dbmsConnection)) { while (fieldResultSet.next()) { if (actualFieldMap.containsKey(fieldResultSet.getInt("ENTRY_SHARED_ID"))) { actualFieldMap.get(fieldResultSet.getInt("ENTRY_SHARED_ID")).put( @@ -424,9 +424,9 @@ void insertMultipleEntries() throws SQLException { assertEquals(expectedFieldMap, actualFieldMap); } - private ResultSet selectFrom(String table, DBMSConnection dbmsConnection, DBMSProcessor dbmsProcessor) { + private ResultSet selectFrom(String table, DBMSConnection dbmsConnection) { try { - return dbmsConnection.getConnection().createStatement().executeQuery("SELECT * FROM " + escape_Table(table, dbmsProcessor)); + return dbmsConnection.getConnection().createStatement().executeQuery("SELECT * FROM " + table); } catch (SQLException e) { fail(e.getMessage()); return null; @@ -434,24 +434,11 @@ private ResultSet selectFrom(String table, DBMSConnection dbmsConnection, DBMSPr } // Oracle does not support multiple tuple insertion in one INSERT INTO command. - // Therefore this function was defined to improve the readability and to keep the code short. - private void insertMetaData(String key, String value, DBMSConnection dbmsConnection, DBMSProcessor dbmsProcessor) { + // Therefore, this function was defined to improve the readability and to keep the code short. + private void insertMetaData(String key, String value, DBMSConnection dbmsConnection) { assertDoesNotThrow(() -> { - dbmsConnection.getConnection().createStatement().executeUpdate("INSERT INTO " + escape_Table("METADATA", dbmsProcessor) + "(" - + escape("KEY", dbmsProcessor) + ", " + escape("VALUE", dbmsProcessor) + ") VALUES(" - + escapeValue(key) + ", " + escapeValue(value) + ")"); + // TODO: maybe use a prepared statement here + dbmsConnection.getConnection().createStatement().executeUpdate("INSERT INTO metadata (\"key\", value) VALUES ('" + key + "', '" + value + "');"); }); } - - private static String escape(String expression, DBMSProcessor dbmsProcessor) { - return dbmsProcessor.escape(expression); - } - - private static String escape_Table(String expression, DBMSProcessor dbmsProcessor) { - return dbmsProcessor.escape_Table(expression); - } - - private static String escapeValue(String value) { - return "'" + value + "'"; - } } diff --git a/src/test/java/org/jabref/logic/shared/TestManager.java b/src/test/java/org/jabref/logic/shared/TestManager.java index b4007370c4b..17cf4ef3745 100644 --- a/src/test/java/org/jabref/logic/shared/TestManager.java +++ b/src/test/java/org/jabref/logic/shared/TestManager.java @@ -1,7 +1,6 @@ package org.jabref.logic.shared; import java.sql.SQLException; -import java.util.Objects; /** * This class provides helping methods for database tests. Furthermore, it determines database systems which are ready to @@ -9,10 +8,6 @@ */ public class TestManager { public static void clearTables(DBMSConnection dbmsConnection) throws SQLException { - Objects.requireNonNull(dbmsConnection); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP TABLE IF EXISTS jabref.\"FIELD\""); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP TABLE IF EXISTS jabref.\"ENTRY\""); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP TABLE IF EXISTS jabref.\"METADATA\""); - dbmsConnection.getConnection().createStatement().executeUpdate("DROP SCHEMA IF EXISTS jabref"); + dbmsConnection.getConnection().createStatement().executeUpdate("DROP SCHEMA IF EXISTS \"jabref-alpha\" CASCADE"); } } From 407b307ab8afa856f6811b7d640aab56df198dbd Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 4 Oct 2024 23:56:31 +0200 Subject: [PATCH 24/47] Fix method name "isFiltered" and also link reasoning --- .../autosaveandbackup/AutosaveManager.java | 2 +- .../gui/autosaveandbackup/BackupManager.java | 2 +- .../jabref/logic/shared/DBMSSynchronizer.java | 2 +- .../jabref/logic/util/CoarseChangeFilter.java | 3 ++- .../event/BibDatabaseContextChangedEvent.java | 19 +++++-------------- 5 files changed, 10 insertions(+), 18 deletions(-) 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 acae02c01c8..0a787061636 100644 --- a/src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java +++ b/src/main/java/org/jabref/gui/autosaveandbackup/BackupManager.java @@ -329,7 +329,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/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 10aa9576b97..5ce4c24e9df 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -97,7 +97,7 @@ public void listen(EntriesAddedEvent event) { */ @Subscribe public void listen(FieldChangedEvent event) { - if (event.isFilteredOut() || !isEventSourceAccepted(event) || !checkCurrentConnection()) { + if (event.isFiltered() || !isEventSourceAccepted(event) || !checkCurrentConnection()) { return; } diff --git a/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java b/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java index 9160fba7938..818aeb6491e 100644 --- a/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java +++ b/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java @@ -44,8 +44,9 @@ public synchronized void listen(BibDatabaseContextChangedEvent event) { // Only deltas of 1 when typing in manually, major change means pasting something (more than one character) boolean isMajorChange = fieldChange.getMajorCharacterChange() > 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/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; } } From 5a36849811485be1a46429ad5b69c48a59e513a9 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 00:17:04 +0200 Subject: [PATCH 25/47] Integrate PostgreSQLProcessor in DBMSProcessor --- .../jabref/logic/shared/DBMSProcessor.java | 162 ++++++++++++-- .../logic/shared/PostgreSQLProcessor.java | 208 ------------------ 2 files changed, 141 insertions(+), 229 deletions(-) delete mode 100644 src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index c925a077f52..6ac67cd19ab 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,10 +16,13 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.StringJoiner; import java.util.UUID; import java.util.stream.Collectors; import org.jabref.logic.shared.exception.OfflineLockException; +import org.jabref.logic.shared.listener.PostgresSQLNotificationListener; +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; @@ -29,22 +33,31 @@ import org.jabref.model.metadata.MetaData; import com.google.common.collect.Lists; +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 final class DBMSProcessor { public static final String PROCESSOR_ID = UUID.randomUUID().toString(); + protected static final Logger LOGGER = LoggerFactory.getLogger(DBMSProcessor.class); protected final Connection connection; protected DatabaseConnectionProperties connectionProperties; + private PostgresSQLNotificationListener listener; + + private int VERSION_DB_STRUCT_DEFAULT = -1; + + // TODO: We need to migrate data - or ask the user to recreate + private final int CURRENT_VERSION_DB_STRUCT = 2; + protected DBMSProcessor(DatabaseConnection dbmsConnection) { this.connection = dbmsConnection.getConnection(); this.connectionProperties = dbmsConnection.getProperties(); @@ -128,9 +141,104 @@ 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; + } + + // TODO: Before a release, fix the names (and migrate data to the new names) + // Think of using Flyway or Liquibase instead of manual migration + // If changed, also adjust {@link org.jabref.logic.shared.TestManager.clearTables} + connection.createStatement().executeUpdate("CREATE SCHEMA IF NOT EXISTS \"jabref-alpha\""); + connection.createStatement().executeUpdate("SET search_path TO \"jabref-alpha\""); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS entry ( + shared_id SERIAL PRIMARY KEY, + entrytype VARCHAR, + version INTEGER DEFAULT 1 + ) + """); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS field ( + entry_shared_id INTEGER REFERENCES entry(shared_id) ON DELETE CASCADE, + name VARCHAR, + value TEXT + ) + """); + connection.createStatement().executeUpdate("CREATE INDEX idx_field_entry_shared_id ON FIELD (ENTRY_SHARED_ID);"); + connection.createStatement().executeUpdate("CREATE INDEX idx_field_name ON FIELD (NAME);"); + + connection.createStatement().executeUpdate(""" + CREATE TABLE IF NOT EXISTS metadata ( + key VARCHAR, + value TEXT + ) + """); + connection.createStatement().executeUpdate("CREATE UNIQUE INDEX idx_metadata_key ON METADATA (key);"); + + Map metadata = getSharedMetaData(); + + 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."); + } - abstract Integer getCURRENT_VERSION_DB_STRUCT(); + 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 + } + + Integer getCURRENT_VERSION_DB_STRUCT() { + return CURRENT_VERSION_DB_STRUCT; + } /** * For use in test only. Inserts the BibEntry into the shared database. @@ -165,12 +273,14 @@ protected void insertIntoEntryTable(List bibEntries) { return; } - StringBuilder insertIntoEntryQuery = new StringBuilder().append("INSERT INTO entry (entrytype) VALUES (?)"); - // Number of commas is bibEntries.size() - 1 - insertIntoEntryQuery.append(", (?)".repeat(Math.max(0, (bibEntries.size() - 1)))); + 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(), - new String[]{"SHARED_ID"})) { + 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()); } @@ -184,11 +294,11 @@ protected void insertIntoEntryTable(List bibEntries) { bibEntry.getSharedBibEntryData().setSharedID(generatedKeys.getInt(1)); } if (generatedKeys.next()) { - LOGGER.error("Error: Some shared IDs left unassigned"); + LOGGER.error("Some shared IDs left unassigned"); } } } catch (SQLException e) { - LOGGER.error("SQL Error", e); + LOGGER.error("SQL Error during entry insertion", e); } } @@ -600,22 +710,32 @@ 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 \"jabref-FieldChangeEvent\""); + // 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); + } } /** * 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); + } } } + +// connection.createStatement().execute("NOTIFY jabrefLiveUpdate, '" + PROCESSOR_ID + "';"); 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 d3b880829d8..00000000000 --- a/src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java +++ /dev/null @@ -1,208 +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 java.util.StringJoiner; - -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; - - // TODO: We need to migrate data - or ask the user to recreate - private final int CURRENT_VERSION_DB_STRUCT = 2; - - public PostgreSQLProcessor(DatabaseConnection connection) { - super(connection); - } - - /** - * 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; - } - - // TODO: Before a release, fix the names (and migrate data to the new names) - // Think of using Flyway or Liquibase instead of manual migration - // If changed, also adjust {@link org.jabref.logic.shared.TestManager.clearTables} - connection.createStatement().executeUpdate("CREATE SCHEMA IF NOT EXISTS \"jabref-alpha\""); - connection.createStatement().executeUpdate("SET search_path TO \"jabref-alpha\""); - - connection.createStatement().executeUpdate(""" - CREATE TABLE IF NOT EXISTS entry ( - shared_id SERIAL PRIMARY KEY, - entrytype VARCHAR, - version INTEGER DEFAULT 1 - ) - """); - - connection.createStatement().executeUpdate(""" - CREATE TABLE IF NOT EXISTS field ( - entry_shared_id INTEGER REFERENCES entry(shared_id) ON DELETE CASCADE, - name VARCHAR, - value TEXT - ) - """); - connection.createStatement().executeUpdate("CREATE INDEX idx_field_entry_shared_id ON FIELD (ENTRY_SHARED_ID);"); - connection.createStatement().executeUpdate("CREATE INDEX idx_field_name ON FIELD (NAME);"); - - connection.createStatement().executeUpdate(""" - CREATE TABLE IF NOT EXISTS metadata ( - key VARCHAR, - value TEXT - ) - """); - connection.createStatement().executeUpdate("CREATE UNIQUE INDEX idx_metadata_key ON METADATA (key);"); - - Map metadata = getSharedMetaData(); - - 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 - } - - @Override - protected void insertIntoEntryTable(List bibEntries) { - 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()); - } - 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 - 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); - } - } -} From c1dda85daac0fa7fdadc44725259e3776d34fb99 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 00:17:34 +0200 Subject: [PATCH 26/47] Let pgConnection do the waiting work (not the Java thread) --- .../shared/listener/PostgresSQLNotificationListener.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java b/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java index 59ef9258874..dcfc4144720 100644 --- a/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java +++ b/src/main/java/org/jabref/logic/shared/listener/PostgresSQLNotificationListener.java @@ -31,21 +31,20 @@ public void run() { stop = false; try { while (!stop && !Thread.currentThread().isInterrupted()) { - PGNotification[] notifications = pgConnection.getNotifications(); + // 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 (!notification.getName().equals(DBMSProcessor.PROCESSOR_ID)) { + notification.getParameter(); // Only process notifications that are not sent by this processor 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); } } From 323118f6af6ff29a746b7b2b608d8119d6d33557 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 00:20:04 +0200 Subject: [PATCH 27/47] Compilefix --- src/main/java/org/jabref/logic/shared/DBMSProcessor.java | 7 ------- .../java/org/jabref/logic/shared/DBMSSynchronizer.java | 2 +- .../java/org/jabref/logic/shared/DBMSProcessorTest.java | 2 +- .../java/org/jabref/logic/shared/DBMSSynchronizerTest.java | 2 +- 4 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 6ac67cd19ab..48817134a03 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -694,13 +694,6 @@ ON CONFLICT (KEY) DO UPDATE } } - /** - * Returns a new instance of the abstract type {@link DBMSProcessor} - */ - public static DBMSProcessor getProcessorInstance(DatabaseConnection connection) { - return new PostgreSQLProcessor(connection); - } - public DatabaseConnectionProperties getDBMSConnectionProperties() { return this.connectionProperties; } diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 5ce4c24e9df..db802d01b72 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -390,7 +390,7 @@ 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); + this.dbmsProcessor = new DBMSProcessor(connection); initializeDatabases(); } diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 1b6a04ee9f9..370f2306287 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -44,7 +44,7 @@ class DBMSProcessorTest { void setup() throws Exception { this.connectorTest = new ConnectorTest(); this.dbmsConnection = connectorTest.getTestDBMSConnection(); - this.dbmsProcessor = DBMSProcessor.getProcessorInstance(dbmsConnection); + this.dbmsProcessor = new DBMSProcessor(dbmsConnection); TestManager.clearTables(this.dbmsConnection); dbmsProcessor.setupSharedDatabase(); } diff --git a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java index 0a300ace8c3..1af261344e1 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java @@ -62,7 +62,7 @@ private BibEntry createExampleBibEntry(int index) { void setup() throws Exception { this.connectorTest = new ConnectorTest(); this.dbmsConnection = connectorTest.getTestDBMSConnection(); - this.dbmsProcessor = DBMSProcessor.getProcessorInstance(this.dbmsConnection); + this.dbmsProcessor = new DBMSProcessor(this.dbmsConnection); TestManager.clearTables(this.dbmsConnection); this.dbmsProcessor.setupSharedDatabase(); From 9f099ed80bc91c5cde73b44a422eaeb484cd61e9 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 00:31:45 +0200 Subject: [PATCH 28/47] Change signature --- .../java/org/jabref/model/entry/BibEntry.java | 4 ++-- .../model/entry/event/EntryChangedEvent.java | 10 +++++----- .../entry/event/FieldAddedOrRemovedEvent.java | 2 +- .../model/entry/event/FieldChangedEvent.java | 15 ++++++--------- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index 8cab3df558a..5b1793b4fa1 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -448,7 +448,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); } @@ -640,7 +640,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); } 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..e28a8d8dbf9 100644 --- a/src/main/java/org/jabref/model/entry/event/EntryChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/EntryChangedEvent.java @@ -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)); } /** - * @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(Collections.singletonList(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 51d0d6606e2..ca5b444862d 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -15,14 +15,14 @@ public class FieldChangedEvent extends EntryChangedEvent { private int majorCharacterChange = 0; /** - * @param location Location affected by this event + * @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 */ - public FieldChangedEvent(EntriesEventSource location, BibEntry bibEntry, Field field, String oldValue, String newValue) { - super(bibEntry, location); + public FieldChangedEvent(EntriesEventSource source, BibEntry bibEntry, Field field, String oldValue, String newValue) { + super(bibEntry, source); this.field = field; this.oldValue = oldValue; this.newValue = newValue; @@ -42,11 +42,8 @@ public FieldChangedEvent(BibEntry bibEntry, Field field, String oldValue, String this.majorCharacterChange = 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(); @@ -54,7 +51,7 @@ public FieldChangedEvent(FieldChange fieldChange, EntriesEventSource location) { } public FieldChangedEvent(FieldChange fieldChange) { - this(fieldChange, EntriesEventSource.LOCAL); + this(EntriesEventSource.LOCAL, fieldChange); } private static int computeMajorCharacterChange(String oldValue, String newValue) { From 98242cf8e016d76daa670125f79fbd1d913f13fd Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 00:35:49 +0200 Subject: [PATCH 29/47] Improve variable name --- .../jabref/logic/util/CoarseChangeFilter.java | 2 +- .../model/entry/event/EntryChangedEvent.java | 6 +++--- .../model/entry/event/FieldChangedEvent.java | 20 +++++++++---------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java b/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java index 818aeb6491e..a0a781c058d 100644 --- a/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java +++ b/src/main/java/org/jabref/logic/util/CoarseChangeFilter.java @@ -42,7 +42,7 @@ 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.setFiltered(!(isEditChanged || isMajorChange)); // Post each FieldChangedEvent - even the ones being marked as "filtered" 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 e28a8d8dbf9..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; @@ -14,7 +14,7 @@ public class EntryChangedEvent extends EntriesEvent { * @param bibEntry where the changes were applied on. */ public EntryChangedEvent(BibEntry bibEntry) { - super(Collections.singletonList(bibEntry)); + super(List.of(bibEntry)); } /** @@ -22,7 +22,7 @@ public EntryChangedEvent(BibEntry bibEntry) { * @param source Source of this event */ public EntryChangedEvent(BibEntry bibEntry, EntriesEventSource source) { - super(Collections.singletonList(bibEntry), source); + super(List.of(bibEntry), source); } public BibEntry getBibEntry() { 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 ca5b444862d..43eff7aec08 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -12,7 +12,7 @@ public class FieldChangedEvent extends EntryChangedEvent { private final Field field; private final String newValue; private final String oldValue; - private int majorCharacterChange = 0; + private int charactersChangedCount = 0; /** * @param source source of this event @@ -26,7 +26,7 @@ public FieldChangedEvent(EntriesEventSource source, BibEntry bibEntry, Field fie this.field = field; this.oldValue = oldValue; this.newValue = newValue; - this.majorCharacterChange = computeMajorCharacterChange(oldValue, newValue); + this.charactersChangedCount = computeMajorCharacterChange(oldValue, newValue); } /** @@ -39,7 +39,7 @@ public FieldChangedEvent(BibEntry bibEntry, Field field, String oldValue, String this.field = field; this.newValue = newValue; this.oldValue = oldValue; - this.majorCharacterChange = computeMajorCharacterChange(oldValue, newValue); + this.charactersChangedCount = computeMajorCharacterChange(oldValue, newValue); } public FieldChangedEvent(EntriesEventSource source, FieldChange fieldChange) { @@ -47,7 +47,7 @@ public FieldChangedEvent(EntriesEventSource source, FieldChange fieldChange) { 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) { @@ -73,15 +73,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; } } From dd6fb1b663447b4ff3a5b2d1ead7446a3bf4b81e Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 00:43:25 +0200 Subject: [PATCH 30/47] Fix package name (and class name) for notifications --- src/main/java/org/jabref/logic/shared/DBMSProcessor.java | 7 +++---- .../NotificationListener.java} | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) rename src/main/java/org/jabref/logic/shared/{listener/PostgresSQLNotificationListener.java => notifications/NotificationListener.java} (86%) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 48817134a03..471ba5d91a3 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -21,7 +21,7 @@ import java.util.stream.Collectors; import org.jabref.logic.shared.exception.OfflineLockException; -import org.jabref.logic.shared.listener.PostgresSQLNotificationListener; +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; @@ -51,7 +51,7 @@ public final class DBMSProcessor { protected DatabaseConnectionProperties connectionProperties; - private PostgresSQLNotificationListener listener; + private NotificationListener listener; private int VERSION_DB_STRUCT_DEFAULT = -1; @@ -711,7 +711,7 @@ public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) { // 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); + listener = new NotificationListener(dbmsSynchronizer, pgConnection); HeadlessExecutorService.INSTANCE.execute(listener); } catch (SQLException e) { LOGGER.error("SQL Error during starting the notification listener", e); @@ -731,4 +731,3 @@ public void stopNotificationListener() { } } -// connection.createStatement().execute("NOTIFY jabrefLiveUpdate, '" + PROCESSOR_ID + "';"); 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 86% 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 dcfc4144720..7c435ae500d 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; } From 451c971868ff4f113580bec99012330ff2cb2e1a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 00:45:52 +0200 Subject: [PATCH 31/47] Reorder methods --- .../java/org/jabref/logic/shared/DatabaseSynchronizer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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); From 2577ac64a58f8590fe6e8a6e1850765901f79e61 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 01:19:54 +0200 Subject: [PATCH 32/47] Switch from UUID to CUID2 --- src/main/java/org/jabref/logic/shared/DBMSProcessor.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 471ba5d91a3..70d3d006e61 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -17,7 +17,6 @@ import java.util.Optional; import java.util.Set; import java.util.StringJoiner; -import java.util.UUID; import java.util.stream.Collectors; import org.jabref.logic.shared.exception.OfflineLockException; @@ -33,6 +32,7 @@ import org.jabref.model.metadata.MetaData; import com.google.common.collect.Lists; +import io.github.thibaultmeyer.cuid.CUID; import org.postgresql.PGConnection; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,8 +42,7 @@ */ public final 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); From b17aa3528f60c6ee8ba4765f7e385d3154a31794 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 01:20:02 +0200 Subject: [PATCH 33/47] Reorder fields --- .../java/org/jabref/model/entry/event/FieldChangedEvent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 43eff7aec08..4222b303db7 100644 --- a/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java +++ b/src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java @@ -10,8 +10,8 @@ public class FieldChangedEvent extends EntryChangedEvent { private final Field field; - private final String newValue; private final String oldValue; + private final String newValue; private int charactersChangedCount = 0; /** From 76da4426d90d81ec218251ef8d2f77c714373103 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 01:20:13 +0200 Subject: [PATCH 34/47] Remove unused method --- src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index db802d01b72..74adc714b38 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -415,10 +415,6 @@ public String getDBName() { return dbName; } - public DBMSProcessor getDBProcessor() { - return dbmsProcessor; - } - @Override public DatabaseConnectionProperties getConnectionProperties() { return dbmsProcessor.getDBMSConnectionProperties(); From 72ed0bb6ce4cfb8c652522fecbc9891d78ebd633 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 01:40:11 +0200 Subject: [PATCH 35/47] Write "Id" (instead of ID) --- .../gui/shared/SharedDatabaseUIManager.java | 2 +- .../jabref/http/server/LibraryResource.java | 2 +- .../jabref/logic/shared/DBMSProcessor.java | 36 +++++++++---------- .../jabref/logic/shared/DBMSSynchronizer.java | 4 +-- .../java/org/jabref/model/entry/BibEntry.java | 2 +- .../jabref/model/entry/CanonicalBibEntry.java | 2 +- .../model/entry/SharedBibEntryData.java | 18 +++++----- .../logic/shared/DBMSProcessorTest.java | 18 +++++----- .../logic/shared/DBMSSynchronizerTest.java | 4 +-- 9 files changed, 43 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java index d3f2989b36e..5fc513a5aa3 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().getSharedId()); 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/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 70d3d006e61..e1d4d6dba8f 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -290,7 +290,7 @@ 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("Some shared IDs left unassigned"); @@ -311,7 +311,7 @@ private List getNotYetExistingEntries(List bibEntries) { List remoteIds = new ArrayList<>(); List localIds = bibEntries.stream() .map(BibEntry::getSharedBibEntryData) - .map(SharedBibEntryData::getSharedID) + .map(SharedBibEntryData::getSharedId) .filter(id -> id != -1) .toList(); if (localIds.isEmpty()) { @@ -330,7 +330,7 @@ private List getNotYetExistingEntries(List bibEntries) { LOGGER.error("SQL Error: ", e); } return bibEntries.stream().filter(entry -> - !remoteIds.contains(entry.getSharedBibEntryData().getSharedID())) + !remoteIds.contains(entry.getSharedBibEntryData().getSharedId())) .collect(Collectors.toList()); } @@ -369,7 +369,7 @@ protected void insertIntoFieldTable(List bibEntries) { 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().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; @@ -394,7 +394,7 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL connection.setAutoCommit(false); // disable auto commit due to transaction try { - Optional sharedEntryOptional = getSharedEntry(localBibEntry.getSharedBibEntryData().getSharedID()); + Optional sharedEntryOptional = getSharedEntry(localBibEntry.getSharedBibEntryData().getSharedId()); if (sharedEntryOptional.isEmpty()) { return; @@ -419,7 +419,7 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL try (PreparedStatement preparedUpdateEntryTypeStatement = connection.prepareStatement(updateEntryTypeQuery)) { preparedUpdateEntryTypeStatement.setString(1, localBibEntry.getType().getName()); - preparedUpdateEntryTypeStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedUpdateEntryTypeStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedId()); preparedUpdateEntryTypeStatement.executeUpdate(); } @@ -450,7 +450,7 @@ private void removeSharedFieldsByDifference(BibEntry localBibEntry, BibEntry sha try (PreparedStatement preparedDeleteFieldStatement = connection .prepareStatement(deleteFieldQuery)) { preparedDeleteFieldStatement.setString(1, nullField.getName()); - preparedDeleteFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedDeleteFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedId()); preparedDeleteFieldStatement.executeUpdate(); } } @@ -477,7 +477,7 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { try (PreparedStatement preparedSelectFieldStatement = connection .prepareStatement(selectFieldQuery)) { preparedSelectFieldStatement.setString(1, field.getName()); - preparedSelectFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedSelectFieldStatement.setInt(2, localBibEntry.getSharedBibEntryData().getSharedId()); try (ResultSet selectFieldResultSet = preparedSelectFieldStatement.executeQuery()) { if (selectFieldResultSet.next()) { // check if field already exists @@ -491,7 +491,7 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { .prepareStatement(updateFieldQuery)) { preparedUpdateFieldStatement.setString(1, value); preparedUpdateFieldStatement.setString(2, field.getName()); - preparedUpdateFieldStatement.setInt(3, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedUpdateFieldStatement.setInt(3, localBibEntry.getSharedBibEntryData().getSharedId()); preparedUpdateFieldStatement.executeUpdate(); } } else { @@ -502,7 +502,7 @@ INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE) try (PreparedStatement preparedFieldStatement = connection .prepareStatement(insertFieldQuery)) { - preparedFieldStatement.setInt(1, localBibEntry.getSharedBibEntryData().getSharedID()); + preparedFieldStatement.setInt(1, localBibEntry.getSharedBibEntryData().getSharedId()); preparedFieldStatement.setString(2, field.getName()); preparedFieldStatement.setString(3, value); preparedFieldStatement.executeUpdate(); @@ -529,7 +529,7 @@ public void removeEntries(List bibEntries) { try (PreparedStatement preparedStatement = connection.prepareStatement(query.toString())) { 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().getSharedId()); } preparedStatement.executeUpdate(); } catch (SQLException e) { @@ -607,7 +607,7 @@ public List getSharedEntries(List sharedIDs) { EntryType entrytype = EntryTypeFactory.parse(selectEntryResultSet.getString("entrytype")); bibEntry = new BibEntry(entrytype); - bibEntry.getSharedBibEntryData().setSharedID(sharedId); + bibEntry.getSharedBibEntryData().setSharedId(sharedId); bibEntry.getSharedBibEntryData().setVersion(version); sharedEntries.add(bibEntry); @@ -638,14 +638,12 @@ public List getSharedEntries() { */ public Map getSharedIDVersionMapping() { Map sharedIDVersionMapping = new HashMap<>(); - String selectEntryQuery = """ - SELECT * FROM ENTRY - ORDER BY 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); @@ -706,7 +704,7 @@ public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) { // Disable cleanup output of ThreadedHousekeeper // Logger.getLogger(ThreadedHousekeeper.class.getName()).setLevel(Level.SEVERE); try { - connection.createStatement().execute("LISTEN \"jabref-FieldChangeEvent\""); + 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); diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 74adc714b38..5bfa31a93a5 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -186,7 +186,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().getSharedId())) { remoteEntryMatchingOneLocalEntryFound = true; if (idVersionEntry.getValue() > localEntry.getSharedBibEntryData().getVersion()) { Optional sharedEntry = dbmsProcessor.getSharedEntry(idVersionEntry.getKey()); @@ -230,7 +230,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().getSharedId())) .collect(Collectors.toList()); if (!entriesToRemove.isEmpty()) { eventBus.post(new SharedEntriesNotPresentEvent(entriesToRemove)); diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index 5b1793b4fa1..fa46b1b02ff 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -916,7 +916,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..6789d1f7b76 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().getSharedId(), 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..4646296a6c4 100644 --- a/src/main/java/org/jabref/model/entry/SharedBibEntryData.java +++ b/src/main/java/org/jabref/model/entry/SharedBibEntryData.java @@ -11,23 +11,23 @@ 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 int sharedId; // Needed for version controlling if used on shared database // This is "Revision" in JabDrive sync private int version; public SharedBibEntryData() { - this.sharedID = -1; + this.sharedId = -1; this.version = 1; } - public int getSharedID() { - return sharedID; + public int getSharedId() { + return sharedId; } - public void setSharedID(int sharedID) { - this.sharedID = sharedID; + public void setSharedId(int sharedId) { + this.sharedId = sharedId; } public int getVersion() { @@ -41,17 +41,17 @@ public void setVersion(int version) { @Override public String toString() { return MoreObjects.toStringHelper(this) - .add("sharedID", sharedID) + .add("sharedId", sharedId) .add("version", version) .toString(); } @Override public int compareTo(SharedBibEntryData o) { - if (this.sharedID == o.sharedID) { + if (this.sharedId == o.sharedId) { return Integer.compare(this.version, o.version); } else { - return Integer.compare(this.sharedID, o.sharedID); + return Integer.compare(this.sharedId, o.sharedId); } } } diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index 370f2306287..a65a9c0ec5d 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -72,7 +72,7 @@ void insertEntry() throws SQLException { dbmsProcessor.insertEntry(expectedEntry); BibEntry emptyEntry = getBibEntryExample(); - emptyEntry.getSharedBibEntryData().setSharedID(1); + emptyEntry.getSharedBibEntryData().setSharedId(1); dbmsProcessor.insertEntry(emptyEntry); // does not insert, due to same sharedID. Map actualFieldMap = new HashMap<>(); @@ -136,7 +136,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().getSharedId()); assertEquals(Optional.of(expectedEntry), actualEntry); } @@ -150,7 +150,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().getSharedId()); assertEquals(Optional.of(expectedEntry), actualEntry); } @@ -192,7 +192,7 @@ void updateEqualEntry() throws OfflineLockException, SQLException { dbmsProcessor.updateEntry(expectedBibEntry); Optional actualBibEntryOptional = dbmsProcessor - .getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedID()); + .getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedId()); assertEquals(Optional.of(expectedBibEntry), actualBibEntryOptional); } @@ -274,7 +274,7 @@ void getSharedEntry() { dbmsProcessor.insertEntry(expectedBibEntry); - Optional actualBibEntryOptional = dbmsProcessor.getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedID()); + Optional actualBibEntryOptional = dbmsProcessor.getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedId()); assertEquals(Optional.of(expectedBibEntry), actualBibEntryOptional); } @@ -296,8 +296,8 @@ void getSharedIDVersionMapping() throws OfflineLockException, SQLException { dbmsProcessor.updateEntry(secondEntry); Map expectedIDVersionMap = new HashMap<>(); - expectedIDVersionMap.put(firstEntry.getSharedBibEntryData().getSharedID(), 1); - expectedIDVersionMap.put(secondEntry.getSharedBibEntryData().getSharedID(), 2); + expectedIDVersionMap.put(firstEntry.getSharedBibEntryData().getSharedId(), 1); + expectedIDVersionMap.put(secondEntry.getSharedBibEntryData().getSharedId(), 2); Map actualIDVersionMap = dbmsProcessor.getSharedIDVersionMapping(); @@ -344,7 +344,7 @@ private static BibEntry getBibEntryExampleWithEmptyFields() { .withField(StandardField.AUTHOR, "Author") .withField(StandardField.TITLE, "") .withField(StandardField.YEAR, ""); - bibEntry.getSharedBibEntryData().setSharedID(1); + bibEntry.getSharedBibEntryData().setSharedId(1); return bibEntry; } @@ -417,7 +417,7 @@ void insertMultipleEntries() throws SQLException { } } Map> expectedFieldMap = entries.stream() - .collect(Collectors.toMap(bibEntry -> bibEntry.getSharedBibEntryData().getSharedID(), + .collect(Collectors.toMap(bibEntry -> bibEntry.getSharedBibEntryData().getSharedId(), bibEntry -> bibEntry.getFieldMap().entrySet().stream() .collect(Collectors.toMap(entry -> entry.getKey().getName(), Map.Entry::getValue)))); diff --git a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java index 1af261344e1..817eb26b067 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java @@ -54,7 +54,7 @@ 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; } @@ -207,7 +207,7 @@ void synchronizeLocalDatabaseWithEntryUpdate() throws Exception { modifiedBibEntry.setType(StandardEntryType.Article); dbmsProcessor.updateEntry(modifiedBibEntry); - assertEquals(1, modifiedBibEntry.getSharedBibEntryData().getSharedID()); + assertEquals(1, modifiedBibEntry.getSharedBibEntryData().getSharedId()); dbmsSynchronizer.synchronizeLocalDatabase(); assertEquals(List.of(modifiedBibEntry), bibDatabase.getEntries()); From c1fbcc47572dbfd0a69dfb2d7c91eb3511207c48 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 01:40:25 +0200 Subject: [PATCH 36/47] Rename "Revision" to "Version" --- docs/code-howtos/remote-storage-jabdrive.md | 76 ++++++++++----------- 1 file changed, 38 insertions(+), 38 deletions(-) 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 From 5b83b84570b37279b73418fc08971f60eaf17576 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 01:54:45 +0200 Subject: [PATCH 37/47] Streamline insertEntries --- .../jabref/logic/shared/DBMSProcessor.java | 45 +++---------------- 1 file changed, 5 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index e1d4d6dba8f..32529ed654b 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -23,7 +23,6 @@ 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; @@ -31,6 +30,7 @@ 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; @@ -244,6 +244,7 @@ Integer getCURRENT_VERSION_DB_STRUCT() { * * @param bibEntry {@link BibEntry} to be inserted. */ + @VisibleForTesting public void insertEntry(BibEntry bibEntry) { insertEntries(Collections.singletonList(bibEntry)); } @@ -254,12 +255,9 @@ public void insertEntry(BibEntry bibEntry) { * @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().getSharedId() != "").findAny().isEmpty(); + insertIntoEntryTable(bibEntries); + insertIntoFieldTable(bibEntries); } /** @@ -301,39 +299,6 @@ protected void insertIntoEntryTable(List bibEntries) { } } - /** - * 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 ENTRY"; - - try (ResultSet resultSet = connection.createStatement().executeQuery(selectQuery)) { - while (resultSet.next()) { - int id = resultSet.getInt("SHARED_ID"); - remoteIds.add(id); - } - } - } catch (SQLException e) { - LOGGER.error("SQL Error: ", e); - } - return bibEntries.stream().filter(entry -> - !remoteIds.contains(entry.getSharedBibEntryData().getSharedId())) - .collect(Collectors.toList()); - } - /** * Inserts the given list of BibEntry into FIELD table. * From 6c1c2b00a47f6c078edefd181c1117ab17d830cb Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Sat, 5 Oct 2024 02:13:27 +0200 Subject: [PATCH 38/47] Begin to revine remote-storage-sql.md --- docs/code-howtos/remote-storage-sql.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/code-howtos/remote-storage-sql.md b/docs/code-howtos/remote-storage-sql.md index 5a28cbc5cc4..e835694df7d 100644 --- a/docs/code-howtos/remote-storage-sql.md +++ b/docs/code-howtos/remote-storage-sql.md @@ -7,6 +7,21 @@ 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. From 5bc7abdcc0fb9f92781d1f5aba23a2f1560d3f68 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 7 Oct 2024 12:26:37 +0200 Subject: [PATCH 39/47] Some minor code comments and SQL optimization --- src/main/java/org/jabref/logic/shared/DBMSProcessor.java | 9 ++++++--- .../java/org/jabref/logic/shared/DBMSSynchronizer.java | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 32529ed654b..2674c667bfa 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -153,6 +153,7 @@ public void setUp() throws SQLException { 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, @@ -301,6 +302,7 @@ protected void insertIntoEntryTable(List bibEntries) { /** * 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 */ @@ -324,11 +326,12 @@ protected void insertIntoFieldTable(List bibEntries) { } 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++) { @@ -435,7 +438,7 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { } String selectFieldQuery = """ - SELECT * FROM FIELD + SELECT name FROM FIELD WHERE NAME = ? AND ENTRY_SHARED_ID = ? """; diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 5bfa31a93a5..6f0a6aefa43 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -84,6 +84,7 @@ public void listen(EntriesAddedEvent event) { pullWithLastEntry(); synchronizeLocalDatabase(); + // TODO: Rewrite dbmsProcessor.insertEntries(event.getBibEntries()); // Reset last changed entry because it just has already been synchronized -> Why necessary? lastEntryChanged_REMOVEME = Optional.empty(); From c901b8464b1c7e3d89e3cfb377e6e88032ce426b Mon Sep 17 00:00:00 2001 From: Siedlerchr Date: Tue, 8 Oct 2024 19:38:45 +0200 Subject: [PATCH 40/47] fix compile errors --- .../org/jabref/logic/shared/DBMSProcessor.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 32529ed654b..ae266bcf51e 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -40,7 +40,7 @@ /** * Processes all incoming or outgoing bib data to external SQL Database and manages its structure. */ -public final class DBMSProcessor { +public class DBMSProcessor { public static final String PROCESSOR_ID = CUID.randomCUID2(8).toString(); @@ -55,7 +55,7 @@ public final class DBMSProcessor { private int VERSION_DB_STRUCT_DEFAULT = -1; // TODO: We need to migrate data - or ask the user to recreate - private final int CURRENT_VERSION_DB_STRUCT = 2; + private int CURRENT_VERSION_DB_STRUCT = 2; protected DBMSProcessor(DatabaseConnection dbmsConnection) { this.connection = dbmsConnection.getConnection(); @@ -235,7 +235,7 @@ ON CONFLICT (KEY) // - table names and field names now lower case } - Integer getCURRENT_VERSION_DB_STRUCT() { + int getCURRENT_VERSION_DB_STRUCT() { return CURRENT_VERSION_DB_STRUCT; } @@ -255,7 +255,8 @@ public void insertEntry(BibEntry bibEntry) { * @param bibEntries List of {@link BibEntry} to be inserted */ public void insertEntries(List bibEntries) { - assert bibEntries.stream().filter(bibEntry -> bibEntry.getSharedBibEntryData().getSharedId() != "").findAny().isEmpty(); + // TODO: Check if this condition makes sense? + assert bibEntries.stream().filter(bibEntry -> bibEntry.getSharedBibEntryData().getSharedId() != -1).findAny().isEmpty(); insertIntoEntryTable(bibEntries); insertIntoFieldTable(bibEntries); } @@ -488,11 +489,11 @@ public void removeEntries(List bibEntries) { if (bibEntries.isEmpty()) { return; } - StringBuilder query = new StringBuilder().append("DELETE FROM ENTRY WHERE SHARED_ID 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()); } From d1fd3f0ec98a1d2c6e0d955ecc5c398faac46750 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 9 Oct 2024 08:03:30 +0200 Subject: [PATCH 41/47] WIP: notificatoin --- .../jabref/logic/shared/DBMSProcessor.java | 27 ++++++------ .../jabref/logic/shared/DBMSSynchronizer.java | 27 ++++++++---- .../shared/notifications/FieldChange.java | 8 ++++ .../logic/shared/notifications/Notifier.java | 22 ++++++++++ .../jabref/model/entry/CanonicalBibEntry.java | 2 +- .../model/entry/SharedBibEntryData.java | 44 +++++++++++++++---- .../logic/shared/DBMSProcessorTest.java | 31 ++++++------- .../logic/shared/DBMSSynchronizerTest.java | 2 +- 8 files changed, 111 insertions(+), 52 deletions(-) create mode 100644 src/main/java/org/jabref/logic/shared/notifications/FieldChange.java create mode 100644 src/main/java/org/jabref/logic/shared/notifications/Notifier.java diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index afb6cbcc814..62d58df68a5 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -256,8 +256,7 @@ public void insertEntry(BibEntry bibEntry) { * @param bibEntries List of {@link BibEntry} to be inserted */ public void insertEntries(List bibEntries) { - // TODO: Check if this condition makes sense? - assert bibEntries.stream().filter(bibEntry -> bibEntry.getSharedBibEntryData().getSharedId() != -1).findAny().isEmpty(); + assert bibEntries.stream().filter(bibEntry -> bibEntry.getSharedBibEntryData().getSharedIdAsInt() != -1).findAny().isEmpty(); insertIntoEntryTable(bibEntries); insertIntoFieldTable(bibEntries); } @@ -303,7 +302,7 @@ protected void insertIntoEntryTable(List bibEntries) { /** * Inserts the given list of BibEntry into FIELD table. - * These entries do not yet exist in the remote database + * These entries do not yet exist in the remote database. * * @param bibEntries {@link BibEntry} to be inserted */ @@ -338,7 +337,7 @@ protected void insertIntoFieldTable(List bibEntries) { 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; @@ -363,7 +362,7 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL 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; @@ -388,7 +387,7 @@ public void updateEntry(BibEntry localBibEntry) throws OfflineLockException, SQL 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(); } @@ -419,7 +418,7 @@ private void removeSharedFieldsByDifference(BibEntry localBibEntry, BibEntry sha 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(); } } @@ -446,7 +445,7 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { 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 @@ -460,7 +459,7 @@ private void insertOrUpdateFields(BibEntry localBibEntry) throws SQLException { .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 { @@ -471,7 +470,7 @@ INSERT INTO FIELD (ENTRY_SHARED_ID, NAME, VALUE) 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(); @@ -498,7 +497,7 @@ public void removeEntries(List bibEntries) { 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) { @@ -511,7 +510,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 { @@ -538,7 +537,7 @@ public List partitionAndGetSharedEntries(List sharedIDs) { /** * Queries the database for shared entries. Optionally, they are filtered by the given list of sharedIds * - * @param sharedIDs the list of Ids to filter. If list is empty, then no filter is applied + * @param sharedIds the list of Ids to filter. If list is empty, then no filter is applied */ public List getSharedEntries(List sharedIDs) { Objects.requireNonNull(sharedIDs); @@ -546,7 +545,7 @@ public List getSharedEntries(List sharedIDs) { List sharedEntries = new ArrayList<>(); StringBuilder query = new StringBuilder() - .append("SELECT entry.shared_id, entry.entrytype, entry.version, ") + .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"); diff --git a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index 6f0a6aefa43..cae2c372b4c 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -19,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; @@ -33,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; @@ -45,16 +47,19 @@ 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; private final FieldPreferences fieldPreferences; private final FileUpdateMonitor fileMonitor; + private Optional lastEntryChanged_REMOVEME; public DBMSSynchronizer(BibDatabaseContext bibDatabaseContext, Character keywordSeparator, @@ -102,14 +107,12 @@ public void listen(FieldChangedEvent event) { return; } + notifier.notifyAboutChangedField(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). - - synchronizeLocalMetaData(); - pullWithLastEntry(); - synchronizeSharedEntry(bibEntry); - synchronizeLocalDatabase(); // Pull changes for the case that there were some + // connection.createStatement().execute("NOTIFY jabrefLiveUpdate, '" + PROCESSOR_ID + "';"); } /** @@ -187,7 +190,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()); @@ -231,7 +234,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)); @@ -391,6 +394,12 @@ public boolean isEventSourceAccepted(EntriesEvent event) { public void openSharedDatabase(DatabaseConnection connection) throws DatabaseNotSupportedException { this.dbName = connection.getProperties().getDatabase(); this.currentConnection = connection.getConnection(); + 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(); } 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/notifications/Notifier.java b/src/main/java/org/jabref/logic/shared/notifications/Notifier.java new file mode 100644 index 00000000000..7c485dc6743 --- /dev/null +++ b/src/main/java/org/jabref/logic/shared/notifications/Notifier.java @@ -0,0 +1,22 @@ +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) {} +} diff --git a/src/main/java/org/jabref/model/entry/CanonicalBibEntry.java b/src/main/java/org/jabref/model/entry/CanonicalBibEntry.java index 6789d1f7b76..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 4646296a6c4..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/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index a65a9c0ec5d..b7d3098b5cc 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -136,7 +136,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 +150,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 +192,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); } @@ -274,7 +274,7 @@ void getSharedEntry() { dbmsProcessor.insertEntry(expectedBibEntry); - Optional actualBibEntryOptional = dbmsProcessor.getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedId()); + Optional actualBibEntryOptional = dbmsProcessor.getSharedEntry(expectedBibEntry.getSharedBibEntryData().getSharedIdAsInt()); assertEquals(Optional.of(expectedBibEntry), actualBibEntryOptional); } @@ -286,7 +286,7 @@ void getNotExistingSharedEntry() { } @Test - void getSharedIDVersionMapping() throws OfflineLockException, SQLException { + void getSharedIdVersionMapping() throws OfflineLockException, SQLException { BibEntry firstEntry = getBibEntryExample(); dbmsProcessor.insertEntry(firstEntry); @@ -296,8 +296,8 @@ void getSharedIDVersionMapping() throws OfflineLockException, SQLException { 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(); @@ -403,21 +403,16 @@ void insertMultipleEntries() throws SQLException { 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)))); diff --git a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java index 817eb26b067..d57fd842e48 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSSynchronizerTest.java @@ -207,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()); From 598d83e8c2d5b5e09246f54adf822d3c798b948e Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 9 Oct 2024 23:39:37 +0200 Subject: [PATCH 42/47] Fix compile error (and typo) --- .../java/org/jabref/gui/shared/SharedDatabaseUIManager.java | 2 +- src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java index 95d489bc3d3..5a97dce0498 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/logic/shared/DBMSSynchronizer.java b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java index cae2c372b4c..6778e879111 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java +++ b/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java @@ -57,7 +57,10 @@ public class DBMSSynchronizer implements DatabaseSynchronizer { private final EventBus eventBus; private final Character keywordSeparator; private final GlobalCitationKeyPatterns globalCiteKeyPattern; + + // will be required later for save actions private final FieldPreferences fieldPreferences; + private final FileUpdateMonitor fileMonitor; private Optional lastEntryChanged_REMOVEME; @@ -161,7 +164,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); } From a78afc99fdf99d2b4446d1bad3c499750b305f1b Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 10 Oct 2024 08:30:37 +0200 Subject: [PATCH 43/47] Add databaseName as parameter --- .../java/org/jabref/logic/shared/DBMSConnection.java | 4 ++-- .../jabref/logic/shared/DBMSConnectionProperties.java | 10 ++++++++-- .../java/org/jabref/logic/shared/ConnectorTest.java | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSConnection.java b/src/main/java/org/jabref/logic/shared/DBMSConnection.java index 3d4a755ddf5..859ae4a3d52 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSConnection.java +++ b/src/main/java/org/jabref/logic/shared/DBMSConnection.java @@ -21,9 +21,9 @@ public class DBMSConnection implements DatabaseConnection { private final DBMSConnectionProperties properties; @VisibleForTesting - public DBMSConnection(Connection connection) { + public DBMSConnection(Connection connection, String databaseName) { this.connection = connection; - this.properties = null; + this.properties = new DBMSConnectionProperties(databaseName); } public DBMSConnection(DBMSConnectionProperties connectionProperties) throws SQLException, InvalidDBMSConnectionPropertiesException { 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/test/java/org/jabref/logic/shared/ConnectorTest.java b/src/test/java/org/jabref/logic/shared/ConnectorTest.java index d87d407798f..a76af43a168 100644 --- a/src/test/java/org/jabref/logic/shared/ConnectorTest.java +++ b/src/test/java/org/jabref/logic/shared/ConnectorTest.java @@ -25,7 +25,7 @@ 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); + return new DBMSConnection(connection, "postgres"); } /** From 7eb4b079429195feffe6b72d33d33f65c71894b7 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 10 Oct 2024 13:51:58 +0200 Subject: [PATCH 44/47] Compilefix --- .../java/org/jabref/gui/shared/SharedDatabaseUIManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java b/src/main/java/org/jabref/gui/shared/SharedDatabaseUIManager.java index 95d489bc3d3..2ae434d58cb 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().getSharedIdAsInt()); mergedBibEntry.getSharedBibEntryData().setVersion(sharedBibEntry.getSharedBibEntryData().getVersion()); dbmsSynchronizer.synchronizeSharedEntry(mergedBibEntry); From bc68c1971ff5fd1de8c01136ea556af708e12076 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 10 Oct 2024 15:41:41 +0200 Subject: [PATCH 45/47] Remove unnecessary JavaDoc --- .../java/org/jabref/logic/shared/DBMSProcessor.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index 62d58df68a5..b5ab257e23b 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -250,22 +250,12 @@ 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) { 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) { if (bibEntries.isEmpty()) { return; From dd648309b22c7935de30e6e30d1157c0c1846ab2 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 10 Oct 2024 15:41:57 +0200 Subject: [PATCH 46/47] Fix tests --- .../org/jabref/logic/shared/DBMSProcessor.java | 2 +- .../jabref/logic/shared/DBMSProcessorTest.java | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java index b5ab257e23b..971e7b3558a 100644 --- a/src/main/java/org/jabref/logic/shared/DBMSProcessor.java +++ b/src/main/java/org/jabref/logic/shared/DBMSProcessor.java @@ -535,7 +535,7 @@ public List getSharedEntries(List sharedIDs) { List sharedEntries = new ArrayList<>(); StringBuilder query = new StringBuilder() - .append("SELECT entry.shared_id, entry.version, entry.entrytype ") + .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"); diff --git a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java index b7d3098b5cc..56c8d896dab 100644 --- a/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java +++ b/src/test/java/org/jabref/logic/shared/DBMSProcessorTest.java @@ -73,7 +73,6 @@ void insertEntry() throws SQLException { BibEntry emptyEntry = getBibEntryExample(); emptyEntry.getSharedBibEntryData().setSharedId(1); - dbmsProcessor.insertEntry(emptyEntry); // does not insert, due to same sharedID. Map actualFieldMap = new HashMap<>(); @@ -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().getSharedIdAsInt()); + bibEntry = getBibEntryExampleWithEmptyFields(); + bibEntry.getSharedBibEntryData().setSharedId(1); - assertEquals(Optional.of(expectedBibEntry), actualBibEntryOptional); + Optional actualBibEntryOptional = dbmsProcessor.getSharedEntry(bibEntry.getSharedBibEntryData().getSharedIdAsInt()); + + assertEquals(Optional.of(bibEntry), actualBibEntryOptional); } @Test @@ -344,7 +349,6 @@ private static BibEntry getBibEntryExampleWithEmptyFields() { .withField(StandardField.AUTHOR, "Author") .withField(StandardField.TITLE, "") .withField(StandardField.YEAR, ""); - bibEntry.getSharedBibEntryData().setSharedId(1); return bibEntry; } From 1b896d033d0b870941cc7301e2ea028b8a995c92 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Thu, 7 Nov 2024 14:02:29 +0100 Subject: [PATCH 47/47] Compile fix --- src/main/java/module-info.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 9d623882183..44898f84027 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -98,9 +98,8 @@ // region: SQL databases requires embedded.postgres; - requires org.tukaani.xz; - requires ojdbc10; requires org.postgresql.jdbc; + requires org.tukaani.xz; // endregion // region: Apache Commons and other (similar) helper libraries