Adding attribute routing #720
Conversation
|
@javiercn come at me bro |
|
Would be really nice to start putting obscure implementation types in sub-namespaces - it can make code reviews easier too because it informs the reader which types are "user types" (and thus API names are more important), and which are the underlying implementation details where there's perhaps more technical logic. |
| 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.
[Route("api/Foo")] + [HttpGet("bar")] = api/Foo/bar
[Route("api/Foo")] + [HttpGet] = api/Foo
[Route("api/Foo")] + (nothing on the action) = api/Foo
(nothing on the controller) + [HttpGet("bar")] = bar
(nothing on the controller) + [HttpGet] = (not attribute routed)
There was a problem hiding this comment.
Also @javiercn there is no 'route prefix'. The [Route] attribute only goes on classes/controllers, and defines the WHOLE controller as attribute routed. If there is an attribute on an individual method/action, then its template is appended.
|
@Eilon sure. I think most of us are still following the initial guidance not to do that. If you think it's time to bucket things, then they shall be bucketed. Excelsior! |
| @@ -29,6 +29,10 @@ public static IBuilder UseMvc([NotNull] this IBuilder app, [NotNull] Action<IRou | |||
| ServiceProvider = app.ApplicationServices | |||
| }; | |||
|
|
|||
| routes.Routes.Add(AttributeRouting.CreateAttributeRoute( | |||
There was a problem hiding this comment.
CreateAttributeMegaRoute - Make it clear that this creates the mega route and not a "single template route"
|
Yeah I think the time for sub-namespaces is upon us. At this point really all code should be written in a reasonably-production-level style. |
| @@ -106,6 +111,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.
|
Branch updated. Addressed namespaces, other naming feedback and doc comments. |
|
Ping |
| var inlineConstraintResolver = services.GetService<IInlineConstraintResolver>(); | ||
|
|
||
| var entries = new List<AttributeRouteEntry>(); | ||
| foreach (var entry in routeTemplatesByGroup) |
There was a problem hiding this comment.
nit: Use a different name for the loop variable, by using entry it makes it look that you are iterating over entries.
|
|
Adds a rough cut of attribute routing for URL matching (incoming requests only).