Skip to content

Commit

Permalink
Adding Swagger file linting infrastructure and rules (Azure#1246)
Browse files Browse the repository at this point in the history
* Modifying settings and command line options

- Adding a ValidationLevel setting to allow users to specify which error levels will fail the program execution (e.g. warning or error)
  - Modifying the Program.cs exit logic to respect this setting when determining if we should return an error code
- Adding a NoOp code generator to allow users to just run validation without generating code, just see output messages.
- Consolidating logic for outputting console messages at the end of program execution
  - Instead of separate Logger methods for outputting each type of message, there is one method that outputs all messages of the specified severity
  - Making the color used for each severity more explicit by putting it in a static dictionary
  - Modifying output logic to respect the ValidationLevel setting. Warnings will be sent to std.err if ValidationLevel: Warning is used

* Adding validation to Modeler.Build() pipeline step

- Adding optional new Build() overload that lets modelers return validation messages that occurred during model building
- Adding validation rule attributes that can decorate models to indicate the rules that should be applied to a property
- Adding Rule class that can be subclassed to provide logic for a validation rule
- Adding ValidationMessage that gets returned by a modeler with information about the validation issue, including name, message and path
- Adding a TypedRule subclass that can provide strong typing for any rules that inherit from it when it validates an entity
- Adding a ValidationExceptionName enum that determines which validation exceptions can be returned by AutoRest
- Adding a ValidationExceptionConstants dictionary to map ValidationExceptionNames to resource strings to output to command line
- Adding a RecursiveObjectValidator that traverses an object graph, runs any rule attributes that decorate the properties of the object class
- Modifying current Modeler classes to implement the Build() overload that returns messages

* Implementing validation rules

    - Adding Rule subclasses for implementing validation logic
    - Decorating Swagger model deserialization classes to specify where to apply validation rules
    - Minor modifications to deserialization classes to consolidate properties from subclasses in their base classes
    - Adding SwaggerModelerValidationTests infrastructure for easily validating that a given swagger file results in the appropriate validation message
    - Adding example swagger files that should result in certain validation warnings or errors
    - Adding SwaggerModelerValidationTests that verify that the swagger files result in the appropriate validation message

* Adding documentation for Swagger linting rules

* Address PR review comments by simplifying linq expressions and making validation rule logic more clear
  • Loading branch information
tbombach committed Jul 11, 2016
1 parent b348fdd commit 0c749da
Show file tree
Hide file tree
Showing 57 changed files with 2,151 additions and 226 deletions.
1 change: 1 addition & 0 deletions Documentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
- Modelers
5. [Building AutoRest](building-code.md)
6. [Writing Tests](writing-tests.md)
6. [Writing Swagger Validation Rules](writing-validation-rules.md)
7. Contributing to the code

[Swagger2.0]:https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md
49 changes: 49 additions & 0 deletions Documentation/writing-validation-rules.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Writing Swagger Validation Rules

## Architecture
In the AutoRest pipeline, Swagger files get deserialized into intermediate classes, which are then used to create the language-independent client model. Our validation logic is performed on these deserialization classes to allow logic written in C# to be used to check the object representation of the Swagger spec.

The root Swagger spec is deserialized into a [`ServiceDefinition`](../src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs) object. The validation step recursively traverses this object tree and applies the validation rules that apply to each property and consolidate the messages from all rules. Validation rules are associated with a property by decorating it with a `RuleAttribute`. This `RuleAttribute` will be passed the value for that property and determines if that value satisfies the rule or not. Multiple `RuleAttribute` attributes can be applied to the same property, and any rules that fail will be part of the output.

## Steps for writing a rule (see [instructions below](#instructions))
1. Define a canonical name that represents the rule and a message that should be shown to the user explaining the validation failure
2. Determine if your rule is an `Info`, a `Warning` or an `Error`
3. Implement the logic that validates this rule against a given object
4. Define where this validation rule gets applied in the object tree
5. Write a test that verifies that this rule correctly validates Swagger specs

## Instructions
### 1. Add the rule name and message
- The name of your validation rule should be added to the end of the `ValidationExceptionName` enum
- Messages are added to the [`AutoRest.Core.Properties.Resource` resx](../src/core/AutoRest.Core/Properties/Resources.resx).

### 2. Specify the severity of your validation rule
- Add a mapping that associates your message with the rule name in [`ValidationExceptionConstants`](../src/core/AutoRest.Core/Validation/ValidationExceptionConstants.cs) in either the `Info`, `Warning` or `Error` sections.

### 3. Add a `Rule` subclass that implements your validation rule logic
- Create a subclass of the `Rule` class, and override the `bool IsValid(object entity)` method.
- For more complex rules (including getting type information in `IsValid()`, see the [Complex rules](#complex-rules) section below.

### 4. Decorate the appropriate Swagger model property that your rule applies to
- Add a `[Rule(typeof({MyRule})]` attribute above the property that should satisfy the rule. Replace `{MyRule}` with the subclass that you implemented.
- The `typeof()` is necessary because C# doesn't support generics in attributes.

### 5. Add a test to `SwaggerModelerValidationTests` that validates your validation rule
- Add an incorrect Swagger file to the [`Swagger/Validation/`](../src/modeler/AutoRest.Swagger.Tests/Swagger/Validation) folder that should trigger your validation rule.
- Add a test case to [`SwaggerModelerValidationTests.cs`](../src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs) that asserts that the validation message returned for the Swagger file is

## Complex rules
### Typed rules
The `IsValid()` method of the `Rule` class only passes an object with no type information. You can have your rule subclass work on a typed model class by inheriting from the `TypedRule<T>` class instead. By replacing `T` with a model class, your override of `IsValid()` will use `T` as the type for the `entity` parameter.

### Message interpolation (e.g. `"'foo' is not a valid MIME type for consumes"`)
Simple rules can simply override the `bool IsValid(object entity)` method when subclassing `Rule` and return true or false, depending on if the object satisfies the rule. However, some messages are more useful if they provide the incorrect value as part of the message.

Rules can override a different `IsValid` overload (`IsValid(object enitity, out object[] formatParameters)`. Any objects returned in `formatParameters` will be passed on to `string.Format()` along with the message associated with the rule. When writing the message, use standard `string.Format()` conventions to define where replacements go (e.g. `"'{0}' is not a valid MIME type for consumes"`).

### Collection rules
Sometimes, a rule should apply to every item in a list or dictionary, but it cannot be applied to the class definition (since the same class can be used in multiple places in the `ServiceDefinition` tree).

An example of this is the `AnonymousTypesDiscouraged` rule. The purpose of this rule is to have schemas defined in the `definitions` section of the Swagger file instead of in the parameter that it will be used for. It validates the `Schema` class, but it cannot be applied to all instances of this class, because the `definitions` section also uses the `Schema` class.

Since we want to apply this rule to parameters in an operation, we can decorate the `Parameters` property of the [`OperationResponse`](../src/modeler/AutoRest.Swagger/Model/Operation.cs) class with the `CollectionRule` attribute. When the object tree is traversed to apply validation rules, each item in the collection will be validated against the `AnonymousParameterTypes` logic.
21 changes: 18 additions & 3 deletions src/core/AutoRest.Core/AutoRest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
using AutoRest.Core.Extensibility;
using AutoRest.Core.Logging;
using AutoRest.Core.Properties;
using AutoRest.Core.Validation;
using System.Collections.Generic;
using System.Linq;

namespace AutoRest.Core
{
Expand All @@ -22,7 +25,7 @@ public static string Version
{
get
{
FileVersionInfo fvi = FileVersionInfo.GetVersionInfo((typeof (Settings)).Assembly.Location);
FileVersionInfo fvi = FileVersionInfo.GetVersionInfo((typeof(Settings)).Assembly.Location);
return fvi.FileVersion;
}
}
Expand All @@ -40,10 +43,22 @@ public static void Generate(Settings settings)
Logger.Entries.Clear();
Logger.LogInfo(Resources.AutoRestCore, Version);
Modeler modeler = ExtensionsLoader.GetModeler(settings);
ServiceClient serviceClient;
ServiceClient serviceClient = null;

try
{
serviceClient = modeler.Build();
IEnumerable<ValidationMessage> messages = new List<ValidationMessage>();
serviceClient = modeler.Build(out messages);

foreach (var message in messages)
{
Logger.Entries.Add(new LogEntry(message.Severity, message.ToString()));
}

if (messages.Any(entry => entry.Severity >= settings.ValidationLevel))
{
throw ErrorManager.CreateError(Resources.CodeGenerationError);
}
}
catch (Exception exception)
{
Expand Down
47 changes: 27 additions & 20 deletions src/core/AutoRest.Core/Extensibility/ExtensionsLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,32 +38,39 @@ public static CodeGenerator GetCodeGenerator(Settings settings)
if (string.IsNullOrEmpty(settings.CodeGenerator))
{
throw new ArgumentException(
string.Format(CultureInfo.InvariantCulture,
string.Format(CultureInfo.InvariantCulture,
Resources.ParameterValueIsMissing, "CodeGenerator"));
}

CodeGenerator codeGenerator = null;

string configurationFile = GetConfigurationFileContent(settings);

if (configurationFile != null)
if (string.Equals("None", settings.CodeGenerator, StringComparison.OrdinalIgnoreCase))
{
try
codeGenerator = new NoOpCodeGenerator(settings);
}
else
{
string configurationFile = GetConfigurationFileContent(settings);

if (configurationFile != null)
{
var config = JsonConvert.DeserializeObject<AutoRestConfiguration>(configurationFile);
codeGenerator = LoadTypeFromAssembly<CodeGenerator>(config.CodeGenerators, settings.CodeGenerator,
settings);
codeGenerator.PopulateSettings(settings.CustomSettings);
try
{
var config = JsonConvert.DeserializeObject<AutoRestConfiguration>(configurationFile);
codeGenerator = LoadTypeFromAssembly<CodeGenerator>(config.CodeGenerators, settings.CodeGenerator,
settings);
codeGenerator.PopulateSettings(settings.CustomSettings);
}
catch (Exception ex)
{
throw ErrorManager.CreateError(ex, Resources.ErrorParsingConfig);
}
}
catch (Exception ex)
else
{
throw ErrorManager.CreateError(ex, Resources.ErrorParsingConfig);
throw ErrorManager.CreateError(Resources.ConfigurationFileNotFound);
}
}
else
{
throw ErrorManager.CreateError(Resources.ConfigurationFileNotFound);
}
Logger.LogInfo(Resources.GeneratorInitialized,
settings.CodeGenerator,
codeGenerator.GetType().Assembly.GetName().Version);
Expand Down Expand Up @@ -137,7 +144,7 @@ public static string GetConfigurationFileContent(Settings settings)

if (!settings.FileSystem.FileExists(path))
{
path = Path.Combine(Path.GetDirectoryName(Assembly.GetAssembly(typeof (Settings)).Location),
path = Path.Combine(Path.GetDirectoryName(Assembly.GetAssembly(typeof(Settings)).Location),
ConfigurationFileName);
}

Expand Down Expand Up @@ -177,10 +184,10 @@ public static string GetConfigurationFileContent(Settings settings)
{
loadedAssembly = Assembly.Load(assemblyName);
}
catch(FileNotFoundException)
catch (FileNotFoundException)
{
loadedAssembly = Assembly.LoadFrom(assemblyName + ".dll");
if(loadedAssembly == null)
if (loadedAssembly == null)
{
throw;
}
Expand All @@ -191,8 +198,8 @@ public static string GetConfigurationFileContent(Settings settings)
t.Name == typeName ||
t.FullName == typeName);

instance = (T) loadedType.GetConstructor(
new[] {typeof (Settings)}).Invoke(new object[] {settings});
instance = (T)loadedType.GetConstructor(
new[] { typeof(Settings) }).Invoke(new object[] { settings });

if (!section[key].Settings.IsNullOrEmpty())
{
Expand Down
35 changes: 35 additions & 0 deletions src/core/AutoRest.Core/Logging/LogEntrySeverityConsoleColor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Globalization;

namespace AutoRest.Core.Logging
{
public static class LogEntrySeverityConsoleColor
{
private static IDictionary<LogEntrySeverity, ConsoleColor> _dict = new Dictionary<LogEntrySeverity, ConsoleColor>
{
{ LogEntrySeverity.Fatal, ConsoleColor.Red },
{ LogEntrySeverity.Error, ConsoleColor.Red },
{ LogEntrySeverity.Warning, ConsoleColor.Yellow },
{ LogEntrySeverity.Info, ConsoleColor.White },
};

/// <summary>
/// Get the console color associated with the severity of the message
/// </summary>
/// <param name="severity">Severity of the log message.</param>
/// <returns>The color to set the console for messages of this severity</returns>
public static ConsoleColor GetColorForSeverity(this LogEntrySeverity severity)
{
ConsoleColor color;
if (!_dict.TryGetValue(severity, out color))
{
throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "No color defined for severity {0}", severity));
}
return color;
}
}
}
64 changes: 12 additions & 52 deletions src/core/AutoRest.Core/Logging/Logger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,68 +90,28 @@ public static void LogError(string message, params object[] args)
LogError(null, message, args);
}

/// <summary>
/// Writes the LogEntry collection to the provided TextWriter.
/// </summary>
/// <param name="writer">TextWriter for output.</param>
/// <param name="verbose">If set to true, output includes full exception stack.</param>
public static void WriteErrors(TextWriter writer, bool verbose)
public static void WriteMessages(TextWriter writer, LogEntrySeverity severity)
{
if (writer == null)
{
throw new ArgumentNullException("writer");
}
foreach (var logEntry in Entries.Where(e => e.Severity == LogEntrySeverity.Error ||
e.Severity == LogEntrySeverity.Fatal)
.OrderByDescending(e => e.Severity))
{
string prefix = "";
if (logEntry.Severity == LogEntrySeverity.Fatal)
{
prefix = "[FATAL] ";
}
writer.WriteLine("error: {0}{1}", prefix, logEntry.Message);
if (logEntry.Exception != null && verbose)
{
writer.WriteLine("{0}", logEntry.Exception);
}
}
WriteMessages(writer, severity, false);
}

/// <summary>
/// Writes the LogEntrySeverity.Warning messages to the provided TextWriter.
/// </summary>
/// <param name="writer">TextWriter for output.</param>
public static void WriteWarnings(TextWriter writer)
public static void WriteMessages(TextWriter writer, LogEntrySeverity severity, bool verbose)
{
if (writer == null)
{
throw new ArgumentNullException("writer");
}
foreach (var logEntry in Entries.Where(e => e.Severity == LogEntrySeverity.Warning))
foreach (var logEntry in Entries.Where(e => e.Severity == severity))
{
writer.WriteLine("{0}: {1}",
logEntry.Severity.ToString().ToUpperInvariant(),
logEntry.Message);
}
}

/// <summary>
/// Writes the LogEntrySeverity.Info messages to the provdied TextWriter.
/// </summary>
/// <param name="writer">TextWriter for output.</param>
public static void WriteInfos(TextWriter writer)
{
if (writer == null)
{
throw new ArgumentNullException("writer");
}

foreach (var logEntry in Entries.Where(e => e.Severity == LogEntrySeverity.Info))
{
writer.WriteLine("{0}: {1}",
logEntry.Severity.ToString().ToUpperInvariant(),
// Write the severity and message to console
writer.WriteLine("{0}: {1}",
logEntry.Severity.ToString().ToUpperInvariant(),
logEntry.Message);
// If verbose is on and the entry has an exception, show it
if (logEntry.Exception != null && verbose)
{
writer.WriteLine("{0}", logEntry.Exception);
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/core/AutoRest.Core/Modeler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

using AutoRest.Core.ClientModel;
using AutoRest.Core.Validation;
using System.Collections.Generic;

namespace AutoRest.Core
{
Expand All @@ -17,5 +19,7 @@ protected Modeler(Settings settings)
}

public abstract ServiceClient Build();

public abstract ServiceClient Build(out IEnumerable<ValidationMessage> messages);
}
}
56 changes: 56 additions & 0 deletions src/core/AutoRest.Core/NoOpCodeGenerator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using AutoRest.Core.ClientModel;
using System.Threading.Tasks;

namespace AutoRest.Core
{
public class NoOpCodeGenerator: CodeGenerator
{
public NoOpCodeGenerator(Settings settings) : base(settings)
{
}

public override string Description
{
get
{
return "No op code generator";
}
}

public override string ImplementationFileExtension
{
get
{
return string.Empty;
}
}

public override string Name
{
get
{
return "No op code generator";
}
}

public override string UsageInstructions
{
get
{
return string.Empty;
}
}

public override Task Generate(ServiceClient serviceClient)
{
return Task.FromResult(0);
}

public override void NormalizeClientModel(ServiceClient serviceClient)
{
}
}
}
Loading

0 comments on commit 0c749da

Please sign in to comment.