Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Consider changing Attribute Route behavior with areas + ambient values #1913

Closed
rynowak opened this issue Jan 29, 2015 · 4 comments
Closed

Comments

@rynowak
Copy link
Member

rynowak commented Jan 29, 2015

Right now, link generation for attribute routes when using areas behaves very similarly to conventional routing. Much like conventional routing, when you have duplicate action and controller names you must either:

1). Be explicit about route when generating links (always specify the area token)
2). Use order to configure the desired precedence.

Example follows...

The question is.. do we want this behavior to be smarter than conventional routing? We might be able to implement something good here. There's a small tuning change we could make to this algorithm to make this work as-desired without the need for setting an order.

Example: Consider the following conventionally routed controllers:

namespace MvcSample.Web1
{
    [Area("A1")]
    public class RoutingTest : Controller
    {
        public string Action()
        {
            return Url.Action();
        }
    }
}

namespace MvcSample.Web2
{
    public class RoutingTest : Controller
    {
        public string Action()
        {
            return Url.Action();
        }
    }
}

with routes

                routes.MapRoute("areaRoute", "{area:exists}/{controller}/{action}");
                routes.MapRoute("controllerActionRoute", "{controller}/{action}",);

Each of these actions tries to link to itself, using only ambient values. If the routes are ordered as-is, then this will work as expected.

http://localhost:5001/A1/RoutingTest/Action -> http://localhost:5001/A1/RoutingTest/Action
http://localhost:5001/RoutingTest/Action ->      http://localhost:5001/RoutingTest/Action

However, if you switch the route ordering, then the controllerActionRoute is too greedy.

http://localhost:5001/A1/RoutingTest/Action -> http://localhost:5001/RoutingTest/Action
http://localhost:5001/RoutingTest/Action ->      http://localhost:5001/RoutingTest/Action

This is an inherent issue in conventional routing. You get the same issue with attribute routes when you have a duplicate action/controller. Order is needed to resolve the problem (if you're relying on ambient values).

Ex:

namespace MvcSample.Web3
{
    [Area("A2")]
    [Route("A2/[controller]/[action]")]
    public class AttributeRoutingTest : Controller
    {
        public string Action()
        {
            return Url.Action();
        }
    }
}

namespace MvcSample.Web4
{
    [Route("[controller]/[action]", Order = 1)]
    public class AttributeRoutingTest : Controller
    {
        public string Action()
        {
            return Url.Action();
        }
    }
}
@rynowak
Copy link
Member Author

rynowak commented Jan 29, 2015

/cc @dougbu - this is what you reported to me. We're as-good-as conventional routing right now, but could potentially be better.

@dougbu
Copy link
Member

dougbu commented Jan 29, 2015

my experience of this bug was frustrating -- took a while to eliminate my code as the problem, then didn't grok the [Route(... Order= {not 0}] workaround 'til I went to the font of all routing knowledge (@rynowak).

note we're not quite as good as conventional routing because the problem is less easily seen with attribute routing. conventional routes are usually defined in one or a few places; attribute routes are defined throughout the app.

@dougbu
Copy link
Member

dougbu commented Jan 29, 2015

separately could calculate how many Values matches (2 points, say) and AmbientValues (1 point) occur before an AttributeRouteLinkGenerationEntry is added to the matches. then the AttributeRouteLinkGenerationEntryComparer could use that WeightedSatisfiedConstraints value together (somehow) with the existing Order and Precedence values.

alternatively could ignore the Values / AmbientValues difference: calculate Precedence up front based on constraints in addition to their use in the route template. right now the literal template /MyArea/MyController/MyAction with 3 constraints has the same Precedence as /come/here/please with 2.

rynowak added a commit that referenced this issue Feb 10, 2015
Attribute route link generation will now have a slight preference for
entries that can use ambient values (vs ignoring an ambient value). This
means that areas will be more 'sticky' with regard to link generation
without the need to specify a better Order.
rynowak added a commit that referenced this issue Feb 12, 2015
Attribute route link generation will now have a slight preference for
entries that can use ambient values (vs ignoring an ambient value). This
means that areas will be more 'sticky' with regard to link generation
without the need to specify a better Order.
@rynowak
Copy link
Member Author

rynowak commented Feb 12, 2015

7cb6c10

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants