locked
Preview 2 URL Bug RRS feed

  • Question

  • User-1902356349 posted

    Hi Phil, Rob, & team. hope you're all having a good time at MIX.

    Just had a first look at Preview 2 and there's a lot of fixes in there, Thanks. But I'm not sure you fully understood some of what i was saying in this thread http://forums.asp.net/t/1197244.aspx (MVCToolkit URL Bugs)

    The premise is simply: "If I change the routes, I don't break the view".

    For example, assume our one and only route is

          routes.Add( new Route( "{a}/{b}", new MvcRouteHandler() )
          {
            Defaults = new RouteValueDictionary( new { controller = "home", action = "index", a = "5", b = "5" } ),
          } );

    Now, assuming we are browsing at /9, the following urls are generated...

    1. <%= Url.Action("index", new { A = 1, B = 2 }) %>  --> /1/2
    2. <%= Url.Action("index", new { A = 3 }) %>  --> /3
    3. <%= Url.Action("index", new { B = 4 }) %>  --> /9/4

    Url 1 will always work, but as you can see there is a semantic difference between Url 2 and Url 3. In Url 2, "A is always 3, and B is always default", and in Url 3 "B is always 4, and A is either default OR if A is present in the URL use that value". If we change our Route to "{b}/{a}", these semantics switch over.

    If we rely on this semantic difference when building our site, we break our site when changing our routes.

    I suggest that Url 3 should generate /5/4 since 5 is the default for A.


    That being said there should be a nice way of retrieving the CurrentOrDefault value of a part of the url so that you can explicitly support the current behaviour.

    Thanks for your time and i look forward to your responses.

    Thursday, March 6, 2008 9:45 AM

All replies

  • User-1902356349 posted

    tagged 

    Friday, March 7, 2008 8:01 AM
  • User-1660457439 posted

    Thanks for your feedback! My current understanding is that we process current values, then explicitly provided values, then defaults, in that order. This may seem confusing at first, but consider these scenarios for {a}, {b}, {c}, and {d}, where they are specified in the route in that particular order.

    Assume that the current values for each of the above are 1, 2, 3, and 4, respectively. Also assume that the default values are 5, 6, 7, and 8, respectively.  Imagine you call Url.Action(), passing in only B = 9.

    First, we create the route using only the current values. In the example above, we'll change {a}/{b}/{c}/{d} into 1/2/3/4. Then, we replace the current values with the explicit values given to us and remove everything after that. This means that 1/2/3/4 changes to 1/9/xx/xx. Finally, we replace the missing values with the defaults from the route. This means that 1/9/xx/xx becomes 1/9/7/8. But since {c} and {d} are now their default values, we can just send the URL 1/9 to the client.

    If you instead write the route as {b}/{a}/{c}/{d}/, then we use the explicitly provided value for {b} and the defaults for {a}, {c}, and {d}.

    The reason for this is that it makes the majority-usage case (having the same controller while naming a different action) easiest for developers to write.  There are ways to get the current tokens from the active route if you wish to supply them to Url.Action.  Was your concern that the syntax for pulling this tokens from the current route is too unwieldly or too obscure, or did you have a different concern?

    Friday, March 7, 2008 2:47 PM
  • User-1902356349 posted

    Firstly, do you agree with the statement "My app shouldn't break if i change my routes"?

    We both agree about what is happening. When it's {a}/{b}/{c}/{d} in your example A is the "current" value, but when it's {b}/{a}/{c}/{d} A is "default".

    This is want i'm saying is a bug. I've changed the routes and broken my app.

     

    I would consider the controller to be special. Such that if you wrote Url.Action( "List" ) it would perform the List action on the current controller. everything else is either default, or explicitly specified.

     

    (However i wouldn't be against requiring the developer to always explicitly specify the controller. Makes it easier to follow the intent of the code)

    Friday, March 7, 2008 10:25 PM
  • User-1902356349 posted
    What is your opinion on this?
    Sunday, March 9, 2008 10:12 PM
  • User-1660457439 posted

    Firstly, do you agree with the statement "My app shouldn't break if i change my routes"?

    I do not agree with this statement.  The ordering of parameters in a route specifies dependencies, so changing the ordering changes the dependencies.  If parameter {a} occurs to the left of parameter {b} in some route, then this tell us that {b} is dependent on {a}.  Therefore, if we change {a}, we've invalidated {b}.

    This might be easier to see by example.  Take the route /blog/{year}/{month}/{day}, where you're currently viewing /blog/2008/03/10.  If you call Url.Action() and provide us the value year = 2007, then we'll invalidate the current month (since it was dependent on the year 2008) and the current day (since it was dependent on the year 2008 and the month 03).  The generated URL will be /blog/2007, with no month or day component specified.  This behavior makes more sense in this scenario than does /blog/2007/03/10.

    In the example you gave above where you're saying the action List should be applied to the current controller, you're assuming that the controller is not dependent on the action, so changing the action shouldn't invalidate the controller.  However, by specifying a route of /{action}/{controller}, you effectively told the routing system that the controller is dependent on the action, therefore changing the action invalidates the current controller.  The behavior you are seeing is by design.

    (However i wouldn't be against requiring the developer to always explicitly specify the controller. Makes it easier to follow the intent of the code)

    Page developers are most certainly welcome to do this. :)  There exist overloads for Url.Action() that allow you to specify the controller explicitly.

    Monday, March 10, 2008 2:57 PM
  • User1553215731 posted

    Hello again :). I apologize for the late delay here - MIX and ISP issues kept me from responding.

    Levi - thanks for jumping in here - tgmdbm and I have been running this dialog for a while and perhaps I can short-circuit the ensuing dialog here and just assert that work on Routing is ongoing - it is by far (in my opinion) our biggest challenge.

    I've sent the details here to Phil and team - will discuss internally and I may have more questions for you, which I'll post here. Thanks for the details! 

    Monday, March 10, 2008 3:25 PM
  • User-1660457439 posted

    it is by far (in my opinion) our biggest challenge

    Oh yes.

    To the original poster - I should clarify that my response was talking to Routing in general, not to MVC specifically.  As of Preview 2 the Routing engine and MVC are separate assemblies, so Routing doesn't know that it's actually being used by an MVC application, hence it doesn't know that the {controller} parameter is special in regard to MVC applications.  We're still debating the interactions between components; however, the functionality you requested would have to be done at the MVC level, not at the Routing level.  We are actively listening to feedback, so don't assume that my comments above are the final word on this matter.  They should be taken as an explanation for why things are the way they are currently, not as an "I say so and that's final" statement. :)

    Monday, March 10, 2008 4:14 PM
  • User1220545395 posted

    Thanks for the feedback! I would say in many cases, changing your routes would not break your app. But there are definitely certain cases in which it certainly would. For example, the way you changed your route is problematic. In routing, we require that if a url parameter has a default value, then any parameter after that should have a default, otherwise your default value is really not a default. A middle parameter with a default value just doesn't work. An example should illustrate this. Consider the following route:

    {a}/{b=123}/{c}

    This is my shorthand notation for a route with the URL = "{a}/{b}/{c}" and {b} has a default value of 123. What happens when the following url comes in "/foo/bar" we don't match because we try to fill in the parameters left to right. You might suggest we try and perform a best match. For example, should we match and assume that a=foo, b=123, c=bar? If so, what about if we had the following route defined?

    {a}/{b=123}/{c}/{d=xyz}

    If we tried best match, the request for the url /foo/bar/baz could match such that a=foo,b=123,c=bar, d=baz or it could be a=foo, b=bar, c=baz,d=xyz? There's an ambiguity here.

    In order to avoid ambiguity and confusion and keep things relatively simple, we simply stated that since we match left to right, in order for a default value to be effective, all parameters to the right of that default should also have defaults.

    Hence, if you have the following route:

    {a}/{b=123}

    and simply reorder it like so:

    {b=123}/{a}

    Then we'd expect your app to possibly break. You would also want to switch which value has a default. For example, maybe your app wouldn't break if you could provide a default for a in this case.

    {b}/{a=xyz}

    Since routing in MVC represents a way to call methods on an object, we sort of expect that changing routes should be done carefully and ideally with unit test coverage. Think of it another way, it's kind of like the URL is a method signature. If you re-ordered parameters of a method, you wouldn't expect your application to just continue working.

    I hope that helps clear this up a bit.

    Monday, March 10, 2008 7:09 PM
  • User-1902356349 posted

    Thanks's Levi, Rob, and Phil for your comments.

    Let me try to come back. 

    If you re-ordered parameters of a method, you wouldn't expect your application to just continue working.
     

    True, but routing is different. With routing we have a chance to reorder incoming parameters and outgoing urls in one fell swoop such that our views and controllers don't feel anything.

    I don't agree that, in your example Phil, /foo/bar would match a=foo, b=123, and c=bar. It would try to match a=foo and b=bar (as you would expect), but because c isn't specified and doesn't have a default value it wouldn't match against that route. That is perfectly fine and I have nothing against that. This obviously implies a "left-to-right"ness of default values, and will affect the routing based on the order of the parameter. And that's fine also, let me explain.

    I've always referring to "Changing my routes" but never fully qualified that. Take a simple example, when changing from the standard mvc route, to restful routes it looks a little like this. (forgive the oversimplification, this is not a demo of rest)

    before:
          routes.Add( new Route( "{controller}/{action}/{id}", new MvcRouteHandler() )
          {
            Defaults = new RouteValueDictionary( new { action = "index", id = "" } ),
          } );

    after:
          routes.Add( new Route( "{controller}/list/", new MvcRouteHandler() )
          {
            Defaults = new RouteValueDictionary( new { action = "list" } ),
          } );
          routes.Add( new Route( "{controller}/{id}/{action}", new MvcRouteHandler() )
          {
            Defaults = new RouteValueDictionary( new { action = "index" } ),
          } );

    ... Because moving the {id} would potentially create a url like /products//list or /products/0/list we've had to create a new route that handles the list action, because {id} should be empty for that action. So the act of "Changing my routes" has involved moving parameters around and creating a new route such that i don't generate faulty Urls. And after I'm done changing my routes, My app should just work.

    This is a simple example, but you get the idea. The problem is not that a route may no longer match, or no longer creates valid Urls.

    The problem arises when you change a Route such that it WILL still match. As in my first example, where the route is "{a}/{b}" and you change it to "{b}/{a}", where both {a} and {b} have a non-empty default.

     

    As for Levi's comment. Of course, calling Url.Action with Year = 2007 will generate /blog/2007 not /blog/2007/03/10. However, what you would say is that calling Url.Action with Month = 5 would generate /blog/2008/05/ because the year would be taken from the "current values". I'm effectively saying that the only "current value" that we should pass to the Routing engine should be the controller. So in my eye's calling Url.Action with ONLY Month = 5 isn't valid. You Should have to specify both Year = 2008 and Month = 5 to generate that Url.

    Not only is that more explicit and easy to follow. It also means that if you really wanted, you could change that Route into the three Routes blog/{day}/{month}/{year} + blog/{month}/{year} + blog/{year} and it would still work.

     

    Let me know if i've answered your concerns. Or raised new ones, :).

    Monday, March 10, 2008 10:35 PM
  • User1220545395 posted

    Hmm, I still am not sure I'm getting exactly what you're saying. It sounds to me that you want us to treat "{controller}" special. But even if we did that, there are still situations in which re-ordering your routing parameters will cause application breaks. I think we've established that.

    Routing doesn't know anything about MVC as I wrote here, so we can't treat {controller} special. Also, does doing that really solve anything? I think it'd be dangerous to say that reordering your parameters is a *safe* operation. I'm not sure we could ever guarantee that because you have to consider the interaction of the route with any route that precedes it. For example, throw constraints into play and you can see how reordering a parameter could suddenly have a route would get matched by a preceding one.

    Changing routes is a code change and should have test coverage to make sure you haven't introduced an inadverdent bug. There is so much that can go wrong in changing a route.

    By the way, I've implemented some restful routes which I will post on my blog soon. In the case you specified, because {id} and list could conflict, I would put a constraint on {id} so that it only matches digits.

    The reason why is "list" is not a special case. Looking closely at how the simply restful plugin was incorporated and adapted to Rails, as an example, you'll notice the following urls.

    /users/1

    /users/filter

    /users/1/edit

    The thing is, "filter" isn't special. It also allows you to map things like

    /users/customaction

    Since Rails 2.0 replaced the semicolon with the slash character as the separator.

    I haven't looked at the rails source, but it seems to me the only way to make this happen is by creating the following.

    /users/{id}/{action=show}  Constraints=new{id=@"\d+"}

    /users/{action=index}

    That's my shorthand where id is required to be one or more digits. Does that make sense?

    Tuesday, March 11, 2008 1:07 AM
  • User-1902356349 posted

    Initially, i never thought {controller} would be special. That was just a compromise when Levi implied people would want to do Url.Action("Edit", ...) and expect the current controller.

    All I'm really asking for is one thing. Route.GetVirtualPath ignores the current context. Nothing about the URL it generates should be dependant on the URL you're currently browsing.


    However, it is possible to make {controller} "special" outside of Routing by saying if( values doesn't contain "controller" ) add the current controller to values before passing the values on to the routing engine.

    I think it'd be dangerous to say that reordering your parameters is a *safe* operation.

    Absolutely. It's very dangerous, if a parameter has an empty string Default, then reordering will break things, in which case you'll typically add new routes to account for the breaks. But for the case where all parameters have a non-empty deafult, it IS possible to gauantee that reordering is safe (ignoring constraints for a moment). The fact is, that even in this simple case, we do break.

    Yes, my example was overly simple and flawed, but it was invented to prove a point, not to explain restful routes.

    Yours does make a lot of sense.

    Tuesday, March 11, 2008 2:18 AM
  • User1651823181 posted

    Thanks Phil for the good example.

    In my opinion we should make some assumtions about the URL that stay true no matter what:

    1. The URL should be treated as a signature to the method call, change the order of your parameters should break the call to the controller's route

    2. Defailt values that are specified in the controller should be just that, default values if no parameter value has been specified. So for {a}{b=123}{c}{d}:

    /house/room/bed/pillow maps to a=house, b=room, c=bed, d=pillow

    /house/room/pillow maps to a=house, b=room, c=pillow, d=null - should generate an error since no default value has been specified for d. Default values should be treated such that if no default value is provided a required/mandatory entry in the signature is assumed

    /house//bed/pillow maps to a=house, b=123, c=bed, d=pillow

    Phil, so in your example, /foo/bar/baz should match such that a=foo, b=bar, c=baz,d=xyz. Does it mean my route will break if I change the order, sure, just like it would if I changed the signature to a method call.

    Tuesday, March 11, 2008 9:58 AM
  • User-1902356349 posted

    Hi Renso, welcome to ASP.NET Forums 

    1. absolutely not, for reasons discussed in this thread.

    2. absolutely. although, no error is generated, the route simply doesn't match. also, I'm not sure whether // is allowed in a URL?

    Tuesday, March 11, 2008 10:19 AM
  • User1768016292 posted

    All I'm really asking for is one thing. Route.GetVirtualPath ignores the current context. Nothing about the URL it generates should be dependant on the URL you're currently browsing.

     A big +1 from me on that one too.

    With all this talk about REST, it's interesting that we seem to forget that the URL of a resource is really specific to the resource (and thus your model, or at least some layer between the model and the view, but that gets messier) - I shouldn't have to have a current context to figure out the URL of a resource.

    Thursday, March 13, 2008 8:57 AM
  • User1127880124 posted

     +1 on not requiring a context to get a url. 

    I have a service layer as part of my application, and it knows nothing of the views/view context.  I was looking for a straightforward way to get a url for a specific controller and action, but there doesn't seem to be a trivial way.  If I'm wrong, please let me know.  I wrote about it here.  Maybe an extension method to ControllerBase is needed that hacks around this?
     

    Thursday, March 13, 2008 9:32 PM
  • User-1902356349 posted

    A big +1 from me

    Is that like a +2? lol.

    Did you know 2 + 2 = 5?

     

    ... for sufficiently large values of 2. ;)
     

    Thursday, March 13, 2008 11:13 PM
  • User1220545395 posted

    Hmmm, I think getting values from the context is a great convenience and important for view re-use or view component reuse. Imagine you have a view user control with a list and paging. The pager link might look like this:  

    Html.ActionLink(new {page=pageNumber})  //Pretend pageNumber came in as a parameter.

    If we didn't use values from the context, you'd have to specify the controller and action that links to. But you don't know that because the view user control might be used in multiple views. Those views might be populated by different controllers and action. 

    By using the context (we refer to it as "ambient" values) to supply the ambient values, you only specify what is actually *changing* in your call to action link. If we didn't use the context, these scenarios become really cumbersome.

    Also, there's a simple workaround if you don't want the "ambient" values to be used (aka the values in the context). Just make sure to specify all parameters when generating urls.

    Phil

    Friday, March 14, 2008 3:40 AM
  • User1768016292 posted

    Is that like a +2? lol.

    Yeah, exactly! :) 

    Friday, March 14, 2008 6:47 AM
  • User1768016292 posted


    Hmmm, I think getting values from the context is a great convenience and important for view re-use or view component reuse.

    I completely agree about having that convenience - but I don't think there should be an explicit dependency on the context just for that purpose. Retrieving a URL could be layered such that we can provide the context if we want to modify current values, as in your example, but we can "plug in" one level deeper to build a URL "context free". Keep in mind that (for now) I'm just using reflector to look inside MVC, but it doesn't appear that the context is used for much other than pulling current values, and some calculation involving the constraints (that isn't even used if you're building a URL, not parsing it).


    Also, there's a simple workaround if you don't want the "ambient" values to be used (aka the values in the context). Just make sure to specify all parameters when generating urls.

    True, but that still requires that we monkey with the data model at a level that assumes an HTTP context. Granted, it's not like we can take what I'm talking about and push it down as deep as I'd like, but this will still help it move deeper - especially with the layering of routing you guys have done. 

    Friday, March 14, 2008 8:19 AM
  • User-1902356349 posted

    and some calculation involving the constraints (that isn't even used if you're building a URL, not parsing it)

    Constraints Are being used when building the Url. If the constraints don't match, then it will return null. And if called from RouteCollection.GetVirtualPath, it will move on to the next Route. Contraints won't alter the generated Url.

    but we can "plug in" one level deeper to build a URL "context free".

    +1, I definitely think supporting both behaviours is ideal. I'm just not sure how you would implement it... A ContextFreeRoute? 

    Friday, March 14, 2008 9:27 AM