[BUG] InlineModelResolver not ignoring additional properties of $ref fixed field

cliffano commented 2 years ago

Description

When the specification has additional properties next to $ref fixed field (in the example below, $ref has an additional property type: object), OpenAPI Generator produces error:

[main] ERROR o.o.codegen.InlineModelResolver - Illegal schema found with $ref combined with other properties, no properties should be defined alongside a $ref:

However, the specification passes validation https://apidevtools.org/swagger-parser/online/ . So the spec passes Swagger Parser, but fails on OpenAPI Generator.

Thanks to Richard Whitehouse on OpenAPI Generator Slack for pointing me to the OpenAPI spec https://spec.openapis.org/oas/latest.html#fixed-fields-18 which states

This object cannot be extended with additional properties and any properties added SHALL be ignored.

Which indicates that OpenAPI Generator should actually ignore those additional properties instead of resulting in error.

openapi-generator version

Tested on latest master as of 27 Aug 2022 and also published version 6.0.1 .

OpenAPI declaration file content or url

          additionalProperties: false
          properties:
            all_of:
              type: array
              items:
                type: object
                $ref: '#/components/schemas/CatalogsProductGroupFilterKeys'
              minItems: 1

The complete specification is available at https://github.com/pinterest/api-description/blob/main/v5/openapi.yaml .

Generation Details

I have a complete build output on GitHub Actions https://github.com/cliffano/pinterest-sdk/runs/7879342459?check_suite_focus=true

Steps to reproduce

curl https://github.com/pinterest/api-description/blob/main/v5/openapi.yaml -o myopenapi.yaml
docker \ run \ --rm \ -v pwd:/local openapitools/openapi-generator-cli:v6.0.1 \ generate \ --input-spec /local/myopenapi.yaml \ --config /local/clients/ruby/conf.json \ --generator-name ruby \ --output /local/clients/ruby/generated;

Suggest a fix

I think InlineModelResolver conditional check (https://github.com/OpenAPITools/openapi-generator/blob/c135139e3dfba367800a02962f269b57a32ab8d4/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java#L224) for additional properties should be removed, allowing the processing to continue.

If this looks correct, I can give it a stab at creating a PR.

wing328 commented 2 years ago

yes please open a PR and we'll review accordingly

wing328 commented 2 years ago

I'll file a PR for this one to update the wording and use LOGGER.warn instead.

UPDATE: filed https://github.com/OpenAPITools/openapi-generator/pull/13498

cliffano commented 2 years ago

@wing328 I read your message from Aug 27 literally the next day, so I didn't end up creating any PR.

I understand that https://github.com/OpenAPITools/openapi-generator/pull/13498 includes the fix to this GH issue, but at the same time it seems to include other changes.

Since #13498 is still open. Would it be ok if I pick the InlineModelResolver-related changes?

wing328 commented 2 years ago

sure, up to you.