From 11423cb07960ee415a7ca8a4ac900f92d97fa633 Mon Sep 17 00:00:00 2001 From: John Blum Date: Tue, 12 Sep 2017 15:36:06 -0700 Subject: [PATCH] DATAGEODE-40 - Fix PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests CI failures. --- .../data/gemfire/config/xml/PoolParser.java | 42 +++-- ...ssionConfiguredPoolsIntegrationTests.java} | 178 +++++++++--------- ...nfiguredPoolsIntegrationTests-context.xml} | 4 +- 3 files changed, 119 insertions(+), 105 deletions(-) rename src/test/java/org/springframework/data/gemfire/client/{PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests.java => SpELExpressionConfiguredPoolsIntegrationTests.java} (51%) rename src/test/resources/org/springframework/data/gemfire/client/{PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests-context.xml => SpELExpressionConfiguredPoolsIntegrationTests-context.xml} (91%) 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 a6040bd1..66a3fa08 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 @@ -17,6 +17,7 @@ package org.springframework.data.gemfire.config.xml; import java.util.List; +import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import org.springframework.beans.factory.BeanDefinitionStoreException; @@ -40,7 +41,7 @@ import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; /** - * Bean definition parser for the <gfe:pool> SDG XML namespace (XSD) element. + * Bean definition parser for <gfe:pool> SDG XML namespace (XSD) elements. * * @author Costin Leau * @author David Turanski @@ -52,19 +53,20 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { static final AtomicBoolean INFRASTRUCTURE_COMPONENTS_REGISTERED = new AtomicBoolean(false); - protected static final int DEFAULT_LOCATOR_PORT = GemfireUtils.DEFAULT_LOCATOR_PORT; - protected static final int DEFAULT_SERVER_PORT = GemfireUtils.DEFAULT_CACHE_SERVER_PORT; + static final int DEFAULT_LOCATOR_PORT = GemfireUtils.DEFAULT_LOCATOR_PORT; + static final int DEFAULT_SERVER_PORT = GemfireUtils.DEFAULT_CACHE_SERVER_PORT; - protected static final String DEFAULT_HOST = "localhost"; - protected static final String HOST_ATTRIBUTE_NAME = "host"; - protected static final String LOCATOR_ELEMENT_NAME = "locator"; - protected static final String LOCATORS_ATTRIBUTE_NAME = "locators"; - protected static final String PORT_ATTRIBUTE_NAME = "port"; - protected static final String SERVER_ELEMENT_NAME = "server"; - protected static final String SERVERS_ATTRIBUTE_NAME = "servers"; + static final String DEFAULT_HOST = "localhost"; + static final String HOST_ATTRIBUTE_NAME = "host"; + static final String LOCATOR_ELEMENT_NAME = "locator"; + static final String LOCATORS_ATTRIBUTE_NAME = "locators"; + static final String PORT_ATTRIBUTE_NAME = "port"; + static final String SERVER_ELEMENT_NAME = "server"; + static final String SERVERS_ATTRIBUTE_NAME = "servers"; /* (non-Javadoc) */ - static void registerInfrastructureComponents(ParserContext parserContext) { + private static void registerInfrastructureComponents(ParserContext parserContext) { + if (INFRASTRUCTURE_COMPONENTS_REGISTERED.compareAndSet(false, true)) { AbstractBeanDefinition beanDefinition = BeanDefinitionBuilder .rootBeanDefinition(ClientRegionPoolBeanFactoryPostProcessor.class) @@ -113,10 +115,11 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { List childElements = DomUtils.getChildElements(element); - ManagedList locators = new ManagedList(childElements.size()); - ManagedList servers = new ManagedList(childElements.size()); + ManagedList locators = new ManagedList<>(childElements.size()); + ManagedList servers = new ManagedList<>(childElements.size()); for (Element childElement : childElements) { + String childElementName = childElement.getLocalName(); if (LOCATOR_ELEMENT_NAME.equals(childElementName)) { @@ -131,7 +134,7 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { 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 + // if neither Locators nor Servers were explicitly configured, then setup a connection to a CacheServer // running on localhost, listening on the default CacheServer port 40404 if (childElements.isEmpty() && !(locatorsSet || serversSet)) { servers.add(buildConnection(DEFAULT_HOST, String.valueOf(DEFAULT_SERVER_PORT), true)); @@ -153,6 +156,7 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { /* (non-Javadoc) */ BeanDefinition buildConnection(String host, String port, boolean server) { + BeanDefinitionBuilder connectionEndpointBuilder = BeanDefinitionBuilder.genericBeanDefinition(ConnectionEndpoint.class); @@ -164,6 +168,7 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { /* (non-Javadoc) */ BeanDefinition buildConnections(String expression, boolean server) { + BeanDefinitionBuilder connectionEndpointListBuilder = BeanDefinitionBuilder.genericBeanDefinition(ConnectionEndpointList.class); @@ -193,9 +198,11 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { /* (non-Javadoc) */ boolean parseLocators(Element element, BeanDefinitionBuilder poolBuilder, BeanDefinitionRegistry registry) { + String locatorsAttributeValue = element.getAttribute(LOCATORS_ATTRIBUTE_NAME); if (StringUtils.hasText(locatorsAttributeValue)) { + BeanDefinitionBuilder addLocatorsMethodInvokingBeanBuilder = BeanDefinitionBuilder.genericBeanDefinition(MethodInvokingBean.class); @@ -224,9 +231,11 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { /* (non-Javadoc) */ boolean parseServers(Element element, BeanDefinitionBuilder poolBuilder, BeanDefinitionRegistry registry) { + String serversAttributeValue = element.getAttribute(SERVERS_ATTRIBUTE_NAME); if (StringUtils.hasText(serversAttributeValue)) { + BeanDefinitionBuilder addServersMethodInvokingBeanBuilder = BeanDefinitionBuilder.genericBeanDefinition(MethodInvokingBean.class); @@ -249,8 +258,9 @@ class PoolParser extends AbstractSingleBeanDefinitionParser { /* (non-Javadoc) */ String resolveId(Element element) { - String id = element.getAttribute(ID_ATTRIBUTE); - return (StringUtils.hasText(id) ? id : GemfireConstants.DEFAULT_GEMFIRE_POOL_NAME); + return Optional.ofNullable(element.getAttribute(ID_ATTRIBUTE)) + .filter(StringUtils::hasText) + .orElse(GemfireConstants.DEFAULT_GEMFIRE_POOL_NAME); } /* (non-Javadoc) */ diff --git a/src/test/java/org/springframework/data/gemfire/client/PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests.java b/src/test/java/org/springframework/data/gemfire/client/SpELExpressionConfiguredPoolsIntegrationTests.java similarity index 51% rename from src/test/java/org/springframework/data/gemfire/client/PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests.java rename to src/test/java/org/springframework/data/gemfire/client/SpELExpressionConfiguredPoolsIntegrationTests.java index 6fb0cc90..dfc72873 100644 --- a/src/test/java/org/springframework/data/gemfire/client/PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests.java +++ b/src/test/java/org/springframework/data/gemfire/client/SpELExpressionConfiguredPoolsIntegrationTests.java @@ -16,81 +16,81 @@ package org.springframework.data.gemfire.client; -import static org.assertj.core.api.Assertions.*; -import static org.mockito.ArgumentMatchers.*; -import static org.mockito.Mockito.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.when; +import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.newIllegalArgumentException; +import java.util.Collections; +import java.util.Optional; import java.util.Properties; import org.apache.geode.cache.client.Pool; import org.apache.geode.cache.client.PoolFactory; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.data.gemfire.support.ConnectionEndpoint; import org.springframework.data.gemfire.support.ConnectionEndpointList; -import org.springframework.data.gemfire.util.CollectionUtils; +import org.springframework.data.gemfire.test.mock.MockGemFireObjectsSupport; import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import org.springframework.util.Assert; +import org.springframework.test.context.junit4.SpringRunner; /** - * The PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests class is a test suite of test cases testing the use of - * property placeholder values in the nested <gfe:locator> and <gfe:server> sub-elements - * of the <gfe:pool> element as well as the locators and servers attributes. + * Integration tests that test the use of property placeholders in nested <gfe:locator> and <gfe:server> + * elements of the SDG XML namespace <gfe:pool> element along with testing property placeholders in + * the <gfe:pool> element locators and servers attributes. * * @author John Blum * @see org.junit.Test * @see org.junit.runner.RunWith + * @see org.apache.geode.cache.client.Pool + * @see org.apache.geode.cache.client.PoolFactory * @see org.springframework.test.context.ContextConfiguration - * @see org.springframework.test.context.junit4.SpringJUnit4ClassRunner + * @see org.springframework.test.context.junit4.SpringRunner * @see org.springframework.data.gemfire.client.PoolFactoryBean - * @see org.springframework.data.gemfire.config.PoolParser + * @see org.springframework.data.gemfire.config.xml.PoolParser * @see SGF-433 * @since 1.6.0 */ -@RunWith(SpringJUnit4ClassRunner.class) +@RunWith(SpringRunner.class) @ContextConfiguration -public class PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests { +@SuppressWarnings("unused") +public class SpELExpressionConfiguredPoolsIntegrationTests { - private static ConnectionEndpointList anotherLocators = new ConnectionEndpointList(); - private static ConnectionEndpointList anotherServers = new ConnectionEndpointList(); - private static ConnectionEndpointList locators = new ConnectionEndpointList(); - private static ConnectionEndpointList servers = new ConnectionEndpointList(); + private static final ConnectionEndpointList anotherLocators = new ConnectionEndpointList(); + private static final ConnectionEndpointList anotherServers = new ConnectionEndpointList(); + private static final ConnectionEndpointList locators = new ConnectionEndpointList(); + private static final ConnectionEndpointList servers = new ConnectionEndpointList(); @Autowired @Qualifier("locatorPool") - @SuppressWarnings("unused") private Pool locatorPool; @Autowired @Qualifier("serverPool") - @SuppressWarnings("unused") private Pool serverPool; @Autowired @Qualifier("anotherLocatorPool") - @SuppressWarnings("unused") private Pool anotherLocatorPool; @Autowired @Qualifier("anotherServerPool") - @SuppressWarnings("unused") private Pool anotherServerPool; - protected static ConnectionEndpoint newConnectionEndpoint(String host, int port) { - return new ConnectionEndpoint(host, port); - } + private static void assertConnectionEndpoints(ConnectionEndpointList connectionEndpoints, + String... expected) { - protected void assertConnectionEndpoints(Iterable connectionEndpoints, String... expected) { assertThat(connectionEndpoints).isNotNull(); + assertThat(connectionEndpoints.size()).isEqualTo(expected.length); + + Collections.sort(connectionEndpoints); int index = 0; @@ -101,50 +101,82 @@ public class PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests { assertThat(index).isEqualTo(expected.length); } + private static ConnectionEndpoint newConnectionEndpoint(String host, int port) { + return new ConnectionEndpoint(host, port); + } + @Test public void anotherLocatorPoolFactoryConfiguration() { - String[] expected = { "localhost[10335]", "cardboardbox[10334]", "safetydepositbox[10336]", "pobox[10334]" }; - assertThat(anotherLocators.size()).isEqualTo(expected.length); + String[] expected = { "cardboardbox[10334]", "localhost[10335]", "pobox[10334]", "safetydepositbox[10336]" }; + assertConnectionEndpoints(anotherLocators, expected); } @Test public void anotherServerPoolFactoryConfiguration() { + String[] expected = { "boombox[1234]", "jambox[40404]", "toolbox[8181]" }; - assertThat(anotherServers.size()).isEqualTo(expected.length); - assertConnectionEndpoints(CollectionUtils.sort(anotherServers), expected); + assertConnectionEndpoints(anotherServers, expected); } @Test public void locatorPoolFactoryConfiguration() { + String[] expected = { "backspace[10334]", "jambox[11235]", "mars[30303]", "pluto[20668]", "skullbox[12480]" }; - assertThat(locators.size()).isEqualTo(expected.length); - assertConnectionEndpoints(CollectionUtils.sort(locators), expected); + assertConnectionEndpoints(locators, expected); } @Test public void serverPoolFactoryConfiguration() { - String[] expected = { "earth[4554]", "jupiter[40404]", "mars[5112]", "mercury[1234]", "neptune[42424]", "saturn[41414]", - "uranis[0]", "venus[9876]" }; - assertThat(servers.size()).isEqualTo(expected.length); - assertConnectionEndpoints(CollectionUtils.sort(servers), expected); + String[] expected = { + "earth[4554]", "jupiter[40404]", "mars[5112]", "mercury[1234]", + "neptune[42424]", "saturn[41414]", "uranis[0]", "venus[9876]" + }; + + assertConnectionEndpoints(servers, expected); + } + + public static class SpELBoundBean { + + private final Properties clientProperties; + + public SpELBoundBean(Properties clientProperties) { + this.clientProperties = Optional.ofNullable(clientProperties) + .orElseThrow(() -> newIllegalArgumentException("clientProperties are required")); + } + + public String locatorsHostsPorts() { + return "safetydepositbox[10336], pobox"; + } + + public String serverTwoHost() { + return this.clientProperties.getProperty("gemfire.cache.client.server.2.host"); + } + + public String serverTwoPort() { + return this.clientProperties.getProperty("gemfire.cache.client.server.2.port"); + } } public static class TestBeanFactoryPostProcessor implements BeanFactoryPostProcessor { + @Override public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { - BeanDefinition anotherLocatorPoolBeanDefinition = beanFactory.getBeanDefinition("anotherLocatorPool"); - anotherLocatorPoolBeanDefinition.setBeanClassName(AnotherLocatorPoolFactoryBean.class.getName()); - BeanDefinition anotherServerPoolBeanDefinition = beanFactory.getBeanDefinition("anotherServerPool"); - anotherServerPoolBeanDefinition.setBeanClassName(AnotherServerPoolFactoryBean.class.getName()); - BeanDefinition locatorPoolBeanDefinition = beanFactory.getBeanDefinition("locatorPool"); - locatorPoolBeanDefinition.setBeanClassName(LocatorPoolFactoryBean.class.getName()); - BeanDefinition serverPoolBeanDefinition = beanFactory.getBeanDefinition("serverPool"); - serverPoolBeanDefinition.setBeanClassName(ServerPoolFactoryBean.class.getName()); + + postProcessBeanDefinition(beanFactory, "anotherLocatorPool", AnotherLocatorPoolFactoryBean.class); + postProcessBeanDefinition(beanFactory, "anotherServerPool", AnotherServerPoolFactoryBean.class); + postProcessBeanDefinition(beanFactory, "locatorPool", LocatorPoolFactoryBean.class); + postProcessBeanDefinition(beanFactory, "serverPool", ServerPoolFactoryBean.class); + } + + private void postProcessBeanDefinition(ConfigurableListableBeanFactory beanFactory, + String beanName, Class beanType) { + + beanFactory.getBeanDefinition(beanName).setBeanClassName(beanType.getName()); } } @@ -172,30 +204,7 @@ public class PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests { } } - @SuppressWarnings("unused") - public static class SpELBoundBean { - - private final Properties clientProperties; - - public SpELBoundBean(Properties clientProperties) { - Assert.notNull(clientProperties, "clientProperties must not be null"); - this.clientProperties = clientProperties; - } - - public String locatorsHostsPorts() { - return "safetydepositbox[10336], pobox"; - } - - public String serverTwoHost() { - return clientProperties.getProperty("gemfire.cache.client.server.2.host"); - } - - public String serverTwoPort() { - return clientProperties.getProperty("gemfire.cache.client.server.2.port"); - } - } - - public static class TestPoolFactoryBean extends PoolFactoryBean { + static class TestPoolFactoryBean extends PoolFactoryBean { ConnectionEndpointList getLocatorList() { throw new UnsupportedOperationException("Not Implemented"); @@ -207,26 +216,21 @@ public class PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests { @Override protected PoolFactory createPoolFactory() { - final PoolFactory mockPoolFactory = mock(PoolFactory.class); - when(mockPoolFactory.addLocator(anyString(), anyInt())).thenAnswer(new Answer() { - @Override - public PoolFactory answer(InvocationOnMock invocation) throws Throwable { - String host = invocation.getArgument(0); - int port = invocation.getArgument(1); - getLocatorList().add(newConnectionEndpoint(host, port)); - return mockPoolFactory; - } + PoolFactory mockPoolFactory = MockGemFireObjectsSupport.mockPoolFactory(); + + when(mockPoolFactory.addLocator(anyString(), anyInt())).thenAnswer(invocation -> { + String host = invocation.getArgument(0); + int port = invocation.getArgument(1); + getLocatorList().add(newConnectionEndpoint(host, port)); + return mockPoolFactory; }); - when(mockPoolFactory.addServer(anyString(), anyInt())).thenAnswer(new Answer() { - @Override - public PoolFactory answer(InvocationOnMock invocation) throws Throwable { - String host = invocation.getArgument(0); - int port = invocation.getArgument(1); - getServerList().add(newConnectionEndpoint(host, port)); - return mockPoolFactory; - } + when(mockPoolFactory.addServer(anyString(), anyInt())).thenAnswer(invocation -> { + String host = invocation.getArgument(0); + int port = invocation.getArgument(1); + getServerList().add(newConnectionEndpoint(host, port)); + return mockPoolFactory; }); return mockPoolFactory; diff --git a/src/test/resources/org/springframework/data/gemfire/client/PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests-context.xml b/src/test/resources/org/springframework/data/gemfire/client/SpELExpressionConfiguredPoolsIntegrationTests-context.xml similarity index 91% rename from src/test/resources/org/springframework/data/gemfire/client/PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests-context.xml rename to src/test/resources/org/springframework/data/gemfire/client/SpELExpressionConfiguredPoolsIntegrationTests-context.xml index 421cdd68..7103ec8f 100644 --- a/src/test/resources/org/springframework/data/gemfire/client/PoolsConfiguredWithLocatorsAndServersExpressionsIntegrationTests-context.xml +++ b/src/test/resources/org/springframework/data/gemfire/client/SpELExpressionConfiguredPoolsIntegrationTests-context.xml @@ -24,9 +24,9 @@ - + - +