#29750: Add a pre-dispatch() hook for class-based views
----------------------------------+--------------------------------------
     Reporter:  François Freitag  |                    Owner:  nobody
         Type:  New feature       |                   Status:  closed
    Component:  Generic views     |                  Version:  2.1
     Severity:  Normal            |               Resolution:  wontfix
     Keywords:                    |             Triage Stage:  Unreviewed
    Has patch:  0                 |      Needs documentation:  0
  Needs tests:  0                 |  Patch needs improvement:  0
Easy pickings:  0                 |                    UI/UX:  0
----------------------------------+--------------------------------------

Comment (by Jon Dufresne):

 I'm +1 on adding this feature.

 François and I work together. We discussed this feature internally within
 our own project before deciding to propose a Django feature.

 Our project takes advantage of a large number of custom mixins for CBVs.
 These mixins reduce code duplication and provide consistent logic across
 views. Some mixins setup instance variables while others perform nuanced
 user authorization checks. The authorization checks depend on a lot of
 context of the view & mixins. Sometimes requiring the instances variables
 of the parent mixins. We'd like these authorization checks to happen in
 order -- parent class first. We often perform these checks in `dispatch()`
 as the instance variables and authorization checks apply universally to
 all HTTP methods.

 We've found a shortcoming in CBVs when used with many intermediate mixins
 such that calling the parent's `dispatch()` (or `get()` or `post()`)
 returns a `response` object. Calling the parent's method first means the
 `response` object has already been created so it is too late to do
 authorization checks. Doing the authorization checks first means the
 parent mixin's authorization work has yet to be done, so instance
 variables are unavailable and the checks are out of order.

 We'd like a hook so mixins could to use the parent's setup code to
 continue these custom nuanced authorization checks in order '''before'''
 preparing the response.

 > and the application specific job of whatever-it-is-your-app-does, for
 which the logic belongs in the method handlers.

 The method handlers have the same problem. Even if the code were moved to
 `post()` and `get()`, we'd still need some pre-`post()` and pre-`get()`.
 In `post()`, calling the parent's `post()` first means a `response` object
 will be returned, making it too late to do additional authorization
 checks. Calling the parent's `post()` last means the parent's work isn't
 ready, therefore the mixin will miss out on instance variables and the
 authorization checks will be out of order. We discussed pre-`dispatch()`
 as our authorization checks normally apply to all HTTP methods.

 > I look at the examples here and think that you're mixing the two, just
 making the code harder to follow, and so creating an unnecessary
 maintenance burden, all for the sake a single line of duplication.

 The example is intentionally simplified to illustrate the ordering problem
 in a short amount of code. Would a more complete example help?

 > If such repetition did become overwhelming, if can be factored to a
 helper that is a single line to call, that remains part of the view logic.
 In the example, sometimes the "Cannot make decisions based on
 self.access_a nor self.access_long" exists in yet another mixin, as the
 pattern is repeated across a large number of views.

 We find the duplication ''increases'' the maintenance burden as when
 writing a new new views, the developer must remember to include these
 repetitive calls. This can be easy to forget for new developers first
 learning the system. If ''x'' views inherit the mixin, then the "single
 line" must be repeated ''x'' times (or 2''x'' times if for both `post()`
 and `get()`). As these mixins are often used for authorization, forgetting
 to repeat oneself could result in accidental leak of private data.

 We found that DRF has implemented this pattern in its
 [https://github.com/encode/django-rest-
 framework/blob/master/rest_framework/views.py#L471-L498 APIView class].
 Notice it has a `initialize_request` and `initial` method in its
 `dispatch()` function. This allows inherited classes and mixins to do
 instance variable setup and authorization checks before preparing the
 `response`. So there is some prior art and use cases out there. Beyond
 meeting our own project needs, I believe this feature would benefit the
 wider Django community by making third party reusable mixins easier.

-- 
Ticket URL: <https://code.djangoproject.com/ticket/29750#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/073.981160aa18587073d66ad52b65c9850e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to