[core] Extracting recommendations to validation framework

[jimschubert]

This is work to extract recommendation logic out of the CLI validate command into a shared evaluation instance which can be used elsewhere (such as Gradle, or the Online tool).

For now, these validations are in addition to those provided by swagger-parser and are only the following recommendations:

• Apache/Nginx warning that header values with underscore are dropped by default
• Unused models/schemas
• Use of properties with oneOf, which is ambiguous in OpenAPI Specification
• GET/HEAD operations defined with body considered an anti-pattern

I've allowed for disabling recommendations via System properties, since this is something that has been requested a few times by users. System properties in this commit include:

• openapi.generator.rule.recommendations=false
  • Allows for disabling recommendations completely. This wouldn't include all warnings and errors, only those we deem to be suggestions
• openapi.generator.rule.apache-nginx-underscore=false
  • Allows for disabling the Apache/Nginx warning when header names have underscore
  • This is a legacy CGI configuration, and doesn't affect all web servers
• openapi.generator.rule.oneof-properties-ambiguity=false
  • We support this functionality, but the specification may not intend for it
  • This is more to reduce noise
• openapi.generator.rule.unused-schemas=false
  • We will warn when a schema is not referenced outside of Components, which users have requested to be able to turn off
• openapi.generator.rule.anti-patterns.uri-unexpected-body=false

This moves validations as suggested in #4412, and prepares for sharing it with other tools in our project.

Example output:

openapi-generator validate -i samples/server/petstore/python-aiohttp/openapi_server/openapi/openapi.yaml --recommend
Validating spec (/Users/jim/projects/openapi-generator/samples/server/petstore/python-aiohttp/openapi_server/openapi/openapi.yaml)
Warnings:
	- Apache and Nginx webservers may fail due to legacy CGI constraints enabled by default in
	  which header keys with underscore are disallowed. See
	  https://stackoverflow.com/a/22856867/151445.
	- Schemas defining properties and oneOf are not clearly defined in the OpenAPI
	  Specification. While our tooling supports this, it may cause issues with other tools.

[info] Spec has 2 recommendation(s).

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.

cc @ybelenko @OpenAPITools/generator-core-team

[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.

[jimschubert]

Looking for feedback before I write out all docs and tests.

[ybelenko]

@jimschubert Thanks for personal invite! 😉

What about #4413? I know that GET request with body isn't supported in php Slim4 and may be supported by other frameworks, but what if we call it Api Antipattern kinda rule? Beside servers there might be client languages or libraries which cannot send GET or DELETE requests with body.

Something like openapi.generator.rule.api-antipattern.get-with-body.

[jimschubert]

@ybelenko good call, added it via 2e3b927 (included GET and HEAD operations).

[jimschubert]

A possible future option would be to reformat how we present recommendations from the validate command. I've captured this in a TODO in this code, but it may not be clear unless someone debugs this branch.

The evaluated rules are captured as Valid or Invalid instances. An Invalid result holds a reference to the rule with generic failure message and (if supported by the rule being evaluated), a more detailed failure message.

For now I've kept the CLI output as the generic failure message because larger specs may become very verbose if we output recommendations at the very detailed level. Plus, I'd like to look further into how we might present the "path" or context within a spec for the suggestion, and I think this will be difficult using a third-party parser. This is lower priority for me, though (focusing on quick win usability type work for both users and contributors).

[ybelenko]

I didn't check PR locally, but I see that both warnings that I need are included. I'm not very skilled with Java, so it looks good to me.

Closes #4217
Closes #4216

[jimschubert]

I've rebased and retargeted to 4.3.x because of my concern about the signature change being a breaking change. Although I don't anticipate people are using the type, it's better to be safe.

If CI passes and there are no other reviews, I'll merge it up to 4.3.x.

[jimschubert]

I've marked this as breaking change without fallback, however I doubt anyone was using the validation framework outside of the codebase and nobody should be affected by the signature changes in this PR.