Skip to content

[suggestion] Get apis property from either SystemProperties or additionalProperties#4937

Closed
shybovycha wants to merge 1 commit intoOpenAPITools:masterfrom
shybovycha:enhancement/get-apis-option-from-additional-properties-if-undefined-in-system
Closed

[suggestion] Get apis property from either SystemProperties or additionalProperties#4937
shybovycha wants to merge 1 commit intoOpenAPITools:masterfrom
shybovycha:enhancement/get-apis-option-from-additional-properties-if-undefined-in-system

Conversation

@shybovycha
Copy link
Copy Markdown
Contributor

@shybovycha shybovycha commented Jan 7, 2020 •
edited
Loading

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

I could not really find a good way to pass values through SystemProperties when using the maven plugin. Hence it is pretty much impossible from my point of view to set the apis property, I think that it could also be passed through the additionalProperties option.

Also, see the other PR ( #4938 ) which fixes a bug in parsing the list options in additionalProperties.

Does not seem like a breaking change, hence targeting master.

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.

This was referenced Jan 7, 2020
Closed
Closed
@shybovycha shybovycha changed the title Get apis property from either SystemProperties or additionalProperties [suggestion] Get apis property from either SystemProperties or additionalProperties Jan 7, 2020
@shybovycha shybovycha mentioned this pull request Jan 8, 2020
Open
5 tasks
jimschubert
jimschubert reviewed Feb 8, 2020
View reviewed changes
Comment thread modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java
String apiNames = GlobalSettings.getProperty(CodegenConstants.APIS);

if (apiNames == null || StringUtils.isBlank(apiNames)) {
apiNames = (String) opts.getConfig().additionalProperties().get(CodegenConstants.APIS);
Copy link
Copy Markdown
Member

@jimschubert jimschubert Feb 8, 2020

Choose a reason for hiding this comment

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

I don't think this will work… it would mean that generators can no longer support an apis additional property for other reasons as any setting here would cause no API classes to get generated.

Can you provide an example of how you've configured the Maven plugin? I think it would be best to fix system property support there.

@jimschubert
Copy link
Copy Markdown
Member

jimschubert commented Feb 8, 2020

@shybovycha I've tested the following, and it works as expected:

diff --git a/modules/openapi-generator-maven-plugin/examples/kotlin.xml b/modules/openapi-generator-maven-plugin/examples/kotlin.xml
index 3a2979232c..a28a2b6095 100644
--- a/modules/openapi-generator-maven-plugin/examples/kotlin.xml
+++ b/modules/openapi-generator-maven-plugin/examples/kotlin.xml
@@ -66,6 +66,10 @@
 							<apiPackage>kotlintest.org.openapitools.client.api</apiPackage>
 							<modelPackage>kotlintest.org.openapitools.client.model</modelPackage>
 							<invokerPackage>kotlintest.org.openapitools.client</invokerPackage>
+
+							<environmentVariables>
+								<apis>Pet</apis>
+							</environmentVariables>
 						</configuration>
 					</execution>
 				</executions>

This generates only PetApi and all associated models. I've looked at the readme for the plugin, and the usage is not clear so I'll get that updated.

@jimschubert jimschubert mentioned this pull request Feb 8, 2020
Merged
5 tasks
@jimschubert
Copy link
Copy Markdown
Member

jimschubert commented Feb 8, 2020

Thanks for submitting this PR and for proposing workarounds here and in #4938. I'm going to close this in favor of #5251, which adds an apisToGenerate property to match the existing modelsToGenerate property. It looks like the environmentVariables documentation was missing and wouldn't have been clear that this option was added for defining systemProperties (or, GlobalSettings). The changes in #5251 should clarify how to do this without workaround and fallbacks.

@jimschubert jimschubert closed this Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jimschubert jimschubert jimschubert left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shybovycha @jimschubert