Eventuous / eventuous

Event Sourcing library for .NET
https://eventuous.dev
Apache License 2.0
447 stars 71 forks source link

ObjectResult is stacked twice in case of ProblemDetails and always returns BadRequest #372

Closed wpavelev closed 4 days ago

wpavelev commented 3 weeks ago

Hi there!

In the ResultExtension.cs there is a problem when creating ProblemDetails. The ResultObject is created twice. Once in line 40 and again in line 51.

public static ActionResult AsActionResult<TState>(this Result<TState> result) where TState : State<TState>, new() 
{
        return result.Match(
            ok => new OkObjectResult(ok),
            error =>
                error.Exception switch {
                    OptimisticConcurrencyException => AsProblem(error, Status409Conflict),
                    AggregateNotFoundException     => AsProblem(error, Status404NotFound),
                    DomainException                => AsValidationProblem(error, Status400BadRequest),
                    _                              => AsProblem(error, Status500InternalServerError)
                }
        );

        static ActionResult AsProblem(Result<TState>.Error error, int statusCode) => new ObjectResult(CreateResult(error, new ProblemDetails(), statusCode));

        static ActionResult AsValidationProblem(Result<TState>.Error error, int statusCode)
            => CreateResult(error, new ValidationProblemDetails(error.AsErrors()), statusCode);

        static ActionResult CreateResult<T>(Result<TState>.Error error, T details, int statusCode) where T : ProblemDetails {
            details.Status = statusCode;
            details.Title  = error.ErrorMessage;
            details.Detail = error.Exception?.ToString();
            details.Type   = error.Exception?.GetType().Name;

            return new ObjectResult(details) {
                StatusCode   = Status400BadRequest,
                ContentTypes = [ContentTypes.ProblemDetails]
            };
        }
    }

Since other ProblemDetails like 'AsValidationProblem' are returned directly, it would be consistent to return the general 'Problem' directly as well.

There is also an issue with StatusCodes return values. The CreateResult method always returns Status400BadRequest. It should pass the status codes given in the parameters.

so the code could be something like that:

public static ActionResult AsActionResult<TState>(this Result<TState> result) where TState : State<TState>, new() 
{
        //...
        static ActionResult AsProblem(Result<TState>.Error error, int statusCode) => CreateResult(error, new ProblemDetails(), statusCode);
        //...
        static ActionResult CreateResult<T>(Result<TState>.Error error, T details, int statusCode) where T : ProblemDetails {
         //...
            return new ObjectResult(details) {
                StatusCode   = statusCode,
                //...
            };
        }
    }
linear[bot] commented 3 weeks ago

EVE-186 ObjectResult is stacked twice in case of Problemdetails an returns always BadRequest