Skip to content

Migrate generators and generated projects to Jackson 3 - #735

Merged
stevehu merged 14 commits into
masterfrom
migrate-jackson-3
Oct 27, 2026
Merged

stevehu merged 14 commits into
masterfrom
migrate-jackson-3

Conversation

@stevehu

@stevehu stevehu commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Migrate generator APIs and generated Java/Kotlin builds to Jackson 3 and Java 25, including matching Docker images. Legacy config booleans use public YAML events, a candidate pre-filter and explicit document/depth/scalar limits. CI compiles generated REST, hybrid, GraphQL, Lambda, Lambda native and Kotlin projects. Generated-project compilation does not qualify generated service execution or container runtimes.

CI pins policy f0d2100f972eec765b2b276e7ace38922394f0a6 and qualified core 97b2359b92ed3e1d05cac04f2da8760aa954e49d. The complete prerequisite closure is checked in at ci/jackson3-source-refs.json, initialized from successful run manifests. Missing/malformed pins fail before resolution; default CI performs no default-head or migration-branch lookup. Explicit replay overrides merge into the complete map and remain subject to core/prerequisite qualification. Core tests reuse verified evidence; other prerequisites and full consumer suites remain enabled. The source-origin audit covers NetworkNT releases and snapshots, with only the explicit digest-pinned validator release exception.

Core-pin adoption used merged light-4j #2826 and its reviewed acceptance record. Producer 40371926971, forced renewal 40371900445 and normal reuse 40373717464 passed for the exact core/policy/prerequisite tuple. No open core branch or obsolete policy pin is used.

The AWS Lambda/native source prerequisites now pin actual merged migrations: AWS #168 at dddbbc9e7f6f690f8688ceac024728b6a84f4808 and native #189 at af992cb093213293a42a3601c3356cc9122033ac. Both were verified as ancestors of fetched master. Only those two entries changed; core/policy and every remaining source pin are unchanged.

Validation: run 40436925938 passed on exact head 57c8ba4e0e4498c76800455c4350c8541702d40b. Downloaded reports confirm 85 tests, 6 skipped and zero failures/errors. Every resolved source SHA matches the checked-in manifest, including the actual AWS/native merge SHAs; the strict artifact audit reports no Jackson 2 runtime dependencies or source-origin failures. The retained run log confirms successful compilation of generated REST, hybrid, GraphQL, Lambda, Lambda native and Kotlin projects. Generated compilation does not qualify generated application execution or container runtimes. No release, deployment or consumer merge.

@stevehu

stevehu commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Reviewed the diff at e2ea241 (templates, generators, tests) and the author's result logs. I did not run the build, tests or any generated project.

  1. Java 25 vs Docker images. Generated Maven/Gradle projects now target Java 25 (Lambda 11→25, REST/hybrid/GraphQL 21→25), but every generated Dockerfile still uses JDK 11 (public.ecr.aws/lambda/java:11, azul/zulu-openjdk-alpine:11, openjdk:11.0.3-slim, amazoncorretto:11.0.21) in rest, hybrid, graphql, restkotlin, lambda and lambdanative. Built images would fail with UnsupportedClassVersionError. The README says no Docker build was done, so nothing catches it.
  2. YAML 1.2 scalars vs flag readers. The new test pins yes as the string "yes". Generator's readers (isOverwriteHandler, isEnableHttps, useLightProxy, ...) call jsonNode.booleanValue() on whatever the user config holds. A config with enableHttps: yes or overwriteHandler: no previously read as a boolean and now arrives as a text node. I did not check what booleanValue() returns for a text node on 3.2.3, but it is unlikely to be the intended value. Consider fail-fast or normalisation, plus a test through the flag readers.
  3. Jackson3GeneratedBuildTest only asserts that strings appear in the generated pom/build.gradle.kts. Compilation of generated projects exists only in local logs, so the README's compilation claims can't be re-checked in CI.
  4. Style: fully-qualified com.networknt.config.Jackson2Defaults inline in several files (e.g. GeneratorServiceHandlerTest five times) and some imports placed above the existing block.
  5. No build workflow; only pre-commit.ci ran.

@stevehu

stevehu commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Re-reviewed 4855f3b (delta since e2ea241). I did not run the build or tests. The Docker JDK, YAML flag and generated-build findings look addressed in the diff.

One remaining concern: GeneratorConfigYaml restores yes/no/on/off by subclassing YAMLFactory and YAMLParser and overriding protected underscore-prefixed internals (_createParser x4, _matchYAMLBoolean, _decodeScalar), and copies the parser constructor signature. Those aren't public API. A Jackson bump (a single BOM property) could break compilation or, worse, stop the override being called so enableHttps: yes reads as a string again, with only two tests guarding it. A note pinning the Jackson version, or normalising the known flag keys after parsing in Generator, would be less brittle.

@stevehu

stevehu commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Review of head c008640. I read the diff and the CI results; I did not run the build.

The public-SnakeYAML-event rewrite in GeneratorConfigYaml addresses my earlier concern about overriding Jackson internals.

One cost remains. Every config is parsed twice, once by the SnakeYAML Engine event pass and once by Jackson, and the event pass uses setCodePointLimit(Integer.MAX_VALUE). A very large or deeply nested generator config is therefore fully scanned before Jackson's configured stream limits can reject it. Passing Jackson's limit into the LoadSettings, or only taking the event pass when a legacy scalar is present, would keep the limits meaningful.

@stevehu

stevehu commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Review of head 57178c0. I read the diff since my last review and ran nothing; test is green.

My earlier GeneratorConfigYaml point looks addressed: a regex pre-filter means most configs get one parse, and there are explicit code-point, nesting, name and string length limits with a test for both paths.

One point on the workflow: policy, core, yaml-rule and http-client are pinned, but json-overlay, openapi-parser, light-rest-4j, light-hybrid-4j and light-graphql-4j resolve to default-branch heads, and light-aws-lambda and light-lambda-native still use the migrate-jackson-3 branch lookup. After those PRs merge, this repo's master CI builds against whatever those repos hold that day, which can turn it red for unrelated reasons and makes runs hard to reproduce. Pinning by SHA, or documenting them as intentionally floating, would fit the stated goal.

@stevehu

stevehu commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Review of head 7811c14. I read the diff since my last review and ran nothing; test is green.

My earlier point looks addressed: ci/jackson3-source-refs.json pins the complete closure by SHA, the workflow validates the key set and core SHA, and the branch lookups are gone.

One follow-up. The manifest pins light-aws-lambda at d68d2b3 and light-lambda-native at 91c4fd7, which are commits on the still-open #168 and #189 branches, and both PRs have since moved (86a8fa4, 8d6db07). After they merge, this repo's master CI keeps building the pre-merge commits, and nothing says how to repoint them. A short repin step per sibling merge, and ideally a check that fails when a pinned SHA is not an ancestor of that repo's default branch once the sibling is merged, would close this.

@stevehu
stevehu merged commit 6cbfb7a into master Oct 27, 2026
2 checks passed
@stevehu
stevehu deleted the migrate-jackson-3 branch October 27, 2026 02:30
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