ISPN-8323 Cache.evict(...) throws NPE if entry was already evicted and

an eviction listener is present * Use regular for loop instead of Collectors
infinispan · Sep 26, 2017 · 7afdef5 · 7afdef5
1 parent 9323b8c
commit 7afdef5
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 4 deletions.
diff --git a/core/src/main/java/org/infinispan/notifications/cachelistener/CacheNotifierImpl.java b/core/src/main/java/org/infinispan/notifications/cachelistener/CacheNotifierImpl.java
@@ -33,7 +33,6 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import javax.transaction.Status;
@@ -506,9 +505,11 @@ public void notifyCacheEntriesEvicted(Collection<InternalCacheEntry<? extends K,
          if (isNotificationAllowed(command, cacheEntriesEvictedListeners)) {
             EventImpl<K, V> e = EventImpl.createEvent(cache, CACHE_ENTRY_EVICTED);
             for (CacheEntryListenerInvocation<K, V> listener : cacheEntriesEvictedListeners) {
-               Map<K, V> evictedKeysAndValues = entries.stream().collect(Collectors.toMap(
-                     entry -> convertKey(listener.getKeyEncoder(), listener.getKeyWrapper(), entry.getKey()),
-                     entry -> convertValue(listener.getValueEncoder(), listener.getValueWrapper(), entry.getValue())));
+               Map<K, V> evictedKeysAndValues = new HashMap<>();
+               for (Map.Entry<? extends K, ? extends V> entry : entries) {
+                  evictedKeysAndValues.put(convertKey(listener.getKeyEncoder(), listener.getKeyWrapper(), entry.getKey()),
+                        convertValue(listener.getValueEncoder(), listener.getValueWrapper(), entry.getValue()));
+               }
                e.setEntries(evictedKeysAndValues);
                listener.invoke(e);
             }

diff --git a/core/src/test/java/org/infinispan/api/APINonTxTest.java b/core/src/test/java/org/infinispan/api/APINonTxTest.java
@@ -144,6 +144,9 @@ public void testEvict() {
       assertFalse(cache.keySet().contains(key2));
       assertFalse(cache.values().contains(value));
       assertCacheIsEmpty();
+
+      // We should be fine if we evict a non existent key
+      cache.evict(key1);
    }
 
 

diff --git a/core/src/test/java/org/infinispan/eviction/impl/EvictionFunctionalTest.java b/core/src/test/java/org/infinispan/eviction/impl/EvictionFunctionalTest.java
@@ -92,6 +92,22 @@ public void testSimpleEvictionMaxEntries() throws Exception {
       assertTrue(evictionListener.evictedEntries.size() > CACHE_SIZE);
    }
 
+   public void testEvictNonExistantEntry() {
+      String key = "key";
+      String value = "some-value";
+      cache.put(key, value);
+
+      cache.evict(key);
+
+      assertEquals(1, evictionListener.evictedEntries.size());
+
+      // Make sure if we evict again that it doesn't increase count
+      cache.evict(key);
+
+      // TODO: this seems like a bug, but many tests rely on this - maybe change later
+      assertEquals(2, evictionListener.evictedEntries.size());
+   }
+
    public void testSimpleExpirationMaxIdle() throws Exception {
       for (int i = 0; i < CACHE_SIZE * 2; i++) {
          cache.put("key-" + (i + 1), "value-" + (i + 1), 1, TimeUnit.MILLISECONDS);

diff --git a/core/src/test/java/org/infinispan/notifications/CacheListenerCacheLoaderTest.java b/core/src/test/java/org/infinispan/notifications/CacheListenerCacheLoaderTest.java
@@ -134,6 +134,9 @@ public void testActivatingAndPassivating() {
       assert l.loaded.contains("k");
       assert l.activated.contains("k");
       assert l.passivated.contains("k");
+
+      // We should be fine if we evict a non existent key
+      c.evict("k");
    }