Skip to content

Commit 199e30a

Browse files
julienrfjulienrf
authored
Support allOf schemas referring to oneOf schemas (#318)
Fixes #317
1 parent 46c7f23 commit 199e30a

File tree

4 files changed

+219
-9
lines changed

4 files changed

+219
-9
lines changed

‎core/src/main/java/org/openapitools/openapidiff/core/compare/SchemaDiff.java

+30-9
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@
1010
import io.swagger.v3.oas.models.media.Schema;
1111
import io.swagger.v3.oas.models.media.XML;
1212
import java.util.ArrayList;
13+
import java.util.HashSet;
1314
import java.util.LinkedHashMap;
1415
import java.util.List;
1516
import java.util.Map;
1617
import java.util.Objects;
1718
import java.util.Optional;
19+
import java.util.Set;
1820
import org.openapitools.openapidiff.core.compare.schemadiffresult.ArraySchemaDiffResult;
1921
import org.openapitools.openapidiff.core.compare.schemadiffresult.ComposedSchemaDiffResult;
2022
import org.openapitools.openapidiff.core.compare.schemadiffresult.SchemaDiffResult;
@@ -79,17 +81,26 @@ public static SchemaDiffResult getSchemaDiffResult(
7981
}
8082
}
8183

82-
protected static Schema<?> resolveComposedSchema(Components components, Schema<?> schema) {
84+
protected static Schema<?> resolveComposedSchema(
85+
Components components, Schema<?> schema, Set<String> visitedRefs) {
8386
if (schema instanceof ComposedSchema) {
8487
ComposedSchema composedSchema = (ComposedSchema) schema;
85-
List<Schema> allOfSchemaList = composedSchema.getAllOf();
86-
if (allOfSchemaList != null) {
87-
for (Schema<?> allOfSchema : allOfSchemaList) {
88-
allOfSchema = refPointer.resolveRef(components, allOfSchema, allOfSchema.get$ref());
89-
allOfSchema = resolveComposedSchema(components, allOfSchema);
90-
schema = addSchema(schema, allOfSchema);
88+
List<Schema> composedSchemas = new ArrayList<>();
89+
Optional.ofNullable(composedSchema.getAllOf()).ifPresent(composedSchemas::addAll);
90+
Optional.ofNullable(composedSchema.getAnyOf()).ifPresent(composedSchemas::addAll);
91+
92+
if (!composedSchemas.isEmpty()) {
93+
for (Schema<?> composed : composedSchemas) {
94+
if (composed.get$ref() == null || !visitedRefs.contains(composed.get$ref())) {
95+
Set<String> updatedVisitedRefs = new HashSet<>(visitedRefs);
96+
updatedVisitedRefs.add(composed.get$ref());
97+
composed = refPointer.resolveRef(components, composed, composed.get$ref());
98+
composed = resolveComposedSchema(components, composed, updatedVisitedRefs);
99+
schema = addSchema(schema, composed);
100+
}
91101
}
92102
composedSchema.setAllOf(null);
103+
composedSchema.setAnyOf(null);
93104
}
94105
}
95106
return schema;
@@ -154,6 +165,16 @@ protected static Schema<?> addSchema(Schema<?> schema, Schema<?> fromSchema) {
154165
}
155166
schema.getExtensions().putAll(fromSchema.getExtensions());
156167
}
168+
if (fromSchema instanceof ComposedSchema && schema instanceof ComposedSchema) {
169+
ComposedSchema composedFromSchema = (ComposedSchema) fromSchema;
170+
ComposedSchema composedSchema = (ComposedSchema) schema;
171+
if (composedFromSchema.getOneOf() != null) {
172+
if (composedSchema.getOneOf() == null) {
173+
composedSchema.setOneOf(new ArrayList<>());
174+
}
175+
composedSchema.getOneOf().addAll(composedFromSchema.getOneOf());
176+
}
177+
}
157178
if (fromSchema.getDiscriminator() != null) {
158179
if (schema.getDiscriminator() == null) {
159180
schema.setDiscriminator(new Discriminator());
@@ -316,8 +337,8 @@ public DeferredChanged<ChangedSchema> computeDiffForReal(
316337
left = refPointer.resolveRef(this.leftComponents, left, getSchemaRef(left));
317338
right = refPointer.resolveRef(this.rightComponents, right, getSchemaRef(right));
318339

319-
left = resolveComposedSchema(leftComponents, left);
320-
right = resolveComposedSchema(rightComponents, right);
340+
left = resolveComposedSchema(leftComponents, left, new HashSet<>());
341+
right = resolveComposedSchema(rightComponents, right, new HashSet<>());
321342

322343
// If type of schemas are different, just set old & new schema, set changedType to true in
323344
// SchemaDiffResult and

‎core/src/test/java/org/openapitools/openapidiff/core/AllOfOneOfDiffTest.java

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package org.openapitools.openapidiff.core;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import org.junit.jupiter.api.Test;
6+
import org.openapitools.openapidiff.core.model.ChangedOpenApi;
7+
8+
public class AllOfOneOfDiffTest {
9+
@Test
10+
void allOfReferringToOneOfSchemasAreSupported() {
11+
ChangedOpenApi diff = OpenApiCompare.fromLocations("issue-317_1.json", "issue-317_2.json");
12+
assertThat(diff.isCoreChanged().isUnchanged());
13+
}
14+
}

‎core/src/test/resources/issue-317_1.json

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
{
2+
"openapi":"3.0.0",
3+
"info":{
4+
"title":"API",
5+
"version":"0.1.0"
6+
},
7+
"paths":{
8+
"/resource":{
9+
"post":{
10+
"responses":{
11+
"200":{
12+
"description":"Created resource",
13+
"content":{
14+
"application/json":{
15+
"schema":{
16+
"type":"string"
17+
}
18+
}
19+
}
20+
}
21+
},
22+
"summary":"Create resource",
23+
"requestBody":{
24+
"content":{
25+
"application/json":{
26+
"schema":{
27+
"$ref":"#/components/schemas/Resource"
28+
}
29+
}
30+
},
31+
"description":"Definition of the resource"
32+
}
33+
}
34+
}
35+
},
36+
"components":{
37+
"schemas":{
38+
"Resource":{
39+
"type":"object",
40+
"properties":{
41+
"assignment":{
42+
"$ref":"#/components/schemas/Foo"
43+
}
44+
}
45+
},
46+
"Foo":{
47+
"oneOf":[
48+
{
49+
"$ref":"#/components/schemas/Foo.Bar"
50+
},
51+
{
52+
"$ref":"#/components/schemas/Foo.Baz"
53+
}
54+
],
55+
"discriminator":{
56+
"propertyName":"type",
57+
"mapping":{
58+
"Bar":"#/components/schemas/Foo.Bar",
59+
"Baz":"#/components/schemas/Foo.Baz"
60+
}
61+
}
62+
},
63+
"Foo.Bar":{
64+
"type":"object",
65+
"properties":{
66+
"type":{
67+
"type":"string"
68+
}
69+
}
70+
},
71+
"Foo.Baz":{
72+
"type":"object",
73+
"properties":{
74+
"type":{
75+
"type":"string"
76+
}
77+
}
78+
}
79+
},
80+
"securitySchemes":{
81+
82+
}
83+
}
84+
}

‎core/src/test/resources/issue-317_2.json

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
{
2+
"openapi":"3.0.0",
3+
"info":{
4+
"title":"API",
5+
"version":"0.1.0"
6+
},
7+
"paths":{
8+
"/resource":{
9+
"post":{
10+
"responses":{
11+
"200":{
12+
"description":"Created resource",
13+
"content":{
14+
"application/json":{
15+
"schema":{
16+
"type":"string"
17+
}
18+
}
19+
}
20+
}
21+
},
22+
"summary":"Create resource",
23+
"requestBody":{
24+
"content":{
25+
"application/json":{
26+
"schema":{
27+
"$ref":"#/components/schemas/Resource"
28+
}
29+
}
30+
},
31+
"description":"Definition of the resource"
32+
}
33+
}
34+
}
35+
},
36+
"components":{
37+
"schemas":{
38+
"Resource":{
39+
"type":"object",
40+
"properties":{
41+
"assignment":{
42+
"default":{
43+
"type":"Bar"
44+
},
45+
"allOf":[
46+
{
47+
"$ref":"#/components/schemas/Foo"
48+
}
49+
]
50+
}
51+
}
52+
},
53+
"Foo":{
54+
"oneOf":[
55+
{
56+
"$ref":"#/components/schemas/Foo.Bar"
57+
},
58+
{
59+
"$ref":"#/components/schemas/Foo.Baz"
60+
}
61+
],
62+
"discriminator":{
63+
"propertyName":"type",
64+
"mapping":{
65+
"Bar":"#/components/schemas/Foo.Bar",
66+
"Baz":"#/components/schemas/Foo.Baz"
67+
}
68+
}
69+
},
70+
"Foo.Bar":{
71+
"type":"object",
72+
"properties":{
73+
"type":{
74+
"type":"string"
75+
}
76+
}
77+
},
78+
"Foo.Baz":{
79+
"type":"object",
80+
"properties":{
81+
"type":{
82+
"type":"string"
83+
}
84+
}
85+
}
86+
},
87+
"securitySchemes":{
88+
89+
}
90+
}
91+
}

0 commit comments

Comments
 (0)