From b91dcb23dc7fd9888e1aa6fe860e7dc05c3e11ad Mon Sep 17 00:00:00 2001 From: John Blum Date: Wed, 4 Dec 2019 23:45:33 -0800 Subject: [PATCH] DATAGEODE-277 - Reduce usage of Apache Geode internal APIs. This also avoids the compiler issue requiring compile-time dependencies on 'geode-logging' and 'geode-serialization'. --- .../data/gemfire/GemfireTemplate.java | 35 +++++++++++++------ .../data/gemfire/GemfireUtils.java | 2 +- .../serialization/json/JSONRegionAdvice.java | 7 ++-- .../data/gemfire/util/CacheUtils.java | 1 - .../gemfire/util/DistributedSystemUtils.java | 7 ---- .../data/gemfire/util/RegionUtils.java | 34 ++++++++++++------ .../gemfire/util/RegionUtilsUnitTests.java | 24 ++++++++++--- 7 files changed, 72 insertions(+), 38 deletions(-) diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireTemplate.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireTemplate.java index 5a2b1467..76204697 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireTemplate.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireTemplate.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire; import java.lang.reflect.InvocationHandler; @@ -23,6 +22,8 @@ import java.lang.reflect.Proxy; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.function.Supplier; import org.apache.geode.GemFireCheckedException; import org.apache.geode.GemFireException; @@ -34,12 +35,14 @@ import org.apache.geode.cache.query.Query; import org.apache.geode.cache.query.QueryInvalidException; import org.apache.geode.cache.query.QueryService; import org.apache.geode.cache.query.SelectResults; -import org.apache.geode.internal.cache.LocalRegion; import org.springframework.dao.DataAccessException; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.gemfire.util.RegionUtils; +import org.springframework.data.gemfire.util.SpringUtils; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** @@ -411,8 +414,8 @@ public class GemfireTemplate extends GemfireAccessor implements GemfireOperation ClientCache clientCache = (ClientCache) region.getRegionService(); return requiresLocalQueryService(region) ? clientCache.getLocalQueryService() - : (requiresPooledQueryService(region) ? clientCache.getQueryService(poolNameFrom(region)) - : queryServiceFrom(region)); + : requiresPooledQueryService(region) ? clientCache.getQueryService(poolNameFrom(region)) + : queryServiceFrom(region); } boolean requiresLocalQueryService(Region region) { @@ -420,21 +423,33 @@ public class GemfireTemplate extends GemfireAccessor implements GemfireOperation } boolean isLocalWithNoServerProxy(Region region) { - return region instanceof LocalRegion && !((LocalRegion) region).hasServerProxy(); + + if (RegionUtils.isLocal(region)) { + + Supplier hasServerProxyMethod = () -> + Optional.ofNullable(ReflectionUtils.findMethod(region.getClass(), "hasServerProxy")) + .map(method -> ReflectionUtils.invokeMethod(method, region)) + .map(Boolean.FALSE::equals) + .orElse(false); + + return SpringUtils.safeGetValue(hasServerProxyMethod, false); + } + + return false; } boolean requiresPooledQueryService(Region region) { return StringUtils.hasText(poolNameFrom(region)); } - QueryService queryServiceFrom(Region region) { - return region.getRegionService().getQueryService(); - } - String poolNameFrom(Region region) { return region.getAttributes().getPoolName(); } + QueryService queryServiceFrom(Region region) { + return region.getRegionService().getQueryService(); + } + /* * (non-Javadoc) * @see org.springframework.data.gemfire.GemfireOperations#execute(org.springframework.data.gemfire.GemfireCallback) @@ -455,7 +470,7 @@ public class GemfireTemplate extends GemfireAccessor implements GemfireOperation try { - Region regionArgument = (exposeNativeRegion ? getRegion() : regionProxy); + Region regionArgument = (exposeNativeRegion ? getRegion() : this.regionProxy); return action.doInGemfire(regionArgument); } diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireUtils.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireUtils.java index 73e65dd9..e1c9be47 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireUtils.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/GemfireUtils.java @@ -26,7 +26,7 @@ import org.springframework.util.ClassUtils; /** * {@link GemfireUtils} is an abstract utility class encapsulating common functionality for accessing features - * and capabilities of Apache Geode or Pivotal GemFire based on version as well as other configuration meta-data. + * and capabilities of Apache Geode based on version as well as other configuration meta-data. * * @author John Blum * @see org.apache.geode.cache.CacheFactory diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java index c0824d45..2463fa8d 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java @@ -35,7 +35,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.geode.cache.Region; import org.apache.geode.cache.query.SelectResults; -import org.apache.geode.cache.query.internal.ResultsBag; import org.apache.geode.pdx.JSONFormatter; import org.apache.geode.pdx.PdxInstance; @@ -264,13 +263,13 @@ public class JSONRegionAdvice { if (returnValue instanceof SelectResults && this.convertReturnedCollections) { - ResultsBag resultsBag = new ResultsBag(); + Collection results = new ArrayList<>(); for (Object obj : (SelectResults) returnValue) { - resultsBag.add(convertToJson(obj)); + results.add(convertToJson(obj)); } - returnValue = resultsBag; + returnValue = results; } else { returnValue = convertToJson(returnValue); diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/CacheUtils.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/CacheUtils.java index 303a7c7c..1f685d0a 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/CacheUtils.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/CacheUtils.java @@ -14,7 +14,6 @@ * limitations under the License. * */ - package org.springframework.data.gemfire.util; import java.util.Optional; diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/DistributedSystemUtils.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/DistributedSystemUtils.java index 04fa30a9..b06f3f36 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/DistributedSystemUtils.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/DistributedSystemUtils.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.util; import java.util.Optional; @@ -53,7 +52,6 @@ public abstract class DistributedSystemUtils extends SpringUtils { public static final String GEMFIRE_PREFIX = DistributionConfig.GEMFIRE_PREFIX; public static final String NAME_PROPERTY_NAME = DistributionConfig.NAME_NAME; - /* (non-Javadoc) */ public static Properties configureDurableClient(Properties gemfireProperties, String durableClientId, Integer durableClientTimeout) { @@ -71,29 +69,24 @@ public abstract class DistributedSystemUtils extends SpringUtils { return gemfireProperties; } - /* (non-Javadoc) */ public static boolean isConnected(DistributedSystem distributedSystem) { return Optional.ofNullable(distributedSystem).filter(DistributedSystem::isConnected).isPresent(); } - /* (non-Javadoc) */ public static boolean isNotConnected(DistributedSystem distributedSystem) { return !isConnected(distributedSystem); } - /* (non-Javadoc) */ @SuppressWarnings("unchecked") public static T getDistributedSystem() { return (T) InternalDistributedSystem.getAnyInstance(); } - /* (non-Javadoc) */ @SuppressWarnings("unchecked") public static T getDistributedSystem(GemFireCache gemfireCache) { return (T) Optional.ofNullable(gemfireCache).map(GemFireCache::getDistributedSystem).orElse(null); } - /* (non-Javadoc)*/ @SuppressWarnings("unchecked") public static T getLocator() { return (T) InternalLocator.getLocator(); diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java index 439d8c6c..5d218b4d 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java @@ -21,6 +21,7 @@ import org.apache.geode.cache.DataPolicy; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionAttributes; import org.apache.geode.cache.client.ClientRegionShortcut; +import org.apache.geode.internal.cache.LocalRegion; import org.springframework.data.gemfire.client.ClientRegionShortcutWrapper; import org.springframework.lang.NonNull; @@ -43,23 +44,23 @@ public abstract class RegionUtils extends CacheUtils { * Assert that the configuration settings for {@link ClientRegionShortcut} and the {@literal persistent} attribute * in <gfe:*-region> elements are compatible. * - * @param resolvedShortcut {@link ClientRegionShortcut} resolved from the SDG XML namespace. + * @param clientRegionShortcut {@link ClientRegionShortcut} resolved from the SDG XML namespace. * @param persistent boolean indicating the value of the {@literal persistent} configuration attribute. * @see org.springframework.data.gemfire.client.ClientRegionShortcutWrapper * @see org.apache.geode.cache.client.ClientRegionShortcut */ public static void assertClientRegionShortcutAndPersistentAttributeAreCompatible( - ClientRegionShortcut resolvedShortcut, Boolean persistent) { + ClientRegionShortcut clientRegionShortcut, Boolean persistent) { boolean persistentUnspecified = persistent == null; - if (ClientRegionShortcutWrapper.valueOf(resolvedShortcut).isPersistent()) { + if (ClientRegionShortcutWrapper.valueOf(clientRegionShortcut).isPersistent()) { Assert.isTrue(persistentUnspecified || Boolean.TRUE.equals(persistent), - String.format("Client Region Shortcut [%s] is not valid when persistent is false", resolvedShortcut)); + String.format("Client Region Shortcut [%s] is not valid when persistent is false", clientRegionShortcut)); } else { Assert.isTrue(persistentUnspecified || Boolean.FALSE.equals(persistent), - String.format("Client Region Shortcut [%s] is not valid when persistent is true", resolvedShortcut)); + String.format("Client Region Shortcut [%s] is not valid when persistent is true", clientRegionShortcut)); } } @@ -67,22 +68,21 @@ public abstract class RegionUtils extends CacheUtils { * Assert that the configuration settings for {@link DataPolicy} and the {@literal persistent} attribute * in <gfe:*-region> elements are compatible. * - * @param resolvedDataPolicy {@link DataPolicy} resolved from the SDG XML namespace. + * @param dataPolicy {@link DataPolicy} resolved from the SDG XML namespace. * @param persistent boolean indicating the value of the {@literal persistent} configuration attribute. * @see org.apache.geode.cache.DataPolicy */ - public static void assertDataPolicyAndPersistentAttributeAreCompatible( - DataPolicy resolvedDataPolicy, Boolean persistent) { + public static void assertDataPolicyAndPersistentAttributeAreCompatible(DataPolicy dataPolicy, Boolean persistent) { boolean persistentUnspecified = persistent == null; - if (resolvedDataPolicy.withPersistence()) { + if (dataPolicy.withPersistence()) { Assert.isTrue(persistentUnspecified || Boolean.TRUE.equals(persistent), - String.format("Data Policy [%s] is not valid when persistent is false", resolvedDataPolicy)); + String.format("Data Policy [%s] is not valid when persistent is false", dataPolicy)); } else { Assert.isTrue(persistentUnspecified || Boolean.FALSE.equals(persistent), - String.format("Data Policy [%s] is not valid when persistent is true", resolvedDataPolicy)); + String.format("Data Policy [%s] is not valid when persistent is true", dataPolicy)); } } @@ -137,6 +137,18 @@ public abstract class RegionUtils extends CacheUtils { .isPresent(); } + /** + * Determines whether the given {@link Region} is a non-distributed, {@literal local} {@link Region}. + * + * @param region {@link Region} to evaluate. + * @return a boolean value indicating whether the given {@link Region} is a non-distributed, + * {@literal local} {@link Region}. + * @see org.apache.geode.cache.Region + */ + public static boolean isLocal(@Nullable Region region) { + return region instanceof LocalRegion; + } + @Nullable public static String toRegionName(@Nullable Region region) { return Optional.ofNullable(region).map(Region::getName).orElse(null); diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/RegionUtilsUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/RegionUtilsUnitTests.java index 90c5a5cc..2b15c088 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/RegionUtilsUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/RegionUtilsUnitTests.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.util; import static org.assertj.core.api.Assertions.assertThat; @@ -23,14 +22,15 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import org.junit.Test; + import org.apache.geode.cache.DataPolicy; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionService; - -import org.junit.Test; +import org.apache.geode.internal.cache.LocalRegion; /** - * Unit tests for {@link RegionUtils}. + * Unit Tests for {@link RegionUtils}. * * @author John Blum * @see org.springframework.data.gemfire.util.RegionUtils @@ -164,4 +164,20 @@ public class RegionUtilsUnitTests { verify(mockRegion, times(1)).getRegionService(); } + + @Test + @SuppressWarnings("all") + public void nullRegionIsNotLocal() { + assertThat(RegionUtils.isLocal(null)).isFalse(); + } + + @Test + public void localRegionIsLocal() { + assertThat(RegionUtils.isLocal(mock(LocalRegion.class))).isTrue(); + } + + @Test + public void nonLocalRegionIsNotLocal() { + assertThat(RegionUtils.isLocal(mock(Region.class))).isFalse(); + } }