U4-2823 - RenderRouteHandler.GetUmbracoRouteDefinition() fails to store the route definition when an incidental alias-controller match is made

Created by Alex Fox Gill 10 Sep 2013, 10:10:15 Updated by Shannon Deminick 11 Sep 2013, 02:17:18

The early-exit condition in the {{else}} block skips the penultimate line which adds the route definition to the data token dictionary: {{requestContext.RouteData.DataTokens["umbraco-route-def"] = def;}}

This causes an InvalidOperationException to be thrown when trying to access the {{CurrentPage}} property from a SurfaceController:

''Cannot find the Umbraco route definition in the route values, the request must be made in the context of an Umbraco request''

//check if that controller exists if (controllerType != null) { //ensure the controller is of type 'IRenderMvcController' and ControllerBase if (TypeHelper.IsTypeAssignableFrom(controllerType) && TypeHelper.IsTypeAssignableFrom(controllerType)) { //set the controller and name to the custom one def.ControllerType = controllerType; def.ControllerName = ControllerExtensions.GetControllerName(controllerType); if (def.ControllerName != defaultControllerName) } else { LogHelper.Warn( "The current Document Type {0} matches a locally declared controller of type {1}. Custom Controllers for Umbraco routing must implement '{2}' and inherit from '{3}'.", () => publishedContentRequest.PublishedContent.DocumentTypeAlias, () => controllerType.FullName, () => typeof(IRenderMvcController).FullName, () => typeof(ControllerBase).FullName); //exit as we cannnot route to the custom controller, just route to the standard one. return def; } }

        //store the route definition
        requestContext.RouteData.DataTokens["umbraco-route-def"] = def;

        return def;{code}

Comments

Alex Fox Gill 10 Sep 2013, 10:11:33

Workaround: rename your controllers or document type aliases so that they don't match


Sebastiaan Janssen 10 Sep 2013, 10:58:48

I think this will be answered by "It's a feature, not a bug" but I'll let Shannon answer it. Related: http://our.umbraco.org/documentation/Reference/Mvc/custom-controllers


Alex Fox Gill 10 Sep 2013, 12:37:23

The problem is that when this exception occurs there is no indication of the reason for failure. The code that is responsible for the error (by not populating {{DataTokens["umbraco-route-def"]}}) is not the same code that throws the exception. I would suggest at the very least that the {{InvalidOperationException}} thrown by {{SurfaceController.TryGetRouteDefinitionFromAncestorViewContexts}} should mention that this might be the issue.

Ultimately I don't see the need for the early-exit. Surely logging the warning is enough for those wanting to take advantage of the feature?

[Edit: sorry, missed the end of a sentence :)]


Sebastiaan Janssen 10 Sep 2013, 13:04:55

Could you please post the full exception as well? I missed that in your initial report. :)


Alex Fox Gill 10 Sep 2013, 13:42:27

Sure, I've added the exception message (you can see it in {{SurfaceController.TryGetRouteDefinitionFromAncestorViewContexts()}})


Shannon Deminick 10 Sep 2013, 13:47:19

Can you provide the full stack trace? And a way to replicate...how you are getting this exception?


Alex Fox Gill 10 Sep 2013, 14:43:01

A full stack trace may not be helpful as it'll just be my own code plus a call to {{SurfaceController.CurrentPage}} :)

To replicate, you need:

  • a Document Type with an alias, e.g. "Registration"
  • a Razor MVC template applied to a Document with the above type
  • a Controller deriving from SurfaceController named the same as the Document Type alias e.g. "RegistrationController"
  • in the MVC template, call the controller e.g. {{@Html.Action("MyAction", "Registration")}}
  • in the controller action, query the {{CurrentPage}} property of the superclass {{SurfaceController}}

When you access the document and the template is rendered and the action is called on the controller, it throws the {{InvalidOperationException}} due to the missing {{DataToken}} entry


Shannon Deminick 11 Sep 2013, 02:15:53

@Alex nice work mate! You're correct the early exit isn't necessary and will cause this problem. This was an oversight with the fixes between these revs:

55f06a7ca6a5d1b9de563aea00e6f9832a7f1249 -> 6a9277a71282c059c996289e1109998bc54c65a4

This has been fixed in 90abe8c25f867bdc20622ef9bda82f949f9062ef


Shannon Deminick 11 Sep 2013, 02:17:18

Of course as a work around for now you can use a diff controller name that doesn't match a doc type so it doesn't think you are trying to hijack the route.


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 6.1.0, 6.1.1, 6.1.2, 6.1.3, 6.1.4, 6.1.5

Due in version: 6.2.0

Sprint:

Story Points:

Cycle: