U4-7056 - In BatchedDatabaseServerMessenger.UmbracoModule_RouteAttempt the Sync method is not being executed when using Virtual Nodes (custom routes)

Created by Dave Woestenborghs 02 Sep 2015, 09:50:54 Updated by Stephan 02 Sep 2015, 12:45:28

We are trying to use the new database load balancing and have issues when using virtual routes.

We have a designated content editing server and a front end server (no backend access).

When we publish changes in the backend instance these get updated on the front end instance when calling a normal route. However when publish changes and the first request to the front end instance is a virtual route the changes are not updated.

See https://our.umbraco.org/forum/umbraco-7/using-umbraco-7/71088-loadbalancing-issues-in-73-beta3 for more details

Comments

Dave Woestenborghs 02 Sep 2015, 10:58:57

It seems that virtual routes are not seen as routeable in Umbraco.

I just attached debugger and have found the following result.

In https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/UmbracoModule.cs the method EnsureUmbracoRoutablePage is called for each (non client side request)

In this method EnsureDocumentRequest is false for a virtual node.

This because the call to the method IsReservedPathOrUrl in https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Core/Configuration/GlobalSettings.cs

In this method a route is found for the virtual node and Umbraco sees this as reserverd path.

Because of this the outcome of EnsureUmbracoRoutablePage is EnsureRoutableOutcome.NotDocumentRequest.

In the method UmbracoModule_RouteAttempt in https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs the sync method is only called for backend request the outcome is EnsureRoutableOutcome.NotDocumentRequest.

I hope you are still with me :-)


Dave Woestenborghs 02 Sep 2015, 11:16:53

I just tried to fix the method with this check (in red)

``C# if (httpContext == null) throw new ArgumentNullException("httpContext"); if (routes == null) throw new ArgumentNullException("routes");

        //check if the current request matches a route, if so then it is reserved.
        var route = routes.GetRouteData(httpContext);
        if (route != null && {color:red}route.RouteHandler == null{color})
           return true;
           

        //continue with the standard ignore routine
        return IsReservedPathOrUrl(url);

``

Afther this the request is seen as routable. But returns a 404. Still have to do some digging.


Shannon Deminick 02 Sep 2015, 11:22:15

Virtual nodes are NOT routable... you are in control of routing virtual nodes so the UmbracoModule doesn't perform any routing.

This is expected, so your problem must exist elsewhere.


Dave Woestenborghs 02 Sep 2015, 12:00:00

So how can I make sure the BatchedDatabaseServerMessenger executes the sync method when a virtual node is hit ?

Dave


Shannon Deminick 02 Sep 2015, 12:05:16

I am only just understanding your issue... sorry this is all very cryptic because I think that you are thinking other things are going on. All I really need to know was:

In BatchedDatabaseServerMessenger.UmbracoModule_RouteAttempt the Sync method is not being executed when using Virtual Nodes (custom routes).

I will update the issues title ;)


Dave Woestenborghs 02 Sep 2015, 12:08:42

Glad it is clear now


Shannon Deminick 02 Sep 2015, 12:10:02

@zpqrtbnk - The issue is that in BatchedDatabaseServerMessenger.UmbracoModule_RouteAttempt we check if the request is Routable (meaning it's an actual Umbraco request). However, when people are defining their own routes and using virtual nodes with the UmbracoVirtualNodeRouteHandler, the Sync method doesn't execute which would cause issues because a server in load balancing will not update it's cache.

Solutions:

  • We remove the check and run Sync on any request ... this might not be perfectly ideal but would work
  • We should be able to check values in the current Route's RouteData in this method and check if the data token "umbraco-custom-route" exists, since it needs to be there if this is a virtual node route. In that case we'd Sync. This is more ideal but needs more work of course


Stephan 02 Sep 2015, 12:12:50

got it - it also means that if a server is only serving eg api requests, it would never sync. TBH "touching" the server (syncing) is throttled anyways so doing it on every request would only add a datetime comparison - I'd vote to sync on every request


Shannon Deminick 02 Sep 2015, 12:16:25

Cool sounds good


Dave Woestenborghs 02 Sep 2015, 12:22:36

I just updated BatchedDatabaseServerMessenger.UmbracoModule_RouteAttempt with this code. Now syncing works fine.

  private void UmbracoModule_RouteAttempt(object sender, RoutableAttemptEventArgs e)
        {
            switch (e.Outcome)
            {
                case EnsureRoutableOutcome.IsRoutable:
                    Sync();
                    break;
                case EnsureRoutableOutcome.NotDocumentRequest:
                    var allRoutes = new RouteCollection();
                    foreach (var route in RouteTable.Routes)
                    {
                        allRoutes.Add(route);
                    }

                    var currentRoute = allRoutes.GetRouteData(e.HttpContext);

                    var isVirtualNode = false;

                    if (currentRoute != null && currentRoute.RouteHandler != null)
                    {
                        if (typeof(UmbracoVirtualNodeRouteHandler).IsAssignableFrom(currentRoute.RouteHandler.GetType()))
                        {
                            isVirtualNode = true;
                        }
                    }

                    //so it's not a document request, we'll check if it's a back office request
                    if (e.HttpContext.Request.Url.IsBackOfficeRequest(HttpRuntime.AppDomainAppVirtualPath) || isVirtualNode)
                    {
                        //it's a back office request, we should sync!
                        Sync();
                    }
                    break;
                //case EnsureRoutableOutcome.NotReady:
                //case EnsureRoutableOutcome.NotConfigured:
                //case EnsureRoutableOutcome.NoContent:
                //default:
                //    break;
            }
        }

Don't know if it needs to be done in other places as well and probably can be done better. But it works !

Dave


Shannon Deminick 02 Sep 2015, 12:27:47

@dawoe probably not the best way to do it since you need to iterate and add all routes and then match route data again when that has already been done. Pretty sure we can get this information from the current request some how. But in any case, you can just remove all of the logic, we're just going to run Sync() on any request as Stephen mentioned.


Dave Woestenborghs 02 Sep 2015, 12:32:00

Super. We can live with this issue for now. Not live yet. So we wait for the next release.

Dave


Stephan 02 Sep 2015, 12:38:03

So the change would be:

// as long as umbraco is ready & configured, sync switch (e.Outcome) { case EnsureRoutableOutcome.IsRoutable: case EnsureRoutableOutcome.NotDocumentRequest: case EnsureRoutableOutcome.NoContent: Sync(); break; //case EnsureRoutableOutcome.NotReady: //case EnsureRoutableOutcome.NotConfigured: //default: // break; }

Which means that any request that we try to route, is going to sync. That excludes (see code in UmbracoModule.ProcessRequest) client-side requests. Makes sense IMHO. Will push in a few, just need to sort-out a few things in UmbracoModule.


Stephan 02 Sep 2015, 12:45:24

Pushed fc8c92e9225c2dff3c9e66c7888a0c8a8032dd46. Will sync on all requests that are not client-side.

NOTE: will ''also'' not sync on BaseRest requests -- they're obsolete anyways and we'd need to change things in UmbracoModule -- not worth it.


Priority: Normal

Type: Bug

State: Fixed

Assignee: Stephan

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.3.0

Due in version: 7.3.0

Sprint:

Story Points:

Cycle: