-
-
Notifications
You must be signed in to change notification settings - Fork 29
Bump org.springframework.boot from 3.5.6 to 4.0.0 #1075
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Bump org.springframework.boot from 3.5.6 to 4.0.0 #1075
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 📝 WalkthroughWalkthroughSpring Boot and framework upgrade from 3.5.6 to 4.0.1 with corresponding dependency refactoring, Jackson library migration to tools.jackson, Spring Security API modernization, and new JSON API compatibility classes for data handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
@dependabot rebase |
Bumps [org.springframework.boot](https://github.com/spring-projects/spring-boot) from 3.5.6 to 4.0.0. - [Release notes](https://github.com/spring-projects/spring-boot/releases) - [Commits](spring-projects/spring-boot@v3.5.6...v4.0.0) --- updated-dependencies: - dependency-name: org.springframework.boot dependency-version: 4.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
826afdd to
92e7c2c
Compare
|
A newer version of org.springframework.boot exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged. |
feb99aa to
b323c38
Compare
b323c38 to
af944ee
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🧹 Nitpick comments (5)
src/inttest/resources/config/application.yml (1)
106-110: Consider re-enabling logging for integration tests.The Hibernate and security logging configuration has been completely commented out. While this may reduce log noise, it could make debugging integration test failures more difficult.
Consider selectively re-enabling at least WARN level for these loggers to catch potential issues:
-#logging: -# level: -# org.hibernate.SQL: DEBUG -# org.hibernate.engine.spi.EntityEntry: TRACE -# org.springframework.security: DEBUG +logging: + level: + org.hibernate.SQL: WARN + org.springframework.security: WARNsrc/main/java/com/faforever/api/data/jackson3compat/KeySerializer.java (1)
26-44: Minor formatting inconsistency in method body.The
serializemethod body uses 4-space indentation while the method signature uses 2-space indentation. Consider aligning the indentation for consistency.🔎 Proposed fix for indentation
@Override - public void serialize(Object value, JsonGenerator jgen, SerializationContext provider) throws JacksonException { - String str; - Class<?> cls = value.getClass(); - - if (cls == String.class) { - str = (String) value; - } else if (Date.class.isAssignableFrom(cls)) { - provider.defaultSerializeDateKey((Date) value, jgen); - return; - } else if (cls == Class.class) { - str = ((Class<?>) value).getName(); - } else if (cls.isEnum()) { - str = ((Enum<?>) value).name(); - } else { - str = value.toString(); - } - jgen.writeName(str); - } + public void serialize(Object value, JsonGenerator jgen, SerializationContext provider) throws JacksonException { + String str; + Class<?> cls = value.getClass(); + + if (cls == String.class) { + str = (String) value; + } else if (Date.class.isAssignableFrom(cls)) { + provider.defaultSerializeDateKey((Date) value, jgen); + return; + } else if (cls == Class.class) { + str = ((Class<?>) value).getName(); + } else if (cls.isEnum()) { + str = ((Enum<?>) value).name(); + } else { + str = value.toString(); + } + jgen.writeName(str); + }src/main/java/com/faforever/api/data/jackson3compat/Data.java (1)
88-90: Consider adding timeout toblock()calls for defensive programming.The
block()call without a timeout can hang indefinitely if the underlyingFluxnever completes. While current usage appears to be with finite sources, adding a timeout would provide a safety net.🔎 Example with timeout
public Collection<T> get() { - return values.collectList().block(); + return values.collectList().block(Duration.ofSeconds(30)); }Apply similar changes to
getSingleValue()andtoResourceIdentifiers().src/main/java/com/faforever/api/data/jackson3compat/Resource.java (2)
48-73: Inconsistent null-id handling between constructors.The two-argument constructor (lines 48-54) throws
InvalidObjectIdentifierExceptionwhenidis null, while the Jackson-annotated constructor (lines 56-73) silently falls back tolid. This asymmetry may be intentional (programmatic creation vs. deserialization), but could lead to unexpected behavior.Consider documenting the intended use case for each constructor, or unifying the validation logic.
147-167:hashCodeandequalsuse different field sets.
hashCodeis computed fromtypeandid/lid, whileequalsadditionally comparesattributesandrelationships. This means twoResourceobjects can have the same hash but be unequal (which is allowed), but it also means the identity semantics are inconsistent:
- Two resources with the same
typeandidbut differentattributeswill hash the same but compare as unequal- This could lead to unexpected behavior in hash-based collections
If identity is based on
type+id/lid, consider aligningequalsto match. If full value equality is intended, consider including all relevant fields inhashCode.🔎 Option A: Identity-based equals (align with hashCode)
@Override public boolean equals(Object obj) { if (this == obj) { return true; } if (obj instanceof Resource) { Resource that = (Resource) obj; - return Objects.equals(this.id, that.id) - && Objects.equals(this.lid, that.lid) - && Objects.equals(this.attributes, that.attributes) - && Objects.equals(this.type, that.type) - && Objects.equals(this.relationships, that.relationships); + String thisIdentifier = this.id != null ? this.id : this.lid; + String thatIdentifier = that.id != null ? that.id : that.lid; + return Objects.equals(this.type, that.type) + && Objects.equals(thisIdentifier, thatIdentifier); } return false; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
build.gradlesrc/inttest/java/com/faforever/api/clan/ClanControllerTest.javasrc/inttest/java/com/faforever/api/config/LeagueDbTestContainers.javasrc/inttest/java/com/faforever/api/config/MainDbTestContainers.javasrc/inttest/java/com/faforever/api/moderationreport/ModerationReportTest.javasrc/inttest/java/com/faforever/api/user/MeControllerTest.javasrc/inttest/java/com/faforever/api/user/UsersControllerTest.javasrc/inttest/resources/config/application.ymlsrc/main/java/com/faforever/api/challonge/ChallongeController.javasrc/main/java/com/faforever/api/clan/ClanService.javasrc/main/java/com/faforever/api/config/FafDatasourceConfig.javasrc/main/java/com/faforever/api/config/FafDatastoreConfig.javasrc/main/java/com/faforever/api/config/IgnoreOctetStreamToObjectHttpMessageConverter.javasrc/main/java/com/faforever/api/config/LeagueDatasourceConfig.javasrc/main/java/com/faforever/api/config/LeagueDatastoreConfig.javasrc/main/java/com/faforever/api/config/MvcConfig.javasrc/main/java/com/faforever/api/config/RabbitConfiguration.javasrc/main/java/com/faforever/api/config/security/MethodSecurityConfig.javasrc/main/java/com/faforever/api/config/security/WebSecurityConfig.javasrc/main/java/com/faforever/api/data/converter/JsonConverter.javasrc/main/java/com/faforever/api/data/domain/Mod.javasrc/main/java/com/faforever/api/data/jackson3compat/Data.javasrc/main/java/com/faforever/api/data/jackson3compat/DataDeserializer.javasrc/main/java/com/faforever/api/data/jackson3compat/DataSerializer.javasrc/main/java/com/faforever/api/data/jackson3compat/JsonApiDocument.javasrc/main/java/com/faforever/api/data/jackson3compat/KeySerializer.javasrc/main/java/com/faforever/api/data/jackson3compat/Meta.javasrc/main/java/com/faforever/api/data/jackson3compat/MetaDeserializer.javasrc/main/java/com/faforever/api/data/jackson3compat/Resource.javasrc/main/java/com/faforever/api/deployment/ExeUploaderController.javasrc/main/java/com/faforever/api/error/ErrorJsonSerializer.javasrc/main/java/com/faforever/api/featuredmods/FeaturedModsController.javasrc/main/java/com/faforever/api/forum/NodebbService.javasrc/main/java/com/faforever/api/map/MapsController.javasrc/main/java/com/faforever/api/security/FafTokenService.javasrc/main/java/com/faforever/api/security/JwtService.javasrc/main/java/com/faforever/api/security/method/CustomMethodSecurityExpressionHandler.javasrc/main/java/com/faforever/api/security/method/CustomMethodSecurityExpressionRoot.javasrc/main/java/com/faforever/api/user/MeController.javasrc/main/java/com/faforever/api/user/RecaptchaService.javasrc/main/java/com/faforever/api/user/SteamService.javasrc/main/java/com/faforever/api/user/UsersController.javasrc/main/resources/config/application.ymlsrc/test/java/com/faforever/api/clan/ClanServiceTest.javasrc/test/java/com/faforever/api/deployment/ExeUploaderControllerTest.javasrc/test/java/com/faforever/api/email/JavaEmailSenderTest.javasrc/test/java/com/faforever/api/error/GlobalControllerExceptionHandlerTest.javasrc/test/java/com/faforever/api/security/FafTokenServiceTest.javasrc/test/java/com/faforever/api/user/RecaptchaServiceTest.javasrc/test/resources/config/application.yml
💤 Files with no reviewable changes (4)
- src/main/java/com/faforever/api/config/MvcConfig.java
- src/main/java/com/faforever/api/config/IgnoreOctetStreamToObjectHttpMessageConverter.java
- src/test/resources/config/application.yml
- src/main/resources/config/application.yml
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/faforever/api/data/jackson3compat/Data.java (2)
src/main/java/com/faforever/api/data/jackson3compat/JsonApiDocument.java (1)
ToString(24-110)src/main/java/com/faforever/api/data/jackson3compat/Resource.java (1)
ToString(35-189)
src/main/java/com/faforever/api/data/jackson3compat/JsonApiDocument.java (1)
src/main/java/com/faforever/api/data/jackson3compat/Resource.java (1)
ToString(35-189)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (50)
src/main/java/com/faforever/api/data/domain/Mod.java (1)
25-25: LGTM! Correct migration to Jakarta validation standard.The migration from
org.hibernate.validator.constraints.NotEmptytojakarta.validation.constraints.NotEmptyis necessary for Spring Boot 4.0 compatibility. Both annotations provide equivalent validation behavior for collections, ensuring no runtime changes.src/test/java/com/faforever/api/email/JavaEmailSenderTest.java (1)
40-40: LGTM! Appropriate simplification for Spring Boot 4.0 upgrade.The migration from
SMTPMessageto the more generalMimeMessageis correct. The test only verifies standard message properties (recipients, sender, subject, content) that are available on the baseMimeMessageclass, so no SMTP-specific functionality is lost.src/main/java/com/faforever/api/challonge/ChallongeController.java (2)
65-65: LGTM! Correct migration from deprecated MediaType constant.The change from
MediaType.APPLICATION_JSON_UTF8_VALUEtoMediaType.APPLICATION_JSON_VALUEis correct. The UTF-8 variant was deprecated in Spring Framework 5 and removed in Spring Framework 6 (which is part of Spring Boot 4.0). UTF-8 is now the default charset for JSON, so this change maintains the same behavior while aligning with the updated framework.Also applies to: 72-72
7-7: No issues found. The RestTemplateBuilder import has been correctly updated toorg.springframework.boot.restclient.RestTemplateBuilderfor Spring Boot 4.0, and all usages across the codebase (RecaptchaService, RecaptchaServiceTest, and ChallongeController) are consistently updated to the new package location.src/test/java/com/faforever/api/user/RecaptchaServiceTest.java (1)
14-14: Test import correctly updated to match production code.The import path change mirrors the update in the production code, maintaining consistency between the test and production implementations. Test logic remains unchanged, confirming API compatibility.
src/main/java/com/faforever/api/user/RecaptchaService.java (1)
9-9: Import path correctly updated for Spring Boot 4.0.0.The
RestTemplateBuilderimport has been correctly updated fromorg.springframework.boot.web.clienttoorg.springframework.boot.restclient, aligning with Spring Boot 4.0.0's package restructuring. The usage ofRestTemplateBuilderremains unchanged.src/main/java/com/faforever/api/config/RabbitConfiguration.java (4)
61-61: LGTM - Parameter type aligns with bean return type.The parameter type correctly matches the return type of the
retryInterceptor()bean method.
67-67: LGTM - Message converter instantiation aligns with updated import.The message converter instantiation correctly uses the new
JacksonJsonMessageConverterclass, consistent with the import change on line 17.
45-51: Change is correct:maxRetries(2)maintains the 3-attempt behavior.The change from
maxAttempts(3)tomaxRetries(2)is semantically equivalent. In Spring Boot 4.0.0,maxRetriescounts only the retry attempts (not including the initial attempt), somaxRetries(2)= 1 initial + 2 retries = 3 total attempts, matching the original behavior described in the comment on line 41.
14-14: All Spring Boot 4.0.0 API changes are correct and maintain the same behavior.The imports and retry configuration have been verified:
StatelessRetryOperationsInterceptoris the correct replacement in Spring AMQP 4.xJacksonJsonMessageConvertercorrectly replaces the deprecatedJackson2JsonMessageConvertermaxRetries(2)maintains semantic equivalence with the previous behavior: 1 initial attempt + 2 retries = 3 total attempts, matching the comment "tries to process a message 3 times"src/main/java/com/faforever/api/user/SteamService.java (1)
37-37: Necessary change for Spring Framework 6.2 deprecation compliance; default configuration is safe.The change from
fromHttpUrltofromUriStringis required becausefromHttpUrlwas deprecated in Spring Framework 6.2 and is scheduled for removal in Spring Framework 7.0. This is a mandatory migration, not a behavioral choice.The default
loginUrlFormatis hardcoded as"https://steamcommunity.com/openid/login"(a valid HTTPS URL), and configuration files show all Steam URLs default to HTTPS endpoints. WhilefromUriStringis more lenient thanfromHttpUrland doesn't strictly validate the URI scheme, there is no practical security risk in this context given the safe defaults and configuration pattern.Minor note: Consider adding a
@Patternor@URLvalidation annotation to theloginUrlFormatfield inFafApiProperties.Steam(line 220) to explicitly enforce HTTP/HTTPS schemes, though this is optional since operational configuration is controlled.src/main/java/com/faforever/api/map/MapsController.java (3)
30-30: LGTM: Media type constant migration is correct.The change from
APPLICATION_JSON_UTF8_VALUEtoAPPLICATION_JSON_VALUEaligns with Spring Framework 6+ (bundled with Spring Boot 4.0), where the UTF-8 specific constant was removed as UTF-8 is now the default charset.
43-43: LGTM: Media type constants consistently updated.All
@RequestMappingproduce attributes have been correctly updated to useAPPLICATION_JSON_VALUE, ensuring consistency with the Spring Boot 4.0 migration.Also applies to: 56-56, 69-69, 80-80
96-96: JacksonException is the correct exception type for ObjectMapper.readTree() in Jackson 3.0.The change from
IOExceptiontoJacksonExceptionis appropriate. In Jackson 3.0,ObjectMapper.readTree()throws unchecked exceptions under theJacksonExceptionbase type, includingStreamReadExceptionfor parsing errors andJacksonIOExceptionfor I/O problems. Catching the baseJacksonExceptiontype covers all cases and aligns with Jackson 3.0's exception hierarchy.src/main/java/com/faforever/api/error/ErrorJsonSerializer.java (4)
13-14: LGTM!The annotation and class declaration changes correctly reflect the Jackson 3.x API migration.
21-25: LGTM!The serialization method updates correctly follow the Jackson 3.x API pattern (
writeXField→writeXProperty). The behavioral output remains functionally equivalent.
17-17: No action required.The method signature update to use
SerializationContextand throwJacksonExceptionis correct for Jackson 3.x. Exception handling is managed by the Jackson framework, not application code, so there are no upstream changes needed.
5-9: Migration to tools.jackson (Jackson 3.x) is fully supported by Spring Boot 4.0.1.Spring Boot 4.0+ officially adopts Jackson 3 as the default JSON library and auto-configures tools.jackson with managed dependency versions. The imports and migration in this file are correct and compatible.
src/main/java/com/faforever/api/security/FafTokenService.java (1)
111-111: Exception hierarchy correctly updated for Jackson 3.x.The change from
JsonProcessingExceptiontoJacksonExceptionis correct. In Jackson 3.x,JacksonExceptionis the new base exception that covers all Jackson parsing and processing exceptions, including the formerJsonProcessingException.src/test/java/com/faforever/api/security/FafTokenServiceTest.java (1)
13-14: ObjectMapper configuration is correct for Jackson 3.x.In Jackson 3 (tools.jackson), java.time support is built into the core databind library and no longer requires JavaTimeModule registration. The test's plain
ObjectMapperinitialization is appropriate. Additionally, FafTokenService does not depend on ObjectMapper for java.time serialization—it manually converts Instant to/from String usingtoString()andInstant.parse(), so ObjectMapper configuration has no impact on java.time handling in this class.src/main/java/com/faforever/api/deployment/ExeUploaderController.java (1)
21-21: LGTM! Correct migration to modern Spring constant.The change from
APPLICATION_JSON_UTF8_VALUEtoAPPLICATION_JSON_VALUEis correct and necessary for Spring Boot 4.x. The UTF-8 charset is now the default for JSON content types in Spring 6+, so the behavior is preserved.Also applies to: 49-49
src/main/java/com/faforever/api/security/method/CustomMethodSecurityExpressionRoot.java (1)
20-22: LGTM! Correct Spring Security 6 migration.The constructor change from
AuthenticationtoSupplier<? extends @Nullable Authentication>is the correct migration path for Spring Security 6+. This pattern enables lazy evaluation of authentication and aligns with Spring Security's updated expression handling.src/main/java/com/faforever/api/forum/NodebbService.java (1)
106-106: No downstream compatibility issues found. ThereadOnlyHttpHeaders()wrapper is safe to use here.RestTemplate does not modify headers passed via
HttpEntity—it only reads them to construct the HTTP request. Additionally, no interceptors are configured in the codebase that could attempt to modify headers. The headers are created, populated, and immediately wrapped as immutable inbuildAuthorizedRequest(), never exposing them for modification.src/test/java/com/faforever/api/deployment/ExeUploaderControllerTest.java (1)
9-9: Import path is correct for Spring Boot 4.x. Theorg.springframework.boot.webmvc.test.autoconfigure.WebMvcTestimport is the proper package location following Spring Boot 4.x migration. The annotation usage remains unchanged, and this import aligns with the new test package structure.src/main/java/com/faforever/api/clan/ClanService.java (1)
21-21: No action required—this is the correct Jackson 3.x migration for Spring Boot 4.0.1.Spring Boot 4.0.1 defaults to Jackson 3.x, which uses the
tools.jacksonpackage namespace instead ofcom.fasterxml.jackson. The import change fromcom.fasterxml.jackson.databind.ObjectMappertotools.jackson.databind.ObjectMapperis the expected and correct migration path.src/main/java/com/faforever/api/user/UsersController.java (1)
191-191: No changes needed. Line 189 already performs a direct redirect with the unvalidatedcallbackUrl, demonstrating that the codebase accepts arbitrary URLs without strict validation. The change fromfromHttpUrl()tofromUriString()is a necessary migration due to the deprecation offromHttpUrl()in Spring Framework 6.2 (scheduled for removal in 7.0), and this pattern is consistent across the entire codebase. No new security issue is introduced.Likely an incorrect or invalid review comment.
src/inttest/java/com/faforever/api/config/LeagueDbTestContainers.java (1)
9-9: LGTM: Import path updated for Spring Boot 4.0.The
DataSourcePropertiesimport path has been correctly updated to reflect the package reorganization in Spring Boot 4.0 (fromorg.springframework.boot.autoconfigure.jdbctoorg.springframework.boot.jdbc.autoconfigure).src/inttest/java/com/faforever/api/user/MeControllerTest.java (1)
20-21: LGTM: Corrected HTTP status for unauthenticated requests.The expectation has been appropriately updated from 403 Forbidden to 401 Unauthorized for requests without authentication tokens. This aligns with HTTP semantics: 401 indicates missing/invalid authentication, while 403 indicates insufficient authorization for an authenticated user.
src/inttest/java/com/faforever/api/config/MainDbTestContainers.java (1)
9-9: LGTM: Import path updated for Spring Boot 4.0.The
DataSourcePropertiesimport path has been correctly updated to reflect the package reorganization in Spring Boot 4.0.src/main/java/com/faforever/api/config/LeagueDatastoreConfig.java (1)
8-8: LGTM: Import path updated for Spring Boot 4.0.The
EntityManagerFactoryBuilderimport path has been correctly updated to reflect the package reorganization in Spring Boot 4.0 (fromorg.springframework.boot.orm.jpatoorg.springframework.boot.jpa).src/main/java/com/faforever/api/config/FafDatasourceConfig.java (1)
7-7: LGTM: Import path updated for Spring Boot 4.0.The
DataSourcePropertiesimport path has been correctly updated to reflect the package reorganization in Spring Boot 4.0.src/inttest/java/com/faforever/api/moderationreport/ModerationReportTest.java (1)
61-66: LGTM: Corrected HTTP status for unauthenticated requests.The expectation has been appropriately updated from 403 Forbidden to 401 Unauthorized for anonymous user requests. This correctly distinguishes between missing authentication (401) and insufficient authorization (403).
src/main/java/com/faforever/api/config/LeagueDatasourceConfig.java (1)
7-7: LGTM: Import path updated for Spring Boot 4.0.The
DataSourcePropertiesimport path has been correctly updated to reflect the package reorganization in Spring Boot 4.0.build.gradle (1)
145-148: No issues found. All four starter artifacts (spring-boot-starter-webmvc,spring-boot-starter-jackson,spring-boot-jackson2,spring-boot-starter-restclient) exist in Maven Central for Spring Boot 4.0.1. The modularization ofspring-boot-starter-webinto separate starters is documented in the Spring Boot 4.0 migration guide and is the recommended approach.src/main/java/com/faforever/api/config/security/WebSecurityConfig.java (1)
38-38: LGTM! Security API modernization.The migration from
authorizeRequests()toauthorizeHttpRequests()correctly aligns with Spring Security 6+ (part of Spring Boot 4.0). The addition ofanyRequest().authenticated()properly enforces authentication for all non-public endpoints.Also applies to: 63-63
src/main/java/com/faforever/api/user/MeController.java (1)
5-7: LGTM! JSON API model migration.The import migration from Yahoo Elide to custom jackson3compat types is consistent with the broader migration across the codebase. The
/meendpoint logic remains unchanged, preserving the existing user authentication data response structure.src/inttest/java/com/faforever/api/user/UsersControllerTest.java (2)
18-18: LGTM! Correct parameter construction.The change from
HttpHeaderstoLinkedMultiValueMap<String, String>correctly reflects that these are request parameters, not HTTP headers.LinkedMultiValueMapis the appropriate type for form parameters in Spring MVC tests.Also applies to: 66-66, 79-79, 112-112, 128-128, 141-141, 157-157, 173-173, 186-186, 202-202, 218-218, 231-231, 348-348, 359-359, 373-373, 389-389, 404-404, 418-418, 430-430, 444-444, 461-461, 484-484, 500-500, 512-512, 528-528
259-259: LGTM! Correct authentication behavior.The expected status change from Forbidden (403) to Unauthorized (401) correctly aligns with Spring Security 6+ behavior in Spring Boot 4.0. Unauthenticated requests now properly return 401 instead of 403, which is the semantically correct HTTP status code.
Also applies to: 354-354
src/main/java/com/faforever/api/data/jackson3compat/DataSerializer.java (1)
24-35: LGTM! JSON API data serialization logic.The serialization logic correctly handles both to-one and to-many relationships per JSON API specification:
- To-one:
nullwhen empty, single object otherwise- To-many: empty array when null/empty, array of objects otherwise
The implementation properly uses Jackson 3.x APIs and integrates with the jackson3compat data model.
src/inttest/java/com/faforever/api/clan/ClanControllerTest.java (2)
14-14: LGTM! Correct parameter construction.The migration from
HttpHeaderstoLinkedMultiValueMap<String, String>correctly uses the appropriate type for request parameters in Spring MVC tests.Also applies to: 81-81, 101-101, 119-119, 141-141, 163-163
109-109: LGTM! Correct authentication behavior.The expected status change from Forbidden (403) to Unauthorized (401) aligns with the updated security configuration and Spring Security 6+ semantics for unauthenticated requests.
src/main/java/com/faforever/api/config/FafDatastoreConfig.java (1)
10-10: LGTM! Spring Boot 4.0 package reorganization.The import paths correctly reflect Spring Boot 4.0's package reorganization:
EntityManagerFactoryBuildermoved fromorg.springframework.boot.orm.jpatoorg.springframework.boot.jpaSpringBeanContainermoved toorg.springframework.orm.jpa.hibernate(aligns with Hibernate 6+ as the new default)src/main/java/com/faforever/api/data/converter/JsonConverter.java (1)
5-6: LGTM! Jackson 3.x migration.The migration to Jackson 3.x (
tools.jackson.*namespace) is correctly implemented:
JacksonExceptionnow serves as the unified exception type replacingJsonProcessingExceptionandIOException- The
ObjectMapperAPI remains compatiblesrc/main/java/com/faforever/api/featuredmods/FeaturedModsController.java (1)
4-6: Migration to jackson3compat types maintains API compatibility.The custom
Data,JsonApiDocument, andResourcetypes properly implement JSON API serialization through customDataSerializer,DataDeserializer, andKeySerializerclasses. Integration tests confirm the response format is correct, and controllers have completed the migration away from Yahoo Elide'scom.yahoo.elide.jsonapi.models.*types. The wrapping pattern isolates the Yahoo Elide dependencies and ensures backward compatibility with existing JSON API clients.src/main/java/com/faforever/api/data/jackson3compat/MetaDeserializer.java (1)
20-27: LGTM!The deserializer correctly handles the JSON:API meta object requirement by validating that the node is an object before constructing a
Metainstance. The use ofreadTreeAsValuewith rawMap.classis appropriate here sinceMetawraps aMap<String, Object>.src/main/java/com/faforever/api/data/jackson3compat/DataDeserializer.java (1)
21-44: LGTM!The deserializer correctly handles both array (to-many) and single object (to-one) JSON:API data payloads, with appropriate validation that the resource type is present. The error message is clear and helpful for debugging.
src/main/java/com/faforever/api/config/security/MethodSecurityConfig.java (1)
9-15: LGTM!Correct migration from the deprecated
@EnableGlobalMethodSecurityandGlobalMethodSecurityConfigurationto the new@EnableMethodSecurityannotation. The removal ofprePostEnabled = trueis appropriate since it's now the default.src/main/java/com/faforever/api/security/method/CustomMethodSecurityExpressionHandler.java (1)
21-29: The code correctly implements the Spring Security 6.x pattern for custom method security expression handlers. The cast toStandardEvaluationContextis safe becauseDefaultMethodSecurityExpressionHandler.createEvaluationContext()returns aMethodSecurityEvaluationContext, which is a subclass ofStandardEvaluationContext. This is the documented approach for customizing method security in Spring Security 6+, and the implementation will not cause runtimeClassCastExceptionerrors.src/main/java/com/faforever/api/data/jackson3compat/Meta.java (1)
8-11: I'm ready to help rewrite the review comment in the specified format. Please provide the review comment that needs to be rewritten along with any relevant code context or file information.src/main/java/com/faforever/api/data/jackson3compat/Resource.java (1)
169-188: Implementation is correct;loadRecordthrowsInvalidObjectIdentifierExceptionwhen the record is not found.The method properly validates the entity type and identifier before delegating to Elide's
PersistentResource.loadRecord. SinceloadRecordthrowsInvalidObjectIdentifierExceptionfor unknown/invalid identifiers (rather than returning null), the caller always receives a valid non-nullPersistentResource<?>. The method signature correctly declares bothForbiddenAccessExceptionandInvalidObjectIdentifierException.
| plugins { | ||
| id "java" | ||
| id "org.springframework.boot" version "3.5.6" | ||
| id "org.springframework.boot" version "4.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Spring Boot 4.0.1 release date official
💡 Result:
Spring Boot 4.0.1 was officially released on December 18, 2025. [1][2]
Sources:
[1] Spring blog announcement — "Spring Boot 4.0.1 available now" (Dec 18, 2025).
[2] Maven Central / Spring Boot artifacts showing 4.0.1 published (Dec 18, 2025).
🌐 Web query:
Spring Boot 4.0 vs 3.5 breaking changes migration guide
💡 Result:
Below is a concise migration checklist and the main breaking-change areas when moving from Spring Boot 3.5 → 4.0, with links to the official migration material and detailed topics to review.
Quick actions (order)
- Upgrade to latest 3.5.x and run tests. [2]
- Update JDK to supported baseline (Java 17 minimum; Java 21/25 supported). [1][2]
- Read the full 4.0 migration guide and release notes, then migrate incrementally (branch + CI). [2][1]
High‑impact breaking changes to review and fix
-
Spring Framework / platform baseline
- Spring Framework 7 / platform upgrades — check removed deprecated APIs your app uses. [2][1]
-
Jackson 2 → Jackson 3 (major migration)
- Boot 4 defaults to Jackson 3 (new groupId/package changes: com.fasterxml.jackson → tools.jackson; jackson‑annotations exception). Many classes/props renamed; Jackson 2 support is available temporarily via a compatibility module but deprecated. Use Spring’s Jackson migration docs and OpenRewrite recipes to automate refactors. Test serialization/deserialization thoroughly. [3][2][4]
-
Configuration/property changes
- Several property names reorganized (e.g., some spring.jackson properties moved under different paths); consult the configuration changelog in the migration guide. [2][4]
-
Build tool / dependency minimums
- Gradle 8.x (8.14+) or 9.x recommended; updated dependency management for core libs. Verify plugin compatibility. [2][4]
-
Kotlin / language support
- Kotlin 2.2 support noted (if you use Kotlin, update and test). [8] (migration guide)
-
Micrometer / tracing changes
- spring-boot-micrometer-tracing modularization (separate Brave / OpenTelemetry modules) and property renames — update dependencies and config. [6]
-
Actuator / endpoint / annotation changes
- Some annotations (nullable/non-null) handling and actuator parameter annotation behavior changed; migrate to recommended JSpecify/org.jspecify annotations or OptionalParameter where applicable. [0][2]
-
Static resources / security defaults
- Added common static location for fonts (/fonts/**) — may affect security rules; exclude if undesired. [5]
-
Third‑party ecosystem impacts
- Spring Data, Spring Security and some Spring projects now target Jackson 3; some ecosystem projects may still require Jackson 2 — review your transitive deps and libraries (Spring Data REST requires Jackson 3). [3][2]
How to proceed (practical)
- Run dependency analysis (mvn dependency:tree or Gradle deps). Replace com.fasterxml.jackson artifacts with tools.jackson where needed. [3]
- Use OpenRewrite automated recipes (UpgradeJackson_2_3 etc.) and Spring Application Advisor for code suggestions. [3]
- Add temporary spring-boot-jackson2 if blocked by libraries (deprecated, short‑term fix). [2]
- Create a migration branch, compile, fix API/import errors, run integration tests, verify JSON contracts and security endpoints. [2][3][4]
Essential reading (start here)
- Spring Blog: Spring Boot 4.0.0 announcement / overview. [1]
- Spring Boot 4.0 Migration Guide (official, exhaustive list + config changelog). [2]
- Spring blog: Introducing Jackson 3 support in Spring (Jackson 3 migration details). [3]
- Spring Boot 4.0 release/RC notes (breaking changes & module notes). [4]
If you want, I can:
- Generate a targeted checklist for your project (scan pom/gradle + source) and list exact imports/classes to change, or
- Run an automated OpenRewrite recipe plan you can apply. Which would you prefer?
Sources
[1] Spring blog — “Spring Boot 4.0.0 available now”.
[2] Spring Boot 4.0 Migration Guide / Release Notes (spring-projects/spring-boot wiki).
[3] Spring blog — “Introducing Jackson 3 support in Spring” (Jackson 3 migration details).
[4] Spring Boot 4.0 Release & RC notes (GitHub wiki).
🌐 Web query:
Spring Boot version history 3.5.6 4.0.1
💡 Result:
-
Spring Boot 3.5.6 — released Sep 18, 2025; maintenance release with ~43 bug fixes, docs improvements and dependency upgrades; available from Maven Central. [1]
-
Spring Boot 4.0.1 — released Dec 18, 2025; maintenance/patch release with ~88 bug fixes, documentation improvements and dependency upgrades; available from Maven Central. [2]
Sources:
[1] Spring blog — "Spring Boot 3.5.6 available now" (Sep 18, 2025).
[2] Spring blog — "Spring Boot 4.0.1 available now" (Dec 18, 2025).
Review migration guide and address breaking changes for Spring Boot 4.0.1 upgrade.
Spring Boot 4.0.1 is officially released (December 2025). This major version upgrade from 3.5.6 introduces several breaking changes that must be addressed:
- Jackson 2 → 3 migration: GroupId changes from
com.fasterxml.jacksontotools.jacksonand package restructuring. Ensure all Jackson dependencies are updated and serialization/deserialization thoroughly tested. - Java 17 minimum: Update baseline JDK requirement; Java 21/25 are supported.
- Spring Framework 7: Review for removed deprecated APIs your application uses.
- Gradle 8.14+ or 9.x: Update build tool version if not already met.
- Configuration changes: Spring Jackson properties have been reorganized; consult the official migration guide.
- Micrometer/tracing modularization: Separate Brave/OpenTelemetry modules with property renames.
- Third-party ecosystem: Some Spring projects now require Jackson 3; verify transitive dependencies.
Use OpenRewrite recipes for automated Jackson migration and the official Spring Boot 4.0 Migration Guide for exhaustive breaking changes. Test thoroughly, including JSON contracts and security endpoints.
| public Collection<ResourceIdentifier> toResourceIdentifiers() { | ||
| return values | ||
| .map(object -> object != null ? ((Resource) object).toResourceIdentifier() : null) | ||
| .collectList().block(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unchecked cast from T to Resource may cause ClassCastException.
The method casts generic type T to Resource without any type check. If Data<T> is instantiated with a type other than Resource, this will throw a ClassCastException at runtime.
Consider adding a type check or restricting this method to only be available when T is Resource. Alternatively, document this constraint clearly in the Javadoc.
🔎 Defensive implementation option
public Collection<ResourceIdentifier> toResourceIdentifiers() {
return values
- .map(object -> object != null ? ((Resource) object).toResourceIdentifier() : null)
+ .map(object -> {
+ if (object == null) {
+ return null;
+ }
+ if (!(object instanceof Resource)) {
+ throw new IllegalStateException("Cannot convert non-Resource to ResourceIdentifier");
+ }
+ return ((Resource) object).toResourceIdentifier();
+ })
.collectList().block();
}🤖 Prompt for AI Agents
In src/main/java/com/faforever/api/data/jackson3compat/Data.java around lines
120 to 124, the method unconditionally casts generic T to Resource which can
throw ClassCastException at runtime; fix by either (A) restricting the API so
this method is only available when T extends Resource (change signature or add a
bounded helper method) or (B) perform a defensive runtime check: filter or map
only values that are instanceof Resource, cast after the check, and handle
non-Resource entries (skip, log, or throw a clear IllegalStateException), and
update Javadoc to document the expected type constraint so callers know the
requirement.
src/main/java/com/faforever/api/data/jackson3compat/JsonApiDocument.java
Show resolved
Hide resolved
| "/users/performPasswordReset", | ||
| "/users/linkToSteam/**" | ||
| ).permitAll(); | ||
| authorizeConfig.anyRequest().authenticated(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without this no endpoint was evaluated. I had to whitelist all endpoints which does not require standard auth
Bumps org.springframework.boot from 3.5.6 to 4.0.0.
Release notes
Sourced from org.springframework.boot's releases.
... (truncated)
Commits
1c0e08bRelease v4.0.03487928Merge branch '3.5.x'29b8e96Switch make-default in preparation for Spring Boot 4.0.088da0ddMerge branch '3.5.x'56feeaaNext development version (v3.5.9-SNAPSHOT)3becdc7Move server.error properties to spring.web.error2b30632Merge branch '3.5.x'4f03b44Merge branch '3.4.x' into 3.5.x3d15c13Next development version (v3.4.13-SNAPSHOT)dc140dfUpgrade to Spring Framework 7.0.1Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.