Fixes Shared Database: Changes filtering in CoarseChangeFilter to attribute property (#6868)

Author: m-mauersberger

docs/advanced-reading/remote-storage.md

@@ -0,0 +1,20 @@
+# Remote Storage
+
+## Using a shared PostgreSQL database
+
+...
+
+## Handling large shared databases
+
+Synchronization times may get long when working with a large database containing several thousand entries. Therefore, synchronization only happens if several conditions are fulfilled:
+
+* 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 has 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`).
+

src/main/java/org/jabref/logic/autosaveandbackup/AutosaveManager.java

@@ -42,6 +42,12 @@ private AutosaveManager(BibDatabaseContext bibDatabaseContext) {
 
     @Subscribe
     public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) {
+        if (!event.isFilteredOut()) {
+            startAutosaveTask();
+        }
+    }
+
+    private void startAutosaveTask() {
         throttler.schedule(() -> {
             eventBus.post(new AutosaveEvent());
         });

src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java

@@ -156,7 +156,9 @@ private void logIfCritical(Path backupPath, IOException e) {
 
     @Subscribe
     public synchronized void listen(@SuppressWarnings("unused") BibDatabaseContextChangedEvent event) {
-        startBackupTask();
+        if (!event.isFilteredOut()) {
+            startBackupTask();
+        }
     }
 
     private void startBackupTask() {

src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java

@@ -54,6 +54,7 @@ public class DBMSSynchronizer implements DatabaseSynchronizer {
     private final Character keywordSeparator;
     private final GlobalCitationKeyPattern globalCiteKeyPattern;
     private final FileUpdateMonitor fileMonitor;
+    private Optional<BibEntry> lastEntryChanged;
 
     public DBMSSynchronizer(BibDatabaseContext bibDatabaseContext, Character keywordSeparator,
                             GlobalCitationKeyPattern globalCiteKeyPattern, FileUpdateMonitor fileMonitor) {
@@ -64,6 +65,7 @@ public DBMSSynchronizer(BibDatabaseContext bibDatabaseContext, Character keyword
         this.eventBus = new EventBus();
         this.keywordSeparator = keywordSeparator;
         this.globalCiteKeyPattern = Objects.requireNonNull(globalCiteKeyPattern);
+        this.lastEntryChanged = Optional.empty();
     }
 
     /**
@@ -77,10 +79,13 @@ public void listen(EntriesAddedEvent event) {
         // In this case DBSynchronizer should not try to insert the bibEntry entry again (but it would not harm).
         if (isEventSourceAccepted(event) && checkCurrentConnection()) {
             synchronizeLocalMetaData();
-            synchronizeLocalDatabase(); // Pull changes for the case that there were some
+            pullWithLastEntry();
+            synchronizeLocalDatabase();
             dbmsProcessor.insertEntries(event.getBibEntries());
-            }
+            // Reset last changed entry because it just has already been synchronized -> Why necessary?
+            lastEntryChanged = Optional.empty();
         }
+    }
 
     /**
      * Listening method. Updates an existing shared {@link BibEntry}.
@@ -89,13 +94,17 @@ public void listen(EntriesAddedEvent event) {
      */
     @Subscribe
     public void listen(FieldChangedEvent event) {
+        BibEntry bibEntry = event.getBibEntry();
         // While synchronizing the local database (see synchronizeLocalDatabase() below), some EntriesEvents may be posted.
         // In this case DBSynchronizer should not try to update the bibEntry entry again (but it would not harm).
-        if (isPresentLocalBibEntry(event.getBibEntry()) && isEventSourceAccepted(event) && checkCurrentConnection()) {
+        if (isPresentLocalBibEntry(bibEntry) && isEventSourceAccepted(event) && checkCurrentConnection() && !event.isFilteredOut()) {
             synchronizeLocalMetaData();
-            BibEntry bibEntry = event.getBibEntry();
+            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);
         }
     }
 
@@ -110,10 +119,10 @@ public void listen(EntriesRemovedEvent event) {
         // While synchronizing the local database (see synchronizeLocalDatabase() below), some EntriesEvents may be posted.
         // In this case DBSynchronizer should not try to delete the bibEntry entry again (but it would not harm).
         if (isEventSourceAccepted(event) && checkCurrentConnection()) {
-            List<BibEntry> entries = event.getBibEntries();
-            dbmsProcessor.removeEntries(entries);
             synchronizeLocalMetaData();
-            synchronizeLocalDatabase(); // Pull changes for the case that there where some
+            pullWithLastEntry();
+            dbmsProcessor.removeEntries(event.getBibEntries());
+            synchronizeLocalDatabase();
         }
     }
 
@@ -313,11 +322,32 @@ public void pullChanges() {
         if (!checkCurrentConnection()) {
             return;
         }
-
+        // First synchronize entry, then synchronize database
+        pullWithLastEntry();
         synchronizeLocalDatabase();
         synchronizeLocalMetaData();
     }
 
+    // Synchronizes local BibEntries only if last entry changes still remain
+    public void pullLastEntryChanges() {
+        if (!lastEntryChanged.isEmpty()) {
+            if (!checkCurrentConnection()) {
+                return;
+            }
+            synchronizeLocalMetaData();
+            pullWithLastEntry();
+            synchronizeLocalDatabase(); // Pull changes for the case that there were some
+        }
+    }
+
+    // Synchronizes local BibEntries and pulls remaining last entry changes
+    private void pullWithLastEntry() {
+        if (!lastEntryChanged.isEmpty() && isPresentLocalBibEntry(lastEntryChanged.get())) {
+            synchronizeSharedEntry(lastEntryChanged.get());
+        }
+        lastEntryChanged = Optional.empty();
+    }
+
     /**
      * Checks whether the current SQL connection is valid. In case that the connection is not valid a new {@link
      * ConnectionLostEvent} is going to be sent.
@@ -360,6 +390,8 @@ public void openSharedDatabase(DatabaseConnection connection) throws DatabaseNot
 
     @Override
     public void closeSharedDatabase() {
+        // Submit remaining entry changes
+        pullLastEntryChanges();
         try {
             dbmsProcessor.stopNotificationListener();
             currentConnection.close();

src/main/java/org/jabref/logic/util/CoarseChangeFilter.java

@@ -4,6 +4,7 @@
 
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
+import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.event.FieldChangedEvent;
 import org.jabref.model.entry.field.Field;
 
@@ -17,54 +18,44 @@ public class CoarseChangeFilter {
 
     private final BibDatabaseContext context;
     private final EventBus eventBus = new EventBus();
-    private final DelayTaskThrottler delayPost;
 
     private Optional<Field> lastFieldChanged;
+    private Optional<BibEntry> lastEntryChanged;
     private int totalDelta;
 
     public CoarseChangeFilter(BibDatabaseContext bibDatabaseContext) {
         // Listen for change events
-        bibDatabaseContext.getDatabase().registerListener(this);
-        bibDatabaseContext.getMetaData().registerListener(this);
         this.context = bibDatabaseContext;
-        // Delay event post by 5 seconds
-        this.delayPost = new DelayTaskThrottler(5000);
+        context.getDatabase().registerListener(this);
+        context.getMetaData().registerListener(this);
         this.lastFieldChanged = Optional.empty();
-        this.totalDelta = 0;
+        this.lastEntryChanged = Optional.empty();
     }
 
     @Subscribe
     public synchronized void listen(BibDatabaseContextChangedEvent event) {
-        Runnable eventPost = () -> {
-            // Reset total change delta
-            totalDelta = 0;
-            // Post event
-            eventBus.post(event);
-        };
 
-        if (!(event instanceof FieldChangedEvent)) {
-            eventPost.run();
-        } else {
+        if (event instanceof FieldChangedEvent) {
             // Only relay event if the field changes are more than one character or a new field is edited
             FieldChangedEvent fieldChange = (FieldChangedEvent) event;
-            // Sum up change delta
-            totalDelta += fieldChange.getDelta();
-
-            // If editing is started
-            boolean isNewEdit = lastFieldChanged.isEmpty();
-            // If other field is edited
-            boolean isEditOnOtherField = !isNewEdit && !lastFieldChanged.get().equals(fieldChange.getField());
-            // Only deltas of 1 registered by fieldChange, major change means editing much content
-            boolean isMajorChange = totalDelta >= 100;
-
-            if ((isEditOnOtherField && !isNewEdit) || isMajorChange) {
-                // Submit old changes immediately
-                eventPost.run();
-            } else {
-                delayPost.schedule(eventPost);
-            }
-            // Set new last field
+
+            // If editing has started
+            boolean isNewEdit = lastFieldChanged.isEmpty() || lastEntryChanged.isEmpty();
+
+            boolean isChangedField = lastFieldChanged.filter(f -> !f.equals(fieldChange.getField())).isPresent();
+            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.getDelta() > 1;
+
+            fieldChange.setFilteredOut(!(isEditChanged || isMajorChange));
+            // Post each FieldChangedEvent - even the ones being marked as "filtered"
+            eventBus.post(fieldChange);
+
             lastFieldChanged = Optional.of(fieldChange.getField());
+            lastEntryChanged = Optional.of(fieldChange.getBibEntry());
+        } else {
+            eventBus.post(event);
         }
     }
 

src/main/java/org/jabref/logic/util/DelayTaskThrottler.java

@@ -25,7 +25,8 @@ public class DelayTaskThrottler {
     private static final Logger LOGGER = LoggerFactory.getLogger(DelayTaskThrottler.class);
 
     private final ScheduledThreadPoolExecutor executor;
-    private final int delay;
+
+    private int delay;
 
     private ScheduledFuture<?> scheduledTask;
 
@@ -41,7 +42,7 @@ public DelayTaskThrottler(int delay) {
 
     public ScheduledFuture<?> schedule(Runnable command) {
         if (scheduledTask != null) {
-            scheduledTask.cancel(false);
+            cancel();
         }
         try {
             scheduledTask = executor.schedule(command, delay, TimeUnit.MILLISECONDS);
@@ -51,9 +52,20 @@ public ScheduledFuture<?> schedule(Runnable command) {
         return scheduledTask;
     }
 
+    // Execute scheduled Runnable early
+    public void execute(Runnable command) {
+        delay = 0;
+        schedule(command);
+    }
+
+    // Cancel scheduled Runnable gracefully
+    public void cancel() {
+        scheduledTask.cancel(false);
+    }
+
     public <T> ScheduledFuture<?> scheduleTask(Callable<?> command) {
         if (scheduledTask != null) {
-            scheduledTask.cancel(false);
+            cancel();
         }
         try {
             scheduledTask = executor.schedule(command, delay, TimeUnit.MILLISECONDS);

src/main/java/org/jabref/model/database/event/BibDatabaseContextChangedEvent.java

@@ -1,11 +1,29 @@
 package org.jabref.model.database.event;
 
+import org.jabref.model.entry.event.EntriesEvent;
 import org.jabref.model.groups.event.GroupUpdatedEvent;
 import org.jabref.model.metadata.event.MetaDataChangedEvent;
 
 /**
  * This Event is automatically fired at the same time as {@link EntriesEvent}, {@link GroupUpdatedEvent} or {@link MetaDataChangedEvent}.
  */
 public class BibDatabaseContextChangedEvent {
-    // no data
+    // If the event has been filtered out
+    private boolean filteredOut;
+
+    public BibDatabaseContextChangedEvent() {
+        this(false);
+    }
+
+    public BibDatabaseContextChangedEvent(boolean filteredOut) {
+        this.filteredOut = filteredOut;
+    }
+
+    public boolean isFilteredOut() {
+        return filteredOut;
+    }
+
+    public void setFilteredOut(boolean filtered) {
+        this.filteredOut = filtered;
+    }
 }

src/main/java/org/jabref/model/entry/event/EntriesEvent.java

@@ -26,6 +26,7 @@ public EntriesEvent(List<BibEntry> bibEntries) {
      * @param location   Location affected by this event
      */
     public EntriesEvent(List<BibEntry> bibEntries, EntriesEventSource location) {
+        super();
         this.bibEntries = Objects.requireNonNull(bibEntries);
         this.location = Objects.requireNonNull(location);
     }

src/main/java/org/jabref/model/entry/event/FieldChangedEvent.java

@@ -27,7 +27,7 @@ public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String
         this.field = field;
         this.newValue = newValue;
         this.oldValue = oldValue;
-        delta = computeDelta(oldValue, newValue);
+        this.delta = computeDelta(oldValue, newValue);
     }
 
     /**
@@ -40,7 +40,7 @@ public FieldChangedEvent(BibEntry bibEntry, Field field, String newValue, String
         this.field = field;
         this.newValue = newValue;
         this.oldValue = oldValue;
-        delta = computeDelta(oldValue, newValue);
+        this.delta = computeDelta(oldValue, newValue);
     }
 
     /**
@@ -51,7 +51,7 @@ public FieldChangedEvent(FieldChange fieldChange, EntriesEventSource location) {
         this.field = fieldChange.getField();
         this.newValue = fieldChange.getNewValue();
         this.oldValue = fieldChange.getOldValue();
-        delta = computeDelta(oldValue, newValue);
+        this.delta = computeDelta(oldValue, newValue);
     }
 
     public FieldChangedEvent(FieldChange fieldChange) {

src/main/java/org/jabref/model/metadata/event/MetaDataChangedEvent.java

@@ -14,6 +14,7 @@ public class MetaDataChangedEvent extends BibDatabaseContextChangedEvent {
      * @param metaData Affected instance
      */
     public MetaDataChangedEvent(MetaData metaData) {
+        super();
         this.metaData = metaData;
     }