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

[core] Initial implementation of a validation framework in core #3183

Merged
merged 5 commits into from Jun 24, 2019

Conversation

jimschubert
Copy link
Member

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

This adds a simple validation framework to the core module, which aims to give us a consistent way of applying errors/warnings. I'm opening this to generate some discussion about the usability of it.

I intend to use this in #2946 to pull validations out of CodegenConfigurator into a self-contained and testable type. I also intend to look into applying this to #1086. I could also see this being useful to validate certain conditions we assume to be required for passing data context to templates (see #837).

The goal here is to create a generic validator which can be applied to an OpenAPI Document, allowing us to provide warnings and errors specific to openapi-generator (in addition to those provided by swagger-parser). This has been made generic so, for example, we could apply validations to other spec documents if/when we support them. We could also create an interface which allows a generator to provide conventional typed validations for that generator.

For example:

package org.openapitools.codegen;

import org.openapitools.codegen.validator.Validator;

public interface ValidatableGenerator<T extends CodegenConfig> {
    Validator<T> getValidator();
}

This could be applied to a generator so we can provide errors and warnings prior to generation. Applying this to a generator might look like this:

diff --git a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AspNetCoreServerCodegen.java b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AspNetCoreServerCodegen.java
index 85d6f7a60d..dcaa4aff1e 100644
--- a/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AspNetCoreServerCodegen.java
+++ b/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AspNetCoreServerCodegen.java
@@ -23,6 +23,9 @@ import io.swagger.v3.oas.models.media.Schema;
 import io.swagger.v3.parser.util.SchemaTypeUtil;
 import org.openapitools.codegen.*;
 import org.openapitools.codegen.utils.URLPathUtils;
+import org.openapitools.codegen.validator.GenericValidator;
+import org.openapitools.codegen.validator.ValidationRule;
+import org.openapitools.codegen.validator.Validator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -34,7 +37,7 @@ import java.util.Map;
 
 import static java.util.UUID.randomUUID;
 
-public class AspNetCoreServerCodegen extends AbstractCSharpCodegen {
+public class AspNetCoreServerCodegen extends AbstractCSharpCodegen implements ValidatableGenerator<AspNetCoreServerCodegen> {
 
     public static final String USE_SWASHBUCKLE = "useSwashbuckle";
     public static final String ASPNET_CORE_VERSION = "aspnetCoreVersion";
@@ -52,6 +55,8 @@ public class AspNetCoreServerCodegen extends AbstractCSharpCodegen {
     public static final String COMPATIBILITY_VERSION = "compatibilityVersion";
     public static final String IS_LIBRARY = "isLibrary";
 
+    private GenericValidator<AspNetCoreServerCodegen> validator;
+
     private String packageGuid = "{" + randomUUID().toString().toUpperCase(Locale.ROOT) + "}";
 
     @SuppressWarnings("hiding")
@@ -91,6 +96,15 @@ public class AspNetCoreServerCodegen extends AbstractCSharpCodegen {
 
         cliOptions.clear();
 
+        this.validator = new GenericValidator<>(Arrays.asList(
+            ValidationRule.warn("library + modelClassModifier", "Option 'modelClassModifier' is not supported for library outputs.",
+                    (AspNetCoreServerCodegen i)  -> "library".equals(i.library) && ("" + i.modelClassModifier.getOptValue()).length() > 0
+            ),
+            ValidationRule.warn("abstract classes/methods", "Methods must be abstract when a class is marked abstract",
+                    (AspNetCoreServerCodegen i) -> "abstract".equals(classModifier.getOptValue()) && operationModifier.getOptValue().length() > 0 && !"abstract".equals(operationModifier.getOptValue())
+            )
+        ));
+
         typeMapping.put("boolean", "bool");
         typeMapping.put("integer", "int");
         typeMapping.put("float", "float");
@@ -228,6 +242,11 @@ public class AspNetCoreServerCodegen extends AbstractCSharpCodegen {
         return "Generates an ASP.NET Core Web API server.";
     }
 
+    @Override
+    public Validator<AspNetCoreServerCodegen> getValidator() {
+        return this.validator;
+    }
+
     @Override
     public void preprocessOpenAPI(OpenAPI openAPI) {
         super.preprocessOpenAPI(openAPI);

With the above, we could allow options to treat warnings as errors and fail before writing any files.

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

@auto-labeler
Copy link

auto-labeler bot commented Jun 20, 2019

👍 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
Copy link
Member Author

This replaces #3067. Package name chosen in the other PR/branch caused issues.

@jimschubert jimschubert merged commit 3ee76a0 into OpenAPITools:master Jun 24, 2019
@jimschubert jimschubert deleted the validator-core branch June 24, 2019 22:24
@wing328 wing328 added this to the 4.1.0 milestone Aug 9, 2019
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.

None yet

2 participants