Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Improve allowNullValue handling when a null value is provided

Browse Source 
This commit improves `AbstractValueAdaptingCache` to throw a dedicated
exception if `allowNullValues` is `false` and a `null` value is provided
anyway. This avoid a lower-level exception from the cache library that
will miss some context.

Issue: SPR-15173
This commit is contained in:
Stephane Nicoll
2017-02-20 15:58:04 +01:00
parent 45b27a85f4
commit fd568f3e96
9 changed files with 118 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheManagerTests.java vendored
View File

@@ -102,14 +102,7 @@ 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");

18
spring-context-support/src/test/java/org/springframework/cache/caffeine/CaffeineCacheTests.java vendored
View File

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

9
spring-context-support/src/test/java/org/springframework/cache/guava/GuavaCacheManagerTests.java vendored
View File

@@ -101,14 +101,7 @@ 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");

20
spring-context-support/src/test/java/org/springframework/cache/jcache/JCacheEhCacheApiTests.java vendored
View File

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

11
spring-context/src/main/java/org/springframework/cache/support/AbstractValueAdaptingCache.java vendored
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* 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.
@@ -95,8 +95,13 @@ public abstract class AbstractValueAdaptingCache implements Cache {
* @return the value to store
*/
protected Object toStoreValue(Object userValue) {
if (this.allowNullValues && userValue == null) {
return NullValue.INSTANCE;
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()));
}
return userValue;
}

2
spring-context/src/test/java/org/springframework/cache/AbstractCacheTests.java vendored
View File

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

50
spring-context/src/test/java/org/springframework/cache/AbstractValueAdaptingCacheTests.java vendored Normal file
View File

@@ -0,0 +1,50 @@
/*
* 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);
}
}

11
spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheManagerTests.java vendored
View File

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

40
spring-context/src/test/java/org/springframework/cache/concurrent/ConcurrentMapCacheTests.java vendored
View File

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