From dbba298a2c170284c4e5fd7ab8238fe1c8fafc0d Mon Sep 17 00:00:00 2001 From: John Blum Date: Thu, 11 May 2017 18:50:52 -0700 Subject: [PATCH] DATAGEODE-8 - Ensure locators and servers and configured correctly when using attributes. Related JIRA: https://jira.spring.io/browse/SGF-628. (cherry picked from commit ffb8c704f0a785f6f441c871756fcc802fd5be81) Signed-off-by: John Blum --- .../data/gemfire/client/PoolFactoryBean.java | 48 +++++++++++-------- .../data/gemfire/config/xml/PoolParser.java | 14 +++--- .../config/xml/PoolParserUnitTests.java | 8 +++- ...lientCacheVariableLocatorsTest-context.xml | 9 ++-- ...ClientCacheVariableServersTest-context.xml | 15 +++--- 5 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/springframework/data/gemfire/client/PoolFactoryBean.java b/src/main/java/org/springframework/data/gemfire/client/PoolFactoryBean.java index a05d9430..ac8bc9b2 100644 --- a/src/main/java/org/springframework/data/gemfire/client/PoolFactoryBean.java +++ b/src/main/java/org/springframework/data/gemfire/client/PoolFactoryBean.java @@ -120,7 +120,7 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis public void afterPropertiesSet() throws Exception { if (!StringUtils.hasText(name)) { Assert.hasText(beanName, "Pool 'name' is required"); - name = beanName; + this.name = beanName; } // check for an existing, configured Pool with name first @@ -131,15 +131,15 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis log.debug(String.format("A Pool with name [%1$s] already exists; using existing Pool.", name)); } - springBasedPool = false; - pool = existingPool; + this.springBasedPool = false; + this.pool = existingPool; } else { if (log.isDebugEnabled()) { log.debug(String.format("No Pool with name [%1$s] was found. Creating new Pool.", name)); } - springBasedPool = true; + this.springBasedPool = true; } } @@ -166,7 +166,7 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis /* (non-Javadoc) */ @Override public Pool getObject() throws Exception { - if (pool == null) { + if (this.pool == null) { eagerlyInitializeClientCacheIfNotPresent(); PoolFactory poolFactory = createPoolFactory(); @@ -190,11 +190,11 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis poolFactory.setSubscriptionRedundancy(subscriptionRedundancy); poolFactory.setThreadLocalConnections(threadLocalConnections); - for (ConnectionEndpoint locator : locators) { + for (ConnectionEndpoint locator : this.locators) { poolFactory.addLocator(locator.getHost(), locator.getPort()); } - for (ConnectionEndpoint server : servers) { + for (ConnectionEndpoint server : this.servers) { poolFactory.addServer(server.getHost(), server.getPort()); } @@ -204,6 +204,18 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis return pool; } + /** + * Determines whether the GemFire DistributedSystem exists yet or not. + * + * @return a boolean value indicating whether the single, GemFire DistributedSystem has been created already. + * @see org.springframework.data.gemfire.GemfireUtils#getDistributedSystem() + * @see org.springframework.data.gemfire.GemfireUtils#isConnected(DistributedSystem) + * @see org.apache.geode.distributed.DistributedSystem + */ + boolean isDistributedSystemPresent() { + return GemfireUtils.isConnected(GemfireUtils.getDistributedSystem()); + } + /** * Attempts to eagerly initialize the GemFire {@link ClientCache} if not already present so that the single * {@link org.apache.geode.distributed.DistributedSystem} will exists, which is required to create @@ -218,18 +230,6 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis } } - /** - * Determines whether the GemFire DistributedSystem exists yet or not. - * - * @return a boolean value indicating whether the single, GemFire DistributedSystem has been created already. - * @see org.springframework.data.gemfire.GemfireUtils#getDistributedSystem() - * @see org.springframework.data.gemfire.GemfireUtils#isConnected(DistributedSystem) - * @see org.apache.geode.distributed.DistributedSystem - */ - boolean isDistributedSystemPresent() { - return GemfireUtils.isConnected(GemfireUtils.getDistributedSystem()); - } - /** * Creates an instance of the GemFire {@link PoolFactory} interface to construct, configure and initialize * a GemFire {@link Pool}. @@ -245,7 +245,7 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis /* (non-Javadoc) */ @Override public Class getObjectType() { - return (pool != null ? pool.getClass() : Pool.class); + return (this.pool != null ? this.pool.getClass() : Pool.class); } /* (non-Javadoc) */ @@ -595,4 +595,12 @@ public class PoolFactoryBean implements FactoryBean, InitializingBean, Dis public void setThreadLocalConnections(boolean threadLocalConnections) { this.threadLocalConnections = threadLocalConnections; } + + /* (non-Javadoc; internal framework use only) */ + public final void setLocatorsConfiguration(Object locatorsConfiguration) { + } + + /* (non-Javadoc; internal framework use only) */ + public final void setServersConfiguration(Object serversConfiguration) { + } } diff --git a/src/main/java/org/springframework/data/gemfire/config/xml/PoolParser.java b/src/main/java/org/springframework/data/gemfire/config/xml/PoolParser.java index 540899d1..a6040bd1 100644 --- a/src/main/java/org/springframework/data/gemfire/config/xml/PoolParser.java +++ b/src/main/java/org/springframework/data/gemfire/config/xml/PoolParser.java @@ -128,8 +128,8 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { } } - boolean locatorsSet = parseLocators(element, getRegistry(parserContext)); - boolean serversSet = parseServers(element, getRegistry(parserContext)); + boolean locatorsSet = parseLocators(element, builder, getRegistry(parserContext)); + boolean serversSet = parseServers(element, builder, getRegistry(parserContext)); // NOTE: if neither Locators nor Servers were configured, then setup a connection to a Server // running on localhost, listening on the default CacheServer port 40404 @@ -192,7 +192,7 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { } /* (non-Javadoc) */ - boolean parseLocators(Element element, BeanDefinitionRegistry registry) { + boolean parseLocators(Element element, BeanDefinitionBuilder poolBuilder, BeanDefinitionRegistry registry) { String locatorsAttributeValue = element.getAttribute(LOCATORS_ATTRIBUTE_NAME); if (StringUtils.hasText(locatorsAttributeValue)) { @@ -207,7 +207,8 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { AbstractBeanDefinition addLocatorsMethodInvokingBean = addLocatorsMethodInvokingBeanBuilder.getBeanDefinition(); - BeanDefinitionReaderUtils.registerWithGeneratedName(addLocatorsMethodInvokingBean, registry); + poolBuilder.addPropertyReference("locatorsConfiguration", + BeanDefinitionReaderUtils.registerWithGeneratedName(addLocatorsMethodInvokingBean, registry)); return true; } @@ -222,7 +223,7 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { } /* (non-Javadoc) */ - boolean parseServers(Element element, BeanDefinitionRegistry registry) { + boolean parseServers(Element element, BeanDefinitionBuilder poolBuilder, BeanDefinitionRegistry registry) { String serversAttributeValue = element.getAttribute(SERVERS_ATTRIBUTE_NAME); if (StringUtils.hasText(serversAttributeValue)) { @@ -237,7 +238,8 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { AbstractBeanDefinition addServersMethodInvokingBean = addServersMethodInvokingBeanBuilder.getBeanDefinition(); - BeanDefinitionReaderUtils.registerWithGeneratedName(addServersMethodInvokingBean, registry); + poolBuilder.addPropertyReference("serversConfiguration", + BeanDefinitionReaderUtils.registerWithGeneratedName(addServersMethodInvokingBean, registry)); return true; } diff --git a/src/test/java/org/springframework/data/gemfire/config/xml/PoolParserUnitTests.java b/src/test/java/org/springframework/data/gemfire/config/xml/PoolParserUnitTests.java index 853d383f..161802de 100644 --- a/src/test/java/org/springframework/data/gemfire/config/xml/PoolParserUnitTests.java +++ b/src/test/java/org/springframework/data/gemfire/config/xml/PoolParserUnitTests.java @@ -507,7 +507,9 @@ public class PoolParserUnitTests { doAnswer(answer).when(mockRegistry).registerBeanDefinition(eq(generateBeanName(MethodInvokingBean.class)), any(BeanDefinition.class)); - assertThat(parser.parseLocators(mockElement, mockRegistry)).isTrue(); + BeanDefinitionBuilder poolBuilder = BeanDefinitionBuilder.genericBeanDefinition(); + + assertThat(parser.parseLocators(mockElement, poolBuilder, mockRegistry)).isTrue(); verify(mockElement, times(1)).getAttribute(eq(PoolParser.ID_ATTRIBUTE)); verify(mockElement, times(1)).getAttribute(eq(PoolParser.LOCATORS_ATTRIBUTE_NAME)); @@ -557,7 +559,9 @@ public class PoolParserUnitTests { doAnswer(answer).when(mockRegistry).registerBeanDefinition(eq(generateBeanName(MethodInvokingBean.class)), any(BeanDefinition.class)); - assertThat(parser.parseServers(mockElement, mockRegistry)).isTrue(); + BeanDefinitionBuilder poolBuilder = BeanDefinitionBuilder.genericBeanDefinition(); + + assertThat(parser.parseServers(mockElement, poolBuilder, mockRegistry)).isTrue(); verify(mockElement, times(1)).getAttribute(eq(PoolParser.ID_ATTRIBUTE)); verify(mockElement, times(1)).getAttribute(eq(PoolParser.SERVERS_ATTRIBUTE_NAME)); diff --git a/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableLocatorsTest-context.xml b/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableLocatorsTest-context.xml index 3a8096b4..f1c2241c 100644 --- a/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableLocatorsTest-context.xml +++ b/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableLocatorsTest-context.xml @@ -12,7 +12,7 @@ "> - localhost[11235] + localhost[11235] @@ -21,11 +21,12 @@ warning - - - + + + + diff --git a/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableServersTest-context.xml b/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableServersTest-context.xml index 0772793d..17ed8d4f 100644 --- a/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableServersTest-context.xml +++ b/src/test/resources/org/springframework/data/gemfire/client/ClientCacheVariableServersTest-context.xml @@ -12,9 +12,9 @@ "> - localhost[23579],localhost[23654] - localhost - 24448 + localhost[23579],localhost[23654] + localhost + 24448 @@ -25,11 +25,12 @@ - - - - + + + + +