Skip to content

Commit

Permalink
Validation rule to check if operationId nouns conflict with model nam…
Browse files Browse the repository at this point in the history
…es (#2150)
  • Loading branch information
dsgouda authored Apr 18, 2017
1 parent 204c91d commit 2e1b32f
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 7 deletions.
2 changes: 2 additions & 0 deletions Samples/2a-arm-validation/shell/stderr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ WARNING (BodyParametersValidation/M2063/SDKViolation): A body parameter must be
WARNING (BodyParametersValidation/M2063/SDKViolation): A body parameter must be named 'parameters'.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:382:12 ($.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/regenerateKey"].post.parameters[2].name)
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:381:10 ($.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/regenerateKey"].post.parameters[2])
WARNING (OperationIdNounConflictingModelNames/M2063/SDKViolation): OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'UsageModel'. Consider using the plural form of 'Usage' to avoid this.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:412:8 ($.paths["/subscriptions/{subscriptionId}/providers/Microsoft.Storage/usages"].get.operationId)
WARNING (DescriptionMissing/M4000/SDKViolation): 'StorageAccountCheckNameAvailabilityParameters' model/property lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:437:4 ($.definitions.StorageAccountCheckNameAvailabilityParameters)
WARNING (DescriptionMissing/M4000/SDKViolation): 'StorageAccountPropertiesCreateParameters' model/property lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.
Expand Down
2 changes: 2 additions & 0 deletions Samples/2b-suppressions/shell/stderr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ WARNING (BodyParametersValidation/M2063/SDKViolation): A body parameter must be
WARNING (BodyParametersValidation/M2063/SDKViolation): A body parameter must be named 'parameters'.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:382:12 ($.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/regenerateKey"].post.parameters[2].name)
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:381:10 ($.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/regenerateKey"].post.parameters[2])
WARNING (OperationIdNounConflictingModelNames/M2063/SDKViolation): OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'UsageModel'. Consider using the plural form of 'Usage' to avoid this.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:412:8 ($.paths["/subscriptions/{subscriptionId}/providers/Microsoft.Storage/usages"].get.operationId)
WARNING (DescriptionMissing/M4000/SDKViolation): 'StorageAccountCheckNameAvailabilityParameters' model/property lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:437:4 ($.definitions.StorageAccountCheckNameAvailabilityParameters)
WARNING (DescriptionMissing/M4000/SDKViolation): 'StorageAccountPropertiesCreateParameters' model/property lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.
Expand Down
2 changes: 2 additions & 0 deletions Samples/3b-custom-transformations/shell/stderr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ WARNING (BodyParametersValidation/M2063/SDKViolation): A body parameter must be
WARNING (BodyParametersValidation/M2063/SDKViolation): A body parameter must be named 'parameters'.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:382:12 ($.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/regenerateKey"].post.parameters[2].name)
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:381:10 ($.paths["/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Storage/storageAccounts/{accountName}/regenerateKey"].post.parameters[2])
WARNING (OperationIdNounConflictingModelNames/M2063/SDKViolation): OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'UsageModel'. Consider using the plural form of 'Usage' to avoid this.
- mem:///composite-transform/transform_1.yaml:undefined:undefined ($.paths["/subscriptions/{subscriptionId}/providers/Microsoft.Storage/usages"].get.operationId)
WARNING (DescriptionMissing/M4000/SDKViolation): 'StorageAccountCheckNameAvailabilityParameters' model/property lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.
- https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-storage/2015-06-15/swagger/storage.json:437:4 ($.definitions.StorageAccountCheckNameAvailabilityParameters)
WARNING (DescriptionMissing/M4000/SDKViolation): 'StorageAccountPropertiesCreateParameters' model/property lacks 'description' property. Consider adding a 'description' element. Accurate description is essential for maintaining reference documentation.
Expand Down
11 changes: 10 additions & 1 deletion src/core/AutoRest.Core/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/core/AutoRest.Core/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ Modelers:
<data name="ParametersPropertiesValidation" xml:space="preserve">
<value>Parameter Must have the "name" property defined with non-empty string as its value</value>
</data>
<data name="OperationIdNounConflictingModelNamesMessage" xml:space="preserve">
<value>OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to '{0}Model'. Consider using the plural form of '{1}' to avoid this.</value>
</data>
<data name="PutOperationRequestResponseSchemaMessage" xml:space="preserve">
<value>A PUT operation request body schema should be the same as its 200 response schema, to allow reusing the same entity between GET and PUT. If the schema of the PUT request body is a superset of the GET response body, make sure you have a PATCH operation to make the resource updatable. Operation: '{0}' Request Model: '{1}' Response Model: '{2}'</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
{
"swagger": "2.0",
"info": {
"title": "Microsoft Azure Redis Cache Management API",
"description": "Some cool documentation.",
"version": "2014-04-01-preview"
},
"host": "management.azure.com",
"schemes": [
"https"
],
"basePath": "/",
"produces": [ "application/json" ],
"consumes": [ "application/json" ],
"paths": {
"/foo": {
"get": {
"operationId": "Model_Get",
"description": "gets a model",
"parameters": [
],
"responses": {
"200": {
"schema": {
"$ref": "#/definitions/Model"
}
}
}
}
}
},
"definitions": {
"Model": {
"properties": {
"prop1": {
"type": "string"
}
}
}
},
"parameters": {
"SubscriptionIdParamterer": {
"name": "subscriptionId",
"in": "path",
"description": "Subscription ID.",
"required": true,
"type": "string"
},
"ApiVersionParameter": {
"name": "apiVersion",
"in": "path",
"description": "API ID.",
"required": true,
"type": "string"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
},
"/foo": {
"get": {
"operationId": "Foo_Get",
"operationId": "Foos_Get",
"x-ms-examples": {
"The foo we get": {

Expand All @@ -189,7 +189,7 @@
}
},
"post": {
"operationId": "Foo_Post",
"operationId": "Foos_Post",
"description": "foo",
"x-ms-examples": {
"The foo we post": {
Expand Down Expand Up @@ -221,7 +221,7 @@
"x-ms-paths": {
"/foo?overload=true": {
"post": {
"operationId": "Foo_PostOverload",
"operationId": "Foos_PostOverload",
"description": "foo",
"x-ms-examples": {
"The foo we post": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/Models"
"$ref": "#/definitions/ModelsCollection"
}
}
}
Expand Down Expand Up @@ -100,7 +100,7 @@
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/Models"
"$ref": "#/definitions/ModelsCollection"
}
}
},
Expand Down Expand Up @@ -140,7 +140,7 @@
}
},
"definitions": {
"Models": {
"ModelsCollection": {
"description": "the models to return",
"properties": {
"value": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,13 @@ public void EmptyParameterNameValidation()
messages.AssertOnlyValidationMessage(typeof(ParameterNameValidation), 2);
}

[Fact]
public void OperationIdNounConflictingModelNameValidationTest()
{
var messages = ValidateSwagger(Path.Combine(Core.Utilities.Extensions.CodeBaseDirectory, "Resource", "Swagger", "Validation", "operationid-noun-conflicting-model.json"));
messages.AssertOnlyValidationMessage(typeof(OperationIdNounConflictingModelNames), 1);
}

[Fact]
public void PutRequestResponseBodySchemaValidation()
{
Expand Down
1 change: 1 addition & 0 deletions src/modeler/AutoRest.Swagger/Model/Operation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public Operation()
[Rule(typeof(PutOperationNameValidation))]
[Rule(typeof(PatchOperationNameValidation))]
[Rule(typeof(DeleteOperationNameValidation))]
[Rule(typeof(OperationIdNounConflictingModelNames))]
public string OperationId { get; set; }

public string Summary
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// 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.Linq;
using System.Text.RegularExpressions;
using AutoRest.Core.Logging;
using AutoRest.Core.Properties;
using AutoRest.Swagger.Model;
using AutoRest.Swagger.Validation.Core;

namespace AutoRest.Swagger.Validation
{
public class OperationIdNounConflictingModelNames : TypedRule<string>
{
private readonly Regex NounVerbPattern = new Regex("^(?<noun>\\w+)?_(\\w+)$");

/// <summary>
/// Id of the Rule.
/// </summary>
public override string Id => "M2063";

/// <summary>
/// Violation category of the Rule.
/// </summary>
public override ValidationCategory ValidationCategory => ValidationCategory.SDKViolation;

/// <summary>
/// Check if the noun part of an operationId (Noun_Verb) conflicts with any model names provided in the spec
/// </summary>
/// <param name="entity">The operation id to test</param>
/// <param name="formatParameters">The noun to be put in the failure message</param>
/// <returns></returns>
public override bool IsValid(string entity, RuleContext context, out object[] formatParameters)
{
var match = NounVerbPattern.Match(entity);
formatParameters = new object[] { };

// if operationId does not match the pattern, return true and let some other validation handle it
if (!match.Success)
{
return true;
}

// Get the noun and verb parts of the operation id
var noun = match.Groups["noun"].Value;

var serviceDefinition = (ServiceDefinition)context.Root;
if (serviceDefinition.Definitions.Keys.Any(key => key.ToLower().Equals(noun.ToLower())))
{
formatParameters = new object[] { noun, noun };
return false;
}

return true;
}

/// <summary>
/// The template message for this Rule.
/// </summary>
/// <remarks>
/// This may contain placeholders '{0}' for parameterized messages.
/// </remarks>
public override string MessageTemplate => Resources.OperationIdNounConflictingModelNamesMessage;

/// <summary>
/// The severity of this message (ie, debug/info/warning/error/fatal, etc)
/// </summary>
public override Category Severity => Category.Warning;

}
}

0 comments on commit 2e1b32f

Please sign in to comment.