fix(serde): honor global serialization inclusion in generated serializers - #1378
fix(serde): honor global serialization inclusion in generated serializers#1378arimu1 wants to merge 2 commits into
Conversation
…zers Compile-time bean/record serializers always wrote properties, including nulls, so micronaut.serde.serialization.inclusion was ignored for simple @Serdeable shapes. Route property writes through GeneratedSerdeInclusionUtil and add regression tests (micronaut-core#12838).
|
@dstepanov we need to check this from a performance perspective |
There was a problem hiding this comment.
🟡 Not ready to approve
GeneratedSerdeInclusionUtil.isDefaultScalar currently treats all Number zero values as default, which will incorrectly omit values like BigInteger(0) under NON_DEFAULT and needs adjustment (and ideally a focused test to lock the behavior in).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes a gap in the compile-time (source-generated) serialization path so that generated bean/record serializers honor the global micronaut.serde.serialization.inclusion setting (e.g. NON_NULL, default NON_EMPTY) instead of always emitting all properties (including null).
Changes:
- Introduces
GeneratedSerdeInclusionUtilto centralize global inclusion checks for generated serializers. - Updates
BeanSerializerSourceGenandRecordSerializerSourceGento conditionally emit properties (including key emission) based on the active inclusion strategy. - Adjusts/extends compile-time Jackson tests to validate generated serializer selection while asserting omission of
null/empty properties under global inclusion.
File summaries
| File | Description |
|---|---|
| serde-processor/src/main/java/io/micronaut/serde/processor/sourcegen/records/RecordSerializerSourceGen.java | Gates record component emission behind global inclusion checks (including primitive default handling). |
| serde-processor/src/main/java/io/micronaut/serde/processor/sourcegen/beans/BeanSerializerSourceGen.java | Gates bean property emission behind global inclusion checks (including primitive default handling). |
| serde-api/src/main/java/io/micronaut/serde/util/GeneratedSerdeInclusionUtil.java | Adds generated-code helper methods to evaluate global inclusion for scalar/primitive/serializer-backed properties. |
| serde-jackson/src/test/groovy/io/micronaut/serde/jackson/compiletime/SerdeableGeneratedSpec.groovy | Adds coverage for global NON_NULL and stabilizes tests needing explicit null output via ALWAYS. |
| serde-jackson/src/test/groovy/io/micronaut/serde/jackson/compiletime/CompileTimeSourceGenSpec.groovy | Adds global-inclusion assertions (NON_NULL, default NON_EMPTY) and introduces an ALWAYS helper for tests that assert null output. |
| serde-jackson/src/test/groovy/io/micronaut/serde/jackson/annotation/SerdeJsonSerializeDeserializeSpec.groovy | Forces ALWAYS inclusion for recursive serialization tests that assert explicit null fields. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| if (value instanceof Number number) { | ||
| return number.doubleValue() == 0d; | ||
| } |
There was a problem hiding this comment.
Confirmed, this was real — fixed in 54c6aeb.
isDefaultScalar treated any Number with doubleValue() == 0d as default. That's inconsistent with this codebase's own runtime path: BigIntegerSerde/BigDecimalSerde (both NumberSerde implementations) never override Serializer#isDefault, so the default impl (return false) applies — CustomizedObjectSerializer's NON_DEFAULT branch (serializer.isDefault(...)) never treats BigInteger/BigDecimal as default regardless of value. Meanwhile BeanSerializerSourceGen/RecordSerializerSourceGen#scalarEncoderMethod route BigInteger/BigDecimal properties through this exact fast path (via ENCODE_BIG_INTEGER_METHOD/ENCODE_BIG_DECIMAL_METHOD), so the bug was reachable for real generated serializers, not just a theoretical gap.
Fix: isDefaultScalar now only treats zero as default for the boxed primitive-wrapper number types that actually override isDefault with a zero check (Byte/Short/Integer/Long/Float/Double — see IntegerSerde, LongSerde, etc.), leaving BigInteger/BigDecimal/other Number types non-default, matching the runtime serializer path.
Added a compile-time test (test generated serializers honor global inclusion NON_DEFAULT for BigInteger and BigDecimal zero in CompileTimeSourceGenSpec.groovy, using a new SourceGenNonDefaultScalarBean fixture) that fails before the fix (BigInteger.ZERO/BigDecimal.ZERO silently omitted) and passes after.
| void 'test generated serializers honor global inclusion NON_NULL'() { | ||
| given: | ||
| def context = ApplicationContext.run([ | ||
| 'micronaut.serde.serialization.inclusion': 'NON_NULL' | ||
| ]) |
…rated scalar inclusion check GeneratedSerdeInclusionUtil.isDefaultScalar treated any Number with doubleValue() == 0d as default under NON_DEFAULT, which incorrectly omitted BigInteger.ZERO/BigDecimal.ZERO. The runtime path (CustomizedObjectSerializer via Serializer#isDefault) never treats BigInteger/BigDecimal as default since BigIntegerSerde/BigDecimalSerde don't override isDefault (default impl returns false), and those two types go through the generated fast path via scalarEncoderMethod. Limit the zero-as-default check to the boxed primitive-wrapper number types that do override isDefault with a zero check (Byte/Short/Integer/Long/ Float/Double), keeping generated and runtime inclusion behavior consistent.
Description
Fixes micronaut-projects/micronaut-core#12838.
Source-generated serializers for simple
@Serdeablebeans/records always wrote every property (includingnull), so configuration such as:had no effect. Runtime (
CustomizedObjectSerializer/SerBean) already honoredmicronaut.serde.serialization.inclusion; the compile-time path did not.Change
GeneratedSerdeInclusionUtilfor inclusion checks used by generated codeBeanSerializerSourceGen/RecordSerializerSourceGento skip properties according to the active global inclusion (NON_NULL,NON_EMPTY,NON_ABSENT,NON_DEFAULT,NEVER, etc.)Workaround from the issue (
disable-generated-serializer=trueor type-level@JsonInclude) is no longer required for this case.Related Issue
Fixes micronaut-projects/micronaut-core#12838
Type of Change
Checklist
Testing
Focused coverage:
CompileTimeSourceGenSpec— globalNON_NULLand defaultNON_EMPTYomit null/empty properties while still selecting generated serializersSerdeableGeneratedSpec—NON_NULLomits null name fieldsinclusion=ALWAYSwhen they assert explicitnulloutput