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 @@ -356,7 +356,9 @@ private void gatherInlineModels(Schema schema, String modelPrefix) {
} else if (schema.getProperties() != null) {
// If non-object type is specified but also properties
LOGGER.error("Illegal schema found with non-object type combined with properties," +
" no properties should be defined:\n " + schema.toString());
" no properties should be defined:" +
" consider using --openapi-normalizer REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT=true\n " +
Copy link
Member

Choose a reason for hiding this comment

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

thanks for the PR

what about automatically fixing it in inline model resolver instead?

[main] ERROR o.o.codegen.InlineModelResolver - Illegal schema found with non-object type combined with properties, no properties should be defined:
 class ArraySchema {
    class Schema {
        type: array
        format: null
        $ref: null
        description: null
        title: null
        multipleOf: null
        maximum: null
        exclusiveMaximum: null
        minimum: null
        exclusiveMinimum: null
        maxLength: null
        minLength: null
        pattern: null
        maxItems: null
        minItems: null
        uniqueItems: null
        maxProperties: null
        minProperties: null
        required: null
        not: null
        properties: {empty=class BooleanSchema {
            class Schema {
                type: boolean
                format: null
                $ref: null
                description: null
                title: null
                multipleOf: null
                maximum: null
                exclusiveMaximum: null
                minimum: null
                exclusiveMinimum: null
                maxLength: null
                minLength: null
                pattern: null
                maxItems: null
                minItems: null
                uniqueItems: null
                maxProperties: null
                minProperties: null

so that we can avoid another normalizer rule.

Copy link
Author

Choose a reason for hiding this comment

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

Hello,

I don't have an opinion about where the normalisation should be done. Mattias-Sehlstedt commented in the bug report, and advised to put it in the normalizer since it solves an oddity.
My thought was to leave the possibility to opt in to apply the normalizer rule and remove this oddity.

Maybe in order to avoid to create another normalizer rule, I could modify my PR in order to leave it in the normalizer but make it default. The drawback is that we would loose the possibility to opt out (and keep the "properties" attribute for a schema other than object, which is the oddity).
Do you think that would fit ?

By the way, I think "additionalProperties" attribute also should be removed for a schema other than object. At least it is what modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java says.

Looking forward to solve this bug, thanks to your advice,

Guillaume

schema.toString());
return;
} else if (schema.getAdditionalProperties() != null) {
// If non-object type is specified but also additionalProperties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ public class OpenAPINormalizer {
// the allOf contains a new schema containing the properties in the top level
final String REFACTOR_ALLOF_WITH_PROPERTIES_ONLY = "REFACTOR_ALLOF_WITH_PROPERTIES_ONLY";

// when set to true, remove the "properties" of a schema with type other than "object"
final String REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT = "REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT";

// when set to true, normalize OpenAPI 3.1 spec to make it work with the generator
final String NORMALIZE_31SPEC = "NORMALIZE_31SPEC";

Expand Down Expand Up @@ -205,6 +208,7 @@ public OpenAPINormalizer(OpenAPI openAPI, Map<String, String> inputRules) {
ruleNames.add(FILTER);
ruleNames.add(SET_CONTAINER_TO_NULLABLE);
ruleNames.add(SET_PRIMITIVE_TYPES_TO_NULLABLE);
ruleNames.add(REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT);


// rules that are default to true
Expand Down Expand Up @@ -721,6 +725,8 @@ public Schema normalizeSchema(Schema schema, Set<Schema> visitedSchemas) {
}
markSchemaAsVisited(schema, visitedSchemas);

processNormalizeOtherThanObjectWithProperties(schema);

if (ModelUtils.isArraySchema(schema)) { // array
Schema result = normalizeArraySchema(schema);
normalizeSchema(result.getItems(), visitedSchemas);
Expand Down Expand Up @@ -1644,5 +1650,21 @@ protected Schema processNormalize31Spec(Schema schema, Set<Schema> visitedSchema
return schema;
}

// ===================== end of rules =====================
/**
* When set to true, remove "properties" attribute on schema other than "object"
* since it should be ignored and may result in odd generated code
*
* @param schema Schema
* @return Schema
*/
protected void processNormalizeOtherThanObjectWithProperties(Schema schema) {
if (getRule(REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT)) {
// Check object models / any type models / composed models for properties,
// if the schema has a type defined that is not "object" it should not define
// any properties
if (schema.getType() != null && !"object".equals(schema.getType())) {
schema.setProperties(null);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,33 @@
import org.openapitools.codegen.utils.ModelUtils;
import org.testng.annotations.Test;

import java.lang.reflect.Array;
import java.util.*;

import static org.testng.Assert.*;

public class OpenAPINormalizerTest {

@Test
public void testOpenAPINormalizerOtherThanObjectWithProperties()
{
// to test the rule REF_AS_PARENT_IN_ALLOF
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/issue_21680_array_with_properties.yaml");

Schema schema = openAPI.getComponents().getSchemas().get("errors");
assertNotNull(schema);
assertNotNull(schema.getProperties());

Map<String, String> options = new HashMap<>();
options.put("REMOVE_PROPERTIES_FROM_TYPE_OTHER_THAN_OBJECT", "true");
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
openAPINormalizer.normalize();

Schema schema2 = openAPI.getComponents().getSchemas().get("errors");
assertNotNull(schema2);
assertNull(schema2.getProperties());
}

@Test
public void testOpenAPINormalizerRefAsParentInAllOf() {
// to test the rule REF_AS_PARENT_IN_ALLOF
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
# Corresponds to bug report 21680: https://github.com/openapitools/openapi-generator/issues/21680
openapi: 3.0.1
info:
title: API that has problem with OpenAPI Generator
version: 1.0.0
paths:
"/forbiddenaccesscsrf":
get:
summary: Forbidden access CSRF
operationId: forbiddenAccessCsrfGet
responses:
'403':
description: Expected response
content:
application/json:
schema:
"$ref": "#/components/schemas/errors"
components:
schemas:
error:
required:
- code
- horodatage
- message
type: object
properties:
code:
type: string
description: Short error description
message:
type: string
description: Complete human readable description
error_uri:
type: string
description: Detailed error description URI
format: uri
horodatage:
type: string
description: Date time of occurence
format: date-time
errors:
type: array
properties:
empty:
type: boolean
items:
"$ref": "#/components/schemas/error"