Skip to content

Commit d3535d1

Browse files
ISSUE-512: Refactor @JsonWrapped implementation - simplify wrapper contract and validation
Cleaned up the @JsonWrapped annotation implementation to use a three-value return contract for the wrapper name: null (not present), empty string (explicitly disabled in mix-ins), or non-empty string (active wrapper). Removed the redundant 'enabled' flag and simplified validation logic in BeanDeserializerBase. Updated WrappedPropertyHandler method signatures. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent 3594e63 commit d3535d1

11 files changed

Lines changed: 65 additions & 83 deletions

src/main/java/tools/jackson/databind/AnnotationIntrospector.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,8 +513,13 @@ public Object findTypeIdResolver(MapperConfig<?> config, Annotated ann) {
513513

514514
/**
515515
* Method for finding the wrapped group name for a member annotated
516-
* with {@code @JsonWrapped}. Returns the wrapper object name if the
517-
* member should be wrapped, or {@code null} if not.
516+
* with {@code @JsonWrapped}. Three-value return contract:
517+
* <ul>
518+
* <li>{@code null} — annotation not present; secondary introspector may supply a value</li>
519+
* <li>{@code ""} (empty string) — annotation present but explicitly disabled;
520+
* secondary introspector must NOT override</li>
521+
* <li>non-empty string — active wrapper name</li>
522+
* </ul>
518523
*
519524
* @since 3.1
520525
*/

src/main/java/tools/jackson/databind/annotation/JsonWrapped.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,11 @@
4141
*
4242
* <p>Constraints:
4343
* <ul>
44-
* <li>Only scalar and primitive types are supported as wrapped fields
45-
* (containers, maps, arrays, and nested POJOs will cause a mapping error).</li>
46-
* <li>The wrapper name ({@code value()}) must be non-empty.</li>
44+
* <li>MVP limitation: only scalar and primitive types are currently supported as wrapped
45+
* fields (containers, maps, arrays, and nested POJOs are not yet supported).</li>
46+
* <li>The wrapper name ({@code value()}) must be non-empty, unless explicitly disabling
47+
* wrapping: an empty {@code value()} ({@code @JsonWrapped("")}) disables wrapping —
48+
* useful in mix-ins to suppress wrapping defined in a supertype.</li>
4749
* <li>The wrapper name must not conflict with an existing non-wrapped property on the same bean.</li>
4850
* <li>Not supported on {@code @JsonCreator} constructor or factory-method parameters.</li>
4951
* <li>MVP limitation: {@code @JsonView} on inner wrapped fields is ignored — the wrapper
@@ -64,12 +66,8 @@
6466
public @interface JsonWrapped {
6567
/**
6668
* Single-level wrapper object name (e.g. "chr").
67-
* Must be non-empty.
69+
* An empty string disables wrapping (useful in mix-ins to suppress
70+
* wrapping defined in a supertype).
6871
*/
6972
String value();
70-
71-
/**
72-
* Allows conditional disabling of wrapping.
73-
*/
74-
boolean enabled() default true;
7573
}

src/main/java/tools/jackson/databind/deser/bean/BeanDeserializer.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1418,8 +1418,7 @@ protected Object deserializeWithWrapped(JsonParser p, DeserializationContext ctx
14181418
p.nextToken();
14191419

14201420
if (_wrappedPropertyHandler.hasWrapperName(propName)) {
1421-
_wrappedPropertyHandler.handleWrappedObject(p, ctxt, bean,
1422-
propName, _anySetter);
1421+
_wrappedPropertyHandler.handleWrappedObject(p, ctxt, bean, propName);
14231422
continue;
14241423
}
14251424

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -669,17 +669,14 @@ public void resolve(DeserializationContext ctxt)
669669
Set<SettableBeanProperty> wrappedPropSet = new HashSet<>();
670670
for (SettableBeanProperty prop : allProps) {
671671
AnnotatedMember member = prop.getMember();
672-
if (member == null) continue;
672+
if (member == null) {
673+
continue;
674+
}
673675
String wrapperName = intr.findWrappedGroupName(ctxt.getConfig(), member);
674-
if (wrapperName == null) continue;
675-
wrappedPropSet.add(prop);
676-
677-
// Validate: empty name
678-
if (wrapperName.isEmpty()) {
679-
ctxt.reportBadDefinition(handledType(),
680-
String.format("@JsonWrapped value must not be empty (on property '%s')",
681-
prop.getName()));
676+
if (wrapperName == null || wrapperName.isEmpty()) {
677+
continue; // not wrapped, or explicitly disabled via @JsonWrapped("")
682678
}
679+
wrappedPropSet.add(prop);
683680

684681
// Validate: creator parameter
685682
if ((prop instanceof CreatorProperty) && ((CreatorProperty) prop).getCreatorIndex() >= 0) {

src/main/java/tools/jackson/databind/deser/impl/WrappedPropertyHandler.java

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import tools.jackson.core.*;
66
import tools.jackson.databind.*;
7-
import tools.jackson.databind.deser.SettableAnyProperty;
87
import tools.jackson.databind.deser.SettableBeanProperty;
98

109
/**
@@ -65,15 +64,13 @@ public boolean hasWrapperName(String name) {
6564
/**
6665
* Deserialize all inner properties from a wrapper object.
6766
*
68-
* @param p Parser positioned at START_OBJECT of the wrapper
69-
* @param ctxt Deserialization context
70-
* @param bean The target bean being populated
67+
* @param p Parser positioned at START_OBJECT of the wrapper
68+
* @param ctxt Deserialization context
69+
* @param bean The target bean being populated
7170
* @param wrapperName The wrapper field name (for error messages)
72-
* @param anySetter Optional any-setter from the parent bean (may be null)
7371
*/
7472
public void handleWrappedObject(JsonParser p, DeserializationContext ctxt,
75-
Object bean, String wrapperName,
76-
/* nullable */ SettableAnyProperty anySetter)
73+
Object bean, String wrapperName)
7774
throws JacksonException
7875
{
7976
JsonToken t = p.currentToken();
@@ -91,22 +88,13 @@ public void handleWrappedObject(JsonParser p, DeserializationContext ctxt,
9188
return;
9289
}
9390

94-
// Retrieve precomputed inner property lookup
91+
// Invariant: caller checked hasWrapperName() so this cannot be null
9592
Map<String, SettableBeanProperty> innerLookup = _innerLookups.get(wrapperName);
96-
if (innerLookup == null) {
97-
innerLookup = Collections.emptyMap();
98-
}
99-
100-
// Iterate inner properties
101-
while ((t = p.nextToken()) != JsonToken.END_OBJECT) {
102-
if (t != JsonToken.PROPERTY_NAME) {
103-
ctxt.reportWrongTokenException(bean.getClass(), JsonToken.PROPERTY_NAME,
104-
"Unexpected token inside wrapped object '%s'", wrapperName);
105-
return;
106-
}
107-
String innerName = p.currentName();
108-
p.nextToken(); // move to value
10993

94+
// Iterate inner properties using idiomatic Jackson pattern
95+
String innerName;
96+
while ((innerName = p.nextName()) != null) {
97+
p.nextToken(); // advance to value token
11098
SettableBeanProperty innerProp = innerLookup.get(innerName);
11199
if (innerProp != null) {
112100
try {
@@ -116,19 +104,15 @@ public void handleWrappedObject(JsonParser p, DeserializationContext ctxt,
116104
new JacksonException.Reference(bean, innerName));
117105
}
118106
} else {
119-
// Unknown inner property
120-
if (anySetter != null) {
121-
try {
122-
anySetter.deserializeAndSet(p, ctxt, bean, innerName);
123-
} catch (Exception e) {
124-
throw DatabindException.wrapWithPath(ctxt, e,
125-
new JacksonException.Reference(bean, innerName));
126-
}
127-
} else {
128-
handleUnknownInnerProperty(p, ctxt, bean, wrapperName, innerName);
129-
}
107+
// Unknown inner property: use standard unknown-property handling (NOT outer anySetter)
108+
handleUnknownInnerProperty(p, ctxt, bean, wrapperName, innerName);
130109
}
131110
}
111+
// Verify we consumed until END_OBJECT for malformed input strictness
112+
if (p.currentToken() != JsonToken.END_OBJECT) {
113+
ctxt.reportWrongTokenException(bean.getClass(), JsonToken.END_OBJECT,
114+
"Expected END_OBJECT after wrapped group '%s'", wrapperName);
115+
}
132116
}
133117

134118
@SuppressWarnings("unused") // wrapperName available for subclass overrides

src/main/java/tools/jackson/databind/introspect/AnnotationIntrospectorPair.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,11 @@ public NameTransformer findUnwrappingNameTransformer(MapperConfig<?> config, Ann
262262
@Override
263263
public String findWrappedGroupName(MapperConfig<?> config, AnnotatedMember member) {
264264
String r = _primary.findWrappedGroupName(config, member);
265-
return (r == null) ? _secondary.findWrappedGroupName(config, member) : r;
265+
// null = not present → try secondary; "" = explicitly disabled → don't fall through
266+
if (r == null) {
267+
return _secondary.findWrappedGroupName(config, member);
268+
}
269+
return r;
266270
}
267271

268272
@Override

src/main/java/tools/jackson/databind/introspect/JacksonAnnotationIntrospector.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -520,10 +520,10 @@ public NameTransformer findUnwrappingNameTransformer(MapperConfig<?> config, Ann
520520
@Override
521521
public String findWrappedGroupName(MapperConfig<?> config, AnnotatedMember member) {
522522
JsonWrapped ann = _findAnnotation(member, JsonWrapped.class);
523-
if (ann == null || !ann.enabled()) {
523+
if (ann == null) {
524524
return null;
525525
}
526-
return ann.value();
526+
return ann.value(); // "" means disabled; non-empty means active
527527
}
528528

529529
@Override // since 2.9

src/main/java/tools/jackson/databind/ser/BeanSerializerFactory.java

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -909,20 +909,13 @@ protected List<BeanPropertyWriter> _groupWrappedProperties(SerializationContext
909909
continue;
910910
}
911911
String wrapperName = intr.findWrappedGroupName(config, member);
912-
if (wrapperName == null) {
913-
continue;
912+
if (wrapperName == null || wrapperName.isEmpty()) {
913+
continue; // not wrapped, or explicitly disabled via @JsonWrapped("")
914914
}
915915

916916
// Cache the wrapper name for later use
917917
wrapperNameCache.put(prop, wrapperName);
918918

919-
// Validate: empty name
920-
if (wrapperName.isEmpty()) {
921-
ctxt.reportBadTypeDefinition(beanDescRef,
922-
"@JsonWrapped value must not be empty (on property '%s')", prop.getName());
923-
continue;
924-
}
925-
926919
// Validate: scalar-only
927920
JavaType type = prop.getType();
928921
if (!_isScalarType(type)) {

src/test/java/tools/jackson/databind/struct/JsonWrappedDeserializationTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ static class MultiWrapperBean {
7272
public MultiWrapperBean() { }
7373
}
7474

75-
private final ObjectMapper MAPPER = JsonMapper.builder().build();
75+
private final ObjectMapper MAPPER = newJsonMapper();
7676

7777
@Nested
7878
@DisplayName("basic deserialization")
@@ -194,7 +194,7 @@ void trueRoundTripMultipleWrappers() throws Exception {
194194
class UnknownInnerPropertyTests {
195195

196196
@Test
197-
@DisplayName("should collect unknown inner properties when @JsonAnySetter present")
197+
@DisplayName("should NOT route unknown inner properties to outer @JsonAnySetter")
198198
public void testUnknownInnerWithAnySetter() throws Exception
199199
{
200200
// setup
@@ -203,10 +203,10 @@ public void testUnknownInnerWithAnySetter() throws Exception
203203
// when
204204
BeanWithAnySetter bean = MAPPER.readValue(json, BeanWithAnySetter.class);
205205

206-
// then
206+
// then: unknown inner props are ignored (not sent to outer anySetter)
207207
assertThat(bean.symbol).isEqualTo("TP53");
208208
assertThat(bean.chrId).isEqualTo("17");
209-
assertThat(bean.extra.get("extra")).isEqualTo("val");
209+
assertThat(bean.extra).isEmpty();
210210
}
211211

212212
@Test

src/test/java/tools/jackson/databind/struct/JsonWrappedSerializationTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public MultiWrapper(int a, int b, int c, int d) {
7474
}
7575

7676
static class DisabledWrapping {
77-
@JsonWrapped(value = "w", enabled = false)
77+
@JsonWrapped("") // "" = explicitly disabled
7878
public int x;
7979

8080
public DisabledWrapping() { }
@@ -254,12 +254,12 @@ void wrapperWithJsonPropertyOrder() throws Exception
254254
}
255255

256256
@Nested
257-
@DisplayName("enabled flag tests")
258-
class EnabledFlagTests {
257+
@DisplayName("disabled wrapping tests")
258+
class DisabledWrappingTests {
259259

260260
@Test
261-
@DisplayName("should serialize flat when enabled=false")
262-
void enabledFalse() throws Exception
261+
@DisplayName("should serialize flat when @JsonWrapped(\"\") disables wrapping")
262+
void emptyValueDisablesWrapping() throws Exception
263263
{
264264
// setup
265265
DisabledWrapping bean = new DisabledWrapping(42);

0 commit comments

Comments
 (0)