diff --git a/src/core/AutoRest.Core/Properties/Resources.Designer.cs b/src/core/AutoRest.Core/Properties/Resources.Designer.cs index cbdfc1a537..1914a67d9f 100644 --- a/src/core/AutoRest.Core/Properties/Resources.Designer.cs +++ b/src/core/AutoRest.Core/Properties/Resources.Designer.cs @@ -419,6 +419,15 @@ public static string OperationIdNounInVerb { } } + /// + /// Looks up a localized string similar to 'GET' operation must use method name 'Get' or Method name start with 'List', 'PUT' operation must use method name 'Create', 'PATCH' operation must use method name 'Update' and 'DELETE' operation must use method name 'Delete'.. + /// + public static string OperationNameNotValid { + get { + return ResourceManager.GetString("OperationNameNotValid", resourceCulture); + } + } + /// /// Looks up a localized string similar to Parameters "subscriptionId" and "api-version" are not allowed in the operations section. /// diff --git a/src/core/AutoRest.Core/Properties/Resources.resx b/src/core/AutoRest.Core/Properties/Resources.resx index c437a15757..097d076661 100644 --- a/src/core/AutoRest.Core/Properties/Resources.resx +++ b/src/core/AutoRest.Core/Properties/Resources.resx @@ -273,4 +273,7 @@ path cannot be null or an empty string or a string with white spaces while getting the parent directory + + 'GET' operation must use method name 'Get' or Method name start with 'List', 'PUT' operation must use method name 'Create', 'PATCH' operation must use method name 'Update' and 'DELETE' operation must use method name 'Delete'. + \ No newline at end of file diff --git a/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/operation-name-not-valid.json b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/operation-name-not-valid.json new file mode 100644 index 0000000000..510094f61a --- /dev/null +++ b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/operation-name-not-valid.json @@ -0,0 +1,85 @@ +{ + "swagger": "2.0", + "info": { + "title": "Operation name not valid", + "description": "Some documentation.", + "version": "2014-04-01-preview" + }, + "host": "management.azure.com", + "schemes": [ + "https" + ], + "basePath": "/", + "produces": [ + "application/json" + ], + "consumes": [ + "application/json" + ], + "paths": { + "/foo": { + "get": { + "operationId": "Noun_NotNamedGet", + "summary": "Foo get path", + "description": "Foo operation", + "responses": { + "default": { + "description": "Unexpected error" + } + } + } + }, + "/foo1": { + "put": { + "operationId": "Noun_NotNamedCreate", + "summary": "Foo2 create path", + "description": "Foo2 operation", + "responses": { + "default": { + "description": "Unexpected error" + } + } + } + }, + "/foo2": { + "delete": { + "operationId": "Noun_NotNamedDelete", + "summary": "Foo2 delete path", + "description": "Foo2 operation", + "responses": { + "default": { + "description": "Unexpected error" + } + } + } + }, + "/foo3": { + "get": { + "operationId": "Noun_List", + "summary": "Foo3 path", + "description": "Foo3 list operation", + "responses": { + "default": { + "description": "Unexpected error" + } + } + } + } + }, + "parameters": { + "SubscriptionIdParameter": { + "name": "subscriptionId", + "in": "path", + "required": true, + "type": "string", + "description": "test subscription id" + }, + "ApiVersion": { + "name": "api-version", + "in": "path", + "required": true, + "type": "string", + "description": "test api version" + } + } +} diff --git a/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/positive/clean-complex-spec.json b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/positive/clean-complex-spec.json index 94d67ed82f..7f7d6ff68a 100644 --- a/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/positive/clean-complex-spec.json +++ b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/positive/clean-complex-spec.json @@ -81,7 +81,7 @@ "/pets/{petId}": { "get": { "summary": "Info for a specific pet", - "operationId": "showPetById", + "operationId": "getPetById", "description": "foo", "tags": [ "pets" diff --git a/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs b/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs index 20f77f6321..c0b66f2898 100644 --- a/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs +++ b/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs @@ -242,6 +242,13 @@ public void Pageable200ResponseNotModeledValidation() var messages = ValidateSwagger(Path.Combine("Swagger", "Validation", "pageable-no-200-response.json")); messages.Any(m => m.Type == typeof(PageableRequires200Response)); } + + [Fact] + public void OperationNameValidation() + { + var messages = ValidateSwagger(Path.Combine("Swagger", "Validation", "operation-name-not-valid.json")); + messages.AssertOnlyValidationMessage(typeof(OperationNameValidation), 3); + } } #region Positive tests diff --git a/src/modeler/AutoRest.Swagger/Model/Operation.cs b/src/modeler/AutoRest.Swagger/Model/Operation.cs index 059573cf0a..43fc366af7 100644 --- a/src/modeler/AutoRest.Swagger/Model/Operation.cs +++ b/src/modeler/AutoRest.Swagger/Model/Operation.cs @@ -36,6 +36,7 @@ public Operation() /// [Rule(typeof(OneUnderscoreInOperationId))] [Rule(typeof(OperationIdNounInVerb))] + [Rule(typeof(OperationNameValidation))] public string OperationId { get; set; } public string Summary diff --git a/src/modeler/AutoRest.Swagger/Validation/OperationNameValidation.cs b/src/modeler/AutoRest.Swagger/Validation/OperationNameValidation.cs new file mode 100644 index 0000000000..d398c1993d --- /dev/null +++ b/src/modeler/AutoRest.Swagger/Validation/OperationNameValidation.cs @@ -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.Collections.Generic; +using AutoRest.Core.Logging; +using AutoRest.Core.Properties; +using AutoRest.Core.Validation; +using AutoRest.Swagger.Model; + +namespace AutoRest.Swagger.Validation +{ + public class OperationNameValidation : TypedRule + { + private const string NOUN_VERB_PATTERN = "^(\\w+)?_(\\w+)$"; + + /// + /// The template message for this Rule. + /// + /// + /// This may contain placeholders '{0}' for parameterized messages. + /// + public override string MessageTemplate => Resources.OperationNameNotValid; + + /// + /// The severity of this message (ie, debug/info/warning/error/fatal, etc) + /// + /// + /// This rule corresponds to M1005, M1006, M1007 & M1009. + /// + public override Category Severity => Category.Warning; + + /// + /// This rule passes if the operation id of HTTP Method confirms to M1005, M1006, M1007 & M1009. + /// e.g. For Get method User_Get or User_List + /// or For Put method User_Create + /// or For Patch method User_Update + /// or For Delete method User_Delete + /// are valid names. + /// + /// Dictionary of the path and respective operations. + /// true if operation name confimes to above rules, otherwise false. + /// + /// Message will be shown at the path level. + /// + public override bool IsValid(string entity, RuleContext context) + { + bool isOperationNameValid = true; + string httpVerb = context?.Parent?.Key; + + if (httpVerb.Equals("GET", StringComparison.InvariantCultureIgnoreCase)) + { + isOperationNameValid = (entity.Contains("_") && (entity.EndsWith("_Get") || entity.Contains("_List"))) || + (entity.StartsWith("get") || entity.StartsWith("list")); + } + else if (httpVerb.Equals("PUT", StringComparison.InvariantCultureIgnoreCase)) + { + isOperationNameValid = (entity.Contains("_") && entity.EndsWith("_Create")) || entity.StartsWith("create"); + } + else if (httpVerb.Equals("PATCH", StringComparison.InvariantCultureIgnoreCase)) + { + isOperationNameValid = (entity.Contains("_") && entity.EndsWith("_Update")) || entity.StartsWith("update"); + } + else if (httpVerb.Equals("DELETE", StringComparison.InvariantCultureIgnoreCase)) + { + isOperationNameValid = (entity.Contains("_") && entity.EndsWith("_Delete")) || entity.StartsWith("update"); + } + + return isOperationNameValid; + } + } +}