From ee242bece8ef7ef9e33d09aea5b9054236e015e0 Mon Sep 17 00:00:00 2001 From: John Blum Date: Mon, 25 Jan 2021 12:51:28 -0800 Subject: [PATCH] Refactor configureTopology(..) to delegate to configureEnvironment(:Environment). The new 'configureEnvironment(:Environment) method attempts to add a custom PropertySource to the Environment if the Environment is configurable, otherwise proceeds in setting a Java System Property as before. The idea is, the Environment is more recycable than Java System Properties, particularly if the Spring ApplicationContext is refreshed. Renames DEFAULT_CLUSTER_CONDITION_MATCH to DEFAULT_CLUSTER_AWARE_CONDITION_MATCH. Renames DEFAULT_CLUSTER_CONDITION_STRICT_MATCH to DEFAULT_CLUSTER_AWARE_CONDITION_STRICT_MATCH. --- .../annotation/ClusterAwareConfiguration.java | 41 ++++++++++++++----- .../config/annotation/EnableClusterAware.java | 2 +- .../ClusterAwareConfigurationUnitTests.java | 10 +++-- ...vailableConfigurationIntegrationTests.java | 5 +-- ...vailableConfigurationIntegrationTests.java | 5 +-- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/spring-geode/src/main/java/org/springframework/geode/config/annotation/ClusterAwareConfiguration.java b/spring-geode/src/main/java/org/springframework/geode/config/annotation/ClusterAwareConfiguration.java index 598d336f..a0668770 100644 --- a/spring-geode/src/main/java/org/springframework/geode/config/annotation/ClusterAwareConfiguration.java +++ b/spring-geode/src/main/java/org/springframework/geode/config/annotation/ClusterAwareConfiguration.java @@ -58,6 +58,7 @@ import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.Environment; +import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; import org.springframework.core.type.AnnotatedTypeMetadata; @@ -116,8 +117,8 @@ import org.slf4j.LoggerFactory; @Import({ ClusterAvailableConfiguration.class, ClusterNotAvailableConfiguration.class }) public class ClusterAwareConfiguration extends AbstractAnnotationConfigSupport implements ImportAware { - static final boolean DEFAULT_CLUSTER_CONDITION_MATCH = false; - static final boolean DEFAULT_CLUSTER_CONDITION_STRICT_MATCH = false; + static final boolean DEFAULT_CLUSTER_AWARE_CONDITION_MATCH = false; + static final boolean DEFAULT_CLUSTER_AWARE_CONDITION_STRICT_MATCH = false; static final int DEFAULT_CACHE_SERVER_PORT = CacheServer.DEFAULT_PORT; static final int DEFAULT_LOCATOR_PORT = 10334; @@ -129,6 +130,9 @@ public class ClusterAwareConfiguration extends AbstractAnnotationConfigSupport i static final String MATCHING_PROPERTY_PATTERN = "spring\\.data\\.gemfire\\.pool\\..*locators|servers"; static final String STRICT_MATCH_ATTRIBUTE_NAME = "strictMatch"; + static final String CLUSTER_AWARE_CONFIGURATION_PROPERTY_SOURCE_NAME = + ClusterAwareConfiguration.class.getSimpleName().concat("PropertySource"); + static final String SPRING_BOOT_DATA_GEMFIRE_CLUSTER_CONDITION_MATCH_PROPERTY = "spring.boot.data.gemfire.cluster.condition.match"; @@ -139,14 +143,14 @@ public class ClusterAwareConfiguration extends AbstractAnnotationConfigSupport i "spring.data.gemfire.cache.client.region.shortcut"; private static final AtomicBoolean strictMatchConfiguration = - new AtomicBoolean(DEFAULT_CLUSTER_CONDITION_STRICT_MATCH); + new AtomicBoolean(DEFAULT_CLUSTER_AWARE_CONDITION_STRICT_MATCH); private static final Function configuredMatchFunction = conditionContext -> Optional.ofNullable(conditionContext) .map(ConditionContext::getEnvironment) .map(environment -> environment.getProperty(SPRING_BOOT_DATA_GEMFIRE_CLUSTER_CONDITION_MATCH_PROPERTY, - Boolean.class, DEFAULT_CLUSTER_CONDITION_MATCH)) - .orElse(DEFAULT_CLUSTER_CONDITION_MATCH); + Boolean.class, DEFAULT_CLUSTER_AWARE_CONDITION_MATCH)) + .orElse(DEFAULT_CLUSTER_AWARE_CONDITION_MATCH); private static final Logger logger = LoggerFactory.getLogger(ClusterAwareConfiguration.class); @@ -283,7 +287,7 @@ public class ClusterAwareConfiguration extends AbstractAnnotationConfigSupport i .map(annotationMetadata -> annotationMetadata.getAnnotationAttributes(annotationName)) .map(AnnotationAttributes::fromMap) .map(annotationAttributes -> annotationAttributes.getBoolean(STRICT_MATCH_ATTRIBUTE_NAME)) - .orElse(DEFAULT_CLUSTER_CONDITION_STRICT_MATCH); + .orElse(DEFAULT_CLUSTER_AWARE_CONDITION_STRICT_MATCH); } return strictMatchEnabled; @@ -587,14 +591,31 @@ public class ClusterAwareConfiguration extends AbstractAnnotationConfigSupport i return !isConnected(connectionCount); } + private void configureEnvironment(@NonNull Environment environment) { + + if (environment != null) { + if (!environment.containsProperty(SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY)) { + if (environment instanceof ConfigurableEnvironment) { + + MutablePropertySources propertySources = ((ConfigurableEnvironment) environment).getPropertySources(); + + propertySources.addFirst(new MapPropertySource(CLUSTER_AWARE_CONFIGURATION_PROPERTY_SOURCE_NAME, + Collections.singletonMap(SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY, + LOCAL_CLIENT_REGION_SHORTCUT.name()))); + } + else { + System.setProperty(SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY, + LOCAL_CLIENT_REGION_SHORTCUT.name()); + } + } + } + } + protected void configureTopology(@NonNull Environment environment, @NonNull ConnectionEndpointList connectionEndpoints, int connectionCount) { if (isNotConnected(connectionCount)) { - if (!environment.containsProperty(SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY)) { - System.setProperty(SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY, - LOCAL_CLIENT_REGION_SHORTCUT.name()); - } + configureEnvironment(environment); } } diff --git a/spring-geode/src/main/java/org/springframework/geode/config/annotation/EnableClusterAware.java b/spring-geode/src/main/java/org/springframework/geode/config/annotation/EnableClusterAware.java index f6da96d3..c6729fb8 100644 --- a/spring-geode/src/main/java/org/springframework/geode/config/annotation/EnableClusterAware.java +++ b/spring-geode/src/main/java/org/springframework/geode/config/annotation/EnableClusterAware.java @@ -75,6 +75,6 @@ public @interface EnableClusterAware { * * @return a boolean value indicating whether strict matching mode is enabled. */ - boolean strictMatch() default ClusterAwareConfiguration.DEFAULT_CLUSTER_CONDITION_STRICT_MATCH; + boolean strictMatch() default ClusterAwareConfiguration.DEFAULT_CLUSTER_AWARE_CONDITION_STRICT_MATCH; } diff --git a/spring-geode/src/test/java/org/springframework/geode/config/annotation/ClusterAwareConfigurationUnitTests.java b/spring-geode/src/test/java/org/springframework/geode/config/annotation/ClusterAwareConfigurationUnitTests.java index f1df0d28..ab57e29a 100644 --- a/spring-geode/src/test/java/org/springframework/geode/config/annotation/ClusterAwareConfigurationUnitTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/config/annotation/ClusterAwareConfigurationUnitTests.java @@ -66,6 +66,7 @@ import org.springframework.data.gemfire.tests.integration.IntegrationTestsSuppor import org.springframework.geode.config.annotation.ClusterAwareConfiguration.PoolConnectionEndpoint; import org.springframework.geode.config.annotation.ClusterAwareConfiguration.SocketCreationException; import org.springframework.lang.NonNull; +import org.springframework.mock.env.MockEnvironment; import org.slf4j.Logger; @@ -160,14 +161,14 @@ public class ClusterAwareConfigurationUnitTests extends IntegrationTestsSupport doReturn(mockEnvironment).when(mockConditionContext).getEnvironment(); doReturn(true).when(mockEnvironment) .getProperty(eq(ClusterAwareConfiguration.SPRING_BOOT_DATA_GEMFIRE_CLUSTER_CONDITION_MATCH_PROPERTY), - eq(Boolean.class), eq(ClusterAwareConfiguration.DEFAULT_CLUSTER_CONDITION_MATCH)); + eq(Boolean.class), eq(ClusterAwareConfiguration.DEFAULT_CLUSTER_AWARE_CONDITION_MATCH)); assertThat(this.condition.isMatch(mockConditionContext)).isTrue(); verify(mockConditionContext, times(1)).getEnvironment(); verify(mockEnvironment, times(1)) .getProperty(eq(ClusterAwareConfiguration.SPRING_BOOT_DATA_GEMFIRE_CLUSTER_CONDITION_MATCH_PROPERTY), - eq(Boolean.class), eq(ClusterAwareConfiguration.DEFAULT_CLUSTER_CONDITION_MATCH)); + eq(Boolean.class), eq(ClusterAwareConfiguration.DEFAULT_CLUSTER_AWARE_CONDITION_MATCH)); verifyNoMoreInteractions(mockConditionContext, mockEnvironment); } @@ -727,18 +728,19 @@ public class ClusterAwareConfigurationUnitTests extends IntegrationTestsSupport assertThat(System.getProperties()) .doesNotContainKey(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY); - ConfigurableEnvironment mockEnvironment = mock(ConfigurableEnvironment.class); + MockEnvironment mockEnvironment = spy(new MockEnvironment()); ConnectionEndpointList connectionEndpoints = new ConnectionEndpointList(new ConnectionEndpoint("localhost", 1234)); this.condition.configureTopology(mockEnvironment, connectionEndpoints,0); - assertThat(System.getProperty(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY)) + assertThat(mockEnvironment.getProperty(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY)) .isEqualTo("LOCAL"); verify(mockEnvironment, times(1)) .containsProperty(eq(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY)); + verify(mockEnvironment, times(1)).getPropertySources(); } @Test diff --git a/spring-geode/src/test/java/org/springframework/geode/config/annotation/PropertyConfiguredStrictMatchingClusterNotAvailableConfigurationIntegrationTests.java b/spring-geode/src/test/java/org/springframework/geode/config/annotation/PropertyConfiguredStrictMatchingClusterNotAvailableConfigurationIntegrationTests.java index c4a8f511..109010b4 100644 --- a/spring-geode/src/test/java/org/springframework/geode/config/annotation/PropertyConfiguredStrictMatchingClusterNotAvailableConfigurationIntegrationTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/config/annotation/PropertyConfiguredStrictMatchingClusterNotAvailableConfigurationIntegrationTests.java @@ -23,7 +23,6 @@ import org.junit.AfterClass; import org.junit.Test; import org.apache.geode.cache.client.ClientCache; -import org.apache.geode.cache.client.ClientRegionShortcut; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.core.NestedExceptionUtils; @@ -90,8 +89,8 @@ public class PropertyConfiguredStrictMatchingClusterNotAvailableConfigurationInt throw expected; } finally { - assertThat(System.getProperty(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY)) - .isEqualTo(ClientRegionShortcut.LOCAL.name()); + assertThat(System.getProperties()) + .doesNotContainKeys(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY); } } diff --git a/spring-geode/src/test/java/org/springframework/geode/config/annotation/StrictMatchingClusterNotAvailableConfigurationIntegrationTests.java b/spring-geode/src/test/java/org/springframework/geode/config/annotation/StrictMatchingClusterNotAvailableConfigurationIntegrationTests.java index 32318229..d5edf7ca 100644 --- a/spring-geode/src/test/java/org/springframework/geode/config/annotation/StrictMatchingClusterNotAvailableConfigurationIntegrationTests.java +++ b/spring-geode/src/test/java/org/springframework/geode/config/annotation/StrictMatchingClusterNotAvailableConfigurationIntegrationTests.java @@ -21,7 +21,6 @@ import org.junit.AfterClass; import org.junit.Test; import org.apache.geode.cache.client.ClientCache; -import org.apache.geode.cache.client.ClientRegionShortcut; import org.springframework.core.NestedExceptionUtils; import org.springframework.data.gemfire.config.annotation.ClientCacheApplication; @@ -71,8 +70,8 @@ public class StrictMatchingClusterNotAvailableConfigurationIntegrationTests throw expected; } finally { - assertThat(System.getProperty(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY)) - .isEqualTo(ClientRegionShortcut.LOCAL.name()); + assertThat(System.getProperties()) + .doesNotContainKeys(ClusterAwareConfiguration.SPRING_DATA_GEMFIRE_CACHE_CLIENT_REGION_SHORTCUT_PROPERTY); } }