Skip to content

Commit c8a1cd9

Browse files
captainsafiacaptainsafia
authored
Handle setting content types for ProducesResponseType attribute
1 parent a3e4171 commit c8a1cd9

7 files changed

+236
-50
lines changed

‎src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs

Lines changed: 65 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,14 @@ private ICollection<ApiResponseType> GetApiResponseTypes(
7979
Type defaultErrorType)
8080
{
8181
var contentTypes = new MediaTypeCollection();
82+
var responseTypeMetadataProviders = _mvcOptions.OutputFormatters.OfType<IApiResponseTypeMetadataProvider>();
8283

83-
var responseTypes = ReadResponseMetadata(responseMetadataAttributes, type, defaultErrorType, contentTypes);
84+
var responseTypes = ReadResponseMetadata(
85+
responseMetadataAttributes,
86+
type,
87+
defaultErrorType,
88+
contentTypes,
89+
responseTypeMetadataProviders);
8490

8591
// Set the default status only when no status has already been set explicitly
8692
if (responseTypes.Count == 0 && type != null)
@@ -102,7 +108,10 @@ private ICollection<ApiResponseType> GetApiResponseTypes(
102108
contentTypes.Add((string)null!);
103109
}
104110

105-
CalculateResponseFormats(responseTypes, contentTypes);
111+
foreach(var apiResponse in responseTypes)
112+
{
113+
CalculateResponseFormatForType(apiResponse, contentTypes, responseTypeMetadataProviders, _modelMetadataProvider);
114+
}
106115

107116
return responseTypes;
108117
}
@@ -112,7 +121,9 @@ internal static List<ApiResponseType> ReadResponseMetadata(
112121
IReadOnlyList<IApiResponseMetadataProvider> responseMetadataAttributes,
113122
Type? type,
114123
Type defaultErrorType,
115-
MediaTypeCollection contentTypes)
124+
MediaTypeCollection contentTypes,
125+
IEnumerable<IApiResponseTypeMetadataProvider>? responseTypeMetadataProviders = null,
126+
IModelMetadataProvider? _modelMetadataProvider = null)
116127
{
117128
var results = new Dictionary<int, ApiResponseType>();
118129

@@ -123,7 +134,18 @@ internal static List<ApiResponseType> ReadResponseMetadata(
123134
{
124135
foreach (var metadataAttribute in responseMetadataAttributes)
125136
{
126-
metadataAttribute.SetContentTypes(contentTypes);
137+
// All ProducesXAttributes, except for ProducesResponseTypeAttribute do
138+
// not allow multiple instances on the same method/class/etc. For those
139+
// scenarios, the `SetContentTypes` method on the attribute continuously
140+
// clears out more general content types in favor of more specific ones
141+
// since we iterate through the attributes in order. For example, if a
142+
// Produces exists on both a controller and an action within the controller,
143+
// we favor the definition in the action. This is a semantic that does not
144+
// apply to ProducesResponseType, which allows multiple instances on an target.
145+
if (metadataAttribute is not ProducesResponseTypeAttribute)
146+
{
147+
metadataAttribute.SetContentTypes(contentTypes);
148+
}
127149

128150
var statusCode = metadataAttribute.StatusCode;
129151

@@ -157,6 +179,18 @@ internal static List<ApiResponseType> ReadResponseMetadata(
157179
}
158180
}
159181

182+
// We special case the handling of ProcuesResponseTypeAttributes since
183+
// multiple ProducesResponseTypeAttributes are permitted on a single
184+
// action/controller/etc. In that scenario, instead of picking the most-specific
185+
// set of content types (like we do with the Produces attribute above) we process
186+
// the content types for each attribute independently.
187+
if (metadataAttribute is ProducesResponseTypeAttribute)
188+
{
189+
var attributeContentTypes = new MediaTypeCollection();
190+
metadataAttribute.SetContentTypes(attributeContentTypes);
191+
CalculateResponseFormatForType(apiResponseType, attributeContentTypes, responseTypeMetadataProviders, _modelMetadataProvider);
192+
}
193+
160194
if (apiResponseType.Type != null)
161195
{
162196
results[apiResponseType.StatusCode] = apiResponseType;
@@ -167,9 +201,15 @@ internal static List<ApiResponseType> ReadResponseMetadata(
167201
return results.Values.ToList();
168202
}
169203

170-
private void CalculateResponseFormats(ICollection<ApiResponseType> responseTypes, MediaTypeCollection declaredContentTypes)
204+
private static void CalculateResponseFormatForType(ApiResponseType apiResponse, MediaTypeCollection declaredContentTypes, IEnumerable<IApiResponseTypeMetadataProvider>? responseTypeMetadataProviders, IModelMetadataProvider? _modelMetadataProvider)
171205
{
172-
var responseTypeMetadataProviders = _mvcOptions.OutputFormatters.OfType<IApiResponseTypeMetadataProvider>();
206+
// If response formats have already been calculate for this type,
207+
// then exit early. This avoids populating the ApiResponseFormat for
208+
// types that have already been handled, specifically ProducesResponseTypes.
209+
if (apiResponse.ApiResponseFormats.Count > 0)
210+
{
211+
return;
212+
}
173213

174214
// Given the content-types that were declared for this action, determine the formatters that support the content-type for the given
175215
// response type.
@@ -179,21 +219,20 @@ private void CalculateResponseFormats(ICollection<ApiResponseType> responseTypes
179219
// 3. When no formatter supports the specified content-type, use the user specified value as is. This is useful in actions where the user
180220
// dictates the content-type.
181221
// e.g. [Produces("application/pdf")] Action() => FileStream("somefile.pdf", "application/pdf");
182-
183-
foreach (var apiResponse in responseTypes)
222+
var responseType = apiResponse.Type;
223+
if (responseType == null || responseType == typeof(void))
184224
{
185-
var responseType = apiResponse.Type;
186-
if (responseType == null || responseType == typeof(void))
187-
{
188-
continue;
189-
}
225+
return;
226+
}
190227

191-
apiResponse.ModelMetadata = _modelMetadataProvider.GetMetadataForType(responseType);
228+
apiResponse.ModelMetadata = _modelMetadataProvider?.GetMetadataForType(responseType);
192229

193-
foreach (var contentType in declaredContentTypes)
194-
{
195-
var isSupportedContentType = false;
230+
foreach (var contentType in declaredContentTypes)
231+
{
232+
var isSupportedContentType = false;
196233

234+
if (responseTypeMetadataProviders != null)
235+
{
197236
foreach (var responseTypeMetadataProvider in responseTypeMetadataProviders)
198237
{
199238
var formatterSupportedContentTypes = responseTypeMetadataProvider.GetSupportedContentTypes(
@@ -216,15 +255,17 @@ private void CalculateResponseFormats(ICollection<ApiResponseType> responseTypes
216255
});
217256
}
218257
}
258+
}
259+
260+
219261

220-
if (!isSupportedContentType && contentType != null)
262+
if (!isSupportedContentType && contentType != null)
263+
{
264+
// No output formatter was found that supports this content type. Add the user specified content type as-is to the result.
265+
apiResponse.ApiResponseFormats.Add(new ApiResponseFormat
221266
{
222-
// No output formatter was found that supports this content type. Add the user specified content type as-is to the result.
223-
apiResponse.ApiResponseFormats.Add(new ApiResponseFormat
224-
{
225-
MediaType = contentType,
226-
});
227-
}
267+
MediaType = contentType,
268+
});
228269
}
229270
}
230271
}

‎src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,9 @@ private static void AddSupportedResponseTypes(
269269
{
270270
AddResponseContentTypes(apiResponseType.ApiResponseFormats, contentTypes);
271271
}
272-
else if (CreateDefaultApiResponseFormat(apiResponseType.Type) is { } defaultResponseFormat)
272+
// Only set the default response type if it hasn't already been set via a
273+
// ProducesResponseTypeAttribute.
274+
else if (apiResponseType.ApiResponseFormats.Count == 0 && CreateDefaultApiResponseFormat(apiResponseType.Type) is { } defaultResponseFormat)
273275
{
274276
apiResponseType.ApiResponseFormats.Add(defaultResponseFormat);
275277
}

‎src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,51 @@ public void GetApiResponseTypes_UsesContentTypeWithoutWildCard_WhenNoFormatterSu
709709
});
710710
}
711711

712+
[Fact]
713+
public void ApiAction_HandlesContentTypesAndStatusCodes()
714+
{
715+
// Arrange
716+
var actionDescriptor = GetControllerActionDescriptor(typeof(TestController), nameof(TestController.GetUser));
717+
actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(new ProducesAttribute("text/xml") { Type = typeof(BaseModel) }, FilterScope.Action));
718+
actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(new ProducesResponseTypeAttribute(typeof(ValidationProblemDetails), 400, "application/validationproblem+json"), FilterScope.Action));
719+
actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(new ProducesResponseTypeAttribute(typeof(ProblemDetails), 404, "application/problem+json"), FilterScope.Action));
720+
actionDescriptor.FilterDescriptors.Add(new FilterDescriptor(new ProducesResponseTypeAttribute(409), FilterScope.Action));
721+
722+
var provider = new ApiResponseTypeProvider(new EmptyModelMetadataProvider(), new ActionResultTypeMapper(), new MvcOptions());
723+
724+
// Act
725+
var result = provider.GetApiResponseTypes(actionDescriptor);
726+
727+
// Assert
728+
Assert.Collection(
729+
result.OrderBy(r => r.StatusCode),
730+
responseType =>
731+
{
732+
Assert.Equal(typeof(BaseModel), responseType.Type);
733+
Assert.Equal(200, responseType.StatusCode);
734+
Assert.Equal(new[] { "text/xml" }, GetSortedMediaTypes(responseType));
735+
736+
},
737+
responseType =>
738+
{
739+
Assert.Equal(typeof(ValidationProblemDetails), responseType.Type);
740+
Assert.Equal(400, responseType.StatusCode);
741+
Assert.Equal(new[] { "application/validationproblem+json" }, GetSortedMediaTypes(responseType));
742+
},
743+
responseType =>
744+
{
745+
Assert.Equal(typeof(ProblemDetails), responseType.Type);
746+
Assert.Equal(404, responseType.StatusCode);
747+
Assert.Equal(new[] { "application/problem+json" }, GetSortedMediaTypes(responseType));
748+
},
749+
responseType =>
750+
{
751+
Assert.Equal(typeof(void), responseType.Type);
752+
Assert.Equal(409, responseType.StatusCode);
753+
Assert.Empty(GetSortedMediaTypes(responseType));
754+
});
755+
}
756+
712757
private static ApiResponseTypeProvider GetProvider()
713758
{
714759
var mvcOptions = new MvcOptions
@@ -719,6 +764,13 @@ private static ApiResponseTypeProvider GetProvider()
719764
return provider;
720765
}
721766

767+
private static IEnumerable<string> GetSortedMediaTypes(ApiResponseType apiResponseType)
768+
{
769+
return apiResponseType.ApiResponseFormats
770+
.OrderBy(format => format.MediaType)
771+
.Select(format => format.MediaType);
772+
}
773+
722774
private static ControllerActionDescriptor GetControllerActionDescriptor(Type type, string name)
723775
{
724776
var method = type.GetMethod(name);

‎src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ public void GetApiDescription_ReturnsActionResultWithProduces_And_ProducesConten
567567
// Arrange
568568
var action = CreateActionDescriptor(methodName, controllerType);
569569
action.FilterDescriptors = filterDescriptors;
570-
var expectedMediaTypes = new[] { "application/json", "application/xml", "text/json", "text/xml" };
570+
var expectedMediaTypes = new[] { "application/json", "text/json" };
571571

572572
// Act
573573
var descriptions = GetApiDescriptions(action);
@@ -677,7 +677,7 @@ public void GetApiDescription_ReturnsVoidWithProducesContentType(
677677
// Arrange
678678
var action = CreateActionDescriptor(methodName, controllerType);
679679
action.FilterDescriptors = filterDescriptors;
680-
var expectedMediaTypes = new[] { "application/json", "application/xml", "text/json", "text/xml" };
680+
var expectedMediaTypes = new[] { "application/json", "text/json" };
681681

682682
// Act
683683
var descriptions = GetApiDescriptions(action);
@@ -740,7 +740,7 @@ public void GetApiDescription_ReturnsActionResultOfTWithProducesContentType(
740740
new ProducesResponseTypeAttribute(typeof(ErrorDetails), 500),
741741
FilterScope.Action)
742742
};
743-
var expectedMediaTypes = new[] { "application/json", "application/xml", "text/json", "text/xml" };
743+
var expectedMediaTypes = new[] { "application/json", "text/json" };
744744

745745
// Act
746746
var descriptions = GetApiDescriptions(action);
@@ -810,7 +810,7 @@ public void GetApiDescription_ReturnsActionResultOfTWithProducesContentType_ForS
810810
new ProducesResponseTypeAttribute(typeof(ErrorDetails), 500),
811811
FilterScope.Action)
812812
};
813-
var expectedMediaTypes = new[] { "application/json", "application/xml", "text/json", "text/xml" };
813+
var expectedMediaTypes = new[] { "application/json", "text/json" };
814814

815815
// Act
816816
var descriptions = GetApiDescriptions(action);
@@ -880,7 +880,7 @@ public void GetApiDescription_ReturnsActionResultOfSequenceOfTWithProducesConten
880880
new ProducesResponseTypeAttribute(typeof(ErrorDetails), 500),
881881
FilterScope.Action)
882882
};
883-
var expectedMediaTypes = new[] { "application/json", "application/xml", "text/json", "text/xml" };
883+
var expectedMediaTypes = new[] { "application/json", "text/json" };
884884

885885
// Act
886886
var descriptions = GetApiDescriptions(action);

0 commit comments

Comments
 (0)