From 811b234d2898e516d85075b6c5a2189e35642416 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Tue, 8 Apr 2025 16:16:33 +0200 Subject: [PATCH 01/16] Added tests for the issue --- ...pointMetadataApiDescriptionProviderTest.cs | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 9a5233a709a1..f67f05729bd8 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -331,6 +331,82 @@ public void AddsResponseDescription() Assert.Equal(expectedBadRequestDescription, badRequestResponseType.Description); } + [Fact] + public void AddsResponseDescription_WorksWithGenerics() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + () => new GenericClass { Value = new TimeSpan() }); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(GenericClass), okResponseType.Type); + Assert.Equal(typeof(GenericClass), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithGenericsAndTypedResults() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + () => TypedResults.Ok(new GenericClass { Value = new TimeSpan() })); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(GenericClass), okResponseType.Type); + Assert.Equal(typeof(GenericClass), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithCollections() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + () => new List { new() }); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(List), okResponseType.Type); // We use List as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(List), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithCollectionsAndTypedResults() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + () => TypedResults.Ok(new List { new() })); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(List), okResponseType.Type); // We use List as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(List), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + [Fact] public void WithEmptyMethodBody_AddsResponseDescription() { @@ -1813,5 +1889,7 @@ private class TestServiceProvider : IServiceProvider return null; } + } + private class GenericClass { public required TType Value { get; set; } } } From bc997eba7765765cddfbc190ae61e3f8664b2316 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 11:54:21 +0200 Subject: [PATCH 02/16] Add better type check to fix issue with description not being set --- .../EndpointMetadataApiDescriptionProvider.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index c920ea01d8aa..3cbfd297696e 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -322,6 +322,7 @@ private static void AddSupportedResponseTypes( Type returnType, EndpointMetadataCollection endpointMetadata) { + System.Diagnostics.Debugger.Break(); var responseType = returnType; // We support attributes (which implement the IApiResponseMetadataProvider) interface @@ -405,7 +406,7 @@ private static void AddSupportedResponseTypes( foreach (var metadata in responseMetadataTypes) { if (metadata.StatusCode == apiResponseType.StatusCode && - metadata.Type == apiResponseType.Type && + TypesAreCompatible(apiResponseType?.Type, metadata?.Type) && metadata.Description is not null) { matchingDescription = metadata.Description; @@ -413,6 +414,19 @@ private static void AddSupportedResponseTypes( } return matchingDescription; } + + static bool TypesAreCompatible(Type? apiResponseType, Type? metadaType) + { + // We need to a special check for cases where the inferred type is different than the one specified in attributes. + // For example, an endpoint that defines [ProducesResponseType>], + // but the endpoint returns weatherForecasts.ToList(). Because List<> is a different type than IEnumerable<>, it would incorrectly return false. + // Currently, we do a "simple" biderectional check to see if the types are assignable to each other. + // This isn't very thorough, but it works for most cases. + // For more information, check the related bug: https://github.com/dotnet/aspnetcore/issues/60518 + return apiResponseType == metadaType || + metadaType?.IsAssignableFrom(apiResponseType) == true || + apiResponseType?.IsAssignableFrom(metadaType) == true; + } } private static ApiResponseType CreateDefaultApiResponseType(Type responseType) From 9508aba087371f96690fd883a158a06be265279a Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 13:28:48 +0200 Subject: [PATCH 03/16] Fix build --- .../src/EndpointMetadataApiDescriptionProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 3cbfd297696e..17e95482a2aa 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -405,9 +405,9 @@ private static void AddSupportedResponseTypes( string? matchingDescription = null; foreach (var metadata in responseMetadataTypes) { - if (metadata.StatusCode == apiResponseType.StatusCode && + if (metadata?.StatusCode == apiResponseType?.StatusCode && TypesAreCompatible(apiResponseType?.Type, metadata?.Type) && - metadata.Description is not null) + metadata?.Description is not null) { matchingDescription = metadata.Description; } From 7fda04b2dadaff7dc7ef78314f1e7de9889d52c1 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 14:00:29 +0200 Subject: [PATCH 04/16] Add test cases where "higher" types are defined in attribute instead of metadata, and removed duplicate code --- ...pointMetadataApiDescriptionProviderTest.cs | 46 +++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index f67f05729bd8..7142a1c5a21f 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -336,7 +336,7 @@ public void AddsResponseDescription_WorksWithGenerics() { const string expectedOkDescription = "The weather forecast for the next 5 days."; - var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] () => new GenericClass { Value = new TimeSpan() }); var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); @@ -355,7 +355,7 @@ public void AddsResponseDescription_WorksWithGenericsAndTypedResults() { const string expectedOkDescription = "The weather forecast for the next 5 days."; - var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] () => TypedResults.Ok(new GenericClass { Value = new TimeSpan() })); var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); @@ -374,7 +374,7 @@ public void AddsResponseDescription_WorksWithCollections() { const string expectedOkDescription = "The weather forecast for the next 5 days."; - var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] () => new List { new() }); var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); @@ -393,7 +393,7 @@ public void AddsResponseDescription_WorksWithCollectionsAndTypedResults() { const string expectedOkDescription = "The weather forecast for the next 5 days."; - var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = "The weather forecast for the next 5 days.")] + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] () => TypedResults.Ok(new List { new() })); var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); @@ -407,6 +407,44 @@ public void AddsResponseDescription_WorksWithCollectionsAndTypedResults() Assert.Equal("application/json", createdOkFormat.MediaType); } + [Fact] + public void AddsResponseDescription_WorksWithCollectionsWhereInnerTypeIsInEndpoint() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => new List { new() }.AsEnumerable()); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithCollectionsAndTypedResultsWhereInnerTypeIsInEndpoint() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => TypedResults.Ok(new List { new() }.AsEnumerable())); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + [Fact] public void WithEmptyMethodBody_AddsResponseDescription() { From b0a26e7b6b9b213ecffa431ead5c0e5819ec15a0 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 14:28:10 +0200 Subject: [PATCH 05/16] Add extra test cases for inner types --- ...pointMetadataApiDescriptionProviderTest.cs | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 7142a1c5a21f..86098b0b275c 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -445,6 +445,44 @@ public void AddsResponseDescription_WorksWithCollectionsAndTypedResultsWhereInne Assert.Equal("application/json", createdOkFormat.MediaType); } + [Fact] + public void AddsResponseDescription_WorksWithCollectionsWhereInnerTypeIsInEndpointAndIsBaseClass() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => new List { new() }.AsEnumerable()); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithCollectionsAndTypedResultsWhereInnerTypeIsInEndpointAndIsBaseClass() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => TypedResults.Ok(new List { new() }.AsEnumerable())); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + [Fact] public void WithEmptyMethodBody_AddsResponseDescription() { @@ -1929,5 +1967,7 @@ private class TestServiceProvider : IServiceProvider } } - private class GenericClass { public required TType Value { get; set; } } + + private class GenericClass : BaseClass { public required TType Value { get; set; } } + private class BaseClass { } } From 4c29f11c84f02250b062300b819c70c799f0a7e8 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 14:28:18 +0200 Subject: [PATCH 06/16] Remove break statement --- .../src/EndpointMetadataApiDescriptionProvider.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 17e95482a2aa..9423546abe3e 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -322,7 +322,6 @@ private static void AddSupportedResponseTypes( Type returnType, EndpointMetadataCollection endpointMetadata) { - System.Diagnostics.Debugger.Break(); var responseType = returnType; // We support attributes (which implement the IApiResponseMetadataProvider) interface From 26be6f5ccca8ada7e23784dce667bb0ebb416a36 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 14:29:03 +0200 Subject: [PATCH 07/16] Fix typo --- .../src/EndpointMetadataApiDescriptionProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 9423546abe3e..4c63c359146f 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -419,7 +419,7 @@ static bool TypesAreCompatible(Type? apiResponseType, Type? metadaType) // We need to a special check for cases where the inferred type is different than the one specified in attributes. // For example, an endpoint that defines [ProducesResponseType>], // but the endpoint returns weatherForecasts.ToList(). Because List<> is a different type than IEnumerable<>, it would incorrectly return false. - // Currently, we do a "simple" biderectional check to see if the types are assignable to each other. + // Currently, we do a "simple" bidirectional check to see if the types are assignable to each other. // This isn't very thorough, but it works for most cases. // For more information, check the related bug: https://github.com/dotnet/aspnetcore/issues/60518 return apiResponseType == metadaType || From 2c1fce245f7834816ffe93110f88a0389cc14e34 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 15:29:12 +0200 Subject: [PATCH 08/16] Remove empty line --- .../test/EndpointMetadataApiDescriptionProviderTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 86098b0b275c..8456329b9742 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -1965,7 +1965,6 @@ private class TestServiceProvider : IServiceProvider return null; } - } private class GenericClass : BaseClass { public required TType Value { get; set; } } From 0d6f97eeee1ca7ab5c3c28431b4ae786bf304df0 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Sat, 12 Jul 2025 23:00:54 +0200 Subject: [PATCH 09/16] Fix typo --- .../src/EndpointMetadataApiDescriptionProvider.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 4c63c359146f..8ca4683aa670 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -414,7 +414,7 @@ private static void AddSupportedResponseTypes( return matchingDescription; } - static bool TypesAreCompatible(Type? apiResponseType, Type? metadaType) + static bool TypesAreCompatible(Type? apiResponseType, Type? metadataType) { // We need to a special check for cases where the inferred type is different than the one specified in attributes. // For example, an endpoint that defines [ProducesResponseType>], @@ -422,9 +422,9 @@ static bool TypesAreCompatible(Type? apiResponseType, Type? metadaType) // Currently, we do a "simple" bidirectional check to see if the types are assignable to each other. // This isn't very thorough, but it works for most cases. // For more information, check the related bug: https://github.com/dotnet/aspnetcore/issues/60518 - return apiResponseType == metadaType || - metadaType?.IsAssignableFrom(apiResponseType) == true || - apiResponseType?.IsAssignableFrom(metadaType) == true; + return apiResponseType == metadataType || + metadataType?.IsAssignableFrom(apiResponseType) == true || + apiResponseType?.IsAssignableFrom(metadataType) == true; } } From 46437fdbed1dbe8dee3489a1a3a9b0e2b3f0286b Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Mon, 14 Jul 2025 18:05:48 +0200 Subject: [PATCH 10/16] Remove unnecessary null checks --- src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln | 30 +++++++++++++++++++ .../EndpointMetadataApiDescriptionProvider.cs | 6 ++-- src/submodules/googletest | 2 +- 3 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln diff --git a/src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln b/src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln new file mode 100644 index 000000000000..88cfa9b13f51 --- /dev/null +++ b/src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln @@ -0,0 +1,30 @@ +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio Version 17 +VisualStudioVersion = 17.5.2.0 +MinimumVisualStudioVersion = 10.0.40219.1 +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Mvc.ApiExplorer.Test", "test\Microsoft.AspNetCore.Mvc.ApiExplorer.Test.csproj", "{B063A27B-F6DE-3734-64D8-302F23449102}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Mvc.ApiExplorer", "src\Microsoft.AspNetCore.Mvc.ApiExplorer.csproj", "{548A6F46-FE70-02B8-27CC-DE23693D4AFF}" +EndProject +Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution + Debug|Any CPU = Debug|Any CPU + Release|Any CPU = Release|Any CPU + EndGlobalSection + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {B063A27B-F6DE-3734-64D8-302F23449102}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {B063A27B-F6DE-3734-64D8-302F23449102}.Debug|Any CPU.Build.0 = Debug|Any CPU + {B063A27B-F6DE-3734-64D8-302F23449102}.Release|Any CPU.ActiveCfg = Release|Any CPU + {B063A27B-F6DE-3734-64D8-302F23449102}.Release|Any CPU.Build.0 = Release|Any CPU + {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Debug|Any CPU.Build.0 = Debug|Any CPU + {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Release|Any CPU.ActiveCfg = Release|Any CPU + {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Release|Any CPU.Build.0 = Release|Any CPU + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE + EndGlobalSection + GlobalSection(ExtensibilityGlobals) = postSolution + SolutionGuid = {B4A90275-5079-4EEF-A9D0-DE917A39A5B4} + EndGlobalSection +EndGlobal diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 8ca4683aa670..101de1a0950c 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -404,9 +404,9 @@ private static void AddSupportedResponseTypes( string? matchingDescription = null; foreach (var metadata in responseMetadataTypes) { - if (metadata?.StatusCode == apiResponseType?.StatusCode && - TypesAreCompatible(apiResponseType?.Type, metadata?.Type) && - metadata?.Description is not null) + if (metadata.StatusCode == apiResponseType.StatusCode && + TypesAreCompatible(apiResponseType.Type, metadata.Type) && + metadata.Description is not null) { matchingDescription = metadata.Description; } diff --git a/src/submodules/googletest b/src/submodules/googletest index a45468c0fcbe..72189081cae8 160000 --- a/src/submodules/googletest +++ b/src/submodules/googletest @@ -1 +1 @@ -Subproject commit a45468c0fcbeda1588573a7d8283b320bf9970cb +Subproject commit 72189081cae8b729422860b195bf2cad625b7eb4 From 77e48fe15eaa985ab068140eaa6be01728ca87c8 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Mon, 14 Jul 2025 18:28:10 +0200 Subject: [PATCH 11/16] Change type check to a one-way approach --- .../src/EndpointMetadataApiDescriptionProvider.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 101de1a0950c..89d45efc5e11 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -418,13 +418,14 @@ static bool TypesAreCompatible(Type? apiResponseType, Type? metadataType) { // We need to a special check for cases where the inferred type is different than the one specified in attributes. // For example, an endpoint that defines [ProducesResponseType>], - // but the endpoint returns weatherForecasts.ToList(). Because List<> is a different type than IEnumerable<>, it would incorrectly return false. - // Currently, we do a "simple" bidirectional check to see if the types are assignable to each other. - // This isn't very thorough, but it works for most cases. + // but the endpoint returns weatherForecasts.ToList(). Because List<> is a different type than IEnumerable<>, it would incorrectly set OpenAPI metadata incorrectly. + // We use a conservative unidirectional check where the attribute type must be assignable from the inferred type. + // This handles inheritance (BaseClass ← DerivedClass) and interface implementation (IEnumerable ← List). + // This should be sufficient, as it's more common to specify an interface or base class type in the attribute and a concrete type in the endpoint implementation, + // compared to doing the opposite. // For more information, check the related bug: https://github.com/dotnet/aspnetcore/issues/60518 return apiResponseType == metadataType || - metadataType?.IsAssignableFrom(apiResponseType) == true || - apiResponseType?.IsAssignableFrom(metadataType) == true; + metadataType?.IsAssignableFrom(apiResponseType) == true; } } From 366f8b8f6df0954c858a9de2b5caf8aba5f6ec5e Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Mon, 14 Jul 2025 18:30:54 +0200 Subject: [PATCH 12/16] Add test that checks that the type check is unidirectional --- ...pointMetadataApiDescriptionProviderTest.cs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 8456329b9742..a85fd8f7df28 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -464,23 +464,30 @@ public void AddsResponseDescription_WorksWithCollectionsWhereInnerTypeIsInEndpoi Assert.Equal("application/json", createdOkFormat.MediaType); } + /// + /// EndpointMetadataApiDescriptionProvider performs a one way type check for discovering response types to match the description that's set in [ProducesResponseType] + /// The reason we do a one-way check instead of a bidirectional check is to prevent too many positive matches. + /// + /// + /// Example: If we did a bidirectional check, we would match something scenarios like this, which can cause confusion: + /// [ProducesResponseType(StatusCodes.Status200OK, Description = "Returned with a string")] -> TypedResults.Ok(new object()) + /// This would match because object is assignable to string, + /// but it doesn't make sense to add the Description to the object type because the attribute says we should return a string. + /// + /// This test documents this desired behavior and will fail if the behavior changes, so the developer can double check if their change is intentional. + /// [Fact] - public void AddsResponseDescription_WorksWithCollectionsAndTypedResultsWhereInnerTypeIsInEndpointAndIsBaseClass() + public void AddsResponseDescription_ShouldFailWhenInferredTypeIsNotDirectlyAssignableToAttributeType() { - const string expectedOkDescription = "The weather forecast for the next 5 days."; - - var apiDescription = GetApiDescription([ProducesResponseType>>(StatusCodes.Status200OK, Description = expectedOkDescription)] - () => TypedResults.Ok(new List { new() }.AsEnumerable())); + var apiDescription = GetApiDescription([ProducesResponseType(StatusCodes.Status200OK, Description = "Only returned with a string")] + () => TypedResults.Ok(new object())); var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); Assert.Equal(200, okResponseType.StatusCode); - Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) - Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); - Assert.Equal(expectedOkDescription, okResponseType.Description); - - var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); - Assert.Equal("application/json", createdOkFormat.MediaType); + Assert.Equal(typeof(object), okResponseType.Type); + Assert.Equal(typeof(object), okResponseType.ModelMetadata?.ModelType); + Assert.Null(okResponseType.Description); } [Fact] From 6979c17b4098347bcd8df3f577e1c587eecc5eb3 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Mon, 14 Jul 2025 18:31:14 +0200 Subject: [PATCH 13/16] Remove bidirectional tests because the feature has been removed --- ...pointMetadataApiDescriptionProviderTest.cs | 57 ------------------- 1 file changed, 57 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index a85fd8f7df28..cd64cee754d5 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -407,63 +407,6 @@ public void AddsResponseDescription_WorksWithCollectionsAndTypedResults() Assert.Equal("application/json", createdOkFormat.MediaType); } - [Fact] - public void AddsResponseDescription_WorksWithCollectionsWhereInnerTypeIsInEndpoint() - { - const string expectedOkDescription = "The weather forecast for the next 5 days."; - - var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] - () => new List { new() }.AsEnumerable()); - - var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); - - Assert.Equal(200, okResponseType.StatusCode); - Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) - Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); - Assert.Equal(expectedOkDescription, okResponseType.Description); - - var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); - Assert.Equal("application/json", createdOkFormat.MediaType); - } - - [Fact] - public void AddsResponseDescription_WorksWithCollectionsAndTypedResultsWhereInnerTypeIsInEndpoint() - { - const string expectedOkDescription = "The weather forecast for the next 5 days."; - - var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] - () => TypedResults.Ok(new List { new() }.AsEnumerable())); - - var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); - - Assert.Equal(200, okResponseType.StatusCode); - Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) - Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); - Assert.Equal(expectedOkDescription, okResponseType.Description); - - var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); - Assert.Equal("application/json", createdOkFormat.MediaType); - } - - [Fact] - public void AddsResponseDescription_WorksWithCollectionsWhereInnerTypeIsInEndpointAndIsBaseClass() - { - const string expectedOkDescription = "The weather forecast for the next 5 days."; - - var apiDescription = GetApiDescription([ProducesResponseType>>(StatusCodes.Status200OK, Description = expectedOkDescription)] - () => new List { new() }.AsEnumerable()); - - var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); - - Assert.Equal(200, okResponseType.StatusCode); - Assert.Equal(typeof(IEnumerable), okResponseType.Type); // We use IEnumerable as the inferred type has higher priority than those set by metadata (attributes) - Assert.Equal(typeof(IEnumerable), okResponseType.ModelMetadata?.ModelType); - Assert.Equal(expectedOkDescription, okResponseType.Description); - - var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); - Assert.Equal("application/json", createdOkFormat.MediaType); - } - /// /// EndpointMetadataApiDescriptionProvider performs a one way type check for discovering response types to match the description that's set in [ProducesResponseType] /// The reason we do a one-way check instead of a bidirectional check is to prevent too many positive matches. From 443f85afb35cf3ccaca6eddf82e211e44bf697b9 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Mon, 14 Jul 2025 18:33:07 +0200 Subject: [PATCH 14/16] Remove unnecessary SLN --- src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln | 30 --------------------- 1 file changed, 30 deletions(-) delete mode 100644 src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln diff --git a/src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln b/src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln deleted file mode 100644 index 88cfa9b13f51..000000000000 --- a/src/Mvc/Mvc.ApiExplorer/Mvc.ApiExplorer.sln +++ /dev/null @@ -1,30 +0,0 @@ -Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio Version 17 -VisualStudioVersion = 17.5.2.0 -MinimumVisualStudioVersion = 10.0.40219.1 -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Mvc.ApiExplorer.Test", "test\Microsoft.AspNetCore.Mvc.ApiExplorer.Test.csproj", "{B063A27B-F6DE-3734-64D8-302F23449102}" -EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Mvc.ApiExplorer", "src\Microsoft.AspNetCore.Mvc.ApiExplorer.csproj", "{548A6F46-FE70-02B8-27CC-DE23693D4AFF}" -EndProject -Global - GlobalSection(SolutionConfigurationPlatforms) = preSolution - Debug|Any CPU = Debug|Any CPU - Release|Any CPU = Release|Any CPU - EndGlobalSection - GlobalSection(ProjectConfigurationPlatforms) = postSolution - {B063A27B-F6DE-3734-64D8-302F23449102}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {B063A27B-F6DE-3734-64D8-302F23449102}.Debug|Any CPU.Build.0 = Debug|Any CPU - {B063A27B-F6DE-3734-64D8-302F23449102}.Release|Any CPU.ActiveCfg = Release|Any CPU - {B063A27B-F6DE-3734-64D8-302F23449102}.Release|Any CPU.Build.0 = Release|Any CPU - {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Debug|Any CPU.Build.0 = Debug|Any CPU - {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Release|Any CPU.ActiveCfg = Release|Any CPU - {548A6F46-FE70-02B8-27CC-DE23693D4AFF}.Release|Any CPU.Build.0 = Release|Any CPU - EndGlobalSection - GlobalSection(SolutionProperties) = preSolution - HideSolutionNode = FALSE - EndGlobalSection - GlobalSection(ExtensibilityGlobals) = postSolution - SolutionGuid = {B4A90275-5079-4EEF-A9D0-DE917A39A5B4} - EndGlobalSection -EndGlobal From 0bf05a67f6180d600d4b9517e688bbf7c9fad8b1 Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Mon, 14 Jul 2025 18:36:47 +0200 Subject: [PATCH 15/16] Fix google test submodule --- src/submodules/googletest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/submodules/googletest b/src/submodules/googletest index 72189081cae8..a45468c0fcbe 160000 --- a/src/submodules/googletest +++ b/src/submodules/googletest @@ -1 +1 @@ -Subproject commit 72189081cae8b729422860b195bf2cad625b7eb4 +Subproject commit a45468c0fcbeda1588573a7d8283b320bf9970cb From 82f196078926923250acc33cd3f1f0c3d97139ea Mon Sep 17 00:00:00 2001 From: Sander ten Brinke Date: Mon, 14 Jul 2025 23:13:59 +0200 Subject: [PATCH 16/16] Remove unused base class in Tests --- .../test/EndpointMetadataApiDescriptionProviderTest.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index cd64cee754d5..7de4d5eb5b89 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -1917,6 +1917,5 @@ private class TestServiceProvider : IServiceProvider } } - private class GenericClass : BaseClass { public required TType Value { get; set; } } - private class BaseClass { } + private class GenericClass { public required TType Value { get; set; } } }