General middleware execution order

[mar1n3r0]

Following the topic here: https://github.com/vardius/gorouter/pull/22#pullrequestreview-346199845

We can take a more general stance and discuss middleware execution order in general. What we know so far is that the core logic behind generating the tree has a default pattern which it strives to maintain throughout it's whole life cycle:

Sort Nodes in order [statics, regexps, wildcards]

If we abstract away the phases of the gorouter we can conceptualize the following:

• tree generation of nodes with routes - based on router.GET calls
• applying middleware and creating orphaned nodes without routes when there is no match with the existing tree nodes - based on router.USE calls
• processing request path two more phases:
• first checking if the path matches nodes with routes to allow access to middleware layer at all
• applying middleware

The concept is quite clear and neat until the last phase of applying middleware. All phases are going through the same Match loops reusing the same structure and order with some custom processing for orphaned nodes.

Where things get unclear is in the phase of applying middleware. Because it goes through the default ordered structure rather than respecting the user defined middleware order it contradicts with one of the basic core features of a router namely user defined middleware order of execution:

Mux supports the addition of middlewares to a Router, which are executed in the order they are added if a match is found, including its subrouters.

Although initially discussed in the light of the isolated case concerning orphaned nodes only this seems to be of critical importance to the overall predictability of middleware execution order as the default ordering mechanism will always aim to process statics before regexps before wildcards even if the user defined order is different and more importantly unique each time(not a repeating pattern).

[vardius]

Give me some time, need to think about a proper solution for it. For now we could increase test coverage, and maybe remove duplication for logic between routers ? Right now we have every case done twice for each router.

[mar1n3r0]

Not to worry better have issues open for extended periods rather than rushing to close them before all implications are fully investigated.

What do you mean by

remove duplication for logic between routers

Between the net/http and fasthttp ?

I thought the idea to maintain both was for compatibility reasons since gorouter is a wrapper around them.

[vardius]

I mean we have same test logic repeated for each implementation of a router, what we should have is one function where you inject router or something like that

[mar1n3r0]

Do we really want to maintain net/http implementation at all since it's way slower for most operations ?

[vardius]

yes, i am using it in my project and besides that i believe native implementation should be default one always available. So in our case net/http should always come before fasthttp

[vardius]

@mar1n3r0 have a look here https://github.com/vardius/gorouter/pull/15/commits/532090c79797ef780711bcac78edf0c33247de14

I split Node interface into two interfaces RouteAware and MiddlewareAware. Still having one Tree and Node types. Router now has two trees one for routes and one for middleware. Route one is sorted by WithRoute method where middleware is not (this solves our issue). Later on when matching path to tree I match separately path against each tree. Most of the tests are failing now because interface has been changed. Most likely there are going to be some bugs but to find them we need to update tests.

Please give it a deep thoughts, if you can see any potential problems now or in the feature with that solution/implementation.

[mar1n3r0]

Amended the tests a tad just to demonstrate that the problem remains: https://github.com/vardius/gorouter/pull/15/commits/1cc5ab9330c32eec5ee3106469597df190242f43

Later on when matching path to tree I match separately path against each tree.

The thing is it's the same tree which is already ordered with static node priority. I think this is a step in the right direction and proves that this can not be solved without a second tree with custom ordering based on middleware index.

It is easier for me to visualize it with layers as an onion like structure. RouteAware nodes are only meant as a gateway and serve no other purpose. So this original RouteTree is used only to validate that the middleware is actually compliant with the registered routes and can start generating its own MiddlewareTree ordered by its own index.

We can take some of the notes from httprouter for the research as gorouter seems to share the same structure: only explicit matches

The limitations of using a single tree with wildcards:

Only explicit matches: With other routers, like http.ServeMux, a requested URL path could match multiple patterns. Therefore they have some awkward pattern priority rules, like longest match or first registered, first matched. By design of this router, a request can only match exactly one or no route. As a result, there are also no unintended matches, which makes it great for SEO and improves the user experience.

named-parameters

Note: Since this router has only explicit matches, you can not register static routes and parameters for the same path segment. For example you can not register the patterns /user/new and /user/:user for the same request method at the same time. The routing of different request methods is independent from each other.