Upgrade to Spring Boot 4.0, Hibernate 7, Liquibase 5, Jackson 3#805
Upgrade to Spring Boot 4.0, Hibernate 7, Liquibase 5, Jackson 3#805
Conversation
- org.springframework.boot: 3.5.9 → 3.5.10 - gradle-git-properties plugin: 2.5.4 → 2.5.7 - postgresql: 42.7.8 → 42.7.9 - ical4j: 4.2.2 → 4.2.3 - itext-core: 9.4.0 → 9.5.0 - html2pdf: 6.3.0 → 6.3.1 - Remove unused logback_version from gradle.properties
Major dependency upgrades: - Spring Boot: 3.5.x → 4.0.2 (Spring Framework 7, Tomcat 11) - Hibernate ORM: 6.6.x → 7.2.1 (managed by Spring Boot BOM) - Liquibase: 4.33.0 → 5.x (via spring-boot-starter-liquibase) - Jackson: 2.x → 3.x (tools.jackson packages) - Spring Security: 6.x → 7.0 Migration changes: - Rename starter: web → webmvc, oauth2 → security-oauth2 - Replace liquibase-core with spring-boot-starter-liquibase - Add spring-boot-starter-webmvc-test for MockMvc support - Migrate Jackson imports to tools.jackson namespace - Migrate @Nullable/@nonnull to org.jspecify.annotations - Update AutoConfigureMockMvc import for new package - Fix Hibernate 7 schema validation (keywords column nullability) - Replace deprecated JsonNode.asText() with asString() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpgrade Spring Boot and Gradle, replace several starters, migrate Jackson usage to tools.jackson with corresponding test API changes, change topic/previous ID handling from String to UUID with validation, rename ErrorDto factory and widen its parameter, tighten an entity column nullability, and simplify Gradle wrapper scripts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/main/java/de/tum/cit/aet/thesis/security/JwtAuthConverter.java (1)
33-35:⚠️ Potential issue | 🟡 MinorPotential NPE:
Converter.convert()is@Nullable.
jwtGrantedAuthoritiesConverter.convert(jwt)can returnnullper theConvertercontract. Calling.stream()directly on line 34 would throw an NPE. In practiceJwtGrantedAuthoritiesConverteralways returns a non-null collection, but it's safer to guard against the contract.Proposed fix
public AbstractAuthenticationToken convert(`@NonNull` Jwt jwt) { + var jwtAuthorities = jwtGrantedAuthoritiesConverter.convert(jwt); Collection<GrantedAuthority> authorities = Stream.concat( - jwtGrantedAuthoritiesConverter.convert(jwt).stream(), + jwtAuthorities != null ? jwtAuthorities.stream() : Stream.empty(), extractResourceRoles(jwt).stream()).collect(Collectors.toSet());
🤖 Fix all issues with AI agents
In `@server/src/main/java/de/tum/cit/aet/thesis/service/ApplicationService.java`:
- Line 134: topicsFilter creation uses UUID::fromString which will throw
IllegalArgumentException for malformed values; update the logic in
ApplicationService where topicsFilter is built to validate or catch parsing
errors (wrap the stream mapping in a try/catch or validate each topic before
mapping) and on invalid UUIDs convert the error to a controlled domain/HTTP
error (e.g., throw a BadRequestException or a custom InvalidRequestException)
instead of allowing a 500; reference the topicsFilter variable and the
UUID::fromString mapping when making the change.
🧹 Nitpick comments (5)
server/src/test/java/de/tum/cit/aet/thesis/mock/BaseIntegrationTest.java (1)
271-271: Inconsistent UUID parsing pattern compared to other test files.These two lines use
.asString().transform(UUID::fromString)while every other file in this PR usesUUID.fromString(node.asString()). Consider aligning for consistency.Suggested diff
- return objectMapper.readTree(response).get("topicId").asString().transform(UUID::fromString); + return UUID.fromString(objectMapper.readTree(response).get("topicId").asString());- return objectMapper.readTree(response).get("thesisId").asString().transform(UUID::fromString); + return UUID.fromString(objectMapper.readTree(response).get("thesisId").asString());Also applies to: 297-297
server/src/main/java/de/tum/cit/aet/thesis/entity/Thesis.java (1)
112-137: Pre-existing: seven EAGER@OneToManycollections on a single entity.Not introduced by this PR, but worth noting: every
Thesisload fetches roles, proposals, assessments, presentations, feedback, files, and states eagerly. This can cause significant N+1 / Cartesian product issues, especially with Hibernate 7's stricter fetch semantics. Consider migrating toLAZYwith entity graphs or fetch joins in a follow-up.server/build.gradle (1)
69-70: Empty version strings on logback dependencies.These resolve via the Spring Boot BOM, which is fine, but the trailing colon with no version is an unusual pattern. The more conventional approach when relying on the BOM is to omit the version segment entirely.
Suggested cleanup
- implementation "ch.qos.logback:logback-classic:" - implementation "ch.qos.logback:logback-core:" + implementation "ch.qos.logback:logback-classic" + implementation "ch.qos.logback:logback-core"server/src/main/java/de/tum/cit/aet/thesis/service/ApplicationService.java (1)
41-41:Collectorsimport can be avoided withStream.toList()pattern (Java 16+).Since you're on Java 25, you could use
Collectors.toUnmodifiableSet()or keep as-is. Minor nit — no action required.server/src/main/java/de/tum/cit/aet/thesis/exception/ResponseExceptionHandler.java (1)
32-37:StreamReadExceptionis redundant — it's a subclass ofJacksonException.In Jackson 3.x,
StreamReadExceptionextendsJacksonException. Catching both meansStreamReadException.classis already covered byJacksonException.class. You can simplify:Remove redundant exception type
`@ExceptionHandler`({ ParseException.class, ResourceInvalidParametersException.class, - StreamReadException.class, JacksonException.class, })
server/src/main/java/de/tum/cit/aet/thesis/service/ApplicationService.java
Outdated
Show resolved
Hide resolved
Wrap UUID.fromString calls with try-catch to return 400 Bad Request instead of 500 for malformed UUID inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/build.gradle (1)
78-83:⚠️ Potential issue | 🟡 MinorRemove the redundant
spring-boot-starter-testdeclaration and migrate the still-neededxmlunit-coreexclusion.Spring Boot 4.0.2 no longer includes
junit-vintage-engineorandroid-jsoninspring-boot-starter-test, making those exclusions obsolete. However,xmlunit-core(2.10.4) is still present. Sincespring-boot-starter-webmvc-testalready bringsspring-boot-starter-testtransitively, the explicit declaration at line 78 is redundant. Move thexmlunit-coreexclusion to the remaining dependency:Recommended fix
- testImplementation("org.springframework.boot:spring-boot-starter-test") { - exclude group: "org.junit.vintage", module: "junit-vintage-engine" - exclude group: "com.vaadin.external.google", module: "android-json" - exclude group: "org.xmlunit", module: "xmlunit-core" - } - testImplementation "org.springframework.boot:spring-boot-starter-webmvc-test" + testImplementation("org.springframework.boot:spring-boot-starter-webmvc-test") { + exclude group: "org.xmlunit", module: "xmlunit-core" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/build.gradle` around lines 78 - 83, Remove the redundant explicit testImplementation("org.springframework.boot:spring-boot-starter-test") declaration and migrate the xmlunit-core exclusion onto the remaining testImplementation "org.springframework.boot:spring-boot-starter-webmvc-test" dependency; specifically delete the explicit spring-boot-starter-test block and add an exclude group: "org.xmlunit", module: "xmlunit-core" to the spring-boot-starter-webmvc-test dependency so xmlunit-core is still excluded while relying on the transitive starter.
🧹 Nitpick comments (2)
server/build.gradle (2)
9-9: Consider replacingio.spring.dependency-managementwith Gradle's native BOM support.A Spring Boot 4.0.2-focused best-practices guide marks
id("io.spring.dependency-management")as no longer necessary and recommends replacing it withimplementation(platform(SpringBootPlugin.BOM_COORDINATES)). Gradle has had native BOM platform support since 5.0, and the Spring Boot Gradle plugin exposes aBOM_COORDINATESconstant that removes the need for the dependency-management shim entirely. This isn't a blocker for 4.0.2, but it removes one layer of dependency resolution magic and aligns with current Spring Boot guidance.♻️ Proposed migration to native BOM platform
plugins { // ... id "org.springframework.boot" version "4.0.2" - id "io.spring.dependency-management" version "1.1.7" // ... } +import org.springframework.boot.gradle.plugin.SpringBootPlugin + dependencies { + implementation platform(SpringBootPlugin.BOM_COORDINATES) implementation "org.springframework.boot:spring-boot-starter-data-jpa" // ... rest unchanged }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/build.gradle` at line 9, Replace the io.spring.dependency-management plugin usage by switching to Gradle's native BOM platform: remove the id "io.spring.dependency-management" plugin declaration and instead add the Spring Boot BOM as a platform dependency using implementation(platform(SpringBootPlugin.BOM_COORDINATES)); update the buildscript/plugins block to rely on the Spring Boot Gradle plugin (referencing SpringBootPlugin.BOM_COORDINATES) for BOM management and remove any dependency-management-specific configuration remnants.
22-24: Merge the twospringBoot {}blocks.
springBootis configured in two separate closures: lines 22–24 setmainClassand lines 107–109 callbuildInfo(). While Groovy DSL merges these at runtime today, keeping them split is an easy source of confusion during future edits (e.g., someone adds a property to one block unaware the other exists).♻️ Proposed consolidation
-springBoot { - mainClass = "de.tum.cit.aet.thesis.ThesisManagementApplication" -} +springBoot { + mainClass = "de.tum.cit.aet.thesis.ThesisManagementApplication" + buildInfo() +} // ... (remove the second block at lines 107-109) -springBoot { - buildInfo() -}Also applies to: 107-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/build.gradle` around lines 22 - 24, The build uses two separate springBoot { } closures; consolidate them into a single springBoot block that sets mainClass = "de.tum.cit.aet.thesis.ThesisManagementApplication" and calls buildInfo() so both configurations live together (remove the duplicate springBoot closure). Locate the existing springBoot closures (the one configuring mainClass and the one invoking buildInfo()) and merge their contents into one closure to avoid split configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/build.gradle`:
- Around line 38-44: The build declares a non-existent dependency
"spring-boot-starter-security-oauth2-client" which will break the build; replace
that artifact with the correct starter coordinate
"spring-boot-starter-oauth2-client" in the dependencies list (the line currently
containing spring-boot-starter-security-oauth2-client) and verify the project’s
Spring Boot version supports that starter (or adjust to the correct coordinate
for Spring Boot 4.0.x if different) so the dependency resolves from Maven
Central.
---
Outside diff comments:
In `@server/build.gradle`:
- Around line 78-83: Remove the redundant explicit
testImplementation("org.springframework.boot:spring-boot-starter-test")
declaration and migrate the xmlunit-core exclusion onto the remaining
testImplementation "org.springframework.boot:spring-boot-starter-webmvc-test"
dependency; specifically delete the explicit spring-boot-starter-test block and
add an exclude group: "org.xmlunit", module: "xmlunit-core" to the
spring-boot-starter-webmvc-test dependency so xmlunit-core is still excluded
while relying on the transitive starter.
---
Nitpick comments:
In `@server/build.gradle`:
- Line 9: Replace the io.spring.dependency-management plugin usage by switching
to Gradle's native BOM platform: remove the id "io.spring.dependency-management"
plugin declaration and instead add the Spring Boot BOM as a platform dependency
using implementation(platform(SpringBootPlugin.BOM_COORDINATES)); update the
buildscript/plugins block to rely on the Spring Boot Gradle plugin (referencing
SpringBootPlugin.BOM_COORDINATES) for BOM management and remove any
dependency-management-specific configuration remnants.
- Around line 22-24: The build uses two separate springBoot { } closures;
consolidate them into a single springBoot block that sets mainClass =
"de.tum.cit.aet.thesis.ThesisManagementApplication" and calls buildInfo() so
both configurations live together (remove the duplicate springBoot closure).
Locate the existing springBoot closures (the one configuring mainClass and the
one invoking buildInfo()) and merge their contents into one closure to avoid
split configuration.
…ception ParseException is a checked exception but handleBadRequest declared RuntimeException as parameter type, causing a type mismatch at runtime. Changed to Exception and renamed ErrorDto.fromRuntimeException to fromException. Also removed redundant StreamReadException (subclass of JacksonException in Jackson 3). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/src/main/java/de/tum/cit/aet/thesis/dto/ErrorDto.java (1)
5-9: Add@JsonInclude(JsonInclude.Include.NON_EMPTY)to this DTO.All DTOs in the project should carry this annotation to minimize payload size. Without it, a null
message(whichException.getMessage()can return) serialises as"message": nullin the error response. Given the Jackson 3 migration in this PR, use thetools.jacksonpackage.♻️ Proposed fix
package de.tum.cit.aet.thesis.dto; import java.time.Instant; +import tools.jackson.annotation.JsonInclude; +@JsonInclude(JsonInclude.Include.NON_EMPTY) public record ErrorDto( Instant timestamp, String message, String exception ) {Based on learnings: "Apply JsonInclude(JsonInclude.Include.NON_EMPTY) to all DTOs to minimize payload size by excluding null or empty values from JSON. Ensure consistent DTO design across the project."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/main/java/de/tum/cit/aet/thesis/dto/ErrorDto.java` around lines 5 - 9, Add the Jackson 3 JsonInclude annotation to the ErrorDto record to omit null/empty fields from JSON: annotate the ErrorDto record declaration (the record named ErrorDto) with `@JsonInclude`(JsonInclude.Include.NON_EMPTY) using the Jackson 3 package import (tools.jackson.annotation.JsonInclude) so that null message/exception fields are not serialized; add the corresponding import and ensure the annotation sits directly above the ErrorDto record declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/main/java/de/tum/cit/aet/thesis/dto/ErrorDto.java`:
- Line 13: The code calls error.getMessage() which may return null; update the
ErrorDto construction (where error.getMessage() is passed) to guard against null
by substituting a fallback (e.g. Objects.toString(error.getMessage(),
error.toString()) or
Optional.ofNullable(error.getMessage()).orElse(error.getClass().getName())) so
the DTO always carries a useful, non-null message; locate the usage in
ErrorDto.java (the ErrorDto constructor/initialiser that currently passes
error.getMessage()) and replace with the null-safe expression.
---
Nitpick comments:
In `@server/src/main/java/de/tum/cit/aet/thesis/dto/ErrorDto.java`:
- Around line 5-9: Add the Jackson 3 JsonInclude annotation to the ErrorDto
record to omit null/empty fields from JSON: annotate the ErrorDto record
declaration (the record named ErrorDto) with
`@JsonInclude`(JsonInclude.Include.NON_EMPTY) using the Jackson 3 package import
(tools.jackson.annotation.JsonInclude) so that null message/exception fields are
not serialized; add the corresponding import and ensure the annotation sits
directly above the ErrorDto record declaration.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/src/main/java/de/tum/cit/aet/thesis/dto/ErrorDto.java (1)
5-9: Add@JsonInclude(JsonInclude.Include.NON_EMPTY)to this record.Per project convention, all DTOs should carry this annotation to minimize payload size.
♻️ Proposed fix
+import tools.jackson.annotation.JsonInclude; import java.time.Instant; +@JsonInclude(JsonInclude.Include.NON_EMPTY) public record ErrorDto( Instant timestamp, String message, String exception ) {Based on learnings: "Apply JsonInclude(JsonInclude.Include.NON_EMPTY) to all DTOs to minimize payload size by excluding null or empty values from JSON."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/main/java/de/tum/cit/aet/thesis/dto/ErrorDto.java` around lines 5 - 9, The ErrorDto record is missing the Jackson annotation to exclude null/empty fields; add the `@JsonInclude`(JsonInclude.Include.NON_EMPTY) annotation above the ErrorDto record declaration and import com.fasterxml.jackson.annotation.JsonInclude so that Instant timestamp, message, and exception fields are omitted from JSON when empty, following the DTO convention used across the project.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/src/main/java/de/tum/cit/aet/thesis/dto/ErrorDto.java`:
- Around line 5-9: The ErrorDto record is missing the Jackson annotation to
exclude null/empty fields; add the `@JsonInclude`(JsonInclude.Include.NON_EMPTY)
annotation above the ErrorDto record declaration and import
com.fasterxml.jackson.annotation.JsonInclude so that Instant timestamp, message,
and exception fields are omitted from JSON when empty, following the DTO
convention used across the project.
Resolve merge conflicts: - build.gradle: Keep spring-boot-starter-liquibase (Spring Boot 4 managed) with newer postgresql 42.7.10 - Thesis.java: Keep nullable = false on keywords column with correct indentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
spring-boot-starter-liquibase)tools.jacksonpackages)Migration Details
Build changes
spring-boot-starter-web→spring-boot-starter-webmvc,oauth2-client→security-oauth2-client,oauth2-resource-server→security-oauth2-resource-serverorg.liquibase:liquibase-corewithspring-boot-starter-liquibasespring-boot-starter-webmvc-testfor MockMvc support in testsSource code changes
com.fasterxml.jacksontotools.jacksonnamespace@Nullable/@NonNullfromorg.springframework.langtoorg.jspecify.annotationsAutoConfigureMockMvcimport for Spring Boot 4.0 modular package structurekeywordscolumn nullability in Thesis entity)JsonNode.asText()withasString()Test plan
Summary by CodeRabbit
Chores
Bug Fixes
Refactor
Tests