From 8ee149e6e4816ce69591dd7bcd6839d698493314 Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 11 Dec 2013 16:04:52 -0800 Subject: [PATCH] Partial Fix and Refactoring for JIRA issue SGF-236 enabling the nested element syntax to function properly. --- .../data/gemfire/RegionFactoryBean.java | 13 +-- .../data/gemfire/RegionLookupFactoryBean.java | 30 +++++- .../gemfire/config/LookupRegionParser.java | 10 +- .../data/gemfire/LookupSubRegionTest.java | 93 +++++++++++++++++++ .../data/gemfire/lookupSubRegion.xml | 40 ++++++++ src/test/resources/subregion-cache.xml | 17 ++++ 6 files changed, 186 insertions(+), 17 deletions(-) create mode 100644 src/test/java/org/springframework/data/gemfire/LookupSubRegionTest.java create mode 100644 src/test/resources/org/springframework/data/gemfire/lookupSubRegion.xml create mode 100644 src/test/resources/subregion-cache.xml diff --git a/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java index fb057bec..93c4b343 100644 --- a/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java @@ -77,8 +77,6 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple private Object[] asyncEventQueues; private Object[] gatewaySenders; - private Region parent; - private RegionAttributes attributes; private Resource snapshot; @@ -169,13 +167,13 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple // get underlying AttributesFactory postProcess(findAttributesFactory(regionFactory)); - Region region = (this.parent != null ? regionFactory.createSubregion(parent, regionName) + Region region = (getParent() != null ? regionFactory.createSubregion(getParent(), regionName) : regionFactory.create(regionName)); if (log.isInfoEnabled()) { - if (parent != null) { + if (getParent() != null) { log.info(String.format("Created new Cache sub-Region [%1$s] under parent Region [%2$s].", - regionName, parent.getName())); + regionName, getParent().getName())); } else { log.info(String.format("Created new Cache Region [%1$s].", regionName)); @@ -249,7 +247,6 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple */ @SuppressWarnings("unused") protected void postProcess(Region region) { - } @Override @@ -374,10 +371,6 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple this.hubId = hubId; } - public void setParent(Region parent) { - this.parent = parent; - } - public void setPersistent(boolean persistent) { this.persistent = persistent; } diff --git a/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java b/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java index b4e18954..1039bcbe 100644 --- a/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java @@ -35,12 +35,14 @@ import com.gemstone.gemfire.cache.Region; * @author Costin Leau * @author John Blum */ +@SuppressWarnings("unused") public class RegionLookupFactoryBean implements FactoryBean>, InitializingBean, BeanNameAware { protected final Log log = LogFactory.getLog(getClass()); private GemFireCache cache; + private Region parent; private Region region; private String beanName; @@ -56,7 +58,13 @@ public class RegionLookupFactoryBean implements FactoryBean>, Assert.hasText(regionName, "The 'regionName', 'name' or 'beanName' property must be set."); synchronized (cache) { - region = cache.getRegion(regionName); + //region = (getParent() != null ? getParent().getSubregion(regionName) : cache.getRegion(regionName)); + if (getParent() != null) { + region = getParent().getSubregion(regionName); + } + else { + region = cache.getRegion(regionName); + } if (region != null) { log.info(String.format("Retrieved Region [%1$s] from Cache [%2$s].", regionName, cache.getName())); @@ -126,6 +134,26 @@ public class RegionLookupFactoryBean implements FactoryBean>, this.name = name; } + /** + * Sets a reference to the parent Region if this FactoryBean represents a GemFire Cache Sub-Region. + *

+ * @param parent a reference to the parent Region if this Region is a Sub-Region. + * @see com.gemstone.gemfire.cache.Region + */ + public void setParent(Region parent) { + this.parent = parent; + } + + /** + * Gets a reference to the parent Region if this FactoryBean represents a GemFire Cache Sub-Region. + *

+ * @return a reference to the parent Region or null if this Region is not a Sub-Region. + * @see com.gemstone.gemfire.cache.Region + */ + protected Region getParent() { + return parent; + } + /** * Sets the name of the Cache Region as expected by GemFire. If no Region is found with the given name, a new one * will be created. If no name is given, the value of the 'name' property will be used. diff --git a/src/main/java/org/springframework/data/gemfire/config/LookupRegionParser.java b/src/main/java/org/springframework/data/gemfire/config/LookupRegionParser.java index 2d20d6e2..c7a271c9 100644 --- a/src/main/java/org/springframework/data/gemfire/config/LookupRegionParser.java +++ b/src/main/java/org/springframework/data/gemfire/config/LookupRegionParser.java @@ -41,15 +41,13 @@ class LookupRegionParser extends AbstractRegionParser { boolean subRegion) { super.doParse(element, builder); + String resolvedCacheRef = ParsingUtils.resolveCacheReference(element.getAttribute("cache-ref")); + + builder.addPropertyReference("cache", resolvedCacheRef); ParsingUtils.setPropertyValue(element, builder, "name", "name"); if (!subRegion) { - String cacheRef = element.getAttribute("cache-ref"); - builder.addPropertyReference("cache", (StringUtils.hasText(cacheRef) ? cacheRef - : GemfireConstants.DEFAULT_GEMFIRE_CACHE_NAME)); - } - else { - builder.addPropertyValue("lookupOnly", true); + parseSubRegions(element, parserContext, resolvedCacheRef); } } diff --git a/src/test/java/org/springframework/data/gemfire/LookupSubRegionTest.java b/src/test/java/org/springframework/data/gemfire/LookupSubRegionTest.java new file mode 100644 index 00000000..8b7a4cf9 --- /dev/null +++ b/src/test/java/org/springframework/data/gemfire/LookupSubRegionTest.java @@ -0,0 +1,93 @@ +/* + * 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; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContext; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import com.gemstone.gemfire.cache.Region; + +/** + * The LookupSubRegionTest class is a test suite of test cases testing the contract and functionality of Region lookups + * using Spring Data GemFire configuration and GemFire native cache.xml. + *

+ * @author John Blum + * @see org.junit.Test + * @see org.springframework.context.ApplicationContext + * @see org.springframework.test.context.ContextConfiguration + * @see org.springframework.test.context.junit4.SpringJUnit4ClassRunner + * @see com.gemstone.gemfire.cache.Region + * @since 1.3.3 + * @since 7.0.1 (GemFire) + */ +@ContextConfiguration("lookupSubRegion.xml") +@RunWith(SpringJUnit4ClassRunner.class) +@SuppressWarnings("unused") +public class LookupSubRegionTest { + + @Autowired + private ApplicationContext context; + + protected void assertRegionExists(final String expectedRegionName, final String expectedRegionPath, final Region region) { + assertNotNull(String.format("The Region with name (%1$s) at path (%2$s) was null!", + expectedRegionName, expectedRegionPath), region); + assertEquals(String.format("Expected Region name of %1$s; but was %2$s!", expectedRegionName, region.getName()), + expectedRegionName, region.getName()); + assertEquals(String.format("Expected Region path of %1$s; but was %2$s!", expectedRegionPath, region.getFullPath()), + expectedRegionPath, region.getFullPath()); + } + + @Test + public void testDirectLookup() { + Region accounts = context.getBean("/Customers/Accounts", Region.class); + + assertRegionExists("Accounts", "/Customers/Accounts", accounts); + assertFalse(context.containsBean("Customers/Accounts")); + + Region items = context.getBean("Customers/Accounts/Orders/Items", Region.class); + + assertRegionExists("Items", "/Customers/Accounts/Orders/Items", items); + assertFalse(context.containsBean("/Customers/Accounts/Orders/Items")); + } + + @Test + public void testNestedLookup() { + Region parent = context.getBean("Parent", Region.class); + + assertRegionExists("Parent", "/Parent", parent); + assertFalse(context.containsBean("/Parent")); + + Region child = context.getBean("/Parent/Child", Region.class); + + assertRegionExists("Child", "/Parent/Child", child); + assertFalse(context.containsBean("Parent/Child")); + + Region grandchild = context.getBean("/Parent/Child/Grandchild", Region.class); + + assertRegionExists("Grandchild", "/Parent/Child/Grandchild", grandchild); + assertFalse(context.containsBean("Parent/Child/Grandchild")); + } + +} diff --git a/src/test/resources/org/springframework/data/gemfire/lookupSubRegion.xml b/src/test/resources/org/springframework/data/gemfire/lookupSubRegion.xml new file mode 100644 index 00000000..6587c3ae --- /dev/null +++ b/src/test/resources/org/springframework/data/gemfire/lookupSubRegion.xml @@ -0,0 +1,40 @@ + + + + + springGemFireLookupSubRegionTest + localhost[11235] + config + 0 + localhost[11235] + + + + + + + + + + + + + + + + + + + diff --git a/src/test/resources/subregion-cache.xml b/src/test/resources/subregion-cache.xml new file mode 100644 index 00000000..2f05449a --- /dev/null +++ b/src/test/resources/subregion-cache.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + +