Reinstate support for Ehcache 3 #30002

chrisdennis · 2022-02-26T21:09:13Z

This is a prototype of the restoration of Ehcache 3 support to Spring Boot. Ehcache uses Gradle feature variants (with appropriate capabilities) to model the split "Jakarta/Java EE" landscape. These are exposed in Maven as artifacts with 'jakarta' classifiers with appropriate optional dependencies. I've therefore implemented a solution to #29298 as a prerequisite here.

Currently this is depending on an alpha release of Ehcache 3.10, hence it's only a draft PR, but I wanted to get this out there for two reasons:

To confirm the acceptability of the classifier/feature variant approach for Jakarta support.
To understand the need for the ehcache-transactions module. The transactions module allows Ehcache instances to be exposed as XA resources and participate in 2PC managed by a JTA transaction manager. It's not clear to me how this relates to Springs caching support - I'm not sure how or why a user would need this dependency in a Spring (Boot) context.

pivotal-cla · 2022-02-26T21:09:15Z

@chrisdennis Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

wilkinsona · 2022-02-28T10:39:05Z

Thanks, @chrisdennis.

I've therefore implemented a solution to #29298 as a prerequisite here.

Thanks for this. Unfortunately, it doesn't quite meet the requirements that we have for some of Kafka's modules. I implemented something earlier this year which should work for both cases, I just haven't had a chance to merge it yet.

To confirm the acceptability of the classifier/feature variant approach for Jakarta support

I don't think we have any dependencies that have taken this approach just yet, but I don't think it'll be a problem.

To understand the need for the ehcache-transactions module

Spring Boot supports JTA-based transaction coordination either by retrieving the UserTransaction and TransactionManager from JNDI when deployed as a war file in an application server, or via integration of a third-party transaction manager such as Narayana. We need to position Spring Boot 3.0 such that this will work in both cases with Jakarta EE 9. That should include cases where a user wants to enlist their cache as a resource.

chrisdennis · 2022-03-04T20:17:57Z

@pivotal-cla This is an Obvious Fix

This reinstates previously existing code and introduces no new IP.

wilkinsona · 2022-03-04T20:36:03Z

@chrisdennis You’ll need to sign the CLA, I’m afraid. The changes proposed here are not considered to be obvious as they will result in a change in functionality.

chrisdennis · 2022-03-04T20:51:01Z

@wilkinsona no problem... I figured I would see what you guys thought before I went ahead and engaged with the bureaucracy on this end.

FYI: the alpha release referenced here is likely very close to what will become the 3.10.0 GA release. That should happen next week. Once I have that out I'll update the deps here and remove the draft status from this PR.

pivotal-cla · 2022-03-04T21:27:26Z

@chrisdennis Thank you for signing the Contributor License Agreement!

wilkinsona

Thanks for the updates, @chrisdennis. I've left a few comments for your consideration when you have a minute.

wilkinsona · 2022-03-07T14:46:16Z

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

@@ -81,7 +86,10 @@
 * @author Ryon Day
 */
 @ClassPathExclusions("hazelcast-client-*.jar")
-class CacheAutoConfigurationTests extends AbstractCacheAutoConfigurationTests {


I can't see the connection between this change (and deleting AbstractCacheAutoConfigurationTests) and reinstating support for EhCache 3. I'd prefer to make the changes as minimal as possible and leave this as-is if we can.

My understanding is that AbstractCacheAutoConfigurationTests was only introduced to allow the Ehcache tests to have a @ClassPathExclusions annotation on the Ehcache 2 JARs. The bug that caused that to be necessary was fixed in Ehcache 2 a while back... and regardless Ehcache 2 I assume isn't part of Spring Boot 3.x?

Unfortunately, I can't recall the exact reason. Folding everything back into a single class is a good thing to do if we can, but it's the sort of thing that we prefer to do separately. If, as you suspect, a bug's been fixed in EhCache 2 such that the separate classes are no longer necessary we can make that change in all our currently maintained branches and not just in main.

If I recall correctly without digging through the issue-tracker we fixed this in Ehcache 3 by shading and relocating some library version clashes that meant the two couldn't co-exist. Regardless, I'll back these changes out of this PR.

wilkinsona · 2022-03-07T14:47:13Z

...a/org/springframework/boot/autoconfigure/orm/jpa/Hibernate2ndLevelCacheIntegrationTests.java

-	void hibernate2ndLevelCacheWithJCacheAndHazelcast() {
-		String cachingProviderFqn = HazelcastServerCachingProvider.class.getName();
-		String configLocation = "classpath:hazelcast.xml";
+	void hibernate2ndLevelCacheWithJCacheAndEhCache3() {
+		String cachingProviderFqn = EhcacheCachingProvider.class.getName();
+		String configLocation = "ehcache3.xml";


I think this could be left as it was before. We don't have to stop using Hazelcast here in order to restore support for EhCache.

I started out here with a simple revert of the commit that removed Ehcache 3 support. That's why this came in. I can remove as you wish.

revert Hibernate2ndLevelCacheIntegrationTests

Yes, please.

wilkinsona · 2022-03-07T14:47:43Z

spring-boot-project/spring-boot-dependencies/build.gradle

+					"ehcache-transactions" {
+						classifier = 'jakarta'
+					},
+					"ehcache-clustered"


We try to list modules in alphabetical order.

modules should be in alphabetical order

wilkinsona · 2022-03-07T14:47:56Z

spring-boot-project/spring-boot-docs/src/docs/asciidoc/documentation/io.adoc

@@ -2,7 +2,7 @@
 == IO
 If your application needs IO capabilities, see one or more of the following sections:

-* *Caching:* <<io#io.caching, Caching support with Hazelcast and more>>
+* *Caching:* <<io#io.caching, Caching support EhCache, Hazelcast and more>>


Could we keep the "with" here please.

Artifact of the initial revert.

restore correct grammar in io.adoc

chrisdennis · 2022-03-07T16:01:25Z

Do you prefer a separate commit for review changes, or a force push?

wilkinsona · 2022-03-07T21:21:37Z

Either's fine. Thank you. We'll squash multiple commits into one as part of merging so a force-push is probably slightly easier to process.

wilkinsona

This looks great now, @chrisdennis. Thank you.

See gh-30002

snicoll · 2022-03-19T08:47:27Z

@chrisdennis thank you for the follow-up and for making your first contribution to Spring Boot.

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 26, 2022

chrisdennis force-pushed the ehcache-restore branch from 3b37102 to e3329f4 Compare March 4, 2022 20:14

chrisdennis force-pushed the ehcache-restore branch from e3329f4 to cfe0d49 Compare March 4, 2022 21:27

wilkinsona requested changes Mar 7, 2022

View reviewed changes

chrisdennis force-pushed the ehcache-restore branch from cfe0d49 to 160c94a Compare March 9, 2022 19:27

[DRAFT] Restore Ehcache 3 Support

748b4dd

chrisdennis force-pushed the ehcache-restore branch from 160c94a to 748b4dd Compare March 11, 2022 19:26

wilkinsona self-requested a review March 15, 2022 19:20

wilkinsona approved these changes Mar 15, 2022

View reviewed changes

chrisdennis marked this pull request as ready for review March 17, 2022 21:13

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

snicoll added this to the 3.0.0-M2 milestone Mar 19, 2022

snicoll self-assigned this Mar 19, 2022

snicoll pushed a commit that referenced this pull request Mar 19, 2022

Restore Ehcache 3 Support

3ed3d3e

See gh-30002

snicoll closed this in bd05100 Mar 19, 2022

snicoll changed the title ~~Restore Ehcache 3 Support~~ Reinstate support for Ehcache 3 Mar 19, 2022

Reinstate support for Ehcache 3 #30002

Reinstate support for Ehcache 3 #30002

Conversation

chrisdennis commented Feb 26, 2022

Uh oh!

pivotal-cla commented Feb 26, 2022

Uh oh!

wilkinsona commented Feb 28, 2022

Uh oh!

chrisdennis commented Mar 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilkinsona commented Mar 4, 2022

Uh oh!

chrisdennis commented Mar 4, 2022

Uh oh!

pivotal-cla commented Mar 4, 2022

Uh oh!

wilkinsona left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisdennis Mar 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisdennis commented Mar 7, 2022

Uh oh!

wilkinsona commented Mar 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilkinsona left a comment

Choose a reason for hiding this comment

Uh oh!

snicoll commented Mar 19, 2022

Uh oh!

Uh oh!

chrisdennis commented Mar 4, 2022 •

edited

Loading

chrisdennis Mar 7, 2022 •

edited

Loading

wilkinsona commented Mar 7, 2022 •

edited

Loading