From 278199d46764ae9861772c0983d692685da36233 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 20 Feb 2017 16:36:23 +0100 Subject: [PATCH] Revert "Improve allowNullValue handling when a null value is provided" This reverts commit fd568f3e9680b816767754d2852cfb38f19e51c2. --- .../caffeine/CaffeineCacheManagerTests.java | 9 +++- .../cache/caffeine/CaffeineCacheTests.java | 18 ++----- .../cache/guava/GuavaCacheManagerTests.java | 9 +++- .../cache/jcache/JCacheEhCacheApiTests.java | 20 ++------ .../support/AbstractValueAdaptingCache.java | 11 ++-- .../cache/AbstractCacheTests.java | 2 +- .../AbstractValueAdaptingCacheTests.java | 50 ------------------- .../ConcurrentMapCacheManagerTests.java | 11 +++- .../concurrent/ConcurrentMapCacheTests.java | 40 ++++++--------- 9 files changed, 52 insertions(+), 118 deletions(-) delete mode 100644 spring-context/src/test/java/org/springframework/cache/AbstractValueAdaptingCacheTests.java diff --git a/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheManagerTests.java b/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheManagerTests.java index 59b6a0049e7..ca4fc2cedfb 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheManagerTests.java +++ b/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"); diff --git a/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java b/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java index 0fbed8b14ce..5ede180c4fb 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java +++ b/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 { +public class CaffeineCacheTests extends AbstractCacheTests { private com.github.benmanes.caffeine.cache.Cache 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 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 diff --git a/spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheManagerTests.java b/spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheManagerTests.java index 3fd4437ec01..0f7f70beaa6 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheManagerTests.java +++ b/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"); diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/JCacheEhCacheApiTests.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/JCacheEhCacheApiTests.java index c2086809faa..bfb5bce39f7 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/JCacheEhCacheApiTests.java +++ b/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 { +public class JCacheEhCacheApiTests extends AbstractCacheTests { private CacheManager cacheManager; @@ -38,19 +38,13 @@ public class JCacheEhCacheApiTests extends AbstractValueAdaptingCacheTests()); - this.cacheManager.createCache(CACHE_NAME_NO_NULL, new MutableConfiguration<>()); this.nativeCache = this.cacheManager.getCache(CACHE_NAME); this.cache = new JCacheCache(this.nativeCache); - Cache 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 - extends AbstractCacheTests { - - @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); - } - -} diff --git a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java index 92e8754b750..1598b0fa390 100644 --- a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java +++ b/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"); diff --git a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java b/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java index d861239b6bc..6836ba70303 100644 --- a/spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java +++ b/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 { +public class ConcurrentMapCacheTests extends AbstractCacheTests { protected ConcurrentMap nativeCache; protected ConcurrentMapCache cache; - protected ConcurrentMap nativeCacheNoNull; - - protected ConcurrentMapCache cacheNoNull; - @Before public void setUp() throws Exception { - this.nativeCache = new ConcurrentHashMap(); - this.cache = new ConcurrentMapCache(CACHE_NAME, this.nativeCache, true); - this.nativeCacheNoNull = new ConcurrentHashMap(); - this.cacheNoNull = new ConcurrentMapCache(CACHE_NAME_NO_NULL, - this.nativeCacheNoNull, false); - this.cache.clear(); + nativeCache = new ConcurrentHashMap(); + 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())); }