Add auto-configuration support for Apache Geode as a caching provider. #6967

jxblum · 2016-09-21T07:17:44Z

This PR enhances Spring Boot's Caching auto-configuration feature to include support for Apache Geode as a caching provider.

The enhanced, auto-configuration Caching support in Spring Boot uses Spring Data Geode's GemfireCacheManager implementation to enable Apache Geode as a provider in Spring's Cache Abstraction.

Both Spring Data Geode and Apache Geode are Open Source and licensed under the ASL 2.0, and both are freely available from Maven Central.

NOTE: this PR SHOULD NOT BE MERGED until @philwebb and I discuss options for Spring Boot's support of either Pivotal GemFire or Apache Geode.

Closes spring-projectsgh-6967

jxblum · 2016-11-03T09:29:19Z

@philwebb , @snicoll & Spring Boot team-

This PR adding support to auto-configure Apache Geode as a caching provider has been updated with the latest release of Spring Data Geode 1.0.0.INCUBATING-RELEASE (here), which is now based on Apache Geode 1.0.0-incubating GA release (here; more details here).

If you have questions, let me know.

Thanks!

snicoll

Thanks for the PR @jxblum! I've added some comments.

snicoll · 2016-11-03T09:48:02Z

...boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/cache/CacheType.java

+	GEODE,
+
+	/**
+>>>>>>> Add auto-configuration support for Apache Geode as a caching provider.


Merge glitch

Argh, sorry. I even went searching for those and still missed a few. Fixed.

snicoll · 2016-11-03T09:49:17Z

...gure/src/main/java/org/springframework/boot/autoconfigure/cache/GeodeCacheConfiguration.java

+ * @since 1.5.0
+ */
+@Configuration
+@ConditionalOnBean({ CacheFactoryBean.class, Cache.class,


Shouldn't the condition be on GemfireCache there?

No. I mean GemfireCache is a class in Spring Data Geode and I cannot obviously use the ConditionalOnBean condition since that is a Spring Boot Condition class.

I somewhat modeled my GeodeCacheConfiguration class after the RedisCacheConfiguration class, and specifically this. RedisTemplate comes from spring-data-redis and it would be weird to put a Condition on the RedisTemplate for caching.

I guess I was maybe a bit paranoid after our discussion about GemFire/Geode being a System of Record (SoR) in certain UCs and overstepping another, perhaps preferred caching solution (e.g. Ehcache) in a Spring Boot user's application. As such I went a little nuts with the Conditional annotations on GeodeCacheConfiguration.

I suppose the following Conditional annotations would be sufficient...

@ConditionalOnClass(GemfireCacheManager.class) @ConditionalOnMissingBean(CacheManager.class)

I could maybe add a Conditional annotation on GemfireCache in SDG, but I'd rather not.

That's not what I mean. Shouldn't that condition above be @ConditionalOnBean({ CacheFactoryBean.class, GemfireCache.class,... rather than @ConditionalOnBean({ CacheFactoryBean.class, Cache.class,...

@snicoll - Alright, I see what you are saying now, but, as an FYI, I think you need to be more careful in what you mean. It is not "GemfireCache", it is "GemFireCache"

Unfortunately, there are a bunch of class names in SDG that are inappropriately named (e.g. GemfireTemplate) or confusing (e.g. GemfireCache). Personally, I like the lower case 'f' in the naming, but, it is inconsistent with GemFire/Geode, and has been that way since the project inception, so would very disrupting to users to change the class names now.

My plan is to introduce new configuration models that will allow them to rely less on the SDG API (and naming conventions; and even the GemFire/Geode API for that matter) and more on abstractions I introduce by way of Annotations, that will allow me to clean up the prior mess.

Regarding the recommendations on @ConditionalOnBean bean class references... the SDG FactoryBeans return very specific types (e.g. Cache and ClientCache). In earlier versions of the Spring Framework (possibly as recent as 4.1 and earlier) you could not refer to and inject a GemFire/Geode cache instance generically (i.e. as in GemFireCache); it simply could not resolve the bean type. In fact, I think this problem was specific to the ClientCache interface in particular. Oddly enough, the actual implementing class in GemFire/Geode is GemFireCacheImpl, which implements both Cache and ClientCache.

Then, something changed in latter versions of core Spring Framework where this started working. I suppose I can reconsider this now since Spring Boot 1.5 is based on core Spring Framework 4.3.

snicoll · 2016-11-03T09:50:34Z

.../src/test/java/org/springframework/boot/autoconfigure/cache/CacheAutoConfigurationTests.java

@@ -161,13 +173,13 @@ public void simpleCacheExplicit() {
 	@Test
 	public void simpleCacheWithCustomizers() {
 		testCustomizers(DefaultCacheAndCustomizersConfiguration.class, "simple",
-				"allCacheManagerCustomizer", "simpleCacheManagerCustomizer");
+			"allCacheManagerCustomizer", "simpleCacheManagerCustomizer");


Could you please push force an update of your PR without those formatting changes please? (there are more below)

I still see a bunch of them with formatting changes...

snicoll · 2016-11-03T09:52:37Z

spring-boot-samples/spring-boot-sample-cache/README.adoc

@@ -98,3 +99,22 @@ a redis instance with the default settings is expected on your local box).
 Simply add the `com.github.ben-manes.caffeine:caffeine` dependency to enable support
 for Caffeine. You can customize how caches are created in different ways, see
 `application.properties` for an example and the documentation for more details.
+<<<<<<< d0eedd23e9bb691dc13c54f66d08f1d29d0fb899


Merge glitch

snicoll · 2016-11-03T09:52:50Z

spring-boot-samples/spring-boot-sample-cache/README.adoc

+is configured properly. This involves creating an instance of either a Geode peer `Cache`
+or `ClientCache` depending on your topology preference and additionally creating
+and configuring Regions that will serve as the caches for your application.
+>>>>>>> Add auto-configuration support for Apache Geode as a caching provider.


Merge glitch

snicoll · 2016-11-03T09:53:11Z

...t-samples/spring-boot-sample-cache/src/main/java/sample/cache/config/GeodeConfiguration.java

+import org.springframework.context.annotation.ImportResource;
+
+/**
+ * Spring {@link Configuration @Configuration} class to configure GemFire as a caching provider.


snicoll · 2016-11-03T09:53:20Z

...t-samples/spring-boot-sample-cache/src/main/java/sample/cache/config/GeodeConfiguration.java

+/**
+ * Spring {@link Configuration @Configuration} class to configure GemFire as a caching provider.
+ * This {@link Configuration @Configuration} class is conditionally loaded based on
+ * whether GemFire is on the classpath.


snicoll · 2016-11-03T09:54:46Z

...t-samples/spring-boot-sample-cache/src/main/java/sample/cache/config/GeodeConfiguration.java

+ * @since 1.5.0
+ */
+@Configuration
+@ConditionalOnClass(name = { "org.apache.geode.cache.Cache" })


This is really unusual. I wouldn't use such condition in a sample. Since it's about importing an XML, you could just have @Import("geode.xml") commented out in the main app with a note in the readme about it.

Perhaps a comprise; I can use a @Profile instead?

I think it would be nicer for Spring Boot users if we kept the commenting and uncommenting to a minimum. Plus, there is not mention of comment and uncommenting caching sample POM file dependencies in the README.

I find it tedious to have to modify the caching sample POM file in order to comment out the current caching provider dependency and uncomment another dependency every time I want to try a different caching solution.

I have changed the sample cache POM file to use Maven profiles.

I have also changed the Geode cache sample configuration to use a Spring profile.

snicoll · 2016-11-03T09:56:01Z

spring-boot-samples/spring-boot-sample-cache/src/main/resources/geode.xml

+
+  <gfe:local-region id="countries" persistent="false"/>
+
+</beans>


Is that the only way to configure it? That is weird for us to introduce an xml config in a sample...

What? Several caching providers have an XML configuration file.

Regions are not automatically created for you in Geode, nor does SD Geode automatically create them for you, and for good reason.

Maybe I am not understanding your question.

Closes spring-projectsgh-6967

jxblum · 2016-11-04T08:21:20Z

@snicoll - Updated my PR based on your review comments. Thank you for reviewing.

I hope you like the spring-boot-samples/spring-boot-sample-cache/pom.xml file changes I made to simplify launching the SampleCacheApplication class with different caching providers using Maven profiles. Beats commenting/uncommenting IMO.

I also switched the Geode caching sample configuration to use a Spring profile (instead of the @ConditionalOnClass annotation), which the POM file automatically triggers when Geode is ran as the caching provider for the SampleCacheApplication from the command-line using the Spring Boot plugin... (e.g. $mvn -P geode spring-boot:run).

snicoll · 2016-11-05T06:47:50Z

See #5445 (comment)

wilkinsona · 2016-11-22T14:41:26Z

As discussed, this code is going to be delivered in a separate module outside of Spring Boot.

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 21, 2016

philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 19, 2016

jxblum force-pushed the autoconfigure-cache-data-geode branch from 707b636 to 7cf67db Compare November 3, 2016 09:27

jxblum added a commit to jxblum/spring-boot that referenced this pull request Nov 3, 2016

Add auto-configuration support for Apache Geode as a caching provider

7cf67db

Closes spring-projectsgh-6967

snicoll requested changes Nov 3, 2016

View reviewed changes

Add auto-configuration support for Apache Geode as a caching provider

3c28920

Closes spring-projectsgh-6967

jxblum force-pushed the autoconfigure-cache-data-geode branch from 7cf67db to 3c28920 Compare November 4, 2016 08:16

snicoll added the status: on-hold We can't start working on this issue yet label Nov 5, 2016

jxblum mentioned this pull request Nov 15, 2016

Add auto-configuration support for Spring Data Geode Repositories. #6952

Closed

wilkinsona closed this Nov 22, 2016

philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: on-hold We can't start working on this issue yet labels Nov 22, 2016

jxblum mentioned this pull request Feb 6, 2017

Improve spring-boot-sample-cache UX by using Maven profiles #8202

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add auto-configuration support for Apache Geode as a caching provider. #6967

Add auto-configuration support for Apache Geode as a caching provider. #6967

jxblum commented Sep 21, 2016

jxblum commented Nov 3, 2016 •

edited

Loading

snicoll left a comment

snicoll Nov 3, 2016

jxblum Nov 4, 2016 •

edited

Loading

snicoll Nov 3, 2016

jxblum Nov 4, 2016

snicoll Nov 5, 2016

jxblum Nov 5, 2016 •

edited

Loading

snicoll Nov 3, 2016

jxblum Nov 4, 2016

snicoll Nov 5, 2016

snicoll Nov 3, 2016

jxblum Nov 4, 2016

snicoll Nov 3, 2016

jxblum Nov 4, 2016

snicoll Nov 3, 2016

jxblum Nov 4, 2016

snicoll Nov 3, 2016

jxblum Nov 4, 2016

snicoll Nov 3, 2016

jxblum Nov 4, 2016

snicoll Nov 3, 2016

jxblum Nov 4, 2016

jxblum commented Nov 4, 2016 •

edited

Loading

snicoll commented Nov 5, 2016

wilkinsona commented Nov 22, 2016


		<gfe:local-region id="countries" persistent="false"/>

		</beans>

Add auto-configuration support for Apache Geode as a caching provider. #6967

Add auto-configuration support for Apache Geode as a caching provider. #6967

Conversation

jxblum commented Sep 21, 2016

jxblum commented Nov 3, 2016 • edited Loading

snicoll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxblum Nov 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxblum Nov 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jxblum commented Nov 4, 2016 • edited Loading

snicoll commented Nov 5, 2016

wilkinsona commented Nov 22, 2016

jxblum commented Nov 3, 2016 •

edited

Loading

jxblum Nov 4, 2016 •

edited

Loading

jxblum Nov 5, 2016 •

edited

Loading

jxblum commented Nov 4, 2016 •

edited

Loading