Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use nested type param names in schema name, resolve types earlier #393

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,19 @@ public Schema register(Type entityType, Schema schema) {
* @return a reference to the newly registered {@link Schema}
*/
private Schema register(TypeKey key, Schema schema, String schemaName) {
String name = deriveName(key, schemaName);
Schema schemaRef = new SchemaImpl();
schemaRef.setRef(OpenApiConstants.REF_PREFIX_SCHEMA + name);

registry.put(key, new GeneratedSchemaInfo(name, schema, schemaRef));
names.add(name);

ModelUtil.components(oai).addSchema(name, schema);

return schemaRef;
}

String deriveName(TypeKey key, String schemaName) {
/*
* We cannot use the 'name' on the SchemaImpl because it may be a
* property name rather then a schema name.
Expand All @@ -264,15 +277,7 @@ private Schema register(TypeKey key, Schema schema, String schemaName) {
name = nameBase + idx++;
}

Schema schemaRef = new SchemaImpl();
schemaRef.setRef(OpenApiConstants.REF_PREFIX_SCHEMA + name);

registry.put(key, new GeneratedSchemaInfo(name, schema, schemaRef));
names.add(name);

ModelUtil.components(oai).addSchema(name, schema);

return schemaRef;
return name;
}

public Schema lookupRef(Type instanceType) {
Expand Down Expand Up @@ -331,13 +336,7 @@ public String defaultName() {

switch (type.kind()) {
case PARAMETERIZED_TYPE:
for (Type param : type.asParameterizedType().arguments()) {
if (param.kind() == Type.Kind.WILDCARD_TYPE) {
name.append(wildcardName(param.asWildcardType()));
} else {
name.append(param.name().local());
}
}
appendParameterNames(name, type.asParameterizedType());
break;
case WILDCARD_TYPE:
name.append(wildcardName(type.asWildcardType()));
Expand All @@ -349,6 +348,23 @@ public String defaultName() {
return name.toString();
}

static void appendParameterNames(StringBuilder name, ParameterizedType type) {
for (Type param : type.asParameterizedType().arguments()) {
switch (param.kind()) {
case PARAMETERIZED_TYPE:
name.append(param.name().local());
appendParameterNames(name, param.asParameterizedType());
break;
case WILDCARD_TYPE:
name.append(wildcardName(param.asWildcardType()));
break;
default:
name.append(param.name().local());
break;
}
}
}

static String wildcardName(WildcardType type) {
Type superBound = type.superBound();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ public Type processType() {
type = TypeUtil.resolveWildcard(type.asWildcardType());
}

if (type.kind() == Type.Kind.TYPE_VARIABLE ||
type.kind() == Type.Kind.UNRESOLVED_TYPE_VARIABLE) {
// Resolve type variable to real variable.
type = resolveTypeVariable(schema, type, false);
}

if (type.kind() == Type.Kind.ARRAY) {
DataObjectLogging.log.processingArray(type);
ArrayType arrayType = type.asArrayType();
Expand Down Expand Up @@ -118,12 +124,6 @@ public Type processType() {
return readParameterizedType(type.asParameterizedType());
}

if (type.kind() == Type.Kind.TYPE_VARIABLE ||
type.kind() == Type.Kind.UNRESOLVED_TYPE_VARIABLE) {
// Resolve type variable to real variable.
return resolveTypeVariable(schema, type);
}

// Raw Collection
if (isA(type, COLLECTION_TYPE)) {
return ARRAY_TYPE_OBJECT;
Expand Down Expand Up @@ -205,7 +205,7 @@ private Schema resolveParameterizedType(Type valueType, Schema propsSchema) {
if (valueType.kind() == Type.Kind.TYPE_VARIABLE ||
valueType.kind() == Type.Kind.UNRESOLVED_TYPE_VARIABLE ||
valueType.kind() == Type.Kind.WILDCARD_TYPE) {
Type resolved = resolveTypeVariable(propsSchema, valueType);
Type resolved = resolveTypeVariable(propsSchema, valueType, true);
if (index.containsClass(resolved)) {
propsSchema.type(Schema.SchemaType.OBJECT);
propsSchema = SchemaRegistry.checkRegistration(valueType, typeResolver, propsSchema);
Expand All @@ -225,23 +225,19 @@ private Schema resolveParameterizedType(Type valueType, Schema propsSchema) {
return propsSchema;
}

private Type resolveTypeVariable(Schema schema, Type fieldType) {
private Type resolveTypeVariable(Schema schema, Type fieldType, boolean pushToStack) {
// Type variable (e.g. A in Foo<A>)
Type resolvedType = typeResolver.getResolvedType(fieldType);

DataObjectLogging.log.resolvedType(fieldType, resolvedType);

if (isTerminalType(resolvedType) || !index.containsClass(resolvedType)) {
DataObjectLogging.log.terminalType(resolvedType);
TypeUtil.applyTypeAttributes(resolvedType, schema);
} else {
DataObjectLogging.log.typeVarSubstitution(fieldType, resolvedType);
if (index.containsClass(resolvedType)) {
// Add resolved type to stack.
objectStack.push(annotationTarget, parentPathEntry, resolvedType, schema);
} else {
DataObjectLogging.log.classNotAvailable(resolvedType);
}
} else if (pushToStack) {
// Add resolved type to stack.
objectStack.push(annotationTarget, parentPathEntry, resolvedType, schema);
}

return resolvedType;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.smallrye.openapi.runtime.scanner;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.UUID;
Expand Down Expand Up @@ -57,6 +58,7 @@ public void testGenericsApplication() throws IOException, JSONException {
ResidentsResource.class,
Result.class,
ResultList.class,
POJO.class,
List.class);
OpenApiAnnotationScanner scanner = new OpenApiAnnotationScanner(nestingSupportConfig(), i);
OpenAPI result = scanner.scan();
Expand Down Expand Up @@ -134,6 +136,18 @@ public boolean equals(Object obj) {
}
}

static class POJO extends BaseModel {
private String name;

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}
}

static class KingCrimson extends BaseModel {
public enum Status {
unknown,
Expand Down Expand Up @@ -260,7 +274,7 @@ public String getDescription() {

}

static class Result<T extends BaseModel> {
static class Result<T> {
private T result;
private Message error;
private Integer status;
Expand All @@ -277,7 +291,7 @@ public T getResult() {
return result;
}

public static class ResultBuilder<T extends BaseModel> {
public static class ResultBuilder<T> {
private Integer status;
private Message error = new Message();
private T result;
Expand Down Expand Up @@ -400,6 +414,15 @@ public ResultList<KingCrimson> getAll(@QueryParam("id") String id,
return super.getAll1();
}

@GET
@Path("/noooooooo")
public Result<List<POJO>> getList() {
return new Result.ResultBuilder<List<POJO>>()
.status(200)
.result(new ArrayList<>())
.build();
}

@POST
@RequestBody(content = @Content(schema = @Schema(implementation = KingCrimson.class)))
public Result<KingCrimson> post(KingCrimson deployment) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void testKitchenSink() throws IOException {
}

/**
* Test parameterised type as a top-level entity (i.e. not just a bare class).
* Test parameterized type as a top-level entity (i.e. not just a bare class).
*
* @see org.jboss.jandex.ParameterizedType
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ public class SchemaRegistryTests extends IndexScannerTestBase {

@Test
public void testParameterizedNameCollisionsUseSequence() throws IOException, JSONException {
Indexer indexer = new Indexer();
index(indexer, "io/smallrye/openapi/runtime/scanner/SchemaRegistryTests$Container.class");
index(indexer, "io/smallrye/openapi/runtime/scanner/SchemaRegistryTests$Nestable.class");
Index index = indexer.complete();

Index index = indexOf(Container.class, Nestable.class);
OpenAPIImpl oai = new OpenAPIImpl();
SchemaRegistry registry = SchemaRegistry.newInstance(emptyConfig(), oai, index);

Expand All @@ -48,9 +44,9 @@ public void testParameterizedNameCollisionsUseSequence() throws IOException, JSO
Schema s2 = registry.register(n2.type(), new SchemaImpl());
Schema s3 = registry.register(n3.type(), new SchemaImpl());

assertEquals("#/components/schemas/NestableStringNestable", s1.getRef());
assertEquals("#/components/schemas/NestableStringNestable1", s2.getRef());
assertEquals("#/components/schemas/NestableStringNestable2", s3.getRef());
assertEquals("#/components/schemas/NestableStringNestableStringString", s1.getRef());
assertEquals("#/components/schemas/NestableStringNestableStringObject", s2.getRef());
assertEquals("#/components/schemas/NestableStringNestableStringNestableStringObject", s3.getRef());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"components": {
"schemas": {
"KustomPairFuzzObject": {
"KustomPairFuzzStringDateObject": {
"required": [
"bar",
"foo"
Expand Down Expand Up @@ -50,7 +50,7 @@
}
}
},
"FuzzKustomPairDouble": {
"FuzzKustomPairFuzzStringDateObjectDouble": {
"type": "object",
"properties": {
"qAgain": {
Expand All @@ -67,13 +67,13 @@
"type": "number"
},
"tAgain2": {
"$ref": "#/components/schemas/KustomPairFuzzObject"
"$ref": "#/components/schemas/KustomPairFuzzStringDateObject"
},
"tAgain4": {
"$ref": "#/components/schemas/KustomPairFuzzObject"
"$ref": "#/components/schemas/KustomPairFuzzStringDateObject"
},
"tValue": {
"$ref": "#/components/schemas/KustomPairFuzzObject"
"$ref": "#/components/schemas/KustomPairFuzzStringDateObject"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
}
}
},
"KustomPairKustomPairInteger": {
"KustomPairKustomPairStringStringInteger": {
"type": "object",
"required": [
"bar",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@
}
}
},
"KustomPairFuzzInteger": {
"KustomPairFuzzStringObjectInteger": {
"required": [
"bar",
"foo"
Expand All @@ -224,7 +224,7 @@
}
}
},
"FuzzKustomPairDouble": {
"FuzzKustomPairFuzzStringObjectIntegerDouble": {
"type": "object",
"properties": {
"qAgain": {
Expand All @@ -241,13 +241,13 @@
"type": "number"
},
"tAgain2": {
"$ref": "#/components/schemas/KustomPairFuzzInteger"
"$ref": "#/components/schemas/KustomPairFuzzStringObjectInteger"
},
"tAgain4": {
"$ref": "#/components/schemas/KustomPairFuzzInteger"
"$ref": "#/components/schemas/KustomPairFuzzStringObjectInteger"
},
"tValue": {
"$ref": "#/components/schemas/KustomPairFuzzInteger"
"$ref": "#/components/schemas/KustomPairFuzzStringObjectInteger"
}
}
},
Expand Down Expand Up @@ -311,7 +311,7 @@
}
}
},
"KustomPairKustomPairInteger": {
"KustomPairKustomPairStringStringInteger": {
"required": [
"bar",
"foo"
Expand Down Expand Up @@ -444,7 +444,7 @@
}
},
"complexNesting": {
"$ref": "#/components/schemas/FuzzKustomPairDouble"
"$ref": "#/components/schemas/FuzzKustomPairFuzzStringObjectIntegerDouble"
},
"creditCardMap": {
"type": "object",
Expand Down Expand Up @@ -475,7 +475,7 @@
}
},
"nesting": {
"$ref": "#/components/schemas/KustomPairKustomPairInteger"
"$ref": "#/components/schemas/KustomPairKustomPairStringStringInteger"
},
"password": {
"type": "string",
Expand Down
Loading