Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump ehcache to version 3 #12526

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

sdudzin
Copy link

@sdudzin sdudzin commented Apr 9, 2024

Pull Request Checklist

Helpful things

Purpose

Upgrades Ehcache to version 3 (version 2 contains security vulnerabilities).

Background Context

This is a bit 'naive' implementation but it does not break the Play API which I think is more important. One big disadvantage is the ehcahce xml configuration which force use of Play API as cache item.

@mkurz
Copy link
Member

mkurz commented Apr 9, 2024

I am pretty sure this needs more work than just bumping the version.

@sdudzin
Copy link
Author

sdudzin commented Apr 9, 2024

I am pretty sure this needs more work than just bumping the version.

Yes this is also my assumption. However to my surprise compilation did not fail for me and I was not able to run tests (I followed the manual on building), there must be a problem with my environment. So, I opened a draft request to see what checks will fail. I hope this is not much trouble?

@mkurz
Copy link
Member

mkurz commented Apr 9, 2024

I am pretty sure this needs more work than just bumping the version.

Yes this is also my assumption. However to my surprise compilation did not fail for me and I was not able to run tests (I followed the manual on building), there must be a problem with my environment. I also opened a draft request to see what checks will fail. I hope this is not much trouble?

Not sure what you did locally, but just changing the version number will definitely not work, because Ehcache 3 is hosted under a different groupId than Ehcache 2 (org.ehcache vs net.sf.ehcache), see https://www.ehcache.org/ ("Ehcache 3 Quick Start" vs "Ehcache 2.x Quick Start") and https://repo1.maven.org/maven2/net/sf/ehcache/ehcache/ vs https://repo1.maven.org/maven2/org/ehcache/ehcache/

@sdudzin
Copy link
Author

sdudzin commented Apr 9, 2024

I apologize for the noise. I am ok with closing this PR until it's ready.

@mkurz
Copy link
Member

mkurz commented Apr 9, 2024

I apologize for the noise. I am ok with closing this PR until it's ready

You can keep it open as draft an push every now and then to validate your changes. However if you don't plan to work on it or give up at some point you probably want to close it.

@sdudzin sdudzin marked this pull request as ready for review May 3, 2024 14:02
@sdudzin
Copy link
Author

sdudzin commented May 3, 2024

@mkurz , I have made a few changes, please have a look.

@mkurz mkurz added this to the 2.10.0 / 3.1.0 milestone May 3, 2024
@mkurz
Copy link
Member

mkurz commented May 3, 2024

@sdudzin Thanks! Can you rebase against latest main branch and also squash your commits? It does not have to be just one commit, but having commits like "format", "codestyle fixes", etc. are not so nice. You can keep commits for "documentation" and others where it makes sense.
Thanks!

@sdudzin
Copy link
Author

sdudzin commented May 3, 2024

@mkurz I rebased and squashed the commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

None yet

2 participants