Blueprint middleware applied globally about sanic HOT 23 CLOSED

To add my two cents: While it might be beneficial to apply middleware registered on a blueprint globally, that's definitely not what I'd expect to happen. I'd regard blueprints as pretty self-contained and separate units and find it kinda surprising to see them interact in such a way. What about adding a @bp.global_middleware?

from sanic.

I have a workaround (although I still think this should be implemented in sanic). Using my last example:

public = Blueprint("public", url_prefix="/public")
public.static("/", public_web_dir)

secure = Blueprint("secure", url_prefix="/secure")
secure.static("/", secure_web_dir)


@secure.middleware("request")
def global_middleware(request):
    if request.path.startswith(secure.url_prefix):

        @authenticated
        def local_middleware(request):
            pass
        local_middleware(request)


app = Sanic()
app.blueprint(public)
app.blueprint(secure)
app.run(host="0.0.0.0", port=8080, debug=True)

from sanic.

I also think this is a counterintuitive behavior. Especially when you introduce this project as flask-like, I expected the blueprint-related features are totally separated and it doesn't affect the app out of blueprint.

I agree about the concolusion the global application offers more, still I also more agree to the other parts its name must not be Blueprint.middleware. Strongly voting to @Tim-Erwin's idea, you don't need to select only one idea of middleware behavior on the counterintuitive name. Please consider to let Blueprint has its own scoped middleware and allow them also to have modulized global middlewares like Blueprint.app_middleware or Blueprint.global_middleware.

from sanic.

Personally I would expect @app.middleware to apply globally and @bp.middleware to apply locally.

Another real life example (which led me here):

public = Blueprint("public", url_prefix="/public")
public.static("/", public_web_dir)

secure = Blueprint("secure", url_prefix="/secure")
secure.static("/", secure_web_dir)


@secure.middleware("request")
@authenticated
async def request_middleware(request):
    pass


app = Sanic()
app.blueprint(public)
app.blueprint(secure)
app.run(host="0.0.0.0", port=8080, debug=True)

In this case I expect to be able to access the static files served via public and be stopped from accessing the static files served via secure without authorization.

Due to the nature of bp.static I cannot add my decorator to the requests it manages, which makes using .middleware for all requests going to secure the logical solution. However, with the current globalness of .middleware I will need to come up with a more complicated solution.

While I definitely agree with @Tim-Erwin that it would be more logical to have a .global_middleware for the current behavior of .middleware for blueprints, maybe the introduction of .local_middleware could be an option to maintain compatibility?

from sanic.

It's defnly unexpected behavior. Well, for me with a long experience with Flask that has same concept the 'Blueprint', it surprised me a lot! I think that the blueprint is a modular sub-application. Unless it stands just for deleting all url prefixes in code, an application with blueprints would be like an 'united' application, not 'single' one. I can understand complexity fears but I believe that it can offer much more capability and convenience with blueprint's independence.

@eth3lbert) What is the different between using @app.middleware and @bp.middleware? Are there any benefits to use @bp.middleware?

@eth3lbert : You can apply various caching policy, control access to specific spaces, or inject something into your response only when it was processed in some bp, based on your blueprint separation.

@channelcat) As for ordering, I'm thinking they should be applied in the order the blueprint was registered, and in the order they were registered in the blueprint.

👍

from sanic.

Let me take a look. If it's an easy enough fix without major refactoring, let me see if I can open a quick PR to address this.

from sanic.

Ah, I missed that in the pull request. I've updated the documentation for now. My thoughts on this are:

Applied locally:

• Is more self-contained, prevents conflicting code.
• Applying global middleware and exception handling requires registering them separately from the blueprint.

Applied globally:

• Modularizes middleware and exception handling.
• Blueprints can facilitate whole modules, allowing a single interface for sharing code between projects.
• Can use decorators to achieve local middleware and exception handling:

def authenticated(func):
    def handler(request, *args, **kwargs):
        # Middleware goes here
        try:
            return func(request, *args, **kwargs)
        except MyCustomException:
            return text("son, i am dissapoint")
    return handler

@bp.route('/')
@authenticated
def index(request):
    return text("We did it fam")

...

In both scenarios you can achieve the same thing, but I think the latter offers more.

As for ordering, I'm thinking they should be applied in the order the blueprint was registered, and in the order they were registered in the blueprint. I think it would allow blueprints to require other blueprints in a way that's easy to discern in the code.

from sanic.

My concern with using a decorator for local middleware/exceptions, is that it requires a different way of adding them, compared to registering on the application object. My fear is, that it becomes too complicated or too involved to handle middleware/exceptions on blueprints.

I would prefer, that you register middleware and exceptions in the same manner (@<obj>.middleware), regardless of applying it on the app-object (affecting all routes) or the blueprint (affecting only those blueprint routes).

from sanic.

What is the different between using @app.middleware and @bp.middleware? Are there any benefits to use @bp.middleware?

from sanic.

Is this still an issue? Will reopen if necessary.

from sanic.

Reopening per request from @r0fls

from sanic.

Is it still there? any update on this?

from sanic.

still there

from sanic.

Does this still apply because of #715 being fixed?

from sanic.

Example from life: I have an application with admin control. In admin blueprint I have to use decorator for check admin rights for all views instead of one blueprint middleware handler.
And do not forget about this please

Explicit is better than implicit.

from sanic.

Another real-world example.
Two URL patterns:

   /a/b/<cred1>/<cred2>
   /a/b?cred1=abc&cred2=xyz

Planned to have two middlewares that gathered the necessary parameters (one from the URL, another from arguments) and store them in the request object so the next layer of code would not need to worry about where the creds 'came from.'
(There's actually more than two, and some are significantly different from the examples.)

from sanic.

I just wanted to agree that my expectation for a blueprint middleware is that it applies only to routes included in the blueprint.

I don't see why it would apply globally. My sample handler setup which seems to do what I would like:

from sanic import response
from sanic import Blueprint
from tempfile import TemporaryFile

blueprint = Blueprint('docx_server', url_prefix='/docx')

@blueprint.middleware('request')
async def type_conversion_dates(request):
  request.args['date'] = pendulum.now()

@blueprint.middleware('response')
async def set_model_response(request, response):
  response.content_type ='application/vnd.openxmlformats-officedocument.wordprocessingml.document'
  return response

@blueprint.route("/<template>", methods=['POST'], strict_slashes=False)
async def get_template_docx(request, template):
  """Returns X3D document
    context (json):
      employee_name (str): name
      employee_title (str): title
      employee_education (str[]): education
      employee_certification (str[]): certifications
      selected_experience (str): experience description
      extra_experience (ob[{title, description}]):
        title: of description
        description: experience description
      employee_organizations(str[]): organizations

    Args:
      template (str): render template name

    Returns:
      docx
  """
  async with request.app.db_pool.acquire() as con:
    template_name = request.args.get('template')
    doc = DocxTemplate(f'/app/templates/{template_name}')
    context = request.json
    doc.render(context)
    tf = TemporaryFile()
    doc.save(tf)

    dt_string = dt.format('YYYY-MM-DD')
    r.headers['Content-Disposition'] = f'filename="{dt_string}-{context.employee_name}-resume.docx"'
    return await response.file(tf)

This seems self contained and easy to manage. Except, now I have to add the middleware somewhere else, disconnecting it from an endpoint I expect to only take certain requests and only serve certain responses.

from sanic.

Hi! I see there's no update for pass 3 years. Is there any plan to fix or somehow differently resolve this issue? The current behavior seems rather counterintuitive.

from sanic.

I believe that this should be fixed (make them local). However, currently routing is performed after request middlewares, so it would be a rather big and potentially breaking architectural change. In particular, one would need to consider whether existing middlewares rely on the current behaviour, e.g. to change request url or method prior to routing.

from sanic.

I believe in that in general (among other languages) middleware should be applied after the routing and is what many people expect the middleware to do. You are right with that it is breaking change so I am not sure what the optimal solution would be. Maybe as @FelixZY suggested add local_middleware? But if this should be implemented, it would probably make more sense to make middleware function local by default.

from sanic.

I just wanted to pop into this thread to point out that the current documentation for blueprint group middleware implies (incorrectly) that blueprint group middleware only executes on the routes defined in that blueprint group (and not globally).

Using this middleware will ensure that you can apply a common middleware to all the blueprints that form the current blueprint group under consideration.

The example code also makes it appear that middleware applied to a blueprint only impacts routes under that blueprint. Specifically this code from the example:

@bp1.middleware('request')
async def bp1_only_middleware(request):
    print('applied on Blueprint : bp1 Only')

from sanic.

I'm wondering where this issue stands. I am using 19.6.0 (but tried 19.6.3) and I am seeing my blueprint middleware being applied globally. I tried wrapping it up and a blueprint group as well but the behavior is the same.

My understanding of the latest documentation is that middleware added to a blueprint will be global unless the blueprint is added to a blueprint group. In this case it should apply only to routes in that group.

While this distinction is a bit confusing, I can live with it but it doesn't seem to be working that way.

Any help would be appreciated.

from sanic.

@huge-success/sanic-core-devs This is getting a bit interesting. I was able to get the blueprint based middleware to work the right way without changing much, But here is a curious case,

def test_bp_middleware(app):
    blueprint = Blueprint("test_middleware")

    @blueprint.middleware("response")
    async def process_response(request, response):
        return text("OK")

    @app.route("/")
    async def handler(request):
        return text("FAIL")

    app.blueprint(blueprint)

    request, response = app.test_client.get("/")

    assert response.status == 200
    assert response.text == "OK"

This is from one of the existing test cases. Now, if we make sure that the blueprint middleware gets applied only on the route registered with the middleware, then this expected output is invalid. Since the route was created using @app and not @blueprint.

What would be the best behavior in this case?

1. If you register a middleware via @blueprint.middleware then it will apply only to the routes defined by the blueprint.
2. If you register a middleware via @blueprint_group.middleware then it will apply to all blueprint based routes that are part of the group.
3. If you define a middleware via @app.middleware then it will be applied on all available routes

With the above in mind, what is the expected precedence in which this can be applied ?

from sanic.