-
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
Update to Spring Boot 2.1.5. #3
Conversation
* Update Gradle to version 5 * EnvironmentTestUtils.addEnvironment has been deprecated in favour of TestPropertyValues * Bean overriding has been disabled by default in Spring Boot 2.1 * Breaking change in Lombok v1.16.20 where Lombok no more adds annotation @ConstructorProperties by default, breaking the immutable bean deserialization we have in tests. * HttpServerErrorException has now a hierarchy of exception classes. * A lot of changes in Spring Jackson library causing some weird failure in deserializing wrapped Hateoas Resources type inside Optional response type.
Hi @polysantiago! Have you had change to look at my PR? Thanks :) |
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.
Great PR, Marika! I added some comments we need to address before merging. But it looks good overall!
build.gradle
Outdated
@@ -44,7 +44,7 @@ repositories { | |||
|
|||
dependencyManagement { | |||
imports { | |||
mavenBom 'io.spring.platform:platform-bom:Brussels-SR2' | |||
mavenBom 'org.springframework.boot:spring-boot-dependencies:2.1.5.RELEASE' |
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.
Why using version 2.1.5
? Is there any specific reason? The latest is 2.24.1
.
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.
Well, for egoistic reasons mainly, it's the version our application uses :) But I guess there's no reason to not use the latest and greatest. Where did you find the latest version to be 2.24.1? I tried checking the Maven central and there it seems to be 2.1.6 but maybe I'm missing something...
https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-dependencies
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.
Ah, nevermind. I was looking at the wrong dependency. You're right. Latest is 2.1.6
@@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
zipStoreBase=GRADLE_USER_HOME | |||
zipStorePath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-3.5-bin.zip | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-5.0-bin.zip |
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.
Is this needed as part of the PR? If not, I can upgrade it myself to the latest version (5.5.1
) as part of another PR.
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.
I think some Gradle version upgrade is needed for Spring Boot 2 (version 4 possibly?)...at least it made it easier to update the project having version 5. I can also change the version to 5.5.1 while I'm at it.
Or if you want to have it as separate PR, I can also pull your changes once you're done?
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.
Yup. Let's do that. I'll upgrade Gradle separately.
src/test/java/io/github/polysantiago/spring/rest/RestClientHateoasTest.java
Outdated
Show resolved
Hide resolved
@mmorsky I have pushed a new branch |
Update Gradle to version 5
EnvironmentTestUtils.addEnvironment has been deprecated in favour of TestPropertyValues
With Spring Boot 2.1 the bean overriding has been disabled by default. Seems like we have been relying on this feature with at least the RestClientSpecification bean, so I needed to enable it with configuration property "allow-bean-definition-overriding" since I'm not sure whether that was done on purpose or not. https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-2.1-Release-Notes
There was a breaking change in Lombok v1.16.20 where by default Lombok does not add the @ConstructorProperties annotation to constructors anymore. This breaks with the immutable bean deserialization, so we need to configure Lombok to generate this annotation on constructors.
HttpServerErrorException has now a hierarchy: Exception hierarchy under HttpClientException and HttpServerException for the RestTemplate [SPR-15404] spring-projects/spring-framework#19967
Something has changed in the Jackson-Spring library, not exactly sure what, but I ran into a failing tests case testOptionalCollectionResource_found. Basically the test was failing cause Jackson didn't know how to deserialize the Hateoas type inside the Optional. Don't ask why or how, but for some reason the objectMapper which was used for the deserialization of the response did not contain the Jackson2HalModule. So I basically "fixed" it by configuring the test class objectMapper to contain all needed modules. I'd love to know what really broke the test case, but it got too complicated for me to follow the deserialization configuration. So if you have a better solution you come across, I'd love to try that.