diff --git a/release-notes/VERSION b/release-notes/VERSION index 93931d0846..e1f8831332 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -152,6 +152,10 @@ Versions: 3.x (for earlier see VERSION-2.x) (fix by @cowtowncoder, w/ Claude code) #4983: `JsonTypeInfo.Id.MINIMAL_CLASS` generates invalid type on sub-package (reported, fix by Benoit C-L) +#5281: Reading into existing instance uses creator property setup instead + of mutator (setter, field) + (reported by @odrotbohm) + (fix by @cowtowncoder, w/ Claude code) #5330: `@JsonProperty`on enum values with `@JsonFormat(shape = JsonFormat.Shape.NUMBER)` is ignored (reported by Christoffer H) diff --git a/src/main/java/tools/jackson/databind/deser/CreatorProperty.java b/src/main/java/tools/jackson/databind/deser/CreatorProperty.java index c17479b17a..bd8185661e 100644 --- a/src/main/java/tools/jackson/databind/deser/CreatorProperty.java +++ b/src/main/java/tools/jackson/databind/deser/CreatorProperty.java @@ -6,6 +6,7 @@ import tools.jackson.core.JacksonException; import tools.jackson.core.JsonParser; +import tools.jackson.core.JsonToken; import tools.jackson.databind.*; import tools.jackson.databind.exc.InvalidDefinitionException; import tools.jackson.databind.introspect.AnnotatedMember; @@ -17,9 +18,9 @@ /** * This concrete sub-class implements property that is passed * via Creator (constructor or static factory method). - * It is not a full-featured implementation in that its set method - * should usually not be called for primary mutation -- instead, value must separately passed -- - * but some aspects are still needed (specifically, injection). + * It is not a full-featured implementation in that its set method should + * usually not be called for primary mutation -- instead, value must be passed + * separately -- but some aspects are still needed (specifically, injection). *

* Note on injectable values: unlike with other mutators, where * deserializer and injecting are separate, here we treat the two as related @@ -56,6 +57,17 @@ public class CreatorProperty */ protected SettableBeanProperty _fallbackSetter; + /** + * Pre-computed flag that is {@code true} if {@link #_fallbackSetter}'s + * declared type matches this property's (creator parameter) type, so that + * the existing value deserializer can be reused when reading into an + * existing instance. Cached to avoid repeating the type comparison on + * every call to {@link #deserializeAndSet}/{@link #deserializeSetAndReturn}. + * + * @since 3.2 + */ + protected boolean _fallbackSetterTypeMatches; + protected final int _creatorIndex; /** @@ -111,6 +123,7 @@ protected CreatorProperty(CreatorProperty src, PropertyName newName) { _annotated = src._annotated; _injectableValue = src._injectableValue; _fallbackSetter = src._fallbackSetter; + _fallbackSetterTypeMatches = src._fallbackSetterTypeMatches; _creatorIndex = src._creatorIndex; _ignorable = src._ignorable; } @@ -121,6 +134,7 @@ protected CreatorProperty(CreatorProperty src, ValueDeserializer deser, _annotated = src._annotated; _injectableValue = src._injectableValue; _fallbackSetter = src._fallbackSetter; + _fallbackSetterTypeMatches = src._fallbackSetterTypeMatches; _creatorIndex = src._creatorIndex; _ignorable = src._ignorable; } @@ -131,6 +145,7 @@ protected CreatorProperty(CreatorProperty src, TypeDeserializer typeDeser) _annotated = src._annotated; _injectableValue = src._injectableValue; _fallbackSetter = src._fallbackSetter; + _fallbackSetterTypeMatches = src._fallbackSetterTypeMatches; _creatorIndex = src._creatorIndex; _ignorable = src._ignorable; } @@ -176,6 +191,8 @@ public void fixAccess(DeserializationConfig config) { */ public void setFallbackSetter(SettableBeanProperty fallbackSetter) { _fallbackSetter = fallbackSetter; + _fallbackSetterTypeMatches = (fallbackSetter != null) + && _type.equals(fallbackSetter.getType()); } @Override @@ -219,7 +236,7 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, Object instance) throws JacksonException { _verifySetter(); - _fallbackSetter.set(ctxt, instance, deserialize(p, ctxt)); + _fallbackSetter.set(ctxt, instance, _deserializeForSetter(p, ctxt)); } @Override @@ -227,7 +244,7 @@ public Object deserializeSetAndReturn(JsonParser p, DeserializationContext ctxt, Object instance) throws JacksonException { _verifySetter(); - return _fallbackSetter.setAndReturn(ctxt, instance, deserialize(p, ctxt)); + return _fallbackSetter.setAndReturn(ctxt, instance, _deserializeForSetter(p, ctxt)); } @Override @@ -294,6 +311,48 @@ public boolean isInjectionOnly() { /********************************************************************** */ + /** + * Helper method for {@code deserializeAndSet} and {@code deserializeSetAndReturn}: + * deserializes value using the fallback setter's type if it differs from the + * creator parameter type. + *

+ * [databind#5281]: When updating an existing instance, the creator parameter type + * (e.g. {@code String[]} from varargs) may differ from the setter/field type + * (e.g. {@code Collection}). Must deserialize using the setter's type + * to avoid {@code ClassCastException}. + * + * @since 3.2 + */ + private Object _deserializeForSetter(JsonParser p, DeserializationContext ctxt) + throws JacksonException + { + // Common case: types match, use this property's (already resolved) deserializer + if (_fallbackSetterTypeMatches) { + return deserialize(p, ctxt); + } + // Types differ: find deserializer for the fallback setter's type. + // Note: we use `_nullProvider` (this CreatorProperty's) rather than the + // fallback setter's: `BeanPropertyDefinition` merges annotations across + // accessors, so `@JsonSetter(nulls=...)` on the setter is already reflected + // here via `BeanDeserializerBase.resolve()`, whereas the fallback setter + // itself is stored as a raw `MethodProperty` and never contextualized. + if (p.hasToken(JsonToken.VALUE_NULL)) { + return _nullProvider.getNullValue(ctxt); + } + ValueDeserializer deser = ctxt.findContextualValueDeserializer( + _fallbackSetter.getType(), _fallbackSetter); + // If fallback setter has a TypeDeserializer (polymorphism via @JsonTypeInfo), + // honor it instead of plain deserialize() + final TypeDeserializer typeDeser = _fallbackSetter.getValueTypeDeserializer(); + Object value = (typeDeser == null) + ? deser.deserialize(p, ctxt) + : deser.deserializeWithType(p, ctxt, typeDeser); + if (value == null) { + value = _nullProvider.getNullValue(ctxt); + } + return value; + } + private final void _verifySetter() throws JacksonException { if (_fallbackSetter == null) { _reportMissingSetter(null, null); diff --git a/src/test/java/tools/jackson/databind/deser/creators/CreatorPropertyReaderForUpdating5281Test.java b/src/test/java/tools/jackson/databind/deser/creators/CreatorPropertyReaderForUpdating5281Test.java new file mode 100644 index 0000000000..3d82adf425 --- /dev/null +++ b/src/test/java/tools/jackson/databind/deser/creators/CreatorPropertyReaderForUpdating5281Test.java @@ -0,0 +1,74 @@ +package tools.jackson.databind.deser.creators; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; + +import org.junit.jupiter.api.Test; + +import tools.jackson.databind.ObjectMapper; +import tools.jackson.databind.json.JsonMapper; +import tools.jackson.databind.testutil.DatabindTestUtil; + +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; + +// [databind#5281] Reading into existing instance uses creator property setup instead +// of accessor (setter, field) +public class CreatorPropertyReaderForUpdating5281Test + extends DatabindTestUtil +{ + // Creator param type (String[]) differs from setter type (Collection) + public static class ArrayListHolder { + Collection values; + + public ArrayListHolder(String... values) { + this.values = new ArrayList<>(); + this.values.addAll(Arrays.asList(values)); + } + + public void setValues(Collection values) { + this.values = values; + } + } + + // Creator param type and setter type match: exercises the + // `_fallbackSetterTypeMatches == true` fast path + public static class MatchingTypeHolder { + Collection values; + + public MatchingTypeHolder(Collection values) { + this.values = (values == null) ? new ArrayList<>() : new ArrayList<>(values); + } + + public void setValues(Collection values) { + this.values = values; + } + } + + // Verify that readerForUpdating uses setter type (Collection), + // not creator parameter type (String[]), avoiding ClassCastException + @Test + public void readerForUpdatingUsesSetterType() throws Exception { + ObjectMapper mapper = JsonMapper.builder().build(); + + ArrayListHolder holder = mapper.readerForUpdating(new ArrayListHolder("A")) + .readValue("{ \"values\" : [ \"A\", \"B\" ]}"); + + // Setter replaces values (no merge), so we get the new array only + assertThat(holder.values).hasSize(2) + .containsExactly("A", "B"); + } + + // Regression: when creator and setter types match, the fast path still works + @Test + public void readerForUpdatingTypeMatchFastPath() throws Exception { + ObjectMapper mapper = JsonMapper.builder().build(); + + MatchingTypeHolder holder = mapper.readerForUpdating( + new MatchingTypeHolder(Arrays.asList("X"))) + .readValue("{ \"values\" : [ \"A\", \"B\" ]}"); + + assertThat(holder.values).hasSize(2) + .containsExactly("A", "B"); + } +} diff --git a/src/test/java/tools/jackson/databind/tofix/ReaderForUpdating5281Test.java b/src/test/java/tools/jackson/databind/tofix/ReaderForUpdating5281Test.java deleted file mode 100644 index 9b993f3e63..0000000000 --- a/src/test/java/tools/jackson/databind/tofix/ReaderForUpdating5281Test.java +++ /dev/null @@ -1,47 +0,0 @@ -package tools.jackson.databind.tofix; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; - -import org.junit.jupiter.api.Test; - -import tools.jackson.databind.ObjectMapper; -import tools.jackson.databind.json.JsonMapper; -import tools.jackson.databind.testutil.DatabindTestUtil; -import tools.jackson.databind.testutil.failure.JacksonTestFailureExpected; - -import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; - -// [databind#5281] Reading into existing instance uses creator property setup instead -// of accessor #5281 -public class ReaderForUpdating5281Test - extends DatabindTestUtil -{ - public static class ArrayListHolder { - // Works when annotated with... - // @JsonMerge - Collection values; - - public ArrayListHolder(String... values) { - this.values = new ArrayList<>(); - this.values.addAll(Arrays.asList(values)); - } - - public void setValues(Collection values) { - this.values = values; - } - } - - @JacksonTestFailureExpected - @Test - public void readsIntoCreator() throws Exception { - ObjectMapper mapper = JsonMapper.builder().build(); - - ArrayListHolder holder = mapper.readerForUpdating(new ArrayListHolder("A")) - .readValue("{ \"values\" : [ \"A\", \"B\" ]}"); - - assertThat(holder.values).hasSize(3) - .containsAll(Arrays.asList("A", "A", "B")); - } -}