Adding attribute routing #720
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,17 +3,41 @@ | |
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using Microsoft.AspNet.Mvc.Routing; | ||
|
|
||
| namespace Microsoft.AspNet.Mvc | ||
| { | ||
| /// <summary> | ||
| /// Identifies an action that only supports the HTTP GET method. | ||
| /// </summary> | ||
| [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] | ||
| public sealed class HttpGetAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider | ||
| { | ||
| private static readonly IEnumerable<string> _supportedMethods = new string[] { "GET" }; | ||
|
|
||
| /// <summary> | ||
| /// Creates a new <see cref="HttpGetAttribute"/>. | ||
| /// </summary> | ||
| public HttpGetAttribute() | ||
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates a new <see cref="HttpGetAttribute"/> with the given route template. | ||
| /// </summary> | ||
| /// <param name="template">The route template. May not be null.</param> | ||
| public HttpGetAttribute([NotNull] string template) | ||
| { | ||
| Template = template; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public IEnumerable<string> HttpMethods | ||
| { | ||
| get { return _supportedMethods; } | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public string Template { get; private set; } | ||
|
There was a problem hiding this comment. During initial implementations of attribute routing in Web API, we had the templates on the verb attributes, which made the route constrained to that particular verb...when CORS feature came into picture this was problematic as now the user had to add HttpOptions verb explicitly on all the actions where he wanted to support CORS...so just FYI as a scenario to watch out for... There was a problem hiding this comment. Might also want to consider that we are adding two constraints with one attribute, will I be able to specify just [HttpGet] to indicate that only get is supported and that that won't plug in this as an attribute route? If that's the case, when you have an attribute route with a route prefix on the controller, what's the behaviour when you just say [HttpGet], will it mark it as an attribute route? (maybe you want to hit that action through convention routing) or will it not, (in which case you will need to do something like [HttpGet("")] which seems unnatural. There was a problem hiding this comment. Good feedback. We already have a conclusion from the previous design meeting. Here's the truth table.
There was a problem hiding this comment. Also @javiercn there is no 'route prefix'. The |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,9 @@ | |
| using System.Reflection; | ||
| #endif | ||
| using Microsoft.AspNet.Mvc.ReflectedModelBuilder; | ||
| using Microsoft.AspNet.Mvc.Routing; | ||
| using Microsoft.AspNet.Routing; | ||
| using Microsoft.AspNet.Routing.Template; | ||
| using Microsoft.Framework.OptionsModel; | ||
|
|
||
| namespace Microsoft.AspNet.Mvc | ||
|
|
@@ -20,16 +23,19 @@ public class ReflectedActionDescriptorProvider : IActionDescriptorProvider | |
| private readonly IActionDiscoveryConventions _conventions; | ||
| private readonly IEnumerable<IFilter> _globalFilters; | ||
| private readonly IEnumerable<IReflectedApplicationModelConvention> _modelConventions; | ||
| private readonly IInlineConstraintResolver _constraintResolver; | ||
|
|
||
| public ReflectedActionDescriptorProvider(IControllerAssemblyProvider controllerAssemblyProvider, | ||
| IActionDiscoveryConventions conventions, | ||
| IEnumerable<IFilter> globalFilters, | ||
| IOptionsAccessor<MvcOptions> optionsAccessor, | ||
| IInlineConstraintResolver constraintResolver) | ||
| { | ||
| _controllerAssemblyProvider = controllerAssemblyProvider; | ||
| _conventions = conventions; | ||
| _globalFilters = globalFilters ?? Enumerable.Empty<IFilter>(); | ||
| _modelConventions = optionsAccessor.Options.ApplicationModelConventions; | ||
| _constraintResolver = constraintResolver; | ||
| } | ||
|
|
||
| public int Order | ||
|
|
@@ -106,6 +112,8 @@ private bool HasConstraint(List<RouteDataActionConstraint> constraints, string r | |
|
|
||
| public List<ReflectedActionDescriptor> Build(ReflectedApplicationModel model) | ||
| { | ||
| var routeGroupsByTemplate = GetRouteGroupsByTemplate(model); | ||
|
There was a problem hiding this comment. Move this closer to where it's first used. There was a problem hiding this comment. 👎 this is part of three statements that initialize state for the loop, and it doesn't belong inside the loop. As discussed, if you think this code can be substantially simplified, then go ahead when you add features to it. |
||
|
|
||
| var actions = new List<ReflectedActionDescriptor>(); | ||
|
|
||
| var removalConstraints = new HashSet<string>(StringComparer.OrdinalIgnoreCase); | ||
|
|
@@ -188,6 +196,50 @@ public List<ReflectedActionDescriptor> Build(ReflectedApplicationModel model) | |
| } | ||
| } | ||
|
|
||
| if (routeGroupsByTemplate.Any()) | ||
| { | ||
| var templateText = AttributeRouteTemplate.Combine( | ||
| controller.RouteTemplate, | ||
| action.RouteTemplate); | ||
|
|
||
| if (templateText == null) | ||
| { | ||
| // A conventional routed action can't match any route group. | ||
| actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( | ||
| AttributeRouting.RouteGroupKey, | ||
| RouteKeyHandling.DenyKey)); | ||
| } | ||
| else | ||
| { | ||
| // An attribute routed action will ignore conventional routed constraints. | ||
| actionDescriptor.RouteConstraints.Clear(); | ||
|
|
||
| var template = TemplateParser.Parse(templateText, _constraintResolver); | ||
|
There was a problem hiding this comment. I guess the template is going to get parsed more than once in this case, and there is some duplicity. Shouldn't we just use the created attribute route in this case? There was a problem hiding this comment. I'd like to leave this as an exercise for @javiercn. The |
||
|
|
||
| if (template.Parameters.Any( | ||
| p => p.IsParameter && | ||
| string.Equals(p.Name, "action", StringComparison.OrdinalIgnoreCase))) | ||
| { | ||
| actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( | ||
| "action", | ||
| action.ActionName)); | ||
| } | ||
|
|
||
| var routeGroup = routeGroupsByTemplate[templateText]; | ||
| actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( | ||
| AttributeRouting.RouteGroupKey, | ||
| routeGroup)); | ||
|
|
||
| actionDescriptor.RouteInfo = new RouteInfo() | ||
| { | ||
| Precedence = AttributeRoutePrecedence.Compute(template), | ||
| Template = template, | ||
| TemplateText = templateText, | ||
| RouteGroup = routeGroup, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| actionDescriptor.FilterDescriptors = | ||
| action.Filters.Select(f => new FilterDescriptor(f, FilterScope.Action)) | ||
| .Concat(controller.Filters.Select(f => new FilterDescriptor(f, FilterScope.Controller))) | ||
|
|
@@ -214,5 +266,25 @@ public List<ReflectedActionDescriptor> Build(ReflectedApplicationModel model) | |
|
|
||
| return actions; | ||
| } | ||
|
|
||
| // Groups the set of all attribute routing templates and returns mapping of [template -> group]. | ||
| private static Dictionary<string, string> GetRouteGroupsByTemplate(ReflectedApplicationModel model) | ||
| { | ||
| var groupsByTemplate = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| foreach (var controller in model.Controllers) | ||
| { | ||
| foreach (var action in controller.Actions) | ||
| { | ||
| var template = AttributeRouteTemplate.Combine(controller.RouteTemplate, action.RouteTemplate); | ||
| if (template != null && !groupsByTemplate.ContainsKey(template)) | ||
| { | ||
| groupsByTemplate.Add(template, "__route__" + template); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return groupsByTemplate; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System; | ||
| using Microsoft.AspNet.Mvc.Routing; | ||
|
|
||
| namespace Microsoft.AspNet.Mvc | ||
| { | ||
| /// <summary> | ||
| /// Specifies an attribute route on a controller. | ||
| /// </summary> | ||
| [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)] | ||
| public class RouteAttribute : Attribute, IRouteTemplateProvider | ||
| { | ||
| /// <summary> | ||
| /// Creates a new <see cref="RouteAttribute"/> with the given route template. | ||
| /// </summary> | ||
| /// <param name="template">The route template. May not be null.</param> | ||
| public RouteAttribute([NotNull] string template) | ||
| { | ||
| Template = template; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public string Template { get; private set; } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this mean at this point (prior to removing Get as a constraint)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It means that the controller-level
[Route(...)]is used without appending anything - in this case it'sapi/REST. I wanted the sample include this since it's the obvious way to code a rest conventionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per discussion will update this to use a different action name.