Features/api description

[Artem-Romanenia]

By default, Asp Net Core and Api Explorer know nothing about [TranslateResultToActionResult] and what it is doing. This PR is an attempt to address this issue and its consequences.

Here, a whole Result<> object is described as response type, instead of just it`s value. 'Failure' responses are not described. This can be addressed by adding bunch of [ProducesResponseType] attributes to each endpoint.

I tried to come up with a soultion that would allow to fix this with just couple of lines of code (or just one line) in Startup file:

services.AddControllers(mvcOptions => mvcOptions.AddDefaultResultConvention());

With just this code Swagger description of the same operation turns into this:

By default, it will add all known result statuses to a description of every endpoint marked with [TranslateResultToActionResult].

If, for example, there is an app that doesn't have authentication or authorization, we can configure it like this:

services.AddControllers(mvcOptions => mvcOptions
    .AddResultConvention(resultStatusMap => resultStatusMap
        .AddDefaultMap()
        .Remove(ResultStatus.Forbidden)
        .Remove(ResultStatus.Unauthorized)
    ));

This will not include description of 401 Unauthorized or 403 Forbidden in any endpoint.

Another example is when you have an endpoint that can only return 200, 404, and 400, while another returns only 200 and 400. Here you can use [ExpectedFailures] attribute:

[TranslateResultToActionResult]
[ExpectedFailures(ResultStatus.NotFound, ResultStatus.Invalid)]
[HttpDelete("Remove/{id}")]
public Result RemovePerson(int id)
{
    //logic
}

[TranslateResultToActionResult]
[ExpectedFailures(ResultStatus.Invalid)]
[HttpPost("DoSomethingDifferent")]
public Result DoSomethingDifferent(Model model)
{
    //logic
}

You can also configure it to list different Status codes for different Http Methods:

services.AddControllers(mvcOptions => mvcOptions
    .AddResultConvention(resultStatusMap => resultStatusMap
        .AddDefaultMap()
        .For(ResultStatus.Ok, HttpStatusCode.OK, resultStatusOptions => resultStatusOptions
            .For("POST", HttpStatusCode.Created)
            .For("DELETE", HttpStatusCode.NoContent))
    ));

Here it would use 200 OK for GET, but 201 for POST and 204 for DELETE

Another configurability feature is what is returned in case of failure:

services.AddControllers(mvcOptions => mvcOptions
    .AddResultConvention(resultStatusMap => resultStatusMap
        .AddDefaultMap()
        .For(ResultStatus.Error, HttpStatusCode.BadRequest, resultStatusOptions => resultStatusOptions
            .With((ctrlr, result) => string.Join("\r\n", result.ValidationErrors)))
    ));

Also, this whole thing can be used together with [ProducesResponseType] attributes, as it will not overwrite/duplicate them if they are added as actual attributes.

If you feel this needs any changes or lacks something useful, please let me know!

[Artem-Romanenia]

I`m not quite sure why the build fails, but I think it may not be something that I can fix in a source code.

[Artem-Romanenia]

@ardalis Gentle ping

[ardalis]

Hi @Artem-Romanenia

Sorry for the delay. I like this! Can you fix the conflict (I'm not sure what to do with the 2nd one in the file)? Also, I feel like there would need to be additional docs on how to do this. You've already provided great explanations in the issue description - can you put that into the README as part of this PR, too? Thanks!

[ardalis]

Let me know when ready to merge

[Artem-Romanenia]

Hi @ardalis,

I`m glad you've liked the idea! I've fixed merge conflicts, but there's couple of things I wanted to mention:

• In my original PR, the logic responsible for extracting data from result object and transforming it into an object which is to be returned as response in failure scenarios was moved from Ardalis.Result.AspNetCore.ResultExtensions class to Ardalis.Result.AspNetCore.ResultStatusMap. Since then there was an addition to Ardalis.Result.AspNetCore.ResultExtensions, so I moved this additional logic to Ardalis.Result.AspNetCore.ResultStatusMap as well.
• In Ardalis.Result.AspNetCore.ResultStatusMap I've made a little refactoring:
  In the logic responsible for handling BadRequest, I've changed

        private static ActionResult BadRequest(ControllerBase controller, IResult result)
        {
            foreach (var error in result.ValidationErrors)
            {
                controller.ModelState.AddModelError(error.Identifier, error.ErrorMessage);
            }

            return controller.BadRequest(controller.ModelState); //Ultimately returns Dictionary<string, string[]>
        }

to

        private static ValidationProblemDetails BadRequest(ControllerBase controller, IResult result)
        {
            foreach (var error in result.ValidationErrors)
            {
                controller.ModelState.AddModelError(error.Identifier, error.ErrorMessage);
            }

            return new ValidationProblemDetails(controller.ModelState); //Returns ProblemDetails
        }

Because there was a discrepancy before, where BadRequest returned by asp net core (for example at the model binding/model validation stage) would have a ProblemDetails structure, but the BadRequest returned from code would be IDictionary<string, string[]>. I've figured code would better work in alignment with asp net core inner logic and RFC 7807. But this can potentially be a breaking change.

• As for documenting this feature, I've previously added some description to Readme, but it is phrased differently than this PR's description, and may lack some screenshots. Please check if the Readme section I've added looks good to you or let me know if there's a way I can improve it.

[Artem-Romanenia]

Hi @ardalis Everything is ready for merge on my side, you can proceed if you are ok with the things I mentioned

[Artem-Romanenia]

Noticed some activity here and fixed merge conflicts

[ardalis]

Thanks, @Artem-Romanenia ! We'll get this in the next release.

[KyleMcMaster]

@Artem-Romanenia Thanks for the contribution, I have just merged this into the .NET 7 branch and have verified the changes are working as expected. We should be able to get a release of this out next week.