Better support for inline schemas in parameters (#12369)

* better support for inline schema in parameters

* fix parameter model type

* add new method for model

* minor update

* fix isModelWithProperties

* fix is model check

* null check for properties

* inline parameter enhance with python-experimental fix [WIP] (#12397)

* Uses unaliasSchema rather than ModelUtils.getReferencedSchema

* Fixes python-experimental, delays param schema setting

* Samples regenerated

* Adds parameterModelName setting back in

* Samples regenerated

* removes needToSetSchema

* Sets schema differently depending on if inline model resolver is used

* Adds step for getting ref schema

* Samples regen

* fix test

* bug fix for rust generator

Co-authored-by: Justin Black <spacether@users.noreply.github.com>
This commit is contained in:
William Cheng 2022-05-20 20:25:15 +08:00 committed by GitHub
parent 8be490d130
commit c270640a36
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 143 additions and 85 deletions

View File

@ -34,6 +34,7 @@ import org.openapitools.codegen.CodegenDiscriminator.MappedModel;
import org.openapitools.codegen.api.TemplatingEngineAdapter; import org.openapitools.codegen.api.TemplatingEngineAdapter;
import org.openapitools.codegen.config.GlobalSettings; import org.openapitools.codegen.config.GlobalSettings;
import org.openapitools.codegen.examples.ExampleGenerator; import org.openapitools.codegen.examples.ExampleGenerator;
import org.openapitools.codegen.languages.RustServerCodegen;
import org.openapitools.codegen.meta.FeatureSet; import org.openapitools.codegen.meta.FeatureSet;
import org.openapitools.codegen.meta.GeneratorMetadata; import org.openapitools.codegen.meta.GeneratorMetadata;
import org.openapitools.codegen.meta.Stability; import org.openapitools.codegen.meta.Stability;
@ -4608,9 +4609,24 @@ public class DefaultCodegen implements CodegenConfig {
} }
Schema parameterSchema; Schema parameterSchema;
// the parameter model name is obtained from the schema $ref
// e.g. #/components/schemas/list_pageQuery_parameter => toModelName(list_pageQuery_parameter)
String parameterModelName = null;
if (parameter.getSchema() != null) { if (parameter.getSchema() != null) {
parameterSchema = parameter.getSchema(); parameterSchema = parameter.getSchema();
CodegenProperty prop = fromProperty(parameter.getName(), parameterSchema); parameterModelName = getParameterDataType(parameter, parameterSchema);
CodegenProperty prop;
if (this instanceof RustServerCodegen) {
// for rust server, we need to do somethings special as it uses
// $ref (e.g. #components/schemas/Pet) to determine whether it's a model
prop = fromProperty(parameter.getName(), parameterSchema);
} else if (getUseInlineModelResolver()) {
prop = fromProperty(parameter.getName(), ModelUtils.getReferencedSchema(openAPI, parameterSchema));
} else {
prop = fromProperty(parameter.getName(), parameterSchema);
}
codegenParameter.setSchema(prop); codegenParameter.setSchema(prop);
} else if (parameter.getContent() != null) { } else if (parameter.getContent() != null) {
Content content = parameter.getContent(); Content content = parameter.getContent();
@ -4620,6 +4636,7 @@ public class DefaultCodegen implements CodegenConfig {
Map.Entry<String, MediaType> entry = content.entrySet().iterator().next(); Map.Entry<String, MediaType> entry = content.entrySet().iterator().next();
codegenParameter.contentType = entry.getKey(); codegenParameter.contentType = entry.getKey();
parameterSchema = entry.getValue().getSchema(); parameterSchema = entry.getValue().getSchema();
parameterModelName = getParameterDataType(parameter, parameterSchema);
} else { } else {
parameterSchema = null; parameterSchema = null;
} }
@ -4646,11 +4663,17 @@ public class DefaultCodegen implements CodegenConfig {
parameterSchema = unaliasSchema(parameterSchema, Collections.emptyMap()); parameterSchema = unaliasSchema(parameterSchema, Collections.emptyMap());
if (parameterSchema == null) { if (parameterSchema == null) {
LOGGER.warn("warning! Schema not found for parameter \" {} \", using String", parameter.getName()); LOGGER.warn("warning! Schema not found for parameter \" {} \"", parameter.getName());
parameterSchema = new StringSchema().description("//TODO automatically added by openapi-generator due to missing type definition.");
finishUpdatingParameter(codegenParameter, parameter); finishUpdatingParameter(codegenParameter, parameter);
return codegenParameter; return codegenParameter;
} }
if (getUseInlineModelResolver() && !(this instanceof RustServerCodegen)) {
// for rust server, we cannot run the following as it uses
// $ref (e.g. #components/schemas/Pet) to determine whether it's a model
parameterSchema = ModelUtils.getReferencedSchema(openAPI, parameterSchema);
}
ModelUtils.syncValidationProperties(parameterSchema, codegenParameter); ModelUtils.syncValidationProperties(parameterSchema, codegenParameter);
codegenParameter.setTypeProperties(parameterSchema); codegenParameter.setTypeProperties(parameterSchema);
codegenParameter.setComposedSchemas(getComposedSchemas(parameterSchema)); codegenParameter.setComposedSchemas(getComposedSchemas(parameterSchema));
@ -4750,9 +4773,8 @@ public class DefaultCodegen implements CodegenConfig {
//} //}
//codegenProperty.required = true; //codegenProperty.required = true;
String parameterDataType = this.getParameterDataType(parameter, parameterSchema); if (parameterModelName != null) {
if (parameterDataType != null) { codegenParameter.dataType = parameterModelName;
codegenParameter.dataType = parameterDataType;
if (ModelUtils.isObjectSchema(parameterSchema)) { if (ModelUtils.isObjectSchema(parameterSchema)) {
codegenProperty.complexType = codegenParameter.dataType; codegenProperty.complexType = codegenParameter.dataType;
} }
@ -4804,6 +4826,7 @@ public class DefaultCodegen implements CodegenConfig {
// https://swagger.io/docs/specification/serialization/ // https://swagger.io/docs/specification/serialization/
if (schema != null) { if (schema != null) {
Map<String, Schema<?>> properties = schema.getProperties(); Map<String, Schema<?>> properties = schema.getProperties();
if (properties != null) {
codegenParameter.items.vars = codegenParameter.items.vars =
properties.entrySet().stream() properties.entrySet().stream()
.map(entry -> { .map(entry -> {
@ -4811,6 +4834,9 @@ public class DefaultCodegen implements CodegenConfig {
property.baseName = codegenParameter.baseName + "[" + entry.getKey() + "]"; property.baseName = codegenParameter.baseName + "[" + entry.getKey() + "]";
return property; return property;
}).collect(Collectors.toList()); }).collect(Collectors.toList());
} else {
//LOGGER.error("properties is null: {}", schema);
}
} else { } else {
LOGGER.warn( LOGGER.warn(
"No object schema found for deepObject parameter{} deepObject won't have specific properties", "No object schema found for deepObject parameter{} deepObject won't have specific properties",
@ -4823,7 +4849,7 @@ public class DefaultCodegen implements CodegenConfig {
} }
/** /**
* Returns the data type of a parameter. * Returns the data type of parameter.
* Returns null by default to use the CodegenProperty.datatype value * Returns null by default to use the CodegenProperty.datatype value
* *
* @param parameter Parameter * @param parameter Parameter
@ -4831,9 +4857,9 @@ public class DefaultCodegen implements CodegenConfig {
* @return data type * @return data type
*/ */
protected String getParameterDataType(Parameter parameter, Schema schema) { protected String getParameterDataType(Parameter parameter, Schema schema) {
if (parameter.get$ref() != null) { Schema unaliasSchema = ModelUtils.unaliasSchema(openAPI, schema);
String refName = ModelUtils.getSimpleRef(parameter.get$ref()); if (unaliasSchema.get$ref() != null) {
return toModelName(refName); return toModelName(ModelUtils.getSimpleRef(unaliasSchema.get$ref()));
} }
return null; return null;
} }

View File

@ -170,6 +170,9 @@ public interface IJsonSchemaValidationProperties {
default void setTypeProperties(Schema p) { default void setTypeProperties(Schema p) {
if (ModelUtils.isTypeObjectSchema(p)) { if (ModelUtils.isTypeObjectSchema(p)) {
setIsMap(true); setIsMap(true);
if (ModelUtils.isModelWithPropertiesOnly(p)) {
setIsModel(true);
}
} else if (ModelUtils.isArraySchema(p)) { } else if (ModelUtils.isArraySchema(p)) {
setIsArray(true); setIsArray(true);
} else if (ModelUtils.isFileSchema(p) && !ModelUtils.isStringSchema(p)) { } else if (ModelUtils.isFileSchema(p) && !ModelUtils.isStringSchema(p)) {
@ -219,6 +222,9 @@ public interface IJsonSchemaValidationProperties {
setIsNull(true); setIsNull(true);
} else if (ModelUtils.isAnyType(p)) { } else if (ModelUtils.isAnyType(p)) {
setIsAnyType(true); setIsAnyType(true);
if (ModelUtils.isModelWithPropertiesOnly(p)) {
setIsModel(true);
}
} }
} }

View File

@ -408,10 +408,10 @@ public class InlineModelResolver {
/** /**
* Flatten inline models in parameters * Flatten inline models in parameters
* *
* @param pathname target pathname * @param modelName model name
* @param operation target operation * @param operation target operation
*/ */
private void flattenParameters(String pathname, Operation operation) { private void flattenParameters(String modelName, Operation operation) {
List<Parameter> parameters = operation.getParameters(); List<Parameter> parameters = operation.getParameters();
if (parameters == null) { if (parameters == null) {
return; return;
@ -422,39 +422,19 @@ public class InlineModelResolver {
continue; continue;
} }
Schema model = parameter.getSchema(); Schema parameterSchema = parameter.getSchema();
if (model instanceof ObjectSchema) {
Schema obj = model; if (parameterSchema == null) {
if (obj.getType() == null || "object".equals(obj.getType())) { continue;
if (obj.getProperties() != null && obj.getProperties().size() > 0) {
flattenProperties(openAPI, obj.getProperties(), pathname);
String modelName = resolveModelName(obj.getTitle(), parameter.getName());
modelName = addSchemas(modelName, model);
parameter.$ref(modelName);
}
}
} else if (model instanceof ArraySchema) {
ArraySchema am = (ArraySchema) model;
Schema inner = am.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(), parameter.getName());
Schema innerModel = modelFromProperty(openAPI, op, modelName);
String existing = matchGenerated(innerModel);
if (existing != null) {
Schema schema = new Schema().$ref(existing);
schema.setRequired(op.getRequired());
am.setItems(schema);
} else {
modelName = addSchemas(modelName, innerModel);
Schema schema = new Schema().$ref(modelName);
schema.setRequired(op.getRequired());
am.setItems(schema);
}
}
} }
String schemaName = resolveModelName(parameterSchema.getTitle(),
(operation.getOperationId() == null ? modelName : operation.getOperationId()) + "_" + parameter.getName() + "_parameter");
// Recursively gather/make inline models within this schema if any
gatherInlineModels(parameterSchema, schemaName);
if (isModelNeeded(parameterSchema)) {
// If this schema should be split into its own model, do so
Schema refSchema = this.makeSchemaInComponents(schemaName, parameterSchema);
parameter.setSchema(refSchema);
} }
} }
} }
@ -564,31 +544,9 @@ public class InlineModelResolver {
flattenComposedChildren(modelName + "_oneOf", m.getOneOf()); flattenComposedChildren(modelName + "_oneOf", m.getOneOf());
} else if (model instanceof Schema) { } else if (model instanceof Schema) {
gatherInlineModels(model, modelName); gatherInlineModels(model, modelName);
} /*else if (ModelUtils.isArraySchema(model)) {
ArraySchema m = (ArraySchema) model;
Schema inner = m.getItems();
if (inner instanceof ObjectSchema) {
ObjectSchema op = (ObjectSchema) inner;
if (op.getProperties() != null && op.getProperties().size() > 0) {
String innerModelName = resolveModelName(op.getTitle(), modelName + "_inner");
Schema innerModel = modelFromProperty(openAPI, op, innerModelName);
String existing = matchGenerated(innerModel);
if (existing == null) {
openAPI.getComponents().addSchemas(innerModelName, innerModel);
addGenerated(innerModelName, innerModel);
Schema schema = new Schema().$ref(innerModelName);
schema.setRequired(op.getRequired());
m.setItems(schema);
} else {
Schema schema = new Schema().$ref(existing);
schema.setRequired(op.getRequired());
m.setItems(schema);
} }
} }
} }
}*/
}
}
/** /**
* This function fix models that are string (mostly enum). Before this fix, the * This function fix models that are string (mostly enum). Before this fix, the

View File

@ -26,6 +26,7 @@ import io.swagger.v3.oas.models.media.ComposedSchema;
import io.swagger.v3.oas.models.media.FileSchema; import io.swagger.v3.oas.models.media.FileSchema;
import io.swagger.v3.oas.models.media.Schema; import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.media.XML; import io.swagger.v3.oas.models.media.XML;
import io.swagger.v3.oas.models.parameters.Parameter;
import io.swagger.v3.oas.models.parameters.RequestBody; import io.swagger.v3.oas.models.parameters.RequestBody;
import io.swagger.v3.oas.models.responses.ApiResponse; import io.swagger.v3.oas.models.responses.ApiResponse;
import io.swagger.v3.oas.models.servers.Server; import io.swagger.v3.oas.models.servers.Server;
@ -1741,4 +1742,13 @@ public class RustServerCodegen extends DefaultCodegen implements CodegenConfig {
@Override @Override
public GeneratorLanguage generatorLanguage() { return GeneratorLanguage.RUST; } public GeneratorLanguage generatorLanguage() { return GeneratorLanguage.RUST; }
@Override
protected String getParameterDataType(Parameter parameter, Schema schema) {
if (parameter.get$ref() != null) {
String refName = ModelUtils.getSimpleRef(parameter.get$ref());
return toModelName(refName);
}
return null;
}
} }

View File

@ -725,7 +725,7 @@ public class ModelUtils {
} }
// has properties // has properties
if (null != schema.getProperties()) { if (null != schema.getProperties() && !schema.getProperties().isEmpty()) {
return true; return true;
} }
@ -733,6 +733,25 @@ public class ModelUtils {
return schema instanceof ComposedSchema || schema instanceof ObjectSchema; return schema instanceof ComposedSchema || schema instanceof ObjectSchema;
} }
/**
* Check to see if the schema is a model with properties only (non-composed model)
*
* @param schema potentially containing a '$ref'
* @return true if it's a model with at least one properties
*/
public static boolean isModelWithPropertiesOnly(Schema schema) {
if (schema == null) {
return false;
}
if (null != schema.getProperties() && !schema.getProperties().isEmpty() && // has properties
(schema.getAdditionalProperties() == null || // no additionalProperties is set
(schema.getAdditionalProperties() instanceof Boolean && !(Boolean)schema.getAdditionalProperties()))) {
return true;
}
return false;
}
public static boolean hasValidation(Schema sc) { public static boolean hasValidation(Schema sc) {
return ( return (
sc.getMaxItems() != null || sc.getMaxItems() != null ||

View File

@ -2061,11 +2061,12 @@ public class DefaultCodegenTest {
CodegenParameter parameter = codegen.fromParameter(openAPI.getPaths().get("/pony").getGet().getParameters().get(0), imports); CodegenParameter parameter = codegen.fromParameter(openAPI.getPaths().get("/pony").getGet().getParameters().get(0), imports);
// TODO: This must be updated to work with flattened inline models // TODO: This must be updated to work with flattened inline models
Assert.assertEquals(parameter.dataType, "PageQuery1"); Assert.assertEquals(parameter.dataType, "ListPageQueryParameter");
Assert.assertEquals(imports.size(), 1); Assert.assertEquals(imports.size(), 1);
Assert.assertEquals(imports.iterator().next(), "PageQuery1"); Assert.assertEquals(imports.iterator().next(), "ListPageQueryParameter");
Assert.assertNotNull(parameter.getSchema()); Assert.assertNotNull(parameter.getSchema());
Assert.assertEquals(parameter.getSchema().dataType, "Object");
Assert.assertEquals(parameter.getSchema().baseType, "object"); Assert.assertEquals(parameter.getSchema().baseType, "object");
} }
@ -3167,8 +3168,8 @@ public class DefaultCodegenTest {
// CodegenOperation puts the inline schema into schemas and refs it // CodegenOperation puts the inline schema into schemas and refs it
assertTrue(co.responses.get(0).isModel); assertTrue(co.responses.get(0).isModel);
assertEquals(co.responses.get(0).baseType, "objectData"); assertEquals(co.responses.get(0).baseType, "objectWithOptionalAndRequiredProps_request");
modelName = "objectData"; modelName = "objectWithOptionalAndRequiredProps_request";
sc = openAPI.getComponents().getSchemas().get(modelName); sc = openAPI.getComponents().getSchemas().get(modelName);
cm = codegen.fromModel(modelName, sc); cm = codegen.fromModel(modelName, sc);
assertEquals(cm.vars, vars); assertEquals(cm.vars, vars);
@ -3180,7 +3181,7 @@ public class DefaultCodegenTest {
cm = codegen.fromModel(modelName, sc); cm = codegen.fromModel(modelName, sc);
CodegenProperty cp = cm.getVars().get(0); CodegenProperty cp = cm.getVars().get(0);
assertTrue(cp.isModel); assertTrue(cp.isModel);
assertEquals(cp.complexType, "objectData"); assertEquals(cp.complexType, "objectWithOptionalAndRequiredProps_request");
} }
@Test @Test
@ -4005,7 +4006,9 @@ public class DefaultCodegenTest {
CodegenMediaType mt = content.get("application/json"); CodegenMediaType mt = content.get("application/json");
assertNull(mt.getEncoding()); assertNull(mt.getEncoding());
CodegenProperty cp = mt.getSchema(); CodegenProperty cp = mt.getSchema();
// TODO need to revise the test below
assertTrue(cp.isMap); assertTrue(cp.isMap);
assertTrue(cp.isModel);
assertEquals(cp.complexType, "object"); assertEquals(cp.complexType, "object");
assertEquals(cp.baseName, "SchemaForRequestParameterCoordinatesInlineSchemaApplicationJson"); assertEquals(cp.baseName, "SchemaForRequestParameterCoordinatesInlineSchemaApplicationJson");

View File

@ -37,9 +37,9 @@ export interface FakeEnumRequestGetInlineRequest {
} }
export interface FakeEnumRequestGetRefRequest { export interface FakeEnumRequestGetRefRequest {
stringEnum?: StringEnum; stringEnum?: FakeEnumRequestGetRefStringEnumEnum;
nullableStringEnum?: StringEnum | null; nullableStringEnum?: StringEnum | null;
numberEnum?: NumberEnum; numberEnum?: FakeEnumRequestGetRefNumberEnumEnum;
nullableNumberEnum?: NumberEnum | null; nullableNumberEnum?: NumberEnum | null;
} }
@ -210,3 +210,21 @@ export const FakeEnumRequestGetInlineNumberEnumEnum = {
NUMBER_3: 3 NUMBER_3: 3
} as const; } as const;
export type FakeEnumRequestGetInlineNumberEnumEnum = typeof FakeEnumRequestGetInlineNumberEnumEnum[keyof typeof FakeEnumRequestGetInlineNumberEnumEnum]; export type FakeEnumRequestGetInlineNumberEnumEnum = typeof FakeEnumRequestGetInlineNumberEnumEnum[keyof typeof FakeEnumRequestGetInlineNumberEnumEnum];
/**
* @export
*/
export const FakeEnumRequestGetRefStringEnumEnum = {
One: 'one',
Two: 'two',
Three: 'three'
} as const;
export type FakeEnumRequestGetRefStringEnumEnum = typeof FakeEnumRequestGetRefStringEnumEnum[keyof typeof FakeEnumRequestGetRefStringEnumEnum];
/**
* @export
*/
export const FakeEnumRequestGetRefNumberEnumEnum = {
NUMBER_1: 1,
NUMBER_2: 2,
NUMBER_3: 3
} as const;
export type FakeEnumRequestGetRefNumberEnumEnum = typeof FakeEnumRequestGetRefNumberEnumEnum[keyof typeof FakeEnumRequestGetRefNumberEnumEnum];

View File

@ -37,9 +37,9 @@ export interface FakeEnumRequestGetInlineRequest {
} }
export interface FakeEnumRequestGetRefRequest { export interface FakeEnumRequestGetRefRequest {
stringEnum?: StringEnum; stringEnum?: FakeEnumRequestGetRefStringEnumEnum;
nullableStringEnum?: StringEnum | null; nullableStringEnum?: StringEnum | null;
numberEnum?: NumberEnum; numberEnum?: FakeEnumRequestGetRefNumberEnumEnum;
nullableNumberEnum?: NumberEnum | null; nullableNumberEnum?: NumberEnum | null;
} }
@ -210,3 +210,21 @@ export enum FakeEnumRequestGetInlineNumberEnumEnum {
NUMBER_2 = 2, NUMBER_2 = 2,
NUMBER_3 = 3 NUMBER_3 = 3
} }
/**
* @export
* @enum {string}
*/
export enum FakeEnumRequestGetRefStringEnumEnum {
One = 'one',
Two = 'two',
Three = 'three'
}
/**
* @export
* @enum {string}
*/
export enum FakeEnumRequestGetRefNumberEnumEnum {
NUMBER_1 = 1,
NUMBER_2 = 2,
NUMBER_3 = 3
}