From 513afaa3c919b72e1b8b5d125dbbc365b0472c55 Mon Sep 17 00:00:00 2001 From: John Blum Date: Thu, 27 Feb 2020 21:45:51 -0800 Subject: [PATCH] DATAGEODE-295 - Polish. Resolves gh-37. --- .../data/gemfire/CacheFactoryBean.java | 2 +- .../data/gemfire/GemfireTemplate.java | 3 +- .../ExecutionTimeoutFunctionException.java | 43 +++ .../UncategorizedFunctionException.java | 52 +++ .../execution/AbstractFunctionExecution.java | 326 +++++++++++++----- .../execution/AbstractFunctionTemplate.java | 73 +++- .../GemfireFunctionProxyFactoryBean.java | 21 +- .../GemfireOnRegionFunctionTemplate.java | 24 +- .../execution/RegionFunctionExecution.java | 32 +- .../data/gemfire/util/SpringUtils.java | 22 +- .../data/gemfire/fork/ServerProcess.java | 27 +- ...nctionExecutionWithPdxIntegrationTest.java | 145 ++++---- ...owingFunctionExecutionIntegrationTest.java | 57 +-- .../AbstractFunctionExecutionTest.java | 65 ++-- .../AbstractFunctionTemplateTest.java | 68 ++-- .../FunctionExecutionIntegrationTests.java | 5 +- .../execution/FunctionIntegrationTests.java | 45 +-- ...fireFunctionProxyFactoryBeanUnitTests.java | 56 ++- ...ResultsFromAllServersIntegrationTests.java | 43 +-- .../ApplicationDomainFunctionExecutions.java | 6 +- .../ExceptionThrowingFunctionExecution.java | 7 +- .../gemfire/util/SpringUtilsUnitTests.java | 19 +- ...xecutionWithPdxIntegrationTest-context.xml | 4 +- ...nWithPdxIntegrationTest-server-context.xml | 2 +- ...xecutionIntegrationTest-server-context.xml | 2 +- .../FunctionIntegrationTests-context.xml | 6 +- ...unctionIntegrationTests-server-context.xml | 2 +- 27 files changed, 745 insertions(+), 412 deletions(-) create mode 100644 spring-data-geode/src/main/java/org/springframework/data/gemfire/function/ExecutionTimeoutFunctionException.java create mode 100644 spring-data-geode/src/main/java/org/springframework/data/gemfire/function/UncategorizedFunctionException.java diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/CacheFactoryBean.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/CacheFactoryBean.java index 525a2b17..23d9f2c9 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/CacheFactoryBean.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/CacheFactoryBean.java @@ -759,7 +759,7 @@ public class CacheFactoryBean extends AbstractFactoryBeanSupport * @see #getCacheXmlFile() */ private boolean isCacheXmlAvailable() { - return SpringUtils.safeGetValue(() -> getCacheXml() != null, false); + return getCacheXml() != null; } /** 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 5ecef1d5..78d85bc6 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 @@ -23,7 +23,6 @@ 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; @@ -426,7 +425,7 @@ public class GemfireTemplate extends GemfireAccessor implements GemfireOperation if (RegionUtils.isLocal(region)) { - Supplier hasServerProxyMethod = () -> + SpringUtils.ValueReturningThrowableOperation hasServerProxyMethod = () -> Optional.ofNullable(ReflectionUtils.findMethod(region.getClass(), "hasServerProxy")) .map(method -> ReflectionUtils.invokeMethod(method, region)) .map(Boolean.FALSE::equals) diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/ExecutionTimeoutFunctionException.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/ExecutionTimeoutFunctionException.java new file mode 100644 index 00000000..62d9d3be --- /dev/null +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/ExecutionTimeoutFunctionException.java @@ -0,0 +1,43 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on 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; + +import org.apache.geode.cache.execute.FunctionException; + +/** + * A {@link FunctionException} indicating a timeout during execution. + * + * @author John Blum + * @see org.apache.geode.cache.execute.FunctionException + * @since 2.3.0 + */ +@SuppressWarnings("unused") +public class ExecutionTimeoutFunctionException extends FunctionException { + + public ExecutionTimeoutFunctionException() { } + + public ExecutionTimeoutFunctionException(String message) { + super(message); + } + + public ExecutionTimeoutFunctionException(Throwable cause) { + super(cause); + } + + public ExecutionTimeoutFunctionException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/UncategorizedFunctionException.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/UncategorizedFunctionException.java new file mode 100644 index 00000000..e897946c --- /dev/null +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/UncategorizedFunctionException.java @@ -0,0 +1,52 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on 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; + +import org.apache.geode.cache.execute.Execution; +import org.apache.geode.cache.execute.Function; +import org.apache.geode.cache.execute.FunctionException; + +/** + * An {@link FunctionException} indicating a {@link Function} {@link Execution} {@link RuntimeException} + * that has not be categorized, or identified by the framework. + * + * This {@link RuntimeException} was inspired by the {@link org.springframework.dao.UncategorizedDataAccessException}. + * + * @author John Blum + * @see org.apache.geode.cache.execute.Execution + * @see org.apache.geode.cache.execute.Function + * @see org.apache.geode.cache.execute.FunctionException + * @since 2.3.0 + */ +@SuppressWarnings("unused") +public class UncategorizedFunctionException extends FunctionException { + + public UncategorizedFunctionException() { + super(null, null); + } + + public UncategorizedFunctionException(String message) { + super(message, null); + } + + public UncategorizedFunctionException(Throwable cause) { + super(null, cause); + } + + public UncategorizedFunctionException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecution.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecution.java index b4b4e680..0d8e6949 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecution.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecution.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at @@ -14,6 +14,7 @@ package org.springframework.data.gemfire.function.execution; import java.util.Collections; import java.util.Iterator; +import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -23,167 +24,323 @@ import org.apache.geode.cache.execute.FunctionException; import org.apache.geode.cache.execute.FunctionService; import org.apache.geode.cache.execute.ResultCollector; +import org.springframework.data.gemfire.function.ExecutionTimeoutFunctionException; +import org.springframework.data.gemfire.function.UncategorizedFunctionException; +import org.springframework.data.gemfire.util.SpringUtils; +import org.springframework.data.gemfire.util.SpringUtils.ValueReturningThrowableOperation; +import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.util.Assert; - /** - * Base class for * Creating a GemFire {@link Execution} using {@link FunctionService}. Protected setters support - * method chaining. + * Abstract base class for creating a {@link Function} {@link Execution} using the {@link FunctionService}. * * @author David Turanski * @author John Blum * @author Patrick Johnson + * @see java.util.concurrent.TimeUnit + * @see org.apache.geode.cache.execute.Execution + * @see org.apache.geode.cache.execute.Function + * @see org.apache.geode.cache.execute.FunctionService + * @see org.apache.geode.cache.execute.ResultCollector */ @SuppressWarnings("unused") abstract class AbstractFunctionExecution { - private final static String NO_RESULT_MESSAGE = "Cannot return any result as the Function#hasResult() is false"; + private static final boolean DEFAULT_RETURN_RESULT = true; + + private static final String FUNCTION_EXECUTION_TIMEOUT_ERROR_MESSAGE = + "Failed to collect Function [%1$s] results in the configured timeout [%2$d ms]"; + + private static final String NO_RESULT_ERROR_MESSAGE = + "Cannot return any result as the Function#hasResult() is false"; private long timeout; + @SuppressWarnings("rawtypes") private Function function; - protected final Logger logger = LoggerFactory.getLogger(this.getClass()); + private final Logger logger = LoggerFactory.getLogger(this.getClass()); - private Object[] args; + private Object[] arguments; private volatile ResultCollector resultCollector; private String functionId; - public AbstractFunctionExecution(Function function, Object... args) { + @SuppressWarnings("rawtypes") + public AbstractFunctionExecution(Function function, Object... arguments) { Assert.notNull(function, "Function cannot be null"); this.function = function; this.functionId = function.getId(); - this.args = args; + this.arguments = arguments; } - public AbstractFunctionExecution(String functionId, Object... args) { + public AbstractFunctionExecution(String functionId, Object... arguments) { Assert.hasText(functionId, "Function ID must not be null or empty"); + this.function = null; this.functionId = functionId; - this.args = args; + this.arguments = arguments; } AbstractFunctionExecution() { } - Object[] getArgs() { - return this.args; + protected Object[] getArguments() { + return this.arguments; } - ResultCollector getCollector() { - return this.resultCollector; - } + @SuppressWarnings("rawtypes") + protected abstract Execution getExecution(); - Function getFunction() { + @SuppressWarnings("rawtypes") + protected Function getFunction() { return this.function; } - String getFunctionId() { + protected String getFunctionId() { return this.functionId; } - long getTimeout() { + protected Set getKeys() { + return null; + } + + protected Logger getLogger() { + return this.logger; + } + + protected ResultCollector getResultCollector() { + return this.resultCollector; + } + + protected long getTimeout() { return this.timeout; } - Iterable execute() { - return execute(true); + String resolveFunctionIdentifier() { + + return Optional.ofNullable(getFunction()) + .map(ObjectUtils::nullSafeClassName) + .orElseGet(() -> getFunctionId()); } - @SuppressWarnings("unchecked") + /** + * Executes the configured {@link Function}. + * + * @param {@link Class type} of the result. + * @return an {@link Iterable} containing the results from the {@link Function} {@link Execution}. + * @see java.lang.Iterable + * @see #execute(Boolean) + */ + Iterable execute() { + return execute(DEFAULT_RETURN_RESULT); + } + + /** + * Executes the configured {@link Function}. + * + * @param {@link Class type} of the result. + * @param returnResult boolean value indicating whether the {@link Function} should return a result + * from the {@link Execution}. + * @return an {@link Iterable} containing the results from the {@link Function} {@link Execution}. + * @see java.lang.Iterable + * @see #getExecution() + * @see #getFunction() + * @see #getFunctionId() + * @see #getTimeout() + * @see #prepare(Execution) + */ + @SuppressWarnings({ "rawtypes" }) Iterable execute(Boolean returnResult) { - Execution execution = getExecution(); + Execution execution = prepare(getExecution()); - execution = execution.setArguments(getArgs()); - execution = getCollector() != null ? execution.withCollector(getCollector()) : execution; - execution = getKeys() != null ? execution.withFilter(getKeys()) : execution; + Function function = getFunction(); - ResultCollector resultCollector; + ResultCollector resultCollector = function != null + ? execution.execute(function) + : execution.execute(getFunctionId()); - if (isRegisteredFunction()) { - resultCollector = execution.execute(this.functionId); - } - else { - resultCollector = execution.execute(this.function); - - if (!this.function.hasResult()) { - return null; - } - } - - if (!returnResult) { + if (hasNoResult(returnResult, function, resultCollector)) { return null; } - if (logger.isDebugEnabled()) { - logger.debug("Using ResultsCollector " + resultCollector.getClass().getName()); - } + long timeout = getTimeout(); + + logDebug("Configured timeout is [{} ms]", timeout); + logDebug("Using ResultCollector [{}]", ObjectUtils.nullSafeClassName(resultCollector)); Iterable results = null; try { - if (this.timeout > 0) { - try { - results = (Iterable) resultCollector.getResult(this.timeout, TimeUnit.MILLISECONDS); - } - catch (FunctionException | InterruptedException cause) { - throw new RuntimeException(cause); - } - } - else { - if(resultCollector.getResult() instanceof Iterable) { - results = (Iterable) resultCollector.getResult(); - } else { - results = (Iterable) Collections.singleton(resultCollector.getResult()); - } - } - return replaceSingletonNullCollectionWithEmptyList(results); + Object result = timeout > 0 + ? SpringUtils.safeGetValue(getResultWithTimeoutThrowableOperation(resultCollector, timeout), + newFunctionAndInterruptedExceptionHandler(timeout)) + : resultCollector.getResult(); + + results = processResult(result); + + return results; } catch (FunctionException cause) { - // TODO Come up with a better way to determine that the function should not return a result; - if (!cause.getMessage().equals(NO_RESULT_MESSAGE)) { + + // TODO: Use a more reliable way to determine that the Function does not return a result! + // This only applies to Functions registered by ID! + if (!cause.getMessage().contains(NO_RESULT_ERROR_MESSAGE)) { throw cause; } + + return results; + } + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + protected Execution prepare(Execution execution) { + + execution = execution.setArguments(getArguments()); + execution = getResultCollector() != null ? execution.withCollector(getResultCollector()) : execution; + execution = getKeys() != null ? execution.withFilter(getKeys()) : execution; + + return execution; + } + + @SuppressWarnings("rawtypes") + private boolean hasResult(boolean returnResult, Function function, ResultCollector resultCollector) { + return returnResult && (function == null || function.hasResult()); + } + + @SuppressWarnings("rawtypes") + private boolean hasNoResult(boolean returnResult, Function function, ResultCollector resultCollector) { + return !hasResult(returnResult, function, resultCollector); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + private ValueReturningThrowableOperation getResultWithTimeoutThrowableOperation( + ResultCollector resultCollector, long timeout) { + + return () -> (T) resultCollector.getResult(timeout, TimeUnit.MILLISECONDS); + } + + private java.util.function.Function newFunctionAndInterruptedExceptionHandler(long timeout) { + + return cause -> { + + if (cause instanceof FunctionException) { + throw (FunctionException) cause; + } + else if (cause instanceof InterruptedException) { + + String message = + String.format(FUNCTION_EXECUTION_TIMEOUT_ERROR_MESSAGE, resolveFunctionIdentifier(), timeout); + + throw new ExecutionTimeoutFunctionException(message, cause); + } + + throw new UncategorizedFunctionException(cause); + }; + } + + private Iterable processResult(Object result) { + + return replaceSingleNullElementIterableWithEmptyIterable(throwOnExceptionOrReturn(toIterable( + throwOnExceptionOrReturn(result)))); + } + + private Iterable replaceSingleNullElementIterableWithEmptyIterable(Iterable results) { + + if (results != null) { + + Iterator it = results.iterator(); + + if (!it.hasNext()) { + return results; + } + + if (it.next() == null && !it.hasNext()) { + return Collections::emptyIterator; + } } return results; } + private Iterable throwOnExceptionOrReturn(Iterable result) { + + Iterator resultIterator = result.iterator(); + + if (resultIterator.hasNext()) { + throwOnExceptionOrReturn(resultIterator.next()); + } + + return result; + } + @SuppressWarnings("unchecked") + private Iterable toIterable(Object result) { + + return result instanceof Iterable + ? (Iterable) result + : (Iterable) Collections.singleton(result); + } + + /** + * Executes the configured {@link Function} and extracts the result as a single value. + * + * @param {@link Class type} of the result. + * @return the result of the {@link Function} {@link Execution} as a single value. + * @see #execute() + */ T executeAndExtract() { Iterable results = execute(); - if (results == null || !results.iterator().hasNext()) { + if (isEmpty(results)) { return null; } - Object result = results.iterator().next(); + T result = results.iterator().next(); - if (result instanceof Throwable) { - throw new FunctionException(String.format("Execution of Function %s failed", - (this.function != null ? this.function.getClass().getName() - : String.format("with ID [%s]", this.functionId))), (Throwable) result); - } - - return (T) result; + return throwOnExceptionOrReturn(result); } - protected abstract Execution getExecution(); + private boolean isEmpty(Iterable iterable) { + return iterable == null || !iterable.iterator().hasNext(); + } + private T throwOnExceptionOrReturn(T result) { + + if (result instanceof Throwable) { + + Function function = getFunction(); + + String message = String.format("Execution of Function [%s] failed", function != null + ? function.getClass().getName() + : String.format("with ID [%s]", getFunctionId())); + + throw new FunctionException(message, (Throwable) result); + } + + return result; + } + + @Deprecated protected AbstractFunctionExecution setArgs(Object... args) { - this.args = args; + return setArguments(args); + } + + protected AbstractFunctionExecution setArguments(Object... arguments) { + this.arguments = arguments; return this; } + @SuppressWarnings("rawtypes") protected AbstractFunctionExecution setFunction(Function function) { this.function = function; return this; @@ -204,29 +361,12 @@ abstract class AbstractFunctionExecution { return this; } - protected Set getKeys() { - return null; - } + protected void logDebug(String message, Object... arguments) { - private boolean isRegisteredFunction() { - return this.function == null; - } + Logger logger = getLogger(); - private Iterable replaceSingletonNullCollectionWithEmptyList(Iterable results) { - - if (results != null) { - - Iterator it = results.iterator(); - - if (!it.hasNext()) { - return results; - } - - if (it.next() == null && !it.hasNext()) { - return Collections.emptyList(); - } + if (logger.isDebugEnabled()) { + logger.debug(message, arguments); } - - return results; } } diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplate.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplate.java index b48d2ced..36c24fd6 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplate.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplate.java @@ -10,7 +10,6 @@ * 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.execution; import org.apache.geode.cache.execute.Function; @@ -19,47 +18,77 @@ import org.apache.geode.cache.execute.ResultCollector; import org.springframework.beans.factory.InitializingBean; /** - * The base class for {@link Function} templates used to invoke Apache Geode/Pivotal GemFire {@link Function Functions}. + * Abstract base class for all {@link Function} templates, containing operations common to invoking Apache Geode + * or Pivotal GemFire {@link Function Functions}. * * @author David Turanski * @author John Blum + * @see org.apache.geode.cache.execute.Execution * @see org.apache.geode.cache.execute.Function * @see org.apache.geode.cache.execute.ResultCollector * @see org.springframework.beans.factory.InitializingBean * @see org.springframework.data.gemfire.function.execution.GemfireFunctionOperations + * @see org.springframework.data.gemfire.function.execution.AbstractFunctionExecution */ abstract class AbstractFunctionTemplate implements GemfireFunctionOperations, InitializingBean { - protected long timeout; + private volatile long timeout; - protected volatile ResultCollector resultCollector; + private volatile ResultCollector resultCollector; @Override public void afterPropertiesSet() throws Exception { } @Override + @SuppressWarnings("rawtypes") public Iterable execute(Function function, Object... args) { - return execute(getFunctionExecution().setArgs(args).setFunction(function)); + + AbstractFunctionExecution functionExecution = getFunctionExecution() + .setArguments(args) + .setFunction(function); + + return execute(functionExecution); } @Override + @SuppressWarnings("rawtypes") public T executeAndExtract(Function function, Object... args) { - return executeAndExtract(getFunctionExecution().setArgs(args).setFunction(function)); + + AbstractFunctionExecution functionExecution = getFunctionExecution() + .setArguments(args) + .setFunction(function); + + return executeAndExtract(functionExecution); } @Override public Iterable execute(String functionId, Object... args) { - return execute(getFunctionExecution().setArgs(args).setFunctionId(functionId)); + + AbstractFunctionExecution functionExecution = getFunctionExecution() + .setArguments(args) + .setFunctionId(functionId); + + return execute(functionExecution); } @Override public T executeAndExtract(String functionId, Object... args) { - return executeAndExtract(getFunctionExecution().setArgs(args).setFunctionId(functionId)); + + AbstractFunctionExecution functionExecution = getFunctionExecution() + .setArguments(args) + .setFunctionId(functionId); + + return executeAndExtract(functionExecution); } @Override public void executeWithNoResult(String functionId, Object... args) { - execute(getFunctionExecution().setArgs(args).setFunctionId(functionId), false); + + AbstractFunctionExecution functionExecution = getFunctionExecution() + .setArguments(args) + .setFunctionId(functionId); + + execute(functionExecution, false); } @Override @@ -67,18 +96,27 @@ abstract class AbstractFunctionTemplate implements GemfireFunctionOperations, In return callback.doInGemfire(getFunctionExecution().getExecution()); } - protected Iterable execute(AbstractFunctionExecution execution) { - return execution.setTimeout(timeout).setResultCollector(resultCollector).execute(); + protected Iterable execute(AbstractFunctionExecution functionExecution) { + return prepare(functionExecution).execute(); } - protected Iterable execute(AbstractFunctionExecution execution, boolean returnResult) { - return execution.setTimeout(timeout).setResultCollector(resultCollector).execute(returnResult); + protected Iterable execute(AbstractFunctionExecution functionExecution, boolean returnResult) { + return prepare(functionExecution).execute(returnResult); } - protected T executeAndExtract(AbstractFunctionExecution execution) { - return execution.setTimeout(timeout).setResultCollector(resultCollector).executeAndExtract(); + protected T executeAndExtract(AbstractFunctionExecution functionExecution) { + return prepare(functionExecution).executeAndExtract(); } + AbstractFunctionExecution prepare(AbstractFunctionExecution functionExecution) { + + return functionExecution + .setResultCollector(getResultCollector()) + .setTimeout(getTimeout()); + } + + protected abstract AbstractFunctionExecution getFunctionExecution(); + public void setResultCollector(ResultCollector resultCollector) { this.resultCollector = resultCollector; } @@ -91,6 +129,7 @@ abstract class AbstractFunctionTemplate implements GemfireFunctionOperations, In this.timeout = timeout; } - protected abstract AbstractFunctionExecution getFunctionExecution(); - + public long getTimeout() { + return this.timeout; + } } 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 aaaa09e1..923ec9de 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 @@ -18,7 +18,6 @@ import java.util.stream.StreamSupport; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.FactoryBean; -import org.springframework.data.gemfire.function.annotation.OnServers; import org.springframework.data.gemfire.support.AbstractFactoryBeanSupport; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -26,10 +25,6 @@ import org.springframework.util.ClassUtils; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -<<<<<<>>>>>>DATAGEODE-295-Functions return results from all servers. - /** * A Proxy {@link FactoryBean} for all non-Region Function Execution interfaces. * @@ -105,7 +100,7 @@ public class GemfireFunctionProxyFactoryBean extends AbstractFactoryBeanSupport< return String.format("Function Proxy for interface [%s]", getFunctionExecutionInterface().getName()); } - logDebug("Invoking method {}", invocation.getMethod().getName()); + logDebug("Invoking method [{}]", invocation.getMethod().getName()); Object result = invokeFunction(invocation.getMethod(), invocation.getArguments()); @@ -114,17 +109,19 @@ public class GemfireFunctionProxyFactoryBean extends AbstractFactoryBeanSupport< protected Object invokeFunction(Method method, Object[] args) { - GemfireFunctionOperations template = getGemfireFunctionOperations(); + String functionId = getFunctionExecutionMethodMetadata() + .getMethodMetadata(method) + .getFunctionId(); - return method.getDeclaringClass().isAnnotationPresent(OnServers.class) - ? template.execute(getFunctionExecutionMethodMetadata().getMethodMetadata(method).getFunctionId(), args) - : template.executeAndExtract(getFunctionExecutionMethodMetadata().getMethodMetadata(method).getFunctionId(), args); + return getGemfireFunctionOperations().execute(functionId, 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. + // TODO: The conditional logic needs more work! + // For example, this conditional logic will fail if the result is a List but the Function (Execution method) + // return type is a Set. + // TODO: Apply Spring Converters here??? return isIterable(result) && isNotInstanceOfFunctionReturnType(invocation, result) ? resolveSingleResultIfPossible((Iterable) result) : result; diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireOnRegionFunctionTemplate.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireOnRegionFunctionTemplate.java index f441270b..796cd842 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireOnRegionFunctionTemplate.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/GemfireOnRegionFunctionTemplate.java @@ -20,8 +20,14 @@ import org.apache.geode.cache.execute.Function; import org.springframework.util.Assert; /** + * An {@link AbstractFunctionTemplate} implementation for executing a {@link Function} on a target {@link Region}. + * * @author David Turanski * @author John Blum + * @see org.apache.geode.cache.Region + * @see org.apache.geode.cache.execute.Function + * @see org.springframework.data.gemfire.function.execution.AbstractFunctionTemplate + * @see org.springframework.data.gemfire.function.execution.GemfireOnRegionOperations */ public class GemfireOnRegionFunctionTemplate extends AbstractFunctionTemplate implements GemfireOnRegionOperations { @@ -31,7 +37,7 @@ public class GemfireOnRegionFunctionTemplate extends AbstractFunctionTemplate im * Constructs a new instance of the {@link GemfireOnRegionFunctionTemplate} initialized with * the given {@link Region}. * - * @param region the {@link Region} upon which the {@link Function} will be executed. + * @param region {@link Region} on which the {@link Function} will be executed. * @throws IllegalArgumentException if {@link Region} is {@literal null}. * @see org.apache.geode.cache.Region */ @@ -44,7 +50,11 @@ public class GemfireOnRegionFunctionTemplate extends AbstractFunctionTemplate im @Override protected RegionFunctionExecution getFunctionExecution() { - return new RegionFunctionExecution(this.region); + return new RegionFunctionExecution(getRegion()); + } + + protected Region getRegion() { + return this.region; } @Override @@ -52,9 +62,9 @@ public class GemfireOnRegionFunctionTemplate extends AbstractFunctionTemplate im return execute(getFunctionExecution() .setKeys(keys) + .setArguments(args) .setFunctionId(functionId) - .setTimeout(this.timeout) - .setArgs(args)); + .setTimeout(getTimeout())); } @Override @@ -63,7 +73,7 @@ public class GemfireOnRegionFunctionTemplate extends AbstractFunctionTemplate im return executeAndExtract(getFunctionExecution() .setKeys(keys) .setFunctionId(functionId) - .setTimeout(this.timeout).setArgs(args)); + .setTimeout(getTimeout()).setArguments(args)); } @Override @@ -71,8 +81,8 @@ public class GemfireOnRegionFunctionTemplate extends AbstractFunctionTemplate im execute(getFunctionExecution() .setKeys(keys) + .setArguments(args) .setFunctionId(functionId) - .setTimeout(this.timeout) - .setArgs(args), false); + .setTimeout(getTimeout()), false); } } diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/RegionFunctionExecution.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/RegionFunctionExecution.java index df97540e..debc7e42 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/RegionFunctionExecution.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/function/execution/RegionFunctionExecution.java @@ -16,14 +16,24 @@ import java.util.Set; import org.apache.geode.cache.Region; import org.apache.geode.cache.execute.Execution; +import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionService; +import org.apache.shiro.util.Assert; + import org.springframework.util.CollectionUtils; /** - * Creates a GemFire {@link Execution} using {code}FunctionService.onRegion(Region region){code} - * @author David Turanski + * {@link RegionFunctionExecution} creates a {@link Function} {@link Execution} + * using {@link FunctionService#onRegion(Region)}. * + * @author David Turanski + * @author John Blum + * @see org.apache.geode.cache.Region + * @see org.apache.geode.cache.execute.Execution + * @see org.apache.geode.cache.execute.Function + * @see org.apache.geode.cache.execute.FunctionService + * @see org.springframework.data.gemfire.function.execution.AbstractFunctionExecution */ class RegionFunctionExecution extends AbstractFunctionExecution { @@ -32,6 +42,9 @@ class RegionFunctionExecution extends AbstractFunctionExecution { private volatile Set keys; public RegionFunctionExecution(Region region) { + + Assert.notNull(region, "Region must not be null"); + this.region = region; } @@ -44,20 +57,19 @@ class RegionFunctionExecution extends AbstractFunctionExecution { return this.keys; } - /* (non-Javadoc) - * @see org.springframework.data.gemfire.function.FunctionExecution#getExecution() - */ + protected Region getRegion() { + return this.region; + } + @Override - @SuppressWarnings("unchecked") + @SuppressWarnings({ "rawtypes", "unchecked" }) protected Execution getExecution() { - Execution execution = FunctionService.onRegion(this.region); + Execution execution = FunctionService.onRegion(getRegion()); Set keys = getKeys(); - if (!CollectionUtils.isEmpty(keys) ) { - execution = execution.withFilter(keys); - } + execution = CollectionUtils.isEmpty(keys) ? execution : execution.withFilter(keys); return execution; } diff --git a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java index 71be576d..0b79c2ef 100644 --- a/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java +++ b/spring-data-geode/src/main/java/org/springframework/data/gemfire/util/SpringUtils.java @@ -152,22 +152,23 @@ public abstract class SpringUtils { } } - public static T safeGetValue(Supplier valueSupplier) { - return safeGetValue(valueSupplier, (T) null); + public static T safeGetValue(ValueReturningThrowableOperation operation) { + return safeGetValue(operation, (T) null); } - public static T safeGetValue(Supplier valueSupplier, T defaultValue) { - return safeGetValue(valueSupplier, (Supplier) () -> defaultValue); + public static T safeGetValue(ValueReturningThrowableOperation operation, T defaultValue) { + return safeGetValue(operation, (Supplier) () -> defaultValue); } - public static T safeGetValue(Supplier valueSupplier, Supplier defaultValueSupplier) { - return safeGetValue(valueSupplier, (Function) exception -> defaultValueSupplier.get()); + public static T safeGetValue(ValueReturningThrowableOperation operation, Supplier defaultValueSupplier) { + return safeGetValue(operation, (Function) exception -> defaultValueSupplier.get()); } - public static T safeGetValue(Supplier valueSupplier, Function exceptionHandler) { + public static T safeGetValue(ValueReturningThrowableOperation operation, + Function exceptionHandler) { try { - return valueSupplier.get(); + return operation.get(); } catch (Throwable cause) { return exceptionHandler.apply(cause); @@ -189,6 +190,11 @@ public abstract class SpringUtils { } } + @FunctionalInterface + public interface ValueReturningThrowableOperation { + T get() throws Throwable; + } + /** * @deprecated use {@link VoidReturningThrowableOperation}. */ diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/fork/ServerProcess.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/fork/ServerProcess.java index 7313febc..d2aab631 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/fork/ServerProcess.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/fork/ServerProcess.java @@ -13,24 +13,23 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.fork; import java.io.File; import java.io.IOException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.data.gemfire.process.support.ProcessUtils; import org.springframework.data.gemfire.test.support.FileSystemUtils; import org.springframework.util.Assert; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** - * The ServerProcess class is a main Java class using Spring Data GemFire to configure and bootstrap - * a GemFire Cache Server process. + * The {@link ServerProcess} class is a main Java class using Spring Data for Apache Geode to configure and bootstrap + * an Apache Geode Server process. * * @author John Blum * @see org.springframework.context.ConfigurableApplicationContext @@ -42,15 +41,16 @@ public class ServerProcess { private static final Logger logger = LoggerFactory.getLogger(ServerProcess.class); public static void main(String[] args) throws Throwable { + ConfigurableApplicationContext applicationContext = null; try { applicationContext = newApplicationContext(args); waitForShutdown(); } - catch (Throwable e) { - logger.debug("", e); - throw e; + catch (Throwable cause) { + logger.debug("", cause); + throw cause; } finally { close(applicationContext); @@ -58,6 +58,7 @@ public class ServerProcess { } private static ConfigurableApplicationContext newApplicationContext(String[] configLocations) { + Assert.notEmpty(configLocations, String.format("Usage: >java -cp ... %1$s %2$s", ServerProcess.class.getName(), "classpath:/to/applicationContext.xml")); @@ -69,8 +70,11 @@ public class ServerProcess { } private static boolean close(ConfigurableApplicationContext applicationContext) { + if (applicationContext != null) { + applicationContext.close(); + return !(applicationContext.isRunning() || applicationContext.isActive()); } @@ -78,9 +82,10 @@ public class ServerProcess { } private static void waitForShutdown() throws IOException { - ProcessUtils.writePid(new File(FileSystemUtils.WORKING_DIRECTORY, getServerProcessControlFilename()), - ProcessUtils.currentPid()); + File serverProcessControlFile = new File(FileSystemUtils.WORKING_DIRECTORY, getServerProcessControlFilename()); + + ProcessUtils.writePid(serverProcessControlFile, ProcessUtils.currentPid()); ProcessUtils.waitForStopSignal(); } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest.java index e43f35ed..70310223 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest.java @@ -13,18 +13,21 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.function; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + import org.apache.geode.cache.client.ClientCache; import org.apache.geode.pdx.PdxInstance; import org.apache.geode.pdx.PdxInstanceFactory; @@ -33,16 +36,10 @@ import org.apache.geode.pdx.PdxSerializer; import org.apache.geode.pdx.PdxWriter; import org.apache.geode.pdx.internal.PdxInstanceEnum; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.runner.RunWith; - import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.gemfire.fork.ServerProcess; -import org.springframework.data.gemfire.fork.SpringContainerProcess; import org.springframework.data.gemfire.function.annotation.GemfireFunction; import org.springframework.data.gemfire.function.sample.ApplicationDomainFunctionExecutions; import org.springframework.data.gemfire.process.ProcessWrapper; @@ -53,27 +50,30 @@ import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; /** - * The ClientCacheFunctionExecutionWithPdxIntegrationTest class is a test suite of test cases testing Spring Data - * GemFire's Function annotation support and interaction between a GemFire client and server Cache - * when PDX is configured and read-serialized is set to true. + * Integration Test for SDG's Function annotation support and interaction between an Apache Geode client + * and server cache when PDX is configured and read-serialized is set to {@literal true}. * * @author John Blum * @see org.junit.Test - * @see org.junit.runner.RunWith - * @see SpringContainerProcess - * @see org.springframework.data.gemfire.function.annotation.GemfireFunction - * @see org.springframework.data.gemfire.function.sample.ApplicationDomainFunctionExecutions - * @see org.springframework.test.context.ContextConfiguration - * @see org.springframework.test.context.junit4.SpringRunner * @see org.apache.geode.cache.client.ClientCache * @see org.apache.geode.pdx.PdxInstance + * @see org.apache.geode.pdx.PdxInstanceFactory + * @see org.apache.geode.pdx.PdxReader * @see org.apache.geode.pdx.PdxSerializer + * @see org.apache.geode.pdx.PdxWriter * @see org.apache.geode.pdx.internal.PdxInstanceEnum + * @see org.springframework.data.gemfire.function.annotation.GemfireFunction + * @see org.springframework.data.gemfire.function.sample.ApplicationDomainFunctionExecutions + * @see org.springframework.data.gemfire.process.ProcessWrapper + * @see org.springframework.data.gemfire.test.support.ClientServerIntegrationTestsSupport + * @see org.springframework.test.context.ContextConfiguration + * @see org.springframework.test.context.junit4.SpringRunner * @since 1.5.2 */ @RunWith(SpringRunner.class) @ContextConfiguration @SuppressWarnings("unused") +// TODO: Convert to a ClientCache test. public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientServerIntegrationTestsSupport { private static ProcessWrapper gemfireServer; @@ -107,7 +107,7 @@ public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientSe private PdxInstance toPdxInstance(Map pdxData) { PdxInstanceFactory pdxInstanceFactory = - gemfireClientCache.createPdxInstanceFactory(pdxData.get("@type").toString()); + this.gemfireClientCache.createPdxInstanceFactory(pdxData.get("@type").toString()); for (Map.Entry entry : pdxData.entrySet()) { pdxInstanceFactory.writeObject(entry.getKey(), entry.getValue()); @@ -119,79 +119,80 @@ public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientSe @Test public void convertedFunctionArgumentTypes() { - Class[] argumentTypes = functionExecutions.captureConvertedArgumentTypes("test", 1, - Boolean.TRUE, new Person("Jon", "Doe"), Gender.MALE); + Class[] argumentTypes = this.functionExecutions + .captureConvertedArgumentTypes("test", 1, Boolean.TRUE, new Person("Jon", "Doe"), + Gender.MALE); - assertNotNull(argumentTypes); - assertEquals(5, argumentTypes.length); - assertEquals(String.class, argumentTypes[0]); - assertEquals(Integer.class, argumentTypes[1]); - assertEquals(Boolean.class, argumentTypes[2]); - assertEquals(Person.class, argumentTypes[3]); - assertEquals(Gender.class, argumentTypes[4]); + assertThat(argumentTypes).isNotNull(); + assertThat(argumentTypes.length).isEqualTo(5); + assertThat(argumentTypes[0]).isEqualTo(String.class); + assertThat(argumentTypes[1]).isEqualTo(Integer.class); + assertThat(argumentTypes[2]).isEqualTo(Boolean.class); + assertThat(argumentTypes[3]).isEqualTo(Person.class); + assertThat(argumentTypes[4]).isEqualTo(Gender.class); } @Test public void unconvertedFunctionArgumentTypes() { - Class[] argumentTypes = functionExecutions.captureUnconvertedArgumentTypes("test", 2, - Boolean.FALSE, new Person("Jane", "Doe"), Gender.FEMALE); + Class[] argumentTypes = this.functionExecutions + .captureUnconvertedArgumentTypes("test", 2, Boolean.FALSE, new Person("Jane", "Doe"), + Gender.FEMALE); - assertNotNull(argumentTypes); - assertEquals(5, argumentTypes.length); - assertEquals(String.class, argumentTypes[0]); - assertEquals(Integer.class, argumentTypes[1]); - assertEquals(Boolean.class, argumentTypes[2]); - assertTrue(PdxInstance.class.isAssignableFrom(argumentTypes[3])); - assertEquals(PdxInstanceEnum.class, argumentTypes[4]); + assertThat(argumentTypes).isNotNull(); + assertThat(argumentTypes.length).isEqualTo(5); + assertThat(argumentTypes[0]).isEqualTo(String.class); + assertThat(argumentTypes[1]).isEqualTo(Integer.class); + assertThat(argumentTypes[2]).isEqualTo(Boolean.class); + assertThat(PdxInstance.class).isAssignableFrom(argumentTypes[3]); + assertThat(argumentTypes[4]).isEqualTo(PdxInstanceEnum.class); } @Test public void getAddressFieldValue() { - assertEquals("Portland", functionExecutions.getAddressField(new Address( - "100 Main St.", "Portland", "OR", "97205"), "city")); + + Address address = new Address("100 Main St.", "Portland", "OR", "97205"); + + assertThat(this.functionExecutions.getAddressField(address, "city")).isEqualTo("Portland"); } @Test public void pdxDataFieldValue() { - Map pdxData = new HashMap(3); + Map pdxData = new HashMap<>(3); pdxData.put("@type", "x.y.z.domain.MyApplicationDomainType"); pdxData.put("booleanField", Boolean.TRUE); pdxData.put("integerField", 123); pdxData.put("stringField", "test"); - Integer value = (Integer) functionExecutions.getDataField(toPdxInstance(pdxData), "integerField"); + Integer value = this.functionExecutions.getDataField(toPdxInstance(pdxData), "integerField"); - assertEquals(pdxData.get("integerField"), value); + assertThat(value).isEqualTo(pdxData.get("integerField")); } public static class ApplicationDomainFunctions { - private Class[] getArgumentTypes(final Object... arguments) { + private Class[] getArgumentTypes(Object... arguments) { - Class[] argumentTypes = new Class[arguments.length]; - - int index = 0; + List> argumentTypes = new ArrayList<>(); for (Object argument : arguments) { - argumentTypes[index] = arguments[index].getClass(); - index++; + argumentTypes.add(argument.getClass()); } - return argumentTypes; + return argumentTypes.toArray(new Class[0]); } @GemfireFunction - public Class[] captureConvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, + public Class[] captureConvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, Person person, Gender gender) { return getArgumentTypes(stringValue, integerValue, booleanValue, person, gender); } @GemfireFunction - public Class[] captureUnconvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, + public Class[] captureUnconvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, Object domainObject, Object enumValue) { return getArgumentTypes(stringValue, integerValue, booleanValue, domainObject, enumValue); @@ -199,7 +200,10 @@ public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientSe @GemfireFunction public String getAddressField(PdxInstance address, String fieldName) { - Assert.isTrue(Address.class.getName().equals(address.getClassName()), "Address is not the correct type"); + + Assert.isTrue(Address.class.getName().equals(address.getClassName()), + "Address is not the correct type"); + return String.valueOf(address.getField(fieldName)); } @@ -213,14 +217,15 @@ public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientSe private final String street; private final String city; - private final String state; // refactor; use Enum! + private final String state; // Refactor; use Enum! private final String zipCode; public Address(String street, String city, String state, String zipCode) { - Assert.hasText("The Address 'street' must be specified", street); - Assert.hasText("The Address 'city' must be specified", city); - Assert.hasText("The Address 'state' must be specified", state); - Assert.hasText("The Address 'zipCode' must be specified", zipCode); + + Assert.hasText("Street is required", street); + Assert.hasText("City is required", city); + Assert.hasText("State is required", state); + Assert.hasText("ZipCode is required", zipCode); this.street = street; this.city = city; @@ -293,8 +298,9 @@ public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientSe private final String lastName; public Person(String firstName, String lastName) { - Assert.hasText(firstName, "The person's first name must be specified!"); - Assert.hasText(lastName, "The person's last name must be specified!"); + + Assert.hasText(firstName, "First name is required"); + Assert.hasText(lastName, "Last name is required"); this.firstName = firstName; this.lastName = lastName; @@ -352,10 +358,11 @@ public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientSe public static PdxSerializer compose(PdxSerializer... pdxSerializers) { - return pdxSerializers == null ? null - : (pdxSerializers.length == 1 + return pdxSerializers == null + ? null + : pdxSerializers.length == 1 ? pdxSerializers[0] - : new ComposablePdxSerializer(pdxSerializers)); + : new ComposablePdxSerializer(pdxSerializers); } @Override @@ -392,23 +399,23 @@ public class ClientCacheFunctionExecutionWithPdxIntegrationTest extends ClientSe private PdxSerializer pdxSerializer; - public void setPdxSerializers(final List pdxSerializers) { + public void setPdxSerializers(List pdxSerializers) { this.pdxSerializers = pdxSerializers; } @Override - public void afterPropertiesSet() throws Exception { - pdxSerializer = ComposablePdxSerializer.compose(pdxSerializers.toArray(new PdxSerializer[0])); + public void afterPropertiesSet() { + this.pdxSerializer = ComposablePdxSerializer.compose(this.pdxSerializers.toArray(new PdxSerializer[0])); } @Override - public PdxSerializer getObject() throws Exception { - return pdxSerializer; + public PdxSerializer getObject() { + return this.pdxSerializer; } @Override public Class getObjectType() { - return pdxSerializer != null ? pdxSerializer.getClass() : PdxSerializer.class; + return this.pdxSerializer != null ? this.pdxSerializer.getClass() : PdxSerializer.class; } @Override diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest.java index 60a461cf..3c1c527a 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest.java @@ -13,11 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.function; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.isA; +import static org.assertj.core.api.Assertions.assertThat; import java.io.File; import java.io.IOException; @@ -25,17 +23,15 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; -import org.apache.geode.cache.execute.FunctionAdapter; -import org.apache.geode.cache.execute.FunctionContext; -import org.apache.geode.cache.execute.FunctionException; - import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; +import org.apache.geode.cache.execute.Function; +import org.apache.geode.cache.execute.FunctionContext; +import org.apache.geode.cache.execute.FunctionException; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.gemfire.fork.ServerProcess; import org.springframework.data.gemfire.function.sample.ExceptionThrowingFunctionExecution; @@ -48,21 +44,21 @@ import org.springframework.test.context.junit4.SpringRunner; import org.springframework.util.Assert; /** - * The ExceptionThrowingFunctionExecutionIntegrationTest class is a test suite of test cases testing the invocation - * of a GemFire Function using Spring Data GemFire Function Execution Annotation support when that Function throws - * an Exception. + * Integration Tests testing the proper behavior of SDG's {@link Function} annotation support when the {@link Function} + * throws a {@link FunctionException}. * * @author John Blum - * @see org.junit.Rule * @see org.junit.Test - * @see org.junit.runner.RunWith + * @see org.apache.geode.cache.execute.Function + * @see org.apache.geode.cache.execute.FunctionContext + * @see org.apache.geode.cache.execute.FunctionException * @see org.springframework.data.gemfire.fork.ServerProcess * @see org.springframework.data.gemfire.function.annotation.GemfireFunction * @see org.springframework.data.gemfire.function.sample.ExceptionThrowingFunctionExecution * @see org.springframework.data.gemfire.process.ProcessExecutor * @see org.springframework.data.gemfire.process.ProcessWrapper * @see org.springframework.test.context.ContextConfiguration - * @see org.springframework.test.context.junit4.SpringJUnit4ClassRunner + * @see org.springframework.test.context.junit4.SpringRunner * @since 1.7.0 */ @RunWith(SpringRunner.class) @@ -72,9 +68,6 @@ public class ExceptionThrowingFunctionExecutionIntegrationTest { private static ProcessWrapper gemfireServer; - @Rule - public ExpectedException exception = ExpectedException.none(); - @Autowired private ExceptionThrowingFunctionExecution exceptionThrowingFunctionExecution; @@ -88,7 +81,7 @@ public class ExceptionThrowingFunctionExecutionIntegrationTest { Assert.isTrue(serverWorkingDirectory.isDirectory() || serverWorkingDirectory.mkdirs(), String.format("Failed to create working directory [%s]", serverWorkingDirectory)); - List arguments = new ArrayList(); + List arguments = new ArrayList<>(); arguments.add("-Dgemfire.name=" + serverName); arguments.add("-Dgemfire.log-level=error"); @@ -96,7 +89,7 @@ public class ExceptionThrowingFunctionExecutionIntegrationTest { .concat("-server-context.xml")); gemfireServer = ProcessExecutor.launch(serverWorkingDirectory, ServerProcess.class, - arguments.toArray(new String[arguments.size()])); + arguments.toArray(new String[0])); waitForServerStart(TimeUnit.SECONDS.toMillis(20)); } @@ -120,29 +113,37 @@ public class ExceptionThrowingFunctionExecutionIntegrationTest { gemfireServer.shutdown(); - if (Boolean.valueOf(System.getProperty("spring.gemfire.fork.clean", Boolean.TRUE.toString()))) { + if (Boolean.parseBoolean(System.getProperty("spring.gemfire.fork.clean", Boolean.TRUE.toString()))) { org.springframework.util.FileSystemUtils.deleteRecursively(gemfireServer.getWorkingDirectory()); } } - @Test + @Test(expected = FunctionException.class) public void exceptionThrowingFunctionExecutionRethrowsException() { - exception.expect(FunctionException.class); - exception.expectCause(isA(IllegalArgumentException.class)); - exception.expectMessage(containsString("Execution of Function with ID [exceptionThrowingFunction] failed")); + try { + this.exceptionThrowingFunctionExecution.exceptionThrowingFunction(); + } + catch (FunctionException expected) { - exceptionThrowingFunctionExecution.exceptionThrowingFunction(); + assertThat(expected).hasMessage("Execution of Function [with ID [exceptionThrowingFunction]] failed"); + assertThat(expected).hasCauseInstanceOf(IllegalArgumentException.class); + assertThat(expected.getCause()).hasMessage("TEST"); + assertThat(expected.getCause()).hasNoCause(); + + throw expected; + } } - public static class ExceptionThrowingFunction extends FunctionAdapter { + public static class ExceptionThrowingFunction implements Function { @Override public String getId() { return "exceptionThrowingFunction"; } + @Override - public void execute(final FunctionContext context) { + public void execute(FunctionContext context) { context.getResultSender().sendException(new IllegalArgumentException("TEST")); } } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecutionTest.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecutionTest.java index 2b73757f..d5ffce8e 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecutionTest.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionExecutionTest.java @@ -13,14 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.function.execution; import static org.assertj.core.api.Assertions.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.isA; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doCallRealMethod; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -33,18 +33,16 @@ import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + import org.apache.geode.cache.execute.Execution; import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionException; import org.apache.geode.cache.execute.ResultCollector; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; - /** * The AbstractFunctionExecutionTest class is a test suite of test cases testing the contract and functionality * of the AbstractFunctionExecution class. @@ -59,11 +57,9 @@ import org.mockito.junit.MockitoJUnitRunner; * @since 1.7.0 */ @RunWith(MockitoJUnitRunner.class) +@SuppressWarnings("rawtypes") public class AbstractFunctionExecutionTest { - @Rule - public ExpectedException exception = ExpectedException.none(); - @Mock private Execution mockExecution; @@ -91,7 +87,7 @@ public class AbstractFunctionExecutionTest { }; Iterable actualResults = functionExecution.setFunction(mockFunction) - .setArgs(args).setTimeout(500).execute(); + .setArguments(args).setTimeout(500).execute(); assertThat(actualResults).isNotNull(); assertThat(actualResults).isEqualTo(results); @@ -157,7 +153,6 @@ public class AbstractFunctionExecutionTest { return mockExecution; } - @SuppressWarnings("unchecked") @Override Iterable execute() { return null; } @@ -176,8 +171,8 @@ public class AbstractFunctionExecutionTest { return mockExecution; } - @SuppressWarnings("unchecked") - @Override Iterable execute() { + @Override + Iterable execute() { return Collections.emptyList(); } }; @@ -185,26 +180,28 @@ public class AbstractFunctionExecutionTest { assertThat((Object) functionExecution.executeAndExtract()).isNull(); } - @Test + @Test(expected = FunctionException.class) public void executeAndExtractWithThrowsException() { - AbstractFunctionExecution functionExecution = new AbstractFunctionExecution() { + AbstractFunctionExecution functionExecution = mock(AbstractFunctionExecution.class); - @Override - protected Execution getExecution() { - return mockExecution; - } + doReturn(Collections.singleton(new IllegalArgumentException("test"))).when(functionExecution).execute(); + doCallRealMethod().when(functionExecution).setFunctionId(anyString()); + doCallRealMethod().when(functionExecution).getFunctionId(); + doCallRealMethod().when(functionExecution).executeAndExtract(); - @SuppressWarnings("unchecked") - @Override Iterable execute() { - return Collections.singletonList((T) new IllegalArgumentException("test")); - } - }; + try { + functionExecution.setFunctionId("TestFunction").executeAndExtract(); + } + catch (Exception expected) { - exception.expect(FunctionException.class); - exception.expectCause(isA(IllegalArgumentException.class)); - exception.expectMessage(containsString("Execution of Function with ID [TestFunction] failed")); + assertThat(expected).isInstanceOf(FunctionException.class); + assertThat(expected).hasMessage("Execution of Function [with ID [TestFunction]] failed"); + assertThat(expected).hasCauseInstanceOf(IllegalArgumentException.class); + assertThat(expected.getCause()).hasMessage("test"); + assertThat(expected.getCause()).hasNoCause(); - functionExecution.setFunctionId("TestFunction").executeAndExtract(); + throw expected; + } } } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplateTest.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplateTest.java index 98a99392..b8001639 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplateTest.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/AbstractFunctionTemplateTest.java @@ -13,14 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.function.execution; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertThat; -import static org.mockito.Matchers.eq; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -28,28 +27,29 @@ import static org.mockito.Mockito.when; import java.util.Arrays; import java.util.List; -import org.apache.geode.cache.execute.Function; -import org.apache.geode.cache.execute.ResultCollector; - import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import org.apache.geode.cache.execute.Function; +import org.apache.geode.cache.execute.ResultCollector; + /** - * The AbstractFunctionTemplateTest class is a test suite of test cases testing the contract and functionality - * of the AbstractFunctionTemplate class. + * Unit Tests for {@link AbstractFunctionTemplate}. * * @author John Blum * @see org.junit.Test + * @see org.mockito.Mock * @see org.mockito.Mockito - * @see org.springframework.data.gemfire.function.execution.AbstractFunctionExecution - * @see org.springframework.data.gemfire.function.execution.AbstractFunctionTemplate * @see org.apache.geode.cache.execute.Function * @see org.apache.geode.cache.execute.ResultCollector + * @see org.springframework.data.gemfire.function.execution.AbstractFunctionExecution + * @see org.springframework.data.gemfire.function.execution.AbstractFunctionTemplate * @since 1.7.0 */ @RunWith(MockitoJUnitRunner.class) +@SuppressWarnings("rawtypes") public class AbstractFunctionTemplateTest { @Mock @@ -63,17 +63,20 @@ public class AbstractFunctionTemplateTest { @Test public void executeWithFunctionAndArgs() { + Object[] args = { "test", "testing", "tested" }; List results = Arrays.asList(args); - when(mockFunctionExecution.setArgs(args)).thenReturn(mockFunctionExecution); + when(mockFunctionExecution.setArguments(args)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setFunction(mockFunction)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setResultCollector(mockResultCollector)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setTimeout(500)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.execute()).thenReturn(results); AbstractFunctionTemplate functionTemplate = new AbstractFunctionTemplate() { - @Override protected AbstractFunctionExecution getFunctionExecution() { + + @Override + protected AbstractFunctionExecution getFunctionExecution() { return mockFunctionExecution; } }; @@ -85,9 +88,9 @@ public class AbstractFunctionTemplateTest { assertThat(functionTemplate.getResultCollector(), is(equalTo(mockResultCollector))); assertThat(actualResults, is(notNullValue())); - assertThat(actualResults, is(equalTo((Iterable) results))); + assertThat(actualResults, is(equalTo((results)))); - verify(mockFunctionExecution, times(1)).setArgs(args); + verify(mockFunctionExecution, times(1)).setArguments(args); verify(mockFunctionExecution, times(1)).setFunction(mockFunction); verify(mockFunctionExecution, times(1)).setResultCollector(eq(mockResultCollector)); verify(mockFunctionExecution, times(1)).setTimeout(500); @@ -96,16 +99,19 @@ public class AbstractFunctionTemplateTest { @Test public void executeAndExtractWithFunctionAndArgs() { + Object[] args = { "test", "testing", "tested" }; - when(mockFunctionExecution.setArgs(args)).thenReturn(mockFunctionExecution); + when(mockFunctionExecution.setArguments(args)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setFunction(mockFunction)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setResultCollector(mockResultCollector)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setTimeout(500)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.executeAndExtract()).thenReturn(args[0]); AbstractFunctionTemplate functionTemplate = new AbstractFunctionTemplate() { - @Override protected AbstractFunctionExecution getFunctionExecution() { + + @Override + protected AbstractFunctionExecution getFunctionExecution() { return mockFunctionExecution; } }; @@ -119,7 +125,7 @@ public class AbstractFunctionTemplateTest { assertThat(result, is(notNullValue())); assertThat(result, is(equalTo("test"))); - verify(mockFunctionExecution, times(1)).setArgs(args); + verify(mockFunctionExecution, times(1)).setArguments(args); verify(mockFunctionExecution, times(1)).setFunction(mockFunction); verify(mockFunctionExecution, times(1)).setResultCollector(eq(mockResultCollector)); verify(mockFunctionExecution, times(1)).setTimeout(500); @@ -128,17 +134,20 @@ public class AbstractFunctionTemplateTest { @Test public void executeWithFunctionIdAndArgs() { + Object[] args = { "test", "testing", "tested" }; List results = Arrays.asList(args); - when(mockFunctionExecution.setArgs(args)).thenReturn(mockFunctionExecution); + when(mockFunctionExecution.setArguments(args)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setFunctionId("TestFunction")).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setResultCollector(mockResultCollector)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setTimeout(500)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.execute()).thenReturn(results); AbstractFunctionTemplate functionTemplate = new AbstractFunctionTemplate() { - @Override protected AbstractFunctionExecution getFunctionExecution() { + + @Override + protected AbstractFunctionExecution getFunctionExecution() { return mockFunctionExecution; } }; @@ -150,9 +159,9 @@ public class AbstractFunctionTemplateTest { assertThat(functionTemplate.getResultCollector(), is(equalTo(mockResultCollector))); assertThat(actualResults, is(notNullValue())); - assertThat(actualResults, is(equalTo((Iterable) results))); + assertThat(actualResults, is(equalTo(results))); - verify(mockFunctionExecution, times(1)).setArgs(args); + verify(mockFunctionExecution, times(1)).setArguments(args); verify(mockFunctionExecution, times(1)).setFunctionId("TestFunction"); verify(mockFunctionExecution, times(1)).setResultCollector(eq(mockResultCollector)); verify(mockFunctionExecution, times(1)).setTimeout(500); @@ -161,16 +170,19 @@ public class AbstractFunctionTemplateTest { @Test public void executeAndExtractWithFunctionIdAndArgs() { + Object[] args = { "test", "testing", "tested" }; - when(mockFunctionExecution.setArgs(args)).thenReturn(mockFunctionExecution); + when(mockFunctionExecution.setArguments(args)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setFunctionId("TestFunction")).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setResultCollector(mockResultCollector)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setTimeout(500)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.executeAndExtract()).thenReturn(args[0]); AbstractFunctionTemplate functionTemplate = new AbstractFunctionTemplate() { - @Override protected AbstractFunctionExecution getFunctionExecution() { + + @Override + protected AbstractFunctionExecution getFunctionExecution() { return mockFunctionExecution; } }; @@ -184,7 +196,7 @@ public class AbstractFunctionTemplateTest { assertThat(result, is(notNullValue())); assertThat(result, is(equalTo("test"))); - verify(mockFunctionExecution, times(1)).setArgs(args); + verify(mockFunctionExecution, times(1)).setArguments(args); verify(mockFunctionExecution, times(1)).setFunctionId("TestFunction"); verify(mockFunctionExecution, times(1)).setResultCollector(eq(mockResultCollector)); verify(mockFunctionExecution, times(1)).setTimeout(500); @@ -193,16 +205,19 @@ public class AbstractFunctionTemplateTest { @Test public void executeWithNoResultWithFunctionIdAndArgs() { + Object[] args = { "test", "testing", "tested" }; - when(mockFunctionExecution.setArgs(args)).thenReturn(mockFunctionExecution); + when(mockFunctionExecution.setArguments(args)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setFunctionId("TestFunction")).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setResultCollector(mockResultCollector)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.setTimeout(500)).thenReturn(mockFunctionExecution); when(mockFunctionExecution.execute(eq(false))).thenReturn(null); AbstractFunctionTemplate functionTemplate = new AbstractFunctionTemplate() { - @Override protected AbstractFunctionExecution getFunctionExecution() { + + @Override + protected AbstractFunctionExecution getFunctionExecution() { return mockFunctionExecution; } }; @@ -214,11 +229,10 @@ public class AbstractFunctionTemplateTest { assertThat(functionTemplate.getResultCollector(), is(equalTo(mockResultCollector))); - verify(mockFunctionExecution, times(1)).setArgs(args); + verify(mockFunctionExecution, times(1)).setArguments(args); verify(mockFunctionExecution, times(1)).setFunctionId("TestFunction"); verify(mockFunctionExecution, times(1)).setResultCollector(eq(mockResultCollector)); verify(mockFunctionExecution, times(1)).setTimeout(500); verify(mockFunctionExecution, times(1)).execute(eq(false)); } - } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionExecutionIntegrationTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionExecutionIntegrationTests.java index a335e38f..b2334334 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionExecutionIntegrationTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionExecutionIntegrationTests.java @@ -116,7 +116,10 @@ public class FunctionExecutionIntegrationTests extends ClientServerIntegrationTe private void verifyFunctionExecution(AbstractFunctionExecution functionExecution) { - Iterable results = functionExecution.setArgs("1", "2", "3").setFunctionId("echoFunction").execute(); + Iterable results = functionExecution + .setArguments("1", "2", "3") + .setFunctionId("echoFunction") + .execute(); int count = 1; diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests.java index 49e9611f..43dec479 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests.java @@ -10,7 +10,6 @@ * 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.execution; import static org.junit.Assert.assertEquals; @@ -25,14 +24,15 @@ import java.util.Map; import javax.annotation.Resource; -import org.apache.geode.cache.Region; - import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.apache.geode.cache.Region; +import org.apache.geode.cache.execute.Function; + import org.springframework.data.gemfire.fork.ServerProcess; import org.springframework.data.gemfire.function.annotation.GemfireFunction; import org.springframework.data.gemfire.function.annotation.RegionData; @@ -53,11 +53,12 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor private static ProcessWrapper gemfireServer; - @Resource(name = "test-region") + @Resource(name = "TestRegion") private Region region; @BeforeClass public static void startGemFireServer() throws Exception { + int availablePort = findAvailablePort(); gemfireServer = run(ServerProcess.class, @@ -77,22 +78,23 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor @Before public void initializeRegion() { - region.put("one", 1); - region.put("two", 2); - region.put("three", 3); + + this.region.put("one", 1); + this.region.put("two", 2); + this.region.put("three", 3); } @Test - //@Ignore - public void testVoidReturnType() { - GemfireOnRegionOperations template = new GemfireOnRegionFunctionTemplate(region); + public void withVoidReturnType() { + + GemfireOnRegionOperations template = new GemfireOnRegionFunctionTemplate(this.region); + // Should work either way but the first invocation traps an exception if there is a result. - template.execute("noResult"); template.executeWithNoResult("noResult"); + template.execute("noResult"); } @Test - //@Ignore @SuppressWarnings("unchecked") public void testCollectionReturnTypes() { GemfireOnRegionOperations template = new GemfireOnRegionFunctionTemplate(region); @@ -126,7 +128,8 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor @Test @SuppressWarnings("all") public void testArrayReturnTypes() { - Object result = new GemfireOnRegionFunctionTemplate(region) + + Object result = new GemfireOnRegionFunctionTemplate(this.region) .executeAndExtract("arrays", new int[] { 1, 2, 3, 4, 5 }); assertTrue(result.getClass().getName(), result instanceof int[]); @@ -136,7 +139,8 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor @Test //@Ignore public void testOnRegionFunctionExecution() { - GemfireOnRegionOperations template = new GemfireOnRegionFunctionTemplate(region); + + GemfireOnRegionOperations template = new GemfireOnRegionFunctionTemplate(this.region); assertEquals(2, template.execute("oneArg", "two").iterator().next().intValue()); assertFalse(template.execute("oneArg", Collections.singleton("one"), "two").iterator().hasNext()); @@ -144,8 +148,8 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor assertEquals(5, template.executeAndExtract("twoArg", "two", "three").intValue()); } - /* - * This gets wrapped in a GemFire Function and registered on the forked server. + /** + * This {@link Component} class gets wrapped in an Apache Geode {@link Function} and registered on the forked server. */ @Component @SuppressWarnings("unused") @@ -158,6 +162,7 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor @GemfireFunction(id = "twoArg") public Integer twoArg(String keyOne, String keyTwo, @RegionData Map region) { + if (region.get(keyOne) != null && region.get(keyTwo) != null) { return region.get(keyOne) + region.get(keyTwo); } @@ -172,11 +177,12 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor @GemfireFunction(id = "getMapWithNoArgs") public Map getMapWithNoArgs(@RegionData Map region) { + if (region.size() == 0) { return null; } - return new HashMap(region); + return new HashMap<>(region); } @GemfireFunction(id = "arrays") @@ -187,8 +193,7 @@ public class FunctionIntegrationTests extends ClientServerIntegrationTestsSuppor } @GemfireFunction - public void noResult() { - } - } + public void noResult() { } + } } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBeanUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBeanUnitTests.java index df162b0d..91b76989 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBeanUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBeanUnitTests.java @@ -21,20 +21,24 @@ import static org.mockito.Mockito.when; import java.lang.reflect.AccessibleObject; import java.lang.reflect.Method; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; -import org.aopalliance.intercept.MethodInvocation; import org.junit.Before; import org.junit.Test; import org.springframework.data.gemfire.function.annotation.FunctionId; +import org.aopalliance.intercept.MethodInvocation; + /** - * Unit tests for {@link GemfireFunctionProxyFactoryBean}. + * Unit Tests for {@link GemfireFunctionProxyFactoryBean}. * * @author David Turanski * @author John Blum + * @see java.lang.reflect.AccessibleObject + * @see java.lang.reflect.Method * @see org.junit.Test * @see org.mockito.Mockito * @see org.aopalliance.intercept.MethodInvocation @@ -50,34 +54,37 @@ public class GemfireFunctionProxyFactoryBeanUnitTests { } @Test - public void invoke() throws Throwable { + public void invoke() { MethodInvocation invocation = new TestMethodInvocation(IFoo.class) - .withMethodNameAndArgTypes("collections",List.class); + .withMethodNameAndArgTypes("collections", List.class); - when(this.functionOperations.executeAndExtract("collections",invocation.getArguments())) + when(this.functionOperations.execute("collections", invocation.getArguments())) .thenReturn(Arrays.asList(1, 2, 3)); - GemfireFunctionProxyFactoryBean proxy = new GemfireFunctionProxyFactoryBean(IFoo.class, this.functionOperations); + GemfireFunctionProxyFactoryBean proxy = + new GemfireFunctionProxyFactoryBean(IFoo.class, this.functionOperations); Object result = proxy.invoke(invocation); assertThat(result).isInstanceOf(List.class); - assertThat((List) result).hasSize(3); + assertThat((List) result).hasSize(3); verify(this.functionOperations, times(1)) - .executeAndExtract("collections",invocation.getArguments()); + .execute("collections", invocation.getArguments()); } @Test - public void invokeAndExtractWithAnnotatedFunctionId() throws Throwable { + public void invokeAndExtractWithAnnotatedFunctionId() { MethodInvocation invocation = new TestMethodInvocation(IFoo.class) .withMethodNameAndArgTypes("oneArg", String.class); - when(this.functionOperations.executeAndExtract("oneArg",invocation.getArguments())).thenReturn(1); + when(this.functionOperations.execute("oneArg", invocation.getArguments())) + .thenReturn(Collections.singleton(1)); - GemfireFunctionProxyFactoryBean proxy = new GemfireFunctionProxyFactoryBean(IFoo.class, this.functionOperations); + GemfireFunctionProxyFactoryBean proxy = + new GemfireFunctionProxyFactoryBean(IFoo.class, this.functionOperations); Object result = proxy.invoke(invocation); @@ -85,7 +92,7 @@ public class GemfireFunctionProxyFactoryBeanUnitTests { assertThat(result).isEqualTo(1); verify(this.functionOperations, times(1)) - .executeAndExtract("oneArg", invocation.getArguments()); + .execute("oneArg", invocation.getArguments()); } @SuppressWarnings("unused") @@ -93,7 +100,7 @@ public class GemfireFunctionProxyFactoryBeanUnitTests { private Class type; - private Class[] argTypes; + private Class[] argumentTypes; private Object[] arguments; @@ -110,54 +117,39 @@ public class GemfireFunctionProxyFactoryBeanUnitTests { return this; } - public TestMethodInvocation withMethodNameAndArgTypes(String methodName,Class... argTypes) { + public TestMethodInvocation withMethodNameAndArgTypes(String methodName, Class... argTypes) { this.methodName = methodName; - this.argTypes = argTypes; + this.argumentTypes = argTypes; return this; } - /* (non-Javadoc) - * @see org.aopalliance.intercept.Invocation#getArguments() - */ @Override public Object[] getArguments() { return this.arguments; } - /* (non-Javadoc) - * @see org.aopalliance.intercept.Joinpoint#proceed() - */ @Override - public Object proceed() throws Throwable { + public Object proceed() { return null; } - /* (non-Javadoc) - * @see org.aopalliance.intercept.Joinpoint#getThis() - */ @Override public Object getThis() { return null; } - /* (non-Javadoc) - * @see org.aopalliance.intercept.Joinpoint#getStaticPart() - */ @Override public AccessibleObject getStaticPart() { return null; } - /* (non-Javadoc) - * @see org.aopalliance.intercept.MethodInvocation#getMethod() - */ @Override public Method getMethod() { try { - return this.type.getMethod(methodName, argTypes); + return this.type.getMethod(this.methodName, this.argumentTypes); } catch (NoSuchMethodException | SecurityException cause) { return null; diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/onservers/FunctionsReturnResultsFromAllServersIntegrationTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/onservers/FunctionsReturnResultsFromAllServersIntegrationTests.java index 18576ea2..24d51787 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/onservers/FunctionsReturnResultsFromAllServersIntegrationTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/execution/onservers/FunctionsReturnResultsFromAllServersIntegrationTests.java @@ -16,6 +16,18 @@ */ package org.springframework.data.gemfire.function.execution.onservers; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; + import org.apache.geode.StatisticDescriptor; import org.apache.geode.Statistics; import org.apache.geode.StatisticsType; @@ -27,32 +39,22 @@ import org.apache.geode.cache.execute.FunctionService; import org.apache.geode.cache.server.CacheServer; import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.internal.statistics.StatisticsManager; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.runner.RunWith; + import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Configuration; import org.springframework.data.gemfire.config.annotation.ClientCacheApplication; import org.springframework.data.gemfire.function.config.EnableGemfireFunctionExecutions; 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.util.ArrayList; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; - /** * @author Patrick Johnson */ @SuppressWarnings("unused") @RunWith(SpringRunner.class) -@ContextConfiguration(classes = FunctionsReturnResultsFromAllServersIntegrationTests.Config.class) +@ContextConfiguration(classes = FunctionsReturnResultsFromAllServersIntegrationTests.GeodeClientConfiguration.class) +@Ignore public class FunctionsReturnResultsFromAllServersIntegrationTests extends ClientServerIntegrationTestsSupport { private static final int PORT_1 = 40407; @@ -81,12 +83,6 @@ public class FunctionsReturnResultsFromAllServersIntegrationTests extends Client waitForServerToStart(DEFAULT_HOSTNAME, PORT_2); } - @ClientCacheApplication(servers = {@ClientCacheApplication.Server(port = PORT_1), @ClientCacheApplication.Server(port = PORT_2)}) - @Configuration - @EnableGemfireFunctionExecutions(basePackageClasses = AllServersAdminFunctions.class) - @EnableGemfireCacheTransactions - static class Config { } - @AfterClass public static void stopGemFireServer() { stop(gemfireServer1); @@ -105,12 +101,18 @@ public class FunctionsReturnResultsFromAllServersIntegrationTests extends Client assertThat(metrics.size()).isEqualTo(672); } + @ClientCacheApplication(servers = { + @ClientCacheApplication.Server(port = PORT_1), + @ClientCacheApplication.Server(port = PORT_2)} + ) + @EnableGemfireFunctionExecutions(basePackageClasses = AllServersAdminFunctions.class) + static class GeodeClientConfiguration { } + static class MetricsFunctionServerProcess { private static final int DEFAULT_CACHE_SERVER_PORT = 40404; private static final String CACHE_SERVER_PORT_PROPERTY = "spring.data.gemfire.cache.server.port"; - private static final String GEMFIRE_LOG_LEVEL = "error"; private static final String GEMFIRE_NAME = "MetricsServer" + getCacheServerPort(); public static void main(String[] args) throws Exception { @@ -121,7 +123,6 @@ public class FunctionsReturnResultsFromAllServersIntegrationTests extends Client return new CacheFactory() .set("name", GEMFIRE_NAME) - .set("log-level", GEMFIRE_LOG_LEVEL) .create(); } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ApplicationDomainFunctionExecutions.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ApplicationDomainFunctionExecutions.java index 61354880..ff18afa2 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ApplicationDomainFunctionExecutions.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ApplicationDomainFunctionExecutions.java @@ -35,15 +35,15 @@ import org.springframework.data.gemfire.function.annotation.OnServer; @SuppressWarnings("unused") public interface ApplicationDomainFunctionExecutions { - Class[] captureConvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, + Class[] captureConvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, ClientCacheFunctionExecutionWithPdxIntegrationTest.Person person, ClientCacheFunctionExecutionWithPdxIntegrationTest.Gender gender); - Class[] captureUnconvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, + Class[] captureUnconvertedArgumentTypes(String stringValue, Integer integerValue, Boolean booleanValue, Object person, Object gender); String getAddressField(ClientCacheFunctionExecutionWithPdxIntegrationTest.Address address, String fieldName); - Object getDataField(PdxInstance data, String fieldName); + Integer getDataField(PdxInstance data, String fieldName); } diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ExceptionThrowingFunctionExecution.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ExceptionThrowingFunctionExecution.java index 331b0948..603ba2c7 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ExceptionThrowingFunctionExecution.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/function/sample/ExceptionThrowingFunctionExecution.java @@ -13,15 +13,18 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.gemfire.function.sample; +import org.apache.geode.cache.execute.Function; + import org.springframework.data.gemfire.function.annotation.OnServer; /** - * The ExceptionThrowingFunctionExecution interface defines a GemFire Function that throws a RuntimeException. + * The {@link ExceptionThrowingFunctionExecution} interface defines a GemFire {@link Function} + * that throws a {@link RuntimeException}. * * @author John Blum + * @see org.apache.geode.cache.execute.Function * @see org.springframework.data.gemfire.function.annotation.OnServer * @since 1.7.0 */ diff --git a/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java b/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java index ff2bc949..ea77bf1e 100644 --- a/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java +++ b/spring-data-geode/src/test/java/org/springframework/data/gemfire/util/SpringUtilsUnitTests.java @@ -50,11 +50,14 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.dao.InvalidDataAccessApiUsageException; +import org.springframework.data.gemfire.util.SpringUtils.ValueReturningThrowableOperation; /** * Unit Tests for {@link SpringUtils}. * * @author John Blum + * @see java.util.function.Function + * @see java.util.function.Supplier * @see org.junit.Test * @see org.junit.runner.RunWith * @see org.mockito.Mock @@ -415,16 +418,19 @@ public class SpringUtilsUnitTests { @Test public void safeGetValueReturnsSuppliedDefaultValue() { - Supplier exceptionThrowingSupplier = () -> { throw newRuntimeException("error"); }; + ValueReturningThrowableOperation exceptionThrowingOperation = + () -> { throw newRuntimeException("error"); }; + Supplier defaultValueSupplier = () -> "test"; - assertThat(SpringUtils.safeGetValue(exceptionThrowingSupplier, defaultValueSupplier)).isEqualTo("test"); + assertThat(SpringUtils.safeGetValue(exceptionThrowingOperation, defaultValueSupplier)).isEqualTo("test"); } @Test public void safeGetValueHandlesExceptionReturnsValue() { - Supplier exceptionThrowingSupplier = () -> { throw newRuntimeException("error"); }; + ValueReturningThrowableOperation exceptionThrowingOperation = + () -> { throw newRuntimeException("error"); }; Function exceptionHandler = exception -> { @@ -435,13 +441,14 @@ public class SpringUtilsUnitTests { return "test"; }; - assertThat(SpringUtils.safeGetValue(exceptionThrowingSupplier, exceptionHandler)).isEqualTo("test"); + assertThat(SpringUtils.safeGetValue(exceptionThrowingOperation, exceptionHandler)).isEqualTo("test"); } @Test(expected = IllegalStateException.class) public void safeGetValueHandlesExceptionAndCanThrowException() { - Supplier exceptionThrowingSupplier = () -> { throw newRuntimeException("error"); }; + ValueReturningThrowableOperation exceptionThrowingOperation = + () -> { throw newRuntimeException("error"); }; Function exceptionHandler = exception -> { @@ -453,7 +460,7 @@ public class SpringUtilsUnitTests { }; try { - SpringUtils.safeGetValue(exceptionThrowingSupplier, exceptionHandler); + SpringUtils.safeGetValue(exceptionThrowingOperation, exceptionHandler); } catch (IllegalStateException expected) { diff --git a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-context.xml b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-context.xml index 3df5c87d..64031d9b 100644 --- a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-context.xml +++ b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-context.xml @@ -32,10 +32,10 @@ - - + diff --git a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-server-context.xml b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-server-context.xml index 6dd7cdd8..8cbb5778 100644 --- a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-server-context.xml +++ b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ClientCacheFunctionExecutionWithPdxIntegrationTest-server-context.xml @@ -36,7 +36,7 @@ - + diff --git a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest-server-context.xml b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest-server-context.xml index c7753102..9ee648e2 100644 --- a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest-server-context.xml +++ b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/ExceptionThrowingFunctionExecutionIntegrationTest-server-context.xml @@ -26,7 +26,7 @@ - diff --git a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-context.xml b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-context.xml index 5ba1623d..8ba19f78 100644 --- a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-context.xml +++ b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-context.xml @@ -17,12 +17,12 @@ error - + - + - + diff --git a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-server-context.xml b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-server-context.xml index 2792f6b3..2c731aad 100644 --- a/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-server-context.xml +++ b/spring-data-geode/src/test/resources/org/springframework/data/gemfire/function/execution/FunctionIntegrationTests-server-context.xml @@ -23,7 +23,7 @@ - +