-
Notifications
You must be signed in to change notification settings - Fork 10
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
Dependency Updates #55
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Spring Framework to 5 * Spring Data to 2.7 * Spring Security to 5 * Apache Tiles to 3 * Hibernate Validator to 5.4 * Various managed dependencies for resolving conflicts
* Remove static init so the default strategy is used * When getting SecurityContext, use the strategy holder
* Remove dead code * Fix missing arguments in logging statements * Unwrap object arrays in logging statements * Other minor api changes
* Replaces xml config with java for easier migrations * Updates to Tiles API
* Remove version where possible * Update security password encoder bean
* xmlns cleanup * add init param in order to remove servlet xml
@dbernstein there was a recent CVE published for spring so I'm going to update to 5.3.27+ shortly. Going to double check duracloud-db as well first. |
dbernstein
approved these changes
Oct 23, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Dependency Updates
Spring Data
Most of the spring data updates are pretty straightforward, moving from
findOne
tofindById
or the equivalent. Since thebyId
methods now returnOptional
s I adjusted the logic with what felt appropriate. There are a few places where I chose to stick with the old behavior by returningnull
if the id doesn't exist otherwise operations could be wrapped withifPresent
.Spring Security
The spring security changes here are pretty minimal. I had to remove the static initialization which set the holder strategy in order to get the method security to work. Other changes were just to the password encoder and using
getContextHolderStrategy
which looked to be the way of getting the SecurityContext internally.There are further security updates which can be made that I think should be handled later, such as migrating from global method security to method security. These would be done in order to make migrating to Spring 6 easier.
Config
The tiles view config moved from xml to java as the packages for the beans changed. This just made it a little easier to see which migrations needed to happen as I wasn't getting any ide hints in the xml.
The xml updates consist mostly of dropping the version from the bean uris. The only exception was the security xmlns which intellij was still yelling at me about.
Web XML
Added
init-param
to the web.xml were made in order to allow for removal of theama-servlet.xml
file.QOL
Updated logging where object arrays were used or parameters were missing
Removed some unused code
Minor API changes (e.g.
Long.valueOf
)Other Notes
I'm not sure if this will run with the current version of the
duracloud
dependency. It looks like I had been testing off ofdevelop
in that project (maybe from when I was deploying a test duracloud), but with some additional dependency updates on commons-lang3 and spring-context-support. I also bumped both dependencies to 7.2.0-SNAPSHOT in order to make sure I was pulling in the latest changes.@dbernstein