Upgrade backend to Spring Boot v2.2.6, Spring v5.2.5, Spring HATEOAS v1.0.3 #2720
Conversation
tdonohue
added
code task
tdonohue
force-pushed
the
fix_security_alerts_for_spring
branch
3 times, most recently
from
b9f888a
to
73e9b58
Compare
|
I reviewed and agree to perform this pull request with understanding more testing will be needed later. Travis CI status needs updated. |
...-server-webapp/src/test/java/org/dspace/app/rest/test/AbstractControllerIntegrationTest.java
Show resolved
Hide resolved
…NPath `NoClassDefFoundError`s in Spring HATEOAS during runtime. Removing from Parent POM (but leaving in dspace-server-webapp) resolves those errors.
…ATEOAS v1. Ensure any BaseObjectRest is referenced by identifier like previously.
tdonohue
force-pushed
the
fix_security_alerts_for_spring
branch
from
73e9b58
to
75d65b0
Compare
tdonohue
changed the title
|
All, this has been rebased on the latest This is ready for a re-review. |
|
I re-reviewed and changes seem good. Thank you Tim for your work on this pull request. |
There was a problem hiding this comment.
Hi @tdonohue
thanks for your effort here. I have just few inline questions, around dependencies versions that are enforced in maven modules that I don't understand if really needed (as we have fixed that in the main pom.xml file).
Another comment is about a small change in the way that the link are created via a new private getIdentifierForLink method in the Utils class.
Last but not least, currently I'm not able to run a single test class from the command line. This is not directly related to this PR as I also have the same issue with the current master. The error that I get is
[ERROR] Failed to execute goal on project dspace: Could not resolve dependencies for project org.dspace:dspace:pom:7.0-beta3-SNAPSHOT: Failure to find org.dspace:dspace-rest:jar:classes:7.0-beta3-SNAPSHOT
for example when I run
mvn test -Dtest=org.dspace.app.rest.AuthorityRestRepositoryIT -Dmaven.test.skip=false -DskipITs=false -DfailIfNoTests=false
| header (bad signature) during the tomcat startup | ||
| force the use of the previous version as the jar file | ||
| looks corrupted in the maven repository --> | ||
| <version>${spring-hal-browser.version}</version> |
There was a problem hiding this comment.
No we cannot drop this, as this dependency spring-data-rest-hal-browser is only included in the dspace-server-webapp. I just moved it's version # over to the DSpace Parent POM because it was easier to see/manage alongside the other Spring versions (the last two times I've upgraded Spring, I forgot about this separate dependency in dspace-server-webapp)
|
@abollini : I've answered all your questions about this PR. Unfortunately, there's no changes that I can make to address them, but hopefully the answers will clarify the changes I made here. With regards to issues running individual tests on commandline, as you noted that will need to be addressed in a separate PR as it seems unrelated to this PR (as it also exists on If you could give this another quick review, I'd appreciate it! Thanks! UPDATE: @abollini , I've figured out how to fix the problem running individual tests and created a separate PR to address it (as it is unrelated to this PR): #2751 |
|
Merging as this is at +3 now. Thanks all! |
tdonohue
moved this from Needs Reviewers Assigned
to Reviewer approved
in DSpace 7 Beta 3
This PR resolves two GitHub security alerts (on
master) by upgrading our dependencies to the following:MediaType.APPLICATION_JSON_UTF8(in favor of justMediaType.APPLICATION_JSON), as JSON best practices are now to only have a generic media type. See Deprecate MediaType.APPLICATION_JSON_UTF8 in favor of APPLICATION_JSON spring-projects/spring-framework#22788 (comment)This PR also includes some minor refactoring of the Parent POM to reorganize/sort the
<properties>section. This is ready for early reviews, but additional testing is still in progress.