diff --git a/org.springframework.core/src/main/java/org/springframework/util/CachingMapDecorator.java b/org.springframework.core/src/main/java/org/springframework/util/CachingMapDecorator.java index f758e412a2f..d0a080fed68 100644 --- a/org.springframework.core/src/main/java/org/springframework/util/CachingMapDecorator.java +++ b/org.springframework.core/src/main/java/org/springframework/util/CachingMapDecorator.java @@ -22,6 +22,7 @@ import java.lang.ref.WeakReference; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; @@ -33,7 +34,7 @@ import java.util.WeakHashMap; * A simple decorator for a Map, encapsulating the workflow for caching * expensive values in a target Map. Supports caching weak or strong keys. * - *

This class is also an abstract template. Caching Map implementations + *

This class is an abstract template. Caching Map implementations * should subclass and override the create(key) method which * encapsulates expensive creation of a new object. * @@ -41,9 +42,9 @@ import java.util.WeakHashMap; * @author Juergen Hoeller * @since 1.2.2 */ -public class CachingMapDecorator implements Map, Serializable { +public abstract class CachingMapDecorator implements Map, Serializable { - protected static Object NULL_VALUE = new Object(); + private static Object NULL_VALUE = new Object(); private final Map targetMap; @@ -150,7 +151,16 @@ public class CachingMapDecorator implements Map, Serializable { } public V remove(Object key) { - return unwrapIfNecessary(this.targetMap.remove(key)); + return unwrapReturnValue(this.targetMap.remove(key)); + } + + @SuppressWarnings("unchecked") + private V unwrapReturnValue(Object value) { + Object returnValue = value; + if (returnValue instanceof Reference) { + returnValue = ((Reference) returnValue).get(); + } + return (returnValue == NULL_VALUE ? null : (V) returnValue); } public void putAll(Map map) { @@ -186,8 +196,16 @@ public class CachingMapDecorator implements Map, Serializable { @SuppressWarnings("unchecked") private Collection valuesCopy() { LinkedList values = new LinkedList(); - for (Object value : this.targetMap.values()) { - values.add(value instanceof Reference ? ((Reference) value).get() : (V) value); + for (Iterator it = this.targetMap.values().iterator(); it.hasNext();) { + Object value = it.next(); + if (value instanceof Reference) { + value = ((Reference) value).get(); + if (value == null) { + it.remove(); + continue; + } + } + values.add(value == NULL_VALUE ? null : (V) value); } return values; } @@ -203,10 +221,20 @@ public class CachingMapDecorator implements Map, Serializable { } } + @SuppressWarnings("unchecked") private Set> entryCopy() { Map entries = new LinkedHashMap(); - for (Entry entry : this.targetMap.entrySet()) { - entries.put(entry.getKey(), unwrapIfNecessary(entry.getValue())); + for (Iterator> it = this.targetMap.entrySet().iterator(); it.hasNext();) { + Entry entry = it.next(); + Object value = entry.getValue(); + if (value instanceof Reference) { + value = ((Reference) value).get(); + if (value == null) { + it.remove(); + continue; + } + } + entries.put(entry.getKey(), value == NULL_VALUE ? null : (V) value); } return entries.entrySet(); } @@ -223,13 +251,13 @@ public class CachingMapDecorator implements Map, Serializable { newValue = NULL_VALUE; } else if (useWeakValue(key, value)) { - newValue = new WeakReference(value); + newValue = new WeakReference(newValue); } - return unwrapIfNecessary(this.targetMap.put(key, newValue)); + return unwrapReturnValue(this.targetMap.put(key, newValue)); } /** - * Decide whether use a weak reference for the value of + * Decide whether to use a weak reference for the value of * the given key-value pair. * @param key the candidate key * @param value the candidate value @@ -252,27 +280,15 @@ public class CachingMapDecorator implements Map, Serializable { @SuppressWarnings("unchecked") public V get(Object key) { Object value = this.targetMap.get(key); - if (value == null) { - V newVal = create((K) key); - if (newVal != null) { - put((K) key, newVal); - } - return newVal; - } - return unwrapIfNecessary(value); - } - - @SuppressWarnings("unchecked") - private V unwrapIfNecessary(Object value) { if (value instanceof Reference) { - return ((Reference) value).get(); + value = ((Reference) value).get(); } - else if (value != null) { - return (V) value; - } - else { - return null; + if (value == null) { + V newValue = create((K) key); + put((K) key, newValue); + return newValue; } + return (value == NULL_VALUE ? null : (V) value); } /** @@ -281,9 +297,7 @@ public class CachingMapDecorator implements Map, Serializable { * @param key the cache key * @see #get(Object) */ - protected V create(K key) { - return null; - } + protected abstract V create(K key); @Override diff --git a/org.springframework.core/src/test/java/org/springframework/util/CachingMapDecoratorTests.java b/org.springframework.core/src/test/java/org/springframework/util/CachingMapDecoratorTests.java index 04623853620..1a1a7ef85dd 100644 --- a/org.springframework.core/src/test/java/org/springframework/util/CachingMapDecoratorTests.java +++ b/org.springframework.core/src/test/java/org/springframework/util/CachingMapDecoratorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2005 the original author or authors. + * Copyright 2002-2009 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,41 +16,91 @@ package org.springframework.util; +import java.util.Collection; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + import junit.framework.TestCase; /** * @author Keith Donald + * @author Juergen Hoeller */ public class CachingMapDecoratorTests extends TestCase { public void testValidCache() { MyCachingMap cache = new MyCachingMap(); - Object value; + + assertFalse(cache.containsKey("value key")); + assertFalse(cache.containsValue("expensive value to cache")); + Object value = cache.get("value key"); + assertTrue(cache.createCalled()); + assertEquals(value, "expensive value to cache"); + assertTrue(cache.containsKey("value key")); + assertTrue(cache.containsValue("expensive value to cache")); + + assertFalse(cache.containsKey("value key 2")); + value = cache.get("value key 2"); + assertTrue(cache.createCalled()); + assertEquals(value, "expensive value to cache"); + assertTrue(cache.containsKey("value key 2")); value = cache.get("value key"); - assertTrue(cache.createCalled()); + assertFalse(cache.createCalled()); assertEquals(value, "expensive value to cache"); cache.get("value key 2"); + assertFalse(cache.createCalled()); + assertEquals(value, "expensive value to cache"); + + assertFalse(cache.containsKey(null)); + assertFalse(cache.containsValue(null)); + value = cache.get(null); assertTrue(cache.createCalled()); + assertNull(value); + assertTrue(cache.containsKey(null)); + assertTrue(cache.containsValue(null)); - value = cache.get("value key"); - assertEquals(cache.createCalled(), false); - assertEquals(value, "expensive value to cache"); + value = cache.get(null); + assertFalse(cache.createCalled()); + assertNull(value); - cache.get("value key 2"); - assertEquals(cache.createCalled(), false); - assertEquals(value, "expensive value to cache"); + Set keySet = cache.keySet(); + assertEquals(3, keySet.size()); + assertTrue(keySet.contains("value key")); + assertTrue(keySet.contains("value key 2")); + assertTrue(keySet.contains(null)); + + Collection values = cache.values(); + assertEquals(3, values.size()); + assertTrue(values.contains("expensive value to cache")); + assertTrue(values.contains(null)); + + Set> entrySet = cache.entrySet(); + assertEquals(3, entrySet.size()); + keySet = new HashSet(); + values = new HashSet(); + for (Map.Entry entry : entrySet) { + keySet.add(entry.getKey()); + values.add(entry.getValue()); + } + assertTrue(keySet.contains("value key")); + assertTrue(keySet.contains("value key 2")); + assertTrue(keySet.contains(null)); + assertEquals(2, values.size()); + assertTrue(values.contains("expensive value to cache")); + assertTrue(values.contains(null)); } - private static class MyCachingMap extends CachingMapDecorator { + private static class MyCachingMap extends CachingMapDecorator { private boolean createCalled; - protected Object create(Object key) { + protected String create(String key) { createCalled = true; - return "expensive value to cache"; + return (key != null ? "expensive value to cache" : null); } public boolean createCalled() {