diff --git a/blackbird/README.md b/blackbird/README.md index b4420ec1..8b898283 100644 --- a/blackbird/README.md +++ b/blackbird/README.md @@ -108,10 +108,15 @@ Deserialization (JSON to POJOs): ones do. Non-final fields (any visibility) store through constant field handles. Final fields keep stock behavior. - Records with their canonical constructor: typed locals in - canonical-constructor order and a direct constructor call. When the - document leaves out a component, the stock creator completes the - arguments (absent values such as `Optional.empty()`, required and - `FAIL_ON_MISSING_CREATOR_PROPERTIES` checks). + canonical-constructor order and a direct constructor call. A component + the document leaves out keeps the direct call when it is a `String`, + `int`, `long` or `boolean` read by the stock deserializer, or a nested + bean with a generated codec, unless it is required, has a null-handling + override (such as `@JsonSetter(nulls = ...)`), or + `FAIL_ON_MISSING_CREATOR_PROPERTIES` is enabled. Otherwise the stock + creator completes the arguments (absent values such as + `Optional.empty()`, required and `FAIL_ON_MISSING_CREATOR_PROPERTIES` + checks). - Builder-based beans (for example Immutables types): fluent setter calls through constant handles, and the build method called through a constant handle. diff --git a/blackbird/src/main/java/tools/jackson/module/blackbird/codegen/ReaderCodegen.java b/blackbird/src/main/java/tools/jackson/module/blackbird/codegen/ReaderCodegen.java index 02ca2f8a..a0246d07 100644 --- a/blackbird/src/main/java/tools/jackson/module/blackbird/codegen/ReaderCodegen.java +++ b/blackbird/src/main/java/tools/jackson/module/blackbird/codegen/ReaderCodegen.java @@ -50,6 +50,18 @@ public record GenProp(String name, Kind kind, SettableBeanProperty stock, public GenProp(String name, Kind kind, SettableBeanProperty stock) { this(name, kind, stock, null, null, null); } + + // Record mode: whether stock gives an absent component the local's + // initial value (see PropertyValueBuffer._findMissing). Only for + // deserializers whose absent value is known: the stock scalar ones + // (exact classes; absent is null, or zero for a primitive) and + // generated codecs (BeanDeserializerBase: null). A STOCK arm's + // deserializer may override getAbsentValue whatever its null access + // pattern, so it keeps the stock creator. + boolean absentIsDefault() { + return kind != Kind.STOCK && !stock.isRequired() + && stock.getNullValueProvider() == stock.getValueDeserializer(); + } } /** How the generated deserialize method obtains the value. */ diff --git a/blackbird/src/main/java/tools/jackson/module/blackbird/deser/BBReaderFactory.java b/blackbird/src/main/java/tools/jackson/module/blackbird/deser/BBReaderFactory.java index fd57ed45..68d4b858 100644 --- a/blackbird/src/main/java/tools/jackson/module/blackbird/deser/BBReaderFactory.java +++ b/blackbird/src/main/java/tools/jackson/module/blackbird/deser/BBReaderFactory.java @@ -58,7 +58,9 @@ */ final class BBReaderFactory { - // Exact classes: a subclass may read differently + // Exact classes: a subclass may read differently, or have another absent + // value (ReaderCodegen.GenProp.absentIsDefault relies on every non-STOCK + // kind using one of these, or a generated codec) private static final Set> STOCK_SCALAR_DESERS = Set.of( StringDeserializer.class, NumberDeserializers.IntegerDeserializer.class, diff --git a/blackbird/src/main/java/tools/jackson/module/blackbird/internal/GeneratedReaderBase.java b/blackbird/src/main/java/tools/jackson/module/blackbird/internal/GeneratedReaderBase.java index 32b12a9b..d4f1ff72 100644 --- a/blackbird/src/main/java/tools/jackson/module/blackbird/internal/GeneratedReaderBase.java +++ b/blackbird/src/main/java/tools/jackson/module/blackbird/internal/GeneratedReaderBase.java @@ -271,14 +271,15 @@ public void handleResolvedForwardReference(DeserializationContext ctxt, } } - // Record codecs call the canonical constructor directly only when the - // document supplied every component, every primitive component received - // its exact wrapper (not null or another type from a custom - // deserializer), and FAIL_ON_NULL_CREATOR_PROPERTIES is off. Everything - // else ends here: values holds the components (boxed, or the raw stock - // value, in creator-index order) and seen marks the ones the document - // supplied. Without a constructor failure, the stock creator completes the - // arguments (absent values, required and FAIL_ON_MISSING / + // Record codecs call the canonical constructor directly only when every + // component was supplied or takes its initial value when absent (and + // then FAIL_ON_MISSING_CREATOR_PROPERTIES is off), every primitive + // component received its exact wrapper (not null or another type from a + // custom deserializer), and FAIL_ON_NULL_CREATOR_PROPERTIES is off. + // Everything else ends here: values holds the components (boxed, or the + // raw stock value, in creator-index order) and seen marks the ones the + // document supplied. Without a constructor failure, the stock creator + // completes the arguments (absent values, required and FAIL_ON_MISSING / // FAIL_ON_NULL_CREATOR_PROPERTIES checks) and constructs; a failure from // the fast path goes where StdValueInstantiator sends it. Either way a // failure ends in wrapInstantiationProblem, as in the stock loop. diff --git a/blackbird/src/main/java25/tools/jackson/module/blackbird/codegen/BeanReaderGenerator.java b/blackbird/src/main/java25/tools/jackson/module/blackbird/codegen/BeanReaderGenerator.java index c3fbffac..ad54a4bf 100644 --- a/blackbird/src/main/java25/tools/jackson/module/blackbird/codegen/BeanReaderGenerator.java +++ b/blackbird/src/main/java25/tools/jackson/module/blackbird/codegen/BeanReaderGenerator.java @@ -665,12 +665,12 @@ private static void emitStockSet(CodeBuilder cob, int parser, int ctxt, int rece // Record mode: components collect into typed locals in canonical order. // At the end of the object the canonical constructor runs directly when - // the document supplied every component and nothing needs stock - // argument handling; otherwise, and when the constructor throws, the base - // finishes through the stock creator (see GeneratedReaderBase - // ._recordBuild). Names no component takes are handled, or buffered and - // replayed against the record, the way the stock property-based loop - // handles them. + // every component was supplied or takes its initial value when absent, + // and nothing needs stock argument handling; otherwise, and when the + // constructor throws, the base finishes through the stock creator (see + // GeneratedReaderBase._recordBuild). Names no component takes are + // handled, or buffered and replayed against the record, the way the + // stock property-based loop handles them. private static void buildRecordDeserialize(CodeBuilder cob, ReaderCodegen.Plan plan) { final ClassDesc base = CD_READER_BASE; final List props = plan.props(); @@ -682,11 +682,15 @@ private static void buildRecordDeserialize(CodeBuilder cob, ReaderCodegen.Plan p int next = 3; int[] componentSlot = new int[n]; boolean anyPrimitive = false; + long defaultAbsent = 0L; for (int i = 0; i < n; i++) { componentSlot[i] = next; Class t = props.get(i).type(); next += (t == long.class || t == double.class) ? 2 : 1; anyPrimitive |= t.isPrimitive(); + if (props.get(i).absentIsDefault()) { + defaultAbsent |= 1L << i; + } } final int matcherSlot = next++; final int ixSlot = next++; @@ -850,10 +854,29 @@ private static void buildRecordDeserialize(CodeBuilder cob, ReaderCodegen.Plan p Label finish = cob.newLabel(); cob.aconst_null().astore(pendingSlot); long allSeen = (n == 64) ? -1L : (1L << n) - 1; + // Direct only if every component was supplied or takes its initial + // value when absent, and with any of the latter absent, + // FAIL_ON_MISSING_CREATOR_PROPERTIES is off. cob.lload(seenSlot); + if (defaultAbsent != 0L) { + cob.loadConstant(defaultAbsent); + cob.lor(); + } cob.loadConstant(allSeen); cob.lcmp(); cob.ifne(slow); + if (defaultAbsent != 0L) { + Label complete = cob.newLabel(); + cob.lload(seenSlot); + cob.loadConstant(allSeen); + cob.lcmp(); + cob.ifeq(complete); + cob.aload(ctxt); + cob.getstatic(CD_DESER_FEATURE, "FAIL_ON_MISSING_CREATOR_PROPERTIES", CD_DESER_FEATURE); + cob.invokevirtual(CD_DESER_CONTEXT, "isEnabled", MTD_IS_ENABLED); + cob.ifne(slow); + cob.labelBinding(complete); + } if (anyPrimitive) { cob.lload(rawPrimSlot); cob.lconst_0(); @@ -883,7 +906,7 @@ private static void buildRecordDeserialize(CodeBuilder cob, ReaderCodegen.Plan p cob.labelBinding(ctorEnd); cob.astore(beanSlot); cob.goto_(finish); - // The constructor failed with every component present: the base + // The constructor failed with every argument in place: the base // reports it the way the stock instantiator would. cob.labelBinding(ctorHandler); cob.astore(pendingSlot); diff --git a/blackbird/src/test/java/tools/jackson/module/blackbird/deser/BBCodecRecordAbsentTest.java b/blackbird/src/test/java/tools/jackson/module/blackbird/deser/BBCodecRecordAbsentTest.java new file mode 100644 index 00000000..e7dbb091 --- /dev/null +++ b/blackbird/src/test/java/tools/jackson/module/blackbird/deser/BBCodecRecordAbsentTest.java @@ -0,0 +1,186 @@ +package tools.jackson.module.blackbird.deser; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.OptionalDouble; +import java.util.OptionalInt; +import java.util.OptionalLong; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; + +import tools.jackson.core.JacksonException; +import tools.jackson.core.JsonParser; +import tools.jackson.databind.DeserializationContext; +import tools.jackson.databind.DeserializationFeature; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.ObjectReader; +import tools.jackson.databind.annotation.JsonDeserialize; +import tools.jackson.databind.cfg.JsonNodeFeature; +import tools.jackson.databind.deser.std.StdScalarDeserializer; +import tools.jackson.databind.deser.std.StdValueInstantiator; +import tools.jackson.databind.json.JsonMapper; + +import tools.jackson.module.blackbird.BlackbirdTestBase; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Absent record components: the generated reader must give what stock gives, + * whether it constructs directly or through the stock creator. + */ +public class BBCodecRecordAbsentTest extends BlackbirdTestBase +{ + enum Color { RED } + + public record Child(String name) {} + + public record Scalars(String s, int i, long l, boolean b, Integer w, double d) {} + + // Only components whose absent value is the local's initial value: any + // subset may be absent and still take the direct constructor call. The + // constructor records whether the stock creator (StdValueInstantiator) + // called it. + public record Defaults(String s, int i, long l, boolean b, Child c) { + static boolean viaStock; + + public Defaults { + viaStock = StackWalker.getInstance().walk(frames -> frames.anyMatch( + f -> f.getClassName().equals(StdValueInstantiator.class.getName()))); + } + } + + public record Refs(Color e, Object o, Child c, List list, Map map) {} + + public record References(Optional opt, AtomicReference ref, JsonNode node) {} + + // [databind#6437]: absent scalar Optionals are empty, not null + public record ScalarOptionals(OptionalInt oi, OptionalLong ol, OptionalDouble od, String s) {} + + // Keeps the inherited ALWAYS_NULL null access (getNullValue is null), but + // its absent value is not null + public static class DefaultingDeserializer extends StdScalarDeserializer { + public DefaultingDeserializer() { super(String.class); } + + @Override + public String deserialize(JsonParser p, DeserializationContext ctxt) { + return p.getString(); + } + + @Override + public Object getAbsentValue(DeserializationContext ctxt) { + return "absent"; + } + } + + public record CustomAbsent(String s, + @JsonDeserialize(using = DefaultingDeserializer.class) String t) {} + + public record NullHandling(@JsonSetter(nulls = Nulls.AS_EMPTY) List empty, String s) {} + + public record NullsFail(@JsonSetter(nulls = Nulls.FAIL) String fail, String s) {} + + public record Required(@JsonProperty(required = true) String req, String s) {} + + private static final List> TYPES = List.of(Scalars.class, Defaults.class, Refs.class, + References.class, ScalarOptionals.class, CustomAbsent.class, NullHandling.class, + NullsFail.class, Required.class); + + private static final List DOCS = List.of("{}", "{\"s\":\"x\"}", + "{\"s\":\"x\",\"i\":1,\"l\":2,\"b\":true,\"w\":3,\"d\":4.5}", + "{\"e\":\"RED\",\"o\":1,\"c\":{\"name\":\"n\"},\"list\":[\"a\"],\"map\":{\"k\":1}}", + "{\"opt\":\"v\",\"ref\":\"r\",\"node\":{\"a\":1}}", + "{\"oi\":1,\"ol\":2,\"od\":3.5}", + "{\"req\":\"r\"}", + "{\"i\":1,\"c\":{\"name\":\"n\"}}", + "{\"s\":\"x\",\"l\":2,\"b\":true}"); + + // Each of these leaves out only Defaults components that take their + // initial value, so the generated reader constructs directly. + private static final List DIRECT_DOCS = List.of("{}", "{\"s\":\"x\"}", + "{\"i\":1,\"c\":{\"name\":\"n\"}}", "{\"s\":\"x\",\"l\":2,\"b\":true}", + "{\"s\":\"x\",\"i\":1,\"l\":2,\"b\":true,\"c\":{\"name\":\"n\"}}"); + + @Test + public void testDefaults() { + assertParity(JsonMapper.builder(), mapperBuilder()); + } + + @Test + public void testFeatures() { + for (DeserializationFeature f : List.of( + DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES, + DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES, + DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES, + DeserializationFeature.USE_NULL_FOR_MISSING_REFERENCE_VALUES)) { + assertParity(JsonMapper.builder().enable(f), mapperBuilder().enable(f)); + } + assertParity(JsonMapper.builder().enable(JsonNodeFeature.MAP_ABSENT_TO_MISSING), + mapperBuilder().enable(JsonNodeFeature.MAP_ABSENT_TO_MISSING)); + } + + @Test + public void testDirectConstruction() { + JsonMapper vanilla = JsonMapper.builder().build(); + JsonMapper bb = mapperBuilder().build(); + assertGeneratedReader(bb, Defaults.class); + for (String doc : DIRECT_DOCS) { + String expected = read(vanilla, Defaults.class, doc); + assertTrue(Defaults.viaStock, "stock should construct through its creator"); + assertEquals(expected, read(bb, Defaults.class, doc), doc); + assertFalse(Defaults.viaStock, "should construct directly: " + doc); + } + } + + // The feature is checked per call: one mapper, so one generated reader, + // reads the same documents with and without it. + @Test + public void testPerCallFailOnMissing() { + ObjectReader vanilla = JsonMapper.builder().build().readerFor(Defaults.class); + JsonMapper bbMapper = mapperBuilder().build(); + assertGeneratedReader(bbMapper, Defaults.class); + ObjectReader bb = bbMapper.readerFor(Defaults.class); + DeserializationFeature f = DeserializationFeature.FAIL_ON_MISSING_CREATOR_PROPERTIES; + for (String doc : DIRECT_DOCS) { + assertEquals(read(vanilla, doc), read(bb, doc), doc); + assertEquals(read(vanilla.with(f), doc), read(bb.with(f), doc), doc); + assertEquals(read(vanilla, doc), read(bb, doc), doc); + } + // With every component present, the feature does not leave the direct path + String full = DIRECT_DOCS.get(DIRECT_DOCS.size() - 1); + read(bb.with(f), full); + assertFalse(Defaults.viaStock); + // and with one absent, it fails + assertNotEquals(read(bb, "{}"), read(bb.with(f), "{}")); + } + + private static void assertParity(JsonMapper.Builder stock, JsonMapper.Builder blackbird) { + JsonMapper vanilla = stock.build(); + JsonMapper bb = blackbird.build(); + for (Class type : TYPES) { + assertGeneratedReader(bb, type); + for (String doc : DOCS) { + assertEquals(read(vanilla, type, doc), read(bb, type, doc), + type.getSimpleName() + " " + doc); + } + } + } + + // toString, as AtomicReference has identity equality; the exception class on failure. + private static String read(JsonMapper mapper, Class type, String doc) { + return read(mapper.readerFor(type), doc); + } + + private static String read(ObjectReader reader, String doc) { + try { + return String.valueOf((Object) reader.readValue(doc)); + } catch (JacksonException e) { + return e.getClass().getName(); + } + } +} diff --git a/release-notes/CREDITS b/release-notes/CREDITS index d0188a1e..4be46965 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -53,3 +53,6 @@ Parker Winchester (@parker89) Viktor Szathmáry (@phraktle) * Contributed fix for #378: (blackbird) Record codegen gated off for generic record components (3.3.1) +* Contributed fix for #380: (blackbird) Record codegen falls back to stock creator + when a component is absent + (3.3.1) diff --git a/release-notes/VERSION b/release-notes/VERSION index 84b3e49f..dffe3823 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -26,6 +26,9 @@ Active maintainers: #378: (blackbird) Record codegen gated off for generic record components (contributed by @phraktle) +#380: (blackbird) Record codegen falls back to stock creator when a component + is absent + (contributed by @phraktle) 3.3.0 (07-Nov-2026)