Skip to content

fix(java): honor record field encoding in generated decode#3626

Merged
chaokunyang merged 3 commits into
apache:mainfrom
mandrean:mandrean/fix-3624
Apr 28, 2026
Merged

fix(java): honor record field encoding in generated decode#3626
chaokunyang merged 3 commits into
apache:mainfrom
mandrean:mandrean/fix-3624

Conversation

@mandrean
Copy link
Copy Markdown
Contributor

@mandrean mandrean commented Apr 28, 2026

Why?

Java record deserialization in generated compatible-mode serializers used type-level primitive decoding for nullable record fields. For boxed primitive record components this could make the generated writer and reader choose different encodings, corrupting values during round trip.

This fixes the shared root cause for #3622 and #3624.

What does this PR do?

  • Adds a ThreadSafeFory cross-pool reproducer for a boxed Long record with number/string compression.
  • Adds a compatible-mode codegen reproducer for a record with boxed Long and Integer components matching Java record with boxed primitive components corrupts fields when codegen is enabled #3622.
  • Decodes nullable record fields through descriptor-aware generated field paths so read encoding matches write encoding.
  • Makes generated float/double literals locale-stable with Locale.ROOT.

Related issues

AI Contribution Checklist

  • Substantial AI assistance was used in this PR: yes
  • If yes, I included a completed AI Contribution Checklist in this PR description and the required AI Usage Disclosure.
  • If yes, my PR description includes the required ai_review summary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes.

Contributor checklist for substantial AI assistance:

  • Substantial AI assistance was used in this PR: yes
  • If yes, I included the standardized AI Usage Disclosure block below.
  • If yes, I can explain and defend all important changes without AI help.
  • If yes, I reviewed AI-assisted code changes line by line before submission.
  • If yes, I completed line-by-line self-review first and fixed issues before requesting AI review.
  • If yes, I ran two fresh AI review agents on the current PR diff or current HEAD after the latest code changes: one using .claude/skills/fory-code-review/SKILL.md and one without that skill.
  • If yes, I addressed all AI review comments and repeated the review loop until both AI reviewers reported no further actionable comments.
  • If yes, I attached screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes in this PR body.
  • If yes, I ran adequate local verification and recorded evidence (checks run locally or in CI, pass/fail summary, and confirmation I reviewed results).
  • If yes, I added/updated tests and specs where required.
  • If yes, I validated protocol/performance impacts with evidence when applicable.
  • If yes, I verified licensing and provenance compliance.

AI Usage Disclosure

  • substantial_ai_assistance: yes
  • scope: code drafting, test drafting, PR description drafting
  • affected_files_or_subsystems: Java generated object codec/read path, Java record latest-JDK tests, generated literal codegen
  • ai_review: Pending final contributor line-by-line review and required two-reviewer AI review loop before maintainer review.
  • ai_review_artifacts: Pending; final clean review screenshots or persistent links have not yet been attached.
  • human_verification: Pending contributor review. Local verification was run in this workspace; each command and result is listed below.
    • PASS: mvn -pl fory-latest-jdk-tests -am -Dtest=org.apache.fory.integration_tests.RecordSerializersTest#testCompatibleCodegenBoxedPrimitiveRecordRoundTrip -Dsurefire.failIfNoSpecifiedTests=false test
    • PASS: mvn -pl fory-latest-jdk-tests -am -Dtest=org.apache.fory.integration_tests.RecordSerializersTest -Dsurefire.failIfNoSpecifiedTests=false test
    • PASS: ENABLE_FORY_DEBUG_OUTPUT=1 mvn -pl fory-latest-jdk-tests -am -Dtest=org.apache.fory.integration_tests.RecordSerializersTest,org.apache.fory.integration_tests.RecordXlangTest -Dsurefire.failIfNoSpecifiedTests=false test
    • PASS: ENABLE_FORY_DEBUG_OUTPUT=1 mvn -pl fory-core -am -Dtest=org.apache.fory.serializer.CodegenSerializerTest,org.apache.fory.builder.ObjectCodecBuilderTest -Dsurefire.failIfNoSpecifiedTests=false test
    • PASS: mvn -pl fory-core,fory-latest-jdk-tests -DskipTests spotless:check checkstyle:check (Maven reported cached metadata warnings for global-maven-virtual, but the build completed successfully.)
  • performance_verification: No benchmark run; this is a targeted Java generated-code bug fix. No public API or binary protocol compatibility change is intended.
  • provenance_license_confirmation: Locally-authored changes only; no third-party code introduced.

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

N/A. This is a targeted Java generated-code bug fix; no benchmark was run.

@mandrean mandrean requested a review from chaokunyang as a code owner April 28, 2026 11:28
Copy link
Copy Markdown
Collaborator

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix. New release wil come in next week

@chaokunyang chaokunyang merged commit b6b18aa into apache:main Apr 28, 2026
70 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants