Revert "Improve allowNullValue handling when a null value is provided"

This reverts commit fd568f3e96.

spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheManagerTests.java

@@ -102,7 +102,14 @@ public class CaffeineCacheManagerTests {
 		assertEquals("value1", cache1x.get("key1").get());
 		cache1x.put("key2", 2);
 		assertEquals(2, cache1x.get("key2").get());
-		
+		try {
+			cache1x.put("key3", null);
+			fail("Should have thrown NullPointerException");
+		}
+		catch (NullPointerException ex) {
+			// expected
+		}
+
 		cm.setAllowNullValues(true);
 		Cache cache1y = cm.getCache("c1");
 

spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -20,7 +20,7 @@ import com.github.benmanes.caffeine.cache.Caffeine;
 import org.junit.Before;
 import org.junit.Test;
 
-import org.springframework.cache.AbstractValueAdaptingCacheTests;
+import org.springframework.cache.AbstractCacheTests;
 import org.springframework.cache.Cache;
 
 import static org.junit.Assert.*;
@@ -29,31 +29,21 @@ import static org.junit.Assert.*;
  * @author Ben Manes
  * @author Stephane Nicoll
  */
-public class CaffeineCacheTests extends AbstractValueAdaptingCacheTests<CaffeineCache> {
+public class CaffeineCacheTests extends AbstractCacheTests<CaffeineCache> {
 
 	private com.github.benmanes.caffeine.cache.Cache<Object, Object> nativeCache;
 
 	private CaffeineCache cache;
 
-	private CaffeineCache cacheNoNull;
-
 	@Before
 	public void setUp() {
 		nativeCache = Caffeine.newBuilder().build();
 		cache = new CaffeineCache(CACHE_NAME, nativeCache);
-		com.github.benmanes.caffeine.cache.Cache<Object, Object> nativeCacheNoNull
-				= Caffeine.newBuilder().build();
-		cacheNoNull =  new CaffeineCache(CACHE_NAME_NO_NULL, nativeCacheNoNull, false);
 	}
 
 	@Override
 	protected CaffeineCache getCache() {
-		return getCache(true);
-	}
-
-	@Override
-	protected CaffeineCache getCache(boolean allowNull) {
-		return allowNull ? this.cache : this.cacheNoNull;
+		return cache;
 	}
 
 	@Override

spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheManagerTests.java

@@ -101,7 +101,14 @@ public class GuavaCacheManagerTests {
 		assertEquals("value1", cache1x.get("key1").get());
 		cache1x.put("key2", 2);
 		assertEquals(2, cache1x.get("key2").get());
-		
+		try {
+			cache1x.put("key3", null);
+			fail("Should have thrown NullPointerException");
+		}
+		catch (NullPointerException ex) {
+			// expected
+		}
+
 		cm.setAllowNullValues(true);
 		Cache cache1y = cm.getCache("c1");
 

spring-context-support/src/test/java/org/springframework/cache/jcache/JCacheEhCacheApiTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -25,12 +25,12 @@ import javax.cache.spi.CachingProvider;
 import org.junit.After;
 import org.junit.Before;
 
-import org.springframework.cache.AbstractValueAdaptingCacheTests;
+import org.springframework.cache.AbstractCacheTests;
 
 /**
  * @author Stephane Nicoll
  */
-public class JCacheEhCacheApiTests extends AbstractValueAdaptingCacheTests<JCacheCache> {
+public class JCacheEhCacheApiTests extends AbstractCacheTests<JCacheCache> {
 
 	private CacheManager cacheManager;
 
@@ -38,19 +38,13 @@ public class JCacheEhCacheApiTests extends AbstractValueAdaptingCacheTests<JCach
 
 	private JCacheCache cache;
 
-	private JCacheCache cacheNoNull;
-
 
 	@Before
 	public void setup() {
 		this.cacheManager = getCachingProvider().getCacheManager();
 		this.cacheManager.createCache(CACHE_NAME, new MutableConfiguration<>());
-		this.cacheManager.createCache(CACHE_NAME_NO_NULL, new MutableConfiguration<>());
 		this.nativeCache = this.cacheManager.getCache(CACHE_NAME);
 		this.cache = new JCacheCache(this.nativeCache);
-		Cache<Object, Object> nativeCacheNoNull =
-				this.cacheManager.getCache(CACHE_NAME_NO_NULL);
-		this.cacheNoNull = new JCacheCache(nativeCacheNoNull, false);
 	}
 
 	protected CachingProvider getCachingProvider() {
@@ -64,14 +58,10 @@ public class JCacheEhCacheApiTests extends AbstractValueAdaptingCacheTests<JCach
 		}
 	}
 
-	@Override
-	protected JCacheCache getCache() {
-		return getCache(true);
-	}
 
 	@Override
-	protected JCacheCache getCache(boolean allowNull) {
-		return allowNull ? this.cache : this.cacheNoNull;
+	protected JCacheCache getCache() {
+		return this.cache;
 	}
 
 	@Override

spring-context/src/main/java/org/springframework/cache/support/AbstractValueAdaptingCache.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -95,13 +95,8 @@ public abstract class AbstractValueAdaptingCache implements Cache {
 	 * @return the value to store
 	 */
 	protected Object toStoreValue(Object userValue) {
-		if (userValue == null) {
-			if (this.allowNullValues) {
-				return NullValue.INSTANCE;
-			}
-			throw new IllegalArgumentException(
-					String.format("Cache '%s' is configured to not allow null " +
-							"values but null was provided", getName()));
+		if (this.allowNullValues && userValue == null) {
+			return NullValue.INSTANCE;
 		}
 		return userValue;
 	}

spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2015 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.

spring-context/src/test/java/org/springframework/cache/AbstractValueAdaptingCacheTests.java

@@ -1,50 +0,0 @@
-/*
- * Copyright 2002-2017 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.
- * 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.springframework.cache;
-
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
-
-import org.springframework.cache.support.AbstractValueAdaptingCache;
-
-/**
- * @author Stephane Nicoll
- */
-public abstract class AbstractValueAdaptingCacheTests<T extends AbstractValueAdaptingCache>
-		extends AbstractCacheTests<T>  {
-
-	@Rule
-	public ExpectedException thrown = ExpectedException.none();
-
-	protected final static String CACHE_NAME_NO_NULL = "testCacheNoNull";
-
-	protected abstract T getCache(boolean allowNull);
-
-	@Test
-	public void testCachePutNullValueAllowNullFalse() {
-		T cache = getCache(false);
-		String key = createRandomKey();
-
-		this.thrown.expect(IllegalArgumentException.class);
-		this.thrown.expectMessage(CACHE_NAME_NO_NULL);
-		this.thrown.expectMessage(
-				"is configured to not allow null values but null was provided");
-		cache.put(key, null);
-	}
-
-}

spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -104,7 +104,14 @@ public class ConcurrentMapCacheManagerTests {
 		assertEquals("value1", cache1x.get("key1").get());
 		cache1x.put("key2", 2);
 		assertEquals(2, cache1x.get("key2").get());
-		
+		try {
+			cache1x.put("key3", null);
+			fail("Should have thrown NullPointerException");
+		}
+		catch (NullPointerException ex) {
+			// expected
+		}
+
 		cm.setAllowNullValues(true);
 		Cache cache1y = cm.getCache("c1");
 

spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -25,7 +25,7 @@ import java.util.concurrent.ConcurrentMap;
 import org.junit.Before;
 import org.junit.Test;
 
-import org.springframework.cache.AbstractValueAdaptingCacheTests;
+import org.springframework.cache.AbstractCacheTests;
 import org.springframework.core.serializer.support.SerializationDelegate;
 
 import static org.junit.Assert.*;
@@ -35,35 +35,23 @@ import static org.junit.Assert.*;
  * @author Juergen Hoeller
  * @author Stephane Nicoll
  */
-public class ConcurrentMapCacheTests
-		extends AbstractValueAdaptingCacheTests<ConcurrentMapCache> {
+public class ConcurrentMapCacheTests extends AbstractCacheTests<ConcurrentMapCache> {
 
 	protected ConcurrentMap<Object, Object> nativeCache;
 
 	protected ConcurrentMapCache cache;
 
-	protected ConcurrentMap<Object, Object> nativeCacheNoNull;
-
-	protected ConcurrentMapCache cacheNoNull;
-
 
 	@Before
 	public void setUp() throws Exception {
-		this.nativeCache = new ConcurrentHashMap<Object, Object>();
-		this.cache = new ConcurrentMapCache(CACHE_NAME, this.nativeCache, true);
-		this.nativeCacheNoNull = new ConcurrentHashMap<Object, Object>();
-		this.cacheNoNull = new ConcurrentMapCache(CACHE_NAME_NO_NULL,
-				this.nativeCacheNoNull, false);
-		this.cache.clear();
+		nativeCache = new ConcurrentHashMap<Object, Object>();
+		cache = new ConcurrentMapCache(CACHE_NAME, nativeCache, true);
+		cache.clear();
 	}
 
 	@Override
 	protected ConcurrentMapCache getCache() {
-		return getCache(true);
-	}
-
-	@Override protected ConcurrentMapCache getCache(boolean allowNull) {
-		return allowNull ? this.cache : this.cacheNoNull;
+		return this.cache;
 	}
 
 	@Override
@@ -96,9 +84,9 @@ public class ConcurrentMapCacheTests
 	public void testNonSerializableContent() {
 		ConcurrentMapCache serializeCache = createCacheWithStoreByValue();
 
-		this.thrown.expect(IllegalArgumentException.class);
-		this.thrown.expectMessage("Failed to serialize");
-		this.thrown.expectMessage(this.cache.getClass().getName());
+		thrown.expect(IllegalArgumentException.class);
+		thrown.expectMessage("Failed to serialize");
+		thrown.expectMessage(this.cache.getClass().getName());
 		serializeCache.put(createRandomKey(), this.cache);
 	}
 
@@ -108,15 +96,15 @@ public class ConcurrentMapCacheTests
 
 		String key = createRandomKey();
 		this.nativeCache.put(key, "Some garbage");
-		this.thrown.expect(IllegalArgumentException.class);
-		this.thrown.expectMessage("Failed to deserialize");
-		this.thrown.expectMessage("Some garbage");
+		thrown.expect(IllegalArgumentException.class);
+		thrown.expectMessage("Failed to deserialize");
+		thrown.expectMessage("Some garbage");
 		serializeCache.get(key);
 	}
 
 
 	private ConcurrentMapCache createCacheWithStoreByValue() {
-		return new ConcurrentMapCache(CACHE_NAME, this.nativeCache, true,
+		return new ConcurrentMapCache(CACHE_NAME, nativeCache, true,
 				new SerializationDelegate(ConcurrentMapCacheTests.class.getClassLoader()));
 	}