Skip to content
Open
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 @@ -115,6 +115,15 @@ public class SpringCodegen extends AbstractJavaCodegen
public static final String USE_SEALED = "useSealed";
public static final String OPTIONAL_ACCEPT_NULLABLE = "optionalAcceptNullable";
public static final String USE_SPRING_BUILT_IN_VALIDATION = "useSpringBuiltInValidation";

/** Default importMapping value registered by {@link AbstractJavaCodegen} for the swagger2 annotation. */
private static final String SWAGGER2_ANNOTATION_SCHEMA_IMPORT = "io.swagger.v3.oas.annotations.media.Schema";

/** Simple or fully-qualified name to use at swagger2 {@code @Schema} annotation usage sites. */
private static final String SCHEMA_ANNOTATION = "swagger2SchemaAnnotation";

/** Whether templates should emit the single-type import for the swagger2 {@code @Schema} annotation. */
private static final String IMPORT_SCHEMA_ANNOTATION = "importSwagger2SchemaAnnotation";
public static final String SPRING_API_VERSION = "springApiVersion";
public static final String USE_JACKSON_3 = "useJackson3";
public static final String JACKSON2_PACKAGE = "com.fasterxml.jackson";
Expand Down Expand Up @@ -937,10 +946,44 @@ public void addOperationToGroup(String tag, String resourcePath, Operation opera

}

/**
* Whether any schema in the document produces the given generated model name.
* Compared against {@link #toModelName(String)} so modelNamePrefix/modelNameSuffix are honoured.
*/
private boolean hasModelNamed(OpenAPI openAPI, String modelName) {
if (openAPI == null || openAPI.getComponents() == null || openAPI.getComponents().getSchemas() == null) {
return false;
}
return openAPI.getComponents().getSchemas().keySet().stream()
.anyMatch(schemaName -> modelName.equals(toModelName(schemaName)));
}

@Override
public void preprocessOpenAPI(OpenAPI openAPI) {
super.preprocessOpenAPI(openAPI);

// A schema named "Schema" collides with io.swagger.v3.oas.annotations.media.Schema. Two

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to break this check out into a more generic

handleSchemaNameCollision(String name, String import, String schemaKey, String importKey)

and then that you do

handleSchemaNameCollision("Schema", SWAGGER2_ANNOTATION_SCHEMA_IMPORT, SCHEMA_ANNOTATION, IMPORT_SCHEMA_ANNOTATION)

?
This should somewhat allow someone to fix this issue for similar issues, and it would also allow us to have a more generic description of the cause as javadoc on handleSchemaNameCollision rather than tying the description exactly to the reported scenario

// problems follow:
// 1. AbstractJavaCodegen registers "Schema" -> the annotation FQN in importMapping, and
// importMapping is consulted before toModelImport(), so the model import is silently
// replaced by the annotation import. The bare "Schema" type in api signatures then
// resolves to the annotation instead of the model.
// 2. Emitting the model import alongside the annotation import the templates add is a
// compile error: two single-type imports for the same simple name (JLS 7.5.1). In the
// model file the declared class shadows the import for the same reason.
// Let the model keep the simple name and fully qualify the annotation at its usage sites
// instead of importing it. An explicit --import-mappings entry for "Schema" carries a
// different value and is deliberately left untouched.
additionalProperties.put(SCHEMA_ANNOTATION, "Schema");
additionalProperties.put(IMPORT_SCHEMA_ANNOTATION, true);
if (hasModelNamed(openAPI, "Schema")) {
if (SWAGGER2_ANNOTATION_SCHEMA_IMPORT.equals(importMapping.get("Schema"))) {
importMapping.remove("Schema");
}
additionalProperties.put(SCHEMA_ANNOTATION, SWAGGER2_ANNOTATION_SCHEMA_IMPORT);
additionalProperties.put(IMPORT_SCHEMA_ANNOTATION, false);
}

if (SPRING_BOOT.equals(library) && ModelUtils.containsEnums(this.openAPI)) {
supportingFiles.add(new SupportingFile("converter.mustache",
(sourceFolder + File.separator + configPackage).replace(".", java.io.File.separator), "EnumConverterConfiguration.java"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.Parameters;
import io.swagger.v3.oas.annotations.media.ArraySchema;
import io.swagger.v3.oas.annotations.media.Content;
{{#importSwagger2SchemaAnnotation}}
import io.swagger.v3.oas.annotations.media.Schema;
{{/importSwagger2SchemaAnnotation}}
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.security.SecurityRequirement;
import io.swagger.v3.oas.annotations.tags.Tag;
Expand Down Expand Up @@ -187,7 +189,7 @@ public interface {{classname}} {
{{#responses}}
@ApiResponse(responseCode = {{#isDefault}}"default"{{/isDefault}}{{^isDefault}}"{{{code}}}"{{/isDefault}}, description = "{{{message}}}"{{#baseType}}, content = {
{{#produces}}
@Content(mediaType = "{{{mediaType}}}", {{#isArray}}array = @ArraySchema({{/isArray}}schema = @Schema(implementation = {{{baseType}}}.class){{#isArray}}){{/isArray}}{{^isJson}}){{^-last}},{{/-last}}{{/isJson}}{{#isJson}}{{^examples.0}}){{^-last}},{{/-last}}{{/examples.0}}{{#examples.0}}, examples = {
@Content(mediaType = "{{{mediaType}}}", {{#isArray}}array = @ArraySchema({{/isArray}}schema = @{{swagger2SchemaAnnotation}}(implementation = {{{baseType}}}.class){{#isArray}}){{/isArray}}{{^isJson}}){{^-last}},{{/-last}}{{/isJson}}{{#isJson}}{{^examples.0}}){{^-last}},{{/-last}}{{/examples.0}}{{#examples.0}}, examples = {
{{#examples}}
@ExampleObject(
name = "{{{exampleName}}}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ package {{package}};
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.media.Content;
{{#importSwagger2SchemaAnnotation}}
import io.swagger.v3.oas.annotations.media.Schema;
{{/importSwagger2SchemaAnnotation}}
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.security.SecurityRequirement;
import io.swagger.v3.oas.annotations.tags.Tag;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
{{/required}}
{{/useBeanValidation}}
{{#swagger2AnnotationLibrary}}
@Schema(name = "{{{baseName}}}"{{#isReadOnly}}, accessMode = Schema.AccessMode.READ_ONLY{{/isReadOnly}}{{#example}}, example = "{{{.}}}"{{/example}}{{#description}}, description = "{{{.}}}"{{/description}}{{#deprecated}}, deprecated = true{{/deprecated}}, requiredMode = {{#required}}Schema.RequiredMode.REQUIRED{{/required}}{{^required}}Schema.RequiredMode.NOT_REQUIRED{{/required}})
@{{swagger2SchemaAnnotation}}(name = "{{{baseName}}}"{{#isReadOnly}}, accessMode = {{swagger2SchemaAnnotation}}.AccessMode.READ_ONLY{{/isReadOnly}}{{#example}}, example = "{{{.}}}"{{/example}}{{#description}}, description = "{{{.}}}"{{/description}}{{#deprecated}}, deprecated = true{{/deprecated}}, requiredMode = {{#required}}{{swagger2SchemaAnnotation}}.RequiredMode.REQUIRED{{/required}}{{^required}}{{swagger2SchemaAnnotation}}.RequiredMode.NOT_REQUIRED{{/required}})
{{/swagger2AnnotationLibrary}}
{{#jackson}}{{>jackson_annotations}}{{/jackson}}
{{/lombok.Data}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import {{jacksonPackage}}.dataformat.xml.annotation.JacksonXmlElementWrapper;
{{/withXml}}
{{/jackson}}
{{#swagger2AnnotationLibrary}}
{{#importSwagger2SchemaAnnotation}}
import io.swagger.v3.oas.annotations.media.Schema;
{{/importSwagger2SchemaAnnotation}}
{{/swagger2AnnotationLibrary}}

{{#withXml}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
@ApiModel(description = "{{{description}}}")
{{/swagger1AnnotationLibrary}}
{{#swagger2AnnotationLibrary}}
@Schema({{#name}}name = "{{name}}", {{/name}}description = "{{{description}}}"{{#deprecated}}, deprecated = true{{/deprecated}})
@{{swagger2SchemaAnnotation}}({{#name}}name = "{{name}}", {{/name}}description = "{{{description}}}"{{#deprecated}}, deprecated = true{{/deprecated}})
{{/swagger2AnnotationLibrary}}
{{/description}}
{{#discriminator}}
Expand Down Expand Up @@ -231,7 +231,7 @@ public {{>sealed}}class {{classname}}{{#parent}} extends {{{parent}}}{{/parent}}
{{#lambda.trim}}{{>notNull}}{{/lambda.trim}}
{{/useBeanValidation}}
{{#swagger2AnnotationLibrary}}
@Schema(name = "{{{baseName}}}"{{#isReadOnly}}, accessMode = Schema.AccessMode.READ_ONLY{{/isReadOnly}}{{#example}}, example = "{{{.}}}"{{/example}}{{#description}}, description = "{{{.}}}"{{/description}}{{#deprecated}}, deprecated = true{{/deprecated}}, requiredMode = {{#required}}Schema.RequiredMode.REQUIRED{{/required}}{{^required}}Schema.RequiredMode.NOT_REQUIRED{{/required}}{{#isNullable}}, nullable = true{{/isNullable}})
@{{swagger2SchemaAnnotation}}(name = "{{{baseName}}}"{{#isReadOnly}}, accessMode = {{swagger2SchemaAnnotation}}.AccessMode.READ_ONLY{{/isReadOnly}}{{#example}}, example = "{{{.}}}"{{/example}}{{#description}}, description = "{{{.}}}"{{/description}}{{#deprecated}}, deprecated = true{{/deprecated}}, requiredMode = {{#required}}{{swagger2SchemaAnnotation}}.RequiredMode.REQUIRED{{/required}}{{^required}}{{swagger2SchemaAnnotation}}.RequiredMode.NOT_REQUIRED{{/required}}{{#isNullable}}, nullable = true{{/isNullable}})
{{/swagger2AnnotationLibrary}}
{{#swagger1AnnotationLibrary}}
@ApiModelProperty({{#example}}example = "{{{.}}}", {{/example}}{{#required}}required = {{required}}, {{/required}}{{#isReadOnly}}readOnly = {{{isReadOnly}}}, {{/isReadOnly}}value = "{{{description}}}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2286,6 +2286,56 @@ public void testTypeMappings() {
Assert.assertEquals(codegen.typeMapping().get("file"), "org.springframework.core.io.Resource");
}

@Test
public void schemaNamedSchemaDoesNotCollideWithSwagger2Annotation_issue16584() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();
String outputPath = output.getAbsolutePath().replace('\\', '/');

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/bugs/issue_16584.yaml", null, new ParseOptions()).getOpenAPI();

SpringCodegen codegen = new SpringCodegen();
codegen.setOutputDir(output.getAbsolutePath());
codegen.additionalProperties().put(INTERFACE_ONLY, "true");

ClientOptInput input = new ClientOptInput();
input.openAPI(openAPI);
input.config(codegen);

DefaultGenerator generator = new DefaultGenerator();
generator.setGenerateMetadata(false);
generator.opts(input).generate();

// The api must reference the generated model, not the swagger annotation. Importing both
// would be a compile error: two single-type imports for the same simple name.
JavaFileAssert.assertThat(Paths.get(outputPath + "/src/main/java/org/openapitools/api/SchemasApi.java"))
.hasImports("org.openapitools.model.Schema")
.hasNoImports("io.swagger.v3.oas.annotations.media.Schema")
.fileContains("ResponseEntity<Schema> getSchema")
.fileContains("@io.swagger.v3.oas.annotations.media.Schema(implementation = Schema.class)");

// In the model the declared class shadows the annotation, so the annotation is qualified.
JavaFileAssert.assertThat(Paths.get(outputPath + "/src/main/java/org/openapitools/model/Schema.java"))
.hasNoImports("io.swagger.v3.oas.annotations.media.Schema")
.fileContains("@io.swagger.v3.oas.annotations.media.Schema(name = \"id\"");
}

@Test
public void schemaNamedSchemaKeepsExplicitImportMapping_issue16584() {
final SpringCodegen codegen = new SpringCodegen();
codegen.processOpts();
// DefaultGenerator applies --import-mappings after processOpts(), so mirror that order.
codegen.importMapping().put("Schema", "com.example.custom.Schema");

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/bugs/issue_16584.yaml", null, new ParseOptions()).getOpenAPI();
codegen.preprocessOpenAPI(openAPI);

// A user-supplied --import-mappings entry must survive the collision workaround.
Assert.assertEquals(codegen.importMapping().get("Schema"), "com.example.custom.Schema");
}

@Test
public void testImportMappings() {
final SpringCodegen codegen = new SpringCodegen();
Expand Down Expand Up @@ -6764,9 +6814,12 @@ public void annotationLibraryDoesNotCauseImportConflictsInSpringWithAnnotationLi
File apiFile = files.get("Schema.java");
assertNotNull(apiFile);

JavaFileAssert.assertThat(apiFile).fileContains(
"import io.swagger.v3.oas.annotations.media.Schema;"
);
// The spec names a schema "Schema". A single-type import of the annotation cannot coexist
// with the model class of the same name declared in this compilation unit (JLS 7.5.1), so
// the annotation is emitted fully qualified instead of imported. See issue #16584.
JavaFileAssert.assertThat(apiFile)
.fileDoesNotContain("import io.swagger.v3.oas.annotations.media.Schema;")
.fileContains("@io.swagger.v3.oas.annotations.media.Schema(");
}

@Test
Expand Down
35 changes: 35 additions & 0 deletions modules/openapi-generator/src/test/resources/bugs/issue_16584.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
openapi: 3.0.3
info:
title: Schema name collision
description: A schema named "Schema" collides with io.swagger.v3.oas.annotations.media.Schema.
version: 1.0.0
paths:
/schemas/{id}:
get:
operationId: getSchema
tags:
- schemas
parameters:
- name: id
in: path
required: true
schema:
type: string
responses:
'200':
description: Schema found.
content:
application/json:
schema:
$ref: '#/components/schemas/Schema'
components:
schemas:
Schema:
type: object
title: Schema
properties:
id:
type: string
readOnly: true
name:
type: string
Loading