From 5ff0f4943e780313e733dba1479fd26df1e5193b Mon Sep 17 00:00:00 2001 From: Samuel Bussmann Date: Thu, 21 Sep 2023 14:11:15 +0200 Subject: [PATCH 1/8] Reapply: PR #3170 Fixes #3169 : Track event listeners keyed by type to allow earlier event firing veto #3170 --- .../core/events/EventListenerWrapper.java | 5 +- .../impl/events/CacheEventDispatcherImpl.java | 99 +++++++++++-------- .../impl/events/EventDispatchTask.java | 10 +- 3 files changed, 67 insertions(+), 47 deletions(-) diff --git a/ehcache-core/src/main/java/org/ehcache/core/events/EventListenerWrapper.java b/ehcache-core/src/main/java/org/ehcache/core/events/EventListenerWrapper.java index 57540d34ab..75ded70e87 100644 --- a/ehcache-core/src/main/java/org/ehcache/core/events/EventListenerWrapper.java +++ b/ehcache-core/src/main/java/org/ehcache/core/events/EventListenerWrapper.java @@ -24,6 +24,7 @@ import org.ehcache.event.EventType; import java.util.EnumSet; +import java.util.Set; /** * Internal wrapper for {@link CacheEventListener} and their configuration. @@ -87,8 +88,8 @@ public void onEvent(CacheEvent event) { return listener; } - public boolean isForEventType(EventType type) { - return forEvents.contains(type); + public Set getEventTypes() { + return forEvents; } public boolean isOrdered() { diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java index de7d891298..81979fe7a0 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java @@ -35,11 +35,21 @@ import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.Collection; +import java.util.EnumMap; import java.util.EnumSet; import java.util.List; +import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +import java.util.stream.Stream; + +import static java.util.Collections.unmodifiableMap; +import static java.util.EnumSet.allOf; +import static java.util.function.Function.identity; +import static java.util.stream.Collectors.toMap; +import static java.util.stream.Stream.concat; /** * Per-cache component that manages cache event listener registrations, and provides event delivery based on desired @@ -56,10 +66,12 @@ public class CacheEventDispatcherImpl implements CacheEventDispatcher> syncListenersList = new CopyOnWriteArrayList<>(); - private final List> aSyncListenersList = new CopyOnWriteArrayList<>(); + + private final Map>> syncListenersList = unmodifiableMap(allOf(EventType.class).stream() + .collect(toMap(identity(), t -> new CopyOnWriteArrayList<>(), (a, b) -> { throw new AssertionError(); }, () -> new EnumMap<>(EventType.class)))); + private final Map>> asyncListenersList = unmodifiableMap(allOf(EventType.class).stream() + .collect(toMap(identity(), t -> new CopyOnWriteArrayList<>(), (a, b) -> { throw new AssertionError(); }, () -> new EnumMap<>(EventType.class)))); + private final StoreEventListener eventListener = new StoreListener(); private volatile Cache listenerSource; @@ -95,69 +107,76 @@ public void registerCacheEventListener(CacheEventListener * @param wrapper the listener wrapper to register */ private synchronized void registerCacheEventListener(EventListenerWrapper wrapper) { - if(aSyncListenersList.contains(wrapper) || syncListenersList.contains(wrapper)) { + + if(allListeners().anyMatch(wrapper::equals)) { throw new IllegalStateException("Cache Event Listener already registered: " + wrapper.getListener()); } - if (wrapper.isOrdered() && orderedListenerCount++ == 0) { + boolean firstListener = !allListeners().findAny().isPresent(); + + if (wrapper.isOrdered() && (firstListener || allListeners().noneMatch(EventListenerWrapper::isOrdered))) { storeEventSource.setEventOrdering(true); } switch (wrapper.getFiringMode()) { case ASYNCHRONOUS: - aSyncListenersList.add(wrapper); + wrapper.getEventTypes().forEach(type -> asyncListenersList.get(type).add(wrapper)); break; case SYNCHRONOUS: - if (syncListenersList.isEmpty()) { + if (!syncListeners().findAny().isPresent()) { storeEventSource.setSynchronous(true); } - syncListenersList.add(wrapper); + wrapper.getEventTypes().forEach(type -> syncListenersList.get(type).add(wrapper)); break; default: throw new AssertionError("Unhandled EventFiring value: " + wrapper.getFiringMode()); } - if (listenersCount++ == 0) { + if (firstListener) { storeEventSource.addEventListener(eventListener); } } + private Stream> allListeners() { + return concat(asyncListeners(), syncListeners()); + } + + private Stream> syncListeners() { + return syncListenersList.values().stream().flatMap(Collection::stream); + } + + private Stream> asyncListeners() { + return asyncListenersList.values().stream().flatMap(Collection::stream); + } + /** * {@inheritDoc} */ @Override - public void deregisterCacheEventListener(CacheEventListener listener) { + public synchronized void deregisterCacheEventListener(CacheEventListener listener) { EventListenerWrapper wrapper = new EventListenerWrapper<>(listener); - if (!removeWrapperFromList(wrapper, aSyncListenersList)) { - if (!removeWrapperFromList(wrapper, syncListenersList)) { - throw new IllegalStateException("Unknown cache event listener: " + listener); - } + boolean removed = Stream.of(asyncListenersList, syncListenersList) + .flatMap(list -> list.values().stream()) + .map(list -> list.remove(wrapper)) + .reduce((a, b) -> a || b).orElse(false); + + if (!removed) { + throw new IllegalStateException("Unknown cache event listener: " + listener); } - } - /** - * Synchronized to make sure listener removal is atomic - * - * @param wrapper the listener wrapper to unregister - * @param listenersList the listener list to remove from - */ - private synchronized boolean removeWrapperFromList(EventListenerWrapper wrapper, List> listenersList) { - int index = listenersList.indexOf(wrapper); - if (index != -1) { - EventListenerWrapper containedWrapper = listenersList.remove(index); - if(containedWrapper.isOrdered() && --orderedListenerCount == 0) { + if (!allListeners().findAny().isPresent()) { + storeEventSource.setSynchronous(false); + storeEventSource.setEventOrdering(false); + storeEventSource.removeEventListener(eventListener); + } else { + if (allListeners().noneMatch(EventListenerWrapper::isOrdered)) { storeEventSource.setEventOrdering(false); } - if (--listenersCount == 0) { - storeEventSource.removeEventListener(eventListener); - } - if (syncListenersList.isEmpty()) { + if (!syncListeners().findAny().isPresent()) { storeEventSource.setSynchronous(false); } - return true; } - return false; } /** @@ -168,8 +187,8 @@ public synchronized void shutdown() { storeEventSource.removeEventListener(eventListener); storeEventSource.setEventOrdering(false); storeEventSource.setSynchronous(false); - syncListenersList.clear(); - aSyncListenersList.clear(); + syncListenersList.values().forEach(Collection::clear); + asyncListenersList.values().forEach(Collection::clear); unOrderedExectuor.shutdown(); orderedExecutor.shutdown(); } @@ -189,11 +208,13 @@ void onEvent(CacheEvent event) { } else { executor = unOrderedExectuor; } - if (!aSyncListenersList.isEmpty()) { - executor.submit(new EventDispatchTask<>(event, aSyncListenersList)); + List> asyncTargets = asyncListenersList.get(event.getType()); + if (!asyncTargets.isEmpty()) { + executor.submit(new EventDispatchTask<>(event, asyncTargets)); } - if (!syncListenersList.isEmpty()) { - Future future = executor.submit(new EventDispatchTask<>(event, syncListenersList)); + List> syncTargets = syncListenersList.get(event.getType()); + if (!syncTargets.isEmpty()) { + Future future = executor.submit(new EventDispatchTask<>(event, syncTargets)); try { future.get(); } catch (Exception e) { diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/events/EventDispatchTask.java b/ehcache-impl/src/main/java/org/ehcache/impl/events/EventDispatchTask.java index cb010960b1..8a296b0cbd 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/events/EventDispatchTask.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/events/EventDispatchTask.java @@ -41,12 +41,10 @@ class EventDispatchTask implements Runnable { @Override public void run() { for(EventListenerWrapper listenerWrapper : listenerWrappers) { - if (listenerWrapper.isForEventType(cacheEvent.getType())) { - try { - listenerWrapper.onEvent(cacheEvent); - } catch (Exception e) { - LOGGER.warn(listenerWrapper.getListener() + " Failed to fire Event due to ", e); - } + try { + listenerWrapper.onEvent(cacheEvent); + } catch (Exception e) { + LOGGER.warn(listenerWrapper.getListener() + " Failed to fire Event due to ", e); } } } From 157aa78d8a501ad3260fb2601be28b6018e7aeb2 Mon Sep 17 00:00:00 2001 From: Samuel Bussmann Date: Tue, 26 Sep 2023 16:22:43 +0200 Subject: [PATCH 2/8] Fixes #3169: Register only those CacheEvents with corresponding listener Signed-off-by: Samuel Bussmann --- .../client/internal/store/ClusteredStore.java | 6 +++++ .../java/org/ehcache/event/EventType.java | 10 ++++++- .../core/events/NullStoreEventDispatcher.java | 5 ++++ .../spi/store/events/StoreEventListener.java | 14 ++++++++++ .../spi/store/events/StoreEventSource.java | 5 ++++ .../impl/events/CacheEventDispatcherImpl.java | 22 +++++++++++++++- .../events/AbstractStoreEventDispatcher.java | 26 ++++++++++++++++++- .../FudgingInvocationScopedEventSink.java | 5 ++-- .../events/InvocationScopedEventSink.java | 26 +++++++++++-------- .../ThreadLocalStoreEventDispatcher.java | 2 +- .../impl/internal/store/basic/NopStore.java | 4 +++ .../FudgingInvocationScopedEventSinkTest.java | 9 ++++++- .../events/InvocationScopedEventSinkTest.java | 5 +++- .../events/TestStoreEventDispatcher.java | 5 ++++ .../DefaultStoreEventDispatcherTest.java | 1 + .../xa/internal/StoreEventSourceWrapper.java | 5 ++++ 16 files changed, 131 insertions(+), 19 deletions(-) diff --git a/clustered/ehcache-client/src/main/java/org/ehcache/clustered/client/internal/store/ClusteredStore.java b/clustered/ehcache-client/src/main/java/org/ehcache/clustered/client/internal/store/ClusteredStore.java index 93db8831ff..19b95353e1 100644 --- a/clustered/ehcache-client/src/main/java/org/ehcache/clustered/client/internal/store/ClusteredStore.java +++ b/clustered/ehcache-client/src/main/java/org/ehcache/clustered/client/internal/store/ClusteredStore.java @@ -1017,6 +1017,12 @@ public synchronized void removeEventListener(StoreEventListener eventListe } delegate.removeEventListener(eventListener); } + + @Override + public void listenerModified() { + delegate.listenerModified(); + } + @Override public void addEventFilter(StoreEventFilter eventFilter) { delegate.addEventFilter(eventFilter); diff --git a/ehcache-api/src/main/java/org/ehcache/event/EventType.java b/ehcache-api/src/main/java/org/ehcache/event/EventType.java index dd3815e299..542d34d55c 100644 --- a/ehcache-api/src/main/java/org/ehcache/event/EventType.java +++ b/ehcache-api/src/main/java/org/ehcache/event/EventType.java @@ -17,6 +17,9 @@ package org.ehcache.event; +import java.util.EnumSet; +import java.util.Set; + /** * The different event types. */ @@ -45,6 +48,11 @@ public enum EventType { /** * Represents an existing {@link org.ehcache.Cache.Entry cache entry} being updated for a given key */ - UPDATED, + UPDATED; + + private static final Set ALL_EVENT_TYPES = EnumSet.allOf(EventType.class); + public static Set allAsSet() { + return ALL_EVENT_TYPES; + } } diff --git a/ehcache-core/src/main/java/org/ehcache/core/events/NullStoreEventDispatcher.java b/ehcache-core/src/main/java/org/ehcache/core/events/NullStoreEventDispatcher.java index a11660271f..c0437156fa 100644 --- a/ehcache-core/src/main/java/org/ehcache/core/events/NullStoreEventDispatcher.java +++ b/ehcache-core/src/main/java/org/ehcache/core/events/NullStoreEventDispatcher.java @@ -88,6 +88,11 @@ public void removeEventListener(StoreEventListener eventListener) { // Do nothing } + @Override + public void listenerModified() { + // Do nothing + } + @Override public void addEventFilter(StoreEventFilter eventFilter) { // Do nothing diff --git a/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventListener.java b/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventListener.java index 79ed0ec607..d8d42c4273 100644 --- a/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventListener.java +++ b/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventListener.java @@ -17,7 +17,10 @@ package org.ehcache.core.spi.store.events; +import java.util.Set; + import org.ehcache.core.events.StoreEventDispatcher; +import org.ehcache.event.EventType; /** * Interface used to register on a {@link StoreEventSource} to get notified of events happening to mappings the @@ -37,4 +40,15 @@ public interface StoreEventListener { * @param event the actual {@link StoreEvent} */ void onEvent(StoreEvent event); + + /** + * Specify which Events this Listener is handling. + *

+ * Defaults return is all values of {@link EventType} + * + * @return Set of the {@link EventType} this listener handles. + */ + default Set getEventTypes() { + return EventType.allAsSet(); + } } diff --git a/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventSource.java b/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventSource.java index 007ee4d949..372f309b65 100644 --- a/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventSource.java +++ b/ehcache-core/src/main/java/org/ehcache/core/spi/store/events/StoreEventSource.java @@ -59,4 +59,9 @@ public interface StoreEventSource { * @return {@code true} if ordering is on, {@code false} otherwise */ boolean isEventOrdering(); + + /** + * Indicates that a listener was modified + */ + void listenerModified(); } diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java index 81979fe7a0..b72a6aa173 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java @@ -40,6 +40,7 @@ import java.util.EnumSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @@ -71,6 +72,7 @@ public class CacheEventDispatcherImpl implements CacheEventDispatcher new CopyOnWriteArrayList<>(), (a, b) -> { throw new AssertionError(); }, () -> new EnumMap<>(EventType.class)))); private final Map>> asyncListenersList = unmodifiableMap(allOf(EventType.class).stream() .collect(toMap(identity(), t -> new CopyOnWriteArrayList<>(), (a, b) -> { throw new AssertionError(); }, () -> new EnumMap<>(EventType.class)))); + private final Set registeredEventTypes = EnumSet.noneOf(EventType.class); private final StoreEventListener eventListener = new StoreListener(); @@ -118,6 +120,8 @@ private synchronized void registerCacheEventListener(EventListenerWrapper storeEventSource.setEventOrdering(true); } + registeredEventTypes.addAll(wrapper.getEventTypes()); // add EventType of new wrapper to list or relevant EntryTypes + switch (wrapper.getFiringMode()) { case ASYNCHRONOUS: wrapper.getEventTypes().forEach(type -> asyncListenersList.get(type).add(wrapper)); @@ -134,6 +138,8 @@ private synchronized void registerCacheEventListener(EventListenerWrapper if (firstListener) { storeEventSource.addEventListener(eventListener); + } else { + storeEventSource.listenerModified(); } } @@ -165,6 +171,8 @@ public synchronized void deregisterCacheEventListener(CacheEventListener newRegisteredEventTypes = EnumSet.noneOf(EventType.class); + allListeners().forEach(listener -> newRegisteredEventTypes.addAll(listener.getEventTypes())); + // drop irrelevant EventTypes + registeredEventTypes.retainAll(newRegisteredEventTypes); + } + /** * {@inheritDoc} */ @@ -265,8 +281,12 @@ public void onEvent(StoreEvent event) { throw new AssertionError("Unexpected StoreEvent value: " + event.getType()); } } - } + @Override + public Set getEventTypes() { + return registeredEventTypes; + } + } /** * {@inheritDoc} */ diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/AbstractStoreEventDispatcher.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/AbstractStoreEventDispatcher.java index e4a770b8eb..b57442297b 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/AbstractStoreEventDispatcher.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/AbstractStoreEventDispatcher.java @@ -21,7 +21,9 @@ import org.ehcache.core.events.StoreEventSink; import org.ehcache.core.spi.store.events.StoreEventFilter; import org.ehcache.core.spi.store.events.StoreEventListener; +import org.ehcache.event.EventType; +import java.util.EnumSet; import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CopyOnWriteArraySet; @@ -78,6 +80,7 @@ public void evicted(Object key, Supplier value) { private final Set> filters = new CopyOnWriteArraySet<>(); private final Set> listeners = new CopyOnWriteArraySet<>(); private final BlockingQueue>[] orderedQueues; + private final Set relevantEventTypes = EnumSet.noneOf(EventType.class); private volatile boolean ordered = false; protected AbstractStoreEventDispatcher(int dispatcherConcurrency) { @@ -93,6 +96,16 @@ protected AbstractStoreEventDispatcher(int dispatcherConcurrency) { } } + private void computeRelevantEventTypes() { + // collect all EventTypes the listeners are interested in. + for (StoreEventListener listener : listeners) { + relevantEventTypes.addAll(listener.getEventTypes()); + } + if (relevantEventTypes.isEmpty()) { // mocks are empty -> handle all types + relevantEventTypes.addAll(EnumSet.allOf(EventType.class)); + } + } + protected Set> getListeners() { return listeners; } @@ -105,14 +118,25 @@ protected BlockingQueue>[] getOrderedQueues() { return orderedQueues; } + protected Set getRelevantEventTypes() { + return relevantEventTypes; + } + @Override public void addEventListener(StoreEventListener eventListener) { listeners.add(eventListener); + computeRelevantEventTypes(); // refresh } @Override public void removeEventListener(StoreEventListener eventListener) { listeners.remove(eventListener); + computeRelevantEventTypes(); // refresh + } + + @Override + public void listenerModified() { + computeRelevantEventTypes(); // refresh } @Override @@ -152,6 +176,6 @@ public void reset(StoreEventSink eventSink) { @Override public StoreEventSink eventSink() { - return new InvocationScopedEventSink<>(getFilters(), isEventOrdering(), getOrderedQueues(), getListeners()); + return new InvocationScopedEventSink<>(getFilters(), isEventOrdering(), getOrderedQueues(), getListeners(), getRelevantEventTypes()); } } diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSink.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSink.java index 920d56312e..b03d60d142 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSink.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSink.java @@ -41,8 +41,9 @@ class FudgingInvocationScopedEventSink extends InvocationScopedEventSink> filters, boolean ordered, BlockingQueue>[] orderedQueues, - Set> listeners) { - super(filters, ordered, orderedQueues, listeners); + Set> listeners, + Set relevantEventTypes) { + super(filters, ordered, orderedQueues, listeners, relevantEventTypes); } @Override diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java index 6b8b467e24..c3c4e5a415 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java @@ -17,9 +17,11 @@ package org.ehcache.impl.internal.events; -import org.ehcache.event.EventType; -import org.ehcache.core.spi.store.events.StoreEventFilter; -import org.ehcache.core.spi.store.events.StoreEventListener; +import static org.ehcache.impl.internal.events.StoreEvents.createEvent; +import static org.ehcache.impl.internal.events.StoreEvents.evictEvent; +import static org.ehcache.impl.internal.events.StoreEvents.expireEvent; +import static org.ehcache.impl.internal.events.StoreEvents.removeEvent; +import static org.ehcache.impl.internal.events.StoreEvents.updateEvent; import java.util.ArrayDeque; import java.util.Deque; @@ -28,11 +30,9 @@ import java.util.concurrent.BlockingQueue; import java.util.function.Supplier; -import static org.ehcache.impl.internal.events.StoreEvents.createEvent; -import static org.ehcache.impl.internal.events.StoreEvents.evictEvent; -import static org.ehcache.impl.internal.events.StoreEvents.expireEvent; -import static org.ehcache.impl.internal.events.StoreEvents.removeEvent; -import static org.ehcache.impl.internal.events.StoreEvents.updateEvent; +import org.ehcache.core.spi.store.events.StoreEventFilter; +import org.ehcache.core.spi.store.events.StoreEventListener; +import org.ehcache.event.EventType; /** * InvocationScopedEventSink @@ -45,13 +45,16 @@ class InvocationScopedEventSink implements CloseableStoreEventSink { private final Set> listeners; private final Deque> events = new ArrayDeque<>(4); + private final Set relevantEventTypes; + InvocationScopedEventSink(Set> filters, boolean ordered, - BlockingQueue>[] orderedQueues, - Set> listeners) { + BlockingQueue>[] orderedQueues, Set> listeners, + Set relevantEventTypes) { this.filters = filters; this.ordered = ordered; this.orderedQueues = orderedQueues; this.listeners = listeners; + this.relevantEventTypes = relevantEventTypes; } @Override @@ -99,7 +102,8 @@ protected boolean acceptEvent(EventType type, K key, V oldValue, V newValue) { return false; } } - return true; + // at least one listener is interested in this event + return relevantEventTypes.contains(type); } @Override diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/ThreadLocalStoreEventDispatcher.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/ThreadLocalStoreEventDispatcher.java index 65be56f847..9104714db7 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/ThreadLocalStoreEventDispatcher.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/ThreadLocalStoreEventDispatcher.java @@ -40,7 +40,7 @@ public StoreEventSink eventSink() { } else { StoreEventSink eventSink = tlEventSink.get(); if (eventSink == null) { - eventSink = new FudgingInvocationScopedEventSink<>(getFilters(), isEventOrdering(), getOrderedQueues(), getListeners()); + eventSink = new FudgingInvocationScopedEventSink<>(getFilters(), isEventOrdering(), getOrderedQueues(), getListeners(), getRelevantEventTypes()); tlEventSink.set(eventSink); usageDepth.set(0); } else { diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/basic/NopStore.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/basic/NopStore.java index 6144448fd6..345574ef4e 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/basic/NopStore.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/basic/NopStore.java @@ -145,6 +145,10 @@ public void setSynchronous(boolean synchronous) throws IllegalArgumentException public boolean isEventOrdering() { return false; } + + @Override + public void listenerModified() { + } }; } diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java index 2700690bbc..16a2a7524a 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java @@ -25,6 +25,7 @@ import org.junit.Test; import org.mockito.InOrder; +import java.util.EnumSet; import java.util.HashSet; import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; @@ -54,7 +55,7 @@ public void setUp() { storeEventListeners.add(listener); @SuppressWarnings({"unchecked", "rawtypes"}) BlockingQueue>[] blockingQueues = new BlockingQueue[] { new ArrayBlockingQueue>(10) }; - eventSink = new FudgingInvocationScopedEventSink<>(new HashSet<>(), false, blockingQueues, storeEventListeners); + eventSink = new FudgingInvocationScopedEventSink<>(new HashSet<>(), false, blockingQueues, storeEventListeners, EnumSet.allOf(EventType.class)); } @Test @@ -64,6 +65,7 @@ public void testEvictedDifferentKeyNoImpact() { eventSink.close(); InOrder inOrder = inOrder(listener); + inOrder.verify(listener, times(2)).getEventTypes(); inOrder.verify(listener).onEvent(argThat(createdMatcher)); inOrder.verify(listener).onEvent(argThat(evictedMatcher)); verifyNoMoreInteractions(listener); @@ -76,6 +78,7 @@ public void testEvictedSameKeyAfterUpdateReplacesWithEvictCreate() { eventSink.close(); InOrder inOrder = inOrder(listener); + inOrder.verify(listener, times(3)).getEventTypes(); inOrder.verify(listener).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -89,6 +92,7 @@ public void testEvictedSameKeyAfterCreateFudgesExpiryToo() { eventSink.close(); InOrder inOrder = inOrder(listener); + inOrder.verify(listener, times(4)).getEventTypes(); inOrder.verify(listener).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -103,6 +107,7 @@ public void testEvictedSameKeyAfterUpdateReplacesWithEvictCreateEvenWithMultiple eventSink.close(); InOrder inOrder = inOrder(listener); + inOrder.verify(listener, times(5)).getEventTypes(); inOrder.verify(listener, times(3)).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -118,6 +123,7 @@ public void testEvictedSameKeyAfterCreateFudgesExpiryTooEvenWithMultipleEvictsIn eventSink.close(); InOrder inOrder = inOrder(listener); + inOrder.verify(listener, times(6)).getEventTypes(); inOrder.verify(listener, times(3)).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -131,6 +137,7 @@ public void testEvictedKeyDoesNotFudgeOlderEvents() { eventSink.close(); InOrder inOrder = inOrder(listener); + inOrder.verify(listener, times(3)).getEventTypes(); Matcher> updatedMatcher = eventType(EventType.UPDATED); inOrder.verify(listener).onEvent(argThat(updatedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java index ef19e87e51..4535d4f5ee 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java @@ -31,6 +31,7 @@ import org.mockito.junit.MockitoRule; import java.util.Collections; +import java.util.EnumSet; import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; @@ -40,6 +41,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.hamcrest.MockitoHamcrest.argThat; @@ -67,7 +69,7 @@ public void setUp() { private InvocationScopedEventSink createEventSink(boolean ordered) { @SuppressWarnings("unchecked") BlockingQueue>[] queues = (BlockingQueue>[]) new BlockingQueue[] { blockingQueue }; - return new InvocationScopedEventSink<>(Collections.emptySet(), ordered, queues, storeEventListeners); + return new InvocationScopedEventSink<>(Collections.emptySet(), ordered, queues, storeEventListeners, EnumSet.allOf(EventType.class)); } @Test @@ -83,6 +85,7 @@ public void testReset() { eventSink.close(); InOrder inOrder = inOrder(listener); + inOrder.verify(listener, times(5)).getEventTypes(); Matcher> createdMatcher = eventType(EventType.CREATED); inOrder.verify(listener).onEvent(argThat(createdMatcher)); Matcher> updatedMatcher = eventType(EventType.UPDATED); diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/TestStoreEventDispatcher.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/TestStoreEventDispatcher.java index a7321a6b44..f661168131 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/TestStoreEventDispatcher.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/TestStoreEventDispatcher.java @@ -76,6 +76,11 @@ public void removeEventListener(StoreEventListener eventListener) { listeners.remove(eventListener); } + @Override + public void listenerModified() { + // No-op + } + @Override public void addEventFilter(StoreEventFilter eventFilter) { filters.add(eventFilter); diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java index 8f7d8f2c6f..59203c10f5 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java @@ -108,6 +108,7 @@ public void testEventFiltering() { sink.created("new", "and shiny"); dispatcher.releaseEventSink(sink); + verify(listener).getEventTypes(); Matcher> matcher = eventOfType(EventType.CREATED); verify(listener).onEvent(argThat(matcher)); verifyNoMoreInteractions(listener); diff --git a/ehcache-transactions/src/common/java/org/ehcache/transactions/xa/internal/StoreEventSourceWrapper.java b/ehcache-transactions/src/common/java/org/ehcache/transactions/xa/internal/StoreEventSourceWrapper.java index 2b7b08344a..ee4705af97 100644 --- a/ehcache-transactions/src/common/java/org/ehcache/transactions/xa/internal/StoreEventSourceWrapper.java +++ b/ehcache-transactions/src/common/java/org/ehcache/transactions/xa/internal/StoreEventSourceWrapper.java @@ -61,6 +61,11 @@ public void removeEventListener(StoreEventListener eventListener) { } } + @Override + public void listenerModified() { + underlying.listenerModified(); + } + @Override public void addEventFilter(final StoreEventFilter eventFilter) { underlying.addEventFilter((type, key, oldValue, newValue) -> { From 11842b4cb2e3beae1041c95b547976b96a88405e Mon Sep 17 00:00:00 2001 From: Samuel Bussmann Date: Tue, 26 Sep 2023 16:23:33 +0200 Subject: [PATCH 3/8] Fixes #3169: Inline unordered/asynch listener execution to avoid submit delay Signed-off-by: Samuel Bussmann --- .../impl/events/CacheEventDispatcherImpl.java | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java index b72a6aa173..205a9bba22 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java @@ -218,23 +218,30 @@ public synchronized void setListenerSource(Cache source) { } void onEvent(CacheEvent event) { - ExecutorService executor; - if (storeEventSource.isEventOrdering()) { - executor = orderedExecutor; - } else { - executor = unOrderedExectuor; - } List> asyncTargets = asyncListenersList.get(event.getType()); - if (!asyncTargets.isEmpty()) { - executor.submit(new EventDispatchTask<>(event, asyncTargets)); - } List> syncTargets = syncListenersList.get(event.getType()); - if (!syncTargets.isEmpty()) { - Future future = executor.submit(new EventDispatchTask<>(event, syncTargets)); - try { - future.get(); - } catch (Exception e) { - LOGGER.error("Exception received as result from synchronous listeners", e); + if (storeEventSource.isEventOrdering()) { + if (!asyncTargets.isEmpty()) { + orderedExecutor.submit(new EventDispatchTask<>(event, asyncTargets)); + } + if (!syncTargets.isEmpty()) { + Future future = orderedExecutor.submit(new EventDispatchTask<>(event, syncTargets)); + try { + future.get(); + } catch (Exception e) { + LOGGER.error("Exception received as result from synchronous listeners", e); + } + } + } else { + if (!asyncTargets.isEmpty()) { + unOrderedExectuor.submit(new EventDispatchTask<>(event, asyncTargets)); + } + if (!syncTargets.isEmpty()) { + try { + new EventDispatchTask<>(event, syncTargets).run(); + } catch (Exception e) { + LOGGER.error("Exception received as result from synchronous listeners", e); + } } } } From 717cd4332bf22632c3eaa52e92a462bdf580c280 Mon Sep 17 00:00:00 2001 From: Samuel Bussmann Date: Wed, 17 Jul 2024 20:40:49 +0200 Subject: [PATCH 4/8] Fixes #3169: clean-ups & test fixes Signed-off-by: Samuel Bussmann --- .../impl/events/CacheEventDispatcherImpl.java | 2 +- .../FudgingInvocationScopedEventSinkTest.java | 40 +++++++-------- .../events/InvocationScopedEventSinkTest.java | 50 +++++++++---------- 3 files changed, 43 insertions(+), 49 deletions(-) diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java index 205a9bba22..3555e26a36 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java @@ -120,7 +120,7 @@ private synchronized void registerCacheEventListener(EventListenerWrapper storeEventSource.setEventOrdering(true); } - registeredEventTypes.addAll(wrapper.getEventTypes()); // add EventType of new wrapper to list or relevant EntryTypes + registeredEventTypes.addAll(wrapper.getEventTypes()); // add EventType of new wrapper to list of relevant EntryTypes switch (wrapper.getFiringMode()) { case ASYNCHRONOUS: diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java index 16a2a7524a..4a6001bdf4 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java @@ -17,13 +17,12 @@ package org.ehcache.impl.internal.events; -import org.ehcache.core.spi.store.events.StoreEvent; -import org.ehcache.event.EventType; -import org.ehcache.core.spi.store.events.StoreEventListener; -import org.hamcrest.Matcher; -import org.junit.Before; -import org.junit.Test; -import org.mockito.InOrder; +import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; +import static org.ehcache.test.MockitoUtil.uncheckedGenericMock; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; import java.util.EnumSet; import java.util.HashSet; @@ -31,12 +30,13 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; -import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; -import static org.ehcache.test.MockitoUtil.uncheckedGenericMock; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.hamcrest.MockitoHamcrest.argThat; +import org.ehcache.core.spi.store.events.StoreEvent; +import org.ehcache.core.spi.store.events.StoreEventListener; +import org.ehcache.event.EventType; +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InOrder; /** * FudgingInvocationScopedEventSinkTest @@ -53,9 +53,11 @@ public void setUp() { Set> storeEventListeners = new HashSet<>(); listener = uncheckedGenericMock(StoreEventListener.class); storeEventListeners.add(listener); - @SuppressWarnings({"unchecked", "rawtypes"}) - BlockingQueue>[] blockingQueues = new BlockingQueue[] { new ArrayBlockingQueue>(10) }; - eventSink = new FudgingInvocationScopedEventSink<>(new HashSet<>(), false, blockingQueues, storeEventListeners, EnumSet.allOf(EventType.class)); + @SuppressWarnings({ "unchecked", "rawtypes" }) + BlockingQueue>[] blockingQueues = new BlockingQueue[] { + new ArrayBlockingQueue>(10) }; + eventSink = new FudgingInvocationScopedEventSink<>(new HashSet<>(), false, blockingQueues, storeEventListeners, + EnumSet.allOf(EventType.class)); } @Test @@ -65,7 +67,6 @@ public void testEvictedDifferentKeyNoImpact() { eventSink.close(); InOrder inOrder = inOrder(listener); - inOrder.verify(listener, times(2)).getEventTypes(); inOrder.verify(listener).onEvent(argThat(createdMatcher)); inOrder.verify(listener).onEvent(argThat(evictedMatcher)); verifyNoMoreInteractions(listener); @@ -78,7 +79,6 @@ public void testEvictedSameKeyAfterUpdateReplacesWithEvictCreate() { eventSink.close(); InOrder inOrder = inOrder(listener); - inOrder.verify(listener, times(3)).getEventTypes(); inOrder.verify(listener).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -92,7 +92,6 @@ public void testEvictedSameKeyAfterCreateFudgesExpiryToo() { eventSink.close(); InOrder inOrder = inOrder(listener); - inOrder.verify(listener, times(4)).getEventTypes(); inOrder.verify(listener).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -107,7 +106,6 @@ public void testEvictedSameKeyAfterUpdateReplacesWithEvictCreateEvenWithMultiple eventSink.close(); InOrder inOrder = inOrder(listener); - inOrder.verify(listener, times(5)).getEventTypes(); inOrder.verify(listener, times(3)).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -123,7 +121,6 @@ public void testEvictedSameKeyAfterCreateFudgesExpiryTooEvenWithMultipleEvictsIn eventSink.close(); InOrder inOrder = inOrder(listener); - inOrder.verify(listener, times(6)).getEventTypes(); inOrder.verify(listener, times(3)).onEvent(argThat(evictedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); verifyNoMoreInteractions(listener); @@ -137,7 +134,6 @@ public void testEvictedKeyDoesNotFudgeOlderEvents() { eventSink.close(); InOrder inOrder = inOrder(listener); - inOrder.verify(listener, times(3)).getEventTypes(); Matcher> updatedMatcher = eventType(EventType.UPDATED); inOrder.verify(listener).onEvent(argThat(updatedMatcher)); inOrder.verify(listener).onEvent(argThat(createdMatcher)); diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java index 4535d4f5ee..e6c95c573a 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java @@ -17,8 +17,21 @@ package org.ehcache.impl.internal.events; +import static org.assertj.core.api.Assertions.assertThat; +import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; + +import java.util.Collections; +import java.util.EnumSet; +import java.util.Set; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.IntStream; + import org.ehcache.core.spi.store.events.StoreEvent; -import org.ehcache.core.spi.store.events.StoreEventFilter; import org.ehcache.core.spi.store.events.StoreEventListener; import org.ehcache.event.EventType; import org.hamcrest.Matcher; @@ -30,21 +43,6 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; -import java.util.Collections; -import java.util.EnumSet; -import java.util.Set; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.IntStream; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.hamcrest.MockitoHamcrest.argThat; - /** * InvocationScopedEventSinkTest */ @@ -68,7 +66,8 @@ public void setUp() { private InvocationScopedEventSink createEventSink(boolean ordered) { @SuppressWarnings("unchecked") - BlockingQueue>[] queues = (BlockingQueue>[]) new BlockingQueue[] { blockingQueue }; + BlockingQueue>[] queues = (BlockingQueue>[])new BlockingQueue[] { + blockingQueue }; return new InvocationScopedEventSink<>(Collections.emptySet(), ordered, queues, storeEventListeners, EnumSet.allOf(EventType.class)); } @@ -85,7 +84,6 @@ public void testReset() { eventSink.close(); InOrder inOrder = inOrder(listener); - inOrder.verify(listener, times(5)).getEventTypes(); Matcher> createdMatcher = eventType(EventType.CREATED); inOrder.verify(listener).onEvent(argThat(createdMatcher)); Matcher> updatedMatcher = eventType(EventType.UPDATED); @@ -96,8 +94,8 @@ public void testReset() { } /** - * Make sure an interrupted sink sets the interrupted flag and keep both event queues in the state - * as of before the event that was interrupted. + * Make sure an interrupted sink sets the interrupted flag and keep both event queues in the state as of before the event that was + * interrupted. * * @throws InterruptedException */ @@ -117,7 +115,7 @@ public void testInterruption() throws InterruptedException { }); t.start(); - while(blockingQueue.remainingCapacity() != 0) { + while (blockingQueue.remainingCapacity() != 0) { System.out.println(blockingQueue.remainingCapacity()); } @@ -127,11 +125,11 @@ public void testInterruption() throws InterruptedException { assertThat(wasInterrupted).isTrue(); assertThat(blockingQueue).hasSize(10); IntStream.range(0, 10).forEachOrdered(i -> { - try { - assertThat(blockingQueue.take().getEvent().getKey()).isEqualTo("k" + i); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } + try { + assertThat(blockingQueue.take().getEvent().getKey()).isEqualTo("k" + i); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } }); assertThat(eventSink.getEvents()).hasSize(10); assertThat(eventSink.getEvents().getLast().getEvent().getKey()).isEqualTo("k9"); From a7b74648595634f02707fa133bb22b4862825f1c Mon Sep 17 00:00:00 2001 From: Samuel Bussmann Date: Thu, 18 Jul 2024 10:14:26 +0200 Subject: [PATCH 5/8] Fixes #3169: fix compile error after rebase Signed-off-by: Samuel Bussmann --- .../store/shared/store/StorePartition.java | 114 ++++++++++-------- 1 file changed, 67 insertions(+), 47 deletions(-) diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java index d286b4ae80..2be71fb44b 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java @@ -17,6 +17,22 @@ package org.ehcache.impl.internal.store.shared.store; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import javax.annotation.Nonnull; + import org.ehcache.Cache; import org.ehcache.config.ResourceType; import org.ehcache.core.CacheConfigurationChangeListener; @@ -34,21 +50,6 @@ import org.ehcache.spi.resilience.StoreAccessException; import org.slf4j.Logger; -import javax.annotation.Nonnull; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.BiFunction; -import java.util.function.Consumer; -import java.util.function.Function; -import java.util.function.Supplier; -import java.util.stream.Collectors; - public class StorePartition extends AbstractPartition, CompositeValue>> implements Store { private final Logger logger = EhcachePrefixLoggerFactory.getLogger(StorePartition.class); @@ -58,14 +59,14 @@ public class StorePartition extends AbstractPartition valueType; public StorePartition(ResourceType type, int id, Class keyType, Class valueType, - Store, CompositeValue> store) { + Store, CompositeValue> store) { super(type, id, store); this.keyType = keyType; this.valueType = valueType; } protected K checkKey(K keyObject) { - if (keyType.isInstance(Objects.requireNonNull((Object) keyObject))) { + if (keyType.isInstance(Objects.requireNonNull((Object)keyObject))) { return keyObject; } else { throw new ClassCastException("Invalid key type, expected : " + keyType.getName() + " but was : " + keyObject.getClass().getName()); @@ -73,10 +74,11 @@ protected K checkKey(K keyObject) { } protected V checkValue(V valueObject) { - if (valueType.isInstance(Objects.requireNonNull((Object) valueObject))) { + if (valueType.isInstance(Objects.requireNonNull((Object)valueObject))) { return valueObject; } else { - throw new ClassCastException("Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); + throw new ClassCastException( + "Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); } } @@ -150,13 +152,16 @@ public ReplaceStatus replace(K key, V oldValue, V newValue) throws StoreAccessEx @Override public ValueHolder getAndCompute(K key, BiFunction mappingFunction) throws StoreAccessException { checkKey(key); - return decode(shared().getAndCompute(composite(key), (k, v) -> composite(mappingFunction.apply(k.getValue(), v != null ? v.getValue() : null)))); + return decode( + shared().getAndCompute(composite(key), (k, v) -> composite(mappingFunction.apply(k.getValue(), v != null ? v.getValue() : null)))); } @Override - public ValueHolder computeAndGet(K key, BiFunction mappingFunction, Supplier replaceEqual, Supplier invokeWriter) throws StoreAccessException { + public ValueHolder computeAndGet(K key, BiFunction mappingFunction, Supplier replaceEqual, + Supplier invokeWriter) throws StoreAccessException { checkKey(key); - return decode(shared().computeAndGet(composite(key), (k, v) -> composite(mappingFunction.apply(k.getValue(), v == null ? null : v.getValue())), replaceEqual, invokeWriter)); + return decode(shared().computeAndGet(composite(key), + (k, v) -> composite(mappingFunction.apply(k.getValue(), v == null ? null : v.getValue())), replaceEqual, invokeWriter)); } @Override @@ -167,30 +172,31 @@ public ValueHolder computeIfAbsent(K key, Function ma @Override public Map> bulkCompute(Set keys, - Function>, Iterable>> remappingFunction) throws StoreAccessException { + Function>, Iterable>> remappingFunction) + throws StoreAccessException { return bulkCompute(keys, remappingFunction, SUPPLY_TRUE); } @Override public Map> bulkCompute(Set keys, - Function>, Iterable>> remappingFunction, - Supplier replaceEqual) throws StoreAccessException { + Function>, Iterable>> remappingFunction, + Supplier replaceEqual) throws StoreAccessException { Map, ValueHolder>> results; if (remappingFunction instanceof Ehcache.PutAllFunction) { - Ehcache.PutAllFunction putAllFunction = (Ehcache.PutAllFunction) remappingFunction; + Ehcache.PutAllFunction putAllFunction = (Ehcache.PutAllFunction)remappingFunction; Ehcache.PutAllFunction, CompositeValue> compositePutAllFunction = new Ehcache.PutAllFunction, CompositeValue>() { @Override public Map, CompositeValue> getEntriesToRemap() { - return putAllFunction.getEntriesToRemap().entrySet().stream().collect(Collectors.toMap(e -> composite(e.getKey()), e -> composite(e.getValue()))); + return putAllFunction.getEntriesToRemap().entrySet().stream() + .collect(Collectors.toMap(e -> composite(e.getKey()), e -> composite(e.getValue()))); } @Override public boolean newValueAlreadyExpired(CompositeValue key, CompositeValue oldValue, CompositeValue newValue) { - return putAllFunction.newValueAlreadyExpired(key.getValue(), - oldValue == null ? null : oldValue.getValue(), - newValue == null ? null : newValue.getValue()); + return putAllFunction.newValueAlreadyExpired(key.getValue(), oldValue == null ? null : oldValue.getValue(), + newValue == null ? null : newValue.getValue()); } @Override @@ -206,7 +212,7 @@ public AtomicInteger getActualUpdateCount() { results = shared().bulkCompute(compositeSet(keys), compositePutAllFunction); } else if (remappingFunction instanceof Ehcache.RemoveAllFunction) { - Ehcache.RemoveAllFunction removeAllFunction = (Ehcache.RemoveAllFunction) remappingFunction; + Ehcache.RemoveAllFunction removeAllFunction = (Ehcache.RemoveAllFunction)remappingFunction; Ehcache.RemoveAllFunction, CompositeValue> compositeRemappingFunction = removeAllFunction::getActualRemoveCount; results = shared().bulkCompute(compositeSet(keys), compositeRemappingFunction); } else { @@ -220,9 +226,10 @@ public AtomicInteger getActualUpdateCount() { @Override public Map> bulkComputeIfAbsent(Set keys, - Function, Iterable>> mappingFunction) throws StoreAccessException { - Map, ValueHolder>> results = - shared().bulkComputeIfAbsent(compositeSet(keys), new BulkComputeIfAbsentMappingFunction<>(id(), keyType, valueType, mappingFunction)); + Function, Iterable>> mappingFunction) + throws StoreAccessException { + Map, ValueHolder>> results = shared().bulkComputeIfAbsent(compositeSet(keys), + new BulkComputeIfAbsentMappingFunction<>(id(), keyType, valueType, mappingFunction)); Map> decodedResults = new HashMap<>(); results.forEach((k, v) -> decodedResults.put(k.getValue(), decode(v))); return decodedResults; @@ -255,8 +262,8 @@ public void addEventListener(StoreEventListener eventListener) { public void onEvent(StoreEvent, CompositeValue> event) { if (event.getKey().getStoreId() == id()) { eventListener.onEvent(new StoreEventImpl<>(event.getType(), event.getKey().getValue(), - event.getOldValue() == null ? null : event.getOldValue().getValue(), - event.getNewValue() == null ? null : event.getNewValue().getValue())); + event.getOldValue() == null ? null : event.getOldValue().getValue(), + event.getNewValue() == null ? null : event.getNewValue().getValue())); } } @@ -275,7 +282,7 @@ public boolean equals(Object obj) { @SuppressWarnings("unchecked") @Override public void removeEventListener(StoreEventListener eventListener) { - storeEventSource.removeEventListener((StoreEventListener, CompositeValue>) eventListener); + storeEventSource.removeEventListener((StoreEventListener, CompositeValue>)eventListener); } @Override @@ -283,7 +290,7 @@ public void addEventFilter(StoreEventFilter eventFilter) { storeEventSource.addEventFilter((type, key, oldValue, newValue) -> { if (key.getStoreId() == id()) { return eventFilter.acceptEvent(type, key.getValue(), oldValue == null ? null : oldValue.getValue(), - newValue == null ? null : newValue.getValue()); + newValue == null ? null : newValue.getValue()); } else { return true; } @@ -304,6 +311,11 @@ public void setSynchronous(boolean synchronous) throws IllegalArgumentException public boolean isEventOrdering() { return storeEventSource.isEventOrdering(); } + + @Override + public void listenerModified() { + storeEventSource.listenerModified(); + } }; } @@ -313,6 +325,7 @@ public Iterator>> iterator() { Iterator, ValueHolder>>> iterator = shared().iterator(); return new Iterator>>() { private Cache.Entry> prefetched = advance(); + @Override public boolean hasNext() { return prefetched != null; @@ -339,6 +352,7 @@ private Cache.Entry> advance() { public K getKey() { return next.getKey().getValue(); } + @Override public ValueHolder getValue() { return decode(next.getValue()); @@ -377,7 +391,7 @@ public T get() { } } - public static abstract class BaseRemappingFunction { + public static abstract class BaseRemappingFunction { protected final int storeId; protected final Class keyType; protected final Class valueType; @@ -396,21 +410,25 @@ protected void keyCheck(Object keyObject) { protected void valueCheck(Object valueObject) { if (!valueType.isInstance(Objects.requireNonNull(valueObject))) { - throw new ClassCastException("Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); + throw new ClassCastException( + "Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); } } } - public static class BulkComputeMappingFunction extends BaseRemappingFunction implements Function, ? extends CompositeValue>>, Iterable, ? extends CompositeValue>>> { + public static class BulkComputeMappingFunction extends BaseRemappingFunction implements + Function, ? extends CompositeValue>>, Iterable, ? extends CompositeValue>>> { private final Function>, Iterable>> function; - BulkComputeMappingFunction(int storeId, Class keyType, Class valueType, Function>, Iterable>> function) { + BulkComputeMappingFunction(int storeId, Class keyType, Class valueType, + Function>, Iterable>> function) { super(storeId, keyType, valueType); this.function = function; } @Override - public Iterable, ? extends CompositeValue>> apply(Iterable, ? extends CompositeValue>> entries) { + public Iterable, ? extends CompositeValue>> apply( + Iterable, ? extends CompositeValue>> entries) { Map decodedEntries = new HashMap<>(); entries.forEach(entry -> { K key = entry.getKey().getValue(); @@ -434,16 +452,19 @@ public static class BulkComputeMappingFunction extends BaseRemappingFuncti } } - public static class BulkComputeIfAbsentMappingFunction extends BaseRemappingFunction implements Function>, Iterable, ? extends CompositeValue>>> { + public static class BulkComputeIfAbsentMappingFunction extends BaseRemappingFunction implements + Function>, Iterable, ? extends CompositeValue>>> { private final Function, Iterable>> function; - BulkComputeIfAbsentMappingFunction(int storeId, Class keyType, Class valueType, Function, Iterable>> function) { + BulkComputeIfAbsentMappingFunction(int storeId, Class keyType, Class valueType, + Function, Iterable>> function) { super(storeId, keyType, valueType); this.function = function; } @Override - public Iterable, ? extends CompositeValue>> apply(Iterable> compositeValues) { + public Iterable, ? extends CompositeValue>> apply( + Iterable> compositeValues) { List keys = new ArrayList<>(); compositeValues.forEach(k -> { keyCheck(k.getValue()); @@ -465,4 +486,3 @@ public static class BulkComputeIfAbsentMappingFunction extends BaseRemappi } } } - From 5687149ae9429880e11e4cd26cb9b920c2a83cf3 Mon Sep 17 00:00:00 2001 From: Samuel Bussmann Date: Thu, 25 Jul 2024 13:53:11 +0200 Subject: [PATCH 6/8] Fixes #3169: signal listenerModified on listener removal Signed-off-by: Samuel Bussmann --- .../java/org/ehcache/impl/events/CacheEventDispatcherImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java index 3555e26a36..2f87320797 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/events/CacheEventDispatcherImpl.java @@ -184,6 +184,7 @@ public synchronized void deregisterCacheEventListener(CacheEventListener Date: Thu, 25 Jul 2024 14:02:27 +0200 Subject: [PATCH 7/8] Fixes #3169: add additional Tests for new EventType filtering Signed-off-by: Samuel Bussmann --- .../events/CacheEventDispatcherImplTest.java | 46 +++++++++- .../events/InvocationScopedEventSinkTest.java | 18 ++++ .../DefaultStoreEventDispatcherTest.java | 91 +++++++++++++++++++ 3 files changed, 153 insertions(+), 2 deletions(-) diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/events/CacheEventDispatcherImplTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/events/CacheEventDispatcherImplTest.java index 5274779bff..a8b11a544b 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/events/CacheEventDispatcherImplTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/events/CacheEventDispatcherImplTest.java @@ -27,10 +27,9 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mockito; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.util.EnumSet; import java.util.concurrent.CountDownLatch; @@ -38,12 +37,15 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -120,6 +122,37 @@ public void testListenerRegistrationEnablesStoreEvents() { verify(storeEventDispatcher).addEventListener(any(StoreEventListener.class)); } + @Test + public void testMultipleListenerRegistrationNotifiesStoreEvent() { + eventService.registerCacheEventListener(listener, EventOrdering.UNORDERED, EventFiring.ASYNCHRONOUS, EnumSet.of(EventType.UPDATED)); + CacheEventListener otherLsnr = mock(CacheEventListener.class); + eventService.registerCacheEventListener(otherLsnr, EventOrdering.ORDERED, EventFiring.SYNCHRONOUS, EnumSet.of(EventType.REMOVED)); + verify(storeEventDispatcher, times(1)).listenerModified(); + } + + @Test + public void testStoreListenerRegisteredEventTypesUpdated() { + eventService.registerCacheEventListener(listener, EventOrdering.UNORDERED, EventFiring.ASYNCHRONOUS, EnumSet.of(EventType.UPDATED)); + CacheEventListener otherLsnr = mock(CacheEventListener.class); + eventService.registerCacheEventListener(otherLsnr, EventOrdering.ORDERED, EventFiring.SYNCHRONOUS, EnumSet.of(EventType.REMOVED)); + verify(storeEventDispatcher, times(1)).listenerModified(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(StoreEventListener.class); + verify(storeEventDispatcher).addEventListener(captor.capture()); + StoreEventListener storeListener = captor.getValue(); + + assertTrue(storeListener.getEventTypes().contains(EventType.UPDATED)); + assertTrue(storeListener.getEventTypes().contains(EventType.REMOVED)); + assertEquals(2, storeListener.getEventTypes().size()); + + eventService.deregisterCacheEventListener(listener); + + verify(storeEventDispatcher, times(2)).listenerModified(); + + assertTrue(storeListener.getEventTypes().contains(EventType.REMOVED)); + assertEquals(1, storeListener.getEventTypes().size()); + } + @Test public void testOrderedListenerRegistrationTogglesOrderedOnStoreEvents() { eventService.registerCacheEventListener(listener, EventOrdering.ORDERED, EventFiring.ASYNCHRONOUS, EnumSet.allOf(EventType.class)); @@ -167,6 +200,15 @@ public void testDeregisterNotLastListenerDoesNotStopStoreEvents() { verify(storeEventDispatcher, never()).removeEventListener(any(StoreEventListener.class)); } + @Test + public void testDeregisterNotLastListenerTriggersListernerModified() { + eventService.registerCacheEventListener(listener, EventOrdering.UNORDERED, EventFiring.SYNCHRONOUS, EnumSet.of(EventType.EVICTED)); + eventService.registerCacheEventListener(mock(CacheEventListener.class), EventOrdering.UNORDERED, EventFiring.SYNCHRONOUS, + EnumSet.of(EventType.EVICTED)); + eventService.deregisterCacheEventListener(listener); + verify(storeEventDispatcher, times(2)).listenerModified(); + } + @Test public void testShutdownDisableStoreEventsAndShutsDownOrderedExecutor() { eventService.registerCacheEventListener(listener, diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java index e6c95c573a..d5b282220c 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java @@ -71,6 +71,13 @@ private InvocationScopedEventSink createEventSink(boolean ordere return new InvocationScopedEventSink<>(Collections.emptySet(), ordered, queues, storeEventListeners, EnumSet.allOf(EventType.class)); } + private InvocationScopedEventSink createEventSink(boolean ordered, EventType eventTypes) { + @SuppressWarnings("unchecked") + BlockingQueue>[] queues = (BlockingQueue>[])new BlockingQueue[] { + blockingQueue }; + return new InvocationScopedEventSink<>(Collections.emptySet(), ordered, queues, storeEventListeners, EnumSet.of(eventTypes)); + } + @Test public void testReset() { eventSink = createEventSink(false); @@ -134,4 +141,15 @@ public void testInterruption() throws InterruptedException { assertThat(eventSink.getEvents()).hasSize(10); assertThat(eventSink.getEvents().getLast().getEvent().getKey()).isEqualTo("k9"); } + + @Test + public void testAcceptEventFiltersIrrelevantEventTypes() { + eventSink = createEventSink(false, EventType.REMOVED); + eventSink.created("k", "v"); + + assertThat(eventSink.getEvents()).isEmpty(); + + eventSink.removed("k", () -> "v"); + assertThat(eventSink.getEvents()).hasSize(1); + } } diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java index 59203c10f5..dab3726947 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/store/DefaultStoreEventDispatcherTest.java @@ -29,16 +29,21 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.EnumSet; import java.util.Random; +import java.util.Set; import java.util.concurrent.CountDownLatch; import static org.ehcache.impl.internal.util.Matchers.eventOfType; import static org.ehcache.test.MockitoUtil.uncheckedGenericMock; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -77,6 +82,92 @@ public void testListenerNotifiedUnordered() { verify(listener).onEvent(any(StoreEvent.class)); } + @Test + @SuppressWarnings("unchecked") + public void testListenerNotifiedForRelevantEventsOnly() { + DefaultStoreEventDispatcher dispatcher = new DefaultStoreEventDispatcher<>(1); + StoreEventListener listener = uncheckedGenericMock(StoreEventListener.class); + when(listener.getEventTypes()).thenReturn(EnumSet.of(EventType.REMOVED)); + + dispatcher.addEventListener(listener); + + StoreEventSink sink = dispatcher.eventSink(); + sink.created("test", "test"); + dispatcher.releaseEventSink(sink); + + verify(listener, never()).onEvent(any(StoreEvent.class)); + + sink.removed("test", () -> "test"); + dispatcher.releaseEventSink(sink); + + verify(listener, times(1)).onEvent(any(StoreEvent.class)); + } + + @Test + @SuppressWarnings("unchecked") + public void testRelevantEventTypesUpdatedOnAddEventListener() { + DefaultStoreEventDispatcher dispatcher = new DefaultStoreEventDispatcher<>(1); + StoreEventListener listener = uncheckedGenericMock(StoreEventListener.class); + dispatcher.addEventListener(listener); + + verify(listener, times(1)).getEventTypes(); + + StoreEventListener listener2 = uncheckedGenericMock(StoreEventListener.class); + + dispatcher.addEventListener(listener2); + + verify(listener, times(2)).getEventTypes(); + verify(listener2, times(1)).getEventTypes(); + } + + @Test + @SuppressWarnings("unchecked") + public void testRelevantEventTypesUpdatedOnRemoveEventListener() { + DefaultStoreEventDispatcher dispatcher = new DefaultStoreEventDispatcher<>(1); + StoreEventListener listener = uncheckedGenericMock(StoreEventListener.class); + dispatcher.addEventListener(listener); + StoreEventListener listener2 = uncheckedGenericMock(StoreEventListener.class); + dispatcher.addEventListener(listener2); + verify(listener2, times(1)).getEventTypes(); + + dispatcher.removeEventListener(listener); + + verify(listener2, times(2)).getEventTypes(); + } + + @Test + @SuppressWarnings("unchecked") + public void testRelevantEventTypesUpdatedOnListenerModifiedSignal() { + DefaultStoreEventDispatcher dispatcher = new DefaultStoreEventDispatcher<>(1); + StoreEventListener listener = uncheckedGenericMock(StoreEventListener.class); + dispatcher.addEventListener(listener); + verify(listener, times(1)).getEventTypes(); + + dispatcher.listenerModified(); + + verify(listener, times(2)).getEventTypes(); + } + + @Test + @SuppressWarnings("unchecked") + public void testRelevantEventTypesContainAllEventsForMocks() { + DefaultStoreEventDispatcherMock dispatcher = new DefaultStoreEventDispatcherMock<>(1); + StoreEventListener listener = uncheckedGenericMock(StoreEventListener.class); + dispatcher.addEventListener(listener); + + assertTrue(dispatcher.spyRelevantEventTypes().containsAll(EventType.allAsSet())); + } + + class DefaultStoreEventDispatcherMock extends DefaultStoreEventDispatcher { + public DefaultStoreEventDispatcherMock(int dispatcherConcurrency) { + super(dispatcherConcurrency); + } + + public Set spyRelevantEventTypes() { + return super.getRelevantEventTypes(); + } + } + @Test @SuppressWarnings("unchecked") public void testListenerNotifiedOrdered() { From ae14b22bfb8b0596a54acb0af921af18c36c63bd Mon Sep 17 00:00:00 2001 From: Samuel Bussmann Date: Thu, 25 Jul 2024 14:23:39 +0200 Subject: [PATCH 8/8] Fixes #3169: clean-up formatting Signed-off-by: Samuel Bussmann --- .../events/InvocationScopedEventSink.java | 21 ++-- .../store/shared/store/StorePartition.java | 109 ++++++++---------- .../FudgingInvocationScopedEventSinkTest.java | 34 +++--- .../events/InvocationScopedEventSinkTest.java | 50 ++++---- 4 files changed, 97 insertions(+), 117 deletions(-) diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java index c3c4e5a415..8dcb615b7d 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/events/InvocationScopedEventSink.java @@ -17,11 +17,9 @@ package org.ehcache.impl.internal.events; -import static org.ehcache.impl.internal.events.StoreEvents.createEvent; -import static org.ehcache.impl.internal.events.StoreEvents.evictEvent; -import static org.ehcache.impl.internal.events.StoreEvents.expireEvent; -import static org.ehcache.impl.internal.events.StoreEvents.removeEvent; -import static org.ehcache.impl.internal.events.StoreEvents.updateEvent; +import org.ehcache.event.EventType; +import org.ehcache.core.spi.store.events.StoreEventFilter; +import org.ehcache.core.spi.store.events.StoreEventListener; import java.util.ArrayDeque; import java.util.Deque; @@ -30,9 +28,11 @@ import java.util.concurrent.BlockingQueue; import java.util.function.Supplier; -import org.ehcache.core.spi.store.events.StoreEventFilter; -import org.ehcache.core.spi.store.events.StoreEventListener; -import org.ehcache.event.EventType; +import static org.ehcache.impl.internal.events.StoreEvents.createEvent; +import static org.ehcache.impl.internal.events.StoreEvents.evictEvent; +import static org.ehcache.impl.internal.events.StoreEvents.expireEvent; +import static org.ehcache.impl.internal.events.StoreEvents.removeEvent; +import static org.ehcache.impl.internal.events.StoreEvents.updateEvent; /** * InvocationScopedEventSink @@ -44,12 +44,11 @@ class InvocationScopedEventSink implements CloseableStoreEventSink { private final BlockingQueue>[] orderedQueues; private final Set> listeners; private final Deque> events = new ArrayDeque<>(4); - private final Set relevantEventTypes; InvocationScopedEventSink(Set> filters, boolean ordered, - BlockingQueue>[] orderedQueues, Set> listeners, - Set relevantEventTypes) { + BlockingQueue>[] orderedQueues, Set> listeners, + Set relevantEventTypes) { this.filters = filters; this.ordered = ordered; this.orderedQueues = orderedQueues; diff --git a/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java b/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java index 2be71fb44b..59e21396ed 100644 --- a/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java +++ b/ehcache-impl/src/main/java/org/ehcache/impl/internal/store/shared/store/StorePartition.java @@ -17,22 +17,6 @@ package org.ehcache.impl.internal.store.shared.store; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.NoSuchElementException; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.BiFunction; -import java.util.function.Consumer; -import java.util.function.Function; -import java.util.function.Supplier; -import java.util.stream.Collectors; - -import javax.annotation.Nonnull; - import org.ehcache.Cache; import org.ehcache.config.ResourceType; import org.ehcache.core.CacheConfigurationChangeListener; @@ -50,6 +34,21 @@ import org.ehcache.spi.resilience.StoreAccessException; import org.slf4j.Logger; +import javax.annotation.Nonnull; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; + public class StorePartition extends AbstractPartition, CompositeValue>> implements Store { private final Logger logger = EhcachePrefixLoggerFactory.getLogger(StorePartition.class); @@ -59,14 +58,14 @@ public class StorePartition extends AbstractPartition valueType; public StorePartition(ResourceType type, int id, Class keyType, Class valueType, - Store, CompositeValue> store) { + Store, CompositeValue> store) { super(type, id, store); this.keyType = keyType; this.valueType = valueType; } protected K checkKey(K keyObject) { - if (keyType.isInstance(Objects.requireNonNull((Object)keyObject))) { + if (keyType.isInstance(Objects.requireNonNull((Object) keyObject))) { return keyObject; } else { throw new ClassCastException("Invalid key type, expected : " + keyType.getName() + " but was : " + keyObject.getClass().getName()); @@ -74,11 +73,10 @@ protected K checkKey(K keyObject) { } protected V checkValue(V valueObject) { - if (valueType.isInstance(Objects.requireNonNull((Object)valueObject))) { + if (valueType.isInstance(Objects.requireNonNull((Object) valueObject))) { return valueObject; } else { - throw new ClassCastException( - "Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); + throw new ClassCastException("Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); } } @@ -152,16 +150,13 @@ public ReplaceStatus replace(K key, V oldValue, V newValue) throws StoreAccessEx @Override public ValueHolder getAndCompute(K key, BiFunction mappingFunction) throws StoreAccessException { checkKey(key); - return decode( - shared().getAndCompute(composite(key), (k, v) -> composite(mappingFunction.apply(k.getValue(), v != null ? v.getValue() : null)))); + return decode(shared().getAndCompute(composite(key), (k, v) -> composite(mappingFunction.apply(k.getValue(), v != null ? v.getValue() : null)))); } @Override - public ValueHolder computeAndGet(K key, BiFunction mappingFunction, Supplier replaceEqual, - Supplier invokeWriter) throws StoreAccessException { + public ValueHolder computeAndGet(K key, BiFunction mappingFunction, Supplier replaceEqual, Supplier invokeWriter) throws StoreAccessException { checkKey(key); - return decode(shared().computeAndGet(composite(key), - (k, v) -> composite(mappingFunction.apply(k.getValue(), v == null ? null : v.getValue())), replaceEqual, invokeWriter)); + return decode(shared().computeAndGet(composite(key), (k, v) -> composite(mappingFunction.apply(k.getValue(), v == null ? null : v.getValue())), replaceEqual, invokeWriter)); } @Override @@ -172,31 +167,30 @@ public ValueHolder computeIfAbsent(K key, Function ma @Override public Map> bulkCompute(Set keys, - Function>, Iterable>> remappingFunction) - throws StoreAccessException { + Function>, Iterable>> remappingFunction) throws StoreAccessException { return bulkCompute(keys, remappingFunction, SUPPLY_TRUE); } @Override public Map> bulkCompute(Set keys, - Function>, Iterable>> remappingFunction, - Supplier replaceEqual) throws StoreAccessException { + Function>, Iterable>> remappingFunction, + Supplier replaceEqual) throws StoreAccessException { Map, ValueHolder>> results; if (remappingFunction instanceof Ehcache.PutAllFunction) { - Ehcache.PutAllFunction putAllFunction = (Ehcache.PutAllFunction)remappingFunction; + Ehcache.PutAllFunction putAllFunction = (Ehcache.PutAllFunction) remappingFunction; Ehcache.PutAllFunction, CompositeValue> compositePutAllFunction = new Ehcache.PutAllFunction, CompositeValue>() { @Override public Map, CompositeValue> getEntriesToRemap() { - return putAllFunction.getEntriesToRemap().entrySet().stream() - .collect(Collectors.toMap(e -> composite(e.getKey()), e -> composite(e.getValue()))); + return putAllFunction.getEntriesToRemap().entrySet().stream().collect(Collectors.toMap(e -> composite(e.getKey()), e -> composite(e.getValue()))); } @Override public boolean newValueAlreadyExpired(CompositeValue key, CompositeValue oldValue, CompositeValue newValue) { - return putAllFunction.newValueAlreadyExpired(key.getValue(), oldValue == null ? null : oldValue.getValue(), - newValue == null ? null : newValue.getValue()); + return putAllFunction.newValueAlreadyExpired(key.getValue(), + oldValue == null ? null : oldValue.getValue(), + newValue == null ? null : newValue.getValue()); } @Override @@ -212,7 +206,7 @@ public AtomicInteger getActualUpdateCount() { results = shared().bulkCompute(compositeSet(keys), compositePutAllFunction); } else if (remappingFunction instanceof Ehcache.RemoveAllFunction) { - Ehcache.RemoveAllFunction removeAllFunction = (Ehcache.RemoveAllFunction)remappingFunction; + Ehcache.RemoveAllFunction removeAllFunction = (Ehcache.RemoveAllFunction) remappingFunction; Ehcache.RemoveAllFunction, CompositeValue> compositeRemappingFunction = removeAllFunction::getActualRemoveCount; results = shared().bulkCompute(compositeSet(keys), compositeRemappingFunction); } else { @@ -226,10 +220,9 @@ public AtomicInteger getActualUpdateCount() { @Override public Map> bulkComputeIfAbsent(Set keys, - Function, Iterable>> mappingFunction) - throws StoreAccessException { - Map, ValueHolder>> results = shared().bulkComputeIfAbsent(compositeSet(keys), - new BulkComputeIfAbsentMappingFunction<>(id(), keyType, valueType, mappingFunction)); + Function, Iterable>> mappingFunction) throws StoreAccessException { + Map, ValueHolder>> results = + shared().bulkComputeIfAbsent(compositeSet(keys), new BulkComputeIfAbsentMappingFunction<>(id(), keyType, valueType, mappingFunction)); Map> decodedResults = new HashMap<>(); results.forEach((k, v) -> decodedResults.put(k.getValue(), decode(v))); return decodedResults; @@ -262,8 +255,8 @@ public void addEventListener(StoreEventListener eventListener) { public void onEvent(StoreEvent, CompositeValue> event) { if (event.getKey().getStoreId() == id()) { eventListener.onEvent(new StoreEventImpl<>(event.getType(), event.getKey().getValue(), - event.getOldValue() == null ? null : event.getOldValue().getValue(), - event.getNewValue() == null ? null : event.getNewValue().getValue())); + event.getOldValue() == null ? null : event.getOldValue().getValue(), + event.getNewValue() == null ? null : event.getNewValue().getValue())); } } @@ -282,7 +275,7 @@ public boolean equals(Object obj) { @SuppressWarnings("unchecked") @Override public void removeEventListener(StoreEventListener eventListener) { - storeEventSource.removeEventListener((StoreEventListener, CompositeValue>)eventListener); + storeEventSource.removeEventListener((StoreEventListener, CompositeValue>) eventListener); } @Override @@ -290,7 +283,7 @@ public void addEventFilter(StoreEventFilter eventFilter) { storeEventSource.addEventFilter((type, key, oldValue, newValue) -> { if (key.getStoreId() == id()) { return eventFilter.acceptEvent(type, key.getValue(), oldValue == null ? null : oldValue.getValue(), - newValue == null ? null : newValue.getValue()); + newValue == null ? null : newValue.getValue()); } else { return true; } @@ -325,7 +318,6 @@ public Iterator>> iterator() { Iterator, ValueHolder>>> iterator = shared().iterator(); return new Iterator>>() { private Cache.Entry> prefetched = advance(); - @Override public boolean hasNext() { return prefetched != null; @@ -352,7 +344,6 @@ private Cache.Entry> advance() { public K getKey() { return next.getKey().getValue(); } - @Override public ValueHolder getValue() { return decode(next.getValue()); @@ -391,7 +382,7 @@ public T get() { } } - public static abstract class BaseRemappingFunction { + public static abstract class BaseRemappingFunction { protected final int storeId; protected final Class keyType; protected final Class valueType; @@ -410,25 +401,21 @@ protected void keyCheck(Object keyObject) { protected void valueCheck(Object valueObject) { if (!valueType.isInstance(Objects.requireNonNull(valueObject))) { - throw new ClassCastException( - "Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); + throw new ClassCastException("Invalid value type, expected : " + valueType.getName() + " but was : " + valueObject.getClass().getName()); } } } - public static class BulkComputeMappingFunction extends BaseRemappingFunction implements - Function, ? extends CompositeValue>>, Iterable, ? extends CompositeValue>>> { + public static class BulkComputeMappingFunction extends BaseRemappingFunction implements Function, ? extends CompositeValue>>, Iterable, ? extends CompositeValue>>> { private final Function>, Iterable>> function; - BulkComputeMappingFunction(int storeId, Class keyType, Class valueType, - Function>, Iterable>> function) { + BulkComputeMappingFunction(int storeId, Class keyType, Class valueType, Function>, Iterable>> function) { super(storeId, keyType, valueType); this.function = function; } @Override - public Iterable, ? extends CompositeValue>> apply( - Iterable, ? extends CompositeValue>> entries) { + public Iterable, ? extends CompositeValue>> apply(Iterable, ? extends CompositeValue>> entries) { Map decodedEntries = new HashMap<>(); entries.forEach(entry -> { K key = entry.getKey().getValue(); @@ -452,19 +439,16 @@ public static class BulkComputeMappingFunction extends BaseRemappingFuncti } } - public static class BulkComputeIfAbsentMappingFunction extends BaseRemappingFunction implements - Function>, Iterable, ? extends CompositeValue>>> { + public static class BulkComputeIfAbsentMappingFunction extends BaseRemappingFunction implements Function>, Iterable, ? extends CompositeValue>>> { private final Function, Iterable>> function; - BulkComputeIfAbsentMappingFunction(int storeId, Class keyType, Class valueType, - Function, Iterable>> function) { + BulkComputeIfAbsentMappingFunction(int storeId, Class keyType, Class valueType, Function, Iterable>> function) { super(storeId, keyType, valueType); this.function = function; } @Override - public Iterable, ? extends CompositeValue>> apply( - Iterable> compositeValues) { + public Iterable, ? extends CompositeValue>> apply(Iterable> compositeValues) { List keys = new ArrayList<>(); compositeValues.forEach(k -> { keyCheck(k.getValue()); @@ -486,3 +470,4 @@ public static class BulkComputeIfAbsentMappingFunction extends BaseRemappi } } } + diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java index 4a6001bdf4..73109fcb3c 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/FudgingInvocationScopedEventSinkTest.java @@ -17,12 +17,13 @@ package org.ehcache.impl.internal.events; -import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; -import static org.ehcache.test.MockitoUtil.uncheckedGenericMock; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.hamcrest.MockitoHamcrest.argThat; +import org.ehcache.core.spi.store.events.StoreEvent; +import org.ehcache.event.EventType; +import org.ehcache.core.spi.store.events.StoreEventListener; +import org.hamcrest.Matcher; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InOrder; import java.util.EnumSet; import java.util.HashSet; @@ -30,13 +31,12 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; -import org.ehcache.core.spi.store.events.StoreEvent; -import org.ehcache.core.spi.store.events.StoreEventListener; -import org.ehcache.event.EventType; -import org.hamcrest.Matcher; -import org.junit.Before; -import org.junit.Test; -import org.mockito.InOrder; +import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; +import static org.ehcache.test.MockitoUtil.uncheckedGenericMock; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; /** * FudgingInvocationScopedEventSinkTest @@ -53,11 +53,9 @@ public void setUp() { Set> storeEventListeners = new HashSet<>(); listener = uncheckedGenericMock(StoreEventListener.class); storeEventListeners.add(listener); - @SuppressWarnings({ "unchecked", "rawtypes" }) - BlockingQueue>[] blockingQueues = new BlockingQueue[] { - new ArrayBlockingQueue>(10) }; - eventSink = new FudgingInvocationScopedEventSink<>(new HashSet<>(), false, blockingQueues, storeEventListeners, - EnumSet.allOf(EventType.class)); + @SuppressWarnings({"unchecked", "rawtypes"}) + BlockingQueue>[] blockingQueues = new BlockingQueue[] { new ArrayBlockingQueue>(10) }; + eventSink = new FudgingInvocationScopedEventSink<>(new HashSet<>(), false, blockingQueues, storeEventListeners, EnumSet.allOf(EventType.class)); } @Test diff --git a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java index d5b282220c..026e2300bb 100644 --- a/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java +++ b/ehcache-impl/src/test/java/org/ehcache/impl/internal/events/InvocationScopedEventSinkTest.java @@ -17,20 +17,6 @@ package org.ehcache.impl.internal.events; -import static org.assertj.core.api.Assertions.assertThat; -import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.hamcrest.MockitoHamcrest.argThat; - -import java.util.Collections; -import java.util.EnumSet; -import java.util.Set; -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.IntStream; - import org.ehcache.core.spi.store.events.StoreEvent; import org.ehcache.core.spi.store.events.StoreEventListener; import org.ehcache.event.EventType; @@ -43,6 +29,20 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; +import java.util.Collections; +import java.util.EnumSet; +import java.util.Set; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.IntStream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.ehcache.impl.internal.store.offheap.AbstractOffHeapStoreTest.eventType; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; + /** * InvocationScopedEventSinkTest */ @@ -66,15 +66,13 @@ public void setUp() { private InvocationScopedEventSink createEventSink(boolean ordered) { @SuppressWarnings("unchecked") - BlockingQueue>[] queues = (BlockingQueue>[])new BlockingQueue[] { - blockingQueue }; + BlockingQueue>[] queues = (BlockingQueue>[]) new BlockingQueue[] { blockingQueue }; return new InvocationScopedEventSink<>(Collections.emptySet(), ordered, queues, storeEventListeners, EnumSet.allOf(EventType.class)); } private InvocationScopedEventSink createEventSink(boolean ordered, EventType eventTypes) { @SuppressWarnings("unchecked") - BlockingQueue>[] queues = (BlockingQueue>[])new BlockingQueue[] { - blockingQueue }; + BlockingQueue>[] queues = (BlockingQueue>[]) new BlockingQueue[] { blockingQueue }; return new InvocationScopedEventSink<>(Collections.emptySet(), ordered, queues, storeEventListeners, EnumSet.of(eventTypes)); } @@ -101,8 +99,8 @@ public void testReset() { } /** - * Make sure an interrupted sink sets the interrupted flag and keep both event queues in the state as of before the event that was - * interrupted. + * Make sure an interrupted sink sets the interrupted flag and keep both event queues in the state + * as of before the event that was interrupted. * * @throws InterruptedException */ @@ -122,7 +120,7 @@ public void testInterruption() throws InterruptedException { }); t.start(); - while (blockingQueue.remainingCapacity() != 0) { + while(blockingQueue.remainingCapacity() != 0) { System.out.println(blockingQueue.remainingCapacity()); } @@ -132,11 +130,11 @@ public void testInterruption() throws InterruptedException { assertThat(wasInterrupted).isTrue(); assertThat(blockingQueue).hasSize(10); IntStream.range(0, 10).forEachOrdered(i -> { - try { - assertThat(blockingQueue.take().getEvent().getKey()).isEqualTo("k" + i); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } + try { + assertThat(blockingQueue.take().getEvent().getKey()).isEqualTo("k" + i); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } }); assertThat(eventSink.getEvents()).hasSize(10); assertThat(eventSink.getEvents().getLast().getEvent().getKey()).isEqualTo("k9");