I've used Asp.net MVC for a few years now but this issue I've stumbled upon just a few days ago seems something everydayish and I wonder how come I've never bumped into it. It has to do with Asp.net MVC routing and action method selection. First of all think of default route definition that looks like this:
1: // default application route
2: routes.MapRoute(
3: "Default",
4: "{controller}/{action}/{id}",
5: new { controller = "Home", action = "Index", id = UrlParameter.Optional }
6: );
id route value is optional? Yes optional. So it should be perfectly feasible to have two action methods: one with the id parameter and one without it:
public ActionResult Index() { ... }
and
public ActionResult Index(int id) { ... }
But you've probably guessed it? This doesn't work out of the box. You'll get a runtime error stating that there are two matching action methods for the current request. A bit strange? I thought so as well. So let's try and accomplish just that!
A bit of REST background
If we look at RESTful architecture and RESTful web requests we can see that Roy Fielding defined following four request method types (GET, PUT, POST and DELETE) that can serve majority of everyday web application/service processing. Some of them are rather unusual in an Asp.net MVC application scenario so I excluded their details (they're marked as irrelevant, but you can always read their intended functionality in Wikipeadia article). Table shows two request URL examples and actions taken with each request method type.
| http://domain.com/Collection | |||
|---|---|---|---|
| GET | PUT | POST | DELETE |
| Return all collection items master view |
irrelevant | Add a new item to collection add new item |
irrelevant |
| http://domain.com/Collection/123 | |||
| GET | PUT | POST | DELETE |
| Return a particular collection item details view |
Replace existing item's data update item |
irrelevant | Delete the particular collection item delete item |
These requests cover most web applications' functionality especially when they're data-centric. Views returned by these requests may of course organise their information in a much more process-centric way (this is called information architecture and experience design)
Optional action parameter
As you can see in the upper table and example request URLs it makes perfect sense that id is an optional action parameter. If it's provided we should return a details view back to the client, and if it's not, a master view should be returned. Of course we could write a single controller action with a nullable parameter:
1: public ActionResult Index(int? id)
2: {
3: if (id.HasValue)
4: {
5: var items = /* functionality that gets collection items */;
6: return View("Master", items);
7: }
8: else
9: {
10: var item = /* functionality that gets item details */;
11: return View("Detail", item);
12: }
13: }
Why doesn't this work as desired in Asp.net MVC
The objective here is to have two simple and much more maintainable controller actions:
1: public ActionResult Index()
2: {
3: var items = /* functionality that gets collection items */;
4: return View("Master", items);
5: }
6:
7: public ActionResult Index(int id)
8: {
9: var item = /* functionality that gets item details */;
10: return View("Detail", item);
11: }
- user requests a particular resource from the server;
- Asp.net MVC passes request URL to each route that is defined in the application;
- first route that doesn't return
nullsignifies a route match; returnedRouteDatainstance provides route values set as per route definition and request URL; - route data is used by Asp.net MVC handler to instantiate controller instance;
- controller creates an instance of
ControllerActionInvoker; ControllerActionInvokertries to find a matching action by executing the process of this flow diagram (diagram is taken from a different blog post: Custom action method selector attributes in Asp.net MVC)
- when action is found it gets executed along with any action filters associated with it;
- etc.
- action name
- action method selectors
What's the solution then?
This simply means that we need to write an action method selector attribute that would examine route data and check for existence of certain values. Our couple of action methods could then easily be defined this way:
1: public ActionResult Index()
2: {
3: var items = /* functionality that gets collection items */;
4: return View("Master", items);
5: }
6:
7: [RequiresRouteValues("id")]
8: public ActionResult Index(int id)
9: {
10: var item = /* functionality that gets item details */;
11: return View("Detail", item);
12: }
id is present the second action will get executed returning collection item details view. And when there's no id the first action will get executed displaying the collection master view. Great!
Custom action method selector class
All I have to provide here is custom action method selector code. Actually I've even added a bit more functionality so it's possible to check for route values, form fields and query variables. All three are being checked by default, but you can exclude form fields and/or query variables if you want to by simply setting appropriate attribute properties. Here's how the code looks like:
1: /// <summary>
2: /// Represents an attribute that is used to restrict action method selection based on route values.
3: /// </summary>
4: [SuppressMessage("Microsoft.Design", "CA1019:DefineAccessorsForAttributeArguments")]
5: [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
6: public sealed class RequiresRouteValuesAttribute : ActionMethodSelectorAttribute
7: {
8: #region Properties
9:
10: /// <summary>
11: /// Gets required route value names.
12: /// </summary>
13: public ReadOnlyCollection<string> Names { get; private set; }
14:
15: /// <summary>
16: /// Gets or sets a value indicating whether to include form fields in the check.
17: /// </summary>
18: /// <value><c>true</c> if form fields should be included; otherwise, <c>false</c>.</value>
19: public bool IncludeFormFields { get; set; }
20:
21: /// <summary>
22: /// Gets or sets a value indicating whether to include query variables in the check.
23: /// </summary>
24: /// <value>
25: /// <c>true</c> if query variables should be included; otherwise, <c>false</c>.
26: /// </value>
27: public bool IncludeQueryVariables { get; set; }
28:
29: #endregion
30:
31: #region Constructors
32:
33: /// <summary>
34: /// Initializes a new instance of the <see cref="RequiresRouteValuesAttribute"/> class.
35: /// </summary>
36: private RequiresRouteValuesAttribute()
37: {
38: this.IncludeFormFields = true;
39: this.IncludeQueryVariables = true;
40: }
41:
42: /// <summary>
43: /// Initializes a new instance of the <see cref="RequiresRouteValuesAttribute"/> class.
44: /// </summary>
45: /// <param name="commaSeparatedNames">Comma separated required route values names.</param>
46: public RequiresRouteValuesAttribute(string commaSeparatedNames)
47: : this((commaSeparatedNames ?? string.Empty).Split(','))
48: {
49: // does nothing
50: }
51:
52: /// <summary>
53: /// Initializes a new instance of the <see cref="RequiresRouteValuesAttribute"/> class.
54: /// </summary>
55: /// <param name="names">Required route value names.</param>
56: public RequiresRouteValuesAttribute(IEnumerable<string> names)
57: : this()
58: {
59: if (names == null || names.Count().Equals(0))
60: {
61: throw new ArgumentNullException("names");
62: }
63:
64: // store names
65: this.Names = new ReadOnlyCollection<string>(names.Select(val => val.Trim()).ToList());
66: }
67:
68: #endregion
69:
70: #region ActionMethodSelectorAttribute implementation
71:
72: /// <summary>
73: /// Determines whether the action method selection is valid for the specified controller context.
74: /// </summary>
75: /// <param name="controllerContext">The controller context.</param>
76: /// <param name="methodInfo">Information about the action method.</param>
77: /// <returns>
78: /// true if the action method selection is valid for the specified controller context; otherwise, false.
79: /// </returns>
80: public override bool IsValidForRequest(ControllerContext controllerContext, MethodInfo methodInfo)
81: {
82: if (controllerContext == null)
83: {
84: throw new ArgumentNullException("controllerContext");
85: }
86:
87: // always include route values
88: HashSet<string> uniques = new HashSet<string>(controllerContext.RouteData.Values.Keys);
89:
90: // include form fields if required
91: if (this.IncludeFormFields)
92: {
93: uniques.UnionWith(controllerContext.HttpContext.Request.Form.AllKeys);
94: }
95:
96: // include query string variables if required
97: if (this.IncludeQueryVariables)
98: {
99: uniques.UnionWith(controllerContext.HttpContext.Request.QueryString.AllKeys);
100: }
101:
102: // determine whether all route values are present
103: return this.Names.All(val => uniques.Contains(val));
104: }
105:
106: #endregion
107: }
1: // requires "id" route value
2: [RequiresRouteValues("id")]
3: public ActionResult Index(int id) { ... }
4:
5: // useful for separating "create" and "update" action methods
6: [RequiresRouteValues("item.Id")]
7: public ActionResult Save(Item item) { ... }
8:
9: [RequiresRouteValues("id1, id2")]
10: // is identical to
11: [RequiresRouteValues(new { "id1", "id2" })]
12: public ActionResult Index(int id1, int id2) { ... }
13:
14: // strictly checks route values only
15: [RequiresRouteValues("id", IncludeQueryVariables = false, IncludeFormFields = false)]
16: public ActionResult Index(int id) { ... }
Important sidenote about action method selectors
I can't stress enough how useful action method selector attributes are. They can really help you keep action methods very very simple and clean while keeping things reusable in other parts of your application as well. Think of them when you start writing code branches due to multi-faceted nature of your action methods. These complexities can probably be avoided in a way that improves your code and not the other way around.
Additional improvements on the RESTful front
I should point out that default route defined as
{controller}/{action}/{id}
will not give you the possibility to make requests as shown in the upper table of REST request. At least the second one wouldn't be consumed by this default route definition. If we wanted to create a pure RESTful URL request pattern as described by Fielding, we should change our routing definition. But this is out of scope of this blog post. It's highly likely I'll try and write a proof of concept application that will work exactly as described by Roy Fielding. So if you're interested check back later.
This is quite an interesting solution, which albeit I've encountered already, I've found it presented very clearly here. My only question though is why wouldn't the default implementation of the action invoker check for such parameters? Maybe the surplus overload for basic websites is too much to worth the added functionality? Or are there some issues or quibbles I'm failing to grasp?
ReplyDeleteIn any case, again my compliments to you for the good article ;)
@macco: Thanks for the compliments and kind words.
ReplyDeleteLet me try to answer your question why default controller action invoker doesn't check parameters. The main reason is overhead. Imagine how it should actually work?
1. It should be checking all parameters because it wouldn't know which ones are relevant.
2. It should be smart enough to leave out optional ones (should work hand in hand with validation which is by itself optional and pluggable)
3. We should be decorating all action method parameters with some sort of PrefixAttribute-like information, because some data may be provided as user.Id or simply as Id
4. etc.
These are just a few reasons why default controller action invoker doesn't do this out of the box. As you may see it would become very complex and due to checking everything on every request it would have much more overhead.
My attribute on the other hand should only be used on those action methods that need it and should provide only those parameters that are required for it to differentiate actions between each other. Basically all it does is it moves the inner action code branch outside of it.
And BTW: Asp.net MVC team left out other useful things as well. They've been adding them through versions but some of them are still not in (consider another action method selector of mine: AjaxAttribute that makes it possible to distinguish Ajax vs. normal HTTP requests which I also tend to use quite frequently)
Your observations are certainly pertinent, but what I was actually thinking is somewhat less hefty. I mean, the system has to choose a route (or it is given one, e.g. through an action link) before actually invoking anything, so in theory it has information about the route's tokens, such as controller=Home, action=Index and so on. These parameters have names, so I was thinking that they could be matched against your actions' parameters' *names*, nothing more.
ReplyDeleteFor the sake of clarity, suppose you've got two List actions, the first of which takes no parameters, since its only purpose is to display a list of something (let's say products). However, the second List method (the overloading one) takes an int parameter in input, named categoryId, and only displays products from a specified category. The invoker could then work as usually to find actions which match the request, and then pick up the correct one based on the data available. So if the route specifies a categoryId token, the List method of choice would be the second, and the same applies if this token is provided by posted values or a query string (more or less as your action selector does).
Of course problems could arise, for example, if the user somehow provides data through multiple sources, i.e. in the route, in a posted form and so forth. The invoker wouldn't know which values to use (but I suspect the same could happen with your method, which only check for present keys, not for duplicates.
Moreover, there could be scenarios in which two or more overloading methods declare parameters with the same name and different types. I believe this is a quite unlikely setting, but never say never.
I don't know if I've made myself clear, if I didn't please let me know :D
@macco. You've made yourself perfectly clear. I hear you yes. The short answer is: No, Asp.net MVC doesn't support any parameter checking in action selection phase.
ReplyDeleteYou're right that route values could be checked. But are not. And you're also correct when you assume that my selector doesn't account for duplicates. There are two things I'd like to point out related to this:
1. The first one being that with duplicate keys you have to think about default model binder as well. Which value would get model bound depends on its code and source precedence (route value, form, query string). I haven't checked its code so I can't say which ones take precedence.
2. My attribute has the capability of checking only route values, and then you can exclude either form values and/or query string values. By default they're all included, but you can exclude the latter couple. Unfortunately you can't exclude route values because it made no sense to me.
Regarding type checking I could say yes that's usable... A different action selector could be added that pairs regular expressions against parameters (or their properties). You have to understand that all values come in as strings and have to be converted to their correct strong type. So checking them against regular expressions would be the most sensible thing to do. You could then provide whatever you like.
And actually you know what? This is a very likely scenario especially when you do something like this in your application:
/books/list
/books/list/1
/books/list/horror
The first one would list all books, the second one would display a particular book, and the third one would list a category. Such route definition wouldn't have any constraints whatsoever.
Type checking (or regular expressions matching) does make sense. If I happen to need one, I'll write it and provide its code on my blog.
@macco: FYI: If you're using my attribute I've slightly changed its code: method IsValidForRequest isn't using IList<T> any more which was used to avoid for loops when adding items to HashSet. I've looked up more info and used a different method that also inserts multiple items at once but this time directly into HashSet. A little less overhead...
ReplyDeleteThanks for the info ;)
ReplyDeleteAnd of course thanks for the tips, one never stops learning...
I appreciate the blog post. One thing I'm seeing today.
ReplyDeleteI have this route defined before my default route:
routes.MapRoute(
"Single",
"{controller}/{id}",
new { action = "index" }
);
The problem I was running in to, was that I had 2 index methods
public ActionResult Index()
and public ActionResult Index(long id)
I put your attribute like this on the second one
[RequiresRouteValues("id")]
It never worked. As I traced it down, I saw that the last line of the IsValidForRequest
return this.Names.All(val => uniques.Contains(val));
Would always be false because the RouteData.Values.Keys had 3 values, Controller, Action, Id. As soon as I decorated my attribute with RequiresRouteValues("controller, action, id") it started working.
However, in your example, all you're putting is the id. Does it have to do with my custom route?
I'm using .Net 4 and MVC 3 if that matters at all.
Thanks
@Nate: That call can't be false since you're checking whether all your required names (in your case it was only id) are present in RouteData.Values which contained three including your id. So that check in the last line works as expected.
ReplyDeleteWhat error exactly are you getting back? 404 or multiple actions for request URL? Or anything else?
I was getting a duplicate action error. As if it couldn't match my RequiresRouteValue(id). It didn't know how to distinguish between it and the other Index method.
ReplyDeleteDoesn't the this.Names contain all 3, i.e. controller, action, id. I believe when I stepped through the code it did. But uniques only contained Id, so names.All returned false.
I don't remember exactly, as it's been a few days and I'm not in front of the source code. I know that if I removed controller & action from the Names collection, then everything worked.
@Nate: If you're getting duplicate error it simply means that your parameterless action Index() also has some action method selector attribute on it. If it didn't then action with such parameter would be automatically selected. Check upper action selection flow diagram and you'll see that if you have only two action methods and only one would have action method selectors on it, that that's the one that would be executed.
ReplyDeleteBut maybe you're having even more than just two Index() action method of which one has no attributes but others do? In this case the problem is distinguishing between those that have attributes on them.
What could be done? As suggested in my post, you could make a reversed functionality action method selector attribute called RejectsRouteValuesAttribute that would check whether certain parameters are not present. This is of course necessary when you have more than two action methods with the same name and some of them share a similar subset of action parameters (while having other action selectors that return identical confirmation for all).
You could of course extend this existing attribute class to be able to process both: required and rejected values. And rename it to RouteValuesPresenceAtribute or similar.
I'm not not sure if you've actually read Fieldings dissertation, or not. I would venture to say that you did not. What you described is not a Restful service at all.
ReplyDeleteWhat I described is just an Action Method Selector Attribute that makes it easier to write RESTful services. I never said any of the upper code represents a RESTful service.
DeleteYou see Asp.net MVC is regarded as a platform which delivers RESTful services to wide audiences but that is not the case. It does give you the ability to write RESTful services but you have to write your code to make them this way. My attribute makes is a little easier to accomplish this.
I'm sorry if I made you believe that any of my code is directly RESTful.
And to be fair it's also much more complicated to write business- or user-centric RESTful services than data-centric ones. Everyday web applications with visual UI are rarely data-centric. And I'm glad they're not. API services are more likely to be that way but these days one would rather use Web API or WCF instead of Asp.net MVC.
I have one addition to make to this.
ReplyDeleteSometimes you need to handle QueryString params that were sent without a value, like ?string1=something&string2&string3&string4=somethingelse
using QueryString.AllKeys will return a single 'null' key for these.
to fix this, add the following to the if(this.IncludeQueryVariables) conditional:
// support query string params that don't send values (show up as 'null' for the key
uniques.UnionWith(controllerContext.HttpContext.Request.QueryString.GetValues(null));
just to add to that previous comment, you need to make sure GetValues doesn't return null before Unioning with the uniques collection...
ReplyDeleteif (controllerContext.HttpContext.Request.QueryString.GetValues(null) != null)
uniques.UnionWith(controllerContext.HttpContext.Request.QueryString.GetValues(null));