Upgrade build to Java 17 (Spring Boot 2.7.18, Gradle 7.6.4) - #62
tobydrinkall wants to merge 1 commit into
Conversation
Co-Authored-By: Toby Drinkall <toby.drinkall@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:
|
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-parent</artifactId> | ||
| <version>2.0.2.RELEASE</version> | ||
| <version>2.7.18</version> |
There was a problem hiding this comment.
🔍 Boot BOM bump pulls H2 2.x, whose SQL grammar may reject the legacy DDL in Application
The parent bump from 2.0.2.RELEASE to 2.7.18 also moves the managed com.h2database:h2 version from 1.4.19x to 2.1.x. hello/Application.java:66-67 issues legacy DDL (DROP TABLE customers IF EXISTS with trailing IF EXISTS, and the SERIAL column type) that H2 2.x tightened/removed in its grammar cleanup. Because these run from a CommandLineRunner, a syntax error would abort the application right after startup. The PR's verification narrative reports the app dying in the other runner (UnknownHostException from the gturnquist-quoters.cfapps.io call at hello/Application.java:39 and hello/Application.java:52-53), so the JDBC path may never have been exercised on the new stack. Worth explicitly running the app with a reachable/stubbed quote endpoint (or pinning <h2.version> to 1.4.200) to confirm the DDL still parses.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The JDBC path was exercised — H2 resolves to 2.1.214 on the new BOM (mvn dependency:tree → com.h2database:h2:jar:2.1.214:compile) and both the legacy DROP TABLE customers IF EXISTS and the SERIAL column still parse. Runtime log from the boot run described in the description (the quote endpoint was stubbed locally, so Application.run completed before the other runner failed):
Creating tables
Inserting customer record for John Woo / Jeff Dean / Josh Bloch / Josh Long
Querying for customer records where first_name = 'Josh':
Customer{id=3, firstName='Josh', lastName='Bloch'}
Customer{id=4, firstName='Josh', lastName='Long'}
So no <h2.version> pin is needed.
| testCompile("junit:junit") | ||
| implementation("org.springframework.boot:spring-boot-starter-web") | ||
| implementation("org.springframework.boot:spring-boot-starter-jdbc") | ||
| runtimeOnly("com.h2database:h2") |
There was a problem hiding this comment.
📝 Info: Gradle declares H2 runtimeOnly while Maven keeps it on the compile classpath
pom.xml:31-34 declares com.h2database:h2 with default (compile) scope, whereas the Gradle build uses runtimeOnly. No source references H2 classes directly, so both builds work, but the two build definitions are no longer symmetric; making the Maven dependency <scope>runtime</scope> would keep them aligned and prevent accidental compile-time coupling to H2.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
True. H2's compile scope in pom.xml is pre-existing, so I left it alone to keep the diff to the upgrade; nothing imports H2 classes, so it's inert either way. Say the word and I'll add <scope>runtime</scope> to align the two builds.
| <groupId>org.springframework</groupId> | ||
| <artifactId>gs-spring-boot</artifactId> | ||
| <packaging>pom</packaging> | ||
| <packaging>jar</packaging> |
There was a problem hiding this comment.
📝 Info: Switching to jar packaging now ships spring-boot-properties-migrator inside the executable jar
With pom packaging no artifact was produced, so spring-boot-properties-migrator was effectively inert. Now that a repackaged jar is built, the migrator is bundled as a runtime dependency of the shipped application. Spring's guidance is to remove it once the migration is complete, since it scans and rewrites properties on every startup; it is also absent from the Gradle build, so the two builds produce different runtime classpaths.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct, and worth deciding on. It resolves on 2.7.18 and reports nothing at startup (the only property file here is a fully commented-out server.port), so per the upgrade brief I kept it rather than silently dropping a declared dependency. I'm happy to remove it in a follow-up commit — it's a migration aid with nothing left to migrate — just want the owner's call since deleting a dependency is a scope change.
| java { | ||
| toolchain { | ||
| languageVersion = JavaLanguageVersion.of(17) | ||
| } | ||
| } |
There was a problem hiding this comment.
📝 Info: Toolchain replaces source/targetCompatibility, so builds now hard-fail without a local JDK 17
Gradle 7.6 has no built-in toolchain provisioning resolver plugin configured here, so if the machine running the build has no JDK 17 installed (and auto-download is not enabled), the build fails with "No matching toolchains found" rather than falling back to the current JVM — a behavioral difference from the previous sourceCompatibility/targetCompatibility settings. Fine for local development on 17, but worth noting if this build ever runs on an environment that only provides a different JDK.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed. Deliberate: a toolchain pins the compile target instead of silently compiling with whatever JVM Gradle runs on, which is what the upgrade brief asked for. There is no CI here, so the only consumer is local dev; toolchain auto-provisioning (foojay-resolver) can be added if this ever builds somewhere without a JDK 17.
Runtime verification of the Java 17 upgrade ✅Built and booted both jars on JDK 17.0.13 and exercised the whole REST API end-to-end. Devin session: https://app.devin.ai/sessions/06b09547bfc84eeeab26864fb8daa79e REST API works on the upgraded runtime (Maven jar, Spring Boot 2.7.18 / Java 17)
Also verified: Bytecode + boot evidenceThe H2 risk of this upgrade is clear: the legacy Gradle jar parityThe Gradle jar boots with the same banner/JDK/H2 output and returns byte-identical JSON for Adversarial inputs — and two pre-existing bugs (not regressions)No response leaked a stack trace. The two 500-ish issues exist on Caveat: the Chrome automation crashed late in the run, so |
Summary
Moves both build systems from Java 8 to Java 17. No source changes were needed — the Java 8 idioms in this repo (Streams,
Files.walk/Files.find,java.time,AtomicLong) all compile unchanged on 17.Maven (
pom.xml)spring-boot-starter-parent2.0.2.RELEASE → 2.7.18 (last 2.x line; Java 17 compatible and stilljavax.*, so no Jakarta migration).<java.version>1.8</java.version>→17, plus an explicit<maven.compiler.release>17</maven.compiler.release>so the bootclasspath is pinned rather than justsource/target. Verified:mvn help:evaluate -Dexpression=maven.compiler.release→17, and emitted classes are class-file major version 61.<packaging>pom</packaging>→jar. Withpompackaging Maven never rancompiler:compileon the 13 sources here, somvn verifywas green while validating nothing about the Java level (and produced no artifact despitespring-boot-maven-pluginbeing configured). Flagging this since it goes slightly beyond a pure version bump — happy to revert ifpompackaging was deliberate.spring-boot-properties-migratorkept: it resolves on 2.7.18 and the app starts cleanly with it (it reports no deprecated properties; the only property file in the repo is a fully commented-outserver.port).Gradle (
build.gradle)spring-boot-gradle-plugin2.0.2.RELEASE → 2.7.18.sourceCompatibility/targetCompatibility = 1.8→ a toolchain:java { toolchain { languageVersion = JavaLanguageVersion.of(17) } }compile→implementation,testCompile→testImplementation.bootJar { baseName / version }→archiveBaseName/archiveVersion(the old properties were removed in the Boot 2.x plugin).spring-boot-starter-jdbc+com.h2database:h2, which the Gradle build was missing entirely. Without themcompileJavafails onJdbcTemplateinApplication.java— the Gradle build was broken before this change, independent of the Java level. This just brings it in line withpom.xml.Wrappers
./gradlew wrapper.distributionUrl3.3.9 → 3.9.6 for the same reason.mvnw/gradlewwere also missing the executable bit.Build / test evidence
Both run on
openjdk 17.0.13(Maven 3.6.3,Java version: 17.0.13):The repo contains no tests, so there is nothing for surefire /
:testto run (Task :test NO-SOURCE). No tests were added, weakened or removed.App boot + endpoints on 17 (
java -jar target/gs-spring-boot-0.1.0.jar):Not fully verified / notes
Application.mainand aCommandLineRunnerboth callhttp://gturnquist-quoters.cfapps.io/api/random; that host no longer exists, so the app dies withUnknownHostExceptionright afterStarted Application. This is pre-existing and unrelated to the Java level — to capture the endpoint evidence above I pointed that hostname at a throwaway local stub via/etc/hosts(nothing in the repo was changed for it). Worth fixing separately.~/.m2/settings.xml/ a Gradle init script. Neither is part of this diff; the build files still point atmavenCentral().Link to Devin session: https://app.devin.ai/sessions/06b09547bfc84eeeab26864fb8daa79e
Requested by: @tobydrinkall
Devin Review