[core] Initial FeatureSet structures

[jimschubert]

PR checklist

• Read the contribution guidelines.
• Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
• Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
• Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Request for input on these structures.

This touches on #840 and #845 (part of the Separation of Concerns project). I've introduced these structures as a way for us to explicitly declare what a generator (and its libraries, if they differ) supports. The end goal is to be able to dynamically create a feature matrix as part of our doc site, as discussed in #503. The feature sets would also allow users to search for generators supporting their targeted features.

cc @OpenAPITools/generator-core-team @OpenAPITools/openapi-generator-collaborators

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[jmini]

I like the idea. Thank you for pushing this forward.

---

How was this list of feature created?

For example I am interested in Cookie based Authentication, would that be a other SecurityFeature enum value?

In the "ModelReuse" feature, what does "Composition" means?

I can imagine that we need to distinguish between:

• one schema is oneOf SchemaA, SchemaB or SchemaC
• Schema is oneOf String or SomeObj (useful in expand parameter scenario)

An other idea: Callback. This is a OpenAPIv3 feature that some generator might support and other not.

We can start with this set of feature, but I think this needs to be extended

---

...(and its libraries, if they differ)...

Is this going only on the "library" flag or is this something different?

For example in the Java-Generator we have stuff such as "java8", or "dateAndTime-library" that influences what a generator support or not.
As discussed in #3570, I might need to introduce a "serialization library" and depending if Jackson or Gson is used some stuff might be present or not.
(example JsonNullable from #3474 is a jackson only feature)

---

Related topic (but I do not want to hijack this review):

How will we ensure that a "feature" is supported?

When we have started to model the features as proposed in this PR, we need to come up with a small TCK (a minimal OpenAPI Spec containing the feature) and some tests (a sever-mock if this is for a generated client, or vice versa for server generator). This is really important to move forward with the quality of the project.

[jimschubert]

@jmini this list of features was pulled from an evaluation that someone on the core team (@cbornet I believe) did maybe two years ago. There was a Google Doc matrix. If you joined the core team after this was shared, ping me in Slack and I'll try to find you the link.

ModelReuse refers to how schemas in a spec document are related. Composition would mean that the generator supports models made up of other models (via allOf, for example). All generators should support this by default, but it's possible that some pre/post processing removes this support. It exists here specifically because it's called out separated via OpenAPI-Specification Schema Composition.

I agree that we'll likely need to distinguish between other states. Maybe we could rename ModelReuse to SchemaSupport and have: Simple, Composite, AllOf, OneOf?

Regarding your comments about Callback and Cookies, I forgot to mention that the design is based off my earlier design which only covered 2.0 specification. I'd still need to do a gap analysis and incorporate 3.x features.

Is this going only on the "library" flag or is this something different?

The library feature set should work only on the library flag, I would think. There aren't any library configurations that I know of which remove OpenAPI Specification feature support. It is possible, I guess, for one library implementation to not support cookies/callbacks… and this is why I've included the library-specific feature set. In general, I think this would be empty and fallback to the defaults for the generator.

How will we ensure that a "feature" is supported?

This is an excellent question, and I don't consider it hijacking the review at all! This is actually a large part of why I'm working toward adding this feature set metadata.

On one end of the usefulness of this metadata, we'd be able to generate a feature matrix for all generator and libraries. We'll also be able to add some feature search to CLI/Gradle list tasks.

On the other hand, my goal is to reach an explicit output data type. We've had a community contribution (see #837) to start on this work. That PR would need to be evaluated and moved forward. Once we have a data type other than a Map<String, Object> being bound to our templates, I think this will allow us to test generators more dynamically and without spinning up a full client/server stack. For instance, if a generator says it supports allOf, we should know the structure of the ADT bound to the template for a given input spec. We could probably even write a base test which generator maintainers could override some checks as needed. On a side note, I also plan to eventually have an immutable copy of the input spec which can be passed via context throughout the process so by the end we're only working on a mutated transformation object. This would also allow us to easily evaluate inputs vs outputs.

[jmini]

While I am working on a test suite for java client, I have "discovered" that google-api-client does not support authentication (see #3698 for tracking)

I understand that taking the library into account for modelling feature makes everything more complex. On the other side, the Java client generator supports so many framework that it will be hard to say "This feature is supported by the generator or not".

[jimschubert]

I added some clarification/comments to the model reuse feature, renaming it to SchemaSupportFeature.

I'll look into creating a "feature set" of OpenAPI 2.0 and 3.0 specifications, so I can analyze whether or not I've missed something.

I think it would be best to keep the features documenting only the specification support, rather than the generator support as that can be queried using config-help. I know that's not what you were referring to with your comment about the library feature support, I just wanted to add that bit of clarification.

[etherealjoy]

I am curious about this

• How would the responses look like, lets say 200, 201, etc
  Currently in most templates it is a pain to write the template to handle these properly.

• consumes/produces

[jimschubert]

@etherealjoy

How would the responses look like, lets say 200, 201, etc
Currently in most templates it is a pain to write the template to handle these properly.

I'm not sure what you mean here…  Are you saying that you'd like to see supported response codes?

Please keep in mind that this PR and metadata proposal has nothing to do with templates. While the metadata could be used at a later point to validate either the data model that gets bound to the templates, or compile and validate the templates directly… setting a feature support flag will not actually change how the templates work.

consumes/produces

I'll make note of this as well.

[jimschubert]

@etherealjoy I added the consumes/produces and about 10 other options, as well as some notes about what is missing in the default codegen (see DefaultFeatureSet assignment).

After some thought about how we could best be explicitly in feature support across generators, I went with this type of API. I'd be interested in feedback:

featureSet = getFeatureSet().modify()
    .includeDocumentationFeature(DocumentationFeature.Readme)
    .includeWireFormatFeatures(WireFormatFeature.PROTOBUF)
    .excludeWireFormatFeatures(
            Arrays.stream(WireFormatFeature.values()).filter(i -> i != WireFormatFeature.PROTOBUF)
            .toArray(WireFormatFeature[]::new)
    )
    .securityFeatures(EnumSet.noneOf(SecurityFeature.class))
    .build();

This way, generators should say "this is what is not supported" and is clear to all contributors up from what is missing from the generator.

Regarding the question about HTTP status codes, I think that's a completely different thing. We should most likely have an http status helpers on CodegenResponse, something like this:

    public boolean isStatus1xx() { return code != null && code.length() == 3 && code.startsWith("1"); }
    public boolean isStatus100() { return "100".equals(code); }
    public boolean isStatus101() { return "101".equals(code); }
    public boolean isStatus102() { return "102".equals(code); }

    public boolean isStatus2xx() { return code != null && code.length() == 3 && code.startsWith("2"); }
    public boolean isStatus200() { return "200".equals(code); }
    public boolean isStatus201() { return "201".equals(code); }
    public boolean isStatus202() { return "202".equals(code); }
    public boolean isStatus203() { return "203".equals(code); }
    public boolean isStatus204() { return "204".equals(code); }
    public boolean isStatus205() { return "205".equals(code); }
    public boolean isStatus206() { return "206".equals(code); }
    public boolean isStatus207() { return "207".equals(code); }

    public boolean isStatus3xx() { return code != null && code.length() == 3 && code.startsWith("3"); }
    public boolean isStatus300() { return "300".equals(code); }
    public boolean isStatus301() { return "301".equals(code); }
    public boolean isStatus302() { return "302".equals(code); }
    public boolean isStatus303() { return "303".equals(code); }
    public boolean isStatus304() { return "304".equals(code); }
    public boolean isStatus305() { return "305".equals(code); }
    public boolean isStatus307() { return "307".equals(code); }

    public boolean isStatus4xx() { return code != null && code.length() == 3 && code.startsWith("4"); }
    public boolean isStatus400() { return "400".equals(code); }
    public boolean isStatus401() { return "401".equals(code); }
    public boolean isStatus402() { return "402".equals(code); }
    public boolean isStatus403() { return "403".equals(code); }
    public boolean isStatus404() { return "404".equals(code); }
    public boolean isStatus405() { return "405".equals(code); }
    public boolean isStatus406() { return "406".equals(code); }
    public boolean isStatus407() { return "407".equals(code); }
    public boolean isStatus408() { return "408".equals(code); }
    public boolean isStatus409() { return "409".equals(code); }
    public boolean isStatus410() { return "410".equals(code); }
    public boolean isStatus411() { return "411".equals(code); }
    public boolean isStatus412() { return "412".equals(code); }
    public boolean isStatus413() { return "413".equals(code); }
    public boolean isStatus414() { return "414".equals(code); }
    public boolean isStatus415() { return "415".equals(code); }
    public boolean isStatus416() { return "416".equals(code); }
    public boolean isStatus417() { return "417".equals(code); }
    public boolean isStatus418() { return "418".equals(code); }
    public boolean isStatus419() { return "419".equals(code); }
    public boolean isStatus420() { return "420".equals(code); }
    public boolean isStatus422() { return "422".equals(code); }
    public boolean isStatus423() { return "423".equals(code); }
    public boolean isStatus424() { return "424".equals(code); }

    public boolean isStatus5xx() { return code != null && code.length() == 3 && code.startsWith("5"); }
    public boolean isStatus500() { return "500".equals(code); }
    public boolean isStatus501() { return "501".equals(code); }
    public boolean isStatus502() { return "502".equals(code); }
    public boolean isStatus503() { return "503".equals(code); }
    public boolean isStatus504() { return "504".equals(code); }
    public boolean isStatus505() { return "505".equals(code); }
    public boolean isStatus507() { return "507".equals(code); }

Even then, it's completely up to templates to honor these status as well as to properly implement the "default" usage to spec. I opened #3994 if you want to try those out and see if they're helpful.

[etherealjoy]

Regarding the question about HTTP status codes, I think that's a completely different thing. We should most likely have an http status helpers on CodegenResponse, something like this:

About this, one of the main issues in most of the generators is how to handle a request with multiple responses. So that the code contains the templating not only for default successful response but also for others. How can we expose this to the templates so that the handling for different responses can be simplified.

[jimschubert]

@etherealjoy I don't think that question pertains to this PR. OneOf support, which I've called Union in the feature so it better matches to the concept, is specific to the language. If a language supports it, the feature set should set the enum for the codegen. If a language doesn't support it, the codegen should remove the enum.

With server implementations, it's simple.... you can often just iterate over the responses collection and have a generic-typed or more general type (Object IActionResult or similar). For clients in many languages, you would need to create a new Union type or use something like a tuple, Either, or custom wrapper. I don't think there's a way to generalize this in a cross-language way.

To clarify, this PR isn't intending to solve problems with the spec. It's only meant to add enums for features defined in OAS 2/3 specs or to add things we consider missed in the spec.

We will want to be careful not to leak too many tooling options into features. Because the spec defines info, examples, and documentation, we have a documentation enum including API, model, and readme options (all places that the docs mentioned in the specs would be written to).

We may want to eventually add generator features (like the client modification enum), but I think we'll need to be very careful with this to include only options that will always be shared. We have issues with CLI "global" switches currently where many don't apply to lots of generators.

[jonschoning]

fwiw, I think of Union as more of a term with meaning dependent to specific languages, and OneOf as something that has meaning according to the openapi spec independent of language.

[jimschubert]

@jonschoning Could you discuss your comment a little more?

OneOf is definitely a spec-only concept, whereas I've used Union because a "Union" and "Union Types" are all common terms in FP and OOP, while OneOf is not common. Compare this to the other SchemaSupportFeature enums: Simple, Composite, and Polymorphism. I've called this "SchemaSupport" where "Schema" is intended to be synonymous with object-model. Do you think this enum should be renamed so there's no confusion with "OpenAPI Schema"?

I have a longer term goal as part of #845 and more directly via #844 to support more specifications than just OpenAPI, so I'm trying to avoid OpenAPI-isms as much as possible. It's difficult in some places because there are specification-specific things such as parameter styling which I haven't seen in other specifications.

[jonschoning]

@jimschubert after some thought and in light of the goals, i think Union will be fine.

(I was only remembering that there is a difference between tagged unions and untagged unions, but in this case it is not a difference that matters, since it appears in all languages it can only be one thing at a time)

[richardwhiuk]

I've added some notes on Rust Server support

[jimschubert]

This has been open for quite a while without additional comments, so I'm assuming there are no objections to merge. As this does not change the overall interface and only adds another property to GeneratorMetadata plus definitions within generators, I think it's safe to go before 5.0 so we can get the generated feature matrix in before 5.0.

I'll merge up master and regenerate samples, then merge once everything passes. If there are any objections from the core team, please feel free to revert the merge from master and reapply it into 4.3 or 5.0.

[jimschubert]

I merged up master locally and there were no changes (forgot I had done that last week), so going ahead with the merge.