Skip to content

Commit ecfae9d

Browse files
frantumafrantuma
authored
Merge pull request #3262 from swagger-api/fix_inheritance
 refs #3197 - resolve properties within allOf for composed schemas
2 parents 5ea42ce + cb35fcc commit ecfae9d

File tree

13 files changed

+529
-144
lines changed

13 files changed

+529
-144
lines changed

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import io.swagger.v3.oas.models.media.IntegerSchema;
4040
import io.swagger.v3.oas.models.media.MapSchema;
4141
import io.swagger.v3.oas.models.media.NumberSchema;
42+
import io.swagger.v3.oas.models.media.ObjectSchema;
4243
import io.swagger.v3.oas.models.media.Schema;
4344
import io.swagger.v3.oas.models.media.StringSchema;
4445
import io.swagger.v3.oas.models.media.UUIDSchema;
@@ -84,6 +85,10 @@
8485
public class ModelResolver extends AbstractModelConverter implements ModelConverter {
8586
Logger LOGGER = LoggerFactory.getLogger(ModelResolver.class);
8687

88+
public static final String SET_PROPERTY_OF_COMPOSED_MODEL_AS_SIBLING = "composed-model-properties-as-sibiling";
89+
90+
public static boolean composedModelPropertiesAsSibling = System.getProperty(SET_PROPERTY_OF_COMPOSED_MODEL_AS_SIBLING) != null ? true : false;
91+
8792
public ModelResolver(ObjectMapper mapper) {
8893
super(mapper);
8994
}
@@ -795,8 +800,27 @@ public Schema resolve(AnnotatedType annotatedType, ModelConverterContext context
795800

796801
});
797802

803+
if (!composedModelPropertiesAsSibling) {
804+
if (composedSchema.getAllOf() != null && !composedSchema.getAllOf().isEmpty()) {
805+
if (composedSchema.getProperties() != null && !composedSchema.getProperties().isEmpty()) {
806+
ObjectSchema propSchema = new ObjectSchema();
807+
propSchema.properties(composedSchema.getProperties());
808+
composedSchema.setProperties(null);
809+
composedSchema.addAllOfItem(propSchema);
810+
}
811+
}
812+
}
798813
}
799-
if (model != null && annotatedType.isResolveAsRef() && "object".equals(model.getType()) && StringUtils.isNotBlank(model.getName())) {
814+
815+
if (!type.isContainerType() && StringUtils.isNotBlank(name)) {
816+
// define the model here to support self/cyclic referencing of models
817+
context.defineModel(name, model, annotatedType, null);
818+
}
819+
820+
if (model != null && annotatedType.isResolveAsRef() &&
821+
(isComposedSchema || "object".equals(model.getType())) &&
822+
StringUtils.isNotBlank(model.getName()))
823+
{
800824
if (context.getDefinedModels().containsKey(model.getName())) {
801825
model = new Schema().$ref(constructRef(model.getName()));
802826
}
@@ -1212,6 +1236,17 @@ private boolean resolveSubtypes(Schema model, BeanDescription bean, ModelConvert
12121236
composedSchema.addAllOfItem(refSchema);
12131237
}
12141238
removeParentProperties(composedSchema, model);
1239+
if (!composedModelPropertiesAsSibling) {
1240+
if (composedSchema.getAllOf() != null && !composedSchema.getAllOf().isEmpty()) {
1241+
if (composedSchema.getProperties() != null && !composedSchema.getProperties().isEmpty()) {
1242+
ObjectSchema propSchema = new ObjectSchema();
1243+
propSchema.properties(composedSchema.getProperties());
1244+
composedSchema.setProperties(null);
1245+
composedSchema.addAllOfItem(propSchema);
1246+
}
1247+
}
1248+
}
1249+
12151250

12161251
// replace previous schema..
12171252
Class<?> currentType = subtype.getType();

‎modules/swagger-core/src/main/java/io/swagger/v3/core/util/AnnotationsUtils.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,7 @@ public static Optional<ArraySchema> getArraySchema(io.swagger.v3.oas.annotations
390390
if (arraySchema.schema() != null) {
391391
if (arraySchema.schema().implementation().equals(Void.class)) {
392392
getSchemaFromAnnotation(arraySchema.schema(), components, jsonViewAnnotation).ifPresent(schema -> {
393-
if (StringUtils.isNotBlank(schema.getType()) || StringUtils.isNotBlank(schema.get$ref())) {
394-
arraySchemaObject.setItems(schema);
395-
}
393+
arraySchemaObject.setItems(schema);
396394
});
397395
} // if present, schema implementation handled upstream
398396
}
@@ -938,11 +936,9 @@ public static Optional<Header> getHeader(io.swagger.v3.oas.annotations.headers.H
938936
if (header.schema() != null) {
939937
if (header.schema().implementation().equals(Void.class)) {
940938
AnnotationsUtils.getSchemaFromAnnotation(header.schema(), jsonViewAnnotation).ifPresent(schema -> {
941-
if (StringUtils.isNotBlank(schema.getType())) {
942-
headerObject.setSchema(schema);
943-
//schema inline no need to add to components
944-
//components.addSchemas(schema.getType(), schema);
945-
}
939+
headerObject.setSchema(schema);
940+
//schema inline no need to add to components
941+
//components.addSchemas(schema.getType(), schema);
946942
});
947943
}
948944
}

‎modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/ComposedSchemaTest.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,18 @@ public void readBilateralComposedSchema_ticket2620() {
3939

4040
schemas = ModelConverters.getInstance().readAll(TestObjectTicket2620Subtypes.class);
4141
model = schemas.get("Child2TestObject");
42-
properties = model.getProperties();
42+
Assert.assertNull(model.getProperties());
43+
properties = ((ComposedSchema)model).getAllOf().get(1).getProperties();
4344
Assert.assertNull(properties.get("name"));
4445
Assert.assertNotNull(properties.get("childName"));
4546
Assert.assertTrue(model instanceof ComposedSchema);
4647
model = schemas.get("ChildTestObject");
47-
properties = model.getProperties();
48+
Assert.assertNull(model.getProperties());
49+
properties = ((ComposedSchema)model).getAllOf().get(1).getProperties();
4850
Assert.assertNull(properties.get("name"));
4951
Assert.assertNotNull(properties.get("childName"));
5052
Assert.assertTrue(model instanceof ComposedSchema);
51-
Assert.assertTrue(((ComposedSchema)model).getAllOf().size() == 1);
53+
Assert.assertTrue(((ComposedSchema)model).getAllOf().size() == 2);
5254

5355
model = schemas.get("TestObjectTicket2620Subtypes");
5456
properties = model.getProperties();
@@ -123,5 +125,4 @@ public void readComposedSchema_ticket2616() {
123125
model = schemas.get("objects");
124126
Assert.assertNull(model);
125127
}
126-
127128
}

‎modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/InheritedBeanTest.java‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import static org.testng.Assert.assertEquals;
1818
import static org.testng.Assert.assertNotNull;
19-
import static org.testng.Assert.assertNull;
2019
import static org.testng.Assert.assertTrue;
2120

2221
public class InheritedBeanTest extends SwaggerTestBase {
@@ -47,7 +46,7 @@ public void testInheritedBean() throws Exception {
4746
assertEquals(cm.getAllOf().get(0).get$ref(), "#/components/schemas/BaseBean");
4847

4948
// make sure parent properties are filtered out of subclass
50-
assertSub1PropertiesValid(cm.getProperties());
49+
assertSub1PropertiesValid(cm.getAllOf().get(1).getProperties());
5150
}
5251

5352
@Test
@@ -60,7 +59,7 @@ public void testInheritedChildBean() throws Exception {
6059
assertEquals(cm.getAllOf().get(0).get$ref(), "#/components/schemas/BaseBean");
6160

6261
// make sure parent properties are filtered out of subclass
63-
assertSub1PropertiesValid(cm.getProperties());
62+
assertSub1PropertiesValid(cm.getAllOf().get(1).getProperties());
6463

6564
final Schema baseModel = context.getDefinedModels().get("BaseBean");
6665
assertNotNull(baseModel);
@@ -77,7 +76,7 @@ public void testComposedChildBean() throws Exception {
7776
assertEquals(cm.getAllOf().get(0).get$ref(), "#/components/schemas/BaseBean2");
7877

7978
// make sure parent properties are filtered out of subclass
80-
assertSub1PropertiesValid(cm.getProperties());
79+
assertSub1PropertiesValid(cm.getAllOf().get(1).getProperties());
8180

8281
final Schema baseModel = context.getDefinedModels().get("BaseBean2");
8382
assertNotNull(baseModel);
@@ -119,7 +118,7 @@ public void testHierarchy() throws Exception {
119118
ComposedSchema cm = (ComposedSchema) subModel;
120119
assertEquals(cm.getAllOf().get(0).get$ref(), "#/components/schemas/BaseBean3");
121120
// make sure parent properties are filtered out of subclass
122-
assertSub1PropertiesValid(cm.getProperties());
121+
assertSub1PropertiesValid(cm.getAllOf().get(1).getProperties());
123122

124123
// assert grandchild
125124
final Schema subSubModel = context.getDefinedModels().get("GrandChildBean3");
@@ -129,7 +128,7 @@ public void testHierarchy() throws Exception {
129128
cm = (ComposedSchema) subSubModel;
130129
assertEquals(cm.getAllOf().get(0).get$ref(), "#/components/schemas/ChildBean3");
131130
// make sure parent properties are filtered out of subclass
132-
assertSub2PropertiesValid(cm.getProperties());
131+
assertSub2PropertiesValid(cm.getAllOf().get(1).getProperties());
133132

134133
}
135134

@@ -310,15 +309,15 @@ public void testMultipleInheritedBean() throws Exception {
310309
ComposedSchema cm1 = (ComposedSchema) sub1Model;
311310
assertEquals(cm1.getAllOf().get(0).get$ref(), "#/components/schemas/MultipleBaseBean");
312311
// make sure parent properties are filtered out of subclass
313-
assertSub1PropertiesValid(cm1.getProperties());
312+
assertSub1PropertiesValid(cm1.getAllOf().get(1).getProperties());
314313

315314
final Schema sub2Model = context.getDefinedModels().get("MultipleSub2Bean");
316315
assertNotNull(sub2Model);
317316
assertTrue(sub2Model instanceof ComposedSchema);
318317
ComposedSchema cm2 = (ComposedSchema) sub2Model;
319318
assertEquals(cm2.getAllOf().get(0).get$ref(), "#/components/schemas/MultipleBaseBean");
320319
// make sure parent properties are filtered out of subclass
321-
assertSub2PropertiesValid(cm2.getProperties());
320+
assertSub2PropertiesValid(cm2.getAllOf().get(1).getProperties());
322321
}
323322

324323
@Test
@@ -330,7 +329,7 @@ public void testMultipleInheritedChildBean() throws Exception {
330329
ComposedSchema cm = (ComposedSchema) subModel;
331330
assertEquals(cm.getAllOf().get(0).get$ref(), "#/components/schemas/MultipleBaseBean");
332331
// make sure parent properties are filtered out of subclass
333-
assertSub1PropertiesValid(cm.getProperties());
332+
assertSub1PropertiesValid(cm.getAllOf().get(1).getProperties());
334333

335334
final Schema baseModel = context.getDefinedModels().get("MultipleBaseBean");
336335
assertNotNull(baseModel);
@@ -343,15 +342,15 @@ public void testMultipleInheritedChildBean() throws Exception {
343342
ComposedSchema cm1 = (ComposedSchema) sub1Model;
344343
assertEquals(cm1.getAllOf().get(0).get$ref(), "#/components/schemas/MultipleBaseBean");
345344
// make sure parent properties are filtered out of subclass
346-
assertSub1PropertiesValid(cm1.getProperties());
345+
assertSub1PropertiesValid(cm1.getAllOf().get(1).getProperties());
347346

348347
final Schema sub2Model = context.getDefinedModels().get("MultipleSub2Bean");
349348
assertNotNull(sub2Model);
350349
assertTrue(sub2Model instanceof ComposedSchema);
351350
ComposedSchema cm2 = (ComposedSchema) sub2Model;
352351
assertEquals(cm2.getAllOf().get(0).get$ref(), "#/components/schemas/MultipleBaseBean");
353352
// make sure parent properties are filtered out of subclass
354-
assertSub2PropertiesValid(cm2.getProperties());
353+
assertSub2PropertiesValid(cm2.getAllOf().get(1).getProperties());
355354
}
356355

357356
private void assertSub2PropertiesValid(Map<String, Schema> subProperties) {

‎modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket3030Test.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package io.swagger.v3.core.resolving;
2+
3+
import com.fasterxml.jackson.databind.ObjectMapper;
4+
import io.swagger.v3.core.converter.AnnotatedType;
5+
import io.swagger.v3.core.converter.ModelConverterContextImpl;
6+
import io.swagger.v3.core.jackson.ModelResolver;
7+
import io.swagger.v3.core.matchers.SerializationMatchers;
8+
import io.swagger.v3.oas.models.media.Schema;
9+
import org.testng.annotations.BeforeTest;
10+
import org.testng.annotations.Test;
11+
12+
import static org.testng.Assert.assertNotNull;
13+
14+
public class Ticket3030Test extends SwaggerTestBase {
15+
16+
private ModelResolver modelResolver;
17+
private ModelConverterContextImpl context;
18+
19+
@BeforeTest
20+
public void setup() {
21+
modelResolver = new ModelResolver(new ObjectMapper());
22+
context = new ModelConverterContextImpl(modelResolver);
23+
}
24+
25+
26+
27+
@Test
28+
public void testTicket3030() throws Exception {
29+
final Schema model = context.resolve(new AnnotatedType(Child.class));
30+
assertNotNull(model);
31+
String yaml = "Child:\n" +
32+
" type: object\n" +
33+
" allOf:\n" +
34+
" - $ref: '#/components/schemas/Parent'\n" +
35+
" - type: object\n" +
36+
" properties:\n" +
37+
" property:\n" +
38+
" type: string\n" +
39+
"Parent:\n" +
40+
" type: object\n" +
41+
" properties:\n" +
42+
" sharedProperty:\n" +
43+
" type: string";
44+
45+
SerializationMatchers.assertEqualsToYaml(context.getDefinedModels(), yaml);
46+
}
47+
48+
@io.swagger.v3.oas.annotations.media.Schema(subTypes = {Child.class})
49+
static class Parent {
50+
public String sharedProperty;
51+
}
52+
53+
@io.swagger.v3.oas.annotations.media.Schema(allOf = Parent.class)
54+
static class Child extends Parent {
55+
public String property;
56+
}
57+
}

‎modules/swagger-core/src/test/java/io/swagger/v3/core/resolving/Ticket3063Test.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package io.swagger.v3.core.resolving;
2+
3+
import com.fasterxml.jackson.annotation.JsonSubTypes;
4+
import com.fasterxml.jackson.annotation.JsonTypeInfo;
5+
import com.fasterxml.jackson.databind.ObjectMapper;
6+
import io.swagger.v3.core.converter.AnnotatedType;
7+
import io.swagger.v3.core.converter.ModelConverterContextImpl;
8+
import io.swagger.v3.core.jackson.ModelResolver;
9+
import io.swagger.v3.core.matchers.SerializationMatchers;
10+
import io.swagger.v3.oas.models.media.Schema;
11+
import org.testng.annotations.BeforeTest;
12+
import org.testng.annotations.Test;
13+
14+
import static org.testng.Assert.assertNotNull;
15+
16+
public class Ticket3063Test extends SwaggerTestBase {
17+
18+
private ModelResolver modelResolver;
19+
private ModelConverterContextImpl context;
20+
21+
@BeforeTest
22+
public void setup() {
23+
modelResolver = new ModelResolver(new ObjectMapper());
24+
context = new ModelConverterContextImpl(modelResolver);
25+
}
26+
27+
28+
29+
@Test
30+
public void testTicket3063() throws Exception {
31+
final Schema model = context.resolve(new AnnotatedType(BaseClass.class));
32+
assertNotNull(model);
33+
String yaml = "BaseClass:\n" +
34+
" required:\n" +
35+
" - type\n" +
36+
" type: object\n" +
37+
" properties:\n" +
38+
" type:\n" +
39+
" type: string\n" +
40+
" description: Type\n" +
41+
" example: AndroidDeviceRequirements\n" +
42+
" description: test\n" +
43+
" discriminator:\n" +
44+
" propertyName: type\n" +
45+
"SubClass:\n" +
46+
" required:\n" +
47+
" - type\n" +
48+
" type: object\n" +
49+
" description: SubClass\n" +
50+
" allOf:\n" +
51+
" - $ref: '#/components/schemas/BaseClass'\n" +
52+
" - type: object\n" +
53+
" properties:\n" +
54+
" additionalPropertyWhichShouldBeThere:\n" +
55+
" type: integer\n" +
56+
" description: Test\n" +
57+
" format: int32";
58+
59+
SerializationMatchers.assertEqualsToYaml(context.getDefinedModels(), yaml);
60+
}
61+
62+
@io.swagger.v3.oas.annotations.media.Schema(description = "SubClass")
63+
public class SubClass extends BaseClass {
64+
@io.swagger.v3.oas.annotations.media.Schema(required = false, description = "Test")
65+
public int additionalPropertyWhichShouldBeThere = -1;
66+
67+
}
68+
69+
@io.swagger.v3.oas.annotations.media.Schema(description = "test", discriminatorProperty="type")
70+
@JsonTypeInfo(
71+
use = JsonTypeInfo.Id.NAME,
72+
include = JsonTypeInfo.As.PROPERTY,
73+
property = "type")
74+
@JsonSubTypes({
75+
@JsonSubTypes.Type(value = SubClass.class, name = "SubClass")
76+
})
77+
public class BaseClass {
78+
79+
@io.swagger.v3.oas.annotations.media.Schema(required = true, description = "Type", example="AndroidDeviceRequirements")
80+
public String type;
81+
82+
public BaseClass(String type) {
83+
this.type = type;
84+
}
85+
86+
public BaseClass() {
87+
}
88+
89+
}
90+
}

0 commit comments

Comments
 (0)