Skip to content

feature: migrate to Java 17 and Spring Boot 3.3 - #1053

Open
ginawang-cog wants to merge 8 commits into
masterfrom
devin/1787854339-java17-boot33-base
Open

ginawang-cog wants to merge 8 commits into
masterfrom
devin/1787854339-java17-boot33-base

Conversation

@ginawang-cog

@ginawang-cog ginawang-cog commented Aug 27, 2026

Copy link
Copy Markdown

Summary

Full migration of the RealWorld app from Java 11 / Spring Boot 2.6.3 to Java 17 / Spring Boot 3.3.13. The public Conduit REST contract, the GraphQL schema and /graphql + /graphiql endpoints, and the JWT token format (HS512, Authorization: Token <jwt>) are all unchanged. ./gradlew clean build is green on JDK 17 (68 tests).

This branch was assembled from three workstreams (the two source-migration PRs, #1054 and #1055, merged into this branch).

Build (WS-A)

  • Spring Boot 2.6.33.3.13, dependency-management 1.0.111.1.7, Gradle wrapper 7.48.10.2, sourceCompatibility/targetCompatibility = 11 → a Java 17 toolchain.
  • MyBatis starter 2.2.23.0.4, DGS codegen 5.0.66.3.0, Spotless 6.2.16.25.0, jjwt 0.11.20.12.6, joda-time and sqlite-jdbc bumped.
  • DGS starter swapped to the Boot-3 flavor: graphql-dgs-spring-boot-starter:4.9.21graphql-dgs-spring-graphql-starter:9.2.2.
  • Rest Assured is no longer pinned at 4.5.1; the Boot BOM supplies 5.4.0.
  • Spotless now targets fileTree('src') instead of the whole rootDir — under Gradle 8 the root-wide tree made spotlessJava an undeclared consumer of build/generated, failing clean build.
  • settings.gradle gained a pluginManagement block, and a Google-hosted Maven Central mirror is listed ahead of mavenCentral() because Central rate-limited (HTTP 429) this environment. See the note below — this is the one thing worth a reviewer decision.

Jakarta + Spring 6 (WS-B, #1054)

  • javax.validation.*jakarta.validation.* across io.spring.api.* / io.spring.application.*, and javax.servlet.*jakarta.servlet.* in JwtTokenFilter.
  • Spring 6 changed the ResponseEntityExceptionHandler hook signature, so the @Override had to follow; the body it builds is untouched, so the 422 {"errors": {...}} payload is byte-for-byte identical:
   protected ResponseEntity<Object> handleMethodArgumentNotValid(
-      MethodArgumentNotValidException e, HttpHeaders headers, HttpStatus status, WebRequest request)
+      MethodArgumentNotValidException e, HttpHeaders headers, HttpStatusCode status, WebRequest request)

Spring Security 6 / GraphQL / jjwt (WS-C, #1055)

  • WebSecurityConfigurerAdapter is gone in Security 6, so WebSecurityConfig now exposes a SecurityFilterChain bean using the lambda DSL and requestMatchers. Every rule is carried over in the same order (OPTIONS, /graphiql, /graphql, GET /articles/feed authenticated, POST /users|/users/login, GET /articles/**|/profiles/**|/tags, then anyRequest().authenticated()), along with csrf-disabled, CORS, the HttpStatusEntryPoint(UNAUTHORIZED), stateless sessions, and jwtTokenFilter before UsernamePasswordAuthenticationFilter.
  • graphql-java renamed the exception-handler hook and made it async: DataFetcherExceptionHandler.onException(params)handleException(params) returning CompletableFuture<DataFetcherExceptionHandlerResult>. Each existing return result is wrapped in CompletableFuture.completedFuture(...), so the emitted GraphQL errors are unchanged.
  • The datafetchers no longer hand graphql-java's DefaultPageInfo to the DGS-generated connection types (the newer generated ArticlesConnection.pageInfo takes the generated types.PageInfo), so buildArticlePageInfo/buildCommentPageInfo now build types.PageInfo directly. Same four fields, same values — cursors are plain strings instead of DefaultConnectionCursor wrappers.
  • DefaultJwtService moved to the jjwt 0.12 API (Jwts.SIG.HS512, subject/expiration, Jwts.parser().verifyWith(key).parseSignedClaims(token).getPayload()). MacAlgorithm no longer exposes getJcaName(), so the SecretKeySpec algorithm is the literal "HmacSHA512". Verified at runtime that the production jwt.secret still produces {"alg":"HS512"} and round-trips.
  • One test fixture changed: DefaultJwtServiceTest's 480-bit secret is below the key length jjwt 0.12 now enforces for HS512, so it was lengthened to 64 chars. No assertions were modified anywhere.
  • Added spring.graphql.graphiql.enabled=true; the spring-graphql starter disables GraphiQL by default, and spring.graphql.path / graphiql.path already default to the existing /graphql and /graphiql.

Notes for reviewers

  1. The Maven Central mirror in build.gradle is a workaround for this build environment, not a project decision. Central returned HTTP 429 here, so https://maven-central.storage-download.googleapis.com/maven2 was added ahead of mavenCentral(). It makes every consumer of this build resolve through a third-party mirror first; happy to drop it if CI can reach Central directly.
  2. Snyk fails on this PR, and it is not a regression. Scanning this branch vs. master: 99 unique findings here vs. 193 on master, and 83 of this branch's 99 are also present on master. The 16 findings unique to this branch are all transitive (spring-graphql 1.3.5, tomcat-embed-core 10.1.42, spring-hateoas 2.3.4, spring-security-crypto/core 6.3.10, jackson-databind 2.17.3, spring-boot-autoconfigure 3.3.13) and none of them have a fix reachable from Boot 3.3.x / DGS 9.x — Snyk's remediation paths point at Boot 3.5.x/4.0.x, Spring 7, Spring Security 7, and DGS 10. Clearing them means leaving the requested 3.3 line.

Devin-Org: engineering

Link to Devin session: https://app.devin.ai/sessions/f39f2107da574652ab427738a2872571
Open in Devin Desktop: https://app.devin.ai/desktop/session/f39f2107da574652ab427738a2872571?variant=devin
Requested by: @ginawang-cog


Devin Review

Status Commit
⚪ Not started

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

Devin Review (Staging)
Devin Review

Co-Authored-By: Gina Wang <ginawang1021@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 2 potential issues.

Devin Review

Comment thread build.gradle
}

repositories {
maven { url = uri('https://maven-central.storage-download.googleapis.com/maven2') }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Google Maven mirror ahead of mavenCentral

A Google-hosted Central mirror sits before mavenCentral() in repositories, added to work around HTTP 429 in one environment. Committing it makes every consumer of the build resolve artifacts through that third-party endpoint first.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this is the questionable part of the diff. The mirror is there because Maven Central answers HTTP 429 from our build environment, which Gradle treats as a hard resolution failure rather than falling through to the next repository — so simply putting mavenCentral() first would not give a working fallback. It is a read-only Google-hosted Central mirror, but it is still a third-party endpoint in the resolution path for every consumer.

Leaving it in for now so the parallel migration branches can resolve, and flagging it to the repo owner: the alternative is to drop it from build.gradle and put it in a per-machine Gradle init script instead. Happy to do that before this lands.

Comment thread build.gradle
@devin-ai-integration devin-ai-integration Bot changed the title feature: WS-A build foundation for Java 17 + Spring Boot 3.3 feature: migrate to Java 17 and Spring Boot 3.3 Aug 27, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Devin Review

Comment on lines +26 to 28
signatureAlgorithm = Jwts.SIG.HS512;
this.signingKey = new SecretKeySpec(secret.getBytes(), JCA_NAME);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: JWT secret now must be at least 64 bytes

jjwt 0.12.6 rejects HS512 keys under 512 bits at signing time, which is why the test secret grew from 60 to 64 chars. The bundled jwt.secret is 86 chars so it works, but any deployment overriding it with a shorter value will now fail at token creation where 0.11.2 tolerated it. See DefaultJwtService.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, and worth flagging in release notes. Confirmed the bundled jwt.secret in application.properties is 86 chars (688 bits) and signs/round-trips fine, but jjwt 0.12 enforces the RFC 7518 512-bit minimum for HS512 at signing time, so any deployment overriding jwt.secret with fewer than 64 ASCII chars will now throw WeakKeyException on token creation instead of silently signing with a short key (which is what 0.11.2 did). That's the reason the test fixture went 60 -> 64 chars — the fixture was the only thing in the repo below the bound.

Leaving the behavior as-is deliberately: the old tolerance was the bug, and adding a fallback that pads or hashes a short secret would change token signatures for anyone currently running a short one. Flagging rather than fixing.

Comment on lines +358 to +365
private PageInfo buildArticlePageInfo(CursorPager<ArticleData> articles) {
return PageInfo.newBuilder()
.startCursor(
articles.getStartCursor() == null ? null : articles.getStartCursor().toString())
.endCursor(articles.getEndCursor() == null ? null : articles.getEndCursor().toString())
.hasPreviousPage(articles.hasPrevious())
.hasNextPage(articles.hasNext())
.build();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Relay PageInfo swap matches schema fields

Codegen 6.3.0 emits io.spring.graphql.types.PageInfo rather than the relay type, so the rewritten builders map startCursor/endCursor/hasNextPage/hasPreviousPage, matching the PageInfo definition in the schema. Generated sources under build/ are not checked in, so the builder signature could not be verified directly.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the part you couldn't see. The generated build/generated/sources/dgs-codegen/io/spring/graphql/types/PageInfo.java exposes exactly Builder.startCursor(String), endCursor(String), hasNextPage(boolean), hasPreviousPage(boolean), matching the schema's type PageInfo { endCursor: String, hasNextPage: Boolean!, hasPreviousPage: Boolean!, startCursor: String }. The switch was forced rather than stylistic: ArticlesConnection.Builder.pageInfo now takes types.PageInfo, so graphql.relay.DefaultPageInfo no longer type-checks there.

Serialized shape is unchanged — the cursors are the same DateTimeCursor.toString() strings, just no longer wrapped in DefaultConnectionCursor, and the null-cursor cases still emit null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant