From cc5c46743b7a7cf7adda3588d18de3d7dcb2a6a1 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 18 Mar 2025 17:50:25 +0100 Subject: [PATCH] Refactor EventMap: * renamed to VaultEventsMap * split between boilerplate and buissness logic (ObservableMapDecorator) * add LRU cache * fixed VaultEvent compareTo method --- .../common/ObservableMapDecorator.java | 101 ++++++++++ .../cryptomator/common/VaultEventsMap.java | 183 +++++++----------- .../org/cryptomator/common/vaults/Vault.java | 3 +- .../org/cryptomator/event/VaultEvent.java | 7 +- .../ui/eventview/EventListCellController.java | 2 +- .../ui/eventview/EventViewController.java | 15 +- .../ui/mainwindow/VaultListController.java | 2 +- 7 files changed, 191 insertions(+), 122 deletions(-) create mode 100644 src/main/java/org/cryptomator/common/ObservableMapDecorator.java diff --git a/src/main/java/org/cryptomator/common/ObservableMapDecorator.java b/src/main/java/org/cryptomator/common/ObservableMapDecorator.java new file mode 100644 index 000000000..5330c189b --- /dev/null +++ b/src/main/java/org/cryptomator/common/ObservableMapDecorator.java @@ -0,0 +1,101 @@ +package org.cryptomator.common; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javafx.beans.InvalidationListener; +import javafx.collections.MapChangeListener; +import javafx.collections.ObservableMap; +import java.util.Collection; +import java.util.Map; +import java.util.Set; + +public abstract class ObservableMapDecorator implements ObservableMap { + + protected final ObservableMap delegate; + + protected ObservableMapDecorator(ObservableMap delegate) { + this.delegate = delegate; + } + + @Override + public void addListener(MapChangeListener mapChangeListener) { + delegate.addListener(mapChangeListener); + } + + @Override + public void removeListener(MapChangeListener mapChangeListener) { + delegate.removeListener(mapChangeListener); + } + + @Override + public int size() { + return delegate.size(); + } + + @Override + public boolean isEmpty() { + return delegate.isEmpty(); + } + + @Override + public boolean containsKey(Object key) { + return delegate.containsKey(key); + } + + @Override + public boolean containsValue(Object value) { + return delegate.containsValue(value); + } + + @Override + public V get(Object key) { + return delegate.get(key); + } + + @Override + public @Nullable V put(K key, V value) { + return delegate.put(key, value); + } + + @Override + public V remove(Object key) { + return delegate.remove(key); + } + + @Override + public void putAll(@NotNull Map m) { + delegate.putAll(m); + } + + @Override + public void clear() { + delegate.clear(); + } + + @Override + public @NotNull Set keySet() { + return delegate.keySet(); + } + + @Override + public @NotNull Collection values() { + return delegate.values(); + } + + @Override + public @NotNull Set> entrySet() { + return delegate.entrySet(); + } + + @Override + public void addListener(InvalidationListener invalidationListener) { + delegate.addListener(invalidationListener); + } + + @Override + public void removeListener(InvalidationListener invalidationListener) { + delegate.removeListener(invalidationListener); + } + +} diff --git a/src/main/java/org/cryptomator/common/VaultEventsMap.java b/src/main/java/org/cryptomator/common/VaultEventsMap.java index 4bbd20bf7..e15d02532 100644 --- a/src/main/java/org/cryptomator/common/VaultEventsMap.java +++ b/src/main/java/org/cryptomator/common/VaultEventsMap.java @@ -8,156 +8,121 @@ import org.cryptomator.cryptofs.event.ConflictResolvedEvent; import org.cryptomator.cryptofs.event.DecryptionFailedEvent; import org.cryptomator.cryptofs.event.FilesystemEvent; import org.cryptomator.event.VaultEvent; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import javax.inject.Inject; import javax.inject.Singleton; -import javafx.beans.InvalidationListener; import javafx.collections.FXCollections; -import javafx.collections.MapChangeListener; -import javafx.collections.ObservableMap; import java.nio.file.Path; -import java.util.Collection; -import java.util.Comparator; -import java.util.Map; -import java.util.Set; +import java.util.List; +import java.util.TreeSet; /** * Map containing {@link VaultEvent}s. - * The map is keyed by the ciphertext path of the affected resource _and_ the {@link FilesystemEvent}s class in order to group same events + * The map is keyed by three elements: + *
    + *
  • the vault where the event occurred
    • + *
  • an identifying path (can be cleartext or ciphertext)
    • + *
  • the class name of the occurred event
    • + *
+ * *

- * Use {@link VaultEventsMap#put(VaultEvent)} to add an element and {@link VaultEventsMap#remove(VaultEvent)} to remove it. + * Use {@link VaultEventsMap#put(Vault, FilesystemEvent)} to add an element and {@link VaultEventsMap#remove(Vault, FilesystemEvent)} to remove it. *

* The map is size restricted to {@value MAX_SIZE} elements. If a _new_ element (i.e. not already present) is added, the least recently added is removed. */ @Singleton -public class VaultEventsMap implements ObservableMap { +public class VaultEventsMap extends ObservableMapDecorator { private static final int MAX_SIZE = 300; public record Key(Vault v, Path key, Class c) {} - public record Value(FilesystemEvent event, int count) {} - private final ObservableMap delegate; + public record Value(FilesystemEvent mostRecentEvent, int count) {} + + /** + * Internal least-recently-used cache. + */ + private final TreeSet lruCache; @Inject public VaultEventsMap() { - delegate = FXCollections.observableHashMap(); + super(FXCollections.observableHashMap()); + this.lruCache = new TreeSet<>(this::compareToMapKeys); } - @Override - public void addListener(MapChangeListener mapChangeListener) { - delegate.addListener(mapChangeListener); + + /** + * Comparsion method for the lru cache. During comparsion the map is accessed. + * First the entries are compared by the event timestamp, then vaultId, then identifying path and lastly by class name. + * + * @param left a {@link Key} object + * @param right another {@link Key} object, compared to {@code left} + * @return a negative integer, zero, or a positive integer as the first argument is less than, equal to, or greater than the second. + */ + private int compareToMapKeys(Key left, Key right) { + var t1 = delegate.get(left).mostRecentEvent.getTimestamp(); + var t2 = delegate.get(right).mostRecentEvent.getTimestamp(); + var timeComparsion = t1.compareTo(t2); + if (timeComparsion != 0) { + return timeComparsion; + } + var vaultIdComparsion = left.v.getId().compareTo(right.v.getId()); + if (vaultIdComparsion != 0) { + return vaultIdComparsion; + } + var pathComparsion = left.key.compareTo(right.key); + if (pathComparsion != 0) { + return pathComparsion; + } + return left.c.getName().compareTo(right.c.getName()); } - @Override - public void removeListener(MapChangeListener mapChangeListener) { - delegate.removeListener(mapChangeListener); + /** + * Lists all entries in this map as {@link VaultEvent}. The list is sorted ascending by the timestamp of event occurral (and more if it is the same timestamp). + * This method is synchronized with {@link VaultEventsMap#put(Vault, FilesystemEvent)} and {@link VaultEventsMap#remove(Vault, FilesystemEvent)}. + * + * @return a list of vault events, mainly sorted ascending by the event timestamp + */ + public synchronized List listAll() { + return lruCache.stream().map(key -> { + var value = delegate.get(key); + return new VaultEvent(key.v(), value.mostRecentEvent(), value.count()); + }).toList(); } - @Override - public int size() { - return delegate.size(); - } - - @Override - public boolean isEmpty() { - return delegate.isEmpty(); - } - - @Override - public boolean containsKey(Object key) { - return delegate.containsKey(key); - } - - @Override - public boolean containsValue(Object value) { - return delegate.containsValue(value); - } - - @Override - public VaultEvent get(Object key) { - return delegate.get(key); - } - - @Override - public @Nullable VaultEvent put(Key key, VaultEvent value) { - return delegate.put(key, value); - } - - @Override - public VaultEvent remove(Object key) { - return delegate.remove(key); - } - - @Override - public void putAll(@NotNull Map m) { - delegate.putAll(m); - } - - @Override - public void clear() { - delegate.clear(); - } - - @Override - public @NotNull Set keySet() { - return delegate.keySet(); - } - - @Override - public @NotNull Collection values() { - return delegate.values(); - } - - @Override - public @NotNull Set> entrySet() { - return delegate.entrySet(); - } - - @Override - public void addListener(InvalidationListener invalidationListener) { - delegate.addListener(invalidationListener); - } - - @Override - public void removeListener(InvalidationListener invalidationListener) { - delegate.removeListener(invalidationListener); - } - - public synchronized void put(VaultEvent e) { - //compute key - var key = computeKey(e); + public synchronized void put(Vault v, FilesystemEvent e) { + var key = computeKey(v, e); //if-else - var nullOrEntry = delegate.get(key); - if (nullOrEntry == null) { + var entry = delegate.get(key); + if (entry == null) { if (size() == MAX_SIZE) { - delegate.entrySet().stream() // - .min(Comparator.comparing(entry -> entry.getValue().actualEvent().getTimestamp())) // - .ifPresent(oldestEntry -> delegate.remove(oldestEntry.getKey())); + var toRemove = lruCache.first(); + lruCache.remove(toRemove); + delegate.remove(toRemove); } - delegate.put(key, e); + delegate.put(key, new Value(e, 1)); + lruCache.add(key); } else { - delegate.put(key, nullOrEntry.incrementCount(e.actualEvent())); + lruCache.remove(key); + delegate.put(key, new Value(e, entry.count() + 1)); + lruCache.add(key); //correct, because cache-sorting uses the map in comparsionMethod } } - public synchronized VaultEvent remove(VaultEvent similar) { - //compute key - var key = computeKey(similar); - return this.remove(key); + public synchronized Value remove(Vault v, FilesystemEvent similar) { + var key = computeKey(v, similar); + lruCache.remove(key); + return delegate.remove(key); } - private Key computeKey(VaultEvent ve) { - var e = ve.actualEvent(); - var p = switch (e) { + private Key computeKey(Vault v, FilesystemEvent event) { + var p = switch (event) { case DecryptionFailedEvent(_, Path ciphertextPath, _) -> ciphertextPath; case ConflictResolvedEvent(_, _, _, _, Path resolvedCiphertext) -> resolvedCiphertext; case ConflictResolutionFailedEvent(_, _, Path conflictingCiphertext, _) -> conflictingCiphertext; case BrokenDirFileEvent(_, Path ciphertext) -> ciphertext; case BrokenFileNodeEvent(_, _, Path ciphertext) -> ciphertext; }; - return new Key(ve.v(), p, e.getClass()); + return new Key(v, p, event.getClass()); } } diff --git a/src/main/java/org/cryptomator/common/vaults/Vault.java b/src/main/java/org/cryptomator/common/vaults/Vault.java index a8d63f139..d3ba4d6fb 100644 --- a/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -261,9 +261,8 @@ public class Vault { private void consumeVaultEvent(FilesystemEvent e) { - var wrapper = new VaultEvent(this, e); Platform.runLater(() -> { - vaultEventsMap.put(wrapper); + vaultEventsMap.put(this, e); }); } diff --git a/src/main/java/org/cryptomator/event/VaultEvent.java b/src/main/java/org/cryptomator/event/VaultEvent.java index 8b31747cf..091a89323 100644 --- a/src/main/java/org/cryptomator/event/VaultEvent.java +++ b/src/main/java/org/cryptomator/event/VaultEvent.java @@ -16,9 +16,12 @@ public record VaultEvent(Vault v, FilesystemEvent actualEvent, int count) implem var timeResult = actualEvent.getTimestamp().compareTo(other.actualEvent().getTimestamp()); if(timeResult != 0) { return timeResult; - } else { - return this.equals(other) ? 0 : this.actualEvent.getClass().getName().compareTo(other.actualEvent.getClass().getName()); } + var vaultIdResult = v.getId().compareTo(other.v.getId()); + if(vaultIdResult != 0) { + return vaultIdResult; + } + return this.actualEvent.getClass().getName().compareTo(other.actualEvent.getClass().getName()); } public VaultEvent incrementCount(FilesystemEvent update) { diff --git a/src/main/java/org/cryptomator/ui/eventview/EventListCellController.java b/src/main/java/org/cryptomator/ui/eventview/EventListCellController.java index 21a61950d..45c0dae0f 100644 --- a/src/main/java/org/cryptomator/ui/eventview/EventListCellController.java +++ b/src/main/java/org/cryptomator/ui/eventview/EventListCellController.java @@ -113,7 +113,7 @@ public class EventListCellController implements FxController { eventActionsMenu.hide(); eventActionsMenu.getItems().clear(); eventTooltip.setText(item.v().getDisplayName()); - addAction("generic.action.dismiss", () -> vaultEventsMap.remove(item)); + addAction("generic.action.dismiss", () -> vaultEventsMap.remove(item.v(),item.actualEvent())); switch (item.actualEvent()) { case ConflictResolvedEvent fse -> this.adjustToConflictResolvedEvent(fse); case ConflictResolutionFailedEvent fse -> this.adjustToConflictEvent(fse); diff --git a/src/main/java/org/cryptomator/ui/eventview/EventViewController.java b/src/main/java/org/cryptomator/ui/eventview/EventViewController.java index 122da0a3c..fd8af28d3 100644 --- a/src/main/java/org/cryptomator/ui/eventview/EventViewController.java +++ b/src/main/java/org/cryptomator/ui/eventview/EventViewController.java @@ -60,8 +60,8 @@ public class EventViewController implements FxController { } }); - eventList.addAll(vaultEventsMap.values()); - vaultEventsMap.addListener((MapChangeListener) this::updateList); + eventList.addAll(vaultEventsMap.listAll()); + vaultEventsMap.addListener((MapChangeListener) this::updateList); eventListView.setCellFactory(cellFactory); eventListView.setItems(reversedEventList); @@ -70,15 +70,16 @@ public class EventViewController implements FxController { vaultFilterChoiceBox.setConverter(new VaultConverter(resourceBundle)); } - private void updateList(MapChangeListener.Change change) { + private void updateList(MapChangeListener.Change change) { + var vault = change.getKey().v(); if (change.wasAdded() && change.wasRemoved()) { //entry updated - eventList.remove(change.getValueRemoved()); - eventList.addLast(change.getValueAdded()); + eventList.remove(new VaultEvent(vault, change.getValueRemoved().mostRecentEvent(), change.getValueRemoved().count())); + eventList.addLast(new VaultEvent(vault, change.getValueAdded().mostRecentEvent(), change.getValueAdded().count())); } else if (change.wasAdded()) { - eventList.addLast(change.getValueAdded()); + eventList.addLast(new VaultEvent(vault, change.getValueAdded().mostRecentEvent(), change.getValueAdded().count())); } else { //removed - eventList.remove(change.getValueRemoved()); + eventList.remove(new VaultEvent(vault, change.getValueRemoved().mostRecentEvent(), change.getValueRemoved().count())); } } diff --git a/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java b/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java index 82eab3c24..d8e6f4242 100644 --- a/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java +++ b/src/main/java/org/cryptomator/ui/mainwindow/VaultListController.java @@ -112,7 +112,7 @@ public class VaultListController implements FxController { this.emptyVaultList = Bindings.isEmpty(vaults); this.vaultEventsMap = vaultEventsMap; this.newEventsPresent = new SimpleBooleanProperty(false); - vaultEventsMap.addListener((MapChangeListener) change -> { + vaultEventsMap.addListener((MapChangeListener) change -> { if (change.wasAdded()) { newEventsPresent.setValue(true); }