Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idea: Improvements to Default OpenAPI Descriptions #432

Closed
austins opened this issue Apr 19, 2023 · 5 comments
Closed

Idea: Improvements to Default OpenAPI Descriptions #432

austins opened this issue Apr 19, 2023 · 5 comments
Labels
enhancement New feature or request implemented requested feature has been implemented

Comments

@austins
Copy link

austins commented Apr 19, 2023

  1. Produces 200 is always set. This is a reasonable default; however, how can we handle cases where the endpoint may only return, for example, just a 201? The Swagger doc will output a 200 where that endpoint doesn't return that. Describe() with clearDefaults=true would require you to have to describe everything again, not just the sucessful response(s), and potentially missing other defaults for an endpoint.

  2. Since endpoints are authorized by default, would it be good to add in a default for 401? AnonymousVerbs is empty unless AllowAnonymous is set for any verb. On an aside, I believe there isn't a way yet to set Produces for different verbs, but for now, here's an example:

if (Definition.AnonymousVerbs?.Any() != true)
{
    b.Produces(401);
}

What are your thoughts?

@dj-nitehawk
Copy link
Member

dj-nitehawk commented Apr 20, 2023

implemented in 5.8.1.11-beta

public override void Configure()
{
    //lack of AllowAnonymous() will auto add 401 produces meta
    Roles("test"); //any Roles/Claims/Permissions will auto add 403 produces meta
    Description(x => x
        .ClearDefaultProduces(200, 401, 403) //user has the choice which default produces to clear
        .Produces<Response>(201, "application/json"));
}

mind the small breaking change in this beta build: https://github.com/FastEndpoints/FastEndpoints/blob/main/Src/Library/changelog.md#breaking-changes

@dj-nitehawk dj-nitehawk added enhancement New feature or request implemented requested feature has been implemented labels Apr 20, 2023
@austins
Copy link
Author

austins commented Apr 20, 2023

Works brilliantly. Thanks!

@austins
Copy link
Author

austins commented Apr 20, 2023

@dj-nitehawk Curious about adding on one more case, what do you think?

When ErrorOptions.ProducesMetadataType is set, it checks if there is a validator and sets the 400 response:

if (opts.ProducesMetadataType is not null && Definition.ValidatorType is not null)

Would it be good to set ProducesMetadataType = typeof(HttpValidationProblemDetails) by default and add default 400 produces if Definition.ThrowIfValidationFails == true? To me, it would seem good since that's what the validation failure is returned as.

            var opts = FastEndpoints.Config.ErrOpts;
            if (opts.ProducesMetadataType is not null && Definition.ValidatorType is not null && Definition.ThrowIfValidationFails)
                b.Produces(opts.StatusCode, opts.ProducesMetadataType, "application/problem+json");

dj-nitehawk added a commit that referenced this issue Apr 20, 2023
@dj-nitehawk
Copy link
Member

done in 5.8.1.13-beta

however, FastEndpoints.ProblemDetails class is not the default.
FastEndpoints.ErrorResponse is.

if you want to make ProblemDetails the default you'd have to do this to change the default:

app.UseFastEndpoints(config =>
{
    config.Errors.UseProblemDetails();
});

@austins
Copy link
Author

austins commented Apr 20, 2023

You're right, Just tried the changes out. It works good so far. Thanks again! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request implemented requested feature has been implemented
Development

No branches or pull requests

2 participants