Allow ProducesAttribute to apply along with conventions

Fixes #8389

src/Microsoft.AspNetCore.Mvc.ApiExplorer/ApiResponseTypeProvider.cs

@@ -38,12 +38,12 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer
             var runtimeReturnType = GetRuntimeReturnType(declaredReturnType);
 
             var responseMetadataAttributes = GetResponseMetadataAttributes(action);
-            if (responseMetadataAttributes.Count == 0 &&
+            if (!HasSignificantMetadataProvider(responseMetadataAttributes) &&
                 action.Properties.TryGetValue(typeof(ApiConventionResult), out var result))
             {
                 // Action does not have any conventions. Use conventions on it if present.
                 var apiConventionResult = (ApiConventionResult)result;
-                responseMetadataAttributes = apiConventionResult.ResponseMetadataProviders;
+                responseMetadataAttributes.AddRange(apiConventionResult.ResponseMetadataProviders);
             }
 
             var defaultErrorType = typeof(void);
@@ -56,11 +56,11 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer
             return apiResponseTypes;
         }
 
-        private IReadOnlyList<IApiResponseMetadataProvider> GetResponseMetadataAttributes(ControllerActionDescriptor action)
+        private static List<IApiResponseMetadataProvider> GetResponseMetadataAttributes(ControllerActionDescriptor action)
         {
             if (action.FilterDescriptors == null)
             {
-                return Array.Empty<IApiResponseMetadataProvider>();
+                return new List<IApiResponseMetadataProvider>();
             }
 
             // This technique for enumerating filters will intentionally ignore any filter that is an IFilterFactory
@@ -70,7 +70,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer
             return action.FilterDescriptors
                 .Select(fd => fd.Filter)
                 .OfType<IApiResponseMetadataProvider>()
-                .ToArray();
+                .ToList();
         }
 
         private ICollection<ApiResponseType> GetApiResponseTypes(
@@ -188,7 +188,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer
             }
 
             // Unwrap the type if it's a Task<T>. The Task (non-generic) case was already handled.
-            Type unwrappedType = declaredReturnType;
+            var unwrappedType = declaredReturnType;
             if (declaredReturnType.IsGenericType &&
                 declaredReturnType.GetGenericTypeDefinition() == typeof(Task<>))
             {
@@ -228,5 +228,24 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer
         {
             return statusCode >= 400 && statusCode < 500;
         }
+
+        private static bool HasSignificantMetadataProvider(IReadOnlyList<IApiResponseMetadataProvider> providers)
+        {
+            for (var i = 0; i < providers.Count; i++)
+            {
+                var provider = providers[i];
+
+                if (provider is ProducesAttribute producesAttribute && producesAttribute.Type is null)
+                {
+                    // ProducesAttribute that does not specify type is considered not significant.
+                    continue;
+                }
+
+                // Any other IApiResponseMetadataProvider is considered significant
+                return true;
+            }
+
+            return false;
+        }
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApiConventionApplicationModelConvention.cs

@@ -54,12 +54,6 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 
         private static void DiscoverApiConvention(ActionModel action)
         {
-            if (action.Filters.OfType<IApiResponseMetadataProvider>().Any())
-            {
-                // If an action already has providers, don't discover any from conventions.
-                return;
-            }
-
             var controller = action.Controller;
             var apiConventionAttributes = controller.Attributes.OfType<ApiConventionTypeAttribute>().ToArray();
             if (apiConventionAttributes.Length == 0)

src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ClientErrorResultFilterConvention.cs

@@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
                 return;
             }
 
-            
+
             action.Filters.Add(_filterFactory);
         }
 

src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ConsumesConstraintForFormFileParameterConvention.cs

@@ -9,7 +9,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding;
 namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 {
     /// <summary>
-    /// An <see cref="IActionModelConvention"/> that adds a <see cref="ConsumesAttribute"/> with <c>multipart/form-data</c> 
+    /// An <see cref="IActionModelConvention"/> that adds a <see cref="ConsumesAttribute"/> with <c>multipart/form-data</c>
     /// to controllers containing form file (<see cref="BindingSource.FormFile"/>) parameters.
     /// </summary>
     public class ConsumesConstraintForFormFileParameterConvention : IActionModelConvention

src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs

@@ -11,7 +11,7 @@ using Resources = Microsoft.AspNetCore.Mvc.Core.Resources;
 namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 {
     /// <summary>
-    /// A <see cref="IControllerModelConvention"/> that 
+    /// A <see cref="IControllerModelConvention"/> that
     /// <list type="bullet">
     /// <item>infers binding sources for parameters</item>
     /// <item><see cref="BindingInfo.BinderModelName"/> for bound properties and parameters.</item>

test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/ApiResponseTypeProviderTest.cs

@@ -550,6 +550,119 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer
                 });
         }
 
+        [Fact]
+        public void GetApiResponseTypes_CombinesProducesAttributeAndConventions()
+        {
+            // Arrange
+            var actionDescriptor = GetControllerActionDescriptor(typeof(TestController), nameof(TestController.PutModel));
+            actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(new ProducesAttribute("application/json"), FilterScope.Controller));
+            actionDescriptor.Properties[typeof(ApiConventionResult)] = new ApiConventionResult(new IApiResponseMetadataProvider[]
+            {
+                new ProducesResponseTypeAttribute(200),
+                new ProducesResponseTypeAttribute(400),
+                new ProducesDefaultResponseTypeAttribute(),
+            });
+            actionDescriptor.Properties[typeof(ProducesErrorResponseTypeAttribute)] = new ProducesErrorResponseTypeAttribute(typeof(ProblemDetails));
+
+            var provider = GetProvider();
+
+            // Act
+            var result = provider.GetApiResponseTypes(actionDescriptor);
+
+            // Assert
+            Assert.Collection(
+                result.OrderBy(r => r.StatusCode),
+                responseType =>
+                {
+                    Assert.True(responseType.IsDefaultResponse);
+                    Assert.Equal(typeof(ProblemDetails), responseType.Type);
+                    Assert.Collection(
+                        responseType.ApiResponseFormats,
+                        format => Assert.Equal("application/json", format.MediaType));
+                },
+                responseType =>
+                {
+                    Assert.Equal(200, responseType.StatusCode);
+                    Assert.Equal(typeof(DerivedModel), responseType.Type);
+                    Assert.False(responseType.IsDefaultResponse);
+                    Assert.Collection(
+                        responseType.ApiResponseFormats,
+                        format => Assert.Equal("application/json", format.MediaType));
+                },
+                responseType =>
+                {
+                    Assert.Equal(400, responseType.StatusCode);
+                    Assert.Equal(typeof(ProblemDetails), responseType.Type);
+                    Assert.False(responseType.IsDefaultResponse);
+                    Assert.Collection(
+                        responseType.ApiResponseFormats,
+                        format => Assert.Equal("application/json", format.MediaType));
+                });
+        }
+
+        [Fact]
+        public void GetApiResponseTypes_DoesNotCombineProducesAttributeThatSpecifiesType()
+        {
+            // Arrange
+            var actionDescriptor = GetControllerActionDescriptor(typeof(TestController), nameof(TestController.PutModel));
+            actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(new ProducesAttribute("application/json") { Type = typeof(string) }, FilterScope.Controller));
+            actionDescriptor.Properties[typeof(ApiConventionResult)] = new ApiConventionResult(new IApiResponseMetadataProvider[]
+            {
+                new ProducesResponseTypeAttribute(200),
+                new ProducesResponseTypeAttribute(400),
+                new ProducesDefaultResponseTypeAttribute(),
+            });
+            actionDescriptor.Properties[typeof(ProducesErrorResponseTypeAttribute)] = new ProducesErrorResponseTypeAttribute(typeof(ProblemDetails));
+
+            var provider = GetProvider();
+
+            // Act
+            var result = provider.GetApiResponseTypes(actionDescriptor);
+
+            // Assert
+            Assert.Collection(
+                result.OrderBy(r => r.StatusCode),
+                responseType =>
+                {
+                    Assert.Equal(200, responseType.StatusCode);
+                    Assert.Equal(typeof(string), responseType.Type);
+                    Assert.False(responseType.IsDefaultResponse);
+                    Assert.Collection(
+                        responseType.ApiResponseFormats,
+                        format => Assert.Equal("application/json", format.MediaType));
+                });
+        }
+
+        [Fact]
+        public void GetApiResponseTypes_DoesNotCombineProducesResponseTypeAttributeThatSpecifiesStatusCode()
+        {
+            // Arrange
+            var actionDescriptor = GetControllerActionDescriptor(typeof(TestController), nameof(TestController.PutModel));
+            actionDescriptor.Properties[typeof(ApiConventionResult)] = new ApiConventionResult(new IApiResponseMetadataProvider[]
+            {
+                new ProducesResponseTypeAttribute(200),
+            });
+            actionDescriptor.Properties[typeof(ProducesErrorResponseTypeAttribute)] = new ProducesErrorResponseTypeAttribute(typeof(ProblemDetails));
+
+            var provider = GetProvider();
+
+            // Act
+            var result = provider.GetApiResponseTypes(actionDescriptor);
+
+            // Assert
+            Assert.Collection(
+                result.OrderBy(r => r.StatusCode),
+                responseType =>
+                {
+                    Assert.Equal(200, responseType.StatusCode);
+                    Assert.Equal(typeof(DerivedModel), responseType.Type);
+                    Assert.False(responseType.IsDefaultResponse);
+                    Assert.Collection(
+                        responseType.ApiResponseFormats,
+                        format => Assert.Equal("application/json", format.MediaType));
+                });
+        }
+
         private static ApiResponseTypeProvider GetProvider()
         {
             var mvcOptions = new MvcOptions

test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ApiConventionApplicationModelConventionTest.cs

@@ -15,38 +15,6 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 {
     public class ApiConventionApplicationModelConventionTest
     {
-        [Fact]
-        public void Apply_DoesNotAddConventionItem_IfActionHasProducesResponseTypeAttribute()
-        {
-            // Arrange
-            var actionModel = GetActionModel(nameof(TestController.Delete));
-            actionModel.Filters.Add(new ProducesResponseTypeAttribute(200));
-
-            var convention = GetConvention();
-
-            // Act
-            convention.Apply(actionModel);
-
-            // Assert
-            Assert.DoesNotContain(typeof(ApiConventionResult), actionModel.Properties.Keys);
-        }
-
-        [Fact]
-        public void Apply_DoesNotAddConventionItem_IfActionHasProducesAttribute()
-        {
-            // Arrange
-            var actionModel = GetActionModel(nameof(TestController.Delete));
-            actionModel.Filters.Add(new ProducesAttribute(typeof(object)));
-
-            var convention = GetConvention();
-
-            // Act
-            convention.Apply(actionModel);
-
-            // Assert
-            Assert.DoesNotContain(typeof(ApiConventionResult), actionModel.Properties.Keys);
-        }
-
         [Fact]
         public void Apply_DoesNotAddConventionItem_IfNoConventionMatches()
         {

test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/InferParameterBindingInfoConventionTest.cs

@@ -387,7 +387,7 @@ Environment.NewLine + "int b";
             // Arrange
             var actionName = nameof(ParameterBindingController.ComplexTypeModelWithCancellationToken);
 
-            // Use the default set of ModelMetadataProviders so we get metadata details for CancellationToken. 
+            // Use the default set of ModelMetadataProviders so we get metadata details for CancellationToken.
             var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
             var context = GetContext(typeof(ParameterBindingController), modelMetadataProvider);
             var controllerModel = Assert.Single(context.Result.Controllers);
@@ -750,7 +750,7 @@ Environment.NewLine + "int b";
         }
 
         private static ActionModel GetActionModel(
-            Type controllerType, 
+            Type controllerType,
             string actionName,
             IModelMetadataProvider modelMetadataProvider = null)
         {

test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/InvalidModelStateFilterConventionTest.cs

@@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
             Assert.Single(action.Filters.OfType<ModelStateInvalidFilterFactory>());
         }
 
-      
+
         private static ActionModel GetActionModel()
         {
             var action = new ActionModel(typeof(object).GetMethods()[0], new object[0]);

test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/RouteTokenTransformerConventionTest.cs

@@ -2,14 +2,12 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.Reflection;
-using System.Text;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
 using Microsoft.AspNetCore.Routing;
 using Xunit;
 
-namespace Microsoft.AspNetCore.Mvc.Test.ApplicationModel
+namespace Microsoft.AspNetCore.Mvc.Test.ApplicationModels
 {
     public class RouteTokenTransformerConventionTest
     {

test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs

@@ -1277,6 +1277,41 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
                 });
         }
 
+        [Fact]
+        public async Task ApiConvention_ForPostActionWithProducesAttribute()
+        {
+            // Arrange
+            var expectedMediaTypes = new[] { "application/json", "text/json", };
+
+            // Act
+            var response = await Client.PostAsync(
+                $"ApiExplorerResponseTypeWithApiConventionController/PostWithProduces",
+                new StringContent(string.Empty));
+            var responseBody = await response.EnsureSuccessStatusCode().Content.ReadAsStringAsync();
+            var result = JsonConvert.DeserializeObject<List<ApiExplorerData>>(responseBody);
+
+            // Assert
+            var description = Assert.Single(result);
+            Assert.Collection(
+                description.SupportedResponseTypes.OrderBy(r => r.StatusCode),
+                responseType =>
+                {
+                    Assert.True(responseType.IsDefaultResponse);
+                },
+                responseType =>
+                {
+                    Assert.Equal(typeof(void).FullName, responseType.ResponseType);
+                    Assert.Equal(201, responseType.StatusCode);
+                    Assert.Empty(responseType.ResponseFormats);
+                },
+                responseType =>
+                {
+                    Assert.Equal(typeof(ProblemDetails).FullName, responseType.ResponseType);
+                    Assert.Equal(400, responseType.StatusCode);
+                    Assert.Equal(expectedMediaTypes, GetSortedMediaTypes(responseType));
+                });
+        }
+
         [Fact]
         public async Task ApiConvention_ForPutActionThatMatchesConvention()
         {

test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerResponseTypeWithApiConventionController.cs

@@ -27,6 +27,10 @@ namespace ApiExplorerWebSite
         [ProducesResponseType(403)]
         public IActionResult PostWithConventions() => null;
 
+        [HttpPost]
+        [Produces("application/json", "text/json")]
+        public IActionResult PostWithProduces(Product p) => null;
+
         [HttpPost]
         public Task<IActionResult> PostTaskOfProduct(Product p) => null;
 
@@ -47,4 +51,4 @@ namespace ApiExplorerWebSite
         [ProducesResponseType(409)]
         public static void CustomConventionMethod() { }
     }
-}
+}