Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#21626 closed Cleanup/optimization (worksforme)

Execute some common code before calling handler in Class Based Views

Reported by: sahilkalra1991 Owned by: nobody
Component: Generic views Version: master
Severity: Normal Keywords: generic views, class based views
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hey,

I have had this problem now for sometime, there always come situations when I want to execute some common code (e.g. user access validation) before calling the handler (i.e. get/post/put) in Class based views

Currently I can do this in following 2 ways:

  1. Create a method and call that separately in all handlers I support
    • Cons: This creates redundant code and doesn't look good
  1. Over-ride dispatch() and execute common code before calling super class's dispatch()
    • Cons: This over-rides the default functionality of dispatch and executes the common code even for handlers I don't support as I am executing the common code before executing dispatch

I want to return 405 for handlers I do not support(default functionality) and execute some common code for handlers I support.

So, it would be really nice if we could have a generic way to do this, like following:

def dispatch(self, request, *args, **kwargs):
        # Try to dispatch to the right method; if a method doesn't exist,
        # defer to the error handler. Also defer to the error handler if the
        # request method isn't on the approved list.
        if request.method.lower() in self.http_method_names:
            handler = getattr(self, request.method.lower(), self.http_method_not_allowed)
        else:
            handler = self.http_method_not_allowed

        validator_result = self.common_validator(request, *args, **kwargs) if getattr(self, 'common_validator', False) else None

        return handler(request, *args, **kwargs) if not validator_result else validator_result

So I can execute my common code like following:

def common_validator(request, *args, **kwargs):
    # my code
    # Return some result if not validated
    # or return None so that handler could be executed

Please provide your view on this. Is there any better way to do this ? Any help is appreciated.

Thanks

Change History (2)

comment:1 Changed 19 months ago by mjtamlyn

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

I am unconvinced that adding this particular approach is necessary. Overriding dispatch as been made sufficiently slick that initialisation code can take place here. I agree that unsupported methods would hit your common code, but I feel that for the vast majority of sites this should not cause an issue as:

  • initialisation code like this must be by definition idempotent as it is called on GET requests
  • this represents a miniscule proportion of requests handled, and in most cases is likely to be down to an incorrect request triggered by another part of the site
  • it is straightforwards to work around where needed.

If you need particular care here for your purpose, I'd encourage you to override the basic view classes with your own. The nature of the class based approach here is that you should only have to do it once.

As a side note, I strongly favour approach (2), though in some cases (1) is more appropriate where the views you're affecting do not edit data and as such handle POST requests. Also, your proposed patch will suffer from the issue you claimed to be a problem in (2) - it will run even if handler is self.http_method_not_allowed.

comment:2 Changed 19 months ago by sahilkalra1991

Hey,

The improved version would be :

def dispatch(self, request, *args, **kwargs):
        # Try to dispatch to the right method; if a method doesn't exist,
        # defer to the error handler. Also defer to the error handler if the
        # request method isn't on the approved list.
        validator_result = None
        if request.method.lower() in self.http_method_names:
            handler = getattr(self, request.method.lower(), self.http_method_not_allowed)
            validator_result = self.common_validator(request, *args, **kwargs) if getattr(self, 'common_validator', False) else None
        else:
            handler = self.http_method_not_allowed

        return handler(request, *args, **kwargs) if not validator_result else validator_result

But I agree with you and would use approach(2). And I would not prefer over-riding Base view class, because it would effect the imports and I would have to over-ride each view (e.g FormView/TemplateView) which is using View. Since it would make work difficult in a team, so approach(2) sounds better.

Thanks

Note: See TracTickets for help on using tickets.
Back to Top