feature: upgrade to Java 21 and Spring Boot 3.5.16 (Maven + Gradle) - #68
amitmanchella-cog wants to merge 2 commits into
Conversation
…radle builds Keep pom.xml and build.gradle in sync, modernize the Gradle build (plugins block, toolchain, implementation/runtimeOnly configurations, wrapper 4.6 -> 8.14.5), replace the JdbcTemplate query overload removed-in-favor-of varargs, stop the decommissioned quote service from aborting startup, and add a context smoke test. Co-Authored-By: Amit Manchella <amit.manchella@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
||
| sourceCompatibility = 1.8 | ||
| targetCompatibility = 1.8 | ||
|
|
||
| dependencies { | ||
| compile("org.springframework.boot:spring-boot-starter-web") | ||
| testCompile("junit:junit") | ||
| implementation 'org.springframework.boot:spring-boot-starter-web' | ||
| implementation 'org.springframework.boot:spring-boot-starter-jdbc' | ||
| runtimeOnly 'org.springframework.boot:spring-boot-properties-migrator' | ||
| runtimeOnly 'com.h2database:h2' | ||
| testImplementation 'org.springframework.boot:spring-boot-starter-test' |
There was a problem hiding this comment.
🟡 The new automated test never runs in the Gradle build
The Gradle build never tells the test runner to use the JUnit 5 engine (missing useJUnitPlatform() configuration in build.gradle:25-31), so the newly added test is silently skipped when building with Gradle.
Impact: The Gradle build reports success without ever exercising the application startup smoke test, giving false confidence in the upgrade.
Gradle defaults to the JUnit 4 framework unless the test task opts into the JUnit Platform
src/test/java/hello/ApplicationTests.java uses org.junit.jupiter.api.Test (JUnit 5). Gradle 8.x (gradle/wrapper/gradle-wrapper.properties pins 8.14.5) still defaults Test tasks to the JUnit 4 framework; the JUnit Platform is only used when useJUnitPlatform() is declared, which is why Spring Initializr-generated Gradle builds always contain tasks.named('test') { useJUnitPlatform() }. The Spring Boot Gradle plugin does not configure this for you. As a result ./gradlew test finds no JUnit 4 tests and executes nothing, while mvn verify (surefire from the Boot parent) does run the test — leaving the two builds inconsistent despite the PR's goal of keeping them in agreement.
| sourceCompatibility = 1.8 | |
| targetCompatibility = 1.8 | |
| dependencies { | |
| compile("org.springframework.boot:spring-boot-starter-web") | |
| testCompile("junit:junit") | |
| implementation 'org.springframework.boot:spring-boot-starter-web' | |
| implementation 'org.springframework.boot:spring-boot-starter-jdbc' | |
| runtimeOnly 'org.springframework.boot:spring-boot-properties-migrator' | |
| runtimeOnly 'com.h2database:h2' | |
| testImplementation 'org.springframework.boot:spring-boot-starter-test' | |
| dependencies { | |
| implementation 'org.springframework.boot:spring-boot-starter-web' | |
| implementation 'org.springframework.boot:spring-boot-starter-jdbc' | |
| runtimeOnly 'org.springframework.boot:spring-boot-properties-migrator' | |
| runtimeOnly 'com.h2database:h2' | |
| testImplementation 'org.springframework.boot:spring-boot-starter-test' | |
| } | |
| tasks.named('test') { | |
| useJUnitPlatform() | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Not accurate — build.gradle already ends with exactly the suggested block:
tasks.named('test') {
useJUnitPlatform()
}The suggestion diff just stops at the dependencies block. Gradle does run the test: ./gradlew clean build writes tests="2" to build/test-results/test/*.xml, matching surefire's Tests run: 2. (Before I fixed the startup crash, ./gradlew build failed on ApplicationTests > contextLoads() FAILED, which also confirms the JUnit 5 engine is wired up.)
| @Test | ||
| void contextLoads() { | ||
| assertThat(jdbcTemplate.queryForObject("SELECT 1", Integer.class)).isEqualTo(1); | ||
| } |
There was a problem hiding this comment.
🔍 New smoke test does not exercise the CommandLineRunner logic that was changed
@SpringBootTest only builds the application context; Spring does not invoke CommandLineRunner/ApplicationRunner beans in tests (they are only called by SpringApplication.callRunners). Therefore the changed database code in Application.run(String...) (src/main/java/hello/Application.java:69-92), including the migrated jdbcTemplate.query(sql, rowMapper, "Josh") varargs call and the H2-specific DROP TABLE customers IF EXISTS / id SERIAL DDL, is never executed by the new test even though H2 moved from 1.4 to 2.x with this upgrade. Consider a @JdbcTest/direct invocation of the runner so the DDL and query path are actually validated against H2 2.x.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch on the H2 1.4 → 2.x risk. Added commandLineRunnerPopulatesCustomers, which calls application.run() and asserts the row count, so the DROP TABLE ... IF EXISTS / id SERIAL DDL, the batch insert and the varargs query all execute against H2 2.x. Both builds now report 2 tests.
Also verified out of band by running the boot jar on JDK 21: the runner logs Customer{id=3, firstName='Josh', lastName='Bloch'} / id=4 ... 'Josh Long', so the path was already working — the test just locks it in.
| RestTemplate restTemplate = new RestTemplate(); | ||
| Quote quote = restTemplate.getForObject("http://gturnquist-quoters.cfapps.io/api/random", Quote.class); | ||
| log.info(quote.toString()); | ||
| logRandomQuote(new RestTemplate()); |
There was a problem hiding this comment.
📝 Info: Quote is fetched twice at startup, once with a non-managed RestTemplate
main still calls logRandomQuote(new RestTemplate()) directly after the context has started, while the CommandLineRunner bean (src/main/java/hello/Application.java:60-61) already performs the same fetch using the Spring-managed RestTemplate. The refactor preserved this pre-existing duplication, so the (now decommissioned) endpoint is hit twice and one of the calls bypasses any builder-configured settings such as timeouts and interceptors. Collapsing to the single runner-based call would be cleaner.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct, the duplication predates this PR — main fetching a quote and the CommandLineRunner doing the same is how the upstream guide sample was written, so I kept both call sites rather than change the demo's shape in an upgrade PR. Collapsing to the runner-only call is a one-line follow-up if @amitmanchella-cog wants it.
| compile("org.springframework.boot:spring-boot-starter-web") | ||
| testCompile("junit:junit") | ||
| implementation 'org.springframework.boot:spring-boot-starter-web' | ||
| implementation 'org.springframework.boot:spring-boot-starter-jdbc' |
There was a problem hiding this comment.
📝 Info: properties-migrator kept as a permanent runtime dependency
spring-boot-properties-migrator is documented as a temporary aid to be removed once the upgrade is complete; keeping it as a permanent runtime/runtimeOnly dependency in both pom.xml and build.gradle:28 adds startup overhead and ships in the boot jar. Since this PR completes the 2.x -> 3.x migration and the repo has essentially no application properties (application.properties only contains a commented-out line and it sits at the repo root rather than src/main/resources), it could be dropped.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed — this PR finishes the 2.x → 3.x migration and there are no properties to migrate, so spring-boot-properties-migrator is dropped from both pom.xml and build.gradle.
| private static void logRandomQuote(RestTemplate restTemplate) { | ||
| try { | ||
| Quote quote = restTemplate.getForObject(QUOTE_URL, Quote.class); | ||
| log.info(String.valueOf(quote)); | ||
| } catch (RestClientException e) { | ||
| log.warn("Could not fetch a quote from {}: {}", QUOTE_URL, e.getMessage()); | ||
| } |
There was a problem hiding this comment.
📝 Info: Quote failures are now swallowed and only warned about
Catching RestClientException covers connection/DNS failures (ResourceAccessException) and HTTP error statuses, which is the stated intent. Note the side effect that any genuine misconfiguration of the endpoint later on will also be silently degraded to a warning, and String.valueOf(quote) will log the literal "null" if the service ever returns an empty body (2xx with no content), instead of throwing. Acceptable for a demo, but worth noting if this call site is ever made meaningful.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — the degradation is deliberate: this is a demo call against a host that no longer exists, and the alternative is the app refusing to start. If the quote fetch ever becomes meaningful it should fail loudly instead.
…migrator The smoke test now invokes Application.run() so the H2 2.x DDL and the varargs JdbcTemplate query are covered; spring-boot-properties-migrator is a temporary upgrade aid and the 2.x -> 3.x migration is complete. Co-Authored-By: Amit Manchella <amit.manchella@cognition.ai>
Summary
Upgrades both build configurations from Java 8 / Spring Boot 2.0.2.RELEASE to Java 21 / Spring Boot 3.5.16, keeping
pom.xmlandbuild.gradlein agreement (same Boot version, same JDK, same dependency set). Bothmvn clean verifyand./gradlew clean buildpass, and the boot jar was run on JDK 21 to confirm the app actually starts and serves.No Jakarta migration was needed: the sources use no
javax.*imports.Maven — also flips
<packaging>pom</packaging>→jar. Withpompackaging Maven never compiledsrc/main/javaat all, somvn verifywas green while proving nothing; the build now actually compiles and repackages a boot jar.spring-boot-properties-migratoris dropped from both builds — it is a temporary upgrade aid and this PR completes the migration.Gradle — the
buildscript/apply pluginform and thecompile/testCompileconfigurations were removed in Gradle 7, so the file is restructured:Gradle wrapper 4.6 → 8.14.5 (4.6 cannot run on Java 21, let alone load the Boot 3 plugin).
Source compatibility fixes
JdbcTemplate.query(sql, Object[], RowMapper)is deprecated in Spring 6; switched to the varargs overloadquery(sql, rowMapper, "Josh").gturnquist-quoters.cfapps.iohas been decommissioned — thegetForObjectcall inmainand in theCommandLineRunnerbean threwResourceAccessExceptionand aborted the context. Both call sites now go through onelogRandomQuote(RestTemplate)helper that logs a warning instead of failing. Happy to instead point it at a live quote service or drop the demo call entirely — say the word.Tests / repo hygiene
ApplicationTests(the only tests in the repo): a context/DataSourcesmoke test, plus one that invokesapplication.run()so the H2-specific DDL (DROP TABLE ... IF EXISTS,id SERIAL) and the migrated varargs query are exercised against H2 2.x rather than assumed to work.target/**/*.classartifacts were tracked in git; untracked them and added/target/,/build/,/.gradle/to.gitignore..idea/alone — it still references the 2.0.2 jars, but it is already gitignored.Note for future sessions: the environment blueprint pinned
JAVA_HOMEto JDK 17, which failsrelease 21; a blueprint update to JDK 21 has been suggested separately.Link to Devin session: https://app.devin.ai/sessions/ad433093e17249f38aa04b275f7e9f3f
Requested by: @amitmanchella-cog
Devin Review