feat: upgrade to Java 17 (--release 17) - #60
tobydrinkall wants to merge 1 commit into
Conversation
Upgrade Spring Boot 2.0.2.RELEASE -> 2.7.18 targeting Java 17 for both the Maven and Gradle builds, aligning both on starter-web (+ starter-test). - pom.xml: packaging pom -> jar; parent 2.7.18; java.version 17 + maven.compiler.release 17; drop starter-jdbc + h2; add starter-test. - build.gradle: buildscript block -> plugins DSL (spring-boot 2.7.18, dependency-management 1.0.15.RELEASE, java); compile/testCompile -> implementation/testImplementation; bootJar baseName/version -> archiveBaseName/archiveVersion; source/targetCompatibility 1.8 -> java toolchain 17 + options.release = 17. - Gradle wrapper 4.6 -> 7.6.4 (regenerated jar + scripts). - Maven wrapper -> 3.9.6. - Application.java: remove JdbcTemplate CommandLineRunner and dead-URL quote fetch so the app boots cleanly (aligns with dropping jdbc/h2). 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:
|
| @@ -1 +1 @@ | |||
| distributionUrl=https://repo1.maven.org/maven2/org/apache/maven/apache-maven/3.3.9/apache-maven-3.3.9-bin.zip | |||
| distributionUrl=https://archive.apache.org/dist/maven/maven-3/3.9.6/binaries/apache-maven-3.9.6-bin.zip | |||
There was a problem hiding this comment.
📝 Info: Maven wrapper distribution bumped but wrapper script/jar left at 3.3.9 era
.mvn/wrapper/maven-wrapper.properties now points at Maven 3.9.6, but mvnw/mvnw.cmd and the committed .mvn/wrapper/maven-wrapper.jar (49 KB, 0.4.x era) were not regenerated, and the properties file still has no wrapperUrl. It works because the jar is committed and simply downloads whatever distributionUrl says, but a full mvn wrapper:wrapper -Dmaven=3.9.6 regeneration would keep the wrapper self-consistent (and archive.apache.org is rate-limited/slower than repo1.maven.org for distribution downloads).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Accurate. The committed wrapper jar is a version-agnostic bootstrapper that just downloads whatever distributionUrl points at, so it works as-is. I switched to archive.apache.org specifically because repo1.maven.org was returning HTTP 429 during this session; archive.apache.org is the canonical, reliably-available mirror. A full mvn wrapper:wrapper -Dmaven=3.9.6 regeneration (which would also refresh the jar and add wrapperUrl) is a reasonable follow-up but wasn't required to get both builds green.
| public class Application { | ||
|
|
||
| public static void main(String[] args) { | ||
|
|
||
| ApplicationContext ctx = SpringApplication.run(Application.class, args); | ||
|
|
||
| System.out.println("Let's inspect the beans provided by Spring Boot:"); | ||
|
|
||
| String[] beanNames = ctx.getBeanDefinitionNames(); | ||
| Arrays.sort(beanNames); | ||
| for (String beanName : beanNames) { | ||
| System.out.println(beanName); |
There was a problem hiding this comment.
📝 Info: Dead model classes left behind after removing JDBC/RestTemplate demo code
hello.model.Customer, hello.model.Quote and hello.model.Value were only referenced by the removed CommandLineRunner/RestTemplate code in Application.java; they are now unreachable dead code. I verified no other source file references them or JdbcTemplate/RestTemplate, so dropping spring-boot-starter-jdbc/h2 does not break compilation of the remaining controllers/services — but the leftover models are worth deleting for clarity.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed — Customer, Quote, and Value are now only self-referential (Quote -> Value) with no other usages, so they're dead after removing the JDBC/RestTemplate demo code, and dropping starter-jdbc/h2 doesn't break compilation. I've left them in place because this is a Java-8-features demo repo where the model POJOs are illustrative content; deleting them is a scope/intent call I'd rather leave to the maintainer. Happy to remove them if preferred.
| dependencies { | ||
| compile("org.springframework.boot:spring-boot-starter-web") | ||
| testCompile("junit:junit") | ||
| implementation("org.springframework.boot:spring-boot-starter-web") |
There was a problem hiding this comment.
📝 Info: starter-test added to both builds with no test sources; JUnit Platform config relies on the Boot plugin
spring-boot-starter-test is now declared in both pom.xml and build.gradle, but there is no src/test directory, so nothing is exercised. Note the Gradle side does not add test { useJUnitPlatform() }; that is fine only because the Spring Boot Gradle plugin (>=2.2) configures Test tasks for the JUnit Platform. If the Boot plugin were ever removed/downgraded, JUnit 5 tests would silently not run.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Correct that there are no test sources yet — starter-test was added per the task's build-alignment requirement so a src/test suite can be dropped in without further build changes. Agreed the Gradle JUnit Platform config is implicit via the Spring Boot plugin (>=2.2); if tests are ever added and the Boot plugin is decoupled, I'd add an explicit test { useJUnitPlatform() }. Not adding it now to keep the diff minimal.
| <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 2.0 -> 2.7 behavioural changes not exercised beyond /topic
Upgrading to Boot 2.7 brings changes that the compile+/topic smoke test would not surface: circular-reference beans are rejected by default since 2.6, and Spring MVC path matching/trailing-slash handling changed. Existing mappings such as /topic/{id} versus /topic/string/operation and /topic/minimum/length/{minLength} in src/main/java/hello/controller/TopicController.java and HelloController.java still resolve by pattern specificity, and TopicService.getTopicWithId (src/main/java/hello/service/TopicService.java:37) still throws NoSuchElementException for unknown ids (pre-existing). Worth manually hitting the other endpoints once.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good flag. I smoke-tested /topic (HTTP 200) on both the Maven and Gradle jars. The other mappings resolve by pattern specificity as you note, and TopicService.getTopicWithId throwing NoSuchElementException for unknown ids is pre-existing behaviour unchanged by this upgrade. No circular-reference beans exist here (plain @RestControllers), so the 2.6+ default rejection doesn't apply. I've offered the requester an end-to-end run via the testing agent to exercise the add/update/delete endpoints beyond /topic.
| <java.version>17</java.version> | ||
| <maven.compiler.release>17</maven.compiler.release> |
There was a problem hiding this comment.
📝 Info: Both java.version and maven.compiler.release are set
spring-boot-starter-parent already derives maven.compiler.source/target from java.version; adding maven.compiler.release=17 is redundant (maven-compiler-plugin gives release precedence). Harmless, but the two properties can drift if someone edits only one of them later.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional — the task brief explicitly asked for both <java.version>17</java.version> and <maven.compiler.release>17</maven.compiler.release>. release takes precedence and pins to 17, which is the safer of the two if they ever drift. Keeping as-is to match the requested spec.
Summary
Upgrades this demo from Spring Boot
2.0.2.RELEASE/ Java 8 to Spring Boot2.7.18/ Java 17, keeping thejavax.*namespace (no Jakarta migration). Both the Maven and Gradle builds are upgraded in lock-step and each produces an equivalent bootable jar.Dependency alignment (deliberate decision): the pom previously declared
starter-jdbc+h2+properties-migrator, while Gradle declared onlystarter-web. Both builds are now aligned onstarter-web(+starter-test), droppingstarter-jdbc/h2. This also sidesteps the H21.x -> 2.1.214breaking change.Source change (deviation from the "no source changes" expectation): the task brief stated the source uses neither JDBC nor H2, but
Application.javaactually did:@Autowired JdbcTemplateCommandLineRunnerthat created/populated acustomerstable (requires jdbc + h2), andRestTemplatefetch ofhttp://gturnquist-quoters.cfapps.io/api/random(a long-dead host) executed during startup, which would throw and fail the boot regardless of Java version.To honor the drop-jdbc/h2 decision and get a clean smoke test,
Application.javais reduced to a plain@SpringBootApplicationthat starts the context and prints the bean names. The REST endpoints (/topic, etc.) are unaffected.The
spring-boot-properties-migratorwas kept during smoke-testing to surface renamed Boot 2.0->2.7 properties; it reported nothing (the only property,server.port, is commented out inapplication.properties), and was removed before finalising, as instructed.Verification
There is no CI and no test suite in this repo, so verification is compile + package + smoke only. All commands were run under Temurin/OpenJDK 17.0.13, Maven 3.9.6, Gradle 7.6.4:
./mvnw -B clean package→ BUILD SUCCESS, producestarget/gs-spring-boot-0.1.0.jar../gradlew clean build→ BUILD SUCCESSFUL, producesbuild/libs/gs-spring-boot-0.1.0.jar.java -jar <artifact>→Started Applicationon port 8080, thencurl localhost:8080/topic→ HTTP 200 returning the topics JSON array.Residual risks / not verified
/topicwas not exercised.~/.m2/settings.xmlfor Maven,~/.gradle/init.gradlefor Gradle) was used to fetch dependencies. This does not affect the repo — defaultmavenCentral()remains configured in both builds..idea/,*.iml,target/) were left untouched, per the brief.Link to Devin session: https://app.devin.ai/sessions/5601a58ca8384f2f84a1243a2753399d
Requested by: @tobydrinkall
Devin Review