Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate spec on generation by default #251

Merged
merged 10 commits into from
Jul 26, 2018

Conversation

jimschubert
Copy link
Member

@jimschubert jimschubert commented Jun 8, 2018

Adds a validation parameter to CodegenConfigurator, and passes through
options from CLI, Maven Plugin and Gradle Plugin to that property.

Default is to validate the spec during generation. If spec has errors,
we will output errors as well as warnings to the user.

Option can be disabled by passing false to skipValidateSpec (Maven/Gradle)
or --skip-validate-spec (CLI).

This was brought up in #208. We want to validate the spec on generation to prevent OpenAPI Spec 3.x options from creeping into OpenAPI Spec 2.0 support. I would also consider this a bug fix, because as a user I would be surprised to find that generation succeeds when my spec is invalid by default.

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 and ./bin/security/{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\.
  • Filed the PR against the correct branch: Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Adds a validation parameter to CodegenConfigurator, and passes through
options from CLI, Maven Plugin and Gradle Plugin to that property.

Default is to validate the spec during generation. If spec has errors,
we will output errors as well as warnings to the user.

Option can be disabled by passing false to validateSpec (Maven/Gradle)
or --validate-spec (CLI).
@etherealjoy
Copy link
Contributor

@jimschubert Can we run a validate without generate?

@jimschubert
Copy link
Member Author

@etherealjoy the CLI has a validate command. Our Homebrew formula can't be merged until the repo is 30 days old (3 days from now). I created a script that you can use to run the last snapshot: https://gist.github.com/jimschubert/ce241b0c78140e364f46914ef8ec4103

@jimschubert
Copy link
Member Author

jimschubert commented Jun 10, 2018

see related #142

* master: (26 commits)
  Prepare 3.0.1 release (OpenAPITools#280)
  [typescript-angular] strict type checking (OpenAPITools#218)
  [C++ server] Adjust the names (script, sample folder, generator) to lang option (OpenAPITools#250)
  Removed warnings for packages included in SDK for Net Core 2.0 (OpenAPITools#269)
  [cli] Completions command for suggestions (OpenAPITools#213)
  [Java][RestTemplate] Fix query parameter URL encoding (OpenAPITools#260)
  [cpp-qt5] Remove std::shared_ptr from Qt5 (OpenAPITools#267)
  Adds some links to the README (OpenAPITools#261)
  Update sec.gpg.enc to binary encoded secret
  Add gpg --check-trustdb, limit gpg to master
  Re-do encrypted gpg and reference in settings.xml
  Fix trailing semicolons in after_success Travis CI scripts
  Use ubuntu keyserver instead of mit (due to timeout)
  [gradle] Plugin release management (OpenAPITools#201)
  Updates small typo in qna.md (OpenAPITools#262)
  Fix ModelUtils.getUnusedSchema() (OpenAPITools#253)
  Add JaxRS to bin/ensure-up-to-date (OpenAPITools#248)
  feat(security): add cookie-auth support (OpenAPITools#240)
  Add 'unblu inc.' to company list (OpenAPITools#246)
  put company list in alphabetical order (OpenAPITools#244)
  ...
@jimschubert
Copy link
Member Author

@wing328 do you have any concerns if we merge this?

@wing328
Copy link
Member

wing328 commented Jun 21, 2018

I did some tests locally but got errors with --validate-spec=false:

➜  openapi-generator git:(jimschubert-generator-validation-option) java -jar "./modules/openapi-generator-cli/target/openapi-generator-cli.jar" generate -i /var/tmp/mapissue.yaml -g aspnetcore  -o /tmp/sam --validate-spec=false
 
Exception in thread "main" io.airlift.airline.ParseArgumentsUnexpectedException: Found unexpected parameters: [--validate-spec=false]
	at io.airlift.airline.Cli.validate(Cli.java:148)
	at io.airlift.airline.Cli.parse(Cli.java:116)
	at io.airlift.airline.Cli.parse(Cli.java:97)
	at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:58)
➜  openapi-generator git:(jimschubert-generator-validation-option) 

Am I doing something wrong here? (clearly I don't have enough sleep these days...)

@ackintosh
Copy link
Contributor

ackintosh commented Jun 27, 2018

Option can be disabled by passing false to validateSpec (Maven/Gradle)
or --validate-spec (CLI).

--validate-spec is Boolean so it doesn't allow argument and should behave like below 💡 :

  • default (omitted): false
  • specified: true

How about --skip-validation ?

jmini and others added 7 commits July 6, 2018 17:40
* master: (116 commits)
  3.1.0 release (OpenAPITools#486)
  Fix broken link to openapi generator plugin README (OpenAPITools#484)
  show warning message for nodejs server only (OpenAPITools#481)
  Add grokify to Go technical committee (OpenAPITools#479)
  [Slim] Improve codebase decouple (OpenAPITools#438)
  Ensure typescript samples are up to date (OpenAPITools#444)
  Update README.md
  [Golang][client] delete sample output dir before rebuild (OpenAPITools#477)
  update petstore samples (OpenAPITools#478)
  Revert "Improve Docker Tags (OpenAPITools#390)"
  update go client test dependencies (OpenAPITools#468)
  [Golang][client] fix for schema definition name `file` (OpenAPITools#433)
  Fix '.travis' file (syntax)
  make LICENSE GitHub display compatible (OpenAPITools#467)
  Improve Docker Tags (OpenAPITools#390)
  [Golang][client] fix file suffix for _test.go (OpenAPITools#449)
  Remove copy section (OpenAPITools#463)
  Add link to presentation (OpenAPITools#465)
  Use postProcessOperationsWithModels(Map, List) (OpenAPITools#431)
  [C] Adding petstore sample written in C (OpenAPITools#306)
  ...
…dation-option

* origin/prepare_311:
  Update README.md
  Use last prod version for the sample
  fix version
  Prepare version 3.1.1-SNAPSHOT
@jimschubert
Copy link
Member Author

@ackintosh great suggestion. Having a true-only switch really only makes sense on the CLI because the option parser doesn't support full range of boolean inputs. This means the CLI will have --skip-validate-spec which takes no value, while Maven and Gradle will have validateSpec which can take true or false. I think this is fine to have a slight difference between the two.

CLI Example

CLI: --skip-validate-spec warns about spec errors being ignored, but generates anyway.

$ java -jar "./modules/openapi-generator-cli/target/openapi-generator-cli.jar" generate -i modules/openapi-generator-gradle-plugin/samples/local-spec/petstore-v3.0-invalid.yaml -g aspnetcore  -o /tmp/sam --skip-validate-spec
[main] WARN  o.o.c.config.CodegenConfigurator - There were issues with the specification, but validation has been explicitly disabled.
Errors:
	-attribute info is missing

[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Models/Error.cs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Models/Pet.cs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Models/Pets.cs
[main] WARN  o.o.codegen.DefaultCodegen - Empty operationId found for path: post /pets. Renamed to auto-generated operationId: petsPost
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Controllers/PetsApi.cs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/build.sh
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/build.bat
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/README.md
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/Org.OpenAPITools.sln
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Dockerfile
[main] INFO  o.o.codegen.DefaultGenerator - writing file /tmp/sam/src/Org.OpenAPITools/.gitignore
[main] INFO  o.o.codegen.DefaultGenerator - writing file /tmp/sam/src/Org.OpenAPITools/appsettings.json
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Startup.cs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Program.cs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Attributes/ValidateModelStateAttribute.cs
[main] INFO  o.o.codegen.DefaultGenerator - writing file /tmp/sam/src/Org.OpenAPITools/web.config
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Org.OpenAPITools.csproj
[main] INFO  o.o.codegen.DefaultGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Properties/launchSettings.json
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Filters/BasePathFilter.cs
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/Filters/GeneratePathParamsValidationFilter.cs
[main] INFO  o.o.codegen.DefaultGenerator - writing file /tmp/sam/src/Org.OpenAPITools/wwwroot/README.md
[main] INFO  o.o.codegen.DefaultGenerator - writing file /tmp/sam/src/Org.OpenAPITools/wwwroot/index.html
[main] INFO  o.o.codegen.DefaultGenerator - writing file /tmp/sam/src/Org.OpenAPITools/wwwroot/web.config
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/src/Org.OpenAPITools/wwwroot/openapi-original.json
[main] INFO  o.o.codegen.AbstractGenerator - writing file /tmp/sam/.openapi-generator/VERSION
jim at engineer in ~/projects/openapi-generator on generator-validation-option* fcc1591e6

Default: fails on invalid spec

$ java -jar "./modules/openapi-generator-cli/target/openapi-generator-cli.jar" generate -i modules/openapi-generator-gradle-plugin/samples/local-spec/petstore-v3.0-invalid.yaml -g aspnetcore  -o /tmp/sam
Exception in thread "main" org.openapitools.codegen.SpecValidationException: Specification has failed validation. | Error count: 1, Warning count: 0
Errors:
	-attribute info is missing

	at org.openapitools.codegen.config.CodegenConfigurator.toClientOptInput(CodegenConfigurator.java:538)
	at org.openapitools.codegen.cmd.Generate.run(Generate.java:323)
	at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:58)

Maven Example

$ mvn -f non-java-invalid-spec.xml compile
[INFO] Scanning for projects...
[INFO]
[INFO] ------------------< org.openapitools:sample-project >-------------------
[INFO] Building sample-project 1.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- openapi-generator-maven-plugin:3.1.1-SNAPSHOT:generate (default) @ sample-project ---
[WARNING] There were issues with the specification, but validation has been explicitly disabled.
Errors:
	-attribute info is missing

[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Models/Error.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Models/Pet.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Models/Pets.cs
[WARNING] Empty operationId found for path: post /pets. Renamed to auto-generated operationId: petsPost
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Controllers/PetsApi.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/build.sh
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/build.bat
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/README.md
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/Org.OpenAPITools.sln
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Dockerfile
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/.gitignore
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/appsettings.json
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Startup.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Program.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Attributes/ValidateModelStateAttribute.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/web.config
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Org.OpenAPITools.csproj
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Properties/launchSettings.json
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Filters/BasePathFilter.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/Filters/GeneratePathParamsValidationFilter.cs
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/wwwroot/README.md
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/wwwroot/index.html
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/wwwroot/web.config
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/src/Org.OpenAPITools/wwwroot/openapi-original.json
[INFO] writing file /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/target/generated-sources/openapi/.openapi-generator/VERSION
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ sample-project ---
[WARNING] Using platform encoding (UTF-8 actually) to copy filtered resources, i.e. build is platform dependent!
[INFO] skip non existing resourceDirectory /Users/jim/projects/openapi-generator/modules/openapi-generator-maven-plugin/examples/src/main/resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ sample-project ---
[INFO] No sources to compile
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.776 s
[INFO] Finished at: 2018-07-06T23:08:10-04:00
[INFO] ------------------------------------------------------------------------

Modify non-java-invalid-spec.xml with <validateSpec>true</validateSpec>:

$ mvn -f non-java-invalid-spec.xml compile
[INFO] Scanning for projects...
[INFO]
[INFO] ------------------< org.openapitools:sample-project >-------------------
[INFO] Building sample-project 1.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- openapi-generator-maven-plugin:3.1.1-SNAPSHOT:generate (default) @ sample-project ---
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.125 s
[INFO] Finished at: 2018-07-06T23:09:10-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.openapitools:openapi-generator-maven-plugin:3.1.1-SNAPSHOT:generate (default) on project sample-project: Execution default of goal org.openapitools:openapi-generator-maven-plugin:3.1.1-SNAPSHOT:generate failed: Specification has failed validation. | Error count: 1, Warning count: 0
[ERROR] Errors:
[ERROR] 	-attribute info is missing
[ERROR]
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException

Gradle Plugin

Example, meant to fail:

$ gradle -PopenApiGeneratorVersion=3.1.1-SNAPSHOT generateGoWithInvalidSpec
> Task :generateGoWithInvalidSpec FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':generateGoWithInvalidSpec'.
> Specification has failed validation. | Error count: 1, Warning count: 0
  Errors:
        -attribute info is missing


* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 0s
1 actionable task: 1 executed

Modify build.gradle in the samples directory to say validateSpec = false for this task; only a warning is displayed:

$ gradle -PopenApiGeneratorVersion=3.1.1-SNAPSHOT -PvalidateSpec=false generateGoWithInvalidSpec

> Task :generateGoWithInvalidSpec
There were issues with the specification, but validation has been explicitly disabled.
Errors:
        -attribute info is missing

Empty operationId found for path: post /pets. Renamed to auto-generated operationId: petsPost
Successfully generated code to /Users/jim/projects/openapi-generator/modules/openapi-generator-gradle-plugin/samples/local-spec/build/go

BUILD SUCCESSFUL in 0s
1 actionable task: 1 executed

@wing328 this should be ready to re-evaluate. Sorry for the confusion with that CLI parameter. I know I had tested it, but I don't recall what I did because it was so long ago. Maybe I had originally implemented it as a String rather than a Boolean property (that's all I can think of).

@jimschubert
Copy link
Member Author

Also, I had to merge #487 into this PR so it would run and so it could be evaluated. I didn't merge #487 because it looked like there might still be validates on the latest release taking place. Please make sure to merge this PR only after #487 has been merged.

* master:
  [CLI] Add --generator-name / -g to config-help (OpenAPITools#491)
  Update README.md (OpenAPITools#493)
  Update README.md (OpenAPITools#488)
  Prepare version 3.1.1-SNAPSHOT (OpenAPITools#487)
@wing328 wing328 changed the base branch from master to 3.2.x July 25, 2018 12:38
@jmini jmini changed the base branch from 3.2.x to master July 25, 2018 20:12
@jmini
Copy link
Member

jmini commented Jul 25, 2018

Changed base branch: 3.2.0-SNAPSHOT is now on master branch

@jimschubert
Copy link
Member Author

Thanks.
What's blocking this getting merged?

@wing328
Copy link
Member

wing328 commented Jul 26, 2018

The AppVeyor test passed via https://ci.appveyor.com/project/WilliamCheng/openapi-generator/build/jimschubert-generator-validation-option-1682.

Shippable CI failure was not repeatable locally. Let's merge this to see if the Shippable build still occurs.

@wing328 wing328 merged commit 77df3d6 into OpenAPITools:master Jul 26, 2018
@wing328 wing328 added this to the 3.2.0 milestone Jul 26, 2018
@wing328
Copy link
Member

wing328 commented Jul 26, 2018

Upgrade Note

The default is to validate the spec during generation. If the spec has errors,
we will output errors as well as warnings to the user.

The option can be disabled by passing false to validateSpec (Maven/Gradle)
or --skip-validate-spec (CLI).

@jmini
Copy link
Member

jmini commented Jul 30, 2018

I think the note belongs to the migration-guide:
See #683

@wing328: I think the option for Maven/Gradle is called validateSpec...

@wing328
Copy link
Member

wing328 commented Jul 30, 2018

@jmini you're right. Just updated the note above.

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
* Validate spec on generation by default

Adds a validation parameter to CodegenConfigurator, and passes through
options from CLI, Maven Plugin and Gradle Plugin to that property.

Default is to validate the spec during generation. If spec has errors,
we will output errors as well as warnings to the user.

Option can be disabled by passing false to validateSpec (Maven/Gradle)
or --validate-spec (CLI).

* Prepare version 3.1.1-SNAPSHOT

* fix version

* Use last prod version for the sample

* Update README.md

Fix

* [cli] Option parser does not support true/false for boolean options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants