Skip to content

Commit be0548d

Browse files
authored
Restore inner property name from map key in handleUnwrapped (#5193)
* Restore property name from map key in handleUnwrapped to avoid null keys `ModelResolver.handleUnwrapped` forwards an inner model's property schemas into the outer model's `props` list. Because `Schema.getName()` is `@JsonIgnore`, an inner property schema can arrive with `name == null` (e.g. after a JSON-based clone such as `AnnotationsUtils.clone`) while the inner properties-map key still carries the correct name. The subsequent `modelProps.put(prop.getName(), prop)` then inserts a `null` key, which makes Jackson fail to serialize the document with `JsonMappingException: Null key for a Map not allowed in JSON`. Make `handleUnwrapped` defensive: iterate the inner properties by entry and restore the property name from the authoritative map key when the schema's transient name has been lost. The prefix/suffix branch falls back to the entry key the same way instead of producing a literal `prefix + "null" + suffix`. Relates to #5126 (reported with Spring HATEOAS `EntityModel<T>`). The exact end-to-end trigger could not be reproduced from a plain annotated model, so this is intentionally a defensive guard on `handleUnwrapped`'s output rather than a fix targeting one specific resolver path. Covered by a unit test that drives `handleUnwrapped` with inner property schemas in the post-clone (null-name) state, asserting non-null keys with and without a prefix/suffix.
1 parent f3de17d commit be0548d

2 files changed

Lines changed: 186 additions & 4 deletions

File tree

modules/swagger-core/src/main/java/io/swagger/v3/core/jackson/ModelResolver.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@
9595
import java.math.BigDecimal;
9696
import java.util.ArrayList;
9797
import java.util.Arrays;
98-
import java.util.Collection;
9998
import java.util.Collections;
10099
import java.util.HashMap;
101100
import java.util.HashSet;
@@ -1461,7 +1460,18 @@ protected boolean ignore(final Annotated member, final XmlAccessorType xmlAccess
14611460
private void handleUnwrapped(List<Schema> props, Schema innerModel, String prefix, String suffix, List<String> requiredProps) {
14621461
if (StringUtils.isBlank(suffix) && StringUtils.isBlank(prefix)) {
14631462
if (innerModel.getProperties() != null) {
1464-
props.addAll(innerModel.getProperties().values());
1463+
// Schema.getName() is @JsonIgnore, so any prior JSON-based clone of innerModel
1464+
// (e.g. AnnotationsUtils.clone) leaves nested property schemas with a null name
1465+
// while the properties-map key still carries the correct name. Restore from the
1466+
// map key so the eventual `modelProps.put(prop.getName(), prop)` does not insert
1467+
// a null key (see swagger-api/swagger-core#5126).
1468+
for (Map.Entry<String, Schema> entry : ((Map<String, Schema>) innerModel.getProperties()).entrySet()) {
1469+
Schema prop = entry.getValue();
1470+
if (prop.getName() == null) {
1471+
prop.setName(entry.getKey());
1472+
}
1473+
props.add(prop);
1474+
}
14651475
if (innerModel.getRequired() != null) {
14661476
requiredProps.addAll(innerModel.getRequired());
14671477
}
@@ -1475,10 +1485,14 @@ private void handleUnwrapped(List<Schema> props, Schema innerModel, String prefi
14751485
suffix = "";
14761486
}
14771487
if (innerModel.getProperties() != null) {
1478-
for (Schema prop : (Collection<Schema>) innerModel.getProperties().values()) {
1488+
for (Map.Entry<String, Schema> entry : ((Map<String, Schema>) innerModel.getProperties()).entrySet()) {
1489+
Schema prop = entry.getValue();
14791490
try {
14801491
Schema clonedProp = Json.mapper().readValue(Json.pretty(prop), Schema.class);
1481-
clonedProp.setName(prefix + prop.getName() + suffix);
1492+
// Fall back to the map key when the prop's transient name has been lost
1493+
// by a prior clone (Schema.getName() is @JsonIgnore).
1494+
String baseName = prop.getName() != null ? prop.getName() : entry.getKey();
1495+
clonedProp.setName(prefix + baseName + suffix);
14821496
props.add(clonedProp);
14831497
} catch (IOException e) {
14841498
LOGGER.error("Exception cloning property", e);
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
package io.swagger.v3.core.resolving;
2+
3+
import com.fasterxml.jackson.databind.ObjectMapper;
4+
import io.swagger.v3.core.util.AnnotationsUtils;
5+
import io.swagger.v3.core.util.Json;
6+
import io.swagger.v3.core.jackson.ModelResolver;
7+
import io.swagger.v3.oas.models.media.Schema;
8+
import org.testng.annotations.BeforeMethod;
9+
import org.testng.annotations.Test;
10+
11+
import java.lang.reflect.Method;
12+
import java.util.ArrayList;
13+
import java.util.Arrays;
14+
import java.util.LinkedHashMap;
15+
import java.util.LinkedHashSet;
16+
import java.util.List;
17+
import java.util.Map;
18+
19+
import static org.testng.Assert.assertEquals;
20+
import static org.testng.Assert.assertFalse;
21+
import static org.testng.Assert.assertNotNull;
22+
import static org.testng.Assert.assertNull;
23+
24+
/**
25+
* Defensive invariant tests related to
26+
* <a href="https://github.com/swagger-api/swagger-core/issues/5126">#5126</a>.
27+
*
28+
* <p>{@link ModelResolver#handleUnwrapped} must not forward null-named property schemas into the
29+
* outer model's properties list. The later {@code modelProps.put(prop.getName(), prop)} step uses
30+
* those names as map keys, and a {@code null} key makes Jackson fail when serializing the schema.
31+
*
32+
* <p>These tests exercise the private method directly because the reported Spring HATEOAS path has
33+
* not been reproduced as a swagger-core-only resolver test. The clone-based case demonstrates the
34+
* concrete intermediate state this guard is intended to tolerate: {@code AnnotationsUtils.clone}
35+
* keeps the properties-map keys while losing nested {@code Schema.name} values because
36+
* {@code Schema.getName()} is {@code @JsonIgnore}.
37+
*/
38+
public class Ticket5126Test extends SwaggerTestBase {
39+
40+
private ModelResolver modelResolver;
41+
private Method handleUnwrapped;
42+
43+
@BeforeMethod
44+
public void setup() throws Exception {
45+
modelResolver = new ModelResolver(new ObjectMapper());
46+
handleUnwrapped = ModelResolver.class.getDeclaredMethod(
47+
"handleUnwrapped", List.class, Schema.class, String.class, String.class, List.class);
48+
handleUnwrapped.setAccessible(true);
49+
}
50+
51+
@Test
52+
public void noPrefixOrSuffix_restoresNameFromClonedInnerPropertiesMapKey() throws Exception {
53+
Schema<Object> inner = cloneLosingNestedPropertyNames(namedInnerSchema());
54+
55+
assertEquals(inner.getProperties().keySet(), new LinkedHashSet<>(Arrays.asList("name", "count")));
56+
assertNull(((Schema<?>) inner.getProperties().get("name")).getName());
57+
assertNull(((Schema<?>) inner.getProperties().get("count")).getName());
58+
59+
List<Schema> props = invokeHandleUnwrapped(inner, "", "");
60+
61+
assertPropertyNames(props, "name", "count");
62+
Schema<Object> outer = schemaWithModelProps(props);
63+
assertFalse(outer.getProperties().containsKey(null));
64+
Json.mapper().writeValueAsString(outer);
65+
}
66+
67+
@Test
68+
public void noPrefixOrSuffix_restoresNameFromInnerPropertiesMapKeyForNullNameState() throws Exception {
69+
Schema<Object> inner = innerSchemaWithNullPropertyNames();
70+
71+
List<Schema> props = invokeHandleUnwrapped(inner, "", "");
72+
73+
assertPropertyNames(props, "name", "count");
74+
}
75+
76+
@Test
77+
public void noPrefixOrSuffix_preservesExistingPropertyName() throws Exception {
78+
Schema<?> nameSchema = new Schema<>().type("string");
79+
nameSchema.setName("schemaName");
80+
Schema<Object> inner = new Schema<>();
81+
Map<String, Schema> innerProps = new LinkedHashMap<>();
82+
innerProps.put("mapName", nameSchema);
83+
inner.setProperties(innerProps);
84+
85+
List<Schema> props = invokeHandleUnwrapped(inner, "", "");
86+
87+
assertPropertyNames(props, "schemaName");
88+
}
89+
90+
@Test
91+
public void withPrefixAndSuffix_combinesMapKeyWhenClonedInnerNameIsNull() throws Exception {
92+
Schema<Object> inner = cloneLosingNestedPropertyNames(namedInnerSchema());
93+
94+
List<Schema> props = invokeHandleUnwrapped(inner, "p_", "_s");
95+
96+
assertPropertyNames(props, "p_name_s", "p_count_s");
97+
Schema<Object> outer = schemaWithModelProps(props);
98+
assertFalse(outer.getProperties().containsKey(null));
99+
Json.mapper().writeValueAsString(outer);
100+
}
101+
102+
@Test
103+
public void withPrefixAndSuffix_combinesMapKeyWhenInnerNameIsNull() throws Exception {
104+
Schema<Object> inner = innerSchemaWithNullPropertyNames();
105+
106+
List<Schema> props = invokeHandleUnwrapped(inner, "p_", "_s");
107+
108+
assertPropertyNames(props, "p_name_s", "p_count_s");
109+
}
110+
111+
private Schema<Object> namedInnerSchema() {
112+
Schema<?> nameSchema = new Schema<>().type("string");
113+
nameSchema.setName("name");
114+
Schema<?> countSchema = new Schema<>().type("integer");
115+
countSchema.setName("count");
116+
117+
Schema<Object> inner = new Schema<>();
118+
inner.setName("Inner");
119+
Map<String, Schema> innerProps = new LinkedHashMap<>();
120+
innerProps.put("name", nameSchema);
121+
innerProps.put("count", countSchema);
122+
inner.setProperties(innerProps);
123+
return inner;
124+
}
125+
126+
private Schema<Object> innerSchemaWithNullPropertyNames() {
127+
Schema<?> nameSchema = new Schema<>().type("string");
128+
Schema<?> countSchema = new Schema<>().type("integer");
129+
Schema<Object> inner = new Schema<>();
130+
Map<String, Schema> innerProps = new LinkedHashMap<>();
131+
innerProps.put("name", nameSchema);
132+
innerProps.put("count", countSchema);
133+
inner.setProperties(innerProps);
134+
return inner;
135+
}
136+
137+
private Schema<Object> cloneLosingNestedPropertyNames(Schema<Object> inner) {
138+
return AnnotationsUtils.clone(inner, false);
139+
}
140+
141+
private List<Schema> invokeHandleUnwrapped(Schema<Object> inner, String prefix, String suffix) throws Exception {
142+
List<Schema> props = new ArrayList<>();
143+
List<String> requiredProps = new ArrayList<>();
144+
handleUnwrapped.invoke(modelResolver, props, inner, prefix, suffix, requiredProps);
145+
return props;
146+
}
147+
148+
private Schema<Object> schemaWithModelProps(List<Schema> props) {
149+
Schema<Object> outer = new Schema<>();
150+
Map<String, Schema> modelProps = new LinkedHashMap<>();
151+
for (Schema prop : props) {
152+
modelProps.put(prop.getName(), prop);
153+
}
154+
outer.setProperties(modelProps);
155+
return outer;
156+
}
157+
158+
private void assertPropertyNames(List<Schema> props, String... expectedNames) {
159+
assertEquals(props.size(), expectedNames.length);
160+
LinkedHashSet<String> names = new LinkedHashSet<>();
161+
for (Schema p : props) {
162+
assertNotNull(p.getName(),
163+
"Each unwrapped property must carry a non-null name to avoid null keys in the outer's properties map");
164+
names.add(p.getName());
165+
}
166+
assertEquals(names, new LinkedHashSet<>(Arrays.asList(expectedNames)));
167+
}
168+
}

0 commit comments

Comments
 (0)