Skip to content

Commit

Permalink
[IDP-1491] Remove default 200 response codes if not defined (#18)
Browse files Browse the repository at this point in the history
* [IDP-1491] Remove unnecessary response codes if not explicitly defined

* use int instead of importing the static class

* Only handle 200 without generic logic

* Account for new spectral rules

* Update logic and unit tests according to feedback

* undo diff
  • Loading branch information
heqianwang authored May 22, 2024
1 parent 911c776 commit 3dc292a
Show file tree
Hide file tree
Showing 8 changed files with 280 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
using System.Net;
using System.Reflection;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Mvc;
using Swashbuckle.AspNetCore.Annotations;
using Workleap.Extensions.OpenAPI.TypedResult;

namespace Workleap.Extensions.OpenAPI.Tests.TypedResult;

public class ExtractSchemaTypeResultFilterTests
{
public static IEnumerable<object[]> GetData()
public static IEnumerable<object[]> GetResponseMetadataData()
{
// Synchronous results
yield return new object[]
Expand Down Expand Up @@ -98,11 +100,30 @@ public static IEnumerable<object[]> GetData()
};
}

public static IEnumerable<object[]> GetAttributesData()
{
var methodsAndExpectedCodes = new Dictionary<string, HashSet<int>>
{
{ "WithOneProducesResponse", [200] },
{ "WithTwoProducesResponse", [200, 404] },
{ "WithProducesResponseWithType", [200, 404] },
{ "WithProducesResponseWithSwaggerResponse", [200, 400, 404] }
};

foreach (var pair in methodsAndExpectedCodes)
{
var attributes = typeof(AnnotationTestClass).GetMethod(pair.Key)?.CustomAttributes;
if (attributes != null)
{
yield return new object[] { attributes, pair.Value };
}
}
}

[Theory]
[MemberData(nameof(GetData))]
[MemberData(nameof(GetResponseMetadataData))]
internal void GetResponsesMetadata_ReturnsCorrectMetadata_ForTypedResultControllerMethods(Type returnType, IList<ExtractSchemaTypeResultFilter.ResponseMetadata> expectedMetadata)
{

// Act
var responsesMetadata = ExtractSchemaTypeResultFilter.GetResponsesMetadata(returnType).ToList();

Expand All @@ -116,8 +137,46 @@ internal void GetResponsesMetadata_ReturnsCorrectMetadata_ForTypedResultControll
}
}

[Theory]
[MemberData(nameof(GetAttributesData))]
internal void ExtractResponseCodesFromAttributes_ReturnsCorrectHashSet(IEnumerable<CustomAttributeData> endpointAttributes, HashSet<int> expectedResponseCodes)
{
// Act
var actualResponsesCodes = ExtractSchemaTypeResultFilter.ExtractResponseCodesFromAttributes(endpointAttributes);

// Assert
Assert.Equal(actualResponsesCodes, expectedResponseCodes);
}

private sealed class TestTypedSchema
{
public int Count { get; set; }
}

private sealed class AnnotationTestClass
{
[ProducesResponseType(StatusCodes.Status200OK)]
public void WithOneProducesResponse()
{
}

[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status404NotFound)]
public void WithTwoProducesResponse()
{
}

[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(string))]
[ProducesResponseType(StatusCodes.Status404NotFound, Type = typeof(ProblemDetails))]
public void WithProducesResponseWithType()
{
}

[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(string))]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
[SwaggerResponse(StatusCodes.Status404NotFound, Type = typeof(ProblemDetails))]
public void WithProducesResponseWithSwaggerResponse()
{
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
<PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="6.5.0" />
<PackageReference Include="xunit" Version="2.8.0" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.5.8">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System.Reflection;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.OpenApi.Models;
using Swashbuckle.AspNetCore.SwaggerGen;

Expand All @@ -11,20 +13,26 @@ internal sealed class ExtractSchemaTypeResultFilter : IOperationFilter

public void Apply(OpenApiOperation operation, OperationFilterContext context)
{
// If the endpoint has annotations defined, we don't want to remove them.
var explicitlyDefinedResponseCodes = ExtractResponseCodesFromAttributes(context.MethodInfo.CustomAttributes);

var usesTypedResultsReturnType = false;
foreach (var responseMetadata in GetResponsesMetadata(context.MethodInfo.ReturnType))
{
explicitlyDefinedResponseCodes.Add(responseMetadata.HttpCode);
// If the response content is already set, we won't overwrite it. This is the case for minimal APIs and
// when the ProducesResponseType attribute is present.
if (operation.Responses.TryGetValue(responseMetadata.HttpCode.ToString(), out var existingResponse))
{
var canEnrichContent = !existingResponse.Content.Any() && responseMetadata.SchemaType != null;

// when the ProducesResponseType attribute is present.
if (!canEnrichContent)
{
continue;
}
}

usesTypedResultsReturnType = true;
var response = new OpenApiResponse();
response.Description = responseMetadata.HttpCode.ToString();

Expand All @@ -36,6 +44,12 @@ public void Apply(OpenApiOperation operation, OperationFilterContext context)

operation.Responses[responseMetadata.HttpCode.ToString()] = response;
}

// The spec is generated with a default 200 response, we need to remove it if the endpoint does not return 200.
if (usesTypedResultsReturnType && !explicitlyDefinedResponseCodes.Contains(200))
{
operation.Responses.Remove("200");
}
}

internal static IEnumerable<ResponseMetadata> GetResponsesMetadata(Type returnType)
Expand Down Expand Up @@ -90,6 +104,28 @@ internal static IEnumerable<ResponseMetadata> GetResponsesMetadata(Type returnTy
}
}

internal static HashSet<int> ExtractResponseCodesFromAttributes(IEnumerable<CustomAttributeData> customAttributes)
{
HashSet<int> responseCodes = [];
foreach (var attribute in customAttributes)
{
if (!typeof(ProducesResponseTypeAttribute).IsAssignableFrom(attribute.AttributeType))
{
continue;
}

foreach (var argument in attribute.ConstructorArguments)
{
if (argument.Value is int httpCode)
{
responseCodes.Add(httpCode);
}
}
}

return responseCodes;
}

// Initialize an instance of the result type to get the response metadata and return null if it's not possible
private static ResponseMetadata? ExtractMetadataFromTypedResult(Type resultType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,31 @@ public Results<Ok<TypedResultExample>, Forbidden<string>, InternalServerError<st
_ => TypedResults.Ok(new TypedResultExample("Example"))
};
}

[HttpGet]
[Route("/validateOkNotPresent")]
public Results<Created<TypedResultExample>, Forbidden<string>, InternalServerError<string>> TypedResultWithOutOk(int id)
{
return id switch
{
0 => TypedResultsExtensions.Forbidden("Forbidden"),
< 0 => TypedResultsExtensions.InternalServerError("An error occured when processing the request."),
_ => TypedResults.Created("hardcoded uri", new TypedResultExample("Example"))
};
}

[HttpGet]
[Route("/validateOkNotPresentButAnnotationPresent")]
[ProducesResponseType(StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status202Accepted, Type = typeof(string))]
[SwaggerResponse(StatusCodes.Status401Unauthorized, Type = typeof(string))]
public Results<Created<TypedResultExample>, Forbidden<string>, InternalServerError<string>> TypedResultWithoutOkButAnnotationPresent(int id)
{
return id switch
{
0 => TypedResultsExtensions.Forbidden("Forbidden"),
< 0 => TypedResultsExtensions.InternalServerError("An error occured when processing the request."),
_ => TypedResults.Created("hardcoded uri", new TypedResultExample("Example"))
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="8.0.4" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.5.0" />
<PackageReference Include="Swashbuckle.AspNetCore.Annotations" Version="6.5.0" />
<PackageReference Include="Workleap.OpenApi.MSBuild" Version="0.7.0">
<PackageReference Include="Workleap.OpenApi.MSBuild" Version="0.8.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
Expand Down
1 change: 0 additions & 1 deletion src/tests/WebApi.OpenAPI.SystemTest/custom.spectral.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
extends: [https://raw.githubusercontent.com/gsoft-inc/wl-api-guidelines/main/.spectral.yaml]
rules:
application-json-response-must-not-be-type-string: false
must-return-content-types: false
operation-operationId: false
78 changes: 76 additions & 2 deletions src/tests/WebApi.OpenAPI.SystemTest/openapi-v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,6 @@ paths:
type: integer
format: int32
responses:
'200':
description: Success
'204':
description: '204'
'401':
Expand Down Expand Up @@ -407,6 +405,82 @@ paths:
application/json:
schema:
type: string
/validateOkNotPresent:
get:
tags:
- TypedResult
operationId: TypedResultWithOutOk
parameters:
- name: id
in: query
style: form
schema:
type: integer
format: int32
responses:
'201':
description: '201'
content:
application/json:
schema:
$ref: '#/components/schemas/TypedResultExample'
'403':
description: '403'
content:
application/json:
schema:
type: string
'500':
description: '500'
content:
application/json:
schema:
type: string
/validateOkNotPresentButAnnotationPresent:
get:
tags:
- TypedResult
operationId: TypedResultWithoutOkButAnnotationPresent
parameters:
- name: id
in: query
style: form
schema:
type: integer
format: int32
responses:
'200':
description: Success
'202':
description: Accepted
content:
application/json:
schema:
type: string
'401':
description: Unauthorized
content:
application/json:
schema:
type: string
'201':
description: '201'
content:
application/json:
schema:
$ref: '#/components/schemas/TypedResultExample'
'403':
description: '403'
content:
application/json:
schema:
type: string
'500':
description: '500'
content:
application/json:
schema:
type: string
/useApplicationJsonContentType:
get:
tags:
Expand Down
Loading

0 comments on commit 3dc292a

Please sign in to comment.