From acdd0c85bf0677b9d411c91f6ad2ce4f4066d9b3 Mon Sep 17 00:00:00 2001 From: John Blum Date: Thu, 18 Sep 2014 16:18:42 -0700 Subject: [PATCH] SGF-327 - Avoid setting null values with GemFire's Cache Region put(key, value) operation when GemFire is used as the caching provider in Spring's Cache Abstraction (@Cacheable). --- .../data/gemfire/support/GemfireCache.java | 13 +- .../gemfire/support/GemfireCacheManager.java | 3 +- .../CachingWithGemFireIntegrationTest.java | 161 ++++++++++++++++++ ...hingWithGemFireIntegrationTest-context.xml | 70 ++++++++ 4 files changed, 240 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest.java create mode 100644 src/test/resources/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest-context.xml diff --git a/src/main/java/org/springframework/data/gemfire/support/GemfireCache.java b/src/main/java/org/springframework/data/gemfire/support/GemfireCache.java index 201690e7..4ef82ffd 100644 --- a/src/main/java/org/springframework/data/gemfire/support/GemfireCache.java +++ b/src/main/java/org/springframework/data/gemfire/support/GemfireCache.java @@ -71,11 +71,11 @@ public class GemfireCache implements Cache { @SuppressWarnings("unchecked") public T get(final Object key, final Class type) { - Object value = region.get(key); if (value != null && type != null && !type.isInstance(value)) { - throw new IllegalStateException("Cached value is not of required type [" + type.getName() + "]: " + value); + throw new IllegalStateException(String.format("Cached value is not of required type [%1$s]: %2$s", + type.getName(), value)); } return (T) value; @@ -83,7 +83,9 @@ public class GemfireCache implements Cache { @SuppressWarnings("unchecked") public void put(final Object key, final Object value) { - region.put(key, value); + if (value != null) { + region.put(key, value); + } } /** @@ -94,8 +96,9 @@ public class GemfireCache implements Cache { */ @SuppressWarnings("unchecked") public ValueWrapper putIfAbsent(Object key, Object value) { - Object existingValue = region.putIfAbsent(key, value); - return existingValue == null ? null : new SimpleValueWrapper(existingValue); + + return (existingValue == null ? null : new SimpleValueWrapper(existingValue)); } + } diff --git a/src/main/java/org/springframework/data/gemfire/support/GemfireCacheManager.java b/src/main/java/org/springframework/data/gemfire/support/GemfireCacheManager.java index 64699ba3..c58e6524 100644 --- a/src/main/java/org/springframework/data/gemfire/support/GemfireCacheManager.java +++ b/src/main/java/org/springframework/data/gemfire/support/GemfireCacheManager.java @@ -84,8 +84,7 @@ public class GemfireCacheManager extends AbstractCacheManager { Cache cache = super.getCache(name); if (cache == null) { - // check the gemfire cache again - // in case the cache was added at runtime + // check the GemFire Cache again in case the Cache (Region) was added at runtime Region region = gemfireCache.getRegion(name); if (region != null) { diff --git a/src/test/java/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest.java b/src/test/java/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest.java new file mode 100644 index 00000000..ae563fbd --- /dev/null +++ b/src/test/java/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest.java @@ -0,0 +1,161 @@ +/* + * Copyright 2010-2013 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.data.gemfire.support; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.Map; +import javax.annotation.PostConstruct; +import javax.annotation.Resource; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cache.annotation.Cacheable; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.util.Assert; + +import com.gemstone.gemfire.cache.Region; + +/** + * The CachingWithGemFireIntegrationTest class is a test suite of test cases testing the contract and functionality + * of Spring Framework's Cache Abstraction using GemFire as a caching provider applied with Spring Data GemFire. + * + * @author John Blum + * @see org.junit.Test + * @see org.junit.runner.RunWith + * @see org.springframework.cache.annotation.Cacheable + * @see org.springframework.test.context.ActiveProfiles + * @see org.springframework.test.context.ContextConfiguration + * @see org.springframework.test.context.junit4.SpringJUnit4ClassRunner + * @since 1.5.1 + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration +@ActiveProfiles("replica") +@SuppressWarnings("unused") +public class CachingWithGemFireIntegrationTest { + + @Autowired + private NamedNumbersService namedNumbersService; + + @Resource(name = "NamedNumbersRegion") + private Region namedNumbersRegion; + + @Test(expected = NullPointerException.class) + public void testRegionCacheHit() { + assertNull(namedNumbersRegion.get("eleven")); + assertFalse(namedNumbersRegion.containsKey("eleven")); + + namedNumbersRegion.put("eleven", 11); + + assertTrue(namedNumbersRegion.containsKey("eleven")); + assertEquals(11, namedNumbersService.get("eleven").intValue()); + assertFalse(namedNumbersService.wasCacheMiss()); + + try { + namedNumbersRegion.put("eleven", null); // GemFire does not accept null values on put(key, value) + } + finally { + assertTrue(namedNumbersRegion.containsKey("eleven")); + assertEquals(11, namedNumbersRegion.get("eleven").intValue()); + assertEquals(11, namedNumbersService.get("eleven").intValue()); + assertFalse(namedNumbersService.wasCacheMiss()); + } + } + + @Test + public void testRegionCaching() { + assertFalse(namedNumbersService.wasCacheMiss()); + assertEquals(1, namedNumbersService.get("one").intValue()); + assertTrue(namedNumbersService.wasCacheMiss()); + assertEquals(1, namedNumbersService.get("one").intValue()); + assertFalse(namedNumbersService.wasCacheMiss()); + assertEquals(2, namedNumbersService.get("two").intValue()); + assertTrue(namedNumbersService.wasCacheMiss()); + assertEquals(2, namedNumbersService.get("two").intValue()); + assertFalse(namedNumbersService.wasCacheMiss()); + assertNull(namedNumbersService.get("twelve")); + assertTrue(namedNumbersService.wasCacheMiss()); + assertNull(namedNumbersService.get("twelve")); + assertTrue(namedNumbersService.wasCacheMiss()); + } + + public static class NamedNumbersService { + + private NamedNumbersInMemoryRepository namedNumbersRepo; + + public final void setNamedNumbersRepo(final NamedNumbersInMemoryRepository namedNumbersRepo) { + Assert.notNull(namedNumbersRepo, "The 'NamedNumbers' Repository must not be null!"); + this.namedNumbersRepo = namedNumbersRepo; + } + + protected NamedNumbersInMemoryRepository getNamedNumbersRepo() { + Assert.state(namedNumbersRepo != null, + "A reference to the 'NamedNumbers' Repository was not properly configured and initialized!"); + return namedNumbersRepo; + } + + @Cacheable("NamedNumbersRegion") + public Integer get(final String namedNumber) { + return getNamedNumbersRepo().get(namedNumber); + } + + public boolean wasCacheMiss() { + return getNamedNumbersRepo().wasCacheMiss(); + } + } + + public static class NamedNumbersInMemoryRepository { + + private volatile boolean cacheMiss; + + private Map namedNumbers; + + @PostConstruct + public void init() { + getNamedNumbers(); + } + + public final void setNamedNumbers(final Map namedNumbers) { + Assert.notNull(namedNumbers, "The reference to the 'NamedNumbers' Map must not be null!"); + this.namedNumbers = namedNumbers; + } + + protected Map getNamedNumbers() { + Assert.state(namedNumbers != null, "The 'NamedNumbers' Map was not properly configured and initialized!"); + return namedNumbers; + } + + public Integer get(final String namedNumber) { + this.cacheMiss = true; + return namedNumbers.get(namedNumber); + } + + public boolean wasCacheMiss() { + boolean localCacheMiss = this.cacheMiss; + this.cacheMiss = false; + return localCacheMiss; + } + } + +} diff --git a/src/test/resources/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest-context.xml b/src/test/resources/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest-context.xml new file mode 100644 index 00000000..b865a24f --- /dev/null +++ b/src/test/resources/org/springframework/data/gemfire/support/CachingWithGemFireIntegrationTest-context.xml @@ -0,0 +1,70 @@ + + + + + SpringGemFireCachingIntegrationTest + 0 + warning + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +