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

[Core/Rust Server] Check references in additionalProperties correctly when checking freeForm status #19605

Merged
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 @@ -2424,7 +2424,7 @@ private String getPrimitiveType(Schema schema) {
return schema.getFormat();
}
return "string";
} else if (ModelUtils.isFreeFormObject(schema)) {
} else if (ModelUtils.isFreeFormObject(schema, openAPI)) {
// Note: the value of a free-form object cannot be an arbitrary type. Per OAS specification,
// it must be a map of string to values.
return "object";
Expand Down Expand Up @@ -2833,7 +2833,7 @@ protected void updateModelForObject(CodegenModel m, Schema schema) {
if (ModelUtils.isMapSchema(schema)) {
// an object or anyType composed schema that has additionalProperties set
addAdditionPropertiesToCodeGenModel(m, schema);
} else if (ModelUtils.isFreeFormObject(schema)) {
} else if (ModelUtils.isFreeFormObject(schema, openAPI)) {
// non-composed object type with no properties + additionalProperties
// additionalProperties must be null, ObjectSchema, or empty Schema
addAdditionPropertiesToCodeGenModel(m, schema);
Expand Down Expand Up @@ -3039,7 +3039,7 @@ public CodegenModel fromModel(String name, Schema schema) {
m.isNullable = Boolean.TRUE;
}

m.setTypeProperties(schema);
m.setTypeProperties(schema, openAPI);
m.setFormat(schema.getFormat());
m.setComposedSchemas(getComposedSchemas(schema));
if (ModelUtils.isArraySchema(schema)) {
Expand Down Expand Up @@ -3698,7 +3698,7 @@ protected void updatePropertyForMap(CodegenProperty property, Schema p) {
}

protected void updatePropertyForObject(CodegenProperty property, Schema p) {
if (ModelUtils.isFreeFormObject(p)) {
if (ModelUtils.isFreeFormObject(p, openAPI)) {
// non-composed object type with no properties + additionalProperties
// additionalProperties must be null, ObjectSchema, or empty Schema
property.isFreeFormObject = true;
Expand Down Expand Up @@ -4022,7 +4022,7 @@ public CodegenProperty fromProperty(String name, Schema p, boolean required, boo
property.datatypeWithEnum = property.dataType;
}

property.setTypeProperties(p);
property.setTypeProperties(p, openAPI);
property.setComposedSchemas(getComposedSchemas(p));
if (ModelUtils.isIntegerSchema(p)) { // integer type
updatePropertyForInteger(property, p);
Expand Down Expand Up @@ -4068,7 +4068,7 @@ public CodegenProperty fromProperty(String name, Schema p, boolean required, boo
!ModelUtils.isComposedSchema(p) &&
p.getAdditionalProperties() == null && p.getNot() == null && p.getEnum() == null);

if (!ModelUtils.isArraySchema(p) && !ModelUtils.isMapSchema(p) && !ModelUtils.isFreeFormObject(p) && !isAnyTypeWithNothingElseSet) {
if (!ModelUtils.isArraySchema(p) && !ModelUtils.isMapSchema(p) && !ModelUtils.isFreeFormObject(p, openAPI) && !isAnyTypeWithNothingElseSet) {
/* schemas that are not Array, not ModelUtils.isMapSchema, not isFreeFormObject, not AnyType with nothing else set
* so primitive schemas int, str, number, referenced schemas, AnyType schemas with properties, enums, or composition
*/
Expand Down Expand Up @@ -4865,7 +4865,7 @@ public CodegenResponse fromResponse(String responseCode, ApiResponse response) {
}
}

r.setTypeProperties(responseSchema);
r.setTypeProperties(responseSchema, openAPI);
r.setComposedSchemas(getComposedSchemas(responseSchema));
if (ModelUtils.isArraySchema(responseSchema)) {
r.simpleType = false;
Expand Down Expand Up @@ -4925,7 +4925,7 @@ public CodegenResponse fromResponse(String responseCode, ApiResponse response) {
r.isDouble = Boolean.TRUE;
}
} else if (ModelUtils.isTypeObjectSchema(responseSchema)) {
if (ModelUtils.isFreeFormObject(responseSchema)) {
if (ModelUtils.isFreeFormObject(responseSchema, openAPI)) {
r.isFreeFormObject = true;
} else {
r.isModel = true;
Expand Down Expand Up @@ -5189,7 +5189,7 @@ public CodegenParameter fromParameter(Parameter parameter, Set<String> imports)
}

ModelUtils.syncValidationProperties(parameterSchema, codegenParameter);
codegenParameter.setTypeProperties(parameterSchema);
codegenParameter.setTypeProperties(parameterSchema, openAPI);
codegenParameter.setComposedSchemas(getComposedSchemas(parameterSchema));

if (Boolean.TRUE.equals(parameterSchema.getNullable())) { // use nullable defined in the spec
Expand Down Expand Up @@ -5239,7 +5239,7 @@ public CodegenParameter fromParameter(Parameter parameter, Set<String> imports)
if (ModelUtils.isMapSchema(parameterSchema)) { // for map parameter
updateParameterForMap(codegenParameter, parameterSchema, imports);
}
if (ModelUtils.isFreeFormObject(parameterSchema)) {
if (ModelUtils.isFreeFormObject(parameterSchema, openAPI)) {
codegenParameter.isFreeFormObject = true;
}
addVarsRequiredVarsAdditionalProps(parameterSchema, codegenParameter);
Expand Down Expand Up @@ -7115,7 +7115,7 @@ public CodegenParameter fromFormProperty(String name, Schema propertySchema, Set

Schema ps = unaliasSchema(propertySchema);
ModelUtils.syncValidationProperties(ps, codegenParameter);
codegenParameter.setTypeProperties(ps);
codegenParameter.setTypeProperties(ps, openAPI);
codegenParameter.setComposedSchemas(getComposedSchemas(ps));
if (ps.getPattern() != null) {
codegenParameter.pattern = toRegularExpression(ps.getPattern());
Expand Down Expand Up @@ -7206,7 +7206,7 @@ public CodegenParameter fromFormProperty(String name, Schema propertySchema, Set
codegenParameter.isPrimitiveType = false;
codegenParameter.items = codegenProperty.items;
codegenParameter.mostInnerItems = codegenProperty.mostInnerItems;
} else if (ModelUtils.isFreeFormObject(ps)) {
} else if (ModelUtils.isFreeFormObject(ps, openAPI)) {
codegenParameter.isFreeFormObject = true;
}
} else if (ModelUtils.isNullType(ps)) {
Expand Down Expand Up @@ -7382,7 +7382,7 @@ protected void updateRequestBodyForMap(CodegenParameter codegenParameter, Schema
if (StringUtils.isBlank(name)) {
useModel = false;
} else {
if (ModelUtils.isFreeFormObject(schema)) {
if (ModelUtils.isFreeFormObject(schema, openAPI)) {
useModel = ModelUtils.shouldGenerateFreeFormObjectModel(name, this);
} else if (ModelUtils.isMapSchema(schema)) {
useModel = ModelUtils.shouldGenerateMapModel(schema);
Expand Down Expand Up @@ -7461,7 +7461,7 @@ protected void updateRequestBodyForObject(CodegenParameter codegenParameter, Sch
if (ModelUtils.isMapSchema(schema)) {
// Schema with additionalproperties: true (including composed schemas with additionalproperties: true)
updateRequestBodyForMap(codegenParameter, schema, name, imports, bodyParameterName);
} else if (ModelUtils.isFreeFormObject(schema)) {
} else if (ModelUtils.isFreeFormObject(schema, openAPI)) {
// non-composed object type with no properties + additionalProperties
// additionalProperties must be null, ObjectSchema, or empty Schema
codegenParameter.isFreeFormObject = true;
Expand Down Expand Up @@ -7724,7 +7724,7 @@ public CodegenParameter fromRequestBody(RequestBody body, Set<String> imports, S
schema = ModelUtils.getReferencedSchema(this.openAPI, schema);

ModelUtils.syncValidationProperties(unaliasedSchema, codegenParameter);
codegenParameter.setTypeProperties(unaliasedSchema);
codegenParameter.setTypeProperties(unaliasedSchema, openAPI);
codegenParameter.setComposedSchemas(getComposedSchemas(unaliasedSchema));
// TODO in the future switch al the below schema usages to unaliasedSchema
// because it keeps models as refs and will not get their referenced schemas
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ void generateModels(List<File> files, List<ModelMap> allModels, List<String> unu
if (schema.getExtensions() != null && Boolean.TRUE.equals(schema.getExtensions().get("x-internal"))) {
LOGGER.info("Model {} not generated since x-internal is set to true", name);
continue;
} else if (ModelUtils.isFreeFormObject(schema)) { // check to see if it's a free-form object
} else if (ModelUtils.isFreeFormObject(schema, openAPI)) { // check to see if it's a free-form object
if (!ModelUtils.shouldGenerateFreeFormObjectModel(name, config)) {
LOGGER.info("Model {} not generated since it's a free-form object", name);
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.openapitools.codegen.meta.features.SchemaSupportFeature;
import org.openapitools.codegen.utils.ModelUtils;

import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.media.Schema;

public interface IJsonSchemaValidationProperties {
Expand Down Expand Up @@ -283,7 +284,7 @@ public interface IJsonSchemaValidationProperties {
*
* @param p the schema which contains the type info
*/
default void setTypeProperties(Schema p) {
default void setTypeProperties(Schema p, OpenAPI openAPI) {
if (ModelUtils.isModelWithPropertiesOnly(p)) {
setIsModel(true);
} else if (ModelUtils.isArraySchema(p)) {
Expand Down Expand Up @@ -336,7 +337,7 @@ default void setTypeProperties(Schema p) {
setIsNull(true);
} else if (ModelUtils.isAnyType(p)) {
setIsAnyType(true);
} else if (ModelUtils.isFreeFormObject(p)) {
} else if (ModelUtils.isFreeFormObject(p, openAPI)) {
setIsFreeFormObject(true);
// TODO: remove below later after updating generators to properly use isFreeFormObject
setIsMap(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1117,7 +1117,7 @@ protected void updateModelForObject(CodegenModel m, Schema schema) {
addAdditionPropertiesToCodeGenModel(m, schema);
} else {
m.setIsMap(false);
if (ModelUtils.isFreeFormObject(schema)) {
if (ModelUtils.isFreeFormObject(schema, openAPI)) {
// non-composed object type with no properties + additionalProperties
// additionalProperties must be null, ObjectSchema, or empty Schema
addAdditionPropertiesToCodeGenModel(m, schema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1702,7 +1702,7 @@ protected void updateModelForObject(CodegenModel m, Schema schema) {
addAdditionPropertiesToCodeGenModel(m, schema);
} else {
m.setIsMap(false);
if (ModelUtils.isFreeFormObject(schema)) {
if (ModelUtils.isFreeFormObject(schema, openAPI)) {
// non-composed object type with no properties + additionalProperties
// additionalProperties must be null, ObjectSchema, or empty Schema
addAdditionPropertiesToCodeGenModel(m, schema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1123,7 +1123,7 @@ protected void updateModelForObject(CodegenModel m, Schema schema) {
addAdditionPropertiesToCodeGenModel(m, schema);
} else {
m.setIsMap(false);
if (ModelUtils.isFreeFormObject(schema)) {
if (ModelUtils.isFreeFormObject(schema, openAPI)) {
// non-composed object type with no properties + additionalProperties
// additionalProperties must be null, ObjectSchema, or empty Schema
addAdditionPropertiesToCodeGenModel(m, schema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ public String toDefaultValue(Schema p) {
return "new " + toModelName(ModelUtils.getSimpleRef(p.get$ref())) + "()";
} else if (ModelUtils.isStringSchema(p)) {
return "utility::conversions::to_string_t(\"\")";
} else if (ModelUtils.isFreeFormObject(p)) {
} else if (ModelUtils.isFreeFormObject(p, openAPI)) {
return "new Object()";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public String toDefaultValue(Schema p) {
if (ModelUtils.isMapSchema(p)) {
String inner = getSchemaType(ModelUtils.getAdditionalProperties(p));
return "Map[String, " + inner + "]() ";
} else if (ModelUtils.isFreeFormObject(p)) {
} else if (ModelUtils.isFreeFormObject(p, openAPI)) {
// We're opinionated in this template to use ujson
return "ujson.Null";
}
Expand All @@ -143,7 +143,7 @@ public String toDefaultValue(Schema p) {

@Override
public String getSchemaType(Schema p) {
if (ModelUtils.isFreeFormObject(p)) {
if (ModelUtils.isFreeFormObject(p, openAPI)) {
// We're opinionated in this template to use ujson
return "Value";
}
Expand Down Expand Up @@ -829,7 +829,7 @@ public CodegenProperty fromProperty(String name, Schema schema) {
CodegenProperty property = super.fromProperty(name, schema);

// Customize type for freeform objects
if (ModelUtils.isFreeFormObject(schema)) {
if (ModelUtils.isFreeFormObject(schema, openAPI)) {
property.dataType = "Value";
property.baseType = "Value";
}
Expand All @@ -839,7 +839,7 @@ public CodegenProperty fromProperty(String name, Schema schema) {

@Override
public String getTypeDeclaration(Schema schema) {
if (ModelUtils.isFreeFormObject(schema)) {
if (ModelUtils.isFreeFormObject(schema, openAPI)) {
return "Value";
}
return super.getTypeDeclaration(schema);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -844,9 +844,10 @@ public static boolean hasValidation(Schema sc) {
* The value can be any type except the 'null' value.
*
* @param schema potentially containing a '$ref'
* @param openAPI document containing the Schema.
* @return true if it's a free-form object
*/
public static boolean isFreeFormObject(Schema schema) {
public static boolean isFreeFormObject(Schema schema, OpenAPI openAPI) {
if (schema == null) {
// TODO: Is this message necessary? A null schema is not a free-form object, so the result is correct.
once(LOGGER).error("Schema cannot be null in isFreeFormObject check");
Expand Down Expand Up @@ -905,6 +906,8 @@ public static boolean isFreeFormObject(Schema schema) {
if (addlProps == null) {
return true;
} else {
addlProps = getReferencedSchema(openAPI, addlProps);

if (addlProps instanceof ObjectSchema) {
ObjectSchema objSchema = (ObjectSchema) addlProps;
// additionalProperties defined as {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public void testIsModelAllowsEmptyBaseModel() {
Schema commandSchema = ModelUtils.getSchema(openAPI, "Command");

Assert.assertTrue(ModelUtils.isModel(commandSchema));
Assert.assertFalse(ModelUtils.isFreeFormObject(commandSchema));
Assert.assertFalse(ModelUtils.isFreeFormObject(commandSchema, openAPI));
}

@Test
Expand Down Expand Up @@ -247,23 +247,23 @@ public void testIsFreeFormObject() {
OpenAPI openAPI = new OpenAPI().openapi("3.0.0");
// Create initial "empty" object schema.
ObjectSchema objSchema = new ObjectSchema();
Assert.assertTrue(ModelUtils.isFreeFormObject(objSchema));
Assert.assertTrue(ModelUtils.isFreeFormObject(objSchema, openAPI));

// Set additionalProperties to an empty ObjectSchema.
objSchema.setAdditionalProperties(new ObjectSchema());
Assert.assertTrue(ModelUtils.isFreeFormObject(objSchema));
Assert.assertTrue(ModelUtils.isFreeFormObject(objSchema, openAPI));

// Add a single property to the schema (no longer a free-form object).
Map<String, Schema> props = new HashMap<>();
props.put("prop1", new StringSchema());
objSchema.setProperties(props);
Assert.assertFalse(ModelUtils.isFreeFormObject(objSchema));
Assert.assertFalse(ModelUtils.isFreeFormObject(objSchema, openAPI));

// Test a non-object schema
Assert.assertFalse(ModelUtils.isFreeFormObject(new StringSchema()));
Assert.assertFalse(ModelUtils.isFreeFormObject(new StringSchema(), openAPI));

// Test a null schema
Assert.assertFalse(ModelUtils.isFreeFormObject(null));
Assert.assertFalse(ModelUtils.isFreeFormObject(null, openAPI));
}

@Test
Expand Down Expand Up @@ -326,9 +326,9 @@ public void test30Schemas() {
Assert.assertTrue(ModelUtils.isMapSchema((Schema) misc.getProperties().get("map1")));

// test free form object
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_1")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_2")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_3")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_1"), openAPI));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_2"), openAPI));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_3"), openAPI));

// test oneOf
Assert.assertTrue(ModelUtils.isOneOf((Schema) misc.getProperties().get("oneof1")));
Expand Down Expand Up @@ -360,9 +360,9 @@ public void test31Schemas() {
Assert.assertTrue(ModelUtils.isMapSchema((Schema) misc.getProperties().get("map1")));

// test free form object
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_1")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_2")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_3")));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_1"), openAPI));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_2"), openAPI));
Assert.assertTrue(ModelUtils.isFreeFormObject((Schema) misc.getProperties().get("free_form_object_3"), openAPI));

// test oneOf property
Assert.assertTrue(ModelUtils.isOneOf((Schema) misc.getProperties().get("oneof1")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,11 @@ components:
$ref: '#/components/schemas/12345AnyOfObject'
required:
- requiredAnyOf
AdditionalPropertiesReferencedAnyOfObject:
description: Check that an object with only additional properties that references another object (e.g. an anyOf object) isn't treated as freeForm
type: object
additionalProperties:
$ref: '#/components/schemas/AnyOfProperty'
AnyOfObject:
description: Test a model containing an anyOf
anyOf:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Cargo.toml
README.md
api/openapi.yaml
bin/cli.rs
docs/AdditionalPropertiesReferencedAnyOfObject.md
docs/AdditionalPropertiesWithList.md
docs/AnotherXmlArray.md
docs/AnotherXmlInner.md
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ Method | HTTP request | Description

## Documentation For Models

- [AdditionalPropertiesReferencedAnyOfObject](docs/AdditionalPropertiesReferencedAnyOfObject.md)
- [AdditionalPropertiesWithList](docs/AdditionalPropertiesWithList.md)
- [AnotherXmlArray](docs/AnotherXmlArray.md)
- [AnotherXmlInner](docs/AnotherXmlInner.md)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,12 @@ components:
$ref: '#/components/schemas/12345AnyOfObject'
required:
- requiredAnyOf
AdditionalPropertiesReferencedAnyOfObject:
additionalProperties:
$ref: '#/components/schemas/AnyOfProperty'
description: Check that an object with only additional properties that references
another object (e.g. an anyOf object) isn't treated as freeForm
type: object
AnyOfObject:
anyOf:
- $ref: '#/components/schemas/AnyOfObject_anyOf'
Expand Down
Loading
Loading