Bug 300: When an operationId has already been used; append a prefix t…

…o get a unique one
domaindrivendev · May 10, 2016 · 3358bfe · 3358bfe
1 parent 07a7a24
commit 3358bfe
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 11 deletions.
diff --git a/Swashbuckle.Core/Swagger/SwaggerGenerator.cs b/Swashbuckle.Core/Swagger/SwaggerGenerator.cs
@@ -45,11 +45,12 @@ public SwaggerDocument GetSwagger(string rootUrl, string apiVersion)
             if (info == null)
                 throw new UnknownApiVersion(apiVersion);
 
+            HashSet<string> operationNames = new HashSet<string>();
             var paths = GetApiDescriptionsFor(apiVersion)
                 .Where(apiDesc => !(_options.IgnoreObsoleteActions && apiDesc.IsObsolete()))
                 .OrderBy(_options.GroupingKeySelector, _options.GroupingKeyComparer)
                 .GroupBy(apiDesc => apiDesc.RelativePathSansQueryString())
-                .ToDictionary(group => "/" + group.Key, group => CreatePathItem(group, schemaRegistry));
+                .ToDictionary(group => "/" + group.Key, group => CreatePathItem(group, schemaRegistry, operationNames));
 
             var rootUri = new Uri(rootUrl);
             var port = (!rootUri.IsDefaultPort) ? ":" + rootUri.Port : string.Empty;
@@ -80,7 +81,7 @@ private IEnumerable<ApiDescription> GetApiDescriptionsFor(string apiVersion)
                 : _apiExplorer.ApiDescriptions.Where(apiDesc => _options.VersionSupportResolver(apiDesc, apiVersion));
         }
 
-        private PathItem CreatePathItem(IEnumerable<ApiDescription> apiDescriptions, SchemaRegistry schemaRegistry)
+        private PathItem CreatePathItem(IEnumerable<ApiDescription> apiDescriptions, SchemaRegistry schemaRegistry, HashSet<string> operationNames)
         {
             var pathItem = new PathItem();
 
@@ -99,33 +100,33 @@ private PathItem CreatePathItem(IEnumerable<ApiDescription> apiDescriptions, Sch
                 switch (httpMethod)
                 {
                     case "get":
-                        pathItem.get = CreateOperation(apiDescription, schemaRegistry);
+                        pathItem.get = CreateOperation(apiDescription, schemaRegistry, operationNames);
                         break;
                     case "put":
-                        pathItem.put = CreateOperation(apiDescription, schemaRegistry);
+                        pathItem.put = CreateOperation(apiDescription, schemaRegistry, operationNames);
                         break;
                     case "post":
-                        pathItem.post = CreateOperation(apiDescription, schemaRegistry);
+                        pathItem.post = CreateOperation(apiDescription, schemaRegistry, operationNames);
                         break;
                     case "delete":
-                        pathItem.delete = CreateOperation(apiDescription, schemaRegistry);
+                        pathItem.delete = CreateOperation(apiDescription, schemaRegistry, operationNames);
                         break;
                     case "options":
-                        pathItem.options = CreateOperation(apiDescription, schemaRegistry);
+                        pathItem.options = CreateOperation(apiDescription, schemaRegistry, operationNames);
                         break;
                     case "head":
-                        pathItem.head = CreateOperation(apiDescription, schemaRegistry);
+                        pathItem.head = CreateOperation(apiDescription, schemaRegistry, operationNames);
                         break;
                     case "patch":
-                        pathItem.patch = CreateOperation(apiDescription, schemaRegistry);
+                        pathItem.patch = CreateOperation(apiDescription, schemaRegistry, operationNames);
                         break;
                 }
             }
 
             return pathItem;
         }
 
-        private Operation CreateOperation(ApiDescription apiDesc, SchemaRegistry schemaRegistry)
+        private Operation CreateOperation(ApiDescription apiDesc, SchemaRegistry schemaRegistry, HashSet<string> operationNames)
         {
             var parameters = apiDesc.ParameterDescriptions
                 .Select(paramDesc =>
@@ -145,7 +146,7 @@ private Operation CreateOperation(ApiDescription apiDesc, SchemaRegistry schemaR
             var operation = new Operation
             { 
                 tags = new [] { _options.GroupingKeySelector(apiDesc) },
-                operationId = apiDesc.FriendlyId(),
+                operationId = this.GetUniqueFriendlyId(apiDesc, operationNames),
                 produces = apiDesc.Produces().ToList(),
                 consumes = apiDesc.Consumes().ToList(),
                 parameters = parameters.Any() ? parameters : null, // parameters can be null but not empty
@@ -161,6 +162,18 @@ private Operation CreateOperation(ApiDescription apiDesc, SchemaRegistry schemaR
             return operation;
         }
 
+        private string GetUniqueFriendlyId(ApiDescription apiDesc, HashSet<string> operationNames)
+        {
+            string friendlyId = apiDesc.FriendlyId();
+            int nextFriendlyIdPostfix = 1;
+            while (operationNames.Contains(friendlyId))
+            {
+                friendlyId = string.Format("{0}_{1}", apiDesc.FriendlyId(), nextFriendlyIdPostfix);
+            }
+            operationNames.Add(friendlyId);
+            return friendlyId;
+        }
+
         private string GetParameterLocation(ApiDescription apiDesc, ApiParameterDescription paramDesc)
         {
             if (apiDesc.RelativePathSansQueryString().Contains("{" + paramDesc.Name + "}"))

diff --git a/Swashbuckle.Dummy.Core/Controllers/OverloadedAttributeRoutesController.cs b/Swashbuckle.Dummy.Core/Controllers/OverloadedAttributeRoutesController.cs
@@ -0,0 +1,21 @@
+using System;
+using System.Collections.Generic;
+using System.Web.Http;
+
+namespace Swashbuckle.Dummy.Controllers
+{
+    public class OverloadedAttributeRoutesController : ApiController
+    {
+        [Route("subscriptions/")]
+        public void GetAll()
+        {
+            throw new NotImplementedException();
+        }
+
+        [Route("users/")]
+        public void GetAll(int id)
+        {
+            throw new NotImplementedException();
+        }
+    }
+}
diff --git a/Swashbuckle.Dummy.Core/Swashbuckle.Dummy.Core.csproj b/Swashbuckle.Dummy.Core/Swashbuckle.Dummy.Core.csproj
@@ -65,6 +65,7 @@
   <ItemGroup>
     <Compile Include="App_Start\CachingSwaggerProvider.cs" />
     <Compile Include="App_Start\CustomCorsPolicyProvider.cs" />
+    <Compile Include="Controllers\OverloadedAttributeRoutesController.cs" />
     <Compile Include="Controllers\ConflictingTypesController.cs" />
     <Compile Include="Controllers\DataContractAnnotatedTypesController.cs" />
     <Compile Include="Controllers\FileDownloadController.cs" />

diff --git a/Swashbuckle.Tests/Swagger/CoreTests.cs b/Swashbuckle.Tests/Swagger/CoreTests.cs
@@ -548,6 +548,18 @@ public void It_handles_attribute_routes()
             Assert.IsNotNull(path);
         }
 
+        [Test]
+        public void It_handles_overloaded_attribute_routes()
+        {
+            SetUpAttributeRoutesFrom(typeof(OverloadedAttributeRoutesController).Assembly);
+
+            var swagger = GetContent<JObject>("http://tempuri.org/swagger/docs/v1");
+            var firstGetOperation = swagger["paths"]["/subscriptions"]["get"];
+            var secondGetOperation = swagger["paths"]["/users"]["get"];
+            Assert.AreEqual("OverloadedAttributeRoutes_GetAll", firstGetOperation["operationId"].ToString());
+            Assert.AreEqual("OverloadedAttributeRoutes_GetAll_1", secondGetOperation["operationId"].ToString());
+        }
+
         [Test]
         public void It_handles_json_formatter_not_present()
         {