SGF-691 - Fix Off-Heap Annotation config to properly handle Entity-defined and Java-based Region configuration.

Additionally, this change set also renames the spring.data.gemfire.cache.off-heap-memory-size property to spring.data.gemfire.cache.off-heap.memory-size and the spring.data.gemfire.cache.region-names property to spring.data.gemfire.cache.off-heap.region-names.

src/main/java/org/springframework/data/gemfire/config/annotation/EnableOffHeap.java

@@ -29,9 +29,8 @@ import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 
 /**
- * The {@link EnableOffHeap} annotation marks a Spring {@link Configuration @Configuration} annotated {@link Class}
+ * The {@link EnableOffHeap} annotation marks a Spring {@link Configuration @Configuration} annotated application
- * to configure and enable Pivotal GemFire/Apache Geode Off-Heap Memory support and data storage
+ * {@link Class} to configure and enable Off-Heap Memory data storage in cache {@link Region Regions}.
- * in cache {@link org.apache.geode.cache.Region Regions}.
  *
  * @author John Blum
  * @see java.lang.annotation.Annotation
@@ -62,16 +61,16 @@ public @interface EnableOffHeap {
 	 *
 	 * Defaults to unset.
 	 *
-	 * Use the {@literal spring.data.gemfire.cache.off-heap-memory-size} property in {@literal application.properties}.
+	 * Use the {@literal spring.data.gemfire.cache.off-heap.memory-size} property in {@literal application.properties}.
 	 */
 	String memorySize();
 
 	/**
-	 * Identifies all the {@link Region Regions} by name in which the off-heap memory setting will be applied.
+	 * Identifies all the {@link Region Regions} by name in which the Off-Heap Memory settings will be applied.
 	 *
-	 * Defaults to all Regions.
+	 * Defaults to all {@link Region Regions}.
 	 *
-	 * Use the {@literal spring.data.gemfire.cache.region-names} property in {@literal application.properties}.
+	 * Use the {@literal spring.data.gemfire.cache.off-heap.region-names} property in {@literal application.properties}.
 	 */
 	String[] regionNames() default {};
 

src/main/java/org/springframework/data/gemfire/config/annotation/OffHeapConfiguration.java

@@ -20,6 +20,7 @@ package org.springframework.data.gemfire.config.annotation;
 import static java.util.Arrays.stream;
 import static org.springframework.data.gemfire.util.ArrayUtils.nullSafeArray;
 
+import java.lang.annotation.Annotation;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -39,11 +40,8 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.context.annotation.ImportBeanDefinitionRegistrar;
 import org.springframework.core.type.AnnotationMetadata;
-import org.springframework.data.gemfire.GenericRegionFactoryBean;
+import org.springframework.data.gemfire.RegionLookupFactoryBean;
-import org.springframework.data.gemfire.LocalRegionFactoryBean;
+import org.springframework.data.gemfire.config.annotation.support.AbstractAnnotationConfigSupport;
-import org.springframework.data.gemfire.PartitionedRegionFactoryBean;
-import org.springframework.data.gemfire.ReplicatedRegionFactoryBean;
-import org.springframework.data.gemfire.client.ClientRegionFactoryBean;
 import org.springframework.data.gemfire.config.annotation.support.EmbeddedServiceConfigurationSupport;
 import org.springframework.data.gemfire.util.CollectionUtils;
 import org.springframework.data.gemfire.util.PropertiesBuilder;
@@ -67,7 +65,7 @@ public class OffHeapConfiguration extends EmbeddedServiceConfigurationSupport {
 	 * @see org.springframework.data.gemfire.config.annotation.EnableOffHeap
 	 */
 	@Override
-	protected Class getAnnotationType() {
+	protected Class<? extends Annotation> getAnnotationType() {
 		return EnableOffHeap.class;
 	}
 
@@ -79,7 +77,7 @@ public class OffHeapConfiguration extends EmbeddedServiceConfigurationSupport {
 		BeanDefinitionBuilder builder =
 			BeanDefinitionBuilder.genericBeanDefinition(OffHeapBeanFactoryPostProcessor.class);
 
-		builder.addConstructorArgValue(resolveProperty(cacheProperty("region-names"),
+		builder.addConstructorArgValue(resolveProperty(cacheOffHeapProperty("region-names"),
 			String[].class, (String[]) annotationAttributes.get("regionNames")));
 
 		registry.registerBeanDefinition(generateBeanName(OffHeapBeanFactoryPostProcessor.class),
@@ -92,63 +90,58 @@ public class OffHeapConfiguration extends EmbeddedServiceConfigurationSupport {
 
 		return PropertiesBuilder.create()
 			.setProperty("off-heap-memory-size",
-				resolveProperty(cacheProperty("off-heap-memory-size"),
+				resolveProperty(cacheOffHeapProperty("memory-size"),
 					(String) annotationAttributes.get("memorySize")))
 			.build();
 	}
 
 	/* (non-Javadoc) */
 	@SuppressWarnings("unused")
-	protected static class OffHeapBeanFactoryPostProcessor implements BeanFactoryPostProcessor {
+	protected static class OffHeapBeanFactoryPostProcessor extends AbstractAnnotationConfigSupport
-
+			implements BeanFactoryPostProcessor {
-		protected static final Set<String> REGION_FACTORY_BEAN_TYPES = new HashSet<>(5);
-
-		static {
-			REGION_FACTORY_BEAN_TYPES.add(ClientRegionFactoryBean.class.getName());
-			REGION_FACTORY_BEAN_TYPES.add(GenericRegionFactoryBean.class.getName());
-			REGION_FACTORY_BEAN_TYPES.add(LocalRegionFactoryBean.class.getName());
-			REGION_FACTORY_BEAN_TYPES.add(PartitionedRegionFactoryBean.class.getName());
-			REGION_FACTORY_BEAN_TYPES.add(ReplicatedRegionFactoryBean.class.getName());
-		}
 
 		private final Set<String> regionNames;
 
-		protected OffHeapBeanFactoryPostProcessor(String[] regionNames) {
-			this(CollectionUtils.asSet(nullSafeArray(regionNames, String.class)));
-		}
-
 		protected OffHeapBeanFactoryPostProcessor(Set<String> regionNames) {
 			this.regionNames = CollectionUtils.nullSafeSet(regionNames);
 		}
 
+		@Override
+		protected Class<? extends Annotation> getAnnotationType() {
+			throw new UnsupportedOperationException("Not Implemented");
+		}
+
 		@Override
 		public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
 
-			stream(nullSafeArray(beanFactory.getBeanDefinitionNames(), String.class)).forEach(beanName -> {
+			stream(nullSafeArray(beanFactory.getBeanDefinitionNames(), String.class)).forEach(beanName ->
-
+				Optional.of(beanFactory.getBeanDefinition(beanName))
-				Optional.ofNullable(beanFactory.getBeanDefinition(beanName))
+					.filter(beanDefinition -> isTargetedRegionBean(beanName, beanDefinition, beanFactory))
-					.filter(bean -> isTargetedRegionBean(beanName, bean, beanFactory))
+					.ifPresent(beanDefinition -> beanDefinition.getPropertyValues()
-					.ifPresent(bean ->
+						.addPropertyValue("offHeap", true)));
-						bean.getPropertyValues().addPropertyValue("offHeap", true));
-			});
 		}
 
-		boolean isTargetedRegionBean(String beanName, BeanDefinition bean,
+		boolean isTargetedRegionBean(String beanName, BeanDefinition beanDefinition,
 				ConfigurableListableBeanFactory beanFactory) {
 
-			return (isRegionBean(bean) && isNamedRegion(beanName, bean, beanFactory));
+			return isNamedRegion(beanName, beanDefinition, beanFactory) && isRegionBean(beanDefinition, beanFactory);
 		}
 
-		boolean isRegionBean(BeanDefinition bean) {
+		boolean isRegionBean(BeanDefinition beanDefinition, ConfigurableListableBeanFactory beanFactory) {
-			return (bean != null && REGION_FACTORY_BEAN_TYPES.contains(bean.getBeanClassName()));
+
+			return Optional.ofNullable(beanDefinition)
+				.flatMap(it -> resolveBeanClass(it, beanFactory.getBeanClassLoader()))
+				.filter(beanClass -> RegionLookupFactoryBean.class.isAssignableFrom(beanClass))
+				.isPresent();
 		}
 
-		boolean isNamedRegion(String beanName, BeanDefinition bean, ConfigurableListableBeanFactory beanFactory) {
+		boolean isNamedRegion(String beanName, BeanDefinition beanDefinition, BeanFactory beanFactory) {
-			return (CollectionUtils.isEmpty(regionNames) || CollectionUtils.containsAny(regionNames,
+
-				getBeanNames(beanName, bean, beanFactory)));
+			return CollectionUtils.isEmpty(regionNames)
+				|| CollectionUtils.containsAny(regionNames, getBeanNames(beanName, beanDefinition, beanFactory));
 		}
 
-		Collection<String> getBeanNames(String beanName, BeanDefinition bean, BeanFactory beanFactory) {
+		Collection<String> getBeanNames(String beanName, BeanDefinition beanDefinition, BeanFactory beanFactory) {
 
 			Collection<String> beanNames = new HashSet<>();
 
@@ -156,7 +149,7 @@ public class OffHeapConfiguration extends EmbeddedServiceConfigurationSupport {
 
 			Collections.addAll(beanNames, beanFactory.getAliases(beanName));
 
-			PropertyValue regionName = bean.getPropertyValues().getPropertyValue("regionName");
+			PropertyValue regionName = beanDefinition.getPropertyValues().getPropertyValue("regionName");
 
 			if (regionName != null) {
 

src/main/java/org/springframework/data/gemfire/config/annotation/support/AbstractAnnotationConfigSupport.java

@@ -682,6 +682,16 @@ public abstract class AbstractAnnotationConfigSupport
 		return String.format("%1$s%2$s", propertyName("cache.client."), propertyNameSuffix);
 	}
 
+	/* (non-Javadoc) */
+	protected String cacheCompressionProperty(String propertyNameSuffix) {
+		return String.format("%1$s%2$s", propertyName("cache.compression."), propertyNameSuffix);
+	}
+
+	/* (non-Javadoc) */
+	protected String cacheOffHeapProperty(String propertyNameSuffix) {
+		return String.format("%1$s%2$s", propertyName("cache.off-heap."), propertyNameSuffix);
+	}
+
 	/* (non-Javadoc) */
 	protected String cachePeerProperty(String propertyNameSuffix) {
 		return String.format("%1$s%2$s", propertyName("cache.peer."), propertyNameSuffix);
@@ -871,7 +881,7 @@ public abstract class AbstractAnnotationConfigSupport
 	 * @see org.springframework.util.ClassUtils#forName(String, ClassLoader)
 	 * @see #resolveBeanClassName(BeanDefinition)
 	 */
-	private Optional<Class<?>> resolveBeanClass(BeanDefinition beanDefinition, ClassLoader classLoader) {
+	protected Optional<Class<?>> resolveBeanClass(BeanDefinition beanDefinition, ClassLoader classLoader) {
 
 		Class<?> beanClass = (beanDefinition instanceof AbstractBeanDefinition
 			? safeResolveType(() -> ((AbstractBeanDefinition) beanDefinition).resolveBeanClass(classLoader)) : null);

src/main/java/org/springframework/data/gemfire/config/annotation/support/CacheTypeAwareRegionFactoryBean.java

@@ -77,6 +77,7 @@ public class CacheTypeAwareRegionFactoryBean<K, V> extends RegionLookupFactoryBe
 	private GemFireCache gemfireCache;
 
 	private Boolean close = false;
+	private Boolean offHeap = false;
 
 	private Class<K> keyConstraint;
 	private Class<V> valueConstraint;
@@ -181,6 +182,7 @@ public class CacheTypeAwareRegionFactoryBean<K, V> extends RegionLookupFactoryBe
 		serverRegionFactory.setDiskStoreName(getDiskStoreName());
 		serverRegionFactory.setKeyConstraint(getKeyConstraint());
 		serverRegionFactory.setLookupEnabled(getLookupEnabled());
+		serverRegionFactory.setOffHeap(getOffHeap());
 		serverRegionFactory.setRegionConfigurers(this.regionConfigurers);
 		serverRegionFactory.setRegionName(regionName);
 		serverRegionFactory.setShortcut(getServerRegionShortcut());
@@ -276,6 +278,14 @@ public class CacheTypeAwareRegionFactoryBean<K, V> extends RegionLookupFactoryBe
 		return this.keyConstraint;
 	}
 
+	public void setOffHeap(Boolean offHeap) {
+		this.offHeap = offHeap;
+	}
+
+	protected Boolean getOffHeap() {
+		return this.offHeap;
+	}
+
 	public void setPoolName(String poolName) {
 		this.poolName = poolName;
 	}

src/test/java/org/springframework/data/gemfire/config/annotation/EnableOffHeapConfigurationUnitTests.java

@@ -0,0 +1,175 @@
+/*
+ * Copyright 2017 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
+ *
+ *      http://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.config.annotation;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.Optional;
+
+import org.apache.geode.cache.GemFireCache;
+import org.apache.geode.cache.Region;
+import org.junit.After;
+import org.junit.Test;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+import org.springframework.data.gemfire.GemfireUtils;
+import org.springframework.data.gemfire.LocalRegionFactoryBean;
+import org.springframework.data.gemfire.PartitionedRegionFactoryBean;
+import org.springframework.data.gemfire.ReplicatedRegionFactoryBean;
+import org.springframework.data.gemfire.test.mock.annotation.EnableGemFireMockObjects;
+import org.springframework.data.gemfire.test.model.Person;
+
+/**
+ * Unit tests for {@link EnableOffHeap} and {@link OffHeapConfiguration}.
+ *
+ * @author John Blum
+ * @see org.junit.Test
+ * @see org.springframework.data.gemfire.config.annotation.EnableOffHeap
+ * @see org.springframework.data.gemfire.config.annotation.OffHeapConfiguration
+ * @since 2.0.2
+ */
+public class EnableOffHeapConfigurationUnitTests {
+
+	private ConfigurableApplicationContext applicationContext;
+
+	@After
+	public void tearDown() {
+		Optional.ofNullable(this.applicationContext).ifPresent(ConfigurableApplicationContext::close);
+	}
+
+	private ConfigurableApplicationContext newApplicationContext(Class<?>... annotatedClasses) {
+		return new AnnotationConfigApplicationContext(annotatedClasses);
+	}
+
+	private void assertRegionOffHeap(Region<?, ?> region, String regionName, boolean offHeapEnabled) {
+
+		assertThat(region).isNotNull();
+		assertThat(region.getName()).isEqualTo(regionName);
+		assertThat(region.getFullPath()).isEqualTo(GemfireUtils.toRegionPath(regionName));
+		assertThat(region.getAttributes()).isNotNull();
+		assertThat(region.getAttributes().getOffHeap()).isEqualTo(offHeapEnabled);
+	}
+
+	@Test
+	public void offHeapConfiguredForAllRegions() {
+
+		this.applicationContext = newApplicationContext(EnableOffHeapForAllRegionsConfiguration.class);
+
+		assertThat(this.applicationContext).isNotNull();
+
+		GemFireCache gemfireCache = this.applicationContext.getBean("gemfireCache", GemFireCache.class);
+
+		assertThat(gemfireCache).isNotNull();
+		assertThat(gemfireCache.getDistributedSystem()).isNotNull();
+		assertThat(gemfireCache.getDistributedSystem().getProperties()).containsKey("off-heap-memory-size");
+		assertThat(gemfireCache.getDistributedSystem().getProperties().getProperty("off-heap-memory-size"))
+			.isEqualTo("8192m");
+
+		Arrays.asList("People", "ExampleLocalRegion", "ExamplePartitionRegion", "ExampleReplicateRegion")
+			.forEach(regionName -> {
+				assertThat(this.applicationContext.containsBean(regionName)).isTrue();
+				assertRegionOffHeap(this.applicationContext.getBean(regionName, Region.class),
+					regionName, true);
+			});
+	}
+
+	@Test
+	public void offHeapConfiguredForSelectRegions() {
+
+		this.applicationContext = newApplicationContext(EnableOffHeapForSelectRegionsConfiguration.class);
+
+		assertThat(this.applicationContext).isNotNull();
+
+		GemFireCache gemfireCache = this.applicationContext.getBean("gemfireCache", GemFireCache.class);
+
+		assertThat(gemfireCache).isNotNull();
+		assertThat(gemfireCache.getDistributedSystem()).isNotNull();
+		assertThat(gemfireCache.getDistributedSystem().getProperties()).containsKey("off-heap-memory-size");
+		assertThat(gemfireCache.getDistributedSystem().getProperties().getProperty("off-heap-memory-size"))
+			.isEqualTo("1024m");
+
+		Arrays.asList("People", "ExampleLocalRegion", "ExamplePartitionRegion", "ExampleReplicateRegion")
+			.forEach(regionName -> {
+				assertThat(this.applicationContext.containsBean(regionName)).isTrue();
+				assertRegionOffHeap(this.applicationContext.getBean(regionName, Region.class),
+					regionName, Arrays.asList("People", "ExamplePartitionRegion").contains(regionName));
+			});
+	}
+
+	@Configuration
+	@SuppressWarnings("unused")
+	static class TestRegionConfiguration {
+
+		@Bean("ExampleLocalRegion")
+		public LocalRegionFactoryBean<Object, Object> localRegion(GemFireCache gemfireCache) {
+
+			LocalRegionFactoryBean<Object, Object> localRegion = new LocalRegionFactoryBean<>();
+
+			localRegion.setCache(gemfireCache);
+			localRegion.setClose(false);
+			localRegion.setPersistent(false);
+
+			return localRegion;
+		}
+
+		@Bean("ExamplePartitionRegion")
+		public PartitionedRegionFactoryBean<Object, Object> partitionRegion(GemFireCache gemfireCache) {
+
+			PartitionedRegionFactoryBean<Object, Object> partitionRegion = new PartitionedRegionFactoryBean<>();
+
+			partitionRegion.setCache(gemfireCache);
+			partitionRegion.setClose(false);
+			partitionRegion.setPersistent(false);
+
+			return partitionRegion;
+		}
+
+		@Bean("ExampleReplicateRegion")
+		public ReplicatedRegionFactoryBean<Object, Object> replicateRegion(GemFireCache gemfireCache) {
+
+			ReplicatedRegionFactoryBean<Object, Object> replicateRegion = new ReplicatedRegionFactoryBean<>();
+
+			replicateRegion.setCache(gemfireCache);
+			replicateRegion.setClose(false);
+			replicateRegion.setPersistent(false);
+
+			return replicateRegion;
+		}
+	}
+
+	@PeerCacheApplication
+	@EnableGemFireMockObjects
+	@EnableEntityDefinedRegions(basePackageClasses = Person.class)
+	@EnableOffHeap(memorySize = "8192m")
+	@Import(TestRegionConfiguration.class)
+	@SuppressWarnings("unused")
+	static class EnableOffHeapForAllRegionsConfiguration {
+	}
+
+	@PeerCacheApplication
+	@EnableGemFireMockObjects
+	@EnableEntityDefinedRegions(basePackageClasses = Person.class)
+	@EnableOffHeap(memorySize = "1024m", regionNames = { "People", "ExamplePartitionRegion" })
+	@Import(TestRegionConfiguration.class)
+	@SuppressWarnings("unused")
+	static class EnableOffHeapForSelectRegionsConfiguration {
+	}
+}