From 0f819f2e04382756d50be6177e7f5766012ed8a3 Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 30 Oct 2013 13:34:53 -0700 Subject: [PATCH] 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(); } }