From 4a489e92dfa77f18f182ff9850fd568dd4531140 Mon Sep 17 00:00:00 2001 From: Thomas Bombach Date: Tue, 12 Jul 2016 15:08:46 -0700 Subject: [PATCH] Adding "clean" Swagger validation test (#1254) * Fixing minor validation errors - Fixing description rule to allow references to define a description - Fixing AutoRest logging output to avoid throwing NRE when an error is logged without an exception - Changing the message that gets output for validation errors to make it more useful * Adding a validation test that validates a clean spec - This verifies that rules are not unintentionally being returned for specs that don't have the issue. If a rule is added that causes this test to fail, this spec should be updated so that it passes those rules (and verify that the rule isn't being triggered inadvertently) --- src/core/AutoRest.Core/AutoRest.cs | 2 +- .../AutoRest.Core/Logging/ErrorManager.cs | 2 +- .../Validation/clean-complex-spec.json | 206 ++++++++++++++++++ .../SwaggerModelerValidationTests.cs | 10 + .../Validation/DescriptionRequired.cs | 2 +- 5 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/clean-complex-spec.json diff --git a/src/core/AutoRest.Core/AutoRest.cs b/src/core/AutoRest.Core/AutoRest.cs index a4e4b9bf8a3e1..5d5e003a9abc3 100644 --- a/src/core/AutoRest.Core/AutoRest.cs +++ b/src/core/AutoRest.Core/AutoRest.cs @@ -57,7 +57,7 @@ public static void Generate(Settings settings) if (messages.Any(entry => entry.Severity >= settings.ValidationLevel)) { - throw ErrorManager.CreateError(Resources.CodeGenerationError); + throw ErrorManager.CreateError(null, Resources.ErrorGeneratingClientModel, "Errors found during Swagger validation"); } } catch (Exception exception) diff --git a/src/core/AutoRest.Core/Logging/ErrorManager.cs b/src/core/AutoRest.Core/Logging/ErrorManager.cs index 674ef1a1ce020..9e57bbb515853 100644 --- a/src/core/AutoRest.Core/Logging/ErrorManager.cs +++ b/src/core/AutoRest.Core/Logging/ErrorManager.cs @@ -30,7 +30,7 @@ public static CodeGenerationException CreateError(Exception exception, string me } var errors = - Logger.Entries.Where(e => e.Severity == LogEntrySeverity.Error).Select(e => e.Exception).ToList(); + Logger.Entries.Where(e => e.Severity == LogEntrySeverity.Error).Select(e => e.Exception).Where(e => e != null).ToList(); Logger.Entries.Add(new LogEntry(LogEntrySeverity.Fatal, FormatMessageString(message, args)) { Exception = exception diff --git a/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/clean-complex-spec.json b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/clean-complex-spec.json new file mode 100644 index 0000000000000..612ddec72b7b2 --- /dev/null +++ b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/clean-complex-spec.json @@ -0,0 +1,206 @@ +{ + "swagger": "2.0", + "info": { + "version": "1.0.0", + "title": "Swagger Petstore", + "license": { + "name": "MIT" + } + }, + "host": "petstore.swagger.io", + "basePath": "/v1", + "schemes": [ + "http" + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "paths": { + "/pets": { + "get": { + "summary": "List all pets", + "operationId": "listPets", + "tags": [ + "pets" + ], + "parameters": [ + { + "name": "limit", + "in": "query", + "description": "How many items to return at one time (max 100)", + "required": false, + "type": "integer", + "format": "int32" + } + ], + "responses": { + "200": { + "description": "An paged array of pets", + "headers": { + "x-next": { + "type": "string", + "description": "A link to the next page of responses" + } + }, + "schema": { + "$ref": "#/definitions/Pets" + } + }, + "default": { + "description": "unexpected error", + "schema": { + "$ref": "#/definitions/Error" + } + } + } + }, + "post": { + "summary": "Create a pet", + "operationId": "createPets", + "tags": [ + "pets" + ], + "responses": { + "201": { + "description": "Null response" + }, + "default": { + "description": "unexpected error", + "schema": { + "$ref": "#/definitions/Error" + } + } + } + } + }, + "/pets/{petId}": { + "get": { + "summary": "Info for a specific pet", + "operationId": "showPetById", + "tags": [ + "pets" + ], + "parameters": [ + { + "name": "petId", + "in": "path", + "required": true, + "description": "The id of the pet to retrieve", + "type": "string" + } + ], + "responses": { + "200": { + "description": "Expected response to a valid request", + "schema": { + "$ref": "#/definitions/Pets" + } + }, + "default": { + "description": "unexpected error", + "schema": { + "$ref": "#/definitions/Error" + } + } + } + } + }, + "/foo": { + "get": { + "operationId": "Foo_Get", + "responses": { + "default": { + "$ref": "#/responses/FooResponse" + } + } + }, + "post": { + "operationId": "Foo_Post", + "parameters": [ + { + "in": "body", + "name": "fooPost", + "schema": { + "type": "object", + "description": "A foo object" + }, + "description": "Foo body parameter" + }, + { + "$ref": "#/parameters/FooQueryParam" + } + ], + "responses": { + "default": { + "$ref": "#/responses/FooResponse" + } + } + } + } + }, + "parameters": { + "FooQueryParam": { + "in": "query", + "name": "FooQueryParam", + "description": "Query parameter for Foo operation", + "type": "string" + } + }, + "responses": { + "FooResponse": { + "description": "Response for Foo" + } + }, + "definitions": { + "Pet": { + "required": [ + "id", + "name" + ], + "properties": { + "id": { + "type": "integer", + "format": "int64", + "description": "The pet id" + }, + "name": { + "type": "string", + "description": "The pet name" + }, + "tag": { + "type": "string", + "description": "The pet tag" + } + }, + "description": "A pet" + }, + "Pets": { + "type": "array", + "items": { + "$ref": "#/definitions/Pet" + }, + "description": "A set of pets" + }, + "Error": { + "required": [ + "code", + "message" + ], + "properties": { + "code": { + "type": "integer", + "format": "int32", + "description": "The code of the error" + }, + "message": { + "type": "string", + "description": "The message of the error" + } + }, + "description": "An error result" + } + } +} \ No newline at end of file diff --git a/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs b/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs index a5a46fecdb6b1..141a72fbf0243 100644 --- a/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs +++ b/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs @@ -40,6 +40,16 @@ private IEnumerable ValidateSwagger(string input) return messages; } + /// + /// Verifies that a clean Swagger file does not result in any validation errors + /// + [Fact] + public void CleanFileValidation() + { + var messages = ValidateSwagger(Path.Combine("Swagger", "Validation", "clean-complex-spec.json")); + Assert.Empty(messages.Where(m => m.Severity >= LogEntrySeverity.Warning)); + } + [Fact] public void MissingDescriptionValidation() { diff --git a/src/modeler/AutoRest.Swagger/Validation/DescriptionRequired.cs b/src/modeler/AutoRest.Swagger/Validation/DescriptionRequired.cs index c27277f80a1c0..f8174db0b7fa8 100644 --- a/src/modeler/AutoRest.Swagger/Validation/DescriptionRequired.cs +++ b/src/modeler/AutoRest.Swagger/Validation/DescriptionRequired.cs @@ -14,7 +14,7 @@ public class DescriptionRequired : TypedRule /// /// public override bool IsValid(SwaggerObject entity) - => entity == null || entity.Description != null || string.IsNullOrEmpty(entity.Reference); + => entity == null || entity.Description != null || !string.IsNullOrEmpty(entity.Reference); public override ValidationExceptionName Exception => ValidationExceptionName.DescriptionRequired; }