Skip to content

Commit 78dfb2c

Browse files
author
Anze Sodja
committed
Fix issue where schemas with inheritance are dropped and also fix schema with recursion
1 parent 9d9f092 commit 78dfb2c

File tree

10 files changed

+635
-21
lines changed

10 files changed

+635
-21
lines changed

‎gradle.properties‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
projectVersion=2.4.1-SNAPSHOT
2-
jacksonVersion=2.11.2
2+
jacksonVersion=2.12.2
33
micronautDocsVersion=1.0.25
4-
micronautVersion=2.3.0
5-
micronautTestVersion=2.3.2
4+
micronautVersion=2.5.0
5+
micronautTestVersion=2.3.3
66
groovyVersion=3.0.5
77
spockVersion=2.0-M2-groovy-3.0
88

‎openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiEndpointVisitor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ private void processParameter(VisitorContext context, OpenAPI openAPI,
437437
}
438438

439439
if (schema != null) {
440-
bindSchemaForElement(context, parameter, parameterType, schema);
440+
schema = bindSchemaForElement(context, parameter, parameterType, schema);
441441
newParameter.setSchema(schema);
442442
}
443443
}

‎openapi/src/main/java/io/micronaut/openapi/visitor/AbstractOpenApiVisitor.java‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import com.fasterxml.jackson.databind.JsonNode;
2424
import com.fasterxml.jackson.databind.ObjectMapper;
2525
import com.fasterxml.jackson.databind.PropertyNamingStrategy;
26-
import com.fasterxml.jackson.databind.SerializationFeature;
2726
import com.fasterxml.jackson.databind.annotation.JsonNaming;
2827
import com.fasterxml.jackson.databind.node.ObjectNode;
2928

@@ -139,6 +138,7 @@ abstract class AbstractOpenApiVisitor {
139138
private static final String ATTR_TEST_MODE = "io.micronaut.OPENAPI_TEST";
140139
private static final Lock VISITED_ELEMENTS_LOCK = new ReentrantLock();
141140
private static final String ATTR_VISITED_ELEMENTS = "io.micronaut.OPENAPI.visited.elements";
141+
private static final Schema<?> EMPTY_SCHEMA = new Schema<>();
142142

143143
/**
144144
* The JSON mapper.
@@ -827,7 +827,13 @@ private String resolvePropertyName(Element element, Element classElement, Schem
827827
*/
828828
protected Schema bindSchemaForElement(VisitorContext context, Element element, ClassElement elementType, Schema schemaToBind) {
829829
AnnotationValue<io.swagger.v3.oas.annotations.media.Schema> schemaAnn = element.getAnnotation(io.swagger.v3.oas.annotations.media.Schema.class);
830-
if (schemaAnn != null) {
830+
Schema originalSchema = schemaToBind;
831+
if (originalSchema.get$ref() != null) {
832+
schemaToBind = new Schema();
833+
}
834+
if (originalSchema.get$ref() == null && schemaAnn != null) {
835+
// Apply @Schema annotation only if not $ref since for $ref schemas
836+
// we already populated values from right @Schema annotation in previous steps
831837
schemaToBind = bindSchemaAnnotationValue(context, element, schemaToBind, schemaAnn);
832838
Optional<String> schemaName = schemaAnn.get("name", String.class);
833839
if (schemaName.isPresent()) {
@@ -896,14 +902,18 @@ protected Schema bindSchemaForElement(VisitorContext context, Element element, C
896902
if (defaultValue != null && schemaToBind.getDefault() == null) {
897903
schemaToBind.setDefault(defaultValue);
898904
}
899-
if (element.isAnnotationPresent(Nullable.class)) {
905+
if (element.isAnnotationPresent(Nullable.class)
906+
|| element.isAnnotationPresent(javax.annotation.Nullable.class)
907+
|| element.isAnnotationPresent(io.micronaut.core.annotation.Nullable.class)) {
900908
schemaToBind.setNullable(true);
901909
}
902910
final String defaultJacksonValue = element.stringValue(JsonProperty.class, "defaultValue").orElse(null);
903911
if (defaultJacksonValue != null && schemaToBind.getDefault() == null) {
904912
schemaToBind.setDefault(defaultJacksonValue);
905913
}
906-
return schemaToBind;
914+
return originalSchema.get$ref() != null && !schemaToBind.equals(EMPTY_SCHEMA)
915+
? new ComposedSchema().addAllOfItem(originalSchema).addAllOfItem(schemaToBind)
916+
: originalSchema;
907917
}
908918

909919
private void setSchemaDocumentation(Element element, Schema schemaToBind) {
@@ -1102,13 +1112,9 @@ private void checkAllOf(ComposedSchema composedSchema) {
11021112
private Schema getSchemaDefinition(
11031113
OpenAPI openAPI,
11041114
VisitorContext context,
1105-
Element type,
1115+
ClassElement type,
11061116
@Nullable Element definingElement,
11071117
List<MediaType> mediaTypes) {
1108-
// To break the recursion
1109-
if (inProgressSchemas.contains(type.getSimpleName())) {
1110-
return null;
1111-
}
11121118
AnnotationValue<io.swagger.v3.oas.annotations.media.Schema> schemaValue = definingElement == null ? null : definingElement.getDeclaredAnnotation(io.swagger.v3.oas.annotations.media.Schema.class);
11131119
if (schemaValue == null) {
11141120
schemaValue = type.getDeclaredAnnotation(io.swagger.v3.oas.annotations.media.Schema.class);
@@ -1175,6 +1181,10 @@ private Schema getSchemaDefinition(
11751181
String schemaName = schemaValue.get("name", String.class).orElse(computeDefaultSchemaName(definingElement, type));
11761182
schema = schemas.get(schemaName);
11771183
if (schema == null) {
1184+
if (inProgressSchemas.contains(schemaName)) {
1185+
// Break recursion
1186+
return new Schema<>().$ref(schemaRef(schemaName));
1187+
}
11781188
inProgressSchemas.add(schemaName);
11791189
try {
11801190
schema = readSchema(schemaValue, openAPI, context, type, mediaTypes);

‎openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiBasicSchemaSpec.groovy‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,6 @@ public class MyBean {}
376376

377377
then:
378378
OpenAPI openAPI = AbstractOpenApiVisitor.testReference
379-
System.out.println(openAPI)
380379
openAPI?.paths?.get("/person/{name}")?.get
381380
openAPI.components.schemas["Person"]
382381
openAPI.components.schemas["Person"].type == "object"

‎openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiInheritedPojoControllerSpec.groovy‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,10 @@ class MyBean {}
307307
petSchema instanceof ComposedSchema
308308
catSchema instanceof ComposedSchema
309309
dogSchema instanceof ComposedSchema
310-
catSchema.type == 'object'
311-
catSchema.properties.size() == 1
312-
catSchema.properties['clawSize'].type == 'integer'
310+
((ComposedSchema) catSchema).allOf[0].$ref == '#/components/schemas/Pet'
311+
((ComposedSchema) catSchema).allOf[1].type == 'object'
312+
((ComposedSchema) catSchema).allOf[1].properties.size() == 1
313+
((ComposedSchema) catSchema).allOf[1].properties['clawSize'].type == 'integer'
313314
petSchema.type == 'object'
314315
petSchema.properties.size() == 3
315316
petSchema.discriminator.propertyName == "type"
@@ -491,7 +492,6 @@ class CatController implements PetOperations {
491492
@javax.inject.Singleton
492493
class MyBean {}
493494
''')
494-
then:
495495
then: "the state is correct"
496496
AbstractOpenApiVisitor.testReference != null
497497

@@ -508,9 +508,10 @@ class MyBean {}
508508
petSchema instanceof ComposedSchema
509509
catSchema instanceof ComposedSchema
510510
dogSchema instanceof ComposedSchema
511-
catSchema.type == 'object'
512-
catSchema.properties.size() == 1
513-
catSchema.properties['clawSize'].type == 'integer'
511+
((ComposedSchema) catSchema).allOf[0].$ref == '#/components/schemas/Pet'
512+
((ComposedSchema) catSchema).allOf[1].type == 'object'
513+
((ComposedSchema) catSchema).allOf[1].properties.size() == 1
514+
((ComposedSchema) catSchema).allOf[1].properties['clawSize'].type == 'integer'
514515
petSchema.type == 'object'
515516
petSchema.properties.size() == 3
516517
petSchema.discriminator.propertyName == "type"

‎openapi/src/test/groovy/io/micronaut/openapi/visitor/OpenApiRecursionSpec.groovy‎

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
package io.micronaut.openapi.visitor
2+
3+
import io.micronaut.annotation.processing.test.AbstractTypeElementSpec
4+
import io.swagger.v3.oas.models.OpenAPI
5+
import io.swagger.v3.oas.models.Operation
6+
import io.swagger.v3.oas.models.media.ComposedSchema
7+
import io.swagger.v3.oas.models.media.Schema
8+
import io.swagger.v3.oas.models.parameters.RequestBody
9+
10+
class OpenApiRecursionSpec extends AbstractTypeElementSpec {
11+
12+
def setup() {
13+
System.setProperty(AbstractOpenApiVisitor.ATTR_TEST_MODE, "true")
14+
}
15+
16+
void "test OpenAPI handles recursion"() {
17+
given:
18+
buildBeanDefinition('test.MyBean', '''
19+
20+
package test;
21+
22+
import io.swagger.v3.oas.annotations.media.*;
23+
import io.swagger.v3.oas.annotations.*;
24+
import io.micronaut.http.annotation.*;
25+
import io.reactivex.Maybe;
26+
27+
@Controller("/")
28+
class MyController {
29+
@Get
30+
public Maybe<TestInterface> hey() {
31+
return Maybe.empty();
32+
}
33+
}
34+
35+
@Schema(oneOf = {TestImpl1.class, TestImpl2.class})
36+
interface TestInterface {
37+
String getType();
38+
}
39+
40+
class TestImpl1 implements TestInterface {
41+
42+
private TestInterface woopsie;
43+
44+
@Override
45+
public String getType() {
46+
return null;
47+
}
48+
49+
public TestInterface getWoopsie() {
50+
return woopsie;
51+
}
52+
53+
public void setWoopsie(TestInterface woopsie) {
54+
this.woopsie = woopsie;
55+
}
56+
}
57+
58+
class TestImpl2 implements TestInterface {
59+
@Override
60+
public String getType() {
61+
return null;
62+
}
63+
}
64+
65+
@javax.inject.Singleton
66+
class MyBean {}
67+
''')
68+
69+
OpenAPI openAPI = AbstractOpenApiVisitor.testReference
70+
Map<String, Schema> schemas = openAPI.getComponents().getSchemas()
71+
72+
expect:
73+
Schema testImpl1 = schemas.get("TestImpl1")
74+
Schema woopsieRef = testImpl1.getProperties()["woopsie"]
75+
woopsieRef
76+
woopsieRef.$ref == "#/components/schemas/TestInterface"
77+
}
78+
79+
void "test OpenAPI handles recursion when recursive item has different name"() {
80+
given:
81+
buildBeanDefinition('test.MyBean', '''
82+
83+
package test;
84+
85+
import io.swagger.v3.oas.annotations.media.*;
86+
import io.swagger.v3.oas.annotations.*;
87+
import io.micronaut.http.annotation.*;
88+
import io.reactivex.Maybe;
89+
90+
@Controller("/")
91+
class MyController {
92+
@Get
93+
public Maybe<TestInterface> hey() {
94+
return Maybe.empty();
95+
}
96+
}
97+
98+
@Schema(oneOf = {TestImpl1.class, TestImpl2.class})
99+
interface TestInterface {
100+
String getType();
101+
}
102+
103+
class TestImpl1 implements TestInterface {
104+
105+
private TestInterface woopsie;
106+
107+
@Override
108+
public String getType() {
109+
return null;
110+
}
111+
112+
@Schema(name = "woopsie-id", description = "woopsie doopsie", oneOf = {TestImpl1.class, TestImpl2.class})
113+
public TestInterface getWoopsie() {
114+
return woopsie;
115+
}
116+
117+
public void setWoopsie(TestInterface woopsie) {
118+
this.woopsie = woopsie;
119+
}
120+
}
121+
122+
class TestImpl2 implements TestInterface {
123+
@Override
124+
public String getType() {
125+
return null;
126+
}
127+
}
128+
129+
@javax.inject.Singleton
130+
class MyBean {}
131+
''')
132+
133+
OpenAPI openAPI = AbstractOpenApiVisitor.testReference
134+
Map<String, Schema> schemas = openAPI.getComponents().getSchemas()
135+
136+
expect:
137+
Schema testImpl1 = schemas.get("TestImpl1")
138+
Schema woopsieRef = testImpl1.getProperties()["woopsie"]
139+
woopsieRef
140+
woopsieRef.$ref == "#/components/schemas/woopsie-id"
141+
Schema woopsie = schemas.get("woopsie-id")
142+
woopsie.description == "woopsie doopsie"
143+
}
144+
145+
void "test OpenAPI applies additional annotations to recursive property"() {
146+
given:
147+
buildBeanDefinition('test.MyBean', '''
148+
149+
package test;
150+
151+
import io.swagger.v3.oas.annotations.media.*;
152+
import io.swagger.v3.oas.annotations.*;
153+
import io.micronaut.http.annotation.*;
154+
import io.micronaut.core.annotation.*;
155+
import io.reactivex.Maybe;
156+
157+
@Controller("/")
158+
class MyController {
159+
@Get
160+
public Maybe<TestInterface> hey() {
161+
return Maybe.empty();
162+
}
163+
}
164+
165+
@Schema(oneOf = {TestImpl1.class, TestImpl2.class})
166+
interface TestInterface {
167+
String getType();
168+
}
169+
170+
class TestImpl1 implements TestInterface {
171+
172+
private TestInterface woopsie;
173+
174+
@Override
175+
public String getType() {
176+
return null;
177+
}
178+
179+
/**
180+
* Some docs
181+
*/
182+
@Nullable
183+
@Deprecated
184+
public TestInterface getWoopsie() {
185+
return woopsie;
186+
}
187+
188+
public void setWoopsie(TestInterface woopsie) {
189+
this.woopsie = woopsie;
190+
}
191+
}
192+
193+
class TestImpl2 implements TestInterface {
194+
@Override
195+
public String getType() {
196+
return null;
197+
}
198+
}
199+
200+
@javax.inject.Singleton
201+
class MyBean {}
202+
''')
203+
204+
OpenAPI openAPI = AbstractOpenApiVisitor.testReference
205+
Map<String, Schema> schemas = openAPI.getComponents().getSchemas()
206+
207+
expect:
208+
Schema testImpl1 = schemas.get("TestImpl1")
209+
Schema woopsieProperty = testImpl1.getProperties()["woopsie"]
210+
woopsieProperty instanceof ComposedSchema
211+
((ComposedSchema) woopsieProperty).allOf[0].$ref == "#/components/schemas/TestInterface"
212+
((ComposedSchema) woopsieProperty).allOf[1].deprecated
213+
((ComposedSchema) woopsieProperty).allOf[1].description == "Some docs"
214+
((ComposedSchema) woopsieProperty).allOf[1].nullable
215+
}
216+
217+
}

0 commit comments

Comments
 (0)