SGF-423 - Handle improper ClassCastException thrown from SDG's Function Execution interface and annotation-based support when a GemFire Function throws an Exception.

Initial refactoring and code cleanup.

src/main/java/org/springframework/data/gemfire/FunctionServiceFactoryBean.java

@@ -27,12 +27,19 @@ import com.gemstone.gemfire.cache.execute.Function;
 import com.gemstone.gemfire.cache.execute.FunctionService;
 
 /**
+ * Spring FactoryBean for registering instance of GemFire Function with the GemFire FunctionService.
+ *
  * @author David Turanski
- * 
+ * @author John Blum
+ * @see org.springframework.beans.factory.FactoryBean
+ * @see org.springframework.beans.factory.InitializingBean
+ * @see com.gemstone.gemfire.cache.execute.Function
+ * @see com.gemstone.gemfire.cache.execute.FunctionService
  */
 public class FunctionServiceFactoryBean implements FactoryBean<FunctionService>, InitializingBean {
-	private static FunctionService functionService;
+
 	private static Log logger = LogFactory.getLog(FunctionServiceFactoryBean.class);
+
 	private List<Function> functions;
 
 	@Override
@@ -40,7 +47,7 @@ public class FunctionServiceFactoryBean implements FactoryBean<FunctionService>,
 		if (!CollectionUtils.isEmpty(functions)) {
 			for (Function function : functions) {
 				if (logger.isInfoEnabled()) {
-					logger.info("registering function with ID: " + function.getId());
+					logger.info(String.format("registering Function with ID (%1$s)", function.getId()));
 				}
 				FunctionService.registerFunction(function);
 			}
@@ -53,7 +60,7 @@ public class FunctionServiceFactoryBean implements FactoryBean<FunctionService>,
 
 	@Override
 	public FunctionService getObject() throws Exception {
-		return functionService;
+		return null;
 	}
 
 	@Override
@@ -65,4 +72,5 @@ public class FunctionServiceFactoryBean implements FactoryBean<FunctionService>,
 	public boolean isSingleton() {
 		return true;
 	}
+
 }

src/main/java/org/springframework/data/gemfire/config/FunctionServiceParser.java

@@ -28,7 +28,9 @@ import org.w3c.dom.Element;
 
 /**
  * Parser for <function-service;gt; definitions.
+ *
  * @author David Turanski
+ * @author John Blum
  */
 class FunctionServiceParser extends AbstractSimpleBeanDefinitionParser {
 
@@ -39,23 +41,23 @@ class FunctionServiceParser extends AbstractSimpleBeanDefinitionParser {
 
 	@Override
 	protected void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder) {
-		builder.setLazyInit(false);
 		super.doParse(element, builder);
-		Element function = DomUtils.getChildElementByTagName(element, "function");
-		if (function != null) {
-			builder.addPropertyValue("functions",
-					ParsingUtils.parseRefOrNestedBeanDeclaration(parserContext, function, builder));
+		builder.setLazyInit(false);
+
+		Element functionElement = DomUtils.getChildElementByTagName(element, "function");
+
+		if (functionElement != null) {
+			builder.addPropertyValue("functions", ParsingUtils.parseRefOrNestedBeanDeclaration(
+				parserContext, functionElement, builder));
 		}
 	}
 
 	@Override
 	protected String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext)
 			throws BeanDefinitionStoreException {
-		String name = super.resolveId(element, definition, parserContext);
-		if (!StringUtils.hasText(name)) {
-			name = GemfireConstants.DEFAULT_GEMFIRE_FUNCTION_SERVICE_NAME;
-		}
-		return name;
+
+		String resolvedId = super.resolveId(element, definition, parserContext);
+		return (StringUtils.hasText(resolvedId) ? resolvedId : GemfireConstants.DEFAULT_GEMFIRE_FUNCTION_SERVICE_NAME);
 	}
 
-}
+}

src/main/java/org/springframework/data/gemfire/function/config/AbstractFunctionExecutionBeanDefinitionBuilder.java

@@ -21,16 +21,18 @@ import org.springframework.beans.factory.support.BeanDefinitionReaderUtils;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
+
 /**
- * Base class for function execution bean definition builders
- * @author David Turanski
+ * Base class for Function Execution bean definition builders.
  *
+ * @author David Turanski
+ * @author John Blum
  */
 abstract class AbstractFunctionExecutionBeanDefinitionBuilder {
-	
-	protected final Log log = LogFactory.getLog(this.getClass());
+
 	protected final FunctionExecutionConfiguration configuration;
-	
+
+	protected final Log log = LogFactory.getLog(getClass());
 
 	/**
 	 * 
@@ -47,32 +49,34 @@ abstract class AbstractFunctionExecutionBeanDefinitionBuilder {
 	 * @return
 	 */
 	 BeanDefinition build(BeanDefinitionRegistry registry) {
-		
-		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(getFunctionProxyFactoryBeanClass());
-	
-		builder.addConstructorArgValue(configuration.getFunctionExecutionInterface());
-		
-		BeanDefinitionBuilder functionTemplateBuilder = getGemfireOperationsBeanDefinitionBuilder(registry);
-		functionTemplateBuilder.setLazyInit(true);
-		
-		String resultCollectorRef = (String)configuration.getAttribute("resultCollector");
-		if (StringUtils.hasText(resultCollectorRef)){
-			functionTemplateBuilder.addPropertyReference("resultCollector",resultCollectorRef);
-		}
-		
-		AbstractBeanDefinition functionTemplate = functionTemplateBuilder
-				.getBeanDefinition();
-		
-		String functionTemplateName = BeanDefinitionReaderUtils.registerWithGeneratedName(functionTemplate, registry);
-		
-		builder.addConstructorArgReference(functionTemplateName);
+		BeanDefinitionBuilder functionProxyFactoryBeanBuilder = BeanDefinitionBuilder.rootBeanDefinition(
+			getFunctionProxyFactoryBeanClass());
+
+		functionProxyFactoryBeanBuilder.addConstructorArgValue(configuration.getFunctionExecutionInterface());
+		functionProxyFactoryBeanBuilder.addConstructorArgReference(BeanDefinitionReaderUtils.registerWithGeneratedName(
+			buildGemfireFunctionOperations(registry), registry));
 		 
-		return builder.getBeanDefinition(); 
+		return functionProxyFactoryBeanBuilder.getBeanDefinition();
 	}	
-	 
-	/*
-	 * Subclasses implement to specify the types to uses. 
-	 */
+
+	protected AbstractBeanDefinition buildGemfireFunctionOperations(BeanDefinitionRegistry registry) {
+		BeanDefinitionBuilder functionTemplateBuilder = getGemfireFunctionOperationsBeanDefinitionBuilder(registry);
+
+		functionTemplateBuilder.setLazyInit(true);
+
+		String resultCollectorReference = (String) configuration.getAttribute("resultCollector");
+
+		if (StringUtils.hasText(resultCollectorReference)){
+			functionTemplateBuilder.addPropertyReference("resultCollector", resultCollectorReference);
+		}
+
+		return functionTemplateBuilder.getBeanDefinition();
+	}
+
+	/* Subclasses implement to specify the types to uses. */
 	protected abstract Class<?> getFunctionProxyFactoryBeanClass();
-	protected abstract BeanDefinitionBuilder getGemfireOperationsBeanDefinitionBuilder(BeanDefinitionRegistry registry);
+
+	protected abstract BeanDefinitionBuilder getGemfireFunctionOperationsBeanDefinitionBuilder(
+		BeanDefinitionRegistry registry);
+
 }

src/main/java/org/springframework/data/gemfire/function/config/AbstractFunctionExecutionConfigurationSource.java

@@ -13,7 +13,9 @@
 package org.springframework.data.gemfire.function.config;
 
 import java.lang.annotation.Annotation;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -37,26 +39,37 @@ import org.springframework.data.gemfire.function.annotation.OnServers;
  *
  */
 abstract class AbstractFunctionExecutionConfigurationSource implements FunctionExecutionConfigurationSource {
-	protected Log logger = LogFactory.getLog(this.getClass());
-		 
-	 
+
 	private static Set<Class<? extends Annotation>> functionExecutionAnnotationTypes;
 	
 	static {
-	  functionExecutionAnnotationTypes = new HashSet<Class<? extends Annotation>>();
-	  functionExecutionAnnotationTypes.add(OnRegion.class);
-	  functionExecutionAnnotationTypes.add(OnServer.class);
-	  functionExecutionAnnotationTypes.add(OnServers.class);
-	  functionExecutionAnnotationTypes.add(OnMember.class);
-	  functionExecutionAnnotationTypes.add(OnMembers.class);
+		Set<Class<? extends Annotation>> annotationTypes = new HashSet<Class<? extends Annotation>>(5);
+
+		annotationTypes.add(OnRegion.class);
+		annotationTypes.add(OnServer.class);
+		annotationTypes.add(OnServers.class);
+		annotationTypes.add(OnMember.class);
+		annotationTypes.add(OnMembers.class);
+
+		functionExecutionAnnotationTypes = Collections.unmodifiableSet(annotationTypes);
 	}
-	
+
+	protected Log logger = LogFactory.getLog(getClass());
 
 	static Set<Class<? extends Annotation>> getFunctionExecutionAnnotationTypes() {
 		return functionExecutionAnnotationTypes;
 	}
-	
-	
+
+	static Set<String> getFunctionExecutionAnnotationTypeNames() {
+		Set<String> functionExecutionTypeNames = new HashSet<String>(getFunctionExecutionAnnotationTypes().size());
+
+		for (Class<? extends Annotation> annotationType : getFunctionExecutionAnnotationTypes()) {
+			functionExecutionTypeNames.add(annotationType.getName());
+		}
+
+		return functionExecutionTypeNames;
+	}
+
 	public Collection<ScannedGenericBeanDefinition> getCandidates(ResourceLoader loader) {
 		ClassPathScanningCandidateComponentProvider scanner = new FunctionExecutionComponentProvider(getIncludeFilters(),functionExecutionAnnotationTypes);
 		scanner.setResourceLoader(loader);

src/main/java/org/springframework/data/gemfire/function/config/FunctionExecutionBeanDefinitionBuilderFactory.java

@@ -19,30 +19,43 @@ import org.springframework.data.gemfire.function.annotation.OnServer;
 import org.springframework.data.gemfire.function.annotation.OnServers;
 
 /**
- * 
- * Maps the annotation type to the corresponding function execution bean definition builder
- * @author David Turanski
+ * Maps the Function Execution Annotation type to the corresponding Function Execution bean definition builder.
  *
+ * @author David Turanski
+ * @author John Blum
+ * @see org.springframework.data.gemfire.function.annotation.OnMember
+ * @see org.springframework.data.gemfire.function.annotation.OnMembers
+ * @see org.springframework.data.gemfire.function.annotation.OnRegion
+ * @see org.springframework.data.gemfire.function.annotation.OnServer
+ * @see org.springframework.data.gemfire.function.annotation.OnServers
+ * @see org.springframework.data.gemfire.function.config.OnMemberExecutionBeanDefinitionBuilder
+ * @see org.springframework.data.gemfire.function.config.OnMembersExecutionBeanDefinitionBuilder
+ * @see org.springframework.data.gemfire.function.config.OnRegionExecutionBeanDefinitionBuilder
+ * @see org.springframework.data.gemfire.function.config.OnServerExecutionBeanDefinitionBuilder
+ * @see org.springframework.data.gemfire.function.config.OnServersExecutionBeanDefinitionBuilder
  */
-abstract class  FunctionExecutionBeanDefinitionBuilderFactory {
+abstract class FunctionExecutionBeanDefinitionBuilderFactory {
 
 	static AbstractFunctionExecutionBeanDefinitionBuilder newInstance(FunctionExecutionConfiguration configuration) {
-		String functionExectionAnnotation = configuration.getAnnotationType();
-		if (functionExectionAnnotation.equals(OnRegion.class.getName())) {
+		String functionExecutionAnnotation = configuration.getAnnotationType();
+
+		if (OnMember.class.getName().equals(functionExecutionAnnotation)) {
+			return new OnMemberExecutionBeanDefinitionBuilder(configuration);
+		}
+		else if (OnMembers.class.getName().equals(functionExecutionAnnotation)) {
+			return new OnMembersExecutionBeanDefinitionBuilder(configuration);
+		}
+		else if (OnRegion.class.getName().equals(functionExecutionAnnotation)) {
 			return new OnRegionExecutionBeanDefinitionBuilder(configuration);
 		} 
-		if (functionExectionAnnotation.equals(OnServer.class.getName())) {
+		else if (OnServer.class.getName().equals(functionExecutionAnnotation)) {
 			return new OnServerExecutionBeanDefinitionBuilder(configuration);
 		} 
-		if (functionExectionAnnotation.equals(OnServers.class.getName())) {
+		else if (OnServers.class.getName().equals(functionExecutionAnnotation)) {
 			return new OnServersExecutionBeanDefinitionBuilder(configuration);
-		} 
-		if (functionExectionAnnotation.equals(OnMember.class.getName())) {
-			return new OnMemberExecutionBeanDefinitionBuilder(configuration);
-		} 
-		if (functionExectionAnnotation.equals(OnMembers.class.getName())) {
-			return new OnMembersExecutionBeanDefinitionBuilder(configuration);
-		} 
-		return null;	
+		}
+
+		return null;
 	}
+
 }

src/main/java/org/springframework/data/gemfire/function/config/FunctionExecutionBeanDefinitionParser.java

@@ -18,8 +18,13 @@ import org.springframework.beans.factory.xml.ParserContext;
 import org.w3c.dom.Element;
 
 /**
- * @author David Turanski
+ * Parse for <function-executions> definitions.
  *
+ * @author David Turanski
+ * @author John Blum
+ * @see org.springframework.beans.factory.config.BeanDefinition
+ * @see org.springframework.beans.factory.xml.BeanDefinitionParser
+ * @see org.springframework.beans.factory.xml.ParserContext
  */
 public class FunctionExecutionBeanDefinitionParser implements BeanDefinitionParser {
 
@@ -28,10 +33,12 @@ public class FunctionExecutionBeanDefinitionParser implements BeanDefinitionPars
 	 */
 	@Override
 	public BeanDefinition parse(Element element, ParserContext parserContext) {
-	
-		AbstractFunctionExecutionConfigurationSource configurationSource = 
-				new XmlFunctionExecutionConfigurationSource(element, parserContext);
+
+		AbstractFunctionExecutionConfigurationSource configurationSource = new XmlFunctionExecutionConfigurationSource(
+			element, parserContext);
+
 		new FunctionExecutionBeanDefinitionRegistrar().registerBeanDefinitions(configurationSource, parserContext.getRegistry());
+
 		return null;
 	}
 

src/main/java/org/springframework/data/gemfire/function/config/FunctionExecutionBeanDefinitionRegistrar.java

@@ -12,6 +12,7 @@
  */
 package org.springframework.data.gemfire.function.config;
 
+import java.lang.annotation.Annotation;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -48,48 +49,42 @@ class FunctionExecutionBeanDefinitionRegistrar implements ImportBeanDefinitionRe
 	/*
 	 * This registers bean definitions from any function execution configuration source
 	 */
-	void registerBeanDefinitions (AbstractFunctionExecutionConfigurationSource configurationSource, BeanDefinitionRegistry registry) {
-		ResourceLoader resourceLoader = new DefaultResourceLoader();
-		Set<String> functionExecutionAnnotationTypes = new HashSet<String>(
-				AnnotationFunctionExecutionConfigurationSource.getFunctionExecutionAnnotationTypes().size());
-		for (Class<?> annotation : AbstractFunctionExecutionConfigurationSource.getFunctionExecutionAnnotationTypes()) {
-			functionExecutionAnnotationTypes.add(annotation.getName());
-		}
-
-		for (ScannedGenericBeanDefinition beanDefinition : configurationSource.getCandidates(resourceLoader)) {
+	void registerBeanDefinitions(AbstractFunctionExecutionConfigurationSource functionExecutionConfigurationSource,
+			BeanDefinitionRegistry registry) {
 
+		for (ScannedGenericBeanDefinition beanDefinition : functionExecutionConfigurationSource.getCandidates(new DefaultResourceLoader())) {
 			String functionExecutionAnnotation = getFunctionExecutionAnnotation(beanDefinition,
-					functionExecutionAnnotationTypes);
+				AnnotationFunctionExecutionConfigurationSource.getFunctionExecutionAnnotationTypeNames());
 
 			Assert.notNull(functionExecutionAnnotation);
 
-			String beanName = (String) beanDefinition.getMetadata()
-					.getAnnotationAttributes(functionExecutionAnnotation).get("id");
+			String beanName = (String) beanDefinition.getMetadata().getAnnotationAttributes(
+				functionExecutionAnnotation).get("id");
 
-			if (!StringUtils.hasLength(beanName)) {
+			if (!StringUtils.hasText(beanName)) {
 				beanName = BeanDefinitionReaderUtils.generateBeanName(beanDefinition, registry);
 			}
-			
+
 			AbstractFunctionExecutionBeanDefinitionBuilder builder = FunctionExecutionBeanDefinitionBuilderFactory
-					.newInstance(new FunctionExecutionConfiguration(beanDefinition, functionExecutionAnnotation));
+				.newInstance(new FunctionExecutionConfiguration(beanDefinition, functionExecutionAnnotation));
 
 			registry.registerBeanDefinition(beanName, builder.build(registry));
 		}
 	}
 
 	private String getFunctionExecutionAnnotation(ScannedGenericBeanDefinition beanDefinition,
-			Set<String> functionExecutionAnnotationTypes) {
+			Set<String> functionExecutionAnnotationTypeNames) {
 
 		Set<String> annotationTypes = beanDefinition.getMetadata().getAnnotationTypes();
 
 		String functionExecutionAnnotation = null;
 
-		for (String annotation : annotationTypes) {
-			if (functionExecutionAnnotationTypes.contains(annotation)) {
+		for (String annotationType : annotationTypes) {
+			if (functionExecutionAnnotationTypeNames.contains(annotationType)) {
 				Assert.isNull(functionExecutionAnnotation, String.format(
-						"interface %s contains multiple function execution annotations: %s, %s",
-						beanDefinition.getBeanClassName(), functionExecutionAnnotation, annotation));
-				functionExecutionAnnotation = annotation;
+					"interface %1$s contains multiple Function Execution Annotations: %2$s, %3$s",
+						beanDefinition.getBeanClassName(), functionExecutionAnnotation, annotationType));
+				functionExecutionAnnotation = annotationType;
 			}
 		}
 

src/main/java/org/springframework/data/gemfire/function/config/FunctionExecutionConfiguration.java

@@ -21,44 +21,46 @@ import org.springframework.util.Assert;
  * Function execution configuration used by bean definition builders
  * 
  * @author David Turanski
- *
+ * @author John Blum
  */
 class FunctionExecutionConfiguration  {
 	
-	private final Map<String,Object> attributes;
 	private Class<?> functionExecutionInterface;
+
+	private final Map<String,Object> attributes;
+
 	private final String annotationType;
 
-
 	FunctionExecutionConfiguration(ScannedGenericBeanDefinition beanDefinition, String annotationType) {
-		this.attributes = beanDefinition.getMetadata().getAnnotationAttributes(annotationType,true);
-		
 		try {
+			this.annotationType = annotationType;
+			this.attributes = beanDefinition.getMetadata().getAnnotationAttributes(annotationType, true);
 			this.functionExecutionInterface = beanDefinition.resolveBeanClass(beanDefinition.getClass().getClassLoader());
+
 			Assert.isTrue(functionExecutionInterface.isInterface(),
-					String.format("The annotation %s only applies to an interface. It is not valid for the type %s",
-							annotationType, functionExecutionInterface.getName()));
+				String.format("The annotation %1$s only applies to an interface. It is not valid for the type %2$s",
+					annotationType, functionExecutionInterface.getName()));
 			
-		} catch (ClassNotFoundException e) {
+		}
+		catch (ClassNotFoundException e) {
 			throw new RuntimeException(e);
 		}
-		this.annotationType = annotationType;
 	}
- 
-	Class<?> getFunctionExecutionInterface() {
-		return this.functionExecutionInterface;
+
+	String getAnnotationType() {
+		return this.annotationType;
 	}
-	 
+
+	Object getAttribute(String name) {
+		return attributes.get(name);
+	}
+
 	Map<String, Object> getAttributes() {
 		return this.attributes;
 	}
 
- 
-	Object getAttribute(String name) {
-		return attributes.get(name);
-	}
-	
-	String getAnnotationType() {
-		return this.annotationType;
+	Class<?> getFunctionExecutionInterface() {
+		return this.functionExecutionInterface;
 	}
+
 }

src/main/java/org/springframework/data/gemfire/function/config/MemberBasedExecutionBeanDefinitionBuilder.java

@@ -35,10 +35,10 @@ import org.springframework.util.StringUtils;
 	 
  
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder#getGemfireOperationsBeanDefinitionBuilder(org.springframework.beans.factory.support.BeanDefinitionRegistry)
+	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder#getGemfireFunctionOperationsBeanDefinitionBuilder(org.springframework.beans.factory.support.BeanDefinitionRegistry)
 	 */
 	@Override
-	protected BeanDefinitionBuilder getGemfireOperationsBeanDefinitionBuilder(BeanDefinitionRegistry registry) {
+	protected BeanDefinitionBuilder getGemfireFunctionOperationsBeanDefinitionBuilder(BeanDefinitionRegistry registry) {
 		
 		BeanDefinitionBuilder functionTemplateBuilder = BeanDefinitionBuilder.genericBeanDefinition(getGemfireOperationsClass());
 		

src/main/java/org/springframework/data/gemfire/function/config/OnMemberExecutionBeanDefinitionBuilder.java

@@ -30,7 +30,7 @@ class OnMemberExecutionBeanDefinitionBuilder extends MemberBasedExecutionBeanDef
 	}
 
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.MemberBasedExecutionBeanDefinitionBuilder#getGemfireOperationsClass()
+	 * @see org.springframework.data.gemfire.function.config.MemberBasedExecutionBeanDefinitionBuilder#getGemfireFunctionOperationsClass()
 	 */
 	@Override
 	protected Class<?> getGemfireOperationsClass() {

src/main/java/org/springframework/data/gemfire/function/config/OnMembersExecutionBeanDefinitionBuilder.java

@@ -29,7 +29,7 @@ class OnMembersExecutionBeanDefinitionBuilder extends MemberBasedExecutionBeanDe
 	}
 
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.MemberBasedExecutionBeanDefinitionBuilder#getGemfireOperationsClass()
+	 * @see org.springframework.data.gemfire.function.config.MemberBasedExecutionBeanDefinitionBuilder#getGemfireFunctionOperationsClass()
 	 */
 	@Override
 	protected Class<?> getGemfireOperationsClass() {

src/main/java/org/springframework/data/gemfire/function/config/OnRegionExecutionBeanDefinitionBuilder.java

@@ -33,10 +33,10 @@ class OnRegionExecutionBeanDefinitionBuilder extends AbstractFunctionExecutionBe
 	 
  
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder#getGemfireOperationsBeanDefinitionBuilder(org.springframework.beans.factory.support.BeanDefinitionRegistry)
+	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder#getGemfireFunctionOperationsBeanDefinitionBuilder(org.springframework.beans.factory.support.BeanDefinitionRegistry)
 	 */
 	@Override
-	protected BeanDefinitionBuilder getGemfireOperationsBeanDefinitionBuilder(BeanDefinitionRegistry registry) {
+	protected BeanDefinitionBuilder getGemfireFunctionOperationsBeanDefinitionBuilder(BeanDefinitionRegistry registry) {
 		BeanDefinitionBuilder functionTemplateBuilder = BeanDefinitionBuilder.genericBeanDefinition(GemfireOnRegionFunctionTemplate.class);
 		functionTemplateBuilder.addConstructorArgReference((String)configuration.getAttribute("region"));
 		return functionTemplateBuilder;

src/main/java/org/springframework/data/gemfire/function/config/OnServerExecutionBeanDefinitionBuilder.java

@@ -16,22 +16,21 @@ import org.springframework.data.gemfire.function.execution.GemfireOnServerFuncti
 
 /**
  * @author David Turanski
- *
+ * @author John Blum
  */
 class OnServerExecutionBeanDefinitionBuilder extends ServerBasedExecutionBeanDefinitionBuilder {
 
-	/**
-	 * @param configuration
-	 */
 	OnServerExecutionBeanDefinitionBuilder(FunctionExecutionConfiguration configuration) {
 		super(configuration);
 	}
 
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.ServerBasedExecutionBeanDefinitionBuilder#getGemfireOperationsClass()
+	 * @see org.springframework.data.gemfire.function.config.ServerBasedExecutionBeanDefinitionBuilder
+	 * 	#getGemfireFunctionOperationsClass()
 	 */
 	@Override
-	protected Class<?> getGemfireOperationsClass() {
+	protected Class<?> getGemfireFunctionOperationsClass() {
 		 return GemfireOnServerFunctionTemplate.class;
 	}
+
 }

src/main/java/org/springframework/data/gemfire/function/config/OnServersExecutionBeanDefinitionBuilder.java

@@ -28,10 +28,10 @@ class OnServersExecutionBeanDefinitionBuilder extends ServerBasedExecutionBeanDe
 	}
 
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.ServerBasedExecutionBeanDefinitionBuilder#getGemfireOperationsClass()
+	 * @see org.springframework.data.gemfire.function.config.ServerBasedExecutionBeanDefinitionBuilder#getGemfireFunctionOperationsClass()
 	 */
 	@Override
-	protected Class<?> getGemfireOperationsClass() {
+	protected Class<?> getGemfireFunctionOperationsClass() {
 		return GemfireOnServersFunctionTemplate.class;
 	}
 }

src/main/java/org/springframework/data/gemfire/function/config/ServerBasedExecutionBeanDefinitionBuilder.java

@@ -33,39 +33,36 @@ abstract class ServerBasedExecutionBeanDefinitionBuilder extends AbstractFunctio
 	}
 
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder#getGemfireOperationsBeanDefinitionBuilder(org.springframework.beans.factory.support.BeanDefinitionRegistry)
+	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder
+	 * 	#getGemfireFunctionOperationsBeanDefinitionBuilder(org.springframework.beans.factory.support.BeanDefinitionRegistry)
 	 */
 	@Override
-	protected BeanDefinitionBuilder getGemfireOperationsBeanDefinitionBuilder(BeanDefinitionRegistry registry) {
+	protected BeanDefinitionBuilder getGemfireFunctionOperationsBeanDefinitionBuilder(BeanDefinitionRegistry registry) {
+		BeanDefinitionBuilder functionTemplateBuilder = BeanDefinitionBuilder.genericBeanDefinition(
+			getGemfireFunctionOperationsClass());
 
-		BeanDefinitionBuilder functionTemplateBuilder = BeanDefinitionBuilder
-				.genericBeanDefinition(getGemfireOperationsClass());
-
-		String pool = (String) configuration.getAttribute("pool");
 		String cache = (String) configuration.getAttribute("cache");
+		String pool = (String) configuration.getAttribute("pool");
 
-		Assert.state(!(StringUtils.hasText(pool) && StringUtils.hasText(cache)), String.format(
-				"invalid configuration for interface %s. Cannot specify both 'pool' and 'cache'", configuration
-						.getFunctionExecutionInterface().getName()));
+		Assert.state(StringUtils.hasText(cache) && !(StringUtils.hasText(pool)), String.format(
+			"invalid configuration for interface %s; cannot specify both 'pool' and 'cache'",
+				configuration.getFunctionExecutionInterface().getName()));
+
+		functionTemplateBuilder.addConstructorArgReference(StringUtils.hasText(pool) ? pool
+			: (StringUtils.hasText(cache) ? cache : GemfireConstants.DEFAULT_GEMFIRE_CACHE_NAME));
 
-		if (StringUtils.hasText(pool)) {
-			functionTemplateBuilder.addConstructorArgReference(pool);
-		} else {
-			if (!StringUtils.hasText(cache)) {
-				cache = GemfireConstants.DEFAULT_GEMFIRE_CACHE_NAME;
-			}
-			functionTemplateBuilder.addConstructorArgReference(cache);
-		}
 		return functionTemplateBuilder;
 	}
 
 	/* (non-Javadoc)
-	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder#getFunctionProxyFactoryBeanClass()
+	 * @see org.springframework.data.gemfire.function.config.AbstractFunctionExecutionBeanDefinitionBuilder
+	 * 	#getFunctionProxyFactoryBeanClass()
 	 */
 	@Override
 	protected Class<?> getFunctionProxyFactoryBeanClass() {
 		return GemfireFunctionProxyFactoryBean.class;
 	}
 
-	protected abstract Class<?> getGemfireOperationsClass();
+	protected abstract Class<?> getGemfireFunctionOperationsClass();
+
 }

src/main/java/org/springframework/data/gemfire/function/execution/GemfireFunctionProxyFactoryBean.java

@@ -25,48 +25,47 @@ import org.springframework.beans.factory.FactoryBean;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
-import com.gemstone.gemfire.cache.execute.FunctionService;
-
 /**
- * A proxy Factory Bean for all non-region function execution interfaces
+ * A Proxy FactoryBean for all non-Region Function Execution interfaces.
  *  
  * @author David Turanski
+ * @author John Blum
+ * @see java.lang.reflect.Method
+ * @see org.aopalliance.intercept.MethodInterceptor
+ * @see org.springframework.beans.factory.BeanClassLoaderAware
+ * @see org.springframework.beans.factory.FactoryBean
  */
 public class GemfireFunctionProxyFactoryBean implements FactoryBean<Object>, MethodInterceptor, BeanClassLoaderAware {
 
-	protected volatile ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader();
-
-	protected final Class<?> serviceInterface;
-
-	protected volatile Object serviceProxy;
+	private volatile ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader();
 
 	private volatile boolean initialized;
 
-	//protected String functionId;
+	private final Class<?> functionExecutionInterface;
+
+	private volatile Object functionExecutionProxy;
+
+	private final GemfireFunctionOperations gemfireFunctionOperations;
 
 	protected Log logger = LogFactory.getLog(this.getClass());
 
-	protected final GemfireFunctionOperations gemfireFunctionOperations;
-
 	private FunctionExecutionMethodMetadata<MethodMetadata> methodMetadata;
 
 	/**
-	 * @param serviceInterface the proxied interface
+	 * @param functionExecutionInterface the proxied interface
 	 * @param gemfireFunctionOperations an interface used to delegate the function invocation (typically a GemFire function template)
 	 */
-	public GemfireFunctionProxyFactoryBean(Class<?> serviceInterface, GemfireFunctionOperations gemfireFunctionOperations) {
-		Assert.notNull(serviceInterface, "'serviceInterface' must not be null");
-		Assert.isTrue(serviceInterface.isInterface(), "'serviceInterface' must be an interface");
-		this.serviceInterface = serviceInterface;
+	public GemfireFunctionProxyFactoryBean(Class<?> functionExecutionInterface, GemfireFunctionOperations gemfireFunctionOperations) {
+		Assert.notNull(functionExecutionInterface, "'functionExecutionInterface' must not be null");
+		Assert.isTrue(functionExecutionInterface.isInterface(), "'functionExecutionInterface' must be an interface");
+
+		this.functionExecutionInterface = functionExecutionInterface;
 		this.gemfireFunctionOperations = gemfireFunctionOperations;
-		this.methodMetadata = new DefaultFunctionExecutionMethodMetadata(serviceInterface);
+		this.methodMetadata = new DefaultFunctionExecutionMethodMetadata(functionExecutionInterface);
 	}
 
-
-
-	protected Object invokeFunction(Method method, Object[] args) {
-		MethodMetadata mmd = this.methodMetadata.getMethodMetadata(method);
-		return this.gemfireFunctionOperations.executeAndExtract(mmd.getFunctionId(), args);
+	protected GemfireFunctionOperations getGemfireFunctionOperations() {
+		return gemfireFunctionOperations;
 	}
 
 	@Override
@@ -78,7 +77,7 @@ public class GemfireFunctionProxyFactoryBean implements FactoryBean<Object>, Met
 	public Object invoke(MethodInvocation invocation) throws Throwable {
 
 		if (AopUtils.isToStringMethod(invocation.getMethod())) {
-			return "Gemfire function proxy for service interface [" + this.serviceInterface + "]";
+			return "Gemfire function proxy for service interface [" + this.functionExecutionInterface + "]";
 		}
 
 		if (logger.isDebugEnabled()) {
@@ -88,19 +87,24 @@ public class GemfireFunctionProxyFactoryBean implements FactoryBean<Object>, Met
 		return invokeFunction(invocation.getMethod(), invocation.getArguments());
 	}
 
+	protected Object invokeFunction(Method method, Object[] args) {
+		return this.gemfireFunctionOperations.executeAndExtract(
+			methodMetadata.getMethodMetadata(method).getFunctionId(), args);
+	}
 
 	@Override
 	public Object getObject() throws Exception {
-		if (this.serviceProxy == null) {
-			this.onInit();
-			Assert.notNull(this.serviceProxy, "failed to initialize proxy");
+		if (functionExecutionProxy == null) {
+			onInit();
+			Assert.notNull(functionExecutionProxy, "failed to initialize proxy");
 		}
-		return this.serviceProxy;
+
+		return functionExecutionProxy;
 	}
 
 	@Override
 	public Class<?> getObjectType() {
-		return (this.serviceInterface != null ? this.serviceInterface : null);
+		return this.functionExecutionInterface;
 	}
 
 	@Override
@@ -109,11 +113,11 @@ public class GemfireFunctionProxyFactoryBean implements FactoryBean<Object>, Met
 	}
 
 	protected void onInit() {
-		if (this.initialized) {
-			return;
+		if (!initialized) {
+			ProxyFactory proxyFactory = new ProxyFactory(functionExecutionInterface, this);
+			functionExecutionProxy = proxyFactory.getProxy(beanClassLoader);
+			initialized = true;
 		}
-		ProxyFactory proxyFactory = new ProxyFactory(serviceInterface, this);
-		this.serviceProxy = proxyFactory.getProxy(this.beanClassLoader);
-		this.initialized = true;
 	}
+
 }

src/main/java/org/springframework/data/gemfire/function/execution/OnRegionFunctionProxyFactoryBean.java

@@ -12,7 +12,6 @@
  */
 package org.springframework.data.gemfire.function.execution;
 
-
 import java.lang.reflect.Method;
 import java.util.Set;
 
@@ -20,7 +19,7 @@ import org.springframework.data.gemfire.util.ArrayUtils;
 
 /**
  * @author David Turanski
- *
+ * @author John Blum
  */
 public class OnRegionFunctionProxyFactoryBean extends GemfireFunctionProxyFactoryBean {
 
@@ -37,33 +36,25 @@ public class OnRegionFunctionProxyFactoryBean extends GemfireFunctionProxyFactor
 
 	@Override
 	protected Iterable<?> invokeFunction(Method method, Object[] args) {
-		
+		GemfireOnRegionOperations gemfireOnRegionOperations = (GemfireOnRegionOperations) getGemfireFunctionOperations();
+
+		OnRegionMethodMetadata onRegionMethodMetadata = methodMetadata.getMethodMetadata(method);
+
+		int filterArgPosition = onRegionMethodMetadata.getFilterArgPosition();
+		String functionId = onRegionMethodMetadata.getFunctionId();
+
 		Set<?> filter = null;
-		
-		Iterable<?> results = null;
-		
-		GemfireOnRegionOperations gemfireOnRegionOperations = (GemfireOnRegionOperations) this.gemfireFunctionOperations;
-		
-		OnRegionMethodMetadata ormmd = methodMetadata.getMethodMetadata(method);
-		
-		int filterArgPosition = ormmd.getFilterArgPosition();
-		
-		String functionId = ormmd.getFunctionId();
-		
+
 		/*
 		 * extract filter from args if necessary
 		 */
-		if (filterArgPosition >=0 ) {
-			filter = (Set<?>)args[filterArgPosition];
+		if (filterArgPosition >= 0) {
+			filter = (Set<?>) args[filterArgPosition];
 			args = ArrayUtils.remove(args, filterArgPosition);
 		}
-		
-		if (filter == null) {
-			results =  gemfireOnRegionOperations.execute(functionId, args);
-		} else {
-			results =  gemfireOnRegionOperations.execute(functionId, filter, args);
-		}
- 
-		return results;
+
+		return (filter == null ?  gemfireOnRegionOperations.execute(functionId, args)
+			: gemfireOnRegionOperations.execute(functionId, filter, args));
 	}
+
 }

src/test/java/org/springframework/data/gemfire/function/config/FunctionExecutionIntegrationTests.java

@@ -45,7 +45,7 @@ public class FunctionExecutionIntegrationTests {
 	public void testProxyFactoryBeanCreated() throws Exception {
 		OnRegionFunctionProxyFactoryBean factoryBean = (OnRegionFunctionProxyFactoryBean) context
 				.getBean("&testFunction");
-		Class<?> serviceInterface = TestUtils.readField("serviceInterface", factoryBean);
+		Class<?> serviceInterface = TestUtils.readField("functionExecutionInterface", factoryBean);
 		assertEquals(serviceInterface, TestOnRegionFunction.class);
 
 		Region<?, ?> r1 = context.getBean("r1", Region.class);

src/test/java/org/springframework/data/gemfire/function/config/XmlConfiguredFunctionExecutionIntegrationTests.java

@@ -43,7 +43,7 @@ public class XmlConfiguredFunctionExecutionIntegrationTests {
 	public void testProxyFactoryBeanCreated() throws Exception {
 		OnRegionFunctionProxyFactoryBean factoryBean = (OnRegionFunctionProxyFactoryBean) context
 				.getBean("&testFunction");
-		Class<?> serviceInterface = TestUtils.readField("serviceInterface", factoryBean);
+		Class<?> serviceInterface = TestUtils.readField("functionExecutionInterface", factoryBean);
 		assertEquals(serviceInterface, TestOnRegionFunction.class);
 
 		Region<?, ?> r1 = context.getBean("r1", Region.class);