From 4b51d5e3985dd03d786016d1e603feda0af24bee Mon Sep 17 00:00:00 2001 From: Mitchell Herrijgers Date: Thu, 15 Dec 2022 08:01:03 +0100 Subject: [PATCH 01/11] Fix caching mechanism for Sagas. They were no longer being cached properly, as well as leading to NPE --- .../org/axonframework/common/caching/Cache.java | 12 ++++++++++++ .../common/caching/EhCacheAdapter.java | 12 ++++++++++++ .../common/caching/JCacheAdapter.java | 12 ++++++++++++ .../org/axonframework/common/caching/NoCache.java | 6 ++++++ .../common/caching/WeakReferenceCache.java | 14 ++++++++++++++ .../saga/repository/CachingSagaStore.java | 3 +-- .../saga/repository/CachingSagaStoreTest.java | 5 +---- .../AggregateStereotypeAutoConfigurationTest.java | 10 ++++++++-- 8 files changed, 66 insertions(+), 8 deletions(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/Cache.java b/messaging/src/main/java/org/axonframework/common/caching/Cache.java index b440a5620f..334524fd46 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/Cache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/Cache.java @@ -18,6 +18,7 @@ import org.axonframework.common.Registration; +import java.util.function.Supplier; import java.util.function.UnaryOperator; /** @@ -59,6 +60,17 @@ public interface Cache { */ boolean putIfAbsent(Object key, Object value); + /** + * Stores the value given by the {@code valueSupplier} in the cache, under given {@code key}, if no element is yet + * available under that key. This operation is performed atomically. + * + * @param key The key under which to store the item + * @param valueSupplier A supplier that lazily supplies the value when necessary + * @return The value that is in the cache after the operation. This can be the original value or the one supplied by + * the {@code valueSupplier}. + */ + T getOrCompute(Object key, Supplier valueSupplier); + /** * Removes the entry stored under given {@code key}. If no such entry exists, nothing happens. * diff --git a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java index fe52e1ec20..cff94d9292 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java @@ -22,6 +22,7 @@ import net.sf.ehcache.event.CacheEventListener; import org.axonframework.common.Registration; +import java.util.function.Supplier; import java.util.function.UnaryOperator; /** @@ -60,6 +61,17 @@ public boolean putIfAbsent(Object key, Object value) { return ehCache.putIfAbsent(new Element(key, value)) == null; } + @Override + public T getOrCompute(Object key, Supplier valueSupplier) { + Element current = ehCache.get(key); + if(current != null) { + return (T) current.getObjectValue(); + } + T newValue = valueSupplier.get(); + ehCache.put(new Element(key, newValue)); + return newValue; + } + @Override public boolean remove(Object key) { return ehCache.remove(key); diff --git a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java index 6c3db4b6d5..1ef285f1f2 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java @@ -19,6 +19,7 @@ import org.axonframework.common.Registration; import java.io.Serializable; +import java.util.function.Supplier; import java.util.function.UnaryOperator; import javax.cache.configuration.CacheEntryListenerConfiguration; import javax.cache.configuration.Factory; @@ -66,6 +67,17 @@ public boolean putIfAbsent(Object key, Object value) { return jCache.putIfAbsent(key, value); } + @Override + public T getOrCompute(Object key, Supplier valueSupplier) { + Object o = jCache.get(key); + if(o != null) { + return (T) o; + } + T newValue = valueSupplier.get(); + jCache.put(key, newValue); + return newValue; + } + @Override public boolean remove(Object key) { return jCache.remove(key); diff --git a/messaging/src/main/java/org/axonframework/common/caching/NoCache.java b/messaging/src/main/java/org/axonframework/common/caching/NoCache.java index d530809db4..604dda89c1 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/NoCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/NoCache.java @@ -18,6 +18,7 @@ import org.axonframework.common.Registration; +import java.util.function.Supplier; import java.util.function.UnaryOperator; /** @@ -51,6 +52,11 @@ public boolean putIfAbsent(Object key, Object value) { return true; } + @Override + public T getOrCompute(Object key, Supplier valueSupplier) { + return valueSupplier.get(); + } + @Override public boolean remove(Object key) { return false; diff --git a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java index 72d0608f9d..edf9313550 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java @@ -27,6 +27,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArraySet; +import java.util.function.Supplier; import java.util.function.UnaryOperator; /** @@ -106,6 +107,19 @@ public boolean putIfAbsent(Object key, Object value) { return false; } + @Override + public T getOrCompute(Object key, Supplier valueSupplier) { + purgeItems(); + Entry entry = cache.computeIfAbsent(key, o -> { + T value = valueSupplier.get(); + for (EntryListener adapter : adapters) { + adapter.onEntryCreated(key, value); + } + return new Entry(o, value); + }); + return (T) entry.get(); + } + @Override public boolean remove(Object key) { if (cache.remove(key) != null) { diff --git a/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java b/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java index bb45104544..33633cbae4 100644 --- a/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java +++ b/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java @@ -76,8 +76,7 @@ public static Builder builder() { @Override public Set findSagas(Class sagaType, AssociationValue associationValue) { final String key = cacheKey(associationValue, sagaType); - associationsCache.putIfAbsent(key, delegate.findSagas(sagaType, associationValue)); - return associationsCache.get(key); + return associationsCache.getOrCompute(key, () -> delegate.findSagas(sagaType, associationValue)); } @Override diff --git a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java index f613dda51f..e2309935eb 100644 --- a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java +++ b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java @@ -22,7 +22,6 @@ import org.axonframework.modelling.saga.repository.inmemory.InMemorySagaStore; import org.junit.jupiter.api.*; -import java.util.Collections; import java.util.Set; import static java.util.Collections.singleton; @@ -98,9 +97,7 @@ void associationsAddedToCacheOnLoad() { Set actual = testSubject.findSagas(StubSaga.class, associationValue); assertEquals(singleton("id"), actual); - verify(associationsCache, atLeast(1)).get("org.axonframework.modelling.saga.repository.StubSaga/key=value"); - verify(associationsCache).putIfAbsent("org.axonframework.modelling.saga.repository.StubSaga/key=value", - Collections.singleton("id")); + verify(associationsCache, atLeast(1)).getOrCompute(eq("org.axonframework.modelling.saga.repository.StubSaga/key=value"), any()); } @Test diff --git a/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java b/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java index e411a3def4..60ec9e3d96 100644 --- a/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java +++ b/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java @@ -43,11 +43,12 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import javax.persistence.Entity; -import javax.persistence.Id; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; import java.util.function.UnaryOperator; +import javax.persistence.Entity; +import javax.persistence.Id; import static org.axonframework.modelling.command.AggregateLifecycle.apply; import static org.junit.jupiter.api.Assertions.*; @@ -309,6 +310,11 @@ public boolean putIfAbsent(Object key, Object value) { return false; } + @Override + public T getOrCompute(Object key, Supplier valueSupplier) { + return valueSupplier.get(); + } + @Override public boolean remove(Object key) { return false; From 0b88b4dff5492dcfec6b4633a9b252393108a916 Mon Sep 17 00:00:00 2001 From: Mitchell Herrijgers Date: Thu, 15 Dec 2022 08:07:52 +0100 Subject: [PATCH 02/11] Rename getOrCompute to computeIfAbsent and update javadoc --- .../main/java/org/axonframework/common/caching/Cache.java | 6 +++--- .../org/axonframework/common/caching/EhCacheAdapter.java | 2 +- .../org/axonframework/common/caching/JCacheAdapter.java | 2 +- .../main/java/org/axonframework/common/caching/NoCache.java | 2 +- .../axonframework/common/caching/WeakReferenceCache.java | 2 +- .../modelling/saga/repository/CachingSagaStore.java | 2 +- .../modelling/saga/repository/CachingSagaStoreTest.java | 2 +- .../AggregateStereotypeAutoConfigurationTest.java | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/Cache.java b/messaging/src/main/java/org/axonframework/common/caching/Cache.java index 334524fd46..54a42d186f 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/Cache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/Cache.java @@ -61,15 +61,15 @@ public interface Cache { boolean putIfAbsent(Object key, Object value); /** - * Stores the value given by the {@code valueSupplier} in the cache, under given {@code key}, if no element is yet - * available under that key. This operation is performed atomically. + * Returns the value under the given {@code key} in the cache. If there is no value present, will invoke the given + * {@code valueSupplier}, put the value in the cache and return the produced value. * * @param key The key under which to store the item * @param valueSupplier A supplier that lazily supplies the value when necessary * @return The value that is in the cache after the operation. This can be the original value or the one supplied by * the {@code valueSupplier}. */ - T getOrCompute(Object key, Supplier valueSupplier); + T computeIfAbsent(Object key, Supplier valueSupplier); /** * Removes the entry stored under given {@code key}. If no such entry exists, nothing happens. diff --git a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java index cff94d9292..464ba3d8f8 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java @@ -62,7 +62,7 @@ public boolean putIfAbsent(Object key, Object value) { } @Override - public T getOrCompute(Object key, Supplier valueSupplier) { + public T computeIfAbsent(Object key, Supplier valueSupplier) { Element current = ehCache.get(key); if(current != null) { return (T) current.getObjectValue(); diff --git a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java index 1ef285f1f2..a7943671c7 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java @@ -68,7 +68,7 @@ public boolean putIfAbsent(Object key, Object value) { } @Override - public T getOrCompute(Object key, Supplier valueSupplier) { + public T computeIfAbsent(Object key, Supplier valueSupplier) { Object o = jCache.get(key); if(o != null) { return (T) o; diff --git a/messaging/src/main/java/org/axonframework/common/caching/NoCache.java b/messaging/src/main/java/org/axonframework/common/caching/NoCache.java index 604dda89c1..ba4430fe62 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/NoCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/NoCache.java @@ -53,7 +53,7 @@ public boolean putIfAbsent(Object key, Object value) { } @Override - public T getOrCompute(Object key, Supplier valueSupplier) { + public T computeIfAbsent(Object key, Supplier valueSupplier) { return valueSupplier.get(); } diff --git a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java index edf9313550..bd639cfc9e 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java @@ -108,7 +108,7 @@ public boolean putIfAbsent(Object key, Object value) { } @Override - public T getOrCompute(Object key, Supplier valueSupplier) { + public T computeIfAbsent(Object key, Supplier valueSupplier) { purgeItems(); Entry entry = cache.computeIfAbsent(key, o -> { T value = valueSupplier.get(); diff --git a/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java b/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java index 33633cbae4..7efd8622df 100644 --- a/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java +++ b/modelling/src/main/java/org/axonframework/modelling/saga/repository/CachingSagaStore.java @@ -76,7 +76,7 @@ public static Builder builder() { @Override public Set findSagas(Class sagaType, AssociationValue associationValue) { final String key = cacheKey(associationValue, sagaType); - return associationsCache.getOrCompute(key, () -> delegate.findSagas(sagaType, associationValue)); + return associationsCache.computeIfAbsent(key, () -> delegate.findSagas(sagaType, associationValue)); } @Override diff --git a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java index e2309935eb..21219232f6 100644 --- a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java +++ b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java @@ -97,7 +97,7 @@ void associationsAddedToCacheOnLoad() { Set actual = testSubject.findSagas(StubSaga.class, associationValue); assertEquals(singleton("id"), actual); - verify(associationsCache, atLeast(1)).getOrCompute(eq("org.axonframework.modelling.saga.repository.StubSaga/key=value"), any()); + verify(associationsCache, atLeast(1)).computeIfAbsent(eq("org.axonframework.modelling.saga.repository.StubSaga/key=value"), any()); } @Test diff --git a/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java b/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java index 60ec9e3d96..86ff825339 100644 --- a/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java +++ b/spring-boot-autoconfigure/src/test/java/org/axonframework/springboot/autoconfig/AggregateStereotypeAutoConfigurationTest.java @@ -311,7 +311,7 @@ public boolean putIfAbsent(Object key, Object value) { } @Override - public T getOrCompute(Object key, Supplier valueSupplier) { + public T computeIfAbsent(Object key, Supplier valueSupplier) { return valueSupplier.get(); } From 0d92bb2eb146bfd5fe304f86c469e5d39fb73556 Mon Sep 17 00:00:00 2001 From: Mitchell Herrijgers Date: Thu, 15 Dec 2022 08:10:56 +0100 Subject: [PATCH 03/11] Re-add missing test check in CachingSagaStoreTest --- .../modelling/saga/repository/CachingSagaStoreTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java index 21219232f6..44a2b7a7da 100644 --- a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java +++ b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java @@ -21,8 +21,11 @@ import org.axonframework.modelling.saga.AssociationValuesImpl; import org.axonframework.modelling.saga.repository.inmemory.InMemorySagaStore; import org.junit.jupiter.api.*; +import org.mockito.*; +import java.util.Collections; import java.util.Set; +import java.util.function.Supplier; import static java.util.Collections.singleton; import static org.junit.jupiter.api.Assertions.*; @@ -97,7 +100,10 @@ void associationsAddedToCacheOnLoad() { Set actual = testSubject.findSagas(StubSaga.class, associationValue); assertEquals(singleton("id"), actual); - verify(associationsCache, atLeast(1)).computeIfAbsent(eq("org.axonframework.modelling.saga.repository.StubSaga/key=value"), any()); + ArgumentCaptor captor = ArgumentCaptor.forClass(Supplier.class); + verify(associationsCache, atLeast(1)).computeIfAbsent(eq("org.axonframework.modelling.saga.repository.StubSaga/key=value"), captor.capture()); + assertEquals(Collections.singleton("id"), captor.getValue().get()); + } @Test From 54fd1b51a7ea3517d8bcecb43259bde371d2b78e Mon Sep 17 00:00:00 2001 From: Steven van Beelen Date: Thu, 15 Dec 2022 15:01:18 +0100 Subject: [PATCH 04/11] Minor clean-ups on provided changes - Fix indentations - Solve warnings - Make Cache#computeIfAbsent a default implementation, throwing an unsupported exception when not implemented. #2517 --- .../java/org/axonframework/common/caching/Cache.java | 9 ++++++--- .../org/axonframework/common/caching/EhCacheAdapter.java | 7 ++++--- .../org/axonframework/common/caching/JCacheAdapter.java | 7 ++++--- .../axonframework/common/caching/WeakReferenceCache.java | 5 +++-- .../modelling/saga/repository/CachingSagaStoreTest.java | 9 ++++++--- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/Cache.java b/messaging/src/main/java/org/axonframework/common/caching/Cache.java index 54a42d186f..14044b47ad 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/Cache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/Cache.java @@ -64,12 +64,15 @@ public interface Cache { * Returns the value under the given {@code key} in the cache. If there is no value present, will invoke the given * {@code valueSupplier}, put the value in the cache and return the produced value. * - * @param key The key under which to store the item - * @param valueSupplier A supplier that lazily supplies the value when necessary + * @param key The key under which the item was cached. If not present, this key is used to cache the + * outcome of the {@code valueSupplier}. + * @param valueSupplier A supplier that lazily supplies the value if there's no {@code key} present. * @return The value that is in the cache after the operation. This can be the original value or the one supplied by * the {@code valueSupplier}. */ - T computeIfAbsent(Object key, Supplier valueSupplier); + default T computeIfAbsent(Object key, Supplier valueSupplier) { + throw new UnsupportedOperationException("Cache#computeIfAbsent is currently unsupported by this version"); + } /** * Removes the entry stored under given {@code key}. If no such entry exists, nothing happens. diff --git a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java index 464ba3d8f8..b88ab4296b 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java @@ -63,9 +63,10 @@ public boolean putIfAbsent(Object key, Object value) { @Override public T computeIfAbsent(Object key, Supplier valueSupplier) { - Element current = ehCache.get(key); - if(current != null) { - return (T) current.getObjectValue(); + Element currentElement = ehCache.get(key); + if (currentElement != null) { + //noinspection unchecked + return (T) currentElement.getObjectValue(); } T newValue = valueSupplier.get(); ehCache.put(new Element(key, newValue)); diff --git a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java index a7943671c7..145f21e1f1 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java @@ -69,9 +69,10 @@ public boolean putIfAbsent(Object key, Object value) { @Override public T computeIfAbsent(Object key, Supplier valueSupplier) { - Object o = jCache.get(key); - if(o != null) { - return (T) o; + Object currentValue = jCache.get(key); + if (currentValue != null) { + //noinspection unchecked + return (T) currentValue; } T newValue = valueSupplier.get(); jCache.put(key, newValue); diff --git a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java index bd639cfc9e..17923dadab 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java @@ -110,13 +110,14 @@ public boolean putIfAbsent(Object key, Object value) { @Override public T computeIfAbsent(Object key, Supplier valueSupplier) { purgeItems(); - Entry entry = cache.computeIfAbsent(key, o -> { + Entry entry = cache.computeIfAbsent(key, k -> { T value = valueSupplier.get(); for (EntryListener adapter : adapters) { adapter.onEntryCreated(key, value); } - return new Entry(o, value); + return new Entry(k, value); }); + //noinspection unchecked return (T) entry.get(); } diff --git a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java index 44a2b7a7da..88bc159c4a 100644 --- a/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java +++ b/modelling/src/test/java/org/axonframework/modelling/saga/repository/CachingSagaStoreTest.java @@ -100,10 +100,13 @@ void associationsAddedToCacheOnLoad() { Set actual = testSubject.findSagas(StubSaga.class, associationValue); assertEquals(singleton("id"), actual); - ArgumentCaptor captor = ArgumentCaptor.forClass(Supplier.class); - verify(associationsCache, atLeast(1)).computeIfAbsent(eq("org.axonframework.modelling.saga.repository.StubSaga/key=value"), captor.capture()); + //noinspection unchecked + ArgumentCaptor> captor = ArgumentCaptor.forClass(Supplier.class); + verify(associationsCache, atLeast(1)).computeIfAbsent( + eq("org.axonframework.modelling.saga.repository.StubSaga/key=value"), + captor.capture() + ); assertEquals(Collections.singleton("id"), captor.getValue().get()); - } @Test From 8af62858822558666725db859ec412c3fc175bf6 Mon Sep 17 00:00:00 2001 From: Steven van Beelen Date: Fri, 16 Dec 2022 16:13:23 +0100 Subject: [PATCH 05/11] Add integration tests for EhCache and WeakReferenceCache Add integration tests for EhCache and WeakReferenceCache, using a common test suite for validation. #2517 --- .../integrationtests/cache/CachedSaga.java | 110 ++++++ .../cache/CachingIntegrationTestSuite.java | 344 ++++++++++++++++++ .../cache/EhCacheIntegrationTest.java | 58 +++ .../WeakReferenceCacheIntegrationTest.java | 17 + 4 files changed, 529 insertions(+) create mode 100644 integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachedSaga.java create mode 100644 integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java create mode 100644 integrationtests/src/test/java/org/axonframework/integrationtests/cache/EhCacheIntegrationTest.java create mode 100644 integrationtests/src/test/java/org/axonframework/integrationtests/cache/WeakReferenceCacheIntegrationTest.java diff --git a/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachedSaga.java b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachedSaga.java new file mode 100644 index 0000000000..f20a2f3624 --- /dev/null +++ b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachedSaga.java @@ -0,0 +1,110 @@ +/* + * Copyright (c) 2010-2018. Axon Framework + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.axonframework.integrationtests.cache; + +import org.axonframework.modelling.saga.SagaEventHandler; +import org.axonframework.modelling.saga.SagaLifecycle; +import org.axonframework.modelling.saga.StartSaga; + +import java.util.ArrayList; +import java.util.List; + +/** + * Test saga used by the {@link CachingIntegrationTestSuite}. + * + * @author Steven van Beelen + */ +public class CachedSaga { + + private String name; + private List state; + + @StartSaga + @SagaEventHandler(associationProperty = "id") + public void on(SagaCreatedEvent event) { + this.name = event.name; + this.state = new ArrayList<>(); + } + + @SagaEventHandler(associationProperty = "id") + public void on(VeryImportantEvent event) { + state.add(event.stateEntry); + } + + @SagaEventHandler(associationProperty = "id") + public void on(SagaEndsEvent event) { + if (event.shouldEnd) { + SagaLifecycle.end(); + } + } + + public String getName() { + return name; + } + + public List getState() { + return state; + } + + @SuppressWarnings({"FieldCanBeLocal", "unused"}) + public static class SagaCreatedEvent { + + private final String id; + private final String name; + + public SagaCreatedEvent(String id, String name) { + this.id = id; + this.name = name; + } + + public String getId() { + return id; + } + } + + @SuppressWarnings({"FieldCanBeLocal", "unused"}) + public static class VeryImportantEvent { + + private final String id; + private final Object stateEntry; + + public VeryImportantEvent(String id, Object stateEntry) { + this.id = id; + this.stateEntry = stateEntry; + } + + public String getId() { + return id; + } + } + + @SuppressWarnings({"FieldCanBeLocal", "unused"}) + public static class SagaEndsEvent { + + private final String id; + private final boolean shouldEnd; + + public SagaEndsEvent(String id, boolean shouldEnd) { + this.id = id; + this.shouldEnd = shouldEnd; + } + + public String getId() { + return id; + } + } +} diff --git a/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java new file mode 100644 index 0000000000..b10b6be095 --- /dev/null +++ b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java @@ -0,0 +1,344 @@ +package org.axonframework.integrationtests.cache; + +import org.axonframework.common.caching.Cache; +import org.axonframework.config.Configuration; +import org.axonframework.config.DefaultConfigurer; +import org.axonframework.config.SagaConfigurer; +import org.axonframework.eventhandling.EventMessage; +import org.axonframework.eventhandling.GenericEventMessage; +import org.axonframework.eventsourcing.eventstore.EmbeddedEventStore; +import org.axonframework.eventsourcing.eventstore.EventStore; +import org.axonframework.eventsourcing.eventstore.inmemory.InMemoryEventStorageEngine; +import org.axonframework.modelling.saga.repository.CachingSagaStore; +import org.axonframework.modelling.saga.repository.SagaStore; +import org.axonframework.modelling.saga.repository.inmemory.InMemorySagaStore; +import org.junit.jupiter.api.*; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; +import java.util.stream.IntStream; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +/** + * Abstract integration test suite to validate the provided {@link org.axonframework.common.caching.Cache} + * implementations to work as intended under various stress scenarios. + * + * @author Steven van Beelen + */ +public abstract class CachingIntegrationTestSuite { + + // This ensures we do not wire Axon Server components + private static final boolean DO_NOT_AUTO_LOCATE_CONFIGURER_MODULES = false; + + protected Configuration config; + + private Cache sagaCache; + private Cache associationsCache; + + @BeforeEach + void setUp() { + EventStore eventStore = spy(EmbeddedEventStore.builder() + .storageEngine(new InMemoryEventStorageEngine()) + .build()); + sagaCache = buildCache("saga"); + associationsCache = buildCache("associations"); + + Consumer> sagaConfigurer = + config -> config.configureSagaStore(c -> CachingSagaStore.builder() + .delegateSagaStore(new InMemorySagaStore()) + .sagaCache(sagaCache) + .associationsCache(associationsCache) + .build()); + + config = DefaultConfigurer.defaultConfiguration(DO_NOT_AUTO_LOCATE_CONFIGURER_MODULES) + .configureEventStore(c -> eventStore) + .eventProcessing( + procConfigurer -> procConfigurer.usingSubscribingEventProcessors() + .registerSaga( + CachedSaga.class, + sagaConfigurer + ) + ) + .start(); + } + + /** + * Construct a {@link Cache} implementation used during testing. + * + * @param name The name to give to the {@link Cache} under construction. + * @return The constructed {@link Cache} instance. + */ + public abstract Cache buildCache(String name); + + @Test + void publishingBigEventTransactionTowardsCachedSagaWorksWithoutException() { + String sagaName = "foo"; + String associationValue = sagaName + "-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + int numberOfUpdates = 4096; + + // Construct the saga... + publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); + // Validate initial cache + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertTrue(cachedSaga.getState().isEmpty()); + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Bulk update the saga... + publishBulkUpdatesTo(associationValue, numberOfUpdates); + // Validate cache again + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertEquals(numberOfUpdates, cachedSaga.getState().size()); + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Destruct the saga... + publish(new CachedSaga.SagaEndsEvent(associationValue, true)); + // Validate cache is empty + assertFalse(associationsCache.containsKey(associationCacheKey)); + assertFalse(sagaCache.containsKey(sagaIdentifier)); + } + + @Test + void publishingBigEventTransactionsConcurrentlyTowardsCachedSagaWorksWithoutException() + throws ExecutionException, InterruptedException, TimeoutException { + String sagaName = "yeet"; + String associationValue = "some-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + int numberOfUpdates = 4096; + int numberOfConcurrentPublishers = 8; + ExecutorService executor = Executors.newFixedThreadPool(numberOfConcurrentPublishers); + + // Construct the saga... + publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); + // Validate initial cache + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertTrue(cachedSaga.getState().isEmpty()); + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Concurrent bulk update the saga... + IntStream.range(0, numberOfConcurrentPublishers) + .mapToObj(i -> CompletableFuture.runAsync( + () -> publishBulkUpdatesTo(associationValue, numberOfUpdates), executor + )) + .reduce(CompletableFuture::allOf) + .orElse(CompletableFuture.completedFuture(null)) + .get(5, TimeUnit.SECONDS); + + // Validate cache again + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertEquals(numberOfUpdates * numberOfConcurrentPublishers, cachedSaga.getState().size()); + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Destruct the saga... + publish(new CachedSaga.SagaEndsEvent(associationValue, true)); + // Validate cache is empty + assertFalse(associationsCache.containsKey(associationCacheKey)); + assertFalse(sagaCache.containsKey(sagaIdentifier)); + } + + @Test + void publishingBigEventTransactionTowardsSeveralCachedSagasWorksWithoutException() + throws ExecutionException, InterruptedException, TimeoutException { + String[] sagaNames = new String[]{"foo", "bar", "baz", "and", "some", "more"}; + int numberOfUpdates = 4096; + ExecutorService executor = Executors.newFixedThreadPool(sagaNames.length); + + // Construct the sagas... + for (String sagaName : sagaNames) { + String associationValue = sagaName + "-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + + publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); + // Validate initial cache + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertTrue(cachedSaga.getState().isEmpty()); + } + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Bulk update the sagas... + Arrays.stream(sagaNames) + .map(name -> CompletableFuture.runAsync( + () -> publishBulkUpdatesTo(name + "-id", numberOfUpdates), executor + )) + .reduce(CompletableFuture::allOf) + .orElse(CompletableFuture.completedFuture(null)) + .get(5, TimeUnit.SECONDS); + // Validate caches again + for (String sagaName : sagaNames) { + String associationValue = sagaName + "-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertEquals(numberOfUpdates, cachedSaga.getState().size(), sagaName); + } + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Destruct the sagas... + for (String sagaName : sagaNames) { + String associationValue = sagaName + "-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + + publish(new CachedSaga.SagaEndsEvent(associationValue, true)); + // Validate cache is empty + assertFalse(associationsCache.containsKey(associationCacheKey)); + } + } + + @Test + void publishingBigEventTransactionsConcurrentlyTowardsSeveralCachedSagasWorksWithoutException() + throws ExecutionException, InterruptedException, TimeoutException { + String[] sagaNames = new String[]{"foo", "bar", "baz", "and", "some", "more"}; + int numberOfUpdates = 4096; + int numberOfConcurrentPublishers = 8; + ExecutorService executor = Executors.newFixedThreadPool(sagaNames.length * numberOfConcurrentPublishers); + + // Construct the sagas... + for (String sagaName : sagaNames) { + String associationValue = sagaName + "-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + + publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); + // Validate initial cache + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertTrue(cachedSaga.getState().isEmpty()); + } + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Bulk update the sagas... + IntStream.range(0, sagaNames.length * numberOfConcurrentPublishers) + .mapToObj(i -> CompletableFuture.runAsync( + () -> publishBulkUpdatesTo(sagaNames[i % sagaNames.length] + "-id", numberOfUpdates), executor + )) + .reduce(CompletableFuture::allOf) + .orElse(CompletableFuture.completedFuture(null)) + .get(5, TimeUnit.SECONDS); + // Validate caches again + for (String sagaName : sagaNames) { + String associationValue = sagaName + "-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + + assertTrue(associationsCache.containsKey(associationCacheKey)); + //noinspection unchecked + String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + assertTrue(sagaCache.containsKey(sagaIdentifier)); + //noinspection unchecked + CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertEquals(numberOfUpdates * numberOfConcurrentPublishers, cachedSaga.getState().size(), sagaName); + } + + // Clear out the cache + associationsCache.removeAll(); + sagaCache.removeAll(); + + // Destruct the sagas... + for (String sagaName : sagaNames) { + String associationValue = sagaName + "-id"; + String associationCacheKey = sagaAssociationCacheKey(associationValue); + + publish(new CachedSaga.SagaEndsEvent(associationValue, true)); + // Validate cache is empty + assertFalse(associationsCache.containsKey(associationCacheKey)); + } + } + + private void publishBulkUpdatesTo(String sagaId, int amount) { + Object[] updateEvents = new Object[amount]; + for (int i = 0; i < amount; i++) { + updateEvents[i] = new CachedSaga.VeryImportantEvent(sagaId, i); + } + publish(updateEvents); + } + + private void publish(Object... events) { + List> eventMessages = new ArrayList<>(); + for (Object event : events) { + eventMessages.add(GenericEventMessage.asEventMessage(event)); + } + config.eventStore().publish(eventMessages); + } + + /** + * This method is based on the private {@code CachingSagaStore#cacheKey(AssociationValue, Class)} method. + * + * @param sagaId The association key within the event. + * @return The caching key used for the association values by a {@link CachingSagaStore}. + */ + private static String sagaAssociationCacheKey(String sagaId) { + return CachedSaga.class.getName() + "/id=" + sagaId; + } +} diff --git a/integrationtests/src/test/java/org/axonframework/integrationtests/cache/EhCacheIntegrationTest.java b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/EhCacheIntegrationTest.java new file mode 100644 index 0000000000..4166a6703d --- /dev/null +++ b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/EhCacheIntegrationTest.java @@ -0,0 +1,58 @@ +package org.axonframework.integrationtests.cache; + +import net.sf.ehcache.CacheManager; +import net.sf.ehcache.Ehcache; +import net.sf.ehcache.config.CacheConfiguration; +import net.sf.ehcache.config.SizeOfPolicyConfiguration; +import net.sf.ehcache.store.MemoryStoreEvictionPolicy; +import org.axonframework.common.caching.Cache; +import org.axonframework.common.caching.EhCacheAdapter; +import org.junit.jupiter.api.*; + +/** + * {@link Ehcache} specific implementation of the {@link CachingIntegrationTestSuite}. + * + * @author Steven van Beelen + */ +class EhCacheIntegrationTest extends CachingIntegrationTestSuite { + + private CacheManager cacheManager; + + @Override + @BeforeEach + void setUp() { + cacheManager = CacheManager.create(getEhCacheConfiguration()); + super.setUp(); + } + + @AfterEach + void tearDown() { + cacheManager.shutdown(); + } + + @Override + public Cache buildCache(String name) { + Ehcache cache = createCache(name); + cacheManager.addCache(cache); + return new EhCacheAdapter(cache); + } + + private Ehcache createCache(String name) { + CacheConfiguration cacheConfig = new CacheConfiguration(name, 10_000) + .name(name) + .memoryStoreEvictionPolicy(MemoryStoreEvictionPolicy.LRU) + .eternal(false) + .timeToLiveSeconds(600) + .timeToIdleSeconds(600); + return new net.sf.ehcache.Cache(cacheConfig); + } + + private net.sf.ehcache.config.Configuration getEhCacheConfiguration() { + net.sf.ehcache.config.Configuration configuration = new net.sf.ehcache.config.Configuration(); + SizeOfPolicyConfiguration sizeOfPolicyConfiguration = new SizeOfPolicyConfiguration(); + sizeOfPolicyConfiguration.maxDepth(13000); + sizeOfPolicyConfiguration.maxDepthExceededBehavior(SizeOfPolicyConfiguration.MaxDepthExceededBehavior.ABORT); + configuration.addSizeOfPolicy(sizeOfPolicyConfiguration); + return configuration; + } +} diff --git a/integrationtests/src/test/java/org/axonframework/integrationtests/cache/WeakReferenceCacheIntegrationTest.java b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/WeakReferenceCacheIntegrationTest.java new file mode 100644 index 0000000000..b551e2330b --- /dev/null +++ b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/WeakReferenceCacheIntegrationTest.java @@ -0,0 +1,17 @@ +package org.axonframework.integrationtests.cache; + +import org.axonframework.common.caching.Cache; +import org.axonframework.common.caching.WeakReferenceCache; + +/** + * {@link WeakReferenceCache} specific implementation of the {@link CachingIntegrationTestSuite}. + * + * @author Steven van Beelen + */ +class WeakReferenceCacheIntegrationTest extends CachingIntegrationTestSuite { + + @Override + public Cache buildCache(String name) { + return new WeakReferenceCache(); + } +} From 5d32c2c6d437b6878c1bcdb959a47ee50a039bdd Mon Sep 17 00:00:00 2001 From: Mitchell Herrijgers Date: Fri, 16 Dec 2022 19:06:54 +0100 Subject: [PATCH 06/11] Fix caches being able to produce a NPE upon read. Also now throws an exception if the value supplier returns null to differentiate between faulty cache and faulty saga stores --- .../common/caching/EhCacheAdapter.java | 9 +++++--- .../common/caching/JCacheAdapter.java | 3 +++ .../common/caching/WeakReferenceCache.java | 23 +++++++++++-------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java index b88ab4296b..3e5c0fc485 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java @@ -68,9 +68,12 @@ public T computeIfAbsent(Object key, Supplier valueSupplier) { //noinspection unchecked return (T) currentElement.getObjectValue(); } - T newValue = valueSupplier.get(); - ehCache.put(new Element(key, newValue)); - return newValue; + T value = valueSupplier.get(); + if (value == null) { + throw new IllegalStateException("Value Supplier of Cache produced a null value for key [" + key + "]!"); + } + ehCache.put(new Element(key, value)); + return value; } @Override diff --git a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java index 145f21e1f1..6e49e30e52 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java @@ -75,6 +75,9 @@ public T computeIfAbsent(Object key, Supplier valueSupplier) { return (T) currentValue; } T newValue = valueSupplier.get(); + if (newValue == null) { + throw new IllegalStateException("Value Supplier of Cache produced a null value for key [" + key + "]!"); + } jCache.put(key, newValue); return newValue; } diff --git a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java index 17923dadab..e657f94731 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java @@ -110,15 +110,20 @@ public boolean putIfAbsent(Object key, Object value) { @Override public T computeIfAbsent(Object key, Supplier valueSupplier) { purgeItems(); - Entry entry = cache.computeIfAbsent(key, k -> { - T value = valueSupplier.get(); - for (EntryListener adapter : adapters) { - adapter.onEntryCreated(key, value); - } - return new Entry(k, value); - }); - //noinspection unchecked - return (T) entry.get(); + Entry currentEntry = cache.get(key); + Object existingValue = currentEntry != null ? currentEntry.get() : null; + if (existingValue != null) { + return (T) currentEntry.get(); + } + T value = valueSupplier.get(); + if (value == null) { + throw new IllegalStateException("Value Supplier of Cache produced a null value for key [" + key + "]!"); + } + cache.put(key, new Entry(key, value)); + for (EntryListener adapter : adapters) { + adapter.onEntryCreated(key, value); + } + return value; } @Override From d5e02dbd7e9741104f1b7f374f145d69946e4d11 Mon Sep 17 00:00:00 2001 From: Steven van Beelen Date: Mon, 19 Dec 2022 11:27:46 +0100 Subject: [PATCH 07/11] Provide working default implementation of computeIfAbsent. Provide working default implementation of computeIfAbsent. As, if users do have their own implementation of the Cache, their migration to a more recent version of Axon Framework would horribly break. #2517 --- .../java/org/axonframework/common/caching/Cache.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/Cache.java b/messaging/src/main/java/org/axonframework/common/caching/Cache.java index 14044b47ad..9547acce91 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/Cache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/Cache.java @@ -71,7 +71,17 @@ public interface Cache { * the {@code valueSupplier}. */ default T computeIfAbsent(Object key, Supplier valueSupplier) { - throw new UnsupportedOperationException("Cache#computeIfAbsent is currently unsupported by this version"); + Object currentValue = get(key); + if (currentValue != null) { + //noinspection unchecked + return (T) currentValue; + } + T newValue = valueSupplier.get(); + if (newValue == null) { + throw new IllegalArgumentException("Value Supplier of Cache produced a null value for key [" + key + "]!"); + } + put(key, newValue); + return newValue; } /** From b3a836b6acb93e0d860172749fdef35f3fe0fe70 Mon Sep 17 00:00:00 2001 From: Steven van Beelen Date: Mon, 19 Dec 2022 13:29:04 +0100 Subject: [PATCH 08/11] Clean up computeIfAbsent implementations - Consistently use newValue as the result of the valueSupplier - Suppress warnings #2517 --- .../axonframework/common/caching/EhCacheAdapter.java | 8 ++++---- .../common/caching/WeakReferenceCache.java | 11 ++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java index 3e5c0fc485..0c0675ac8f 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java @@ -68,12 +68,12 @@ public T computeIfAbsent(Object key, Supplier valueSupplier) { //noinspection unchecked return (T) currentElement.getObjectValue(); } - T value = valueSupplier.get(); - if (value == null) { + T newValue = valueSupplier.get(); + if (newValue == null) { throw new IllegalStateException("Value Supplier of Cache produced a null value for key [" + key + "]!"); } - ehCache.put(new Element(key, value)); - return value; + ehCache.put(new Element(key, newValue)); + return newValue; } @Override diff --git a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java index e657f94731..58ea1cf67a 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java @@ -113,17 +113,18 @@ public T computeIfAbsent(Object key, Supplier valueSupplier) { Entry currentEntry = cache.get(key); Object existingValue = currentEntry != null ? currentEntry.get() : null; if (existingValue != null) { + //noinspection unchecked return (T) currentEntry.get(); } - T value = valueSupplier.get(); - if (value == null) { + T newValue = valueSupplier.get(); + if (newValue == null) { throw new IllegalStateException("Value Supplier of Cache produced a null value for key [" + key + "]!"); } - cache.put(key, new Entry(key, value)); + cache.put(key, new Entry(key, newValue)); for (EntryListener adapter : adapters) { - adapter.onEntryCreated(key, value); + adapter.onEntryCreated(key, newValue); } - return value; + return newValue; } @Override From 7c60cf3b82f40e229bc3bf02c5e974c0432f6794 Mon Sep 17 00:00:00 2001 From: Steven van Beelen Date: Mon, 19 Dec 2022 13:32:50 +0100 Subject: [PATCH 09/11] Fine tune test cases for resiliency - Move common used fields to constants - Remove manual cache clearing, as that's a job of the IT implementations to define when these are cleared. - Maintain references to the associations set, to ensure the weak reference implementation doesn't fail on the missing associations - Validate whether the saga cache entry is present before verifying the contents, instead of an assertion. This it to cover for cache implementation that may have cleared out the entries between the (bulk) updates and validation. As this is a perfectly valid scenario, just don't validate the cache if it's not there instead of breaking the test case. - Suppress unimportant warnings #2517 --- .../cache/CachingIntegrationTestSuite.java | 176 ++++++++---------- 1 file changed, 80 insertions(+), 96 deletions(-) diff --git a/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java index b10b6be095..8c65b51b81 100644 --- a/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java +++ b/integrationtests/src/test/java/org/axonframework/integrationtests/cache/CachingIntegrationTestSuite.java @@ -16,7 +16,9 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -40,6 +42,9 @@ public abstract class CachingIntegrationTestSuite { // This ensures we do not wire Axon Server components private static final boolean DO_NOT_AUTO_LOCATE_CONFIGURER_MODULES = false; + private static final int NUMBER_OF_UPDATES = 4096; + private static final int NUMBER_OF_CONCURRENT_PUBLISHERS = 8; + private static final String[] SAGA_NAMES = new String[]{"foo", "bar", "baz", "and", "some", "more"}; protected Configuration config; @@ -64,11 +69,8 @@ void setUp() { config = DefaultConfigurer.defaultConfiguration(DO_NOT_AUTO_LOCATE_CONFIGURER_MODULES) .configureEventStore(c -> eventStore) .eventProcessing( - procConfigurer -> procConfigurer.usingSubscribingEventProcessors() - .registerSaga( - CachedSaga.class, - sagaConfigurer - ) + procConfig -> procConfig.usingSubscribingEventProcessors() + .registerSaga(CachedSaga.class, sagaConfigurer) ) .start(); } @@ -83,42 +85,35 @@ void setUp() { @Test void publishingBigEventTransactionTowardsCachedSagaWorksWithoutException() { - String sagaName = "foo"; + String sagaName = SAGA_NAMES[0]; String associationValue = sagaName + "-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); - int numberOfUpdates = 4096; // Construct the saga... publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); // Validate initial cache assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + Set associations = associationsCache.get(associationCacheKey); + + String sagaIdentifier = associations.iterator().next(); assertTrue(sagaCache.containsKey(sagaIdentifier)); //noinspection unchecked CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); assertEquals(sagaName, cachedSaga.getName()); assertTrue(cachedSaga.getState().isEmpty()); - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); - // Bulk update the saga... - publishBulkUpdatesTo(associationValue, numberOfUpdates); + publishBulkUpdatesTo(associationValue, NUMBER_OF_UPDATES); // Validate cache again assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + associations = associationsCache.get(associationCacheKey); + + sagaIdentifier = associations.iterator().next(); assertTrue(sagaCache.containsKey(sagaIdentifier)); //noinspection unchecked cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); assertEquals(sagaName, cachedSaga.getName()); - assertEquals(numberOfUpdates, cachedSaga.getState().size()); - - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); + assertEquals(NUMBER_OF_UPDATES, cachedSaga.getState().size()); // Destruct the saga... publish(new CachedSaga.SagaEndsEvent(associationValue, true)); @@ -130,33 +125,28 @@ void publishingBigEventTransactionTowardsCachedSagaWorksWithoutException() { @Test void publishingBigEventTransactionsConcurrentlyTowardsCachedSagaWorksWithoutException() throws ExecutionException, InterruptedException, TimeoutException { - String sagaName = "yeet"; + String sagaName = SAGA_NAMES[0]; String associationValue = "some-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); - int numberOfUpdates = 4096; - int numberOfConcurrentPublishers = 8; - ExecutorService executor = Executors.newFixedThreadPool(numberOfConcurrentPublishers); + ExecutorService executor = Executors.newFixedThreadPool(NUMBER_OF_CONCURRENT_PUBLISHERS); // Construct the saga... publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); // Validate initial cache assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + Set associations = associationsCache.get(associationCacheKey); + + String sagaIdentifier = associations.iterator().next(); assertTrue(sagaCache.containsKey(sagaIdentifier)); //noinspection unchecked CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); assertEquals(sagaName, cachedSaga.getName()); assertTrue(cachedSaga.getState().isEmpty()); - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); - // Concurrent bulk update the saga... - IntStream.range(0, numberOfConcurrentPublishers) + IntStream.range(0, NUMBER_OF_CONCURRENT_PUBLISHERS) .mapToObj(i -> CompletableFuture.runAsync( - () -> publishBulkUpdatesTo(associationValue, numberOfUpdates), executor + () -> publishBulkUpdatesTo(associationValue, NUMBER_OF_UPDATES), executor )) .reduce(CompletableFuture::allOf) .orElse(CompletableFuture.completedFuture(null)) @@ -164,17 +154,14 @@ void publishingBigEventTransactionsConcurrentlyTowardsCachedSagaWorksWithoutExce // Validate cache again assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + associations = associationsCache.get(associationCacheKey); + + sagaIdentifier = associations.iterator().next(); assertTrue(sagaCache.containsKey(sagaIdentifier)); //noinspection unchecked cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); assertEquals(sagaName, cachedSaga.getName()); - assertEquals(numberOfUpdates * numberOfConcurrentPublishers, cachedSaga.getState().size()); - - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); + assertEquals(NUMBER_OF_UPDATES * NUMBER_OF_CONCURRENT_PUBLISHERS, cachedSaga.getState().size()); // Destruct the saga... publish(new CachedSaga.SagaEndsEvent(associationValue, true)); @@ -186,60 +173,57 @@ void publishingBigEventTransactionsConcurrentlyTowardsCachedSagaWorksWithoutExce @Test void publishingBigEventTransactionTowardsSeveralCachedSagasWorksWithoutException() throws ExecutionException, InterruptedException, TimeoutException { - String[] sagaNames = new String[]{"foo", "bar", "baz", "and", "some", "more"}; - int numberOfUpdates = 4096; - ExecutorService executor = Executors.newFixedThreadPool(sagaNames.length); + Map> associationReferences = new HashMap<>(); + ExecutorService executor = Executors.newFixedThreadPool(SAGA_NAMES.length); // Construct the sagas... - for (String sagaName : sagaNames) { + for (String sagaName : SAGA_NAMES) { String associationValue = sagaName + "-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); // Validate initial cache assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + associationReferences.put(associationCacheKey, associationsCache.get(associationCacheKey)); + + String sagaIdentifier = (associationReferences.get(associationCacheKey)).iterator().next(); assertTrue(sagaCache.containsKey(sagaIdentifier)); - //noinspection unchecked - CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + + SagaStore.Entry sagaEntry = sagaCache.get(sagaIdentifier); + CachedSaga cachedSaga = sagaEntry.saga(); assertEquals(sagaName, cachedSaga.getName()); assertTrue(cachedSaga.getState().isEmpty()); } - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); - // Bulk update the sagas... - Arrays.stream(sagaNames) + Arrays.stream(SAGA_NAMES) .map(name -> CompletableFuture.runAsync( - () -> publishBulkUpdatesTo(name + "-id", numberOfUpdates), executor + () -> publishBulkUpdatesTo(name + "-id", NUMBER_OF_UPDATES), executor )) .reduce(CompletableFuture::allOf) .orElse(CompletableFuture.completedFuture(null)) .get(5, TimeUnit.SECONDS); // Validate caches again - for (String sagaName : sagaNames) { + for (String sagaName : SAGA_NAMES) { String associationValue = sagaName + "-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); - assertTrue(sagaCache.containsKey(sagaIdentifier)); - //noinspection unchecked - CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); - assertEquals(sagaName, cachedSaga.getName()); - assertEquals(numberOfUpdates, cachedSaga.getState().size(), sagaName); + associationReferences.put(associationCacheKey, associationsCache.get(associationCacheKey)); + + String sagaIdentifier = (associationReferences.get(associationCacheKey)).iterator().next(); + SagaStore.Entry sagaEntry = sagaCache.get(sagaIdentifier); + // The saga cache may have been cleared already when doing bulk updates, which is a fair scenario. + // Hence, only validate the entry if it's still present in the Cache. + if (sagaEntry != null) { + CachedSaga cachedSaga = sagaEntry.saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertEquals(NUMBER_OF_UPDATES, cachedSaga.getState().size(), sagaName); + } } - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); - // Destruct the sagas... - for (String sagaName : sagaNames) { + for (String sagaName : SAGA_NAMES) { String associationValue = sagaName + "-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); @@ -252,61 +236,60 @@ void publishingBigEventTransactionTowardsSeveralCachedSagasWorksWithoutException @Test void publishingBigEventTransactionsConcurrentlyTowardsSeveralCachedSagasWorksWithoutException() throws ExecutionException, InterruptedException, TimeoutException { - String[] sagaNames = new String[]{"foo", "bar", "baz", "and", "some", "more"}; - int numberOfUpdates = 4096; - int numberOfConcurrentPublishers = 8; - ExecutorService executor = Executors.newFixedThreadPool(sagaNames.length * numberOfConcurrentPublishers); + Map> associationReferences = new HashMap<>(); + ExecutorService executor = Executors.newFixedThreadPool(SAGA_NAMES.length * NUMBER_OF_CONCURRENT_PUBLISHERS); // Construct the sagas... - for (String sagaName : sagaNames) { + for (String sagaName : SAGA_NAMES) { String associationValue = sagaName + "-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); publish(new CachedSaga.SagaCreatedEvent(associationValue, sagaName)); // Validate initial cache assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); + associationReferences.put(associationCacheKey, associationsCache.get(associationCacheKey)); + + String sagaIdentifier = (associationReferences.get(associationCacheKey)).iterator().next(); assertTrue(sagaCache.containsKey(sagaIdentifier)); - //noinspection unchecked - CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); + + SagaStore.Entry sagaEntry = sagaCache.get(sagaIdentifier); + CachedSaga cachedSaga = sagaEntry.saga(); assertEquals(sagaName, cachedSaga.getName()); assertTrue(cachedSaga.getState().isEmpty()); } - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); - // Bulk update the sagas... - IntStream.range(0, sagaNames.length * numberOfConcurrentPublishers) + IntStream.range(0, SAGA_NAMES.length * NUMBER_OF_CONCURRENT_PUBLISHERS) .mapToObj(i -> CompletableFuture.runAsync( - () -> publishBulkUpdatesTo(sagaNames[i % sagaNames.length] + "-id", numberOfUpdates), executor + () -> publishBulkUpdatesTo(SAGA_NAMES[i % SAGA_NAMES.length] + "-id", NUMBER_OF_UPDATES), + executor )) .reduce(CompletableFuture::allOf) .orElse(CompletableFuture.completedFuture(null)) .get(5, TimeUnit.SECONDS); // Validate caches again - for (String sagaName : sagaNames) { + for (String sagaName : SAGA_NAMES) { String associationValue = sagaName + "-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); assertTrue(associationsCache.containsKey(associationCacheKey)); - //noinspection unchecked - String sagaIdentifier = ((Set) associationsCache.get(associationCacheKey)).iterator().next(); - assertTrue(sagaCache.containsKey(sagaIdentifier)); - //noinspection unchecked - CachedSaga cachedSaga = ((SagaStore.Entry) sagaCache.get(sagaIdentifier)).saga(); - assertEquals(sagaName, cachedSaga.getName()); - assertEquals(numberOfUpdates * numberOfConcurrentPublishers, cachedSaga.getState().size(), sagaName); + associationReferences.put(associationCacheKey, associationsCache.get(associationCacheKey)); + + String sagaIdentifier = (associationReferences.get(associationCacheKey)).iterator().next(); + SagaStore.Entry sagaEntry = sagaCache.get(sagaIdentifier); + // The saga cache may have been cleared already when doing bulk updates, which is a fair scenario. + // Hence, only validate the entry if it's still present in the Cache. + if (sagaEntry != null) { + CachedSaga cachedSaga = sagaEntry.saga(); + assertEquals(sagaName, cachedSaga.getName()); + assertEquals(NUMBER_OF_UPDATES * NUMBER_OF_CONCURRENT_PUBLISHERS, + cachedSaga.getState().size(), + sagaName); + } } - // Clear out the cache - associationsCache.removeAll(); - sagaCache.removeAll(); - // Destruct the sagas... - for (String sagaName : sagaNames) { + for (String sagaName : SAGA_NAMES) { String associationValue = sagaName + "-id"; String associationCacheKey = sagaAssociationCacheKey(associationValue); @@ -316,7 +299,8 @@ void publishingBigEventTransactionsConcurrentlyTowardsSeveralCachedSagasWorksWit } } - private void publishBulkUpdatesTo(String sagaId, int amount) { + private void publishBulkUpdatesTo(String sagaId, + @SuppressWarnings("SameParameterValue") int amount) { Object[] updateEvents = new Object[amount]; for (int i = 0; i < amount; i++) { updateEvents[i] = new CachedSaga.VeryImportantEvent(sagaId, i); From d540d038bac073f2b358cfead2c50f7455a77e65 Mon Sep 17 00:00:00 2001 From: Steven van Beelen Date: Mon, 19 Dec 2022 14:00:15 +0100 Subject: [PATCH 10/11] Remove implementation specific versions of computeIfAbsent By introducing the default implementation on the Cache interface, we can get rid of the concrete implementation on the EhCache and JCache adapters. #2517 --- .../common/caching/EhCacheAdapter.java | 16 ---------------- .../common/caching/JCacheAdapter.java | 16 ---------------- 2 files changed, 32 deletions(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java index 0c0675ac8f..fe52e1ec20 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/EhCacheAdapter.java @@ -22,7 +22,6 @@ import net.sf.ehcache.event.CacheEventListener; import org.axonframework.common.Registration; -import java.util.function.Supplier; import java.util.function.UnaryOperator; /** @@ -61,21 +60,6 @@ public boolean putIfAbsent(Object key, Object value) { return ehCache.putIfAbsent(new Element(key, value)) == null; } - @Override - public T computeIfAbsent(Object key, Supplier valueSupplier) { - Element currentElement = ehCache.get(key); - if (currentElement != null) { - //noinspection unchecked - return (T) currentElement.getObjectValue(); - } - T newValue = valueSupplier.get(); - if (newValue == null) { - throw new IllegalStateException("Value Supplier of Cache produced a null value for key [" + key + "]!"); - } - ehCache.put(new Element(key, newValue)); - return newValue; - } - @Override public boolean remove(Object key) { return ehCache.remove(key); diff --git a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java index 6e49e30e52..6c3db4b6d5 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java +++ b/messaging/src/main/java/org/axonframework/common/caching/JCacheAdapter.java @@ -19,7 +19,6 @@ import org.axonframework.common.Registration; import java.io.Serializable; -import java.util.function.Supplier; import java.util.function.UnaryOperator; import javax.cache.configuration.CacheEntryListenerConfiguration; import javax.cache.configuration.Factory; @@ -67,21 +66,6 @@ public boolean putIfAbsent(Object key, Object value) { return jCache.putIfAbsent(key, value); } - @Override - public T computeIfAbsent(Object key, Supplier valueSupplier) { - Object currentValue = jCache.get(key); - if (currentValue != null) { - //noinspection unchecked - return (T) currentValue; - } - T newValue = valueSupplier.get(); - if (newValue == null) { - throw new IllegalStateException("Value Supplier of Cache produced a null value for key [" + key + "]!"); - } - jCache.put(key, newValue); - return newValue; - } - @Override public boolean remove(Object key) { return jCache.remove(key); From f6419765c40bf091c9ad6bc3e271288e3437c083 Mon Sep 17 00:00:00 2001 From: Steven van Beelen Date: Mon, 19 Dec 2022 14:04:07 +0100 Subject: [PATCH 11/11] Fine tune computeIfAbsent implementation - Reuse existingValue instead of retrieving it from the entry. - Use ObjectUtils.getOrDefault #2517 --- .../org/axonframework/common/caching/WeakReferenceCache.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java index 58ea1cf67a..f38bb0cef3 100644 --- a/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java +++ b/messaging/src/main/java/org/axonframework/common/caching/WeakReferenceCache.java @@ -17,6 +17,7 @@ package org.axonframework.common.caching; import org.axonframework.common.Assert; +import org.axonframework.common.ObjectUtils; import org.axonframework.common.Registration; import java.lang.ref.Reference; @@ -111,10 +112,10 @@ public boolean putIfAbsent(Object key, Object value) { public T computeIfAbsent(Object key, Supplier valueSupplier) { purgeItems(); Entry currentEntry = cache.get(key); - Object existingValue = currentEntry != null ? currentEntry.get() : null; + Object existingValue = ObjectUtils.getOrDefault(currentEntry, Entry::get, null); if (existingValue != null) { //noinspection unchecked - return (T) currentEntry.get(); + return (T) existingValue; } T newValue = valueSupplier.get(); if (newValue == null) {