Merge pull request #534 from domaindrivendev/required-params

Improved handling for optional/required parameters
domaindrivendev · Nov 14, 2017 · 1c8db8a · 1c8db8a
2 parents 5a52215 + 9aad3d5
commit 1c8db8a
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 31 deletions.
diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/Generator/ApiParameterDescriptionExtensions.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/Generator/ApiParameterDescriptionExtensions.cs
@@ -14,5 +14,11 @@ public static bool IsPartOfCancellationToken(this ApiParameterDescription parame
                 || name == "IsCancellationRequested"
                 || name.StartsWith("WaitHandle.");
         }
+
+        public static bool IsRequired(this ApiParameterDescription parameterDescription)
+        {
+            var modelMetadata = parameterDescription.ModelMetadata;
+            return (modelMetadata == null) ? false : modelMetadata.IsRequired;
+        }
     }
 }
diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/Generator/SwaggerGenerator.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/Generator/SwaggerGenerator.cs
@@ -186,7 +186,7 @@ private Operation CreateOperation(ApiDescription apiDescription, ISchemaRegistry
             {
                 Name = name,
                 In = location,
-                Required = (location == "path")
+                Required = (location == "path") || paramDescription.IsRequired()
             };
 
             if (schema == null)

diff --git a/src/Swashbuckle.AspNetCore.SwaggerUI/Swashbuckle.AspNetCore.SwaggerUI.csproj b/src/Swashbuckle.AspNetCore.SwaggerUI/Swashbuckle.AspNetCore.SwaggerUI.csproj
@@ -22,9 +22,9 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.AspNetCore.Routing" Version="1.0.0" />
-    <PackageReference Include="Microsoft.AspNetCore.StaticFiles" Version="1.0.3" />
-    <PackageReference Include="Microsoft.Extensions.FileProviders.Embedded" Version="1.0.0" />
+    <PackageReference Include="Microsoft.AspNetCore.Routing" Version="1.0.4" />
+    <PackageReference Include="Microsoft.AspNetCore.StaticFiles" Version="1.0.4" />
+    <PackageReference Include="Microsoft.Extensions.FileProviders.Embedded" Version="1.0.1" />
     <PackageReference Include="Newtonsoft.Json" Version="9.0.1" />
   </ItemGroup>
 

diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Generator/SwaggerGeneratorTests.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Generator/SwaggerGeneratorTests.cs
@@ -229,16 +229,34 @@ public void GetSwagger_SetsParameterRequired_ForAllRouteParams(string routeTempl
         }
 
         [Theory]
-        [InlineData(nameof(FakeActions.AcceptsStringFromQuery))]
-        [InlineData(nameof(FakeActions.AcceptsStringFromHeader))]
-        public void GetSwagger_SetsParameterRequiredFalse_ForQueryAndHeaderParams(string actionFixtureName)
+        [InlineData(nameof(FakeActions.AcceptsStringFromQuery), false)]
+        [InlineData(nameof(FakeActions.AcceptsIntegerFromQuery), true)]
+        public void GetSwagger_SetsParameterRequired_ForNonNullableActionParams(
+            string actionFixtureName, bool expectedRequired)
         {
             var subject = Subject(setupApis: apis => apis.Add("GET", "collection", actionFixtureName));
 
             var swagger = subject.GetSwagger("v1");
 
             var param = swagger.Paths["/collection"].Get.Parameters.First();
-            Assert.Equal(false, param.Required);
+            Assert.Equal(expectedRequired, param.Required);
+        }
+
+        [Theory]
+        [InlineData("Property2", false)] // DateTime
+        [InlineData("Property1", true)] // bool
+        [InlineData("Property4", true)] // string with RequiredAttribute
+        public void GetSwagger_SetsParameterRequired_ForNonNullableOrExplicitlyRequiredPropertyBasedParams(
+            string paramName, bool expectedRequired)
+        {
+            var subject = Subject(setupApis: apis => apis
+                .Add("GET", "collection", nameof(FakeActions.AcceptsComplexTypeFromQuery)));
+
+            var swagger = subject.GetSwagger("v1");
+
+            var operation = swagger.Paths["/collection"].Get;
+            var param = swagger.Paths["/collection"].Get.Parameters.First(p => p.Name == paramName);
+            Assert.Equal(expectedRequired, param.Required);
         }
 
         [Fact]
@@ -257,23 +275,6 @@ public void GetSwagger_SetsParameterTypeString_ForUnboundRouteParams()
             Assert.Equal("string", nonBodyParam.Type);
         }
 
-        [Fact]
-        public void GetSwagger_ExpandsOutQueryParameters_ForComplexParamsWithFromQueryAttribute()
-        {
-            var subject = Subject(setupApis: apis => apis
-                .Add("GET", "collection", nameof(FakeActions.AcceptsComplexTypeFromQuery)));
-
-            var swagger = subject.GetSwagger("v1");
-
-            var operation = swagger.Paths["/collection"].Get;
-            Assert.Equal(5, operation.Parameters.Count);
-            Assert.Equal("Property1", operation.Parameters[0].Name);
-            Assert.Equal("Property2", operation.Parameters[1].Name);
-            Assert.Equal("Property3", operation.Parameters[2].Name);
-            Assert.Equal("Property4", operation.Parameters[3].Name);
-            Assert.Equal("Property5", operation.Parameters[4].Name);
-        }
-
         [Fact]
         public void GetSwagger_IgnoresParameters_IfPartOfCancellationToken()
         {

diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/TestFixtures/Fakes/FakeActions.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/TestFixtures/Fakes/FakeActions.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Threading;
+using System.ComponentModel.DataAnnotations;
 using Microsoft.AspNetCore.Mvc;
 using Newtonsoft.Json.Linq;
 
@@ -45,6 +46,9 @@ public void AcceptsStringFromRoute([FromRoute]string param)
         public void AcceptsStringFromQuery([FromQuery]string param)
         { }
 
+        public void AcceptsIntegerFromQuery([FromQuery]int param)
+        { }
+
         public void AcceptsComplexTypeFromQuery([FromQuery]ComplexType param)
         { }
 

diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/TestFixtures/Types/ComplexType.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/TestFixtures/Types/ComplexType.cs
@@ -1,17 +1,19 @@
 using System;
+using System.ComponentModel.DataAnnotations;
 
 namespace Swashbuckle.AspNetCore.SwaggerGen.Test
 {
     public class ComplexType
     {
-        public bool Property1 { get; set; } 
+        public bool Property1 { get; set; }
 
-        public DateTime Property2 { get; set; } 
+        public DateTime? Property2 { get; set; }
 
-        public DateTimeOffset Property3 { get; set; } 
+        public DateTimeOffset Property3 { get; set; }
 
-        public string Property4 { get; set; } 
+        [Required]
+        public string Property4 { get; set; }
 
-        public char Property5 { get; set; } 
+        public char Property5 { get; set; }
     }
 }
diff --git a/test/WebSites/Basic/Controllers/CrudActionsController.cs b/test/WebSites/Basic/Controllers/CrudActionsController.cs
@@ -35,7 +35,7 @@ public int Create([FromBody, Required]Product product)
         /// <param name="keywords">A list of search terms</param>
         /// <returns></returns>
         [HttpGet]
-        public IEnumerable<Product> Search([FromQuery(Name = "kw")]string keywords = null)
+        public IEnumerable<Product> Search([FromQuery(Name = "kw")]string keywords)
         {
             return new[]
             {

diff --git a/test/WebSites/Basic/Controllers/FromQueryParamsController.cs b/test/WebSites/Basic/Controllers/FromQueryParamsController.cs
@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using Microsoft.AspNetCore.Mvc;
+using System.ComponentModel.DataAnnotations;
 
 namespace Basic.Controllers
 {
@@ -24,6 +25,7 @@ public class Address
         /// <summary>
         /// 3-letter ISO country code
         /// </summary>
+        [Required]
         public string Country { get; set; }
 
         /// <summary>