Middleware for Blueprint.group()

[chenjr0719]

Is your feature request related to a problem? Please describe.
Not related to any problem but about better usage of Blueprint.group(). I've to say that middleware of Blueprint() is a really useful feature. And I hope that Blueprint.group() can also support this useful feature.

Describe the solution you'd like
For example, I have lots of blueprints and all of them are grouped by Blueprint.group(). In this case, if I want to apply a function(authorization for example) to all blueprints, I would like to just add middleware to Blueprint.group().

Additional context
Here is an example:

from sanic import Blueprint

# Assume there are already lots of blueprints are created.
bp_1 = Blueprint('bp_1', url_prefix='/bp_1')
.
.
.
bp_10 = Blueprint('bp_10', url_prefix='/bp_10')

# Use Blueprint.group() to group all blueprint.
bp_api_group = Blueprint.group(bp1, ..., bp_10, url_prefix='/api)

# Apply an authorization function to Blueprint.group()
@bp_api_group.middleware('request')
def authorization(request):
    pass

Let me know your idea.

[harshanarayana]

@chenjr0719 @ahopkins @sjsadowski Let me take a stab at this?

[chenjr0719]

@harshanarayana Yes please go get a try. Actually, I have no idea how to implement this feature.:sweat_smile:

[harshanarayana]

@chenjr0719 @ahopkins @sjsadowski I just created a new PR #1399 for the same.

[ahopkins]

@harshanarayana Thanks! I will take a look. @vltr has been working on sanic-boom that does add this ability now.

So, if you want to use it now, maybe give that a try.

I am marking this as a Future Release as I do not see this as a feature we want to add for 18.12. We are trying to keep the upcoming December (and first) LTS release with a limited amount of "new" features.

I will let @sjsadowski and @yunstanford decide if they want to add that to 19.03 or not.

[harshanarayana]

@ahopkins That sounds good. Since this modifies the existing behavior of the Blueprint.group method, I would like it to go through a serious review and pretty extensive testing. I don't want any of the existing feature to break. So, I wholeheartedly agree with your idea of pushing this as part of a future release.

Thanks for pointing me in the direction of sanic-boom from @vltr Let me see if I can leverage his work here.

[vltr]

@harshanarayana as @ahopkins mentioned, I had my time working on a solution for this. Since I wanted a more performant router for my Sanic applications, I chose to add "route specific" middlewares inside the routing tree (I created a project called xrtr for that, based on a Radix Tree).

Basically (in sanic-boom), you can register a middleware under any level (global or "layered" between URL paths), and the router, on the get function (to search for a specific route and method (GET, POST), swipes the tree and gets every middleware inside it before returning them to the router.

You can check this working here. Unfortunatelly, sanic-boom still needs a lot of documentation yet for me to spread the word on it.

I also wanted the flexibility to add a middleware for a certain route under a certain method, ex. a middleware that would only execute on POST methods on /hello/world matches.

[vltr]

@harshanarayana oh, and I found your solution quite interesting, but had no time to test it yet. I hope I find some soon 😉

[harshanarayana]

@vltr I saw your benchmark script a few days ago and I've to say its super handy and useful.

I've been going over the implementation in sanic-boom and it seems really interesting. I still have a bunch more files to go over to understand all the implementation details but I am getting there. Thanks for kicking off another great utility around sanic. Hopefully, we will see your sanic-boom inside the sanic core soon.

[harshanarayana]

@vltr I've been wondering of a few more middleware cases other than the one you mentioned above, I was wondering how you feel about these middleware behaviors.

1. Ability to add conditional middleware based on certain URL path parameters (This can come handy when you have a parameterized API but want to authorize it in two different ways. i.e. JWT/SAML etc)
2. Ability to conditionally add middleware to Request based on certain param in the request header.
3. Ability to add a Pre-Request middleware that can be invoked before the actual request is handled. (I think we can use the existing one in this manner. Need to test)
4. Additional Optional parameters in the Middleware (middleware attached to response mandates to have two arguments in the method now, I may not always want to have it. Is there a way to make this optional?)

[harshanarayana]

@harshanarayana oh, and I found your solution quite interesting, but had no time to test it yet. I hope I find some soon 😉

Intention behind the current implementation if provide a possible future enhancement where we can bring in the following to Blueprint Group Level. This can come handly when you don't want to add boiler plate code just to register listener/any other such items to each blueprints individually. Hope that makes sense

1. websocker
2. listener
3. exception

[ahopkins]

I am also of the opinion that a more prrformant router and modular Middleware are features we should add to sanic core.

My initial reaction to the conditional Middleware idea was that it might add overhead to achieve, and I'm not sure it couldn't add anything that wouldn't be solved with blueprint Middleware and an if statement.

[harshanarayana]

@ahopkins

1. This might be an overreach, but what if Sanic provides a default router that benchmarked and validated while provides hooks/mechanism to add your own custom/preferred routing mechanism if required? (I know this might not even be a valid case) ? (i.e. Our router uses some basic abc structure to provide a layer of abstraction that any one can implement and use their favorite routing mechanism as they see fit. And then monkeypatch it into the sanic or provide it as a hook which the sanic app can hook into and use the custom implementation instead of the default mechanism. Like it said, might be a huge overkill, but sounded really cool in my head.)

2. The reason I was thinking of conditional middleware is that we register middleware during the startup and don't worry about it later. So, adding a conditional middleware via the Blueprint Group that can apply conditionally based on the patterns of the routes defined in the blueprint might make it easy to tag it directly to a route itself.

bp1 = Blueprint('bp1', url_prefix='/bp1')
bp2 = Blueprint('bp2', url_prefix='/bp2')

@bp1.route("/somepath/<some_param>")
def somepath_bp1(request, some_param):
     return text("OK")

@bp1.route("/something/<some_param>")
def something_bp1(request, some_param):
     return text("OK")

@bp2.route("/somepath/<some_param>")
def somepath_bp2(request, some_param):
     return text("OK")

@bp2.route("/something/<some_param>")
def something_bp2(request, some_param):
     return text("OK")

group = Blueprint.group(bp1, bp2, url_prefix="/api")

@group.middleware('request', condition={
      'route': {
             'pattern': r'.*something.*',
             'check': 'match'
       }
})
def conditional_group_middleware(request):
   # do something

Let's take the above example, I want the Middleware to be applied to both Blueprints but only for those routes that match the regex criteria .*something.*. This is just a thought. Unsure about if it will be useful as a generic feature or not.

This reason I was thinking this is, I don't want my request to go into some of the middleware at all if it's not essential. I would rather wait for an extra second during the bootup rather wait during the response. Wouldn't you agree?

[ahopkins]

@harshanarayana

For number 1, I totally agree and would love to see that!

As for 2, if we can apply Middleware at the blueprint level (in addition to app and blueprint group), wouldn't that essentially do the same thing? If the condition is a function of the route, then it seems this could be achieved with nesting of blueprints. ESPECIALLY, if there was the ability in number one for customizing the router or providing a drop in replacement.

[ahopkins]

But yes, I agree... it's a web server. Bootup time is not so important. I was thinking you were talking about request level conditions.

[vltr]

@harshanarayana

I saw your benchmark script a few days ago and I've to say its super handy and useful.

Yeap, I want to add some other routers in there in the future, but only Python ones (or ones that provide a Python interface at least).

Hopefully, we will see your sanic-boom inside the sanic core soon.

Thanks! I have a slightly different approach with sanic-boom than I have with Sanic (like not having any static file utilities; in my opinion it's useless), but component injection and all that neat stuff would be great to incorporate into Sanic itself, but I don't know if this would go on because it has a performance penalty on each request ... There's always C and Cython to the rescue, but this may add another layer of complexity to the code. This need a broader discussion with the core developers and the whole community to be honest.

Ability to add conditional middleware based on certain URL path parameters (This can come handy when you have a parameterized API but want to authorize it in two different ways. i.e. JWT/SAML etc)

Like /auth?t=jwt and /auth?t=saml, as an example?

@app.route("/auth", param={"t": match_ignorecase("jwt")})
async def myjwtfunc(...):
    # code here


@app.route("/auth", param={"t": match_ignorecase("saml")})
async def mysamlfunc(...):
    # code here

I have thought on that. This also can be used to implement header based conditions to the router as well:

@app.route("/api", header={"content-type": match_ignorecase("application/json")})
async def myjson(...):
    # code here


@app.route("/api", header={"content-type": match_ignorecase("application/xml")})
async def myxml(...):
    # code here

Ability to add a Pre-Request middleware that can be invoked before the actual request is handled. (I think we can use the existing one in this manner. Need to test)

This already exists in Sanic, doesn't it? @app.middleware("request") and @app.middleware("response") ... Or do you have another idea?

Additional Optional parameters in the Middleware (middleware attached to response mandates to have two arguments in the method now, I may not always want to have it. Is there a way to make this optional?)

Not with vanilla Sanic, only in sanic-boom I implemented this feature (you can use any parameters you want as long as they match some Component criteria or is named request or req to receive the request instance, as well as response for the response (on response middlewares).

@ahopkins

I am also of the opinion that a more prrformant router and modular Middleware are features we should add to sanic core.

Yeap, the Sanic router does it job with some possible pitfalls here and there but it doesn't scale well if the LRU cache is not used very often; as well being very complex. I'm rewriting xrtr to be in pure C with Python bindings, allowing the usage of regex (using pcre2) and a configurable "format", so you can write your route as /api/:var:type, /api/<var:type> or /api/{var:type}; this way it can be used in more than just one web server implementation (in the actual case a Sanic "plugin").

[harshanarayana]

@vltr I'll be glad to lend you a hand with any enhancement to the benchmarking script if and where there is a need for it.

like not having any static file utilities; in my opinion it's useless

I personally like the idea of having a static file utility handy just if I need it. it doesn't have to be over the top fancy, but if it can do the bare minimum that is more than sufficient.

I have thought on that. This also can be used to implement header based conditions to the router as well:

Exactly my thought. It might look like some kinda trickery when you read the code and try to figure out how the flow even gets there, but from an end user implementation standpoint, I can have distinctive methods defined that can do one single thing it's supposed to do. Easy to test and easy to manage. (PS, I am not a big fan of methods having if-else constructs. It's purely a personal thing)

@app.route('/secure')
def secure_method(request):
    # do something 007 would do.

@app.middleware('request', header={'X-Auth-Mode': match_ignorecase('jwt')}
def auth_with_jwt(request):
   # use sanic-jwt and wish, let there be light.

@app.middleware('request', header={'X-Auth-Mode': match_ignorecase('saml')}
def auth_with_saml(request):
   # Please go back to using JWT 🗡 or authenticate yourself. 

The above example just tells you what I had in mind. Almost similar to what you mentioned but with a minor tweak. This can be applied at the Blueprint, Blueprint Group or at all level and you will have an easy way to control middleware invocation. But, like @ahopkins mentioned, PERFORMANCE overhead!

This already exists in Sanic, doesn't it?

Yes it does. I did some basic testing to ensure it works the way I expected and it does. Any middleware attached to request will execute before the handler and anything attached to response will execute at the end of the response. I tested this by writing some basic cases wher the requests are modified before processing and response is enhanced as part of the return.

These things can come extremely handly when you are implementing a rate-limiting behavior on the API as well as exporting some API metrics to a destination of your choosing. Not to forget response caching behaviors that can be put into the system as a plugin.

Not with vanilla Sanic, only in sanic-boom I implemented this feature

Oh Okay. I don't know if it's just me or not, but I would love to see this feature made available in vanilla sanic as well since it can make it much easier when writing code. Un-used parameters to the methods are always a bad idea.
The other way I was thinking was to make sure the middleware definition takes an extra param that tells you if the arguments are to be passed to the method or not and conditionally do so when handling the request. But this will be an additional overhead which doesn't sound really useful.

The other way is to implement the middleware using **kwargs or **args and the request is done such a way that it can address the implementation of the middleware method.

regex (using pcre2) and a configurable "format", so you can write your route as /api/:var:type, /api/var:type or /api/{var:type}

This sounds like a great idea. Looking forward to seeing how that works.

[harshanarayana]

I think this can now be closed in favor of merged PR #1399