DATAGO-59967: Move the EMA to JDK 17 + Spring 3.1.2 #112
Conversation
…he /scan REST endpoint
| @@ -13,12 +13,14 @@ | |||
| <exclude name="DoubleBraceInitialization"/> | |||
| <exclude name="GuardLogStatement"/> | |||
| <exclude name="JUnit4TestShouldUseBeforeAnnotation"/> | |||
| <exclude name="JUnit5TestShouldBePackagePrivate"/> | |||
There was a problem hiding this comment.
This seems to be new to Junit5. If we decide we want to go in this direction, then we can create a Jira to update our tests to be package private. For now, disabling to maintain parity.
| @@ -90,6 +92,7 @@ | |||
| <exclude name="EmptyCatchBlock"/> | |||
| <exclude name="MissingSerialVersionUID"/> | |||
| <exclude name="UseLocaleWithCaseConversions"/> | |||
| <exclude name="TestClassWithoutTestCases"/> | |||
There was a problem hiding this comment.
We use inheritance to reduce the amount of code, but this means some of the top-level classes have no tests. I don't agree with this "test classes must have test cases" rule and also want to maintain parity so I'm excluding it for now.
| @@ -95,18 +98,17 @@ private List<Map<String, Object>> getResultsListMapFromSemp(String uriPath, | |||
| try { | |||
| getSempListRequest(sempObject, createUriBuilderFunction(uriPath, substitutionMap, selectFields)); | |||
| } catch (WebClientResponseException ex) { | |||
| switch (ex.getStatusCode()) { | |||
There was a problem hiding this comment.
The switch no longer works in JDK17. Re-wrote it as an if statement.
| @@ -16,7 +14,7 @@ public CamelContextConfiguration contextConfiguration() { | |||
| @Override | |||
| public void beforeApplicationStart(CamelContext context) { | |||
| context.setUseMDCLogging(true); | |||
| context.adapt(ExtendedCamelContext.class).setUnitOfWorkFactory(CustomUnitOfWork::new); | |||
There was a problem hiding this comment.
Removed this since it wasn't actually changing the MDC properties.
| @@ -16,7 +14,7 @@ public CamelContextConfiguration contextConfiguration() { | |||
| @Override | |||
| public void beforeApplicationStart(CamelContext context) { | |||
| context.setUseMDCLogging(true); | |||
| context.adapt(ExtendedCamelContext.class).setUnitOfWorkFactory(CustomUnitOfWork::new); | |||
| context.setStreamCaching(false); | |||
There was a problem hiding this comment.
This is to maintain feature parity with our previous camel version (3.16.0). In 3.16.0, stream caching was disabled by default. If we decide we want to enable it then we can create a Jira to determine the proper stream caching parameters.
| @@ -44,7 +45,10 @@ public class AsyncRoutePublisherImplTests { | |||
| public void testStart() { | |||
| AsyncWrapper asyncWrapper = mock(AsyncWrapper.class); | |||
|
|
|||
| Exchange exchange = ExchangeBuilder.anExchange(mock(ExtendedCamelContext.class)) | |||
| CamelContext cc = mock(CamelContext.class); | |||
There was a problem hiding this comment.
ExtendedCamelContext previously extend CamelContext. In the 3 -> 4 camel migration they broker this, so now the ExtendedCamelContext is a member of the CamelContext. Needed to update the tests to reflect this.
| <exclude name="JUnitAssertionsShouldIncludeMessage"/> | ||
| <exclude name="JUnitTestContainsTooManyAsserts"/> | ||
| <exclude name="JUnitTestsShouldIncludeAssert"/> | ||
| <exclude name="PreserveStackTrace"/> | ||
| <exclude name="SwitchStmtsShouldHaveDefault"/> | ||
| <exclude name="UseVarargs"/> | ||
| <exclude name="LiteralsFirstInComparisons"/> |
There was a problem hiding this comment.
This rule makes no sense. It was picking up code like this:
assertTrue(thrown.getMessage().contentEquals("Some value"));
There is no opportunity to do a literal first comparison in this case.
Excluding.
| @@ -135,6 +139,46 @@ public void badConnectionUrlTest() throws IOException { | |||
| assertEquals("Could not construct URI from worst url ever", exception.getMessage()); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
Added coverage for the if check above.
|
Not sure why SonarQube is failing. I have 100% coverage in the file in question when running locally. |
I'll take a look on Monday. |
|
SonarQube Quality Gate
|
What is the purpose of this change?
There are high vulnerabilities in the dependencies of Spring 2.7.6 that we cannot fix without upgrading Spring. We decided to go to Spring 3.1.2 and as part of the change, we need to upgrade to JDK 17.
This PR is an attempt to maintain EMA feature parity while updating our dependencies to newer versions.
How was this change implemented?
After updating the Spring libraries from 2.7.6 to 3.1.2 I also needed to update other dependencies. For instance, Camel went from 3.16.0 to 4.0.
Part of the move also involved changing the javax package to jakarta.
How was this change tested?
A full regression on the EMA:
Kafka broker scan
Confluent schema registry scan
Solace broker scan
online scan from EP
offline scan from REST
offline get zip file from REST
online push scan to EP from REST
Is there anything the reviewers should focus on/be aware of?
No