Skip to content

Commit 6f0bf2a

Browse files
cowtowncodercowtowncoder
authored
Implement #4277: fail explicitly on unsupported combination (#5555)
1 parent 795f125 commit 6f0bf2a

File tree

4 files changed

+234
-0
lines changed

4 files changed

+234
-0
lines changed

‎release-notes/CREDITS‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ Victor Noël (@victornoel)
167167
`DeserializationFeature.USE_NULL_FOR_EMPTY_UNWRAPPED` enabled
168168
[3.1.0]
169169

170+
James Mudd (@jamesmudd)
171+
* Reported #4277: Deserialization `@JsonFormat(shape = JsonFormat.Shape.ARRAY)` POJO with
172+
`JsonTypeInfo.As.EXTERNAL_PROPERTY` does not work
173+
[3.1.0]
174+
170175
Viktor Szathmáry (@phraktle)
171176
* Reported #5115: `@JsonUnwrapped` Record deserialization can't handle name collision
172177
(reported by Viktor S)

‎release-notes/VERSION‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ Versions: 3.x (for earlier see VERSION-2.x)
4444
#3964: Deserialization issue: MismatchedInputException, Bean not
4545
yet resolved
4646
(reported by @detomarco)
47+
#4277: Deserialization `@JsonFormat(shape = JsonFormat.Shape.ARRAY)` POJO with
48+
`JsonTypeInfo.As.EXTERNAL_PROPERTY` does not work
49+
(reported by James M)
50+
(implemented by @cowtowncoder, w/ Claude code)
4751
#4629: `@JsonIncludeProperties` and `@JsonIgnoreProperties` ignored when
4852
deserializing Records
4953
(reported by Sim Y-T)

‎src/main/java/tools/jackson/databind/deser/bean/BeanDeserializerBase.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,17 @@ public ValueDeserializer<?> createContextual(DeserializationContext ctxt,
878878
shape = _serializationShape;
879879
}
880880
if (shape == JsonFormat.Shape.ARRAY) {
881+
// [databind#4277]: Detect incompatible combination of ARRAY shape with EXTERNAL_PROPERTY
882+
if (contextual._externalTypeIdHandler != null) {
883+
return (BeanDeserializerBase) ctxt.reportBadDefinition(_beanType,
884+
"""
885+
Cannot use `@JsonFormat(shape=JsonFormat.Shape.ARRAY)` with `@JsonTypeInfo(include=JsonTypeInfo.As.EXTERNAL_PROPERTY)` for type %s: `EXTERNAL_PROPERTY` requires object-style JSON with named properties, but `ARRAY` shape uses positional JSON arrays.
886+
Working alternatives:
887+
1. Use `@JsonTypeInfo(include=JsonTypeInfo.As.PROPERTY)` - type ID inside value object
888+
2. Use `@JsonTypeInfo(include=JsonTypeInfo.As.WRAPPER_ARRAY)` - wrap value with type
889+
3. Implement a custom deserializer for protocol-specific requirements
890+
""".formatted(ClassUtil.getTypeDescription(_beanType)));
891+
}
881892
contextual = contextual.asArrayDeserializer();
882893
}
883894
return contextual;

‎src/test/java/tools/jackson/databind/jsontype/ExternalPropertyWithArrayShape4277Test.java‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
package tools.jackson.databind.jsontype;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import com.fasterxml.jackson.annotation.*;
6+
import tools.jackson.databind.*;
7+
import tools.jackson.databind.exc.InvalidDefinitionException;
8+
import tools.jackson.databind.testutil.DatabindTestUtil;
9+
10+
import static org.junit.jupiter.api.Assertions.*;
11+
12+
/**
13+
* Tests for issue #4277: Combining {@code @JsonFormat(shape=ARRAY)} with
14+
* {@code @JsonTypeInfo(include=EXTERNAL_PROPERTY)} should fail with a clear
15+
* error message, as these features are architecturally incompatible.
16+
*/
17+
public class ExternalPropertyWithArrayShape4277Test extends DatabindTestUtil
18+
{
19+
// Base classes for polymorphism
20+
static class Animal {
21+
public String name;
22+
23+
protected Animal() { }
24+
public Animal(String n) { name = n; }
25+
}
26+
27+
@JsonTypeName("cat")
28+
static class Cat extends Animal {
29+
public Cat() { }
30+
public Cat(String n) { super(n); }
31+
}
32+
33+
@JsonTypeName("dog")
34+
static class Dog extends Animal {
35+
public Dog() { }
36+
public Dog(String n) { super(n); }
37+
}
38+
39+
// Test class combining ARRAY shape with EXTERNAL_PROPERTY (should fail)
40+
@JsonFormat(shape = JsonFormat.Shape.ARRAY)
41+
@JsonPropertyOrder({"type", "uniqueId", "animal"})
42+
static class WrapperWithExternalProperty {
43+
public String type;
44+
public String uniqueId;
45+
46+
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,
47+
include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
48+
property = "type")
49+
@JsonSubTypes({
50+
@JsonSubTypes.Type(value = Cat.class, name = "cat"),
51+
@JsonSubTypes.Type(value = Dog.class, name = "dog")
52+
})
53+
public Animal animal;
54+
55+
protected WrapperWithExternalProperty() { }
56+
public WrapperWithExternalProperty(String type, String id, Animal a) {
57+
this.type = type;
58+
this.uniqueId = id;
59+
this.animal = a;
60+
}
61+
}
62+
63+
// Test class with ARRAY shape and PROPERTY inclusion (should work)
64+
@JsonFormat(shape = JsonFormat.Shape.ARRAY)
65+
@JsonPropertyOrder({"uniqueId", "animal"})
66+
static class WrapperWithPropertyInclusion {
67+
public String uniqueId;
68+
69+
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,
70+
include = JsonTypeInfo.As.PROPERTY,
71+
property = "type")
72+
@JsonSubTypes({
73+
@JsonSubTypes.Type(value = Cat.class, name = "cat"),
74+
@JsonSubTypes.Type(value = Dog.class, name = "dog")
75+
})
76+
public Animal animal;
77+
78+
protected WrapperWithPropertyInclusion() { }
79+
public WrapperWithPropertyInclusion(String id, Animal a) {
80+
this.uniqueId = id;
81+
this.animal = a;
82+
}
83+
}
84+
85+
// Test class with ARRAY shape and WRAPPER_ARRAY inclusion (should work)
86+
@JsonFormat(shape = JsonFormat.Shape.ARRAY)
87+
@JsonPropertyOrder({"uniqueId", "animal"})
88+
static class WrapperWithWrapperArrayInclusion {
89+
public String uniqueId;
90+
91+
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,
92+
include = JsonTypeInfo.As.WRAPPER_ARRAY)
93+
@JsonSubTypes({
94+
@JsonSubTypes.Type(value = Cat.class, name = "cat"),
95+
@JsonSubTypes.Type(value = Dog.class, name = "dog")
96+
})
97+
public Animal animal;
98+
99+
protected WrapperWithWrapperArrayInclusion() { }
100+
public WrapperWithWrapperArrayInclusion(String id, Animal a) {
101+
this.uniqueId = id;
102+
this.animal = a;
103+
}
104+
}
105+
106+
107+
private final ObjectMapper MAPPER = jsonMapperBuilder()
108+
.registerSubtypes(Cat.class, Dog.class)
109+
.build();
110+
111+
/*
112+
/**********************************************************************
113+
/* Test methods
114+
/**********************************************************************
115+
*/
116+
117+
/**
118+
* Main test for issue #4277: combining EXTERNAL_PROPERTY with ARRAY shape
119+
* should fail with a clear error message.
120+
*/
121+
@Test
122+
public void testExternalPropertyWithArrayShapeFailsClearly() throws Exception
123+
{
124+
// Try to deserialize - should fail with clear error
125+
String json = "[\"cat\",\"id123\",{\"name\":\"Fluffy\"}]";
126+
127+
InvalidDefinitionException e = assertThrows(InvalidDefinitionException.class, () -> {
128+
MAPPER.readValue(json, WrapperWithExternalProperty.class);
129+
});
130+
131+
// Verify error message is present
132+
assertNotNull(e.getMessage());
133+
}
134+
135+
/**
136+
* Verify that the error message contains helpful information about
137+
* why the combination doesn't work and what alternatives exist.
138+
*/
139+
@Test
140+
public void testErrorMessageContainsAlternatives() throws Exception
141+
{
142+
String json = "[\"cat\",\"id123\",{\"name\":\"Fluffy\"}]";
143+
144+
InvalidDefinitionException e = assertThrows(InvalidDefinitionException.class, () -> {
145+
MAPPER.readValue(json, WrapperWithExternalProperty.class);
146+
});
147+
148+
String msg = e.getMessage();
149+
150+
// Check that error mentions both features
151+
assertTrue(msg.contains("ARRAY") || msg.contains("array"),
152+
"Error should mention ARRAY shape: " + msg);
153+
assertTrue(msg.contains("EXTERNAL_PROPERTY") || msg.contains("external"),
154+
"Error should mention EXTERNAL_PROPERTY: " + msg);
155+
156+
// Check that error mentions at least one alternative
157+
assertTrue(msg.contains("PROPERTY") || msg.contains("WRAPPER_ARRAY") ||
158+
msg.contains("alternative") || msg.contains("custom deserializer"),
159+
"Error should mention alternatives: " + msg);
160+
}
161+
162+
/**
163+
* Verify that ARRAY shape works fine with PROPERTY inclusion (type ID inside object).
164+
*/
165+
@Test
166+
public void testArrayShapeWithPropertyInclusion() throws Exception
167+
{
168+
WrapperWithPropertyInclusion input = new WrapperWithPropertyInclusion("id123",
169+
new Cat("Fluffy"));
170+
171+
// Serialize
172+
String json = MAPPER.writeValueAsString(input);
173+
174+
// JSON should be an array with embedded type
175+
assertTrue(json.startsWith("["), "Should be JSON array");
176+
177+
// Deserialize
178+
WrapperWithPropertyInclusion result = MAPPER.readValue(json,
179+
WrapperWithPropertyInclusion.class);
180+
181+
assertNotNull(result);
182+
assertEquals("id123", result.uniqueId);
183+
assertNotNull(result.animal);
184+
assertTrue(result.animal instanceof Cat);
185+
assertEquals("Fluffy", result.animal.name);
186+
}
187+
188+
/**
189+
* Verify that ARRAY shape works fine with WRAPPER_ARRAY inclusion.
190+
*/
191+
@Test
192+
public void testArrayShapeWithWrapperArrayInclusion() throws Exception
193+
{
194+
WrapperWithWrapperArrayInclusion input = new WrapperWithWrapperArrayInclusion("id123",
195+
new Cat("Fluffy"));
196+
197+
// Serialize
198+
String json = MAPPER.writeValueAsString(input);
199+
200+
// JSON should be an array
201+
assertTrue(json.startsWith("["), "Should be JSON array");
202+
203+
// Deserialize
204+
WrapperWithWrapperArrayInclusion result = MAPPER.readValue(json,
205+
WrapperWithWrapperArrayInclusion.class);
206+
207+
assertNotNull(result);
208+
assertEquals("id123", result.uniqueId);
209+
assertNotNull(result.animal);
210+
assertTrue(result.animal instanceof Cat);
211+
assertEquals("Fluffy", result.animal.name);
212+
}
213+
214+
}

0 commit comments

Comments
 (0)