[BUG][codegen] Do not use the "title" attribute to control code generation

[sebastien-rosset]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• What's the version of OpenAPI Generator used?
• Have you search for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Bounty to sponsor the fix (example)

Description

IMO, the "title" attribute in a OAS document should not have any impact whatsover on the code generation (codegen currently uses title to flatten inline schemas). Using 'title' can lead to completely unexpected outcomes for the generated code.

The OAS spec clearly states the 'title' attribute is for human consumption, so codegen should not use it.

I was working with a draft OAS document that has a composed allOf schema. The "title" of the inner schema happens to be the name of the outer (composed) object, but it could also have been any value, including names of totally unrelated schema.

That had the weird side effect to nullify the inheritance hierarchy. Then you have to debug the code to finally understand why the "parent" attribute ends up being nullified as a side effect of the "title" attribute

Currently, codegen uses the "title" attribute in the logic to flatten composed schemas with inline models. When a OAS model is defined inline, the generator flattens the model and tries to use the inlined "title" attribute in the OAS document to create a model name. If "title" is not present in the spec, it uses the hard-coded 'InlineObject'' model name.

The logic is at

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/InlineModelResolver.java

Line 160 in 4208f3d

String modelName = resolveModelName(op.getTitle(), null);

openapi-generator version

master February 8th 2020

OpenAPI declaration file content or url

Consider the following OAS document. It is meant to model an inheritance hierarchy, where some properties are inherited from "os.BaseConfig", plus the XYZ property which is defined as an inlined object.

In Python, the generated name of the outer class is "OsInstall", because the code generator applies a sanitization and camel casing transformations. Furthermore, the inline object does not have "title", in which case the code generator artificially creates a schema with a name hard-coded to "InlineObject". A log message warns to set the "title" attribute.

    os.Install:
        allOf:
            - $ref: '#/components/schemas/os.BaseConfig'
            - type: object
              properties:
                  XYZ:
                          type: string

If, on the other hand, the OAS spec includes the "title" attribute in the inline object, the code generator still creates a schema for the inline object, and the name of that schema is now based on the "title" attribute. This is a really surprising feature. IMO, at a minimum we should log a warning to indicate the user that the code generator uses the title attribute.

    os.Install:
        allOf:
            - $ref: '#/components/schemas/os.Abstract'
            - type: object
              title: OS Install
              properties:
                  Name:
                          type: string

In the case when the title attribute is used, the name of the generated schema does not use the same transformation function. The generated name ends up being "OSInstall", unlike the outer class name which is "OsInstall". Of course, the author is supposed to write whatever title they want, so it could be "os installation", "the OS installation configuration file", or any other string that makes sense for humans. The author does not know this will be used for code generation.

In the case of Python, the two strings "OSInstall" versus "OsInstall" cause a import failure when the SDK is executed.

An even more unexpected result is with the following OAS schema. The inner schema within 'allOf' has a "title attribute, and its value happens to match the name of the outer schema.

    os.Install:
        allOf:
            - $ref: '#/components/schemas/os.Abstract'
            - type: object
              title: os.Install
              properties:
                  Name:
                          type: string

Without the "title" attribute, the generated Java class is:

class OsInstall extends OsAbstract

With the title attribute set to "os.Install", the inheritance is no longer generated, which is really weird.

class OsInstall

Potentially, the value of the "title" attribute could be the name of a completely unrelated schema.

Command line used for generation

Steps to reproduce

Related issues/PRs

#843

Suggest a fix

Don't use the "title" attribute to generate a unique model name.

[wing328]

A bit more background on why we use title to name the inline model. Some users prefer to define the model using the inline schema instead of defining the model separately, which gives users total control in naming the model. We do advise them to define the model separately if they want to have complete control on the model naming but they still prefer to use the inline schema.

title is probably the closest or most reasonable pick to name the inline schema (and we agree it's not the most ideal way to name the model defined in the inline schema) so that's what we've chosen at the moment to name models defined by the inline schemas.

The generated name ends up being "OSInstall", unlike the outer class name which is "OsInstall".

For the issue you mentioned, it's a bug as the naming should be done using toModelName override in the generator (regardless of what attribute, title or something else, to use when naming the inline schema)

    os.Install:
        allOf:
            - $ref: '#/components/schemas/os.Abstract'
            - type: object
              title: OS Install
              properties:
                  Name:
                          type: string

There was a PR to generate the last inline schema as a separate model while the old behavior was that the last inline schema's properties are added to the object/model/class (e.g. os.Install in this case). There were a couple of issues opened that users do not like the new behavior so we may consider switching back or add an option to make the behavior configurable.

[sebastien-rosset]

A bit more background on why we use title to name the inline model. Some users prefer to define the model using the inline schema instead of defining the model separately, which gives users total control in naming the model. We do advise them to define the model separately if they want to have complete control on the model naming but they still prefer to use the inline schema.

title is probably the closest or most reasonable pick to name the inline schema (and we agree it's not the most ideal way to name the model defined in the inline schema) so that's what we've chosen at the moment to name models defined by the inline schemas.

Wouldn't it be better to use a vendor extension? Or a build-time configuration property? IMO, a doc team should be able to edit the "title" without impacting the generated SDK. It's very surprising that "title" (a human-friendly title) ends up being used for code generation. At the very least, we should document it. If you point me to the right .md file where to document, I can submit a PR, but IMO I'd rather not rely on "title" in the first place.

For example, they should be able to fix typos, such as changing the title from "OS instaler" to "OS installer". Or decide to change the title to "operating system installation", because maybe it's used for the UI and customer feedback indicated it's a better title. Or use i18n and set the title for each language, e.g. 운영 체제 설치.

The generated name ends up being "OSInstall", unlike the outer class name which is "OsInstall".

For the issue you mentioned, it's a bug as the naming should be done using toModelName override in the generator (regardless of what attribute, title or something else, to use when naming the inline schema)

That would work for "OSInstall", but on the other hand I don't see how this would work for 'allOf' composed schemas because toModelName only takes a string argument. It would not have the contextual information, i.e. no information that it's a composed schema. Furthermore, I think generating the proper name for allOf is a language independent problem.

    os.Install:
        allOf:
            - $ref: '#/components/schemas/os.Abstract'
            - type: object
              title: OS Install
              properties:
                  Name:
                          type: string

There was a PR to generate the last inline schema as a separate model while the old behavior was that the last inline schema's properties are added to the object/model/class (e.g. os.Install in this case). There were a couple of issues opened that users do not like the new behavior so we may consider switching back or add an option to make the behavior configurable.

[wing328]

The OAS spec clearly states the 'title' attribute is for human consumption, so codegen should not use it.

I don't see "human consumption" mentioned in the spec. Can you quote directly in your reply or provide an URL to the section describing it?

IMO, a doc team should be able to edit the "title" without impacting the generated SDK. It's very surprising that "title" (a human-friendly title) ends up being used for code generation

To have complete control of the model name (for composed schema or non-composed schema), define it separately instead. Is that something you would consider?

[sebastien-rosset]

The OAS spec clearly states the 'title' attribute is for human consumption, so codegen should not use it.

I don't see "human consumption" mentioned in the spec. Can you quote directly in your reply or provide an URL to the section describing it?

I think it's more the spirit than a literal statement, I should have written differently. It does mention. "Title" is used once in the spec with the following example, which is a clear example for human consumption.

title: Sample Pet Store App
description: This is a sample server for a pet store.

Otherwise, it refers to the JSON schema:
https://tools.ietf.org/html/draft-wright-json-schema-validation-00#section-6.1

   Both of these keywords can be used to decorate a user interface with
   information about the data produced by this user interface.  A title
   will preferrably be short, whereas a description will provide
   explanation about the purpose of the instance described by this
   schema.

I think there is no ambiguity about the fact that this is used for user interfaces, not for interpreting the data to generate code.
There are also references to examples (http://json-schema.org/learn/), such as the one below:

{
  "$id": "https://example.com/geographical-location.schema.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "Longitude and Latitude Values",
  "description": "A geographical coordinate."
}

I think somebody who types "Longitude and Latitude Values" would not anticipate this is used by a code generator.

IMO, a doc team should be able to edit the "title" without impacting the generated SDK. It's very surprising that "title" (a human-friendly title) ends up being used for code generation

To have complete control of the model name (for composed schema or non-composed schema), define it separately instead. Is that something you would consider?

Yes, actually it would be a good way, because that would make it possible to decouple the authoring versus customization for generation purpose.

[wing328]

I think it's more the spirit than a literal statement, I should have written differently

It's fine. Just want to make sure I didn't miss it in the official spec. Even the spec didn't say anything about whether title can be used in code generation or is human consumption only, your suggestion is still valid.

Again title is not the best way to come up with the model name (I believe we all agree on this). To avoid using it to name the model, the best way is to define the model separately instead.

[sebastien-rosset]

Just so I understand what you are proposing, are you suggesting there would be a configuration file in YAML or JSON format, outside the OAS spec, that would provide additional parameters to help with code generation? Essentially it would be a way to externalize what is currently specified as vendor extensions?

[wing328]

No additional configuration files or vendor extensions.

Just define the schema separately instead of inline to have complete control of the model name.