Upgrade Spring boot To 4.0.3#227
Conversation
…led annotation and associated ticket number
…les for upgrade to Spring
…@KafkaListener
…@KafkaListener
…ountDownLatch for message validation
…ad of EmbeddedKafkaZKBroker for transition to kraft instead of zookeeper
…er, remove EmbeddedKafkaHolder usage
…er, remove EmbeddedKafkaHolder usage
…ner, replace EmbeddedKafkaHolder with CountDownLatch for async validation
…ner, replace EmbeddedKafkaHolder with CountDownLatch for async validation
…er, replace EmbeddedKafkaHolder with CountDownLatch for async validation
…ner, replace EmbeddedKafkaHolder with CountDownLatch for async validation
…@KafkaListener, replace EmbeddedKafkaHolder with CountDownLatch for async validation
…@KafkaListener, remove EmbeddedKafkaHolder usage
…ner in all tests, refactor tests to use CountDownLatch for async validation
…er, remove EmbeddedKafkaHolder usage
…ing package changes
…CertPresent field
…ntrollerTest and change order of creation of consumers
…rConfigTest and update topic handling logic
…on of isCertPresent field" This reverts commit 65a81c0.
… to accessors" This reverts commit 9b8242e.
…s and revert lombok.config settings
| @@ -1,6 +1,5 @@ | |||
| package us.dot.its.jpo.ode.udp.generic; | |||
|
|
|||
| import io.netty.handler.codec.UnsupportedMessageTypeException; | |||
There was a problem hiding this comment.
After upgrading to springboot 4, this package is no longer included and we only use the import here. This is also the only place that throws and catches this exception since the method that is throwing it is a private method and the method that catches it just logs that there was an unsupported message. Instead of including it back into the project, I adjusted the class to throw and subsequentially catch a custom UnsupportedMessageTypeException. I'm not sure if we wanted to switch this to an IllegalArgumentException instead, but I felt it doesn't really fit the situation a 100 percent.
| vmImage: ubuntu-latest | ||
|
|
||
| steps: | ||
| - checkout: self |
There was a problem hiding this comment.
This change is not related to the spring upgrade, but is instead needed to be able to build and deploy the ode.
…s and JavaBeans naming convention workaround
mcook42
left a comment
mcook42
left a comment
There was a problem hiding this comment.
Looks really great so far! A few chores, but otherwise I'm stoked
iyourshaw
left a comment
iyourshaw
left a comment
There was a problem hiding this comment.
Will look at this in more detail, but to start with, a question on first look:
There appear to be some fairly extensive refactorings done to many of the unit tests. Are all those test refactorings necessary because of the Spring and Kafka version upgrades? Or are there additional refactorings there for other purposes that aren't described in the scope of this PR?
There was a problem hiding this comment.
Overall it looks good!
Just one important documentation issue.
I tested this out and verified that the unit tests all pass.
I did some interoperability testing too, and there is an issue with compatibility with other applications that consume the library JARs produced by this app. This includes jpo-geojsonconverter, jpo-conflictmonitor, and jpo-cvmanager. For example jpo-geojsonconverter has dependencies on the jpo-ode-core and jpo-ode-plugins JARs.
Compatibility was tested as follows:
In a checked-out copy: changed the version number in this PR to a unique value, and changed the dependency version to match in the cdot/develop branch of jpo-geojsonconverter, to force it to pick up the library built from this PR from local maven on the test machine.
Then built this PR branch and deployed the JARs to maven local via:
ode: mvn clean install
and built the geojson converter with and without running tests:
gjc: mvn clean package -DskipTests
That succeeds: It is possible to compile the GJC using the JAR from this PR
However when running the GCJ tests:
gjc: mvn clean test
there are a number of test errors. In fact, all the @SpringBootTest-annotated unit tests in the GeoJSON converter fail when using the library JARs from this PR. This makes sense, but it should be noted in the description for this PR, and in the documentation and compatibility matrix that this update is a breaking change for downstream applications that depend on it, and the major version number should be incremented when it is released.
A library consumer (gjc) unit test error example is attached
us.dot.its.jpo.geojsonconverter.converter.bsm.BsmTopologyTest.txt
…PATJsonRouterTest
4 participants
PR Details
Description
This PR upgrades the spring boot version to 4.0.3. This upgrade DOES NOT include the optional upgrade to Jackson 3. This is because Jackson 3 mappers cannot map abstract classes so it causes issues when trying to serialize or deserialize certain classes. In addition, since Jackson 2 is still supported, it didn't fall into scope and caused a bigger lift than expected. This upgrade does include all the other upgrades found here: https://github.com/spring-projects/spring-boot/wiki/Spring-Boot-4.0-Release-Notes. Most notable is the Kafka upgrade to 4.
Kafka tests were updated to use @embeddedkafka and @KafkaListener to allow kafka the ability to manage the tests more similarly to how it is done when the system is running and removed the manual wiring.
Related Issue
https://dev.azure.com/SOC-OIT/CDOT/_workitems/edit/471731/
Motivation and Context
The current version is set to have an EOS by 6/26 (https://spring.io/projects/spring-boot#support).
How Has This Been Tested?
mvn test was ran to verify functionality had not changed.
It was also tested locally running docker compose up --build -d. I ran the scripts included in the scripts folder and compared what the develop branch produced to the given streams to what the updates produced to the given streams to ensure the messages were unchanged.
A deployment to dev was made to ensure new configurations worked properly. After it was deployed, I went through messages in the kafka streams before and after deployment to ensure the messages formats remained unchanged after the Spring upgrade and that it was still able to receive messages.
Types of changes
Checklist:
ODE Contributing Guide