Additional fixes for SGF-203 involving the persistent, data-policy and shortcut attributes of client Regions.

src/main/java/org/springframework/data/gemfire/client/ClientRegionFactoryBean.java

@@ -16,8 +16,6 @@
 
 package org.springframework.data.gemfire.client;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
@@ -42,93 +40,56 @@ import com.gemstone.gemfire.cache.client.Pool;
 import com.gemstone.gemfire.internal.cache.GemFireCacheImpl;
 
 /**
- * Client extension for GemFire regions.
- * 
+ * Client extension for GemFire Regions.
+ * <p/>
  * @author Costin Leau
  * @author David Turanski
+ * @author John Blum
  */
 public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V> implements BeanFactoryAware,
 		DisposableBean {
 
-	private static final Log log = LogFactory.getLog(ClientRegionFactoryBean.class);
-
+	private boolean close = true;
 	private boolean destroy = false;
 
-	private boolean close = true;
-
-	private Resource snapshot;
-
-	private CacheListener<K, V> cacheListeners[];
-
-	private Interest<K>[] interests;
-
-	private String poolName;
-
 	private BeanFactory beanFactory;
 
+	private Boolean persistent;
+
+	private CacheListener<K, V>[] cacheListeners;
+
 	private ClientRegionShortcut shortcut = null;
 
 	private DataPolicy dataPolicy;
 
+	private Interest<K>[] interests;
+
 	private RegionAttributes<K, V> attributes;
 
-	private Region<K, V> region;
+	private Resource snapshot;
 
 	private String diskStoreName;
-
-	private String dataPolicyName;
+	private String poolName;
 
 	@Override
 	public void afterPropertiesSet() throws Exception {
 		super.afterPropertiesSet();
-		region = getRegion();
-		postProcess(region);
+		postProcess(getRegion());
 	}
 
 	@Override
 	protected Region<K, V> lookupFallback(GemFireCache cache, String regionName) throws Exception {
 		Assert.isTrue(cache instanceof ClientCache, "Unable to create regions from " + cache);
-		ClientCache c = (ClientCache) cache;
 
+		// TODO use of GemFire internal class!
 		if (cache instanceof GemFireCacheImpl) {
-			Assert.isTrue(((GemFireCacheImpl) cache).isClient(), "A client-cache instance is required");
-		}
-		
-		Assert.isTrue(!(StringUtils.hasText(dataPolicyName) && dataPolicy != null), "Only one of 'dataPolicy' or 'dataPolicyName' can be set");
-		
-		
-		if (StringUtils.hasText(dataPolicyName)) {
-			dataPolicy = new DataPolicyConverter().convert(dataPolicyName);
-			Assert.notNull(dataPolicy, "Data policy " + dataPolicyName + " is invalid");
+			Assert.isTrue(((GemFireCacheImpl) cache).isClient(), "A client-cache instance is required.");
 		}
 
+		final ClientCache clientCache = (ClientCache) cache;
+
 		// first look at shortcut
-		ClientRegionShortcut s = null;
-
-		if (shortcut == null) {
-			if (dataPolicy != null) {
-				if (DataPolicy.EMPTY.equals(dataPolicy)) {
-					s = ClientRegionShortcut.PROXY;
-				}
-				else if (DataPolicy.PERSISTENT_REPLICATE.equals(dataPolicy)) {
-					s = ClientRegionShortcut.LOCAL_PERSISTENT;
-				}
-				else if (DataPolicy.NORMAL.equals(this.dataPolicy)) {
-					s = ClientRegionShortcut.CACHING_PROXY;
-				}
-				else {
-					s = ClientRegionShortcut.LOCAL;
-				}
-			}
-			else {
-				s = ClientRegionShortcut.LOCAL;
-			}
-		}
-		else {
-			s = shortcut;
-		}
-
-		ClientRegionFactory<K, V> factory = c.createClientRegionFactory(s);
+		ClientRegionFactory<K, V> factory = clientCache.createClientRegionFactory(resolveClientRegionShortcut());
 
 		// map the attributes onto the client
 		if (attributes != null) {
@@ -173,7 +134,8 @@ public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V>
 			}
 
 			factory.setPoolName(poolName);
-		} else {
+		}
+		else {
 			Pool pool = beanFactory.getBean(Pool.class);
 			factory.setPoolName(pool.getName());
 		}
@@ -182,26 +144,63 @@ public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V>
 			factory.setDiskStoreName(diskStoreName);
 		}
 
-		Region<K, V> reg = factory.create(regionName);
+		Region<K, V> region = factory.create(regionName);
 		log.info("Created new cache region [" + regionName + "]");
+
 		if (snapshot != null) {
-			reg.loadSnapshot(snapshot.getInputStream());
+			region.loadSnapshot(snapshot.getInputStream());
 		}
 
-		return reg;
+		return region;
+	}
+
+	protected ClientRegionShortcut resolveClientRegionShortcut() {
+		ClientRegionShortcut resolvedShortcut = this.shortcut;
+
+		if (resolvedShortcut == null) {
+			if (this.dataPolicy != null) {
+				assertDataPolicyAndPersistentAttributeAreCompatible(this.dataPolicy);
+
+				if (DataPolicy.EMPTY.equals(this.dataPolicy)) {
+					resolvedShortcut = ClientRegionShortcut.PROXY;
+				}
+				else if (DataPolicy.NORMAL.equals(this.dataPolicy)) {
+					resolvedShortcut = ClientRegionShortcut.CACHING_PROXY;
+				}
+				else if (DataPolicy.PERSISTENT_REPLICATE.equals(this.dataPolicy)) {
+					resolvedShortcut = ClientRegionShortcut.LOCAL_PERSISTENT;
+				}
+				else {
+					// NOTE the Data Policy validation is based on the ClientRegionShortcut initialization logic
+					// in com.gemstone.gemfire.internal.cache.GemFireCacheImpl.initializeClientRegionShortcuts
+					throw new IllegalArgumentException(String.format(
+						"Data Policy '%1$s' is invalid for Client Regions.", this.dataPolicy));
+				}
+			}
+			else {
+				resolvedShortcut = (isPersistent() ? ClientRegionShortcut.LOCAL_PERSISTENT
+					: ClientRegionShortcut.LOCAL);
+			}
+
+		}
+
+		// NOTE the ClientRegionShortcut and Persistent attribute will be compatible if the shortcut was derived from
+		// the Data Policy.
+		assertClientRegionShortcutAndPersistentAttributeAreCompatible(resolvedShortcut);
+
+		return resolvedShortcut;
 	}
 
 	protected void postProcess(Region<K, V> region) {
 		if (!ObjectUtils.isEmpty(interests)) {
 			for (Interest<K> interest : interests) {
 				if (interest instanceof RegexInterest) {
-					// do the cast since it's safe
 					region.registerInterestRegex((String) interest.getKey(), interest.getPolicy(),
-							interest.isDurable(), interest.isReceiveValues());
+						interest.isDurable(), interest.isReceiveValues());
 				}
 				else {
 					region.registerInterest(interest.getKey(), interest.getPolicy(), interest.isDurable(),
-							interest.isReceiveValues());
+						interest.isReceiveValues());
 				}
 			}
 		}
@@ -247,6 +246,20 @@ public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V>
 		region = null;
 	}
 
+	/**
+	 * Sets the region attributes used for the region used by this factory.
+	 * Allows maximum control in specifying the region settings. Used only when
+	 * a new region is created. Note that using this method allows for advanced
+	 * customization of the region - while it provides a lot of flexibility,
+	 * note that it's quite easy to create misconfigured regions (especially in
+	 * a client/server scenario).
+	 *
+	 * @param attributes the attributes to set on a newly created region
+	 */
+	public void setAttributes(RegionAttributes<K, V> attributes) {
+		this.attributes = attributes;
+	}
+
 	@Override
 	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
 		this.beanFactory = beanFactory;
@@ -269,19 +282,9 @@ public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V>
 		return interests;
 	}
 
-	/**
-	 * Sets the pool name used by this client.
-	 * 
-	 * @param poolName
-	 */
-	public void setPoolName(String poolName) {
-		Assert.hasText(poolName, "pool name is required");
-		this.poolName = poolName;
-	}
-
 	/**
 	 * Sets the pool used by this client.
-	 * 
+	 *
 	 * @param pool
 	 */
 	public void setPool(Pool pool) {
@@ -289,6 +292,16 @@ public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V>
 		setPoolName(pool.getName());
 	}
 
+	/**
+	 * Sets the pool name used by this client.
+	 *
+	 * @param poolName
+	 */
+	public void setPoolName(String poolName) {
+		Assert.hasText(poolName, "pool name is required");
+		this.poolName = poolName;
+	}
+
 	/**
 	 * Initializes the client using a GemFire {@link ClientRegionShortcut}. The
 	 * recommended way for creating clients since it covers all the major
@@ -355,23 +368,77 @@ public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V>
 		this.cacheListeners = cacheListeners;
 	}
 
+	protected void assertClientRegionShortcutAndPersistentAttributeAreCompatible(final ClientRegionShortcut resolvedShortcut) {
+		final boolean persistentNotSpecified = (this.persistent == null);
+
+		if (ClientRegionShortcut.LOCAL_PERSISTENT.equals(resolvedShortcut)
+				|| ClientRegionShortcut.LOCAL_PERSISTENT_OVERFLOW.equals(resolvedShortcut)) {
+			Assert.isTrue(persistentNotSpecified || isPersistent(), String.format(
+				"Client Region Shortcut '%1$s' is invalid when persistent is false.", resolvedShortcut));
+		}
+		else {
+			Assert.isTrue(persistentNotSpecified || isNotPersistent(), String.format(
+				"Client Region Shortcut '%1$s' is invalid when persistent is true.", resolvedShortcut));
+		}
+	}
 	/**
-	 * Sets the data policy. Used only when a new region is created.
-	 * 
-	 * @param dataPolicy the region data policy
+	 * Validates that the settings for Data Policy and the 'persistent' attribute in <gfe:*-region/> elements
+	 * are compatible.
+	 * <p/>
+	 * @param resolvedDataPolicy the GemFire Data Policy resolved form the Spring GemFire XML namespace configuration
+	 * meta-data.
+	 * @see #isPersistent()
+	 * @see #isNotPersistent()
+	 * @see com.gemstone.gemfire.cache.DataPolicy
+	 */
+	protected void assertDataPolicyAndPersistentAttributeAreCompatible(final DataPolicy resolvedDataPolicy) {
+		final boolean persistentNotSpecified = (this.persistent == null);
+
+		if (resolvedDataPolicy.withPersistence()) {
+			Assert.isTrue(persistentNotSpecified || isPersistent(), String.format(
+				"Data Policy '%1$s' is invalid when persistent is false.", resolvedDataPolicy));
+		}
+		else {
+			// NOTE otherwise, the Data Policy is with persistence, so...
+			Assert.isTrue(persistentNotSpecified || isNotPersistent(), String.format(
+				"Data Policy '%1$s' is invalid when persistent is true.", resolvedDataPolicy));
+		}
+	}
+
+	/**
+	 * Sets the Data Policy. Used only when a new Region is created.
+	 * <p/>
+	 * @param dataPolicy the client Region's Data Policy.
+	 * @see com.gemstone.gemfire.cache.DataPolicy
 	 */
 	public void setDataPolicy(DataPolicy dataPolicy) {
 		this.dataPolicy = dataPolicy;
 	}
-	
+
 	/**
-	 * An alternative way to set the data policy as a string. Useful for 
-	 * property placeholders, etc.
-	 * 
-	 * @param dataPolicyName
+	 * An alternate way to set the Data Policy, using the String name of the enumerated value.
+	 * <p/>
+	 * @param dataPolicyName the enumerated value String name of the Data Policy.
+	 * @see com.gemstone.gemfire.cache.DataPolicy
+	 * @see #setDataPolicy(com.gemstone.gemfire.cache.DataPolicy)
+	 * @deprecated use setDataPolicy(:DataPolicy) instead.
 	 */
 	public void setDataPolicyName(String dataPolicyName) {
-		this.dataPolicyName = dataPolicyName;
+		final DataPolicy resolvedDataPolicy = new DataPolicyConverter().convert(dataPolicyName);
+		Assert.notNull(resolvedDataPolicy, String.format("Data Policy '%1$s' is invalid.", dataPolicyName));
+		setDataPolicy(resolvedDataPolicy);
+	}
+
+	protected boolean isPersistent() {
+		return Boolean.TRUE.equals(persistent);
+	}
+
+	protected boolean isNotPersistent() {
+		return Boolean.FALSE.equals(persistent);
+	}
+
+	public void setPersistent(final boolean persistent) {
+		this.persistent = persistent;
 	}
 
 	/**
@@ -382,17 +449,4 @@ public class ClientRegionFactoryBean<K, V> extends RegionLookupFactoryBean<K, V>
 		this.diskStoreName = diskStoreName;
 	}
 
-	/**
-	 * Sets the region attributes used for the region used by this factory.
-	 * Allows maximum control in specifying the region settings. Used only when
-	 * a new region is created. Note that using this method allows for advanced
-	 * customization of the region - while it provides a lot of flexibility,
-	 * note that it's quite easy to create misconfigured regions (especially in
-	 * a client/server scenario).
-	 * 
-	 * @param attributes the attributes to set on a newly created region
-	 */
-	public void setAttributes(RegionAttributes<K, V> attributes) {
-		this.attributes = attributes;
-	}
-}
+}

src/main/java/org/springframework/data/gemfire/config/ClientRegionParser.java

@@ -29,14 +29,11 @@ import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 import org.w3c.dom.Element;
 
-import com.gemstone.gemfire.cache.DataPolicy;
-
 /**
- * Parser for <client-region;gt; definitions.
- * 
- * To avoid eager evaluations, the region interests are declared as nested
- * definition.
- * 
+ * Parser for <client-region;gt; bean definitions.
+ * <p/>
+ * To avoid eager evaluations, the region interests are declared as nested definition.
+ * <p/>
  * @author Costin Leau
  * @author David Turanski
  * @author John Blum
@@ -52,121 +49,103 @@ class ClientRegionParser extends AbstractRegionParser {
 	protected void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder) {
 		super.doParse(element, builder);
 
-		// set scope
-		// since the user can define both client and p2p regions
-		// setting the cache/DS to a be 'loner' isn't feasible
-		// so to prevent both client and p2p communication in the region,
-		// the scope is fixed to local
+		validateDataPolicyShortcutMutualExclusion(element, parserContext);
+
+		String cacheRefAttributeValue = element.getAttribute("cache-ref");
+
+		builder.addPropertyReference("cache", (StringUtils.hasText(cacheRefAttributeValue) ? cacheRefAttributeValue
+			: GemfireConstants.DEFAULT_GEMFIRE_CACHE_NAME));
+
+		ParsingUtils.setPropertyValue(element, builder, "close");
+		ParsingUtils.setPropertyValue(element, builder, "destroy");
 		ParsingUtils.setPropertyValue(element, builder, "data-policy", "dataPolicyName");
 		ParsingUtils.setPropertyValue(element, builder, "name");
+		ParsingUtils.setPropertyValue(element, builder, "persistent");
 		ParsingUtils.setPropertyValue(element, builder, "pool-name");
 		ParsingUtils.setPropertyValue(element, builder, "shortcut");
 
-		// set the persistent policy
-		String attr = element.getAttribute("persistent");
+		String diskStoreRefAttributeValue = element.getAttribute("disk-store-ref");
 
-		boolean frozenDataPolicy = false;
-
-		if (Boolean.parseBoolean(attr)) {
-			// check first for GemFire 6.5
-			builder.addPropertyValue("dataPolicy", DataPolicy.PERSISTENT_REPLICATE);
-			frozenDataPolicy = true;
-		}
-
-		attr = element.getAttribute("cache-ref");
-		// add cache reference (fallback to default if nothing is specified)
-		builder.addPropertyReference("cache", (StringUtils.hasText(attr) ? attr : GemfireConstants.DEFAULT_GEMFIRE_CACHE_NAME));
-		ParsingUtils.setPropertyValue(element, builder, "close");
-		ParsingUtils.setPropertyValue(element, builder, "destroy");
-		// eviction + overflow attributes
-		// client attributes
-		BeanDefinitionBuilder attrBuilder = BeanDefinitionBuilder
-				.genericBeanDefinition(RegionAttributesFactoryBean.class);
-
-		ParsingUtils.parseStatistics(element, attrBuilder);
-
-		if (StringUtils.hasText(element.getAttribute("disk-store-ref"))) {
+		if (StringUtils.hasText(diskStoreRefAttributeValue)) {
 			ParsingUtils.setPropertyValue(element, builder, "disk-store-ref", "diskStoreName");
-			builder.addDependsOn(element.getAttribute("disk-store-ref"));
+			builder.addDependsOn(diskStoreRefAttributeValue);
 		}
 
-		boolean overwriteDataPolicy = false;
+		// Client RegionAttributes for overflow/eviction, expiration and statistics
+		BeanDefinitionBuilder regionAttributesBuilder = BeanDefinitionBuilder.genericBeanDefinition(
+			RegionAttributesFactoryBean.class);
 
-		overwriteDataPolicy |= ParsingUtils.parseEviction(parserContext, element, attrBuilder);
-		ParsingUtils.parseStatistics(element, attrBuilder);
-		ParsingUtils.parseExpiration(parserContext, element, attrBuilder);
-		ParsingUtils.parseEviction(parserContext, element, attrBuilder);
-		ParsingUtils.parseOptionalRegionAttributes(parserContext, element, attrBuilder);
-		
-		if (!frozenDataPolicy && overwriteDataPolicy) {
-			builder.addPropertyValue("dataPolicy", DataPolicy.NORMAL);
-		}
+		ParsingUtils.parseOptionalRegionAttributes(parserContext, element, regionAttributesBuilder);
+		ParsingUtils.parseStatistics(element, regionAttributesBuilder);
+		ParsingUtils.parseEviction(parserContext, element, regionAttributesBuilder);
+		ParsingUtils.parseExpiration(parserContext, element, regionAttributesBuilder);
 
-		builder.addPropertyValue("attributes", attrBuilder.getBeanDefinition());
-		
-		
+		builder.addPropertyValue("attributes", regionAttributesBuilder.getBeanDefinition());
 
 		ManagedList<Object> interests = new ManagedList<Object>();
 
 		// parse nested declarations
 		List<Element> subElements = DomUtils.getChildElements(element);
 
-		// parse nested cache-listener elements
 		for (Element subElement : subElements) {
 			String name = subElement.getLocalName();
 
 			if ("cache-listener".equals(name)) {
-				builder.addPropertyValue("cacheListeners", parseCacheListener(parserContext, subElement, builder));
+				builder.addPropertyValue("cacheListeners", ParsingUtils.parseRefOrNestedBeanDeclaration(
+					parserContext, subElement, builder));
 			}
-
 			else if ("key-interest".equals(name)) {
-				interests.add(parseKeyInterest(parserContext, subElement));
+				interests.add(parseKeyInterest(subElement, parserContext));
 			}
-
 			else if ("regex-interest".equals(name)) {
-				interests.add(parseRegexInterest(parserContext, subElement));
+				interests.add(parseRegexInterest(subElement));
 			}
 		}
 
-		if (!subElements.isEmpty()) {
+		if (!interests.isEmpty()) {
 			builder.addPropertyValue("interests", interests);
 		}
 	}
 
+	private void validateDataPolicyShortcutMutualExclusion(final Element element, final ParserContext parserContext) {
+		if (element.hasAttribute("data-policy") && element.hasAttribute("shortcut")) {
+			parserContext.getReaderContext().error(String.format(
+				"Only one of [data-policy, shortcut] may be specified with element '%1$s'.", element.getTagName()),
+					element);
+		}
+	}
+
 	@Override
 	protected void doParseRegion(Element element, ParserContext parserContext, BeanDefinitionBuilder builder,
 			boolean subRegion) {
 		throw new UnsupportedOperationException(String.format(
-			"doParseRegion(:Element, :ParserContext, :BeanDefinitionBuilder, subRegion:boolean) is not supported on %1$s",
-			getClass().getName()));
+			"doParseRegion(:Element, :ParserContext, :BeanDefinitionBuilder, :boolean) is not supported on %1$s",
+				getClass().getName()));
 	}
 
-	private Object parseCacheListener(ParserContext parserContext, Element subElement, BeanDefinitionBuilder builder) {
-		return ParsingUtils.parseRefOrNestedBeanDeclaration(parserContext, subElement, builder);
-	}
-
-	private Object parseKeyInterest(ParserContext parserContext, Element subElement) {
+	private Object parseKeyInterest(Element keyInterestElement, ParserContext parserContext) {
 		BeanDefinitionBuilder keyInterestBuilder = BeanDefinitionBuilder.genericBeanDefinition(Interest.class);
-		parseCommonInterestAttr(subElement, keyInterestBuilder);
 
-		Object key = ParsingUtils.parseRefOrNestedBeanDeclaration(parserContext, subElement, keyInterestBuilder,
-				"key-ref");
-		keyInterestBuilder.addConstructorArgValue(key);
+		parseCommonInterestAttributes(keyInterestElement, keyInterestBuilder);
+		keyInterestBuilder.addConstructorArgValue(ParsingUtils.parseRefOrNestedBeanDeclaration(parserContext,
+			keyInterestElement, keyInterestBuilder, "key-ref"));
+
 		return keyInterestBuilder.getBeanDefinition();
 	}
 
-	private Object parseRegexInterest(ParserContext parserContext, Element subElement) {
+	private Object parseRegexInterest(Element regexInterestElement) {
 		BeanDefinitionBuilder regexInterestBuilder = BeanDefinitionBuilder.genericBeanDefinition(RegexInterest.class);
 
-		parseCommonInterestAttr(subElement, regexInterestBuilder);
-		ParsingUtils.setPropertyValue(subElement, regexInterestBuilder, "pattern", "key");
+		parseCommonInterestAttributes(regexInterestElement, regexInterestBuilder);
+		ParsingUtils.setPropertyValue(regexInterestElement, regexInterestBuilder, "pattern", "key");
 
 		return regexInterestBuilder.getBeanDefinition();
 	}
 
-	private void parseCommonInterestAttr(Element element, BeanDefinitionBuilder builder) {
+	private void parseCommonInterestAttributes(Element element, BeanDefinitionBuilder builder) {
 		ParsingUtils.setPropertyValue(element, builder, "durable", "durable");
 		ParsingUtils.setPropertyValue(element, builder, "result-policy", "policy");
 		ParsingUtils.setPropertyValue(element, builder, "receive-values", "receiveValues");
 	}
+
 }

src/test/java/org/springframework/data/gemfire/client/ClientRegionFactoryBeanTest.java

@@ -15,12 +15,21 @@
  */
 package org.springframework.data.gemfire.client;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Mockito;
 import org.springframework.beans.factory.BeanFactory;
+import org.springframework.data.gemfire.TestUtils;
 
+import com.gemstone.gemfire.cache.DataPolicy;
 import com.gemstone.gemfire.cache.Region;
 import com.gemstone.gemfire.cache.client.ClientCache;
 import com.gemstone.gemfire.cache.client.ClientRegionFactory;
@@ -29,36 +38,251 @@ import com.gemstone.gemfire.cache.client.Pool;
 
 public class ClientRegionFactoryBeanTest {
 
-	@Test
-	public void testLookupFallbackFailingToUseProvidedShortcut()
-			throws Exception {
-		ClientRegionFactoryBean<Object, Object> fb = new ClientRegionFactoryBean<Object, Object>();
-		fb.setShortcut(ClientRegionShortcut.CACHING_PROXY);
-		
-		Pool pool = Mockito.mock(Pool.class);
-		
-		BeanFactory beanFactory = Mockito.mock(BeanFactory.class);
-		
-		Mockito.when(beanFactory.getBean(Pool.class)).thenReturn(pool);
-		
-		fb.setBeanFactory(beanFactory);
+	private ClientRegionFactoryBean<Object, Object> factoryBean;
 
-		String regionName = "regionName";
-		ClientCache cache = Mockito.mock(ClientCache.class);
-
-		@SuppressWarnings("unchecked")
-		ClientRegionFactory<Object, Object> factory = Mockito
-				.mock(ClientRegionFactory.class);
-		Mockito.when(
-				cache.createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY))
-				.thenReturn(factory);
-
-		@SuppressWarnings("unchecked")
-		Region<Object, Object> region = Mockito.mock(Region.class);
-		Mockito.when(factory.create(regionName)).thenReturn(region);
-
-		Region<Object, Object> result = fb.lookupFallback(cache, regionName);
-
-		assertSame(region, result);
+	@Before
+	public void setup() {
+		factoryBean = new ClientRegionFactoryBean<Object, Object>();
 	}
+
+	@After
+	public void tearDown() throws Exception {
+		factoryBean.destroy();
+		factoryBean = null;
+	}
+
+	@SuppressWarnings("unchecked")
+	@Test
+	public void testLookupFallbackFailingToUseProvidedShortcut() throws Exception {
+		factoryBean.setShortcut(ClientRegionShortcut.CACHING_PROXY);
+
+		BeanFactory beanFactory = mock(BeanFactory.class);
+		Pool pool = mock(Pool.class);
+
+		when(beanFactory.getBean(Pool.class)).thenReturn(pool);
+
+		factoryBean.setBeanFactory(beanFactory);
+
+		ClientCache cache = mock(ClientCache.class);
+		ClientRegionFactory<Object, Object> clientRegionFactory = mock(ClientRegionFactory.class);
+		Region<Object, Object> expectedRegion = mock(Region.class);
+
+		when(cache.createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY)).thenReturn(clientRegionFactory);
+		when(clientRegionFactory.create("testRegion")).thenReturn(expectedRegion);
+
+		Region<Object, Object> actualRegion = factoryBean.lookupFallback(cache, "testRegion");
+
+		assertSame(expectedRegion, actualRegion);
+	}
+
+	@Test
+	public void testSetDataPolicyName() throws Exception {
+		factoryBean.setDataPolicyName("NORMAL");
+		assertEquals(DataPolicy.NORMAL, TestUtils.readField("dataPolicy", factoryBean));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testSetDataPolicyNameWithInvalidName() throws Exception {
+		try {
+			factoryBean.setDataPolicyName("INVALID");
+		}
+		catch (IllegalArgumentException e) {
+			assertEquals("Data Policy 'INVALID' is invalid.", e.getMessage());
+			throw e;
+		}
+		finally {
+			assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		}
+	}
+
+	@Test
+	public void testIsPersistent() {
+		assertFalse(factoryBean.isPersistent());
+		factoryBean.setPersistent(false);
+		assertFalse(factoryBean.isPersistent());
+		factoryBean.setPersistent(true);
+		assertTrue(factoryBean.isPersistent());
+	}
+
+	@Test
+	public void testIsNotPersistent() {
+		assertFalse(factoryBean.isNotPersistent());
+		factoryBean.setPersistent(true);
+		assertFalse(factoryBean.isNotPersistent());
+		factoryBean.setPersistent(false);
+		assertTrue(factoryBean.isNotPersistent());
+	}
+
+	@Test
+	public void testResolveClientRegionShortcut() throws Exception {
+		assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		assertNull(TestUtils.readField("persistent", factoryBean));
+		assertNull(TestUtils.readField("shortcut", factoryBean));
+		assertEquals(ClientRegionShortcut.LOCAL, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutWhenNotPersistent() throws Exception {
+		factoryBean.setPersistent(false);
+
+		assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		assertTrue(factoryBean.isNotPersistent());
+		assertNull(TestUtils.readField("shortcut", factoryBean));
+		assertEquals(ClientRegionShortcut.LOCAL, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutWhenPersistent() throws Exception {
+		factoryBean.setPersistent(true);
+
+		assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		assertTrue(factoryBean.isPersistent());
+		assertNull(TestUtils.readField("shortcut", factoryBean));
+		assertEquals(ClientRegionShortcut.LOCAL_PERSISTENT, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingShortcut() throws Exception {
+		factoryBean.setShortcut(ClientRegionShortcut.CACHING_PROXY_OVERFLOW);
+
+		assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		assertNull(TestUtils.readField("persistent", factoryBean));
+		assertEquals(ClientRegionShortcut.CACHING_PROXY_OVERFLOW, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingShortcutWhenNotPersistent() throws Exception {
+		factoryBean.setPersistent(false);
+		factoryBean.setShortcut(ClientRegionShortcut.CACHING_PROXY_HEAP_LRU);
+
+		assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		assertTrue(factoryBean.isNotPersistent());
+		assertEquals(ClientRegionShortcut.CACHING_PROXY_HEAP_LRU, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testResolveClientRegionShortcutUsingShortcutWhenPersistent() throws Exception {
+		try {
+			factoryBean.setPersistent(true);
+			factoryBean.setShortcut(ClientRegionShortcut.CACHING_PROXY);
+
+			assertNull(TestUtils.readField("dataPolicy", factoryBean));
+			assertTrue(factoryBean.isPersistent());
+
+			factoryBean.resolveClientRegionShortcut();
+		}
+		catch (IllegalArgumentException e) {
+			assertEquals("Client Region Shortcut 'CACHING_PROXY' is invalid when persistent is true.", e.getMessage());
+			throw e;
+		}
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingPersistentShortcut() throws Exception {
+		factoryBean.setShortcut(ClientRegionShortcut.LOCAL_PERSISTENT);
+
+		assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		assertNull(TestUtils.readField("persistent", factoryBean));
+		assertEquals(ClientRegionShortcut.LOCAL_PERSISTENT, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testResolveClientRegionShortcutUsingPersistentShortcutWhenNotPersistent() throws Exception {
+		try {
+			factoryBean.setPersistent(false);
+			factoryBean.setShortcut(ClientRegionShortcut.LOCAL_PERSISTENT);
+
+			assertNull(TestUtils.readField("dataPolicy", factoryBean));
+			assertTrue(factoryBean.isNotPersistent());
+
+			factoryBean.resolveClientRegionShortcut();
+		}
+		catch (IllegalArgumentException e) {
+			assertEquals("Client Region Shortcut 'LOCAL_PERSISTENT' is invalid when persistent is false.", e.getMessage());
+			throw e;
+		}
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingPersistentShortcutWhenPersistent() throws Exception {
+		factoryBean.setPersistent(true);
+		factoryBean.setShortcut(ClientRegionShortcut.LOCAL_PERSISTENT_OVERFLOW);
+
+		assertNull(TestUtils.readField("dataPolicy", factoryBean));
+		assertTrue(factoryBean.isPersistent());
+		assertEquals(ClientRegionShortcut.LOCAL_PERSISTENT_OVERFLOW, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingEmptyDataPolicy() throws Exception {
+		factoryBean.setDataPolicy(DataPolicy.EMPTY);
+
+		assertNull(TestUtils.readField("persistent", factoryBean));
+		assertNull(TestUtils.readField("shortcut", factoryBean));
+		assertEquals(ClientRegionShortcut.PROXY, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingNormalDataPolicyWhenNotPersistent() throws Exception {
+		factoryBean.setDataPolicy(DataPolicy.NORMAL);
+		factoryBean.setPersistent(false);
+
+		assertTrue(factoryBean.isNotPersistent());
+		assertNull(TestUtils.readField("shortcut", factoryBean));
+		assertEquals(ClientRegionShortcut.CACHING_PROXY, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testResolveClientRegionShortcutUsingNormalDataPolicyWhenPersistent() throws Exception {
+		try {
+			factoryBean.setDataPolicy(DataPolicy.NORMAL);
+			factoryBean.setPersistent(true);
+
+			assertTrue(factoryBean.isPersistent());
+			assertNull(TestUtils.readField("shortcut", factoryBean));
+
+			factoryBean.resolveClientRegionShortcut();
+		}
+		catch (IllegalArgumentException e) {
+			assertEquals("Data Policy 'NORMAL' is invalid when persistent is true.", e.getMessage());
+			throw e;
+		}
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingPersistentReplicateDataPolicy() throws Exception {
+		factoryBean.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
+
+		assertNull(TestUtils.readField("persistent", factoryBean));
+		assertNull(TestUtils.readField("shortcut", factoryBean));
+		assertEquals(ClientRegionShortcut.LOCAL_PERSISTENT, factoryBean.resolveClientRegionShortcut());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testResolveClientRegionShortcutUsingPersistentReplicateDataPolicyWhenNotPersistent() throws Exception {
+		try {
+			factoryBean.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
+			factoryBean.setPersistent(false);
+
+			assertTrue(factoryBean.isNotPersistent());
+			assertNull(TestUtils.readField("shortcut", factoryBean));
+
+			factoryBean.resolveClientRegionShortcut();
+		}
+		catch (IllegalArgumentException e) {
+			assertEquals("Data Policy 'PERSISTENT_REPLICATE' is invalid when persistent is false.", e.getMessage());
+			throw e;
+		}
+	}
+
+	@Test
+	public void testResolveClientRegionShortcutUsingPersistentReplicateDataPolicyWhenPersistent() throws Exception {
+		factoryBean.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE);
+		factoryBean.setPersistent(true);
+
+		assertNull(TestUtils.readField("shortcut", factoryBean));
+		assertTrue(factoryBean.isPersistent());
+		assertEquals(ClientRegionShortcut.LOCAL_PERSISTENT, factoryBean.resolveClientRegionShortcut());
+	}
+
 }

src/test/java/org/springframework/data/gemfire/config/ClientRegionNamespaceTest.java

@@ -124,7 +124,6 @@ public class ClientRegionNamespaceTest {
 	public void testOverflowToDisk() throws Exception {
 		assertTrue(context.containsBean("overflow"));
 		ClientRegionFactoryBean fb = context.getBean("&overflow", ClientRegionFactoryBean.class);
-		assertEquals(DataPolicy.NORMAL, TestUtils.readField("dataPolicy", fb));
 		RegionAttributes attrs = TestUtils.readField("attributes", fb);
 		EvictionAttributes evicAttr = attrs.getEvictionAttributes();
 		assertEquals(EvictionAction.OVERFLOW_TO_DISK, evicAttr.getAction());

src/test/java/org/springframework/data/gemfire/config/ClientRegionUsingDataPolicyAndShortcutTest.java

@@ -0,0 +1,47 @@
+/*
+ * 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.config;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
+import org.springframework.context.support.ClassPathXmlApplicationContext;
+
+/**
+ * The ClientRegionUsingDataPolicyAndShortcutTest class is a test suite of test case testing a client Region
+ * bean definition with both data-policy and shortcut specified.
+ * <p/>
+ * @author John Blum
+ * @see org.junit.Assert
+ * @since 1.3.3
+ */
+public class ClientRegionUsingDataPolicyAndShortcutTest {
+
+	@Test(expected = BeanDefinitionParsingException.class)
+	public void testClientRegionBeanDefinitionWithDataPolicyAndShortcut() {
+		try {
+			new ClassPathXmlApplicationContext("/org/springframework/data/gemfire/config/client-region-using-datapolicy-and-shortcut.xml");
+		}
+		catch (BeanDefinitionParsingException e) {
+			assertTrue(e.getMessage().contains("Only one of [data-policy, shortcut] may be specified with element"));
+			throw e;
+		}
+	}
+
+}

src/test/resources/org/springframework/data/gemfire/config/client-region-using-datapolicy-and-shortcut.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+	   xmlns:gfe="http://www.springframework.org/schema/gemfire"
+	   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	   xsi:schemaLocation="
+		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
+	   	http://www.springframework.org/schema/gemfire http://www.springframework.org/schema/gemfire/spring-gemfire.xsd
+">
+
+	<gfe:pool>
+		<gfe:locator host="localhost" port="10334"/>
+	</gfe:pool>
+
+	<gfe:client-cache/>
+	<gfe:client-region id="localAppData" data-policy="NORMAL" shortcut="CACHING_PROXY"/>
+	<!--
+	<gfe:client-region id="localAppData" data-policy="NORMAL"/>
+	<gfe:client-region id="localAppData" shortcut="CACHING_PROXY"/>
+	-->
+
+</beans>