From c34adc9f6a4f9399546680f3f6d9628248a52769 Mon Sep 17 00:00:00 2001 From: John Blum Date: Fri, 18 Oct 2013 14:23:35 -0700 Subject: [PATCH 1/5] Refactored the DataPolicyConverter for basic code cleanup and added a value to the Policy enum for PARTITION. Added additional test cases in the corresponding test class. Refactored the resolveDataPolicy method in the RegionFactoryBean making use of helper methods. --- .../data/gemfire/DataPolicyConverter.java | 73 +++++++++---------- .../data/gemfire/RegionFactoryBean.java | 34 +++++---- .../data/gemfire/DataPolicyConverterTest.java | 32 +++++++- 3 files changed, 85 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/springframework/data/gemfire/DataPolicyConverter.java b/src/main/java/org/springframework/data/gemfire/DataPolicyConverter.java index 4b84645c..db587979 100644 --- a/src/main/java/org/springframework/data/gemfire/DataPolicyConverter.java +++ b/src/main/java/org/springframework/data/gemfire/DataPolicyConverter.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.data.gemfire; import org.springframework.core.convert.converter.Converter; @@ -24,54 +25,50 @@ import com.gemstone.gemfire.cache.DataPolicy; * */ public class DataPolicyConverter implements Converter { - private static enum Policy { - EMPTY, DEFAULT, NORMAL, PERSISTENT_PARTITION, PERSISTENT_REPLICATE, PRELOADED, REPLICATE; - public DataPolicy toDataPolicy() { - DataPolicy dataPolicy = null; - switch (this) { - case EMPTY: - dataPolicy = DataPolicy.EMPTY; - break; - case DEFAULT: - dataPolicy = DataPolicy.DEFAULT; - break; - case NORMAL: - dataPolicy = DataPolicy.NORMAL; - break; - case PERSISTENT_PARTITION: - dataPolicy = DataPolicy.PERSISTENT_PARTITION; - break; - case PERSISTENT_REPLICATE: - dataPolicy = DataPolicy.PERSISTENT_REPLICATE; - break; - case PRELOADED: - dataPolicy = DataPolicy.PRELOADED; - break; - case REPLICATE: - dataPolicy = DataPolicy.REPLICATE; - break; - } - return dataPolicy; + + static enum Policy { + DEFAULT, EMPTY, NORMAL, PRELOADED, PARTITION, PERSISTENT_PARTITION, REPLICATE, PERSISTENT_REPLICATE; + + private static String toUpperCase(String value) { + return (value == null ? null : value.toUpperCase()); } public static Policy getValue(String value) { - Policy policy = null; try { - policy = valueOf(value); + return valueOf(toUpperCase(value)); } catch (Exception e) { + return null; } - return policy; } - }; + + public DataPolicy toDataPolicy() { + switch (this) { + case EMPTY: + return DataPolicy.EMPTY; + case NORMAL: + return DataPolicy.NORMAL; + case PRELOADED: + return DataPolicy.PRELOADED; + case PARTITION : + return DataPolicy.PARTITION; + case PERSISTENT_PARTITION: + return DataPolicy.PERSISTENT_PARTITION; + case REPLICATE: + return DataPolicy.REPLICATE; + case PERSISTENT_REPLICATE: + return DataPolicy.PERSISTENT_REPLICATE; + case DEFAULT: + default: + return DataPolicy.DEFAULT; + } + } + } @Override - public DataPolicy convert(String source) { - if (source == null) { - return null; - } - source = source.toUpperCase(); - return Policy.getValue(source) == null ? null : Policy.getValue(source).toDataPolicy(); + public DataPolicy convert(String policyValue) { + Policy policy = Policy.getValue(policyValue); + return (policy == null ? null : policy.toDataPolicy()); } } diff --git a/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java index ceaea032..aa971267 100644 --- a/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java @@ -195,21 +195,27 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple * @param dataPolicy requested data policy */ protected void resolveDataPolicy(RegionFactory regionFactory, Boolean persistent, String dataPolicy) { - if (dataPolicy == null) { - if (isPersistent()) { - regionFactory.setDataPolicy(DataPolicy.PERSISTENT_REPLICATE); - } else { - regionFactory.setDataPolicy(DataPolicy.DEFAULT); - } - return; + if (dataPolicy != null) { + regionFactory.setDataPolicy(convertAndValidate(dataPolicy)); + } + else { + regionFactory.setDataPolicy(isPersistent() ? DataPolicy.PERSISTENT_REPLICATE : DataPolicy.DEFAULT); + } + } + + private DataPolicy convertAndValidate(final String dataPolicy) { + final DataPolicy dataPolicyType = new DataPolicyConverter().convert(dataPolicy); + + Assert.notNull(dataPolicyType, String.format("Data policy %1$s is invalid", dataPolicy)); + + if (dataPolicyType.withPersistence()) { + // NOTE isNotPersistent means the user explicitly set the persistent attribute for the Region to false, + // and this conflicts with the Data Policy (via the that was set by the user, which indicates persistence. + Assert.isTrue(!isNotPersistent(), String.format("Data policy %1$s is invalid when persistent is false", + dataPolicy)); } - DataPolicy dp = new DataPolicyConverter().convert(dataPolicy); - Assert.notNull(dp, "Data policy " + dataPolicy + " is invalid"); - if (dp.withPersistence()) { - Assert.isTrue(!isNotPersistent(), "Data policy " + dataPolicy + " is invalid when persistent is false"); - } - regionFactory.setDataPolicy(dp); + return dataPolicyType; } @SuppressWarnings("unchecked") @@ -343,7 +349,7 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple /** * Sets the dataPolicy as a String. Required to support property * placeholders - * @param dataPolicy the dataPolicy name (NORMAL, PRELOADED, etc) + * @param dataPolicyName the dataPolicy name (NORMAL, PRELOADED, etc) */ public void setDataPolicy(String dataPolicyName) { this.dataPolicy = dataPolicyName; diff --git a/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java b/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java index c0a6b054..c9fe3d69 100644 --- a/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java +++ b/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java @@ -27,11 +27,39 @@ import com.gemstone.gemfire.cache.DataPolicy; * */ public class DataPolicyConverterTest { - DataPolicyConverter converter = new DataPolicyConverter(); + + private final DataPolicyConverter converter = new DataPolicyConverter(); + + protected int getDataPolicyEnumerationSize() { + for (byte ordinal = 0; true; ordinal++) { + try { + DataPolicy.fromOrdinal(ordinal); + } + catch (Exception e) { + return ordinal; + } + } + } @Test - public void test() { + public void testPolicyToDataPolicy() { + // exclude DEFAULT + assertEquals(getDataPolicyEnumerationSize(), DataPolicyConverter.Policy.values().length - 1); + assertEquals(DataPolicy.EMPTY, DataPolicyConverter.Policy.EMPTY.toDataPolicy()); + assertEquals(DataPolicy.NORMAL, DataPolicyConverter.Policy.NORMAL.toDataPolicy()); + assertEquals(DataPolicy.PRELOADED, DataPolicyConverter.Policy.PRELOADED.toDataPolicy()); + assertEquals(DataPolicy.PARTITION, DataPolicyConverter.Policy.PARTITION.toDataPolicy()); + assertEquals(DataPolicy.PERSISTENT_PARTITION, DataPolicyConverter.Policy.PERSISTENT_PARTITION.toDataPolicy()); + assertEquals(DataPolicy.REPLICATE, DataPolicyConverter.Policy.REPLICATE.toDataPolicy()); + assertEquals(DataPolicy.PERSISTENT_REPLICATE, DataPolicyConverter.Policy.PERSISTENT_REPLICATE.toDataPolicy()); + assertEquals(DataPolicy.DEFAULT, DataPolicyConverter.Policy.DEFAULT.toDataPolicy()); + } + + @Test + public void testConvert() { assertEquals(DataPolicy.EMPTY, converter.convert("empty")); + assertEquals(DataPolicy.PARTITION, converter.convert("Partition")); + assertEquals(DataPolicy.PERSISTENT_REPLICATE, converter.convert("PERSISTENT_REPLICATE")); assertNull(converter.convert("invalid")); assertNull(converter.convert(null)); } From ff6c121d2918a63c00583fc334ebb3c0ddedf33b Mon Sep 17 00:00:00 2001 From: John Blum Date: Fri, 25 Oct 2013 15:44:14 -0700 Subject: [PATCH 2/5] Fixes JIRA issue SGF-203 involving the element's persistent, data-policy, and for client Regions, shortcut attributes. --- .../data/gemfire/LocalRegionFactoryBean.java | 52 +++-- .../gemfire/PartitionedRegionFactoryBean.java | 48 ++--- .../data/gemfire/RegionFactoryBean.java | 188 +++++++++------- .../data/gemfire/RegionLookupFactoryBean.java | 10 +- .../gemfire/ReplicatedRegionFactoryBean.java | 35 +-- .../data/gemfire/DataPolicyConverterTest.java | 8 +- .../gemfire/LocalRegionFactoryBeanTest.java | 133 ++++++++++-- .../PartitionedRegionFactoryBeanTest.java | 168 +++++++++++++++ .../ReplicatedRegionFactoryBeanTest.java | 204 ++++++++++++++++++ 9 files changed, 682 insertions(+), 164 deletions(-) create mode 100644 src/test/java/org/springframework/data/gemfire/PartitionedRegionFactoryBeanTest.java create mode 100644 src/test/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBeanTest.java diff --git a/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java index 5d6f4d0b..59206a91 100644 --- a/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java @@ -16,6 +16,7 @@ package org.springframework.data.gemfire; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import com.gemstone.gemfire.cache.DataPolicy; import com.gemstone.gemfire.cache.RegionFactory; @@ -23,12 +24,13 @@ import com.gemstone.gemfire.cache.Scope; /** * @author David Turanski - * + * @author John Blum */ public class LocalRegionFactoryBean extends RegionFactoryBean { + @Override public void setScope(Scope scope) { - throw new UnsupportedOperationException("setScope() is not allowed for Local Regions"); + throw new UnsupportedOperationException("Setting the Scope on Local Regions is not allowed."); } @Override @@ -37,24 +39,38 @@ public class LocalRegionFactoryBean extends RegionFactoryBean { super.afterPropertiesSet(); } + /** + * Resolves the Data Policy used by this "local" GemFire Region (i.e. locally Scoped; Scope.LOCAL) based on the + * enumerated value from com.gemstone.gemfire.cache.RegionShortcuts (LOCAL, LOCAL_PERSISTENT, LOCAL_HEAP_LRU, + * LOCAL_OVERFLOW, and LOCAL_PERSISTENT_OVERFLOW), but without consideration of the Eviction settings. + *

+ * @param regionFactory the GemFire RegionFactory used to created the Local Region. + * @param persistent a boolean value indicating whether the Local Region should persist it's data. + * @param dataPolicy requested Data Policy as set by the user in the Spring GemFire configuration meta-data. + * @see com.gemstone.gemfire.cache.DataPolicy + * @see com.gemstone.gemfire.cache.RegionFactory + * @see com.gemstone.gemfire.cache.RegionShortcut + */ @Override protected void resolveDataPolicy(RegionFactory regionFactory, Boolean persistent, String dataPolicy) { - if (dataPolicy != null) { - dataPolicy = dataPolicy.toUpperCase(); - if ("NORMAL".equals(dataPolicy) || dataPolicy == null) { - regionFactory.setDataPolicy(DataPolicy.NORMAL); - } - else if ("PRELOADED".equals(dataPolicy)) { - regionFactory.setDataPolicy(DataPolicy.PRELOADED); - } - else if ("EMPTY".equals(dataPolicy)) { - Assert.isTrue(persistent == null || !persistent, "Cannot have persistence on an empty region"); - regionFactory.setDataPolicy(DataPolicy.EMPTY); - } - else { - throw new IllegalArgumentException("Data policy '" + dataPolicy - + "' is unsupported or invalid for local regions."); - } + DataPolicy resolvedDataPolicy = new DataPolicyConverter().convert(dataPolicy); + + Assert.isTrue(resolvedDataPolicy != null || (resolvedDataPolicy == null && !StringUtils.hasText(dataPolicy)), + String.format("Data Policy '%1$s' is invalid.", dataPolicy)); + + if (resolvedDataPolicy == null || DataPolicy.NORMAL.equals(resolvedDataPolicy)) { + // NOTE this is safe since a LOCAL Scoped NORMAL Region requiring persistence can be satisfied with + // PERSISTENT_REPLICATE, per the RegionShortcut.LOCAL_PERSISTENT + regionFactory.setDataPolicy(isPersistent() ? DataPolicy.PERSISTENT_REPLICATE : DataPolicy.NORMAL); + } + else if (DataPolicy.PRELOADED.equals(resolvedDataPolicy)) { + // NOTE this is safe since a LOCAL Scoped PRELOADED Region requiring persistence can be satisfied with + // PERSISTENT_REPLICATE, per the RegionShortcut.LOCAL_PERSISTENT + regionFactory.setDataPolicy(isPersistent() ? DataPolicy.PERSISTENT_REPLICATE : DataPolicy.PRELOADED); + } + else { + throw new IllegalArgumentException(String.format("Data Policy '%1$s' is not supported in Local Regions.", + dataPolicy)); } } diff --git a/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java index 11226876..546093e4 100644 --- a/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java @@ -26,41 +26,41 @@ import com.gemstone.gemfire.cache.RegionFactory; /** * @author David Turanski - * + * @author John Blum */ public class PartitionedRegionFactoryBean extends RegionFactoryBean { @Override protected void resolveDataPolicy(RegionFactory regionFactory, Boolean persistent, String dataPolicy) { if (dataPolicy != null) { - DataPolicy dp = new DataPolicyConverter().convert(dataPolicy); - Assert.notNull(dp, "Data policy " + dataPolicy + " is invalid"); - Assert.isTrue(dp.withPartitioning(), "Data Policy " + dp.toString() - + " is not supported in partitioned regions"); - if (!isPersistent()) { - regionFactory.setDataPolicy(dp); - } - else { - Assert.isTrue(dp.withPersistence(), "Data Policy " + dp.toString() - + "is invalid when persistent is false"); - regionFactory.setDataPolicy(dp); - } - return; + DataPolicy resolvedDataPolicy = new DataPolicyConverter().convert(dataPolicy); + + Assert.notNull(resolvedDataPolicy, String.format("Data Policy '%1$s' is invalid.", dataPolicy)); + + // Validate that the user-defined Data Policy matches the appropriate Spring GemFire XML namespace + // configuration meta-data element for Region (i.e. )! + Assert.isTrue(resolvedDataPolicy.withPartitioning(), String.format( + "Data Policy '%1$s' is not supported in Partitioned Regions.", resolvedDataPolicy)); + + // Validate that the data-policy and persistent attributes are compatible when specified! + assertDataPolicyAndPersistentAttributesAreCompatible(resolvedDataPolicy); + + regionFactory.setDataPolicy(resolvedDataPolicy); } - if (isPersistent()) { - // check first for GemFire 6.5 - if (ConcurrentMap.class.isAssignableFrom(Region.class)) { - regionFactory.setDataPolicy(DataPolicy.PERSISTENT_PARTITION); - } - else { - throw new IllegalArgumentException( - "Can define persistent partitions only from GemFire 6.5 onwards - current version is [" - + CacheFactory.getVersion() + "]"); - } + else if (isPersistent()) { + // first, check the presence of GemFire 6.5 or Higher + Assert.isTrue(isGemFireVersion65orHigher(), String.format( + "Can define Persistent Partitioned Regions only from GemFire 6.5 onwards; current version is [%1$s]", + CacheFactory.getVersion())); + regionFactory.setDataPolicy(DataPolicy.PERSISTENT_PARTITION); } else { regionFactory.setDataPolicy(DataPolicy.PARTITION); } } + private boolean isGemFireVersion65orHigher() { + return ConcurrentMap.class.isAssignableFrom(Region.class); + } + } diff --git a/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java index aa971267..427df3bc 100644 --- a/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/RegionFactoryBean.java @@ -24,14 +24,12 @@ import org.springframework.beans.factory.DisposableBean; import org.springframework.context.SmartLifecycle; import org.springframework.core.io.Resource; import org.springframework.data.gemfire.client.ClientRegionFactoryBean; -import org.springframework.data.gemfire.wan.GatewaySenderWrapper; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import com.gemstone.gemfire.cache.AttributesFactory; import com.gemstone.gemfire.cache.Cache; -import com.gemstone.gemfire.cache.CacheClosedException; import com.gemstone.gemfire.cache.CacheListener; import com.gemstone.gemfire.cache.CacheLoader; import com.gemstone.gemfire.cache.CacheWriter; @@ -56,20 +54,19 @@ import com.gemstone.gemfire.cache.wan.GatewaySender; * * @author Costin Leau * @author David Turanski + * @author John Blum */ public class RegionFactoryBean extends RegionLookupFactoryBean implements DisposableBean, SmartLifecycle { - private boolean autoStartup = true; - - private boolean running; - protected final Log log = LogFactory.getLog(getClass()); - private boolean destroy = false; - + private boolean autoStartup = true; private boolean close = true; + private boolean destroy = false; + private boolean running; - private Resource snapshot; + private Boolean enableGateway; + private Boolean persistent; private CacheListener cacheListeners[]; @@ -77,31 +74,23 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple private CacheWriter cacheWriter; - private Object gatewaySenders[]; - - private Object asyncEventQueues[]; + private Object[] asyncEventQueues; + private Object[] gatewaySenders; private RegionAttributes attributes; + private Resource snapshot; + private Scope scope; - private Boolean persistent; - - private Boolean enableGateway; - - private String hubId; - - private String diskStoreName; - private String dataPolicy; - - private Region region; + private String diskStoreName; + private String hubId; @Override public void afterPropertiesSet() throws Exception { super.afterPropertiesSet(); - region = getRegion(); - postProcess(region); + postProcess(getRegion()); } @Override @@ -110,11 +99,12 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple Cache c = (Cache) cache; - if (attributes != null) + if (attributes != null) { AttributesFactory.validateAttributes(attributes); + } - final RegionFactory regionFactory = (attributes != null ? c.createRegionFactory(attributes) : c - . createRegionFactory()); + final RegionFactory regionFactory = (attributes != null ? c.createRegionFactory(attributes) : + c. createRegionFactory()); if (hubId != null) { enableGateway = enableGateway == null ? true : enableGateway; @@ -122,12 +112,14 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple regionFactory.setGatewayHubId(hubId); } + if (enableGateway != null) { if (enableGateway) { Assert.notNull(hubId, "enableGateway requires the hubId property to be true"); } regionFactory.setEnableGateway(enableGateway); } + if (!ObjectUtils.isEmpty(cacheListeners)) { for (CacheListener listener : cacheListeners) { regionFactory.addCacheListener(listener); @@ -135,9 +127,8 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple } if (!ObjectUtils.isEmpty(gatewaySenders)) { - Assert.isTrue( - hubId == null, - "It is invalid to configure a region with both a hubId and gatewaySenders. Note that the enableGateway and hubId properties are deprecated since Gemfire 7.0"); + Assert.isTrue(hubId == null, "It is invalid to configure a region with both a hubId and gatewaySenders." + + " Note that the enableGateway and hubId properties are deprecated since Gemfire 7.0"); for (Object gatewaySender : gatewaySenders) { regionFactory.addGatewaySenderId(((GatewaySender) gatewaySender).getId()); } @@ -169,60 +160,56 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple if (attributes != null) { Assert.state(!attributes.isLockGrantor() || scope.isGlobal(), - "Lock grantor only applies to a global scoped region"); + "Lock grantor only applies to a global scoped region"); } + // get underlying AttributesFactory postProcess(findAttrFactory(regionFactory)); - Region reg = regionFactory.create(regionName); + Region region = regionFactory.create(regionName); log.info("Created new cache region [" + regionName + "]"); + if (snapshot != null) { - reg.loadSnapshot(snapshot.getInputStream()); + region.loadSnapshot(snapshot.getInputStream()); } if (attributes != null && attributes.isLockGrantor()) { - reg.becomeLockGrantor(); + region.becomeLockGrantor(); } - return reg; + return region; } /** - * This validates the configured data policy and may override it, taking - * into account the persistent attribute and constraints for the region type - * @param regionFactory - * @param persistent - * @param dataPolicy requested data policy + * Validates the configured Data Policy and may override it, taking into account the 'persistent' attribute + * and constraints for the Region type. + *

+ * @param regionFactory the GemFire RegionFactory used to created the Local Region. + * @param persistent a boolean value indicating whether the Local Region should persist it's data. + * @param dataPolicy requested Data Policy as set by the user in the Spring GemFire configuration meta-data. + * @see com.gemstone.gemfire.cache.DataPolicy + * @see com.gemstone.gemfire.cache.RegionFactory */ protected void resolveDataPolicy(RegionFactory regionFactory, Boolean persistent, String dataPolicy) { if (dataPolicy != null) { - regionFactory.setDataPolicy(convertAndValidate(dataPolicy)); + DataPolicy resolvedDataPolicy = new DataPolicyConverter().convert(dataPolicy); + + Assert.notNull(resolvedDataPolicy, String.format("Data Policy '%1$s' is invalid.", dataPolicy)); + assertDataPolicyAndPersistentAttributesAreCompatible(resolvedDataPolicy); + + regionFactory.setDataPolicy(resolvedDataPolicy); } else { regionFactory.setDataPolicy(isPersistent() ? DataPolicy.PERSISTENT_REPLICATE : DataPolicy.DEFAULT); } } - private DataPolicy convertAndValidate(final String dataPolicy) { - final DataPolicy dataPolicyType = new DataPolicyConverter().convert(dataPolicy); - - Assert.notNull(dataPolicyType, String.format("Data policy %1$s is invalid", dataPolicy)); - - if (dataPolicyType.withPersistence()) { - // NOTE isNotPersistent means the user explicitly set the persistent attribute for the Region to false, - // and this conflicts with the Data Policy (via the that was set by the user, which indicates persistence. - Assert.isTrue(!isNotPersistent(), String.format("Data policy %1$s is invalid when persistent is false", - dataPolicy)); - } - - return dataPolicyType; - } - @SuppressWarnings("unchecked") private AttributesFactory findAttrFactory(RegionFactory regionFactory) { - Field attrField = ReflectionUtils.findField(RegionFactory.class, "attrsFactory", AttributesFactory.class); - ReflectionUtils.makeAccessible(attrField); - return (AttributesFactory) ReflectionUtils.getField(attrField, regionFactory); + Field attrsFactoryField = ReflectionUtils.findField(RegionFactory.class, "attrsFactory", + AttributesFactory.class); + ReflectionUtils.makeAccessible(attrsFactoryField); + return (AttributesFactory) ReflectionUtils.getField(attrsFactoryField, regionFactory); } /** @@ -252,32 +239,22 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple @Override public void destroy() throws Exception { - if (region != null) { + if (getRegion() != null) { if (close) { - if (!region.getRegionService().isClosed()) { + if (!getRegion().getRegionService().isClosed()) { try { - region.close(); - region = null; + getRegion().close(); } catch (Exception cce) { // nothing to see folks, move on. } } } if (destroy) { - region.destroyRegion(); - region = null; + getRegion().destroyRegion(); } } } - /** - * Indicates whether the region referred by this factory bean, will be - * destroyed on shutdown (default false). - */ - public void setDestroy(boolean destroy) { - this.destroy = destroy; - } - /** * Indicates whether the region referred by this factory bean, will be * closed on shutdown (default true). @@ -286,6 +263,14 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple this.close = close; } + /** + * Indicates whether the region referred by this factory bean, will be + * destroyed on shutdown (default false). + */ + public void setDestroy(boolean destroy) { + this.destroy = destroy; + } + /** * Sets the snapshots used for loading a newly created region. That * is, the snapshot will be used only when a new region is created - @@ -400,12 +385,54 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple this.attributes = attributes; } - protected boolean isPersistent() { - return persistent != null && persistent; + /** + * Validates that the settings for Data Policy and the 'persistent' attribute in elements + * are compatible. + *

+ * @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 assertDataPolicyAndPersistentAttributesAreCompatible(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)); + } } + /** + * Returns true when the user explicitly specified a value for the persistent attribute and it is true. If the + * persistent attribute was not explicitly specified, then the persistence setting is implicitly undefined + * and will be determined by the Data Policy. + *

+ * @return true when the user specified an explicit value for the persistent attribute and it is true; + * false otherwise. + * @see #isNotPersistent() + */ + protected boolean isPersistent() { + return Boolean.TRUE.equals(persistent); + } + + /** + * Returns true when the user explicitly specified a value for the persistent attribute and it is false. If the + * persistent attribute was not explicitly specified, then the persistence setting is implicitly undefined + * and will be determined by the Data Policy. + *

+ * @return true when the user specified an explicit value for the persistent attribute and it is false; + * false otherwise. + * @see #isPersistent() + */ protected boolean isNotPersistent() { - return persistent != null && !persistent; + return Boolean.FALSE.equals(persistent); } /* (non-Javadoc) @@ -413,12 +440,11 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple */ @Override public void start() { - if (!ObjectUtils.isEmpty(gatewaySenders)) { synchronized (gatewaySenders) { for (Object obj : gatewaySenders) { GatewaySender gws = (GatewaySender) obj; - if (!gws.isManualStart() && !gws.isRunning()) { + if (!(gws.isManualStart() || gws.isRunning())) { gws.start(); } } @@ -435,9 +461,8 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple if (!ObjectUtils.isEmpty(gatewaySenders)) { synchronized (gatewaySenders) { for (Object obj : gatewaySenders) { - GatewaySender gws = (GatewaySender) obj; - gws.stop(); - } + ((GatewaySender) obj).stop(); + } } } this.running = false; @@ -475,4 +500,5 @@ public class RegionFactoryBean extends RegionLookupFactoryBean imple stop(); callback.run(); } + } diff --git a/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java b/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java index 5c51dfc2..f21843d7 100644 --- a/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/RegionLookupFactoryBean.java @@ -39,16 +39,19 @@ public class RegionLookupFactoryBean implements FactoryBean>, protected final Log log = LogFactory.getLog(getClass()); - private String beanName; private GemFireCache cache; - private String name; private Region region; + private String beanName; + private String name; + public void afterPropertiesSet() throws Exception { Assert.notNull(cache, "Cache property must be set"); + name = (!StringUtils.hasText(name) ? beanName : name); Assert.hasText(name, "Name (or beanName) property must be set"); + synchronized (cache) { region = cache.getRegion(name); if (region != null) { @@ -116,4 +119,5 @@ public class RegionLookupFactoryBean implements FactoryBean>, protected Region getRegion() { return region; } -} \ No newline at end of file + +} diff --git a/src/main/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBean.java index bf6f16f6..50ff9cae 100644 --- a/src/main/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBean.java @@ -22,32 +22,35 @@ import com.gemstone.gemfire.cache.RegionFactory; /** * @author David Turanski - * + * @author John Blum */ public class ReplicatedRegionFactoryBean extends RegionFactoryBean { + @Override protected void resolveDataPolicy(RegionFactory regionFactory, Boolean persistent, String dataPolicy) { - DataPolicy dp = null; if (dataPolicy != null) { - dataPolicy = dataPolicy.toUpperCase(); - if ("EMPTY".equals(dataPolicy)) { - Assert.isTrue(persistent == null || !persistent, "Cannot have persistence on an empty region"); - dp = DataPolicy.EMPTY; + DataPolicy resolvedDataPolicy = new DataPolicyConverter().convert(dataPolicy); + + Assert.notNull(resolvedDataPolicy, String.format("Data Policy '%1$s' is invalid.", dataPolicy)); + + // Validate that the data-policy and persistent attributes are compatible when specified! + assertDataPolicyAndPersistentAttributesAreCompatible(resolvedDataPolicy); + + if (DataPolicy.EMPTY.equals(resolvedDataPolicy)) { + resolvedDataPolicy = DataPolicy.EMPTY; } else { - dp = new DataPolicyConverter().convert(dataPolicy); - Assert.notNull(dp, "Data policy " + dataPolicy + " is invalid"); - Assert.isTrue(dp.withReplication(), "Data policy " + dataPolicy - + " is invalid or unsupported in replicated regions"); - if (isPersistent()) { - Assert.isTrue(dp.withPersistence(), "Data policy " + dataPolicy - + " is invalid when persistent is false"); - } + // Validate that the user-defined Data Policy matches the appropriate Spring GemFire XML namespace + // configuration meta-data element for Region (i.e. )! + Assert.isTrue(resolvedDataPolicy.withReplication(), String.format("" + + "Data Policy '%1$s' is not supported in Replicated Regions.", resolvedDataPolicy)); } + + regionFactory.setDataPolicy(resolvedDataPolicy); } else { - dp = (isPersistent()) ? DataPolicy.PERSISTENT_REPLICATE : DataPolicy.REPLICATE; + regionFactory.setDataPolicy(isPersistent() ? DataPolicy.PERSISTENT_REPLICATE : DataPolicy.REPLICATE); } - regionFactory.setDataPolicy(dp); } + } diff --git a/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java b/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java index c9fe3d69..ee1ea18d 100644 --- a/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java +++ b/src/test/java/org/springframework/data/gemfire/DataPolicyConverterTest.java @@ -24,14 +24,14 @@ import com.gemstone.gemfire.cache.DataPolicy; /** * @author David Turanski - * + * @author John Blum */ public class DataPolicyConverterTest { private final DataPolicyConverter converter = new DataPolicyConverter(); protected int getDataPolicyEnumerationSize() { - for (byte ordinal = 0; true; ordinal++) { + for (byte ordinal = 0; ordinal < Byte.MAX_VALUE; ordinal++) { try { DataPolicy.fromOrdinal(ordinal); } @@ -39,6 +39,9 @@ public class DataPolicyConverterTest { return ordinal; } } + + throw new IndexOutOfBoundsException("The size of the Data Policy enumeration could not be determined" + + " because the ordinal based on Byte.MAX_VALUE was exhausted!"); } @Test @@ -63,4 +66,5 @@ public class DataPolicyConverterTest { assertNull(converter.convert("invalid")); assertNull(converter.convert(null)); } + } diff --git a/src/test/java/org/springframework/data/gemfire/LocalRegionFactoryBeanTest.java b/src/test/java/org/springframework/data/gemfire/LocalRegionFactoryBeanTest.java index 5e1aa71d..1a4b2702 100644 --- a/src/test/java/org/springframework/data/gemfire/LocalRegionFactoryBeanTest.java +++ b/src/test/java/org/springframework/data/gemfire/LocalRegionFactoryBeanTest.java @@ -17,18 +17,34 @@ package org.springframework.data.gemfire; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import org.junit.Test; import org.springframework.data.gemfire.support.AbstractRegionFactoryBeanTest; import com.gemstone.gemfire.cache.DataPolicy; import com.gemstone.gemfire.cache.Region; +import com.gemstone.gemfire.cache.RegionFactory; /** + * The PartitionedRegionFactoryBeanTest class is a test suite of test cases testing the component functionality + * and correct behavior of the PartitionedRegionFactoryBean class. + *

* @author David Turanski - * + * @author John Blum + * @see org.mockito.Mockito + * @see org.junit.Test + * @see org.springframework.data.gemfire.PartitionedRegionFactoryBean + * @since 1.3.x */ +@SuppressWarnings("unchecked") public class LocalRegionFactoryBeanTest extends AbstractRegionFactoryBeanTest { + private final LocalRegionFactoryBean factoryBean = new LocalRegionFactoryBean(); + @SuppressWarnings("rawtypes") private RegionFactoryBeanConfig defaultConfig() { return new RegionFactoryBeanConfig(new LocalRegionFactoryBean(), "default") { @@ -42,24 +58,6 @@ public class LocalRegionFactoryBeanTest extends AbstractRegionFactoryBeanTest { @Override public void configureRegionFactoryBean() { - // TODO Auto-generated method stub - } - }; - } - - @SuppressWarnings("rawtypes") - private RegionFactoryBeanConfig emptyConfig() { - return new RegionFactoryBeanConfig(new LocalRegionFactoryBean(), "empty") { - - @Override - public void verify() { - Region region = regionFactoryBean.getRegion(); - assertEquals(DataPolicy.EMPTY, region.getAttributes().getDataPolicy()); - } - - @Override - public void configureRegionFactoryBean() { - regionFactoryBean.setDataPolicy("empty"); } }; } @@ -83,7 +81,102 @@ public class LocalRegionFactoryBeanTest extends AbstractRegionFactoryBeanTest { @Override protected void createRegionFactoryBeanConfigs() { addRFBConfig(defaultConfig()); - addRFBConfig(emptyConfig()); addRFBConfig(invalidConfig()); } + + protected RegionFactory createMockRegionFactory() { + return mock(RegionFactory.class); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithInvalidDataPolicyName() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.resolveDataPolicy(mockRegionFactory, null, "INVALID_DATA_POLICY_NAME"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'INVALID_DATA_POLICY_NAME' is invalid.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.NORMAL)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PRELOADED)); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithInvalidDataPolicyRegionType() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.resolveDataPolicy(mockRegionFactory, null, "PARTITION"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'PARTITION' is not supported in Local Regions.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.NORMAL)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PRELOADED)); + } + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.NORMAL)); + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndNormalDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "Normal"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.NORMAL)); + } + + @Test + public void testResolveDataPolicyWhenNotPersistentAndNormalDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "NORMAL"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.NORMAL)); + } + + @Test + public void testResolveDataPolicyWhenPersistentAndNormalDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "NORMAL"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndPreloadedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "preloaded"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PRELOADED)); + } + + @Test + public void testResolveDataPolicyWhenNotPersistentAndPreloadedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "PreLoaded"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PRELOADED)); + } + + @Test + public void testResolveDataPolicyWhenPersistentAndPreloadedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "PRELOADED"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + } diff --git a/src/test/java/org/springframework/data/gemfire/PartitionedRegionFactoryBeanTest.java b/src/test/java/org/springframework/data/gemfire/PartitionedRegionFactoryBeanTest.java new file mode 100644 index 00000000..f8238e85 --- /dev/null +++ b/src/test/java/org/springframework/data/gemfire/PartitionedRegionFactoryBeanTest.java @@ -0,0 +1,168 @@ +/* + * 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.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import org.junit.Test; + +import com.gemstone.gemfire.cache.DataPolicy; +import com.gemstone.gemfire.cache.RegionFactory; + +/** + * The PartitionedRegionFactoryBeanTest class is a test suite of test cases testing the component functionality + * and correct behavior of the PartitionedRegionFactoryBean class. + *

+ * @author John Blum + * @see org.mockito.Mockito + * @see org.junit.Test + * @see org.springframework.data.gemfire.PartitionedRegionFactoryBean + * @since 1.3.3 + */ +@SuppressWarnings("unchecked") +public class PartitionedRegionFactoryBeanTest { + + private final PartitionedRegionFactoryBean factoryBean = new PartitionedRegionFactoryBean(); + + protected RegionFactory createMockRegionFactory() { + return mock(RegionFactory.class); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithInvalidDataPolicyName() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.resolveDataPolicy(mockRegionFactory, null, "INVALID_DATA_POLICY_NAME"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'INVALID_DATA_POLICY_NAME' is invalid.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithInvalidDataPolicyRegionType() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.resolveDataPolicy(mockRegionFactory, null, "REPLICATE"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'REPLICATE' is not supported in Partitioned Regions.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.REPLICATE)); + } + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PARTITION)); + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "PARTITION"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PARTITION)); + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndPersistentPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "PERSISTENT_PARTITION"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_PARTITION)); + } + + @Test + public void testResolveDataPolicyWithPersistentFalseAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PARTITION)); + } + + @Test + public void testResolveDataPolicyWithPersistentTrueAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_PARTITION)); + } + + @Test + public void testResolveDataPolicyWithPersistentFalseAndPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "PARTITION"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PARTITION)); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithPersistentFalseAndPersistentPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "PERSISTENT_PARTITION"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'PERSISTENT_PARTITION' is invalid when persistent is false.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_PARTITION)); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithPersistentTrueAndPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "PARTITION"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'PARTITION' is invalid when persistent is true.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PARTITION)); + } + } + + @Test + public void testResolveDataPolicyWithPersistentTrueAndPersistentPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "PERSISTENT_PARTITION"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_PARTITION)); + } + +} diff --git a/src/test/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBeanTest.java b/src/test/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBeanTest.java new file mode 100644 index 00000000..5553f596 --- /dev/null +++ b/src/test/java/org/springframework/data/gemfire/ReplicatedRegionFactoryBeanTest.java @@ -0,0 +1,204 @@ +/* + * 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.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import org.junit.Test; + +import com.gemstone.gemfire.cache.DataPolicy; +import com.gemstone.gemfire.cache.RegionFactory; + +/** + * The ReplicatedRegionFactoryBeanTest class is a test suite of test cases testing the component functionality + * and correct behavior of the ReplicatedRegionFactoryBean class. + *

+ * @author John Blum + * @see org.mockito.Mockito + * @see org.junit.Test + * @see org.springframework.data.gemfire.ReplicatedRegionFactoryBean + * @since 1.3.3 + */ +@SuppressWarnings("unchecked") +public class ReplicatedRegionFactoryBeanTest { + + private final ReplicatedRegionFactoryBean factoryBean = new ReplicatedRegionFactoryBean(); + + protected RegionFactory createMockRegionFactory() { + return mock(RegionFactory.class); + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.REPLICATE)); + } + + @Test + public void testResolveDataPolicyWhenNotPersistentAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.REPLICATE)); + } + + @Test + public void testResolveDataPolicyWhenPersistentAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndEmptyDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "EMPTY"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.EMPTY)); + } + + @Test + public void testResolveDataPolicyWhenNotPersistentAndEmptyDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "empty"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.EMPTY)); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWhenPersistentAndEmptyDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + try { + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "empty"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'EMPTY' is invalid when persistent is true.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.EMPTY)); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithPersistentUnspecifiedAndInvalidDataPolicyName() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.resolveDataPolicy(mockRegionFactory, null, "INVALID_DATA_POLICY_NAME"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'INVALID_DATA_POLICY_NAME' is invalid.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.REPLICATE)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithPersistentUnspecifiedAndInvalidDataPolicyRegionType() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.resolveDataPolicy(mockRegionFactory, null, "PARTITION"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'PARTITION' is not supported in Replicated Regions.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PARTITION)); + } + } + + @Test + public void testResolveDataPolicyWhenPersistentUnspecifiedAndReplicatedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "REPLICATE"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.REPLICATE)); + } + + @Test + public void testResolveDataPolicyWhenNotPersistentAndReplicatedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "REPLICATE"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.REPLICATE)); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWhenPersistentAndReplicatedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "REPLICATE"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'REPLICATE' is invalid when persistent is true.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.REPLICATE)); + } + } + + @Test + public void testResolveDataPolicyWhenPersistentUnspecifiedAndPersistentReplicatedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "PERSISTENT_REPLICATE"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWhenNotPersistentAndPersistentReplicatedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + + try { + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "PERSISTENT_REPLICATE"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'PERSISTENT_REPLICATE' is invalid when persistent is false.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.REPLICATE)); + } + } + + @Test + public void testResolveDataPolicyWhenPersistentAndPersistentReplicatedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "PERSISTENT_REPLICATE"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + +} From 70f4ddbd35964e0681e99f53e17ad7562e41003a Mon Sep 17 00:00:00 2001 From: John Blum Date: Fri, 25 Oct 2013 16:51:00 -0700 Subject: [PATCH 3/5] Additional tests cases for SGF-203 involving persistence. --- .../data/gemfire/RegionFactoryBeanTest.java | 129 +++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/springframework/data/gemfire/RegionFactoryBeanTest.java b/src/test/java/org/springframework/data/gemfire/RegionFactoryBeanTest.java index 738e6cf6..0c12941d 100644 --- a/src/test/java/org/springframework/data/gemfire/RegionFactoryBeanTest.java +++ b/src/test/java/org/springframework/data/gemfire/RegionFactoryBeanTest.java @@ -17,18 +17,27 @@ package org.springframework.data.gemfire; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import org.junit.Test; import org.springframework.data.gemfire.support.AbstractRegionFactoryBeanTest; import com.gemstone.gemfire.cache.DataPolicy; import com.gemstone.gemfire.cache.Region; +import com.gemstone.gemfire.cache.RegionFactory; /** * @author David Turanski - * + * @author John Blum */ +@SuppressWarnings("unchecked") public class RegionFactoryBeanTest extends AbstractRegionFactoryBeanTest { + private final RegionFactoryBean factoryBean = new RegionFactoryBean(); + @SuppressWarnings("rawtypes") private RegionFactoryBeanConfig defaultConfig() { return new RegionFactoryBeanConfig(new RegionFactoryBean(), "default") { @@ -77,8 +86,8 @@ public class RegionFactoryBeanTest extends AbstractRegionFactoryBeanTest { @Override public void verify() { assertNotNull(this.exception); - assertEquals("Data policy persistent_replicate is invalid when persistent is false", - exception.getMessage()); + assertEquals("Data Policy 'PERSISTENT_REPLICATE' is invalid when persistent is false.", + exception.getMessage()); } }; } @@ -89,4 +98,118 @@ public class RegionFactoryBeanTest extends AbstractRegionFactoryBeanTest { addRFBConfig(persistent()); addRFBConfig(invalidPersistence()); } + + protected RegionFactory createMockRegionFactory() { + return mock(RegionFactory.class); + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.DEFAULT)); + } + + @Test + public void testResolveDataPolicyWhenNotPersistentAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.DEFAULT)); + } + + @Test + public void testResolveDataPolicyWhenPersistentAndDataPolicyUnspecified() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, null); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWithInvalidDataPolicyName() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + try { + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "CSV"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'CSV' is invalid.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.DEFAULT)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndNormalDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "NORMAL"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.NORMAL)); + } + + @Test + public void testResolveDataPolicyWhenNotPersistentUnspecifiedAndPreloadedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "PRELOADED"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PRELOADED)); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWhenPersistentAndPersistentEmptyDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + try { + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "EMPTY"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'EMPTY' is invalid when persistent is true.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.DEFAULT)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.EMPTY)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + } + + @Test + public void testResolveDataPolicyWithPersistentUnspecifiedAndPersistentPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.resolveDataPolicy(mockRegionFactory, null, "PERSISTENT_PARTITION"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_PARTITION)); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveDataPolicyWhenNotPersistentAndPersistentPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + try { + factoryBean.setPersistent(false); + factoryBean.resolveDataPolicy(mockRegionFactory, false, "PERSISTENT_PARTITION"); + } + catch (IllegalArgumentException e) { + assertEquals("Data Policy 'PERSISTENT_PARTITION' is invalid when persistent is false.", e.getMessage()); + throw e; + } + finally { + verify(mockRegionFactory, never()).setDataPolicy(null); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.DEFAULT)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_PARTITION)); + verify(mockRegionFactory, never()).setDataPolicy(eq(DataPolicy.PERSISTENT_REPLICATE)); + } + } + + @Test + public void testResolveDataPolicyWhenPersistentAndPersistentPartitionedDataPolicy() { + RegionFactory mockRegionFactory = createMockRegionFactory(); + factoryBean.setPersistent(true); + factoryBean.resolveDataPolicy(mockRegionFactory, true, "PERSISTENT_PARTITION"); + verify(mockRegionFactory).setDataPolicy(eq(DataPolicy.PERSISTENT_PARTITION)); + } + } From 339393b7945a7b07c7570a9f8bcfd5053fe2c2d9 Mon Sep 17 00:00:00 2001 From: John Blum Date: Mon, 28 Oct 2013 21:21:13 -0700 Subject: [PATCH 4/5] Additional fixes for SGF-203 involving the persistent, data-policy and shortcut attributes of client Regions. --- .../client/ClientRegionFactoryBean.java | 250 +++++++++------ .../gemfire/config/ClientRegionParser.java | 121 +++----- .../client/ClientRegionFactoryBeanTest.java | 286 ++++++++++++++++-- .../config/ClientRegionNamespaceTest.java | 1 - ...tRegionUsingDataPolicyAndShortcutTest.java | 47 +++ ...t-region-using-datapolicy-and-shortcut.xml | 21 ++ 6 files changed, 525 insertions(+), 201 deletions(-) create mode 100644 src/test/java/org/springframework/data/gemfire/config/ClientRegionUsingDataPolicyAndShortcutTest.java create mode 100644 src/test/resources/org/springframework/data/gemfire/config/client-region-using-datapolicy-and-shortcut.xml diff --git a/src/main/java/org/springframework/data/gemfire/client/ClientRegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/client/ClientRegionFactoryBean.java index 1e71f5a3..941e9214 100644 --- a/src/main/java/org/springframework/data/gemfire/client/ClientRegionFactoryBean.java +++ b/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. + *

* @author Costin Leau * @author David Turanski + * @author John Blum */ public class ClientRegionFactoryBean extends RegionLookupFactoryBean 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 cacheListeners[]; - - private Interest[] interests; - - private String poolName; - private BeanFactory beanFactory; + private Boolean persistent; + + private CacheListener[] cacheListeners; + private ClientRegionShortcut shortcut = null; private DataPolicy dataPolicy; + private Interest[] interests; + private RegionAttributes attributes; - private Region 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 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 factory = c.createClientRegionFactory(s); + ClientRegionFactory factory = clientCache.createClientRegionFactory(resolveClientRegionShortcut()); // map the attributes onto the client if (attributes != null) { @@ -173,7 +134,8 @@ public class ClientRegionFactoryBean extends RegionLookupFactoryBean } factory.setPoolName(poolName); - } else { + } + else { Pool pool = beanFactory.getBean(Pool.class); factory.setPoolName(pool.getName()); } @@ -182,26 +144,63 @@ public class ClientRegionFactoryBean extends RegionLookupFactoryBean factory.setDiskStoreName(diskStoreName); } - Region reg = factory.create(regionName); + Region 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 region) { if (!ObjectUtils.isEmpty(interests)) { for (Interest 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 extends RegionLookupFactoryBean 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 attributes) { + this.attributes = attributes; + } + @Override public void setBeanFactory(BeanFactory beanFactory) throws BeansException { this.beanFactory = beanFactory; @@ -269,19 +282,9 @@ public class ClientRegionFactoryBean extends RegionLookupFactoryBean 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 extends RegionLookupFactoryBean 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 extends RegionLookupFactoryBean 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 elements + * are compatible. + *

+ * @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. + *

+ * @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. + *

+ * @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 extends RegionLookupFactoryBean 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 attributes) { - this.attributes = attributes; - } -} \ No newline at end of file +} diff --git a/src/main/java/org/springframework/data/gemfire/config/ClientRegionParser.java b/src/main/java/org/springframework/data/gemfire/config/ClientRegionParser.java index 7e61d11b..642cc283 100644 --- a/src/main/java/org/springframework/data/gemfire/config/ClientRegionParser.java +++ b/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. + *

+ * To avoid eager evaluations, the region interests are declared as nested definition. + *

* @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 interests = new ManagedList(); // parse nested declarations List 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"); } + } diff --git a/src/test/java/org/springframework/data/gemfire/client/ClientRegionFactoryBeanTest.java b/src/test/java/org/springframework/data/gemfire/client/ClientRegionFactoryBeanTest.java index 751f8da7..d05e86ff 100644 --- a/src/test/java/org/springframework/data/gemfire/client/ClientRegionFactoryBeanTest.java +++ b/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 fb = new ClientRegionFactoryBean(); - 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 factoryBean; - String regionName = "regionName"; - ClientCache cache = Mockito.mock(ClientCache.class); - - @SuppressWarnings("unchecked") - ClientRegionFactory factory = Mockito - .mock(ClientRegionFactory.class); - Mockito.when( - cache.createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY)) - .thenReturn(factory); - - @SuppressWarnings("unchecked") - Region region = Mockito.mock(Region.class); - Mockito.when(factory.create(regionName)).thenReturn(region); - - Region result = fb.lookupFallback(cache, regionName); - - assertSame(region, result); + @Before + public void setup() { + factoryBean = new ClientRegionFactoryBean(); } + + @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 clientRegionFactory = mock(ClientRegionFactory.class); + Region expectedRegion = mock(Region.class); + + when(cache.createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY)).thenReturn(clientRegionFactory); + when(clientRegionFactory.create("testRegion")).thenReturn(expectedRegion); + + Region 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()); + } + } diff --git a/src/test/java/org/springframework/data/gemfire/config/ClientRegionNamespaceTest.java b/src/test/java/org/springframework/data/gemfire/config/ClientRegionNamespaceTest.java index e9139cc3..8477ad17 100644 --- a/src/test/java/org/springframework/data/gemfire/config/ClientRegionNamespaceTest.java +++ b/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()); diff --git a/src/test/java/org/springframework/data/gemfire/config/ClientRegionUsingDataPolicyAndShortcutTest.java b/src/test/java/org/springframework/data/gemfire/config/ClientRegionUsingDataPolicyAndShortcutTest.java new file mode 100644 index 00000000..91727065 --- /dev/null +++ b/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. + *

+ * @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; + } + } + +} diff --git a/src/test/resources/org/springframework/data/gemfire/config/client-region-using-datapolicy-and-shortcut.xml b/src/test/resources/org/springframework/data/gemfire/config/client-region-using-datapolicy-and-shortcut.xml new file mode 100644 index 00000000..589ef054 --- /dev/null +++ b/src/test/resources/org/springframework/data/gemfire/config/client-region-using-datapolicy-and-shortcut.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + From 0f819f2e04382756d50be6177e7f5766012ed8a3 Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 30 Oct 2013 13:34:53 -0700 Subject: [PATCH 5/5] Code changes based on code review by David Turanski in PR #33 for JIRA issue SGF-203 concerning persistence. --- .../data/gemfire/GemfireUtils.java | 68 +++++++++++++++++++ .../data/gemfire/LocalRegionFactoryBean.java | 2 +- .../gemfire/PartitionedRegionFactoryBean.java | 9 +-- .../gemfire/config/AbstractRegionParser.java | 5 +- .../gemfire/config/AsyncEventQueueParser.java | 3 +- .../data/gemfire/config/ParsingUtils.java | 60 ++-------------- .../data/gemfire/GemfireUtilsTest.java | 67 ++++++++++++++++++ .../config/GemfireV7GatewayNamespaceTest.java | 3 +- 8 files changed, 151 insertions(+), 66 deletions(-) create mode 100644 src/main/java/org/springframework/data/gemfire/GemfireUtils.java create mode 100644 src/test/java/org/springframework/data/gemfire/GemfireUtilsTest.java diff --git a/src/main/java/org/springframework/data/gemfire/GemfireUtils.java b/src/main/java/org/springframework/data/gemfire/GemfireUtils.java new file mode 100644 index 00000000..b593b42b --- /dev/null +++ b/src/main/java/org/springframework/data/gemfire/GemfireUtils.java @@ -0,0 +1,68 @@ +/* + * 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 java.util.concurrent.ConcurrentMap; + +import org.springframework.util.ClassUtils; + +import com.gemstone.gemfire.cache.CacheFactory; +import com.gemstone.gemfire.cache.Region; + +/** + * The GemfireUtils class is a utility class encapsulating common functionality to access features and capabilities + * of GemFire based on version and other configuration meta-data. + *

+ * @author John Blum + * @since 1.3.3 + */ +public abstract class GemfireUtils { + + public final static String GEMFIRE_VERSION = CacheFactory.getVersion(); + + public static boolean isGemfireVersion65OrAbove() { + // expected 'major.minor' + try { + double version = Double.parseDouble(GEMFIRE_VERSION.substring(0, 3)); + return version >= 6.5; + } + catch (NumberFormatException e) { + // NOTE based on logic from the PartitionedRegionFactoryBean class... + return ConcurrentMap.class.isAssignableFrom(Region.class); + } + } + + public static boolean isGemfireVersion7OrAbove() { + try { + int version = Integer.parseInt(GEMFIRE_VERSION.substring(0, 1)); + return version >= 7; + } + catch (NumberFormatException e) { + // NOTE the com.gemstone.gemfire.distributed.ServerLauncher class only exists in GemFire v 7.0.x or above... + return ClassUtils.isPresent("com.gemstone.gemfire.distributed.ServerLauncher", + Thread.currentThread().getContextClassLoader()); + } + + } + + public static void main(final String... args) { + System.out.printf("GemFire Version %1$s%n", GEMFIRE_VERSION); + //System.out.printf("Is GemFire Version 6.5 of Above? %1$s%n", isGemfireVersion65OrAbove()); + //System.out.printf("Is GemFire Version 7.0 of Above? %1$s%n", isGemfireVersion7OrAbove()); + } + +} diff --git a/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java index 59206a91..f2012a3e 100644 --- a/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/LocalRegionFactoryBean.java @@ -55,7 +55,7 @@ public class LocalRegionFactoryBean extends RegionFactoryBean { protected void resolveDataPolicy(RegionFactory regionFactory, Boolean persistent, String dataPolicy) { DataPolicy resolvedDataPolicy = new DataPolicyConverter().convert(dataPolicy); - Assert.isTrue(resolvedDataPolicy != null || (resolvedDataPolicy == null && !StringUtils.hasText(dataPolicy)), + Assert.isTrue(resolvedDataPolicy != null || !StringUtils.hasText(dataPolicy), String.format("Data Policy '%1$s' is invalid.", dataPolicy)); if (resolvedDataPolicy == null || DataPolicy.NORMAL.equals(resolvedDataPolicy)) { diff --git a/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java b/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java index 546093e4..85dd5586 100644 --- a/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/PartitionedRegionFactoryBean.java @@ -15,13 +15,10 @@ */ package org.springframework.data.gemfire; -import java.util.concurrent.ConcurrentMap; - import org.springframework.util.Assert; import com.gemstone.gemfire.cache.CacheFactory; import com.gemstone.gemfire.cache.DataPolicy; -import com.gemstone.gemfire.cache.Region; import com.gemstone.gemfire.cache.RegionFactory; /** @@ -49,7 +46,7 @@ public class PartitionedRegionFactoryBean extends RegionFactoryBean } else if (isPersistent()) { // first, check the presence of GemFire 6.5 or Higher - Assert.isTrue(isGemFireVersion65orHigher(), String.format( + Assert.isTrue(GemfireUtils.isGemfireVersion65OrAbove(), String.format( "Can define Persistent Partitioned Regions only from GemFire 6.5 onwards; current version is [%1$s]", CacheFactory.getVersion())); regionFactory.setDataPolicy(DataPolicy.PERSISTENT_PARTITION); @@ -59,8 +56,4 @@ public class PartitionedRegionFactoryBean extends RegionFactoryBean } } - private boolean isGemFireVersion65orHigher() { - return ConcurrentMap.class.isAssignableFrom(Region.class); - } - } diff --git a/src/main/java/org/springframework/data/gemfire/config/AbstractRegionParser.java b/src/main/java/org/springframework/data/gemfire/config/AbstractRegionParser.java index 9bd816bb..2856f47c 100644 --- a/src/main/java/org/springframework/data/gemfire/config/AbstractRegionParser.java +++ b/src/main/java/org/springframework/data/gemfire/config/AbstractRegionParser.java @@ -28,6 +28,7 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.ManagedArray; import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; +import org.springframework.data.gemfire.GemfireUtils; import org.springframework.data.gemfire.SubRegionFactoryBean; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; @@ -100,14 +101,14 @@ abstract class AbstractRegionParser extends AbstractSingleBeanDefinitionParser { String hubId = element.getAttribute("hub-id"); // Factory will enable gateway if it is not set and hub-id is set. if (StringUtils.hasText(enableGateway)) { - if (ParsingUtils.isGemfireV7OrAbove()) { + if (GemfireUtils.isGemfireVersion7OrAbove()) { log.warn("'enable-gateway' is deprecated since Gemfire 7.0"); } } ParsingUtils.setPropertyValue(element, builder, "enable-gateway"); if (StringUtils.hasText(hubId)) { - if (ParsingUtils.isGemfireV7OrAbove()) { + if (GemfireUtils.isGemfireVersion7OrAbove()) { log.warn("'hub-id' is deprecated since Gemfire 7.0"); } if (!CollectionUtils.isEmpty(DomUtils.getChildElementsByTagName(element, "gateway-sender"))) { diff --git a/src/main/java/org/springframework/data/gemfire/config/AsyncEventQueueParser.java b/src/main/java/org/springframework/data/gemfire/config/AsyncEventQueueParser.java index ec0e1ce3..661bc270 100644 --- a/src/main/java/org/springframework/data/gemfire/config/AsyncEventQueueParser.java +++ b/src/main/java/org/springframework/data/gemfire/config/AsyncEventQueueParser.java @@ -18,6 +18,7 @@ package org.springframework.data.gemfire.config; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; +import org.springframework.data.gemfire.GemfireUtils; import org.springframework.data.gemfire.wan.AsyncEventQueueFactoryBean; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; @@ -58,7 +59,7 @@ public class AsyncEventQueueParser extends AbstractSingleBeanDefinitionParser { ParsingUtils.setPropertyValue(element, builder, "persistent"); ParsingUtils.setPropertyValue(element, builder, "parallel"); - if (ParsingUtils.GEMFIRE_VERSION.compareTo("7.0.1") >= 0 ) { + if (GemfireUtils.GEMFIRE_VERSION.compareTo("7.0.1") >= 0 ) { ParsingUtils.setPropertyValue(element, builder, "batch-conflation-enabled"); ParsingUtils.setPropertyValue(element, builder, "disk-synchronous"); ParsingUtils.setPropertyValue(element, builder, "batch-time-interval"); diff --git a/src/main/java/org/springframework/data/gemfire/config/ParsingUtils.java b/src/main/java/org/springframework/data/gemfire/config/ParsingUtils.java index fdfb591e..b8455c83 100644 --- a/src/main/java/org/springframework/data/gemfire/config/ParsingUtils.java +++ b/src/main/java/org/springframework/data/gemfire/config/ParsingUtils.java @@ -20,22 +20,17 @@ import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.beans.BeanMetadataAttribute; -import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.config.BeanDefinitionHolder; import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.ManagedArray; import org.springframework.beans.factory.support.ManagedList; -import org.springframework.beans.factory.xml.AbstractBeanDefinitionParser; -import org.springframework.beans.factory.xml.BeanDefinitionParserDelegate; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.core.Conventions; +import org.springframework.data.gemfire.GemfireUtils; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; -import com.gemstone.gemfire.cache.CacheFactory; import com.gemstone.gemfire.cache.ExpirationAction; import com.gemstone.gemfire.cache.ExpirationAttributes; import com.gemstone.gemfire.cache.LossAction; @@ -49,16 +44,11 @@ import com.gemstone.gemfire.cache.Scope; * @author Costin Leau * @author David Turanski * @author Lyndon Adams + * @author John Blum */ abstract class ParsingUtils { - - private static Log log = LogFactory.getLog(ParsingUtils.class); - - final static String GEMFIRE_VERSION = CacheFactory.getVersion(); - - private static final String ALIASES_KEY = ParsingUtils.class.getName() + ":aliases"; - + private static final Log log = LogFactory.getLog(ParsingUtils.class); static void setPropertyValue(Element element, BeanDefinitionBuilder builder, String attributeName, String propertyName, Object defaultValue) { @@ -90,35 +80,6 @@ abstract class ParsingUtils { } } - /** - * Utility for parsing bean aliases. Normally parsed by - * AbstractBeanDefinitionParser however due to the attribute clash (bean - * uses 'name' for aliases while region use it to indicate their name), the - * parser needs to handle this differently by storing them as metadata which - * gets deleted just before registration. - * - * @param element - * @param builder - */ - static void addBeanAliasAsMetadata(Element element, BeanDefinitionBuilder builder) { - String[] aliases = new String[0]; - String name = element.getAttributeNS(BeanDefinitionParserDelegate.BEANS_NAMESPACE_URI, - AbstractBeanDefinitionParser.NAME_ATTRIBUTE); - - if (StringUtils.hasLength(name)) { - aliases = StringUtils.trimArrayElements(StringUtils.commaDelimitedListToStringArray(name)); - } - BeanMetadataAttribute attr = new BeanMetadataAttribute(ALIASES_KEY, aliases); - attr.setSource(element); - builder.getRawBeanDefinition().addMetadataAttribute(attr); - } - - static BeanDefinitionHolder replaceBeanAliasAsMetadata(BeanDefinitionHolder holder) { - BeanDefinition beanDefinition = holder.getBeanDefinition(); - return new BeanDefinitionHolder(beanDefinition, holder.getBeanName(), - (String[]) beanDefinition.removeAttribute(ALIASES_KEY)); - } - /** * Utility method handling parsing of nested definition of the type: * @@ -349,7 +310,7 @@ abstract class ParsingUtils { String concurrencyChecksEnabled = element.getAttribute("concurrency-checks-enabled"); if (StringUtils.hasText(concurrencyChecksEnabled)) { - if (!ParsingUtils.isGemfireV7OrAbove()) { + if (!GemfireUtils.isGemfireVersion7OrAbove()) { log.warn("'concurrency-checks-enabled' is only available in Gemfire 7.0 or above"); } else { ParsingUtils.setPropertyValue(element, attrBuilder, "concurrency-checks-enabled"); @@ -387,21 +348,14 @@ abstract class ParsingUtils { } static void throwExceptionIfNotGemfireV7(String elementName, String attributeName, ParserContext parserContext) { - if (!isGemfireV7OrAbove()) { + if (!GemfireUtils.isGemfireVersion7OrAbove()) { String messagePrefix = (attributeName == null) ? "element '" + elementName + "'" : "attribute '" + attributeName + " of element '" + elementName + "'"; parserContext.getReaderContext().error( - messagePrefix + " requires Gemfire version 7 or later. The current version is " + GEMFIRE_VERSION, + messagePrefix + " requires Gemfire version 7 or later. The current version is " + GemfireUtils.GEMFIRE_VERSION, null); } } - - static boolean isGemfireV7OrAbove() { - - int version = Integer.parseInt(GEMFIRE_VERSION.substring(0,1)); - return version >= 7; - - } static void parseScope(Element element, BeanDefinitionBuilder builder) { String scope = element.getAttribute("scope"); @@ -469,4 +423,4 @@ abstract class ParsingUtils { return true; } -} \ No newline at end of file +} diff --git a/src/test/java/org/springframework/data/gemfire/GemfireUtilsTest.java b/src/test/java/org/springframework/data/gemfire/GemfireUtilsTest.java new file mode 100644 index 00000000..3e3ab4d3 --- /dev/null +++ b/src/test/java/org/springframework/data/gemfire/GemfireUtilsTest.java @@ -0,0 +1,67 @@ +/* + * 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.Assume.assumeTrue; + +import org.junit.Test; + +import com.gemstone.gemfire.internal.GemFireVersion; + +/** + * The GemfireUtilsTest class is a test suite of test cases testing the contract and functionality of the GemfireUtils + * abstract utility class. + *

+ * @author John Blum + * @see org.junit.Test + * @see org.springframework.data.gemfire.GemfireUtils + * @since 1.3.3 + */ +public class GemfireUtilsTest { + + // NOTE implementation is based on a GemFire internal class... com.gemstone.gemfire.internal.GemFireVersion. + protected int getGemFireVersion() { + try { + String gemfireVersion = GemFireVersion.getGemFireVersion(); + StringBuilder buffer = new StringBuilder(); + + buffer.append(GemFireVersion.getMajorVersion(gemfireVersion)); + buffer.append(GemFireVersion.getMinorVersion(gemfireVersion)); + + return Integer.decode(buffer.toString()); + } + catch (NumberFormatException ignore) { + return -1; + } + } + + @Test + public void testIsGemfireVersion65OrAbove() { + int gemfireVersion = getGemFireVersion(); + assumeTrue(gemfireVersion > -1); + assertEquals(getGemFireVersion() >= 65, GemfireUtils.isGemfireVersion65OrAbove()); + } + + @Test + public void testIsGemfireVersion7OrAbove() { + int gemfireVersion = getGemFireVersion(); + assumeTrue(gemfireVersion > -1); + assertEquals(getGemFireVersion() >= 70, GemfireUtils.isGemfireVersion7OrAbove()); + } + +} diff --git a/src/test/java/org/springframework/data/gemfire/config/GemfireV7GatewayNamespaceTest.java b/src/test/java/org/springframework/data/gemfire/config/GemfireV7GatewayNamespaceTest.java index ea0260f0..de71944c 100644 --- a/src/test/java/org/springframework/data/gemfire/config/GemfireV7GatewayNamespaceTest.java +++ b/src/test/java/org/springframework/data/gemfire/config/GemfireV7GatewayNamespaceTest.java @@ -28,6 +28,7 @@ import java.util.List; import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; +import org.springframework.data.gemfire.GemfireUtils; import org.springframework.data.gemfire.RecreatingContextTest; import org.springframework.data.gemfire.RegionFactoryBean; import org.springframework.data.gemfire.TestUtils; @@ -71,7 +72,7 @@ public class GemfireV7GatewayNamespaceTest extends RecreatingContextTest { @Before @Override public void createCtx() { - if (ParsingUtils.GEMFIRE_VERSION.startsWith("7")) { + if (GemfireUtils.GEMFIRE_VERSION.startsWith("7")) { super.createCtx(); } }