diff --git a/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java b/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java index 68fb28b1..34624009 100644 --- a/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java +++ b/src/main/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdvice.java @@ -17,6 +17,11 @@ package org.springframework.data.gemfire.serialization.json; +import static org.springframework.data.gemfire.util.ArrayUtils.nullSafeArray; +import static org.springframework.data.gemfire.util.CollectionUtils.nullSafeList; +import static org.springframework.data.gemfire.util.RegionUtils.toRegionName; +import static org.springframework.data.gemfire.util.RegionUtils.toRegionPath; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -24,6 +29,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.stream.Collectors; import com.fasterxml.jackson.databind.ObjectMapper; @@ -41,7 +47,7 @@ import org.springframework.data.gemfire.GemfireTemplate; import org.springframework.util.CollectionUtils; /** - * Spring/AspectJ AOP Aspect to adapt a GemFire {@link Region} to handle JSON data. + * Spring/AspectJ AOP Aspect adapting a {@link Region} to handle JSON data. * * @author David Turanski * @author John Blum @@ -58,7 +64,7 @@ public class JSONRegionAdvice { private boolean convertReturnedCollections = true; private boolean prettyPrint = false; - private List includedRegions; + private List includedRegions = new ArrayList<>(); protected final Log log = LogFactory.getLog(JSONRegionAdvice.class); @@ -79,10 +85,7 @@ public class JSONRegionAdvice { * @param regions a List of region names to include */ public void setIncludedRegions(List> regions) { - this.includedRegions = new ArrayList(); - for (Region region : regions) { - includedRegions.add(region.getName()); - } + nullSafeList(regions).forEach(region -> this.includedRegions.add(toRegionName(region))); } /** @@ -91,7 +94,7 @@ public class JSONRegionAdvice { * @param regionNames a List of region names to include */ public void setIncludedRegionNames(List regionNames) { - this.includedRegions = regionNames; + this.includedRegions = nullSafeList(regionNames); } /** @@ -103,79 +106,25 @@ public class JSONRegionAdvice { this.prettyPrint = prettyPrint; } - @Around("execution(* org.apache.geode.cache.Region.put(..)) || " - + "execution(* org.apache.geode.cache.Region.create(..)) ||" - + "execution(* org.apache.geode.cache.Region.putIfAbsent(..)) ||" - + "execution(* org.apache.geode.cache.Region.replace(..))") - public Object put(ProceedingJoinPoint pjp) { - boolean JSONRegion = isIncludedSONRegion(pjp.getTarget()); - Object returnValue = null; - - try { - if (JSONRegion) { - Object[] newArgs = Arrays.copyOf(pjp.getArgs(), pjp.getArgs().length); - Object val = newArgs[1]; - newArgs[1] = convertArgumentToPdxInstance(val); - returnValue = pjp.proceed(newArgs); - log.debug("converting " + returnValue + " to JSON string"); - returnValue = convertPdxInstanceToJSONString(returnValue); - } - else { - returnValue = pjp.proceed(); - } - } - catch (Throwable t) { - handleThrowable(t); - } - - return returnValue; - } - - @Around("execution(* org.apache.geode.cache.Region.putAll(..))") - public Object putAll(ProceedingJoinPoint pjp) { - boolean JSONRegion = isIncludedSONRegion(pjp.getTarget()); - Object returnValue = null; - - try { - if (JSONRegion) { - Object[] newArgs = Arrays.copyOf(pjp.getArgs(), pjp.getArgs().length); - Map val = (Map) newArgs[0]; - Map newArg = new HashMap(); - for (Entry entry : val.entrySet()) { - newArg.put(entry.getKey(), convertArgumentToPdxInstance(entry.getValue())); - } - newArgs[0] = newArg; - returnValue = pjp.proceed(newArgs); - } - else { - returnValue = pjp.proceed(); - } - } - catch (Throwable t) { - handleThrowable(t); - } - - return returnValue; - } - - @Around("execution(* org.apache.geode.cache.Region.get(..)) " - + "|| execution(* org.apache.geode.cache.Region.selectValue(..))" - + "|| execution(* org.apache.geode.cache.Region.remove(..))") + @Around("execution(* org.apache.geode.cache.Region.get(..))" + + " || execution(* org.apache.geode.cache.Region.remove(..))" + + " || execution(* org.apache.geode.cache.Region.selectValue(..))") public Object get(ProceedingJoinPoint pjp) { + Object returnValue = null; try { - if (isIncludedSONRegion(pjp.getTarget())) { + if (isIncludedJsonRegion(pjp.getTarget())) { returnValue = pjp.proceed(); log.debug("converting " + returnValue + " to JSON string"); - returnValue = convertPdxInstanceToJSONString(returnValue); + returnValue = convertToJson(returnValue); } else { returnValue = pjp.proceed(); } } - catch (Throwable t) { - handleThrowable(t); + catch (Throwable cause) { + handleThrowable(cause); } return returnValue; @@ -184,19 +133,21 @@ public class JSONRegionAdvice { @SuppressWarnings("unchecked") @Around("execution(* org.apache.geode.cache.Region.getAll(..))") public Map getAll(ProceedingJoinPoint pjp) { + Map result = null; try { - Map retVal = (Map) pjp.proceed(); - if (!convertReturnedCollections || CollectionUtils.isEmpty(retVal) || !isIncludedSONRegion( - pjp.getTarget())) { - result = retVal; + + Map returnValue = (Map) pjp.proceed(); + + if (!this.convertReturnedCollections || CollectionUtils.isEmpty(returnValue) + || !isIncludedJsonRegion(pjp.getTarget())) { + + result = returnValue; } else { - result = new HashMap(); - for (Entry entry : retVal.entrySet()) { - result.put(entry.getKey(), convertPdxInstanceToJSONString(entry.getValue())); - } + result = returnValue.entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, entry -> convertToJson(entry.getValue()))); } } catch (Throwable t) { @@ -206,26 +157,88 @@ public class JSONRegionAdvice { return result; } + @Around("execution(* org.apache.geode.cache.Region.create(..))" + + " || execution(* org.apache.geode.cache.Region.put(..))" + + " || execution(* org.apache.geode.cache.Region.putIfAbsent(..))" + + " || execution(* org.apache.geode.cache.Region.replace(..))") + public Object put(ProceedingJoinPoint pjp) { + + Object returnValue = null; + + try { + if (isIncludedJsonRegion(pjp.getTarget())) { + + Object[] newArgs = Arrays.copyOf(pjp.getArgs(), pjp.getArgs().length); + Object val = newArgs[1]; + + newArgs[1] = convertToPdx(val); + returnValue = pjp.proceed(newArgs); + log.debug(String.format("Converting [%s] to JSON", returnValue)); + returnValue = convertToJson(returnValue); + } + else { + returnValue = pjp.proceed(); + } + } + catch (Throwable cause) { + handleThrowable(cause); + } + + return returnValue; + } + + @Around("execution(* org.apache.geode.cache.Region.putAll(..))") + public Object putAll(ProceedingJoinPoint pjp) { + + Object returnValue = null; + + try { + if (isIncludedJsonRegion(pjp.getTarget())) { + + Object[] newArgs = Arrays.copyOf(pjp.getArgs(), pjp.getArgs().length); + + Map map = (Map) newArgs[0]; + Map newArg = new HashMap<>(); + + for (Entry entry : map.entrySet()) { + newArg.put(entry.getKey(), convertToPdx(entry.getValue())); + } + + newArgs[0] = newArg; + returnValue = pjp.proceed(newArgs); + } + else { + returnValue = pjp.proceed(); + } + } + catch (Throwable cause) { + handleThrowable(cause); + } + + return returnValue; + } + @SuppressWarnings("unchecked") @Around("execution(* org.apache.geode.cache.Region.values(..))") public Collection values(ProceedingJoinPoint pjp) { + Collection result = null; try { - Collection retVal = (Collection) pjp.proceed(); - if (!convertReturnedCollections || CollectionUtils.isEmpty(retVal) || !isIncludedSONRegion( - pjp.getTarget())) { - result = retVal; + + Collection returnValue = (Collection) pjp.proceed(); + + if (!this.convertReturnedCollections || CollectionUtils.isEmpty(returnValue) + || !isIncludedJsonRegion(pjp.getTarget())) { + + result = returnValue; } else { - result = new ArrayList(); - for (Object obj : retVal) { - result.add(convertArgumentToPdxInstance(obj)); - } + result = returnValue.stream().map(this::convertToPdx).collect(Collectors.toList()); } } - catch (Throwable t) { - handleThrowable(t); + catch (Throwable cause) { + handleThrowable(cause); } return result; @@ -235,36 +248,89 @@ public class JSONRegionAdvice { "|| execution(* org.springframework.data.gemfire.GemfireOperations.findUnique(..)) " + "|| execution(* org.springframework.data.gemfire.GemfireOperations.query(..))") public Object templateQuery(ProceedingJoinPoint pjp) { + GemfireTemplate template = (GemfireTemplate) pjp.getTarget(); - boolean jsonRegion = isIncludedSONRegion(template.getRegion()); + + boolean jsonRegion = isIncludedJsonRegion(template.getRegion()); + Object returnValue = null; try { if (jsonRegion) { + returnValue = pjp.proceed(); - if (returnValue instanceof SelectResults && convertReturnedCollections) { + + if (returnValue instanceof SelectResults && this.convertReturnedCollections) { + ResultsBag resultsBag = new ResultsBag(); + for (Object obj : (SelectResults) returnValue) { - resultsBag.add(convertPdxInstanceToJSONString(obj)); + resultsBag.add(convertToJson(obj)); } + returnValue = resultsBag; } else { - returnValue = convertPdxInstanceToJSONString(returnValue); + returnValue = convertToJson(returnValue); } } else { returnValue = pjp.proceed(); } } - catch (Throwable t) { - handleThrowable(t); + catch (Throwable cause) { + handleThrowable(cause); } return returnValue; } - private PdxInstance convertArgumentToPdxInstance(Object value) { + private boolean isIncludedJsonRegion(Object target) { + return target instanceof Region && isIncludedJsonRegion((Region) target); + } + + private boolean isIncludedJsonRegion(Region region) { + + boolean result = false; + + if (isIncludedJsonRegion(toRegionName(region), toRegionPath(region))) { + + if (log.isDebugEnabled()) { + log.debug(String.format("Region [%s] is included for JSON conversion", region.getName())); + } + + result = true; + } + + return result; + } + + private boolean isIncludedJsonRegion(String... regionReferences) { + + List regionReferencesList = Arrays.asList(nullSafeArray(regionReferences, String.class)); + + return CollectionUtils.isEmpty(this.includedRegions) || this.includedRegions.stream() + .anyMatch(includeRegion -> regionReferencesList.contains(includeRegion)); + } + + private Object convertToJson(Object returnValue) { + + Object result = returnValue; + + if (returnValue instanceof PdxInstance) { + + result = JSONFormatter.toJSON((PdxInstance) returnValue); + + if (!this.prettyPrint) { + result = flattenString(result); + } + } + + return result; + } + + private PdxInstance convertToPdx(Object value) { + PdxInstance pdx = null; if (value instanceof PdxInstance) { @@ -274,62 +340,32 @@ public class JSONRegionAdvice { pdx = JSONFormatter.fromJSON((String) value); } else { + ObjectMapper mapper = new ObjectMapper(); + try { String json = mapper.writeValueAsString(value); pdx = JSONFormatter.fromJSON(json); } - catch (Throwable t) { - handleThrowable(t); + catch (Throwable cause) { + handleThrowable(cause); } } return pdx; } - private boolean isIncludedSONRegion(Object target) { - Region region = (Region) target; - boolean result = false; - - if (includedRegions == null || includedRegions.contains(region.getName())) { - if (log.isDebugEnabled()) { - log.debug(region.getName() + " is included for JSON conversion"); - } - result = true; - } - - return result; - } - - private Object convertPdxInstanceToJSONString(Object returnValue) { - Object result = returnValue; - - if (returnValue != null && returnValue instanceof PdxInstance) { - result = JSONFormatter.toJSON((PdxInstance) returnValue); - if (!prettyPrint) { - result = flattenString(result); - } - } - - return result; - } - private Object flattenString(Object result) { - if (result instanceof String) { - String json = (String) result; - return json.replaceAll("\\s*", ""); - } - - return result; + return result instanceof String ? ((String) result).replaceAll("\\s*", "") : result; } - private void handleThrowable(Throwable t) { - if (t instanceof RuntimeException) { - throw (RuntimeException) t; + private void handleThrowable(Throwable cause) { + + if (cause instanceof RuntimeException) { + throw (RuntimeException) cause; } else { - throw new RuntimeException(t); + throw new RuntimeException(cause); } - } } diff --git a/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java b/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java index 24d7955d..11fdd218 100644 --- a/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java +++ b/src/main/java/org/springframework/data/gemfire/util/RegionUtils.java @@ -20,6 +20,8 @@ import java.util.Optional; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionAttributes; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; /** @@ -30,6 +32,7 @@ import org.springframework.util.StringUtils; * @see org.apache.geode.cache.RegionAttributes * @since 2.0.0 */ +@SuppressWarnings("unused") public abstract class RegionUtils extends CacheUtils { public static boolean isClient(Region region) { @@ -42,6 +45,32 @@ public abstract class RegionUtils extends CacheUtils { } /* (non-Javadoc) */ + @Nullable + public static String toRegionName(@Nullable Region region) { + return Optional.ofNullable(region).map(Region::getName).orElse(null); + } + + /* (non-Javadoc) */ + @Nullable + public static String toRegionName(String regionPath) { + + return Optional.ofNullable(regionPath) + .filter(StringUtils::hasText) + .map(StringUtils::trimWhitespace) + .map(it -> it.lastIndexOf(Region.SEPARATOR)) + .filter(index -> index > -1) + .map(index -> regionPath.substring(index + 1)) + .orElse(regionPath); + } + + /* (non-Javadoc) */ + @Nullable + public static String toRegionPath(@Nullable Region region) { + return Optional.ofNullable(region).map(Region::getFullPath).orElse(null); + } + + /* (non-Javadoc) */ + @NonNull public static String toRegionPath(String regionName) { return String.format("%1$s%2$s", Region.SEPARATOR, regionName); } diff --git a/src/test/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceTest.java b/src/test/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceIntegrationTests.java similarity index 51% rename from src/test/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceTest.java rename to src/test/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceIntegrationTests.java index 62166334..50a415f0 100644 --- a/src/test/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceTest.java +++ b/src/test/java/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceIntegrationTests.java @@ -18,10 +18,10 @@ package org.springframework.data.gemfire.serialization.json; import static org.junit.Assert.assertEquals; +import static org.springframework.data.gemfire.util.RuntimeExceptionFactory.newIllegalArgumentException; import java.io.IOException; import java.util.Arrays; -import java.util.HashMap; import java.util.Map; import javax.annotation.Resource; @@ -37,12 +37,13 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.gemfire.GemfireOperations; import org.springframework.data.gemfire.repository.sample.Person; +import org.springframework.data.gemfire.test.support.MapBuilder; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringRunner; /** - * Integration test to test SDG support for storing and reading JSON data to/from - * a GemFire Cache {@link Region} by (un)marshalled JSON data using Jackson. + * Integration test to test SDG support for storing and reading JSON data to/from a {@link Region} + * by (un)marshalling JSON data using Jackson. * * @author David Turanski * @author John Blum @@ -51,11 +52,12 @@ import org.springframework.test.context.junit4.SpringRunner; * @see org.springframework.data.gemfire.serialization.json.JSONRegionAdvice * @see org.springframework.test.context.ContextConfiguration * @see org.springframework.test.context.junit4.SpringRunner + * @see com.fasterxml.jackson.databind.ObjectMapper */ @RunWith(SpringRunner.class) @ContextConfiguration @SuppressWarnings({ "unchecked", "unused" }) -public class JSONRegionAdviceTest { +public class JSONRegionAdviceIntegrationTests { // TODO figure out why auto-proxying the Region for JSON support prevents the GemfireTemplate from being "auto-wired", // as a GemfireTemplate rather than GemfireOperations, resulting in a NoSuchBeanDefinitionException thrown by the @@ -63,7 +65,7 @@ public class JSONRegionAdviceTest { @Autowired private GemfireOperations template; - @Resource(name = "jsonRegion") + @Resource(name = "JsonRegion") private Region jsonRegion; @Before @@ -71,74 +73,101 @@ public class JSONRegionAdviceTest { jsonRegion.clear(); } - protected static String toJson(Object bean) { + private static String toJson(Object bean) { + try { return new ObjectMapper().writeValueAsString(bean); } - catch (JsonProcessingException e) { - throw new IllegalArgumentException(String.format("Failed to convert object (%1$s) into JSON", bean), e); + catch (JsonProcessingException cause) { + throw newIllegalArgumentException(cause, "Failed to convert object (%1$s) into JSON", bean); } } @Test - public void testPutString() { + public void putAndCreate() { + String json = "{\"hello\":\"world\"}"; - jsonRegion.put("key", json); + this.jsonRegion.put("keyOne", json); - assertEquals(json, jsonRegion.put("key", json)); + assertEquals(json, this.jsonRegion.put("keyOne", json)); - jsonRegion.create("key2", json); + this.jsonRegion.create("keyTwo", json); - assertEquals(json, jsonRegion.get("key")); + assertEquals(json, this.jsonRegion.get("keyTwo")); } @Test @SuppressWarnings("unchecked") - public void testPutAll() { - Map map = new HashMap(); - map.put("key1", "{\"hello1\":\"world1\"}"); - map.put("key2", "{\"hello2\":\"world2\"}"); + public void putAll() { - jsonRegion.putAll(map); + Map map = MapBuilder.newMapBuilder() + .put("key1", "{\"hello1\":\"world1\"}") + .put("key2", "{\"hello2\":\"world2\"}") + .build(); - Map results = jsonRegion.getAll(Arrays.asList("key1", "key2")); + this.jsonRegion.putAll(map); + + Map results = this.jsonRegion.getAll(Arrays.asList("key1", "key2")); assertEquals("{\"hello1\":\"world1\"}", results.get("key1")); assertEquals("{\"hello2\":\"world2\"}", results.get("key2")); } @Test - public void testObjectToJSon() throws IOException { - Person daveTuranski = new Person(1L, "Dave", "Turanski"); - jsonRegion.put("dave", daveTuranski); - String json = String.valueOf(jsonRegion.get("dave")); - assertEquals(json, toJson(daveTuranski), json); - Object result = jsonRegion.put("dave", daveTuranski); - assertEquals(toJson(daveTuranski), result); + public void objectToJSon() throws IOException { + + Person davidTuranski = new Person(1L, "David", "Turanski"); + + this.jsonRegion.put("dave", davidTuranski); + + String json = String.valueOf(this.jsonRegion.get("dave")); + + System.out.printf("JSON [%s]%n", json); + + assertEquals(json, toJson(davidTuranski)); + + Object result = jsonRegion.put("dave", davidTuranski); + + assertEquals(toJson(davidTuranski), result); } @Test - public void testTemplateFindUnique() { - Person daveTuranski = new Person(1L, "Dave", "Turanski"); - jsonRegion.put("dave", daveTuranski); - String json = template.findUnique("SELECT * FROM /jsonRegion WHERE firstname=$1", "Dave"); - assertEquals(toJson(daveTuranski), json); + public void templateFind() { + + Person davidTuranski = new Person(1L, "David", "Turanski"); + + this.jsonRegion.put("dave", davidTuranski); + + SelectResults results = this.template.find(String.format("SELECT * FROM %s WHERE firstname=$1", + this.jsonRegion.getFullPath()), davidTuranski.getFirstname()); + + assertEquals(toJson(davidTuranski), results.iterator().next()); } @Test - public void testTemplateFind() { - Person daveTuranski = new Person(1L, "Dave", "Turanski"); - jsonRegion.put("dave", daveTuranski); - SelectResults results = template.find("SELECT * FROM /jsonRegion WHERE firstname=$1", "Dave"); - assertEquals(toJson(daveTuranski), results.iterator().next()); + public void templateFindUnique() { + + Person davidTuranski = new Person(1L, "David", "Turanski"); + + this.jsonRegion.put("dave", davidTuranski); + + String json = this.template.findUnique(String.format("SELECT * FROM %s WHERE firstname=$1", + this.jsonRegion.getFullPath()), davidTuranski.getFirstname()); + + assertEquals(toJson(davidTuranski), json); } @Test - public void testTemplateQuery() { - Person daveTuranski = new Person(1L, "Dave", "Turanski"); - jsonRegion.put("dave", daveTuranski); - SelectResults results = template.query("firstname='Dave'"); - assertEquals(toJson(daveTuranski), results.iterator().next()); + public void templateQuery() { + + Person davidTuranski = new Person(1L, "David", "Turanski"); + + this.jsonRegion.put("dave", davidTuranski); + + SelectResults results = + this.template.query(String.format("firstname='%s'", davidTuranski.getFirstname())); + + assertEquals(toJson(davidTuranski), results.iterator().next()); } } diff --git a/src/test/resources/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceTest-context.xml b/src/test/resources/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceIntegrationTests-context.xml similarity index 74% rename from src/test/resources/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceTest-context.xml rename to src/test/resources/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceIntegrationTests-context.xml index d7e3bcec..a778744f 100644 --- a/src/test/resources/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceTest-context.xml +++ b/src/test/resources/org/springframework/data/gemfire/serialization/json/JSONRegionAdviceIntegrationTests-context.xml @@ -15,20 +15,15 @@ JSONRegionAdviceTest 0 - warning + error - + - - - + - +