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

[Inline model resolver] better handling of inline responses and bug fixes #12353

Merged
merged 10 commits into from May 13, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -90,10 +90,34 @@ private void flattenPaths() {
}

for (Map.Entry<String, PathItem> pathsEntry : paths.entrySet()) {
String pathname = pathsEntry.getKey();
PathItem path = pathsEntry.getValue();
List<Operation> operations = new ArrayList<>(path.readOperations());

// use path name (e.g. /foo/bar) and HTTP verb to come up with a name
// in case operationId is not defined later in other methods
String pathname = pathsEntry.getKey();
String name = pathname;
if (path.getDelete() != null) {
name = pathname + "_delete";
} else if (path.getGet() != null) {
name = pathname + "_get";
} else if (path.getHead() != null) {
name = pathname + "_head";
} else if (path.getOptions() != null) {
name = pathname + "_options";
} else if (path.getPatch() != null) {
name = pathname + "_patch";
} else if (path.getPost() != null) {
name = pathname + "_post";
} else if (path.getPut() != null) {
name = pathname + "_put";
} else if (path.getTrace() != null) {
name = pathname + "_trace";
} else {
// no HTTP verb defined?
//throw new RuntimeException("No HTTP verb found/detected in the inline model resolver");
}

// Include callback operation as well
for (Operation operation : path.readOperations()) {
Map<String, Callback> callbacks = operation.getCallbacks();
Expand All @@ -106,9 +130,9 @@ private void flattenPaths() {
}

for (Operation operation : operations) {
flattenRequestBody(pathname, operation);
flattenParameters(pathname, operation);
flattenResponses(pathname, operation);
flattenRequestBody(name, operation);
flattenParameters(name, operation);
flattenResponses(name, operation);
}
}
}
Expand Down Expand Up @@ -206,7 +230,7 @@ private void gatherInlineModels(Schema schema, String modelPrefix) {
if (schema.getAdditionalProperties() != null) {
if (schema.getAdditionalProperties() instanceof Schema) {
Schema inner = (Schema) schema.getAdditionalProperties();
String schemaName = resolveModelName(schema.getTitle(), modelPrefix + "_" + "_value");
String schemaName = resolveModelName(schema.getTitle(), modelPrefix + "_value");
// Recurse to create $refs for inner models
gatherInlineModels(inner, schemaName);
if (isModelNeeded(inner)) {
Expand Down Expand Up @@ -362,10 +386,10 @@ private void flattenContent(Content content, String name) {
/**
* Flatten inline models in RequestBody
*
* @param pathname target pathname
* @param modelName inline model name prefix
* @param operation target operation
*/
private void flattenRequestBody(String pathname, Operation operation) {
private void flattenRequestBody(String modelName, Operation operation) {
RequestBody requestBody = operation.getRequestBody();
if (requestBody == null) {
return;
Expand All @@ -377,8 +401,8 @@ private void flattenRequestBody(String pathname, Operation operation) {
requestBody = openAPI.getComponents().getRequestBodies().get(ref);
}

String name = operation.getOperationId() == null ? "inline_request" : operation.getOperationId() + "_request";
flattenContent(requestBody.getContent(), name);
flattenContent(requestBody.getContent(),
(operation.getOperationId() == null ? modelName : operation.getOperationId()) + "_request");
}

/**
Expand Down Expand Up @@ -438,10 +462,10 @@ private void flattenParameters(String pathname, Operation operation) {
/**
* Flatten inline models in ApiResponses
*
* @param pathname target pathname
* @param modelName model name prefix
* @param operation target operation
*/
private void flattenResponses(String pathname, Operation operation) {
private void flattenResponses(String modelName, Operation operation) {
ApiResponses responses = operation.getResponses();
if (responses == null) {
return;
Expand All @@ -450,78 +474,9 @@ private void flattenResponses(String pathname, Operation operation) {
for (Map.Entry<String, ApiResponse> responsesEntry : responses.entrySet()) {
String key = responsesEntry.getKey();
ApiResponse response = responsesEntry.getValue();
if (ModelUtils.getSchemaFromResponse(response) == null) {
continue;
}

Schema property = ModelUtils.getSchemaFromResponse(response);
if (property instanceof ObjectSchema) {
ObjectSchema op = (ObjectSchema) property;
if (op.getProperties() != null && op.getProperties().size() > 0) {
String modelName = resolveModelName(op.getTitle(), "inline_response_" + key);
Schema model = modelFromProperty(openAPI, op, modelName);
String existing = matchGenerated(model);
Content content = response.getContent();
for (MediaType mediaType : content.values()) {
if (existing != null) {
Schema schema = this.makeSchema(existing, property);
schema.setRequired(op.getRequired());
mediaType.setSchema(schema);
} else {
modelName = addSchemas(modelName, model);
Schema schema = this.makeSchema(modelName, property);
schema.setRequired(op.getRequired());
mediaType.setSchema(schema);
}
}
}
} else if (property instanceof ArraySchema) {
ArraySchema ap = (ArraySchema) property;
Schema inner = ap.getItems();
if (inner instanceof ObjectSchema) {
ObjectSchema op = (ObjectSchema) inner;
if (op.getProperties() != null && op.getProperties().size() > 0) {
flattenProperties(openAPI, op.getProperties(), pathname);
String modelName = resolveModelName(op.getTitle(),
"inline_response_" + key);
Schema innerModel = modelFromProperty(openAPI, op, modelName);
String existing = matchGenerated(innerModel);
if (existing != null) {
Schema schema = this.makeSchema(existing, op);
schema.setRequired(op.getRequired());
ap.setItems(schema);
} else {
modelName = addSchemas(modelName, innerModel);
Schema schema = this.makeSchema(modelName, op);
schema.setRequired(op.getRequired());
ap.setItems(schema);
}
}
}
} else if (property instanceof MapSchema) {
MapSchema mp = (MapSchema) property;
Schema innerProperty = ModelUtils.getAdditionalProperties(openAPI, mp);
if (innerProperty instanceof ObjectSchema) {
ObjectSchema op = (ObjectSchema) innerProperty;
if (op.getProperties() != null && op.getProperties().size() > 0) {
flattenProperties(openAPI, op.getProperties(), pathname);
String modelName = resolveModelName(op.getTitle(),
"inline_response_" + key);
Schema innerModel = modelFromProperty(openAPI, op, modelName);
String existing = matchGenerated(innerModel);
if (existing != null) {
Schema schema = new Schema().$ref(existing);
schema.setRequired(op.getRequired());
mp.setAdditionalProperties(schema);
} else {
modelName = addSchemas(modelName, innerModel);
Schema schema = new Schema().$ref(modelName);
schema.setRequired(op.getRequired());
mp.setAdditionalProperties(schema);
}
}
}
}
flattenContent(response.getContent(),
(operation.getOperationId() == null ? modelName : operation.getOperationId()) + "_" + key + "_response");
}
}

Expand Down Expand Up @@ -608,12 +563,7 @@ private void flattenComponents() {
flattenComposedChildren(modelName + "_anyOf", m.getAnyOf());
flattenComposedChildren(modelName + "_oneOf", m.getOneOf());
} else if (model instanceof Schema) {
//Schema m = model;
//Map<String, Schema> properties = m.getProperties();
//flattenProperties(openAPI, properties, modelName);
//fixStringModel(m);
gatherInlineModels(model, modelName);

} /*else if (ModelUtils.isArraySchema(model)) {
ArraySchema m = (ArraySchema) model;
Schema inner = m.getItems();
Expand Down
Expand Up @@ -2356,6 +2356,7 @@ public void testUseOneOfInterfaces() {
.get("application/json")
.getSchema());
Assert.assertEquals(s.getExtensions().get("x-one-of-name"), "CreateStateRequest");

Assert.assertEquals(
openAPI.getPaths()
.get("/state")
Expand All @@ -2364,11 +2365,13 @@ public void testUseOneOfInterfaces() {
.get("200")
.getContent()
.get("application/json")
.getSchema()
.getExtensions()
.get("x-one-of-name"),
"GetState200"
.getSchema().get$ref(),
"#/components/schemas/getState_200_response"
);
Schema getState200 = openAPI.getComponents().getSchemas().get("getState_200_response");
//Assert.assertEquals(getState200, "");
Assert.assertEquals(getState200.getExtensions().get("x-one-of-name"), "GetState200Response");

// for the array schema, assert that a oneOf interface was added to schema map
Schema items = ((ArraySchema) openAPI.getComponents().getSchemas().get("CustomOneOfArraySchema")).getItems();
Assert.assertEquals(items.get$ref(), "#/components/schemas/CustomOneOfArraySchema_inner");
Expand Down
Expand Up @@ -269,7 +269,7 @@ public void testInlineResponseModelType() {
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/6150_model_json_inline.yaml");
new InlineModelResolver().flatten(openAPI);

Schema InlineResponse200 = openAPI.getComponents().getSchemas().get("inline_response_200");
Schema InlineResponse200 = openAPI.getComponents().getSchemas().get("testOperation_200_response");
assertEquals("object", InlineResponse200.getType());
assertEquals("unknown", InlineResponse200.getFormat());
Schema FooBarObject = openAPI.getComponents().getSchemas().get("FooBarObject");
Expand Down Expand Up @@ -475,7 +475,7 @@ public void resolveInlineArrayResponse() {
assertTrue(mediaType.getSchema() instanceof ArraySchema);

ArraySchema responseSchema = (ArraySchema) mediaType.getSchema();
assertEquals("#/components/schemas/inline_response_200", responseSchema.getItems().get$ref());
assertEquals("#/components/schemas/resolveInlineArrayResponse_200_response_inner", responseSchema.getItems().get$ref());

Schema items = ModelUtils.getReferencedSchema(openAPI, responseSchema.getItems());
assertTrue(items.getProperties().get("array_response_property") instanceof StringSchema);
Expand Down Expand Up @@ -566,13 +566,13 @@ public void resolveInlineMapSchemaInResponse() {

Schema additionalProperties = (Schema) mediaType.getSchema().getAdditionalProperties();
assertNotNull(additionalProperties.get$ref());
assertTrue(additionalProperties.get$ref().startsWith("#/components/schemas/inline_response_"));
assertEquals("#/components/schemas/resolveInlineMapSchemaInResponse_200_response_value", additionalProperties.get$ref());

Schema referencedSchema = ModelUtils.getReferencedSchema(openAPI, additionalProperties);
Schema referencedSchemaProperty = (Schema) referencedSchema.getProperties().get("resolve_inline_map_schema_in_response_property");

assertEquals(
"#/components/schemas/_resolve_inline_map_schema_in_response_resolve_inline_map_schema_in_response_property",
"#/components/schemas/resolveInlineMapSchemaInResponse_200_response_value_resolve_inline_map_schema_in_response_property",
referencedSchemaProperty.get$ref()
);
assertNotNull(ModelUtils.getReferencedSchema(openAPI, referencedSchemaProperty));
Expand Down
Expand Up @@ -517,8 +517,8 @@ public void testDoGenerateRequestBodyRequiredAttribute_3134_Regression() throws
generator.opts(input).generate();

JavaFileAssert.assertThat(Paths.get(outputPath + "/src/main/java/org/openapitools/api/ExampleApi.java"))
.assertMethod("exampleApiPost", "InlineRequest")
.hasParameter("inlineRequest")
.assertMethod("exampleApiPost", "ExampleApiPostRequest")
.hasParameter("exampleApiPostRequest")
.assertParameterAnnotations()
.containsWithNameAndAttributes("RequestBody", ImmutableMap.of("required", "false"));

Expand Down Expand Up @@ -950,7 +950,7 @@ public void shouldNotAddNotNullOnReadOnlyAttributes() throws IOException {
}

@Test
public void oneOf_5381() throws IOException {
public void testOneOf5381() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();
String outputPath = output.getAbsolutePath().replace('\\', '/');
Expand Down Expand Up @@ -988,7 +988,7 @@ public void oneOf_5381() throws IOException {
}

@Test
public void oneOf_allOf() throws IOException {
public void testOneOfAndAllOf() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();
String outputPath = output.getAbsolutePath().replace('\\', '/');
Expand All @@ -1007,7 +1007,7 @@ public void oneOf_allOf() throws IOException {
DefaultGenerator generator = new DefaultGenerator();
codegen.setHateoas(true);
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
// generator.setGeneratorPropertyDefault(CodegenConstants.USE_ONEOF_DISCRIMINATOR_LOOKUP, "true");
//generator.setGeneratorPropertyDefault(CodegenConstants.USE_ONEOF_DISCRIMINATOR_LOOKUP, "true");
generator.setGeneratorPropertyDefault(CodegenConstants.LEGACY_DISCRIMINATOR_BEHAVIOR, "false");

codegen.setUseOneOfInterfaces(true);
Expand Down
Expand Up @@ -47,9 +47,9 @@ private Generator getGenerator(CodegenConfigurator config) {
private void checkAPIFile(List<File> files, String apiFileName) throws IOException {
File apiFile = files.stream().filter(file->file.getName().contains(apiFileName)).findFirst().get();
String apiFileContent = FileUtils.readFileToString(apiFile, StandardCharsets.UTF_8);
Assert.assertTrue(!apiFileContent.contains("import { OrganizationWrapper | PersonWrapper }"));
Assert.assertTrue(!apiFileContent.contains("import { GetCustomer200Response | PersonWrapper }"));
Assert.assertEquals(StringUtils.countMatches(apiFileContent,"import { PersonWrapper }"),1);
Assert.assertEquals(StringUtils.countMatches(apiFileContent,"import { OrganizationWrapper }"),1);
Assert.assertEquals(StringUtils.countMatches(apiFileContent,"import { GetCustomer200Response }"),1);
}

@Test
Expand Down
Expand Up @@ -56,7 +56,7 @@ public void testComposedSchemasImportTypesIndividually() {
// TODO revise the commented test below as oneOf is no longer defined inline
//but instead defined using $ref with the new inline model resolver in 6.x
//Assert.assertEquals(operation.imports, Sets.newHashSet("Cat", "Dog"));
Assert.assertEquals(operation.imports, Sets.newHashSet("InlineRequest"));
Assert.assertEquals(operation.imports, Sets.newHashSet("PetsPatchRequest"));

}

Expand Down
Expand Up @@ -56,11 +56,12 @@ public void testGetAllUsedSchemas() {
"SomeObj15",
"SomeMapObj16",
"MapItem16",
"p17_200_response",
"SomeObj17",
"SomeObj18",
"Common18",
"SomeObj18_allOf",
"inline_request",
"_some_p19_patch_request",
"Obj19ByAge",
"Obj19ByType",
"SomeObj20",
Expand Down Expand Up @@ -217,11 +218,11 @@ public void testComposedSchemasAreNotUnaliased() {
}

@Test
public void testAliasedTypeIsNotUnaliasedIfUsedForImportMapping(){
public void testAliasedTypeIsNotUnaliasedIfUsedForImportMapping() {
Schema emailSchema = new Schema().$ref("#/components/schemas/Email").type("string");
StringSchema stringSchema = new StringSchema();
HashMap<String, String> importMappings = new HashMap<>();
importMappings.put("Email","foo.bar.Email");
importMappings.put("Email", "foo.bar.Email");

OpenAPI openAPI = TestUtils.createOpenAPIWithOneSchema("Email", stringSchema);

Expand Down
@@ -1,40 +1,44 @@
openapi: 3.0.1
info:
title: 'Buggy Api'
version: '1.0'
title: Buggy Api
version: "1.0"
servers:
- url: /
paths:
/foo/bar:
post:
responses:
'200':
description: ok
"200":
content:
'*/*':
schema:
$ref: '#/components/schemas/inline_response_200'

$ref: '#/components/schemas/_foo_bar_post_200_response'
description: ok
/foo/bar2:
post:
responses:
'200':
description: ok
"200":
content:
'*/*':
schema:
$ref: '#/components/schemas/bar2'
description: ok
components:
schemas:
bar2:
type: object
example: { n: 4.56 }
example:
"n": 4.56
properties:
n:
type: number
"n":
example: 4.56
inline_response_200:
type: number
type: object
example: { n: 1.23 }
_foo_bar_post_200_response:
example:
"n": 1.23
properties:
n:
type: number
"n":
example: 1.23
type: number
type: object
x-original-swagger-version: "2.0"