diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBean.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBean.java index 3b359c69..cf625a37 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBean.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBean.java @@ -15,20 +15,18 @@ package org.springframework.data.gemfire.function.execution; import java.lang.reflect.Method; import java.util.stream.StreamSupport; -import org.aopalliance.intercept.MethodInterceptor; -import org.aopalliance.intercept.MethodInvocation; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.support.AopUtils; -import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.FactoryBean; +import org.springframework.data.gemfire.support.AbstractFactoryBeanSupport; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.aopalliance.intercept.MethodInterceptor; +import org.aopalliance.intercept.MethodInvocation; + /** - * A Proxy FactoryBean for all non-Region Function Execution interfaces. + * A Proxy {@link FactoryBean} for all non-Region Function Execution interfaces. * * @author David Turanski * @author John Blum @@ -37,76 +35,111 @@ import org.springframework.util.ClassUtils; * @see org.aopalliance.intercept.MethodInterceptor * @see org.springframework.beans.factory.BeanClassLoaderAware * @see org.springframework.beans.factory.FactoryBean + * @see org.springframework.data.gemfire.support.AbstractFactoryBeanSupport */ -public class GemfireFunctionProxyFactoryBean implements BeanClassLoaderAware, FactoryBean, MethodInterceptor { - - private volatile ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader(); +public class GemfireFunctionProxyFactoryBean extends AbstractFactoryBeanSupport implements MethodInterceptor { private volatile boolean initialized; private final Class functionExecutionInterface; - private volatile Object functionExecutionProxy; + private FunctionExecutionMethodMetadata methodMetadata; private final GemfireFunctionOperations gemfireFunctionOperations; - protected Logger logger = LoggerFactory.getLogger(this.getClass()); - - private FunctionExecutionMethodMetadata methodMetadata; + private volatile Object functionExecutionProxy; /** - * @param functionExecutionInterface the proxied interface - * @param gemfireFunctionOperations an interface used to delegate the function invocation (typically a GemFire function template) + * Constructs a new instance of the {@link GemfireFunctionProxyFactoryBean} initialized with the given + * {@link Class Function Excution Interface} and {@link GemfireFunctionOperations}. + * + * @param functionExecutionInterface {@link Class Function Execution Interface} to proxy. + * @param gemfireFunctionOperations Template class used to delegate the Function invocation. + * @see org.springframework.data.gemfire.function.execution.GemfireFunctionOperations + * @throws IllegalArgumentException if the {@link Class Function Execution Interface} is {@literal null} + * or the {@link Class Function Execution Type} is not an actual interface. */ - public GemfireFunctionProxyFactoryBean(Class functionExecutionInterface, GemfireFunctionOperations gemfireFunctionOperations) { + public GemfireFunctionProxyFactoryBean(Class functionExecutionInterface, + GemfireFunctionOperations gemfireFunctionOperations) { - Assert.notNull(functionExecutionInterface, "Function execution interface must not be null"); + Assert.notNull(functionExecutionInterface, "Function Execution Interface must not be null"); Assert.isTrue(functionExecutionInterface.isInterface(), - String.format("Function execution type [%s] must be an interface", - functionExecutionInterface.getClass().getName())); + String.format("Function Execution type [%s] must be an interface", + functionExecutionInterface.getName())); this.functionExecutionInterface = functionExecutionInterface; this.gemfireFunctionOperations = gemfireFunctionOperations; this.methodMetadata = new DefaultFunctionExecutionMethodMetadata(functionExecutionInterface); } + @Override + public ClassLoader getBeanClassLoader() { + + ClassLoader beanClassLoader = super.getBeanClassLoader(); + + return beanClassLoader != null ? beanClassLoader : ClassUtils.getDefaultClassLoader(); + } + + protected Class getFunctionExecutionInterface() { + return this.functionExecutionInterface; + } + + protected FunctionExecutionMethodMetadata getFunctionExecutionMethodMetadata() { + return this.methodMetadata; + } + protected GemfireFunctionOperations getGemfireFunctionOperations() { return this.gemfireFunctionOperations; } @Override - public void setBeanClassLoader(ClassLoader classLoader) { - this.beanClassLoader = classLoader; - } - - @Override - public Object invoke(MethodInvocation invocation) throws Throwable { + public Object invoke(MethodInvocation invocation) { if (AopUtils.isToStringMethod(invocation.getMethod())) { - return String.format("Function Proxy for interface [%s]", this.functionExecutionInterface.getName()); + return String.format("Function Proxy for interface [%s]", getFunctionExecutionInterface().getName()); } - if (logger.isDebugEnabled()) { - logger.debug("Invoking method {}", invocation.getMethod().getName()); - } + logDebug("Invoking method {}", invocation.getMethod().getName()); Object result = invokeFunction(invocation.getMethod(), invocation.getArguments()); - if(!invocation.getMethod().getReturnType().isAssignableFrom(result.getClass()) && result instanceof Iterable) { - Iterable iter = (Iterable)result; - if(StreamSupport.stream(iter.spliterator(), false).count() == 1) { - result = iter.iterator().next(); - } - } - - return result; + return resolveResult(invocation, result); } protected Object invokeFunction(Method method, Object[] args) { - return this.gemfireFunctionOperations - .executeAndExtract(this.methodMetadata.getMethodMetadata(method).getFunctionId(), args); + return getGemfireFunctionOperations() + .executeAndExtract(getFunctionExecutionMethodMetadata().getMethodMetadata(method).getFunctionId(), args); + } + + protected Object resolveResult(MethodInvocation invocation, Object result) { + + // TODO: This conditional logic needs more work! For instance, this conditional logic fails if the result + // is a List, but the Function (Execution method) return type is a Set. + return isIterable(result) && isNotInstanceOfFunctionReturnType(invocation, result) + ? resolveSingleResultIfPossible((Iterable) result) + : result; + } + + protected Object resolveSingleResultIfPossible(Iterable results) { + + // TODO: Determine whether to throw an IncorrectResultSizeDataAccessException if the cardinality does not match. + return StreamSupport.stream(results.spliterator(), false).count() == 1 + ? results.iterator().next() + : results; + } + + protected boolean isInstanceOfFunctionReturnType(MethodInvocation invocation, Object value) { + return invocation.getMethod().getReturnType().isInstance(value); + } + + protected boolean isNotInstanceOfFunctionReturnType(MethodInvocation invocation, Object value) { + return !isInstanceOfFunctionReturnType(invocation, value); + } + + protected boolean isIterable(Object value) { + return value instanceof Iterable; } @Override @@ -122,21 +155,16 @@ public class GemfireFunctionProxyFactoryBean implements BeanClassLoaderAware, Fa @Override public Class getObjectType() { - return this.functionExecutionInterface; - } - - @Override - public boolean isSingleton() { - return true; + return getFunctionExecutionInterface(); } protected void onInit() { if (!this.initialized) { - ProxyFactory proxyFactory = new ProxyFactory(this.functionExecutionInterface, this); + ProxyFactory proxyFactory = new ProxyFactory(getFunctionExecutionInterface(), this); - this.functionExecutionProxy = proxyFactory.getProxy(this.beanClassLoader); + this.functionExecutionProxy = proxyFactory.getProxy(getBeanClassLoader()); this.initialized = true; } } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/result/FunctionResultTypeIntegrationTest.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/result/FunctionResultTypeIntegrationTest.java index c612abde..765419e2 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/result/FunctionResultTypeIntegrationTest.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/result/FunctionResultTypeIntegrationTest.java @@ -10,75 +10,56 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ - package org.springframework.data.gemfire.function.result; -import org.apache.geode.cache.DataPolicy; -import org.apache.geode.cache.GemFireCache; -import org.apache.geode.cache.client.ClientRegionShortcut; -import org.junit.AfterClass; -import org.junit.BeforeClass; +import static org.assertj.core.api.Assertions.assertThat; + +import java.math.BigDecimal; +import java.util.Collections; +import java.util.List; + import org.junit.Test; import org.junit.runner.RunWith; + +import org.apache.geode.cache.GemFireCache; + import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.data.gemfire.ReplicatedRegionFactoryBean; -import org.springframework.data.gemfire.client.ClientRegionFactoryBean; -import org.springframework.data.gemfire.config.annotation.CacheServerApplication; -import org.springframework.data.gemfire.config.annotation.ClientCacheApplication; +import org.springframework.data.gemfire.config.annotation.PeerCacheApplication; import org.springframework.data.gemfire.function.annotation.FunctionId; import org.springframework.data.gemfire.function.annotation.GemfireFunction; import org.springframework.data.gemfire.function.annotation.OnRegion; import org.springframework.data.gemfire.function.config.EnableGemfireFunctionExecutions; import org.springframework.data.gemfire.function.config.EnableGemfireFunctions; -import org.springframework.data.gemfire.process.ProcessWrapper; import org.springframework.data.gemfire.test.support.ClientServerIntegrationTestsSupport; -import org.springframework.data.gemfire.transaction.config.EnableGemfireCacheTransactions; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringRunner; -import java.io.IOException; -import java.math.BigDecimal; -import java.util.Collections; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; - /** + * Integration Tests for Function Execution Return Types. + * * @author Patrick Johnson + * @author John Blum + * @see org.apache.geode.cache.GemFireCache + * @see org.springframework.data.gemfire.client.ClientRegionFactoryBean + * @see org.springframework.data.gemfire.config.annotation.ClientCacheApplication + * @see org.springframework.data.gemfire.function.annotation.GemfireFunction + * @see org.springframework.data.gemfire.function.annotation.OnRegion + * @see org.springframework.data.gemfire.function.config.EnableGemfireFunctionExecutions + * @see org.springframework.data.gemfire.function.config.EnableGemfireFunctions + * @see org.springframework.data.gemfire.test.support.ClientServerIntegrationTestsSupport + * @see org.springframework.test.context.ContextConfiguration + * @see org.springframework.test.context.junit4.SpringRunner */ @RunWith(SpringRunner.class) -@ContextConfiguration(classes = FunctionResultTypeIntegrationTest.FunctionInvocationClientApplicationConfig.class) +@ContextConfiguration(classes = FunctionResultTypeIntegrationTest.TestGeodeConfiguration.class) +@SuppressWarnings("unused") public class FunctionResultTypeIntegrationTest extends ClientServerIntegrationTestsSupport { - private static ProcessWrapper server; - @Autowired private FunctionExecutions functionExecutions; - @BeforeClass - public static void startServer() throws IOException { - - final String GEMFIRE_LOCALHOST_PORT = "localhost[%d]"; - - int availablePort = findAvailablePort(); - - System.setProperty(GEMFIRE_CACHE_SERVER_PORT_PROPERTY, String.valueOf(availablePort)); - System.setProperty(GEMFIRE_POOL_SERVERS_PROPERTY, String.format(GEMFIRE_LOCALHOST_PORT, availablePort)); - - server = run(FunctionServerApplicationConfig.class, - String.format("-D%s=%d", GEMFIRE_CACHE_SERVER_PORT_PROPERTY, availablePort)); - - waitForServerToStart(DEFAULT_HOSTNAME, availablePort); - } - - @AfterClass - public static void stopServer() { - server.stop(); - server = null; - } - @Test public void singleResultFunctionsExecuteCorrectly() { BigDecimal num = functionExecutions.returnFive(); @@ -97,40 +78,20 @@ public class FunctionResultTypeIntegrationTest extends ClientServerIntegrationTe assertThat(num).isEqualTo(7); } + @PeerCacheApplication(name = "FunctionResultTypeIntegrationTest") @EnableGemfireFunctionExecutions(basePackageClasses = FunctionExecutions.class) - @ClientCacheApplication(name = "SingleResultFunctionExecutionTest") - @EnableGemfireCacheTransactions - public static class FunctionInvocationClientApplicationConfig { + @EnableGemfireFunctions + public static class TestGeodeConfiguration { @Bean("Numbers") - protected ClientRegionFactoryBean configureProxyClientNumberRegion(GemFireCache gemFireCache) { - ClientRegionFactoryBean clientRegionFactoryBean = new ClientRegionFactoryBean<>(); - clientRegionFactoryBean.setCache(gemFireCache); - clientRegionFactoryBean.setName("Numbers"); - clientRegionFactoryBean.setShortcut(ClientRegionShortcut.PROXY); - return clientRegionFactoryBean; - } - } + protected ReplicatedRegionFactoryBean numbersRegion(GemFireCache gemFireCache) { - @EnableGemfireFunctions - @CacheServerApplication(name = "SingleResultFunctionExecutionTestServer") - public static class FunctionServerApplicationConfig { + ReplicatedRegionFactoryBean numbersRegion = new ReplicatedRegionFactoryBean<>(); - public static void main(String[] args) { + numbersRegion.setCache(gemFireCache); + numbersRegion.setPersistent(false); - AnnotationConfigApplicationContext applicationContext = - new AnnotationConfigApplicationContext(FunctionServerApplicationConfig.class); - - applicationContext.registerShutdownHook(); - } - - @Bean - ReplicatedRegionFactoryBean createNumberRegion(GemFireCache gemfireCache) { - ReplicatedRegionFactoryBean replicatedRegionFactoryBean = new ReplicatedRegionFactoryBean(); - replicatedRegionFactoryBean.setCache(gemfireCache); - replicatedRegionFactoryBean.setRegionName("Numbers"); - replicatedRegionFactoryBean.setDataPolicy(DataPolicy.REPLICATE); - return replicatedRegionFactoryBean; + return numbersRegion; } @GemfireFunction(id = "returnSingleObject", hasResult = true) @@ -161,4 +122,5 @@ interface FunctionExecutions { @FunctionId("returnPrimitive") int returnPrimitive(); + }