dotnet / dotnet/aspnetcore

Fixing Attribute Filtering Logic in DefaultApplicationModelProvider"

Open
#54,920 0 comments 0 reactions 0 assignees View on GitHub
area-mvc design-proposal
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

## Summary

This document discusses the issue with the current logic in the code snippet provided. The issue is related to the ineffective filtering of attributes in the `filteredAttributes` list. in the code range from lines 558 to 583 in the `DefaultApplicationModelProvider` class

GitHub file link: [DefaultApplicationModelProvider.cs](https://github.com/dotnet/aspnetcore/blob/dc1acba9cd1374a8a8560bee655682e1a72de3eb/src/Mvc/Mvc.Core/src/ApplicationModels/DefaultApplicationModelProvider.cs)

## Motivation and goals

The code in lines 558 to 583 of `DefaultApplicationModelProvider` The current logic in the code snippet always adds all attributes to the `filteredAttributes` list, regardless of the conditions. This leads to incorrect behavior and inefficiency in the code. The goal is to correct the logic so that attributes are filtered based on specific conditions, improving the accuracy and performance of the code.

## In scope

- Modify the filtering logic in the specified code range to correctly filter attributes based on conditions related to `routeProvider`.
- Ensure that only attributes meeting the specified conditions are added to the `filteredAttributes` list.

## Out of scope

- Detailed refactoring of other parts of the `DefaultApplicationModelProvider` class.
- Changes unrelated to attribute filtering within the specified code range.

## Risks / unknowns

- Developers might misinterpret the intended conditions for filtering attributes, leading to incorrect behavior.
- Implementing a more complex filtering logic might introduce performance concerns if not optimized properly.

## Examples

```csharp
// Inside the DefaultApplicationModelProvider class, lines 558-583
var routeProviders = new List();

var createSelectorForSilentRouteProviders = false;
foreach (var attribute in attributes)
{
if (attribute is IRouteTemplateProvider routeTemplateProvider)
{
if (IsSilentRouteAttribute(routeTemplateProvider))
{
createSelectorForSilentRouteProviders = true;
}
else
{
routeProviders.Add(routeTemplateProvider);
}
}
}

foreach (var routeProvider in routeProviders)
{
var filteredAttributes = new List();

foreach (var attribute in attributes)
{
**if (ReferenceEquals(attribute, routeProvider))
{
filteredAttributes.Add(attribute);
}
else if (InRouteProviders(routeProviders, attribute))
{
// Exclude other route template providers
// Example:
// [HttpGet("template")]
// [Route("template/{id}")]
}
else if (
routeProvider is IActionHttpMethodProvider &&
attribute is IActionHttpMethodProvider &&
ReferenceEquals(attribute, routeProvider))
{
// Example:
// [HttpGet("template")]
// [AcceptVerbs("GET", "POST")]
//
// Exclude other http method providers if this route is an
// http method provider.
}**
// Other conditions as needed for filtering
}

selectorModels.Add(CreateSelectorModel(routeProvider, filteredAttributes));
}

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.