ProducesResponseTypeAttribute is the only direct concrete implementation of IFilterMetadata

[tuespetre]

Another nitpicky issue from your friendly neighborhood nitpicker.

The two following interfaces are not derived from IFilterMetadata:

• IApiRequestMetadataProvider
  • Places that look for it in the action's filters:
    • DefaultApiDescriptionProvider
  • Classes that implement it:
    • ConsumesAttribute
• IApiResponseMetadataProvider
  • Places that look for it in the action's filters:
    • DefaultApiDescriptionProvider
    • FormatFilter
  • Classes that implement it:
    • ProducesAttribute
    • ProducesResponseTypeAttribute

Because of this, ProducesResponseTypeAttribute is the only direct concrete implementation of IFilterMetadata.

Now it's clear that the concept of 'filters' is overloaded to the effect that a 'filter' doesn't have to do anything, and it can instead just supply some kind of token to influence behavior or provide some kind of metadata. There are several examples of such interfaces deriving from IFilterMetadata:

• IFormatFilter
  • FormatFilter
• IAllowAnonymousFilter
  • AllowAnonymousFilter
• IAntiForgeryPolicy
  • IgnoreAntiForgeryTokenAttribute
  • ValidateAntiForgeryTokenAuthorizationFilter
    • AutoValidateAntiForgeryTokenAuthorizationFilter

Furthermore, we see several out-of-the-box examples demonstrating that implementations of IFilterMetadata do not strictly have to be attributes.

Considering that, I think that IApiRequestMetadataProvider and IApiResponseMetadataProvider should derive from IFilterMetadata in order to:

1. Be consistent with other interfaces in how they are retrieved from actions' filter collections, and
2. Have all out-of-the-box implementations of IFilterMetadata such as ProducesResponseTypeAttribute implement IFilterMetadata via a more specialized, purposeful interface rather than directly.

[rynowak]

Now it's clear that the concept of 'filters' is overloaded to the effect that a 'filter' doesn't have to do anything

This is correct, which is why it's IFilterMetadata and not IFilter 👍

The intent is that using IFilterMetadata replaces a lot of things that people used to do reflection for. When we see IFilterMetadata we cache it for you, and we also order them based on scopes, this is powerful for building all kinds of protocols to support overriding policy. See the antiforgery classes for an example.

We actually did design a set of APIs to formalize this notion of policy so that extensibility authors wouldn't have to read between the lines to figure it out. But - there are only so many hours in the day, and it ended up being pretty rocket-sciency. It's often 2 hours of work to do an initial design, and 20 hours of work to implement/test it. We ultimately decided to wait for customer feedback on this one rather than build another set of abstractions for something we already do in our own code.

As to your specific feedback - I don't recall exactly why I didn't do this. I reserve my right to change my mind about things, but I agree right now with your suggestion and your rationale.

IApiRequestMetadataProvider and IApiResponseMetadataProvider are intended to be retrieved from ActionDescriptor.Filters, so it's clear that they are filter metadata and that should be reflected by the API.

[tuespetre]

[There] are only so many hours in the day, and it ended up being pretty rocket-sciency.

😆

As to your specific feedback - I don't recall exactly why I didn't do this.

It looks like it may be loosely related to how they are sitting as siblings to other ApiExplorer interfaces which are not filters:

• IApiDescriptionGroupNameProvider
• IApiDescriptionVisibilityProvider
• IApiRequestFormatMetadataProvider
• IApiResponseTypeMetadataProvider

I understand the purposes of each of these and can see how, when looking at them together, they seem like kind of their own 'thing'.

I reserve my right to change my mind about things, but I agree right now with your suggestion and your rationale.

Cool... I will submit a PR for it and y'all can just do whatever you will! 😸

[Eilon]

I think I understand all the points made here, and I think I even agree with them. But what does this change benefit? What task becomes easier? Or possible?

[rynowak]

This removes a potential mistake if you wanted to implement your own [Produces] or [Consumes].

[Eilon]

Ah ok, that's a good enough reason.

@ajaybhargavb I've assigned #5261 to you to review and merge.

[ajaybhargavb]

92682b7

[ajaybhargavb]

Thanks @tuespetre