sjahedhussini opened a new pull request, #3797: URL: https://github.com/apache/fory/pull/3797
<!-- **Thanks for contributing to Apache Fory™.** **If this is your first time opening a PR on fory, you can refer to [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).** Contribution Checklist - The **Apache Fory™** community has requirements on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md). - Apache Fory™ has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here. --> ## Why? `Fory.copy(obj)` throws `UnsupportedOperationException: Class java.lang.Package doesn't support serialization` (wrapped in `CopyException`) when the object graph contains a non-`Serializable` JDK class, even though the same object round-trips fine through `serialize` + `deserialize`. This makes deep-copying common real-world object graphs (e.g. Hibernate objects) fail. ## What does this PR do? Root cause: `ClassResolver.getSerializerClass` rejects non-`Serializable` JDK classes via the `checkJdkClassSerializable` guard. The check is correct for serialization, but because copy and serialize share the same per-class serializer created in `createSerializer`, the guard fired at serializer-creation time and broke `copy()` as collateral damage. Fix: route these classes to a new `NonSerializableSerializer`. Its `write`/`read` still throw `UnsupportedOperationException` (binary serialization behavior is unchanged — the failure is simply deferred to write time), but it inherits the field-copy implementation from `AbstractObjectSerializer`, so `Fory.copy()` now works. Transient fields are still copied (e.g. `HashMap`'s `size`/`table`), since the inherited copy path copies all non-static fields. `NonSerializableSerializer` is kept deliberately distinct from `CopyOnlyObjectSerializer`: the latter blocks serialization for registration-security reasons (remediable by registering the class), whereas a non-`Serializable` JDK class is intrinsically non-serializable and not remediable by registration, so the failure semantics differ. The new serializer is also registered in `GraalvmSupport`'s default serializer set, consistent with every other class returned by `getSerializerClass`. Files changed: - `resolver/ClassResolver.java` — route non-`Serializable` JDK classes to the new serializer instead of throwing. - `serializer/NonSerializableSerializer.java` — new serializer (copy supported, serialize unsupported). - `platform/GraalvmSupport.java` — register the new serializer for native-image builds. - `test/ForyCopyTest.java` — tests. Behavior note: serialization of these classes now surfaces as `SerializationException` wrapping `UnsupportedOperationException`, rather than the raw `UnsupportedOperationException` thrown eagerly at serializer creation. ## Related issues Fixes issue #2941. ## AI Contribution Checklist - [x] Substantial AI assistance was used in this PR: `yes` - [x] If `yes`, I included a completed AI Contribution Checklist in this PR description and the required `AI Usage Disclosure`. - [x] If `yes`, my PR description includes the `ai_review` summary and screenshot evidence/links from both fresh reviewers on the current diff. - [x] Substantial AI assistance was used: `yes` - [x] I can explain and defend all important changes without AI help. - [x] I reviewed AI-assisted code changes line by line before submission. - [x] I completed line-by-line self-review first and fixed issues before requesting AI review. - [x] I ran two fresh AI review agents on the current diff: one using `.claude/skills/fory-code-review/SKILL.md` and one without. - [x] I addressed all AI review comments and repeated the loop until both reviewers reported no further actionable comments. - [x] I attached evidence of the final clean AI review from both reviewers below. - [x] I ran human verification and recorded evidence below. - [x] I added/updated tests where required. - [x] N/A — no protocol/wire-format change; no separate performance evidence required (see below). - [x] I verified licensing and provenance compliance. \``` AI Usage Disclosure - substantial_ai_assistance: yes - scope: design drafting, code drafting, tests, PR rationale - affected_files_or_subsystems: java/fory-core — resolver/ClassResolver.java, serializer/NonSerializableSerializer.java (new), platform/GraalvmSupport.java, test/ForyCopyTest.java - ai_review: Completed line-by-line self-review and fixed issues first. Ran two fresh reviewers on the final diff — one using .claude/skills/fory-code-review/SKILL.md, one without. Addressed all actionable comments (duplicate-serializer justification, removal of issue/history references in comments, GraalVM registration, test assertion strength, typos) and reran both until neither reported further actionable comments. - ai_review_artifacts: Attached are the result of AI Agents. The claude command for independent agent is: Do a thorough, independent code review of the changes on my current branch versus main: `git diff main...fix-issue-2941`. Review every changed line for correctness, edge cases, performance, maintainability, test quality, and style. Do NOT use any project-specific review skill — I want a fresh general review. List concrete actionable issues. The claude command for SKILL based agent is: Review the changes on my current branch versus main using the guidance in .claude/skills/fory-code-review/SKILL.md. Diff: `git diff main...fix-issue-2941`. Go line by line and list any actionable issues. <img width="1859" height="304" alt="Screenshot from 2026-06-28 02-06-37" src="https://github.com/user-attachments/assets/65d5b72d-ccce-4690-98fa-16da2173e08c"; /> <img width="1877" height="148" alt="Screenshot from 2026-06-28 01-58-47" src="https://github.com/user-attachments/assets/b5703716-eb0b-4ed1-856e-d1cab00f1d87"; /> - human_verification: cd java && mvn -pl fory-core test -Dtest=org.apache.fory.ForyCopyTest — pass. cd java && mvn -pl fory-core test — pass. Local spotless:check could not run due to a google-java-format/JDK incompatibility (JCAnyPattern NoClassDefFoundError) affecting files unrelated to this change; relying on project CI for the format gate. Reviewed all results. - performance_verification: N/A — change only redirects serializer resolution for non-Serializable JDK classes that previously threw; existing hot paths unchanged. - provenance_license_confirmation: Apache-2.0-compatible provenance confirmed; no incompatible third-party code introduced. New file carries the standard ASF license header. \``` ## Does this PR introduce any user-facing change? <!-- If any user-facing interface changes, please [open an issue](https://github.com/apache/fory/issues/new/choose) describing the need to do so and update the document if necessary. Delete section if not applicable. --> - [x] No public API change. `Fory.copy()` now succeeds where it previously threw; no signature or contract change. - [x] No binary protocol compatibility change. Serialization of these classes is still refused (now at write time). ## Benchmark N/A — see performance_verification above. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
