[Breaking change without fallback] [suggestion] [discussion] Split apis option as a semicolon-separated list

[shybovycha]

Introduction

There is one quite nifty but undocumented option, apis, which could be set in system properties. It defines which API groups should be generated. Given the undocumented internal behavior of grouping all operations by tags, this gives an easy way to migrate existing projects to Open-API with minimal changes by automatically generating API definitions and then generating API interfaces with operations automatically grouped according to the existing endpoints.

Hence I am creating a few PRs (#4937, #4938, #4939) addressing this hidden gem of openapi-generator.

This change

Since apis is a (comma-separated) list itself and the additionalProperties option is also a (comma-separated) list, it is impossible to have a comma-separated list inside a comma-separated list with the existing parsing.

This PR introduces a behavioral change by treating the apisToGenerate list as semicolon-separated list. This way it is possible to pass it inside the list (such as additionalProperties).

Other two possible work-arounds I see are:

1. introduce a x-generate-apis property and make it a normal comma-separated list; this will, however, require a bit of rework of the core logic by altering between apis and x-generate-apis values to populate the apisToGenerate variable in DefaultGenerator#generateApis, like in [feature] [jax-rs] Get apis property from vendor extensions #4945 (since it is a private method - this can't be easily overriden for JavaJAXRSSpecServerCodegen only)
2. there is one documented but weirdly used option, generateApis, which only stores the string representation of a boolean value in configuration (either in POM file or in CLI options), but later is transformed to either an empty string (representing the true) or null (representing false), see this. This could be replaced with a list of APIs to generate, essentially following option 1, just for a different config option

Happy to hear your thoughts on this.

PR checklist

• Read the contribution guidelines.
• If contributing template-only or documentation-only changes which will change sample output, build the project before.
• Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
• File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
• Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

[jimschubert]

I like the idea of making these 'more accessible' from a tooling perspective. However, I think the target of adding this to additionalProperties is the wrong approach, possibly because the doc site wasn't found or maybe additionalProperties isn't defined clearly enough (see https://openapi-generator.tech/docs/usage#additional-properties) . The additionalProperties map is literally only meant to be additional properties provided to the generators and templates. I'd be interested in ideas about how/where to make this more clear.

The apis option is meant for configuring DefaultGenerator, for which we do have a settings object. I wonder if rather than trying to add these selective generation options to additionalProperties properties, we could maybe add another option such as selectiveGeneration with the same structure/format as additionalProperties? I think that would be much cleaner, and less confusing. Following our cycle for breaking changes, we'd need to support the current format and the new format through at least one full minor version (breaking change with fallback).

The apis and other selective generation options are documented here: https://openapi-generator.tech/docs/customization#selective-generation. Related to grouping by tags documentation, I believe this is on the wiki, but should definitely be more prominent on the docsite and README.

Also, please find the ignore file documentation on the site. I feel like this might better address your concerns and is a much more powerful option (although, tied to file rather than OpenAPI tag/schema simple names).

[jimschubert]

I also wanted to comment on the "weird" apisToGenerate, I think the Maven plugin sets this to an empty string because the contents are populated later. I'm on mobile and not really able to fully look into it at the moment. I wanted to point you to CodegenConfigurator, which is where the workflow settings are managed. This is the best place to look for configuration management because the CLI and plugins have some idiosyncrasies that are normalized by CodegenConfigurator. See:

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

Line 444 in b22f7f0

for (Map.Entry<String, String> entry : workflowSettings.getSystemProperties().entrySet()) {

I have some goals to rename things a bit to match our terminology, for instance DefaultGenerator is confusing because it's the workflow engine, not a "Generator" (these are currently called "Codegen"). These are all documented in work via project no. 5, if you're interested to see my plans and progress.

[shybovycha]

Agreed, additionalProperties though somewhat misleading, should be used for a different purpose.

The selective generation is slightly different from what I mean - currently, it is a crude short-circuit, (generate) "all-or-nothing" switch. Whilst what I mean is actually grouping the generated operations into resources so that (as in our case) it matches the existing REST API structure. Users might disagree with the current (or "default") grouping.

Talking about generateApis, I meant the internal use of this variable:

// org.openapitools.codegen.plugin.CodeGenMojo
@Parameter(name = "generateApis", property = "openapi.generator.maven.plugin.generateApis", required = false)
    private Boolean generateApis = true;

// ...

// Set generation options
if (null != generateApis && generateApis) {
    GlobalSettings.setProperty(CodegenConstants.APIS, ""); // <--- a representation of "true"
} else {
    GlobalSettings.clearProperty(CodegenConstants.APIS); // <--- a representation of "false"
}

[shybovycha]

Getting back to the topic of grouping the operations into resources by user-defined configuration, what do you think about such a feature/enhancement? Do you think grouping operations by tags is a good approach?

[jimschubert]

@shybovycha I do think it would be good to allow users to control the grouping. Our tag grouping may fall apart when people have defined more than one tag on a path, as we take the first tag only.

Regarding your comment about generateApis,
I want to reiterate that this is Maven plugin logic only. DefaultGenerator looks again at this apis option and evaluates it.

The apis "system property" (we've inherited code that defined -D to look like system properties, but are not) allows for two states: empty string and a CSV of operations to generate. The empty string means a user wants to generate APIs (-Dapis) while the CSV requests selective generation (-Dapis=secret,public,admin).

See

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

Line 533 in 0344f14

String apiNames = GlobalSettings.getProperty("apis");

The weird if condition in CodegenMojo is probably a weird case with Maven, but should be commented at the line you've linked. You can still define the "system properties" in the Maven plugin, but I'm assuming the reason this "all-or-nothing" exists is because Maven probably treats an empty node as if it doesn't exist. That is,<apis></apis> probably doesn't result in the apis property being an empty string, but rather a null. Our plugins attempt to be feature complete with options supported by the CLI and this sometimes means oddities like the one you've pointed out.

The reason we have the all-or-nothing is because if you pass one of the selective generation options, it'll limit to only that option. For instance,-Dmodels=Cat results in just the cat model, while -Dapis -Dmodels=Catresults in the model plus all APIs.

We've since moved toward the more robust ignore file style patterns, but keep these other properties for existing users.

All that said, I'd personally prefer if the selective generation provided something different to users. For example, if it allowed tags and operation ids. Or, http methods. I'm just not sure how much it's used in the first place, or how much effort should go into extending it.

[jimschubert]

I'll be addressing this issue via #5251 documentation and adding the new apisToGenerate property to match the maven plugin's already existing modelsToGenerate.

The plugin supports the following structure:

<configuration>
	<environmentVariables>
		<apis>First,Second,Third</apis>
	</environmentVariables>
</configuration>

Where First,Second,Third are the names of three APIs you'd like to generate. This wasn't clear in the docs because environmentVariables wasn't listed in the options in the README and I assume not all IDEs present the option as IntelliJ does. Even if it did, I don't know if everyone knows how to configure maps in Maven. I hope the new docs in the linked PR help clarify all this.

[shybovycha]

I'm gonna close this PR in favor of #5251