AutoWrapIgnore (and RequestDataLogIgnore) not working with metadata, and can be simplified

[abelfiore]

Have you ever tried to apply AutoWrapIgnore with the MS MapHealthChecks endpoints? In this case, there is no controller, and so the attribute needs to be applied programmatically. In this case, it doesn't seem to work for me. The health check endpoints are wrapping and logging. I also tried applying the RequestDataLogIgnore attribute in the same way.

 app.UseEndpoints(endpoints =>
        {
            endpoints.MapHealthChecks($"/{Health}/{HealthCheck.Live}", 
                new HealthCheckOptions()
                {
                    Predicate = (check) => !check.Tags.Contains(HealthCheck.Ready),
                    ResponseWriter = HealthCheckExtensions.WriteHealthCheckLiveResponse,
                    AllowCachingResponses = false
                }
            )
            .WithMetadata(new AutoWrapIgnore() { ShouldLogRequestData = false });
        });

[abelfiore]

I think I better understand the issue now. Basically, when you associate attributes with endpoints, they are considered metadata. There is an easy way, in your middleware, to check and see if that metadata exists (without needing to add your attributes to the request header - like you are doing in AutoWrapIgnore and RequestDataLogIgnore). It looks something like this:

                var endpoint = context.GetEndpoint();
                if (endpoint?.Metadata?.GetMetadata<AutoWrapIgnore>() is object)
                {
                    await awm.RevertResponseBodyStreamAsync(memoryStream, originalResponseBodyStream);
                    return;
                }

Your code currently looks like this:

                var actionIgnore = context.Request.Headers[TypeIdentifier.AutoWrapIgnoreFilterHeader];
                if (actionIgnore.Count > 0) 
                { 
                    await awm.RevertResponseBodyStreamAsync(memoryStream, originalResponseBodyStream);
                    return;
                }

It seems as though you can simplify your attribute classes by eliminating all the request header logic (the attributes no longer need to implement IActionFilter), and just look for this metadata instead in the endpoint. I grabbed your code, made this change myself and did a simple test to verify everything still works (for controller based endpoints, and programmatic ones like healthcheck too).

[proudmonkey]

Hi @abelfiore - Thank you for your feeback and looking into this. I appreciate it. Does your changes work if you have an endpoint that don't need to be wrapped and also don't want to log the data in the requests:

[HttpGet]
[AutoWrapIgnore(ShouldLogRequestData = false)]
public async Task<IEnumerable<PersonResponse>> Get()
{
     // Rest of the code here
}

I haven't looked the code in details because I still have a lot of work to do on my plate but I really like the approach you presented. If all possible tests will work with this change then I will merge your code :)

[abelfiore]

I think I would recommend removing the ShouldLogRequestData property from AutoWrapIgnore, and just have people use RequestDataLogIgnore. It seems strange to me for this "logic" to live in two different places. For example, what if someone uses RequestDataLogIgnore, but then does [AutoWrapIgnore(ShouldLogRequestData = true)]? This makes things even more simple, and eliminates any possible confusion. And... you are right, my PR does not handle ShouldLogRequestData... yet.

[proudmonkey]

Agreed on simplifying things :) Could you please post the test scenarios you've tested for reference? I will try to revalidate them within the week and merge the code.

Thank you so much for your valuable contribution!

[abelfiore]

I tested these scenarios manually

• Controller based [AutoWrapIgnore] -- 200 response -- 400 validation issues in request -- 500 throw exception

• Programmatic based (WithMetadata on my health check endpoints) -- 200 response -- 500 throw exception

Note my PR does not include removing the ShouldLogRequestData from AutoWrapIgnore, as I'm suggesting.

[proudmonkey]

Thank you! I've merged your PR and will start verifying some scenarios. I have also added you to the Contributors list :)

[proudmonkey]

@abelfiore as a quick update, I've updated the following code for ShouldLogRequestData method:

if (_options.ShouldLogRequestData)
{
        var endpoint = context.GetEndpoint();
        return !(endpoint?.Metadata?.GetMetadata<RequestDataLogIgnoreAttribute>() is object);
}

The change there was added ! so it will log all request when [RequestDataLogIgnore] is not applied. Otherwise, it behaves the opposite.