From 26bebdafe451c5992398c8c69e06de11845e6666 Mon Sep 17 00:00:00 2001 From: Tomas Tulka Date: Fri, 23 Nov 2018 00:51:19 -0700 Subject: [PATCH] [COLLECTIONS-703] The PassiveExpiringMap#put() method should return the previous record only if not expired. --- src/changes/changes.xml | 3 ++ .../collections4/map/PassiveExpiringMap.java | 3 ++ .../map/PassiveExpiringMapTest.java | 30 ++++++++++++------- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 4881baa47..28a02c9a2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -39,6 +39,9 @@ StackOverflowError in SetUniqueList.add() when it receives itself. + + The PassiveExpiringMap#put() method should return the previous record only if not expired. + diff --git a/src/main/java/org/apache/commons/collections4/map/PassiveExpiringMap.java b/src/main/java/org/apache/commons/collections4/map/PassiveExpiringMap.java index d8565e67e..76a0d842c 100644 --- a/src/main/java/org/apache/commons/collections4/map/PassiveExpiringMap.java +++ b/src/main/java/org/apache/commons/collections4/map/PassiveExpiringMap.java @@ -430,6 +430,9 @@ public class PassiveExpiringMap */ @Override public V put(final K key, final V value) { + // remove the previous record + removeIfExpired(key, now()); + // record expiration time of new entry final long expirationTime = expiringPolicy.expirationTime(key, value); expirationMap.put(key, Long.valueOf(expirationTime)); diff --git a/src/test/java/org/apache/commons/collections4/map/PassiveExpiringMapTest.java b/src/test/java/org/apache/commons/collections4/map/PassiveExpiringMapTest.java index f09105c47..0dee5986d 100644 --- a/src/test/java/org/apache/commons/collections4/map/PassiveExpiringMapTest.java +++ b/src/test/java/org/apache/commons/collections4/map/PassiveExpiringMapTest.java @@ -181,6 +181,15 @@ public class PassiveExpiringMapTest extends AbstractMapTest { assertEquals(3, m.entrySet().size()); } + public void testExpiration() { + validateExpiration(new PassiveExpiringMap(500), 500); + validateExpiration(new PassiveExpiringMap(1000), 1000); + validateExpiration(new PassiveExpiringMap<>( + new PassiveExpiringMap.ConstantTimeToLiveExpirationPolicy(500)), 500); + validateExpiration(new PassiveExpiringMap<>( + new PassiveExpiringMap.ConstantTimeToLiveExpirationPolicy(1, TimeUnit.SECONDS)), 1000); + } + public void testGet() { final Map m = makeTestMap(); assertNull(m.get(Integer.valueOf(1))); @@ -208,6 +217,16 @@ public class PassiveExpiringMapTest extends AbstractMapTest { assertEquals(3, m.keySet().size()); } + public void testPut() { + final Map m = makeTestMap(); + assertNull(m.put(Integer.valueOf(1), "ONE")); + assertEquals("two", m.put(Integer.valueOf(2), "TWO")); + assertNull(m.put(Integer.valueOf(3), "THREE")); + assertEquals("four", m.put(Integer.valueOf(4), "FOUR")); + assertNull(m.put(Integer.valueOf(5), "FIVE")); + assertEquals("six", m.put(Integer.valueOf(6), "SIX")); + } + public void testSize() { final Map m = makeTestMap(); assertEquals(3, m.size()); @@ -225,15 +244,6 @@ public class PassiveExpiringMapTest extends AbstractMapTest { assertNull(m.get("a")); } - public void testExpiration() { - validateExpiration(new PassiveExpiringMap(500), 500); - validateExpiration(new PassiveExpiringMap(1000), 1000); - validateExpiration(new PassiveExpiringMap<>( - new PassiveExpiringMap.ConstantTimeToLiveExpirationPolicy(500)), 500); - validateExpiration(new PassiveExpiringMap<>( - new PassiveExpiringMap.ConstantTimeToLiveExpirationPolicy(1, TimeUnit.SECONDS)), 1000); - } - private void validateExpiration(final Map map, final long timeout) { map.put("a", "b"); @@ -247,5 +257,5 @@ public class PassiveExpiringMapTest extends AbstractMapTest { assertNull(map.get("a")); } - + }