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.

Sign up for GitHub

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

Better importMapping handling when aliasing references #4350

Merged
merged 9 commits into from
Feb 13, 2020
Merged

Better importMapping handling when aliasing references #4350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d9c874f
type aliasing issue
bkoziak Aug 8, 2019
626540f
Add example OpenAPI document from issue 3589
pe-st Oct 11, 2019
d8e27dd
Add test to reproduce the issue
pe-st Oct 17, 2019
6ac436b
Merge 'bkoziak/import-mapping-with-type-aliases-issue-3589'
pe-st Nov 1, 2019
47da69c
Remove duplicate test file
pe-st Nov 1, 2019
df9fd22
Update branch from upstream master
pe-st Nov 1, 2019
5e80a64
Merge https://github.com/openapitools/openapi-generator into java-imp…
wing328 Feb 2, 2020
128f9c6
Add new method override handleMethodResponse
pe-st Feb 9, 2020
4375a33
Merge branch 'master'
pe-st Feb 9, 2020
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
3 changes: 2 additions & 1 deletion ...api-generator-maven-plugin/src/main/java/org/openapitools/codegen/plugin/CodeGenMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ public void execute() throws MojoExecutionException {
configurator);
}

// Retained for backwards-compataibility with configOptions -> import-mappings
// Retained for backwards-compatibility with configOptions -> import-mappings
if (importMappings == null && configOptions.containsKey("import-mappings")) {
applyImportMappingsKvp(configOptions.get("import-mappings").toString(),
configurator);
Expand Down Expand Up @@ -869,6 +869,7 @@ private void addCompileSourceRootIfConfigured() {
}
}
}

/**
* This method enables conversion of true/false strings in
* config.additionalProperties (configuration/configOptions) to proper booleans.
Expand Down
58 changes: 41 additions & 17 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1671,12 +1671,15 @@ public String toOneOfName(List<String> names, ComposedSchema composedSchema) {
* @return the string representation of the schema type.
*/
private String getSingleSchemaType(Schema schema) {
Schema unaliasSchema = ModelUtils.unaliasSchema(this.openAPI, schema);
Schema unaliasSchema = ModelUtils.unaliasSchema(this.openAPI, schema, importMapping);

if (StringUtils.isNotBlank(unaliasSchema.get$ref())) { // reference to another definition/schema
// get the schema/model name from $ref
String schemaName = ModelUtils.getSimpleRef(unaliasSchema.get$ref());
if (StringUtils.isNotEmpty(schemaName)) {
if (importMapping.containsKey(schemaName)) {
return schemaName;
}
return getAlias(schemaName);
} else {
LOGGER.warn("Error obtaining the datatype from ref:" + unaliasSchema.get$ref() + ". Default to 'object'");
Expand Down Expand Up @@ -1769,7 +1772,6 @@ public String lowerCamelCase(String name) {
return (name.length() > 0) ? (Character.toLowerCase(name.charAt(0)) + name.substring(1)) : "";
}


/**
* Output the type declaration of a given name
*
Expand Down Expand Up @@ -1887,7 +1889,7 @@ public CodegenModel fromModel(String name, Schema schema) {
}

// unalias schema
schema = ModelUtils.unaliasSchema(this.openAPI, schema);
schema = ModelUtils.unaliasSchema(this.openAPI, schema, importMapping);
if (schema == null) {
LOGGER.warn("Schema {} not found", name);
return null;
Expand Down Expand Up @@ -2238,7 +2240,7 @@ public CodegenProperty fromProperty(String name, Schema p) {
LOGGER.debug("debugging fromProperty for " + name + " : " + p);

// unalias schema
p = ModelUtils.unaliasSchema(this.openAPI, p);
p = ModelUtils.unaliasSchema(this.openAPI, p, importMapping);

CodegenProperty property = CodegenModelFactory.newInstance(CodegenModelType.PROPERTY);

Expand Down Expand Up @@ -2392,12 +2394,13 @@ public CodegenProperty fromProperty(String name, Schema p) {
} else if (ModelUtils.isArraySchema(p)) {
// default to string if inner item is undefined
ArraySchema arraySchema = (ArraySchema) p;
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, getSchemaItems(arraySchema));
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, getSchemaItems(arraySchema), importMapping);
if (arraySchema.getItems() == null) {
arraySchema.setItems(innerSchema);
}
} else if (ModelUtils.isMapSchema(p)) {
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getAdditionalProperties(p));
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getAdditionalProperties(p),
importMapping);
if (innerSchema == null) {
LOGGER.error("Undefined map inner type for `{}`. Default to String.", p.getName());
innerSchema = new StringSchema().description("//TODO automatically added by openapi-generator due to undefined type");
Expand Down Expand Up @@ -2473,7 +2476,7 @@ public CodegenProperty fromProperty(String name, Schema p) {
itemName = property.name;
}
ArraySchema arraySchema = (ArraySchema) p;
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, getSchemaItems(arraySchema));
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, getSchemaItems(arraySchema), importMapping);
if (arraySchema.getItems() == null) {
arraySchema.setItems(innerSchema);
}
Expand All @@ -2488,7 +2491,8 @@ public CodegenProperty fromProperty(String name, Schema p) {
property.maxItems = p.getMaxProperties();

// handle inner property
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getAdditionalProperties(p));
Schema innerSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getAdditionalProperties(p),
importMapping);
if (innerSchema == null) {
LOGGER.error("Undefined map inner type for `{}`. Default to String.", p.getName());
innerSchema = new StringSchema().description("//TODO automatically added by openapi-generator due to undefined type");
Expand Down Expand Up @@ -2707,7 +2711,24 @@ protected void handleMethodResponse(Operation operation,
Map<String, Schema> schemas,
CodegenOperation op,
ApiResponse methodResponse) {
Schema responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(methodResponse));
handleMethodResponse(operation, schemas, op, methodResponse, Collections.<String, String>emptyMap());
}

/**
* Set op's returnBaseType, returnType, examples etc.
*
* @param operation endpoint Operation
* @param schemas a map of the schemas in the openapi spec
* @param op endpoint CodegenOperation
* @param methodResponse the default ApiResponse for the endpoint
* @param importMappings mappings of external types to be omitted by unaliasing
*/
protected void handleMethodResponse(Operation operation,
Map<String, Schema> schemas,
CodegenOperation op,
ApiResponse methodResponse,
Map<String, String> importMappings) {
Schema responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(methodResponse), importMappings);

if (responseSchema != null) {
CodegenProperty cm = fromProperty("response", responseSchema);
Expand Down Expand Up @@ -2862,7 +2883,7 @@ public CodegenOperation fromOperation(String path,
op.responses.get(op.responses.size() - 1).hasMore = false;

if (methodResponse != null) {
handleMethodResponse(operation, schemas, op, methodResponse);
handleMethodResponse(operation, schemas, op, methodResponse, importMapping);
}
}

Expand Down Expand Up @@ -2996,9 +3017,12 @@ public CodegenOperation fromOperation(String path,
Collections.sort(allParams, new Comparator<CodegenParameter>() {
@Override
public int compare(CodegenParameter one, CodegenParameter another) {
if (one.required == another.required) return 0;
else if (one.required) return -1;
else return 1;
if (one.required == another.required)
return 0;
else if (one.required)
return -1;
else
return 1;
}
});
}
Expand Down Expand Up @@ -3077,7 +3101,8 @@ public CodegenResponse fromResponse(String responseCode, ApiResponse response) {
}
Schema responseSchema;
if (this.openAPI != null && this.openAPI.getComponents() != null) {
responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(response));
responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(response),
importMapping);
} else { // no model/alias defined
responseSchema = ModelUtils.getSchemaFromResponse(response);
}
Expand Down Expand Up @@ -3309,7 +3334,7 @@ public CodegenParameter fromParameter(Parameter parameter, Set<String> imports)
}

if (s != null) {
Schema parameterSchema = ModelUtils.unaliasSchema(this.openAPI, s);
Schema parameterSchema = ModelUtils.unaliasSchema(this.openAPI, s, importMapping);
if (parameterSchema == null) {
LOGGER.warn("warning! Schema not found for parameter \"" + parameter.getName() + "\", using String");
parameterSchema = new StringSchema().description("//TODO automatically added by openapi-generator due to missing type definition.");
Expand Down Expand Up @@ -3913,7 +3938,7 @@ protected void addImport(CodegenModel m, String type) {
private Map<String, Schema> unaliasPropertySchema(Map<String, Schema> properties) {
if (properties != null) {
for (String key : properties.keySet()) {
properties.put(key, ModelUtils.unaliasSchema(this.openAPI, properties.get(key)));
properties.put(key, ModelUtils.unaliasSchema(this.openAPI, properties.get(key), importMapping()));

}
}
Expand Down Expand Up @@ -5288,7 +5313,6 @@ public CodegenParameter fromRequestBody(RequestBody body, Set<String> imports, S
codegenParameter.maxLength = codegenProperty.maxLength;
codegenParameter.pattern = codegenProperty.pattern;


if (codegenProperty.complexType != null) {
imports.add(codegenProperty.complexType);
}
Expand Down
2 changes: 1 addition & 1 deletion ...i-generator/src/main/java/org/openapitools/codegen/languages/CppRestSdkClientCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation

if (methodResponse != null) {
Schema response = ModelUtils.getSchemaFromResponse(methodResponse);
response = ModelUtils.unaliasSchema(this.openAPI, response);
response = ModelUtils.unaliasSchema(this.openAPI, response, importMapping);
if (response != null) {
CodegenProperty cm = fromProperty("response", response);
op.vendorExtensions.put("x-codegen-response", cm);
Expand Down
22 changes: 20 additions & 2 deletions ...tor/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ public CodegenResponse fromResponse(String responseCode, ApiResponse response) {
// When we serialize/deserialize ModelSimple models, validations and enums will be checked.
Schema responseSchema;
if (this.openAPI != null && this.openAPI.getComponents() != null) {
responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(response));
responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(response), importMapping);
} else { // no model/alias defined
responseSchema = ModelUtils.getSchemaFromResponse(response);
}
Expand Down Expand Up @@ -582,12 +582,30 @@ public void handleMethodResponse(Operation operation,
Map<String, Schema> schemas,
CodegenOperation op,
ApiResponse methodResponse) {
handleMethodResponse(operation, schemas, op, methodResponse, Collections.<String, String>emptyMap());
}

/**
* Set op's returnBaseType, returnType, examples etc.
*
* @param operation endpoint Operation
* @param schemas a map of the schemas in the openapi spec
* @param op endpoint CodegenOperation
* @param methodResponse the default ApiResponse for the endpoint
* @param importMappings mappings of external types to be omitted by unaliasing
*/
@Override
protected void handleMethodResponse(Operation operation,
Map<String, Schema> schemas,
CodegenOperation op,
ApiResponse methodResponse,
Map<String, String> importMappings) {
// we have a custom version of this method to handle endpoints that return models where
// type != object the model has validations and/or enums
// we do this by invoking our custom fromResponse method to create defaultResponse
// which we then use to set op.returnType and op.returnBaseType
CodegenResponse defaultResponse = fromResponse("defaultResponse", methodResponse);
Schema responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(methodResponse));
Schema responseSchema = ModelUtils.unaliasSchema(this.openAPI, ModelUtils.getSchemaFromResponse(methodResponse), importMappings);

if (responseSchema != null) {
op.returnBaseType = defaultResponse.baseType;
Expand Down
11 changes: 10 additions & 1 deletion ...nerator/src/main/java/org/openapitools/codegen/languages/TypeScriptNodeClientCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,16 @@ public String getTypeDeclaration(Schema p) {
@Override
protected void handleMethodResponse(Operation operation, Map<String, Schema> schemas, CodegenOperation op,
ApiResponse methodResponse) {
super.handleMethodResponse(operation, schemas, op, methodResponse);
handleMethodResponse(operation, schemas, op, methodResponse, Collections.<String, String>emptyMap());
}

@Override
protected void handleMethodResponse(Operation operation,
Map<String, Schema> schemas,
CodegenOperation op,
ApiResponse methodResponse,
Map<String, String> importMappings) {
super.handleMethodResponse(operation, schemas, op, methodResponse, importMappings);

// see comment in getTypeDeclaration
if (op.isResponseFile) {
Expand Down
38 changes: 31 additions & 7 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class ModelUtils {
private static final String URI_FORMAT = "uri";

private static final String generateAliasAsModelKey = "generateAliasAsModel";

public static void setGenerateAliasAsModel(boolean value) {
GlobalSettings.setProperty(generateAliasAsModelKey, Boolean.toString(value));
}
Expand Down Expand Up @@ -240,7 +241,7 @@ private static void visitPathItem(PathItem pathItem, OpenAPI openAPI, OpenAPISch
}

private static void visitParameters(OpenAPI openAPI, List<Parameter> parameters, OpenAPISchemaVisitor visitor,
List<String> visitedSchemas) {
List<String> visitedSchemas) {
if (parameters != null) {
for (Parameter p : parameters) {
Parameter parameter = getReferencedParameter(openAPI, p);
Expand Down Expand Up @@ -830,7 +831,22 @@ private static Schema getSchemaFromContent(Content content) {
* @param schema schema (alias or direct reference)
* @return actual schema
*/
public static Schema unaliasSchema(OpenAPI openAPI, Schema schema) {
public static Schema unaliasSchema(OpenAPI openAPI,
Schema schema) {
return unaliasSchema(openAPI, schema, Collections.<String, String>emptyMap());
}

/**
* Get the actual schema from aliases. If the provided schema is not an alias, the schema itself will be returned.
*
* @param openAPI specification being checked
* @param schema schema (alias or direct reference)
* @param importMappings mappings of external types to be omitted by unaliasing
* @return actual schema
*/
public static Schema unaliasSchema(OpenAPI openAPI,
Schema schema,
Map<String, String> importMappings) {
Map<String, Schema> allSchemas = getSchemas(openAPI);
if (allSchemas == null || allSchemas.isEmpty()) {
// skip the warning as the spec can have no model defined
Expand All @@ -839,7 +855,12 @@ public static Schema unaliasSchema(OpenAPI openAPI, Schema schema) {
}

if (schema != null && StringUtils.isNotEmpty(schema.get$ref())) {
Schema ref = allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref()));
String simpleRef = ModelUtils.getSimpleRef(schema.get$ref());
if (importMappings.containsKey(simpleRef)) {
LOGGER.info("Schema unaliasing of {} omitted because aliased class is to be mapped to {}", simpleRef, importMappings.get(simpleRef));
return schema;
}
Schema ref = allSchemas.get(simpleRef);
if (ref == null) {
once(LOGGER).warn("{} is not defined", schema.get$ref());
return schema;
Expand All @@ -850,7 +871,8 @@ public static Schema unaliasSchema(OpenAPI openAPI, Schema schema) {
if (isGenerateAliasAsModel()) {
return schema; // generate a model extending array
} else {
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())));
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())),
importMappings);
}
} else if (isComposedSchema(ref)) {
return schema;
Expand All @@ -862,17 +884,19 @@ public static Schema unaliasSchema(OpenAPI openAPI, Schema schema) {
return schema; // generate a model extending map
} else {
// treat it as a typical map
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())));
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())),
importMappings);
}
}
} else if (isObjectSchema(ref)) { // model
if (ref.getProperties() != null && !ref.getProperties().isEmpty()) { // has at least one property
return schema;
} else { // free form object (type: object)
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())));
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())),
importMappings);
}
} else {
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())));
return unaliasSchema(openAPI, allSchemas.get(ModelUtils.getSimpleRef(schema.get$ref())), importMappings);
}
}
return schema;
Expand Down
15 changes: 15 additions & 0 deletions modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,21 @@ public void modelDoNotContainInheritedVars() {
Assert.assertEquals(codegenModel.vars.size(), 1);
}

@Test
public void importMapping() {
DefaultCodegen codegen = new DefaultCodegen();
codegen.importMapping.put("TypeAlias", "foo.bar.TypeAlias");

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/3_0/type-alias.yaml", null, new ParseOptions()).getOpenAPI();
codegen.setOpenAPI(openAPI);

CodegenModel codegenModel = codegen.fromModel("ParentType", openAPI.getComponents().getSchemas().get("ParentType"));

Assert.assertEquals(codegenModel.vars.size(), 1);
Assert.assertEquals(codegenModel.vars.get(0).getBaseType(), "TypeAlias");
}

@Test
public void modelWithPrefixDoNotContainInheritedVars() {
DefaultCodegen codegen = new DefaultCodegen();
Expand Down
Loading