Class based views: A standard hook for http-method-independent code

819 views
Skip to first unread message

Marc Tamlyn

unread,
Mar 1, 2012, 1:38:08 PM3/1/12
to django-d...@googlegroups.com
Hi all,

Apologies if this has been raised before. I've had a look around and can't find a good way of doing this with the current code.

I regularly have views which rely on some custom code which runs some sanity checking on the request and is independent of method. As an example, consider a view which creates an object related to a parent. This is easily achievable by overriding the form_valid method of CreateView and excluding the foreign key from the form. However, the view should return a 404 if the related object specified by the url does not exist. Written as a non class based view, the natural flow is to try to load the parent object from the database, handle it as necessary, and then split paths depending on whether we have a get or post. It is currently very difficult to emulate this without duplication of some sort.

My proposal is that we add a process_request (or similar name) method which is called by the dispatch method immediately before the method handler is called. (i.e. here). This would then allow pre-processing and sanity checking of the request object, using the args, kwargs and request that have been saved on the class, before delegating off the the respective views. The method should return None or an HttpResponse subclass. If it returns something, then we return that directly from the dispatch method.


I can supply some code (my proposal is pretty simple I think) but I thought I'd open it up for discussion first.

Marc Tamlyn

Charlie "meshy" Denton

unread,
Mar 2, 2012, 5:58:05 AM3/2/12
to django-d...@googlegroups.com
I would like to see something like this too. I my suggestion is here: https://gist.github.com/1957251

This method has two advantages:
1. You can modify the request, args, and kwargs before they get saved to the view.
2. You can execute code after request, args, and kwargs are saved but before the dispatch handler is called.
3. You can save extra variables to self as required

I expect 2 is probably the most common use case.

Meshy.

Charlie "meshy" Denton

unread,
Mar 2, 2012, 4:36:37 AM3/2/12
to Django developers
I'd like to see something like this too: my suggestion is here:
https://gist.github.com/1957251

This implementation also allows you to play with the request, args and
kwargs before they are saved on the model.

On Mar 1, 6:38 pm, Marc Tamlyn <marc.tam...@gmail.com> wrote:
> Hi all,
>
> Apologies if this has been raised before. I've had a look around and can't
> find a good way of doing this with the current code.
>
> I regularly have views which rely on some custom code which runs some
> sanity checking on the request and is independent of method. As an example,
> consider a view which creates an object related to a parent. This is easily
> achievable by overriding the form_valid method of CreateView and excluding
> the foreign key from the form. However, the view should return a 404 if the
> related object specified by the url does not exist. Written as a non class
> based view, the natural flow is to try to load the parent object from the
> database, handle it as necessary, and then split paths depending on whether
> we have a get or post. It is currently very difficult to emulate this
> without duplication of some sort.
>
> My proposal is that we add a process_request (or similar name) method which
> is called by the dispatch method immediately before the method handler is
> called. (i.e. here<https://github.com/django/django/blob/master/django/views/generic/bas...>).

Michael van Tellingen

unread,
Mar 2, 2012, 8:40:37 AM3/2/12
to django-d...@googlegroups.com
Hi,

This should already be quite easy to implement, do something like:

def dispatch(self, *args, **kwargs):
# Some code
return super(YourView, self).dispatch(*args, **kwargs)

Regards,
Michael

> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/django-developers/-/z63TmT57twQJ.
>
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to
> django-develop...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.

Jonathan French

unread,
Mar 2, 2012, 9:53:15 AM3/2/12
to django-d...@googlegroups.com
That's true, but I agree it seems a useful enough hook to make it a hook, rather than needing to do it yourself like that. I would vote for it being called 'preprocess', to make it clear that it's both optional and run before the method-specific function.

- ojno

Andre Terra

unread,
Mar 2, 2012, 10:22:08 AM3/2/12
to django-d...@googlegroups.com
Then that would also be the case for many other methods in generic class-based views, like get_context_data, get_queryset, etc.. I hate boilerplate code as much as the next guy, but I'm not sure I follow why this single method would get a special hook. Correct me if I'm wrong, but special cases aren't special enough to break the rules.


Cheers,
AT

Jonathan French

unread,
Mar 2, 2012, 12:15:10 PM3/2/12
to django-d...@googlegroups.com
That's not the case - get_context_data, get_queryset, are examples of hooks like this preprocess one, not places where more would be added. I'm not sure if there's a better analogy.

Overriding the dispatch method would be the correct solution if you wanted to behave the same towards all methods, but seems a bit heavy for factoring out validation or common code between methods where you still want your get/post/etc to be called (or not, as the case may be.)

- ojno

Tino de Bruijn

unread,
Mar 4, 2012, 12:45:55 PM3/4/12
to django-d...@googlegroups.com
I don't really see what difference another function makes. Say https://gist.github.com/1957251 is implemented, what makes:

def prepare_view(self, request, *args, **kwargs):
    # the thing I want to do
    super(ClassName, self).prepare_view(request, *args, **kwargs)

preferable over:

def dispatch(self, request, *args, **kwargs):
    # the thing I want to do
    super(ClassName, self).dispatch(request, *args, **kwargs)

?

You'll still have a super call because otherwise you have to repeat the


self.request = request
self.args = args
self.kwargs = kwargs

part of prepare_view.

What is wrong with overriding dispatch and calling super? (Or not, if you don't want to progress in the view)


Tino

Charlie "meshy" Denton

unread,
Mar 4, 2012, 12:57:39 PM3/4/12
to django-d...@googlegroups.com
Firstly, please excuse my double post: I didn't realise posts are vetted, so I thought it had been lost.

The very significant advantage of this being a hook (instead of overriding dispatch) is that it allows you to call methods that require dispatch to have already set variables (eg: self.kwargs) on the class before the handler (eg: get()) is called.

Normally I have no problem overriding dispatch, but this has come up a couple of times for me, and overriding dispatch in this way has felt ugly.

Mike Fogel

unread,
Mar 16, 2012, 4:52:43 PM3/16/12
to Django developers
> I don't really see what difference another function makes. Sayhttps://gist.github.com/1957251is implemented, what makes:
>
> def prepare_view(self, request, *args, **kwargs):
>     # the thing I want to do
>     super(ClassName, self).prepare_view(request, *args, **kwargs)
>
> preferable over:
>
> def dispatch(self, request, *args, **kwargs):
>     # the thing I want to do
>     super(ClassName, self).dispatch(request, *args, **kwargs)
>
> ?

https://gist.github.com/1957251 would allow:

def prepare_view(self, request, *args, **kwargs):
    super(ClassName, self).prepare_view(request, *args, **kwargs)
# the thing I want to do - can use self.request, self.args,
self.kwargs

As things stand now, I don't know of a graceful manner to use
self.request in a http-method independent way.

FWIW, I've ran into this restriction a number of times, including
today. If one of the core devs will nod approval on this, I'll open a
ticket and attach this gist to it.

Cheers,

Mike

On Mar 4, 9:45 am, Tino de Bruijn <tin...@gmail.com> wrote:
> I don't really see what difference another function makes. Sayhttps://gist.github.com/1957251is implemented, what makes:
> > On 2 March 2012 15:22, Andre Terra <andrete...@gmail.com> wrote:
>
> >> Then that would also be the case for many other methods in generic
> >> class-based views, like get_context_data, get_queryset, etc.. I hate
> >> boilerplate code as much as the next guy, but I'm not sure I follow why
> >> this single method would get a special hook. Correct me if I'm wrong, but
> >> special cases aren't special enough to break the rules.
>
> >> Cheers,
> >> AT
>
> >> On Fri, Mar 2, 2012 at 11:53 AM, Jonathan French <m...@jonathanfrench.net>wrote:
>
> >>> That's true, but I agree it seems a useful enough hook to make it a
> >>> hook, rather than needing to do it yourself like that. I would vote for it
> >>> being called 'preprocess', to make it clear that it's both optional and run
> >>> before the method-specific function.
>
> >>> - ojno
>
> >>> On 2 March 2012 13:40, Michael van Tellingen <

Diederik van der Boor

unread,
Oct 31, 2012, 5:48:24 AM10/31/12
to django-d...@googlegroups.com
Hi,

Please allow me to add my €0.02.
In a large project I've experienced a similar issue; and we solved it in a slightly different way.
What we also noticed was:
- overriding dispatch(), get() or post() wasn't good enough anymore.
- the views need some initialization moment before their workflow (in get/post of the base classes) start.

What we ended up with is this base class (simplified a bit):

I seriously propose having such init() function in the Django views.
Mind you, that took a heated debate in the organization I was contacted for, so please allow me to explain to context here.
I think we've found a missing cornerstone in the way the class based views are structured, and there is an easy fix.

What is the problem with overriding dispatch()?
When overriding dispatch(), get() or post() the flow is always:

def dispatch(self, request, *args, **kwargs):
    # my code here.
    return super(…).dispatch(request, *args, **kwargs)

The same also applies to get() and post().
In other words, the last deriving class on top of the inheritance chain is always initializing first before it's base classes.
It can't rely on a base class to do some initialization.

With our permission check in the base class' dispatch() method, anything deriving from that effectively
couldn't override dispatch() anymore because that would run before the permission check.

How does the init method fix this?
By doing a self.init() in the top-most dispatch() method, each class in the inheritance chain has a chance to fetch the objects it needs to have.

That code can be written as:

def init(self):
    super(..).init()
    # my code here.

Now, the base class can initialize, then the deriving class.
With a larger inheritance chain, this behavior becomes crucial.
Each class can build upon what the other has prepared already.


All of a sudden, we could do things like this:

class PhotoListView(TabbedListView):
    """
    Contents of an photo album; a list of photo's.
    """
    model = Photo

    template_name = "photoalbum_album.html"
    permission_class = permissions.PhotoAlbumViewPermission

    def init(self):
        super(PhotoListView, self).init()
        self.photoalbum = get_object_or_404(PhotoAlbum, pk=self.kwargs['pk'])  # parent object that filters the list

    def get_queryset(self):
        return super(PhotoListView, self).get_queryset().in_album(self.photoalbum)

    def get_context_data(self, **kwargs):
        context = super(PhotoListView, self).get_context_data(**kwargs)
        context['photoalbum'] = self.photoalbum
        context['can_delete'] = self.is_authorized_for(PhotoDeleteView)
        return context

This is a list view for photo's, and it's limited to a current photo album.
The alternative is making a DetailView, and putting the photo's somewhere in get_context_data() and thereby loose what the list view has to offer.
Now we can just state it's a list view (which it is), and introduce the filter easily.

Without the init() method, you're probably knotting that somewhere in the get_queryset() and get_context_data(),
without having a clear path of that's happening. Thanks to the simple init() method is all remains clean.


Some background of our use-case
The project is made for health care and privacy must be fully guaranteed.
Hence, all views have to implement a permission check, which we wanted to have in the base class.
The only place to hook things up, was before the super call to  dispatch(), otherwise the view already executed.

At the same time, the permission check needs information from things like "self.object", and the URL kwargs.
That's because the permission check is role based; clients only see their views, counselors may inspect their clients, etc..
Implementing the check half-way in the regular get() and post() workflow wasn't an option as it's easy to miss.

With the init() method, we allow the view to initialize, and fetch all objects, so more sophisticated code can be performed afterwards.
Currently the permission check still fetches the objects itself as well, which will likely change in the future when the checks have more role-based options.


I hope this gives a clear explanation why such method would be beneficial to Django's class based views.

Looking forward to your suggestions and response,

Diederik




Op 30 okt. 2012, om 22:44 heeft Jordan Hagan het volgende geschreven:

I would really like to see something like Meshy's proposed solution implemented as this is an issue that I've run into a few times as well.

Although I can appreciate TiNo's argument of:

> self.request = request
> ...

This creates a problem for methods that are going to be used in the overridden dispatch method and the dispatched method that need access to these attributes as they will need to be passed in as a parameter:

in dispatch:
self.some_method(request, *args, **kwargs)

in dispatched:
self.some_method(self.request, *self.args, **self.kwargs)

which is just really messy.

In addition to this methods from other generic view mixins cannot be used in the overridden dispatch method as they expect these class attributes to be available - 'get_object' on SingleObjectMixin is a good example of this as it requires self.kwargs to function:


The only options available to us are monkey patching or code duplication, neither of which offer a good solution to this problem. Generic views are great for reducing boilerplate in code, and adding a hook in this case would do just that. Without this hook I'm forced to add code like the following to each of my projects as a workaround https://gist.github.com/3983252

Cheers,
Jordan
--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/uYmm9IR6P7QJ.

Meshy

unread,
Oct 31, 2012, 5:57:28 AM10/31/12
to django-d...@googlegroups.com
Marc and I have been using a mixin to override `dispatch()` with this functionality. He has an ongoing pull request on django-braces with the code. I hope this can be useful to some of you.

Meshy.

Jordan Hagan

unread,
Oct 31, 2012, 2:42:32 PM10/31/12
to django-d...@googlegroups.com
Diedreik,

Thanks for your comments - your solution seems similar to the one proposed by Meshy unless I'm missing something (https://gist.github.com/1957251). It is good to see multiple people coming up with the same solution to the issue independently though as that to me is an indication that we're moving in the right direction.

Meshy,

Thanks for the link - the django-braces project looks useful - I'll probably start using that.

I would love to get some input from a core developer (or two) on this to see where they stand. From where I'm sitting there seems to be a number of people working around this problem in more or less the same manner, and most of the arguments against haven't taken into consideration compatibility with other mixins, already existing class methods, etc.

I would be happy to put together a patch in a ticket on trac and do any other grunt work required to make this happen.

Cheers,
Jordan

Aaron Merriam

unread,
Nov 1, 2012, 10:31:40 AM11/1/12
to django-d...@googlegroups.com
Just wanted to put my +1 into this thread.  I've been fighting for a clean way to implement permissions in class based views and the 'init' method would help my implementation be a lot more DRY.

Andre Terra

unread,
Nov 1, 2012, 10:59:48 AM11/1/12
to django-d...@googlegroups.com

At first I wasn't sure about this hook, but after working with permissions in CBVs I can see how this would allow for much simpler code, especially when you're implementing a lot of subclassing. I tend to get carried away in writing mixins and base classes for my views, so yeah, I'm +1 on this too.

Cheers,
AT

-- Sent from my phone, please excuse any typos. --

To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/Um5fZBppDqMJ.

Jordan Hagan

unread,
Nov 5, 2012, 1:20:02 PM11/5/12
to django-d...@googlegroups.com
As it seems that there is no longer any real opposition to this ticket (if there is, now would be the time to speak up) I'll go ahead and prepare a patch against the current trunk and get it uploaded to trac and see where we get to from there.

Hopefully I'll get a chance to take a look at this this week, I'll post a link to the trac ticket here once I've opened it.

Cheers,
Jordan

Russell Keith-Magee

unread,
Nov 5, 2012, 6:06:31 PM11/5/12
to django-d...@googlegroups.com
On Tue, Nov 6, 2012 at 2:20 AM, Jordan Hagan <f...@ephess.co.nz> wrote:
As it seems that there is no longer any real opposition to this ticket (if there is, now would be the time to speak up) I'll go ahead and prepare a patch against the current trunk and get it uploaded to trac and see where we get to from there.

Hopefully I'll get a chance to take a look at this this week, I'll post a link to the trac ticket here once I've opened it.

It's a bit presumptuous to say there's "no longer any real opposition" when the people who raised the original opposition haven't said anything.

I haven't seen anything that addresses my original concerns -- that this is a niche requirement that complicates the default case, and can be achieved in end-user code for that small subset of people that are affected.

The internal mechanics of CBVs are already complex. I'm afraid I still don't see the general-purpose value in what is being proposed here.

Yours,
Russ Magee %-)

Jordan Hagan

unread,
Nov 5, 2012, 6:48:17 PM11/5/12
to django-d...@googlegroups.com
I'm sorry if I came across that way, that wasn't my intention at all. Andre Terra who was the one to initially raise opposition has changed his stance on the functionality since he first posted, as per his email 4 days ago. Aside from him there is Tino de Bruijn who voiced opposition, although he seems to be the only other person in the history of the thread (not counting your post just now). There was also Michael van Tellingen, however I'm not 100% sure what his position on the topic is.

From my count there are 8 people in this thread in support of the functionality, and 2 people against it (1 at the time of my previous message).

I figured that to be a good amount of support for the feature, and having not had any direction from a core dev at that time (that I was aware of anyway, once again excuse my ignorance if that is not the case) I decided to go ahead and try and get something moving.

In regards to your concerns, which mainly seem to be in regards to how many people would actually utilize this feature / are currently working around it I'm not sure how to address that. I suppose mixins are the solution for now then, however it does seem to me like something that anyone who is making extensive use of class based views will eventually come up against.

Cheers,
Jordan

Łukasz Rekucki

unread,
Nov 5, 2012, 7:41:01 PM11/5/12
to django-d...@googlegroups.com
On 6 November 2012 00:48, Jordan Hagan <f...@ephess.co.nz> wrote:
>
> From my count there are 8 people in this thread in support of the
> functionality, and 2 people against it (1 at the time of my previous
> message).

There is also lots of other people that don't feel the need to join
the thread as they feel their views are represented.

Personally, I don't see any big advantage of having a yet another init
method, but OTOH I already replaced Django's view hierarchy with my
own, as I needed a more advanced data flow.

--
Łukasz Rekucki

Russell Keith-Magee

unread,
Nov 5, 2012, 7:56:09 PM11/5/12
to django-d...@googlegroups.com
On Tue, Nov 6, 2012 at 7:48 AM, Jordan Hagan <f...@ephess.co.nz> wrote:
I'm sorry if I came across that way, that wasn't my intention at all. Andre Terra who was the one to initially raise opposition has changed his stance on the functionality since he first posted, as per his email 4 days ago. Aside from him there is Tino de Bruijn who voiced opposition, although he seems to be the only other person in the history of the thread (not counting your post just now). There was also Michael van Tellingen, however I'm not 100% sure what his position on the topic is.

No worries - I'm not accusing you of malicious intent. I'm just calling out some language that possibly reflects a misunderstanding on your part of the current status of the discussion.
 
From my count there are 8 people in this thread in support of the functionality, and 2 people against it (1 at the time of my previous message).

The bit you're possibly missing due to the way GMail handles some replied: this thread was a respawn of an older thread from 6 months ago. The google group has the full thread.

 
Plus:

1) As Łukasz points out -- silence doesn't imply consent. There are 7340 people subscribed to django-developers. Getting 5 people to agree doesn't really reflect any sort of statistical sample.

2) Unless you've got buy in from a core developer, you can have all the consensus you like -- your code still isn't going to make it to trunk. 

Of course, you might just be using your 5-person consensus to establish that it's worth going to the trouble of actually working up a patch, but it sounded like you were under the impression that your 5-person consensus was enough to end up with a patch in trunk, and I want to moderate your expectations.

In regards to your concerns, which mainly seem to be in regards to how many people would actually utilize this feature / are currently working around it I'm not sure how to address that. I suppose mixins are the solution for now then, however it does seem to me like something that anyone who is making extensive use of class based views will eventually come up against.

I'm not saying that you don't have a use for this type of entry point. What I'm saying is that you're advocating making the basic entry sequence of class based views (and thus, the documentation and learning curve) more complex, all to service a use case that can be achieved with subclassing. And, in the limited subset of people that have *huge* subclassing overheads and find themselves writing that subclassing code over and over again, that subset can write a new base subclass or mixing that introduces the complexity. 

What I *don't* see is a generic, across the board need that warrants *every* user being forced to carry the overhead so that *some* users have a convenience that can be achieved by other means.

Yours,
Russ Magee %-)

Jordan Hagan

unread,
Nov 5, 2012, 8:20:15 PM11/5/12
to django-d...@googlegroups.com

From my count there are 8 people in this thread in support of the functionality, and 2 people against it (1 at the time of my previous message).

The bit you're possibly missing due to the way GMail handles some replied: this thread was a respawn of an older thread from 6 months ago. The google group has the full thread.

 

I'm using Google groups to interface, I'm pretty sure I've read everything, everything in that discussion you linked seems familiar.
 
Of course, you might just be using your 5-person consensus to establish that it's worth going to the trouble of actually working up a patch, but it sounded like you were under the impression that your 5-person consensus was enough to end up with a patch in trunk, and I want to moderate your expectations.

That is what I was doing, I was in no way expecting my patch to be accepted just because I opened a ticket - merely that I would create a patch against the current development trunk and see where it ended up. I wasn't expecting it to get anywhere without a core developer being involved, I just figured chances of getting a core developer involved and/or interested may be increased if it was a decision to be made, rather than a patch to be written.
 
In regards to your concerns, which mainly seem to be in regards to how many people would actually utilize this feature / are currently working around it I'm not sure how to address that. I suppose mixins are the solution for now then, however it does seem to me like something that anyone who is making extensive use of class based views will eventually come up against.

I'm not saying that you don't have a use for this type of entry point. What I'm saying is that you're advocating making the basic entry sequence of class based views (and thus, the documentation and learning curve) more complex, all to service a use case that can be achieved with subclassing. And, in the limited subset of people that have *huge* subclassing overheads and find themselves writing that subclassing code over and over again, that subset can write a new base subclass or mixing that introduces the complexity. 

What I *don't* see is a generic, across the board need that warrants *every* user being forced to carry the overhead so that *some* users have a convenience that can be achieved by other means.

That's fine, I was simply trying to get a discussion going and perhaps a decision from someone who is capable of making one, as that hadn't been done up until now. I'll do my best to try and convey what I'm trying to accomplish better in the future, as it seems there may have been a bit of a misunderstanding here.

Cheers,
Jordan

Aaron Merriam

unread,
Nov 7, 2012, 11:49:04 AM11/7/12
to django-d...@googlegroups.com
I wanted to post and modified version of a gist posted earlier in this thread.


I originally implemented the original structure of having an `init` hook which was called between setting request, args, and kwargs, but i quickly found that I had a few situations where I needed to fully hijack the response rather than just checking a permission or throwing an exception.  

I'm curious what others think of this.

Mike Fogel

unread,
Nov 8, 2012, 2:43:45 AM11/8/12
to django-d...@googlegroups.com
I prefer django-rest-framework's flow through dispatch(). You can override self.initial() to safely execute method-agnostic logic, but unlike your example, it doesn't have to return a response object. The response object is returned by the appropriate self.<method>() as usual.

Diederik van der Boor

unread,
Nov 8, 2012, 7:42:03 AM11/8/12
to django-d...@googlegroups.com
Op 7 nov. 2012, om 17:49 heeft Aaron Merriam het volgende geschreven:

I wanted to post and modified version of a gist posted earlier in this thread.


I originally implemented the original structure of having an `init` hook which was called between setting request, args, and kwargs, but i quickly found that I had a few situations where I needed to fully hijack the response rather than just checking a permission or throwing an exception.  

I'm curious what others think of this.

I really like the idea of this implementation. I do like to see some examples associated with this feature,
and I think that would be valuable for everyone :)

I still think such init() or initial() feature would be beneficial for CBV's,
and actually reduce complexity (cc Russell here) but the examples make the difference here :)

For example, how would this be written without a init method?


class PhotoListView(TabbedListView):
    """
    Contents of an photo album; a list of photo's.
    """
    model = Photo

    template_name = "photoalbum_album.html"
    permission_class = permissions.PhotoAlbumViewPermission

    def init(self):
        super(PhotoListView, self).init()  # runs permission checks
        self.photoalbum = get_object_or_404(PhotoAlbum, pk=self.kwargs['pk'])  # parent object that filters the list

    def get_queryset(self):
        return super(PhotoListView, self).get_queryset().in_album(self.photoalbum)

    def get_context_data(self, **kwargs):
        context = super(PhotoListView, self).get_context_data(**kwargs)
        context['photoalbum'] = self.photoalbum
        context['can_delete'] = self.is_authorized_for(PhotoDeleteView)
        return context


Off course you can, but I'd like to initiate that challenge to get a good view of the complexity trade-offs here.


Greetings,

Diederik

Russell Keith-Magee

unread,
Nov 8, 2012, 6:29:08 PM11/8/12
to django-d...@googlegroups.com
I'd like to offer an answer here, but it isn't clear to me at all what this is trying to do (or rather, what ordering dependencies are assumed to exist.

For some reason, init() is apparently doing permission checks by default -- but it isn't clear what causes those permission checks to be done; it also isn't clear how you can do permission checks before you've actually got an object to work with. 

As far as setting self.photoset -- that could be done in dispatch(), or in get(), or possibly even in get_queryset(). 

In short, there isn't enough detail here for me to pass comment.

Yours,
Russ Magee %-)

Aaron Merriam

unread,
Nov 9, 2012, 12:05:43 AM11/9/12
to django-d...@googlegroups.com
Without setting request, args, and kwargs on on the view instance (which is done during the base dispatch view), anything in the view that assumes these values are present cannot run.  

Most of my views end up with functions which retrieve an object and then do some basic validation to ensure that a user has permissions, or that the object is valid in some fashion, or that some set of conditions is met prior to allowing any method call to happen.  

I have found that without this init method, the vast majority of my views end up re-writing dispatch which includes the super call.  This is especially annoying when you have to compare some aspect of the requesting user with an object that must be looked up with something from args or kwargs.  My view often already has this machinery built in, but it can't function without dispatch setting request, args, and kwargs, so to accomplish my check, I have to duplicate the lookup code in my dispatch method.

I don't propose mine is the best solution, but I know that it is non-intrusive, simple, and covers my use cases well.  It is also simple to accomplish any number of things since `init` merely needs to return a falsy value to allow the request to pass on through, raise an exception if that type of failure is desired, or return a response of it wants to hijack the view entirely.

Russell Keith-Magee

unread,
Nov 9, 2012, 12:25:41 AM11/9/12
to django-d...@googlegroups.com
On Fri, Nov 9, 2012 at 1:05 PM, Aaron Merriam <aaronm...@gmail.com> wrote:
Without setting request, args, and kwargs on on the view instance (which is done during the base dispatch view), anything in the view that assumes these values are present cannot run.  

Most of my views end up with functions which retrieve an object and then do some basic validation to ensure that a user has permissions, or that the object is valid in some fashion, or that some set of conditions is met prior to allowing any method call to happen.  

I have found that without this init method, the vast majority of my views end up re-writing dispatch which includes the super call.  This is especially annoying when you have to compare some aspect of the requesting user with an object that must be looked up with something from args or kwargs.  My view often already has this machinery built in, but it can't function without dispatch setting request, args, and kwargs, so to accomplish my check, I have to duplicate the lookup code in my dispatch method.

I don't propose mine is the best solution, but I know that it is non-intrusive, simple, and covers my use cases well.  It is also simple to accomplish any number of things since `init` merely needs to return a falsy value to allow the request to pass on through, raise an exception if that type of failure is desired, or return a response of it wants to hijack the view entirely.


I'm starting to feel like I'm incredibly dense, because I still don't understand what your use case *is* - or, at least, why what your proposing provides any significant advantages over what you can do using basic Python inheritance techniques.

Specifically, I still can't see why:

class MyView(View):
    def  dispatch(self, request, *args, **kwargs):
        init()
        return super(MyView, self).dispatch(request, *args, **kwargs)

    def init():
        # your init logic here

is superior to the solution provided by using basic Python inheritance:

class MyView(View):
    def  dispatch(self, request, *args, **kwargs):
        # your init logic here
        return super(MyView, self).dispatch(request, *args, **kwargs)

You have exactly the same workflow, and exactly the same order of operations. You don't need to document any special CBV-specific API -- e.g., when/how init() will be invoked, and with what assumptions about the request environment can be made -- and you don't have to incur the overhead of a function call (ok - the overhead is tiny, but let's not pretend it's zero).

So - can someone explain to me what the advantage is? Why is this init() method needed?

Yours,
Russ Magee %-)

Jordan Hagan

unread,
Nov 9, 2012, 12:37:37 AM11/9/12
to django-d...@googlegroups.com
Hey Russ,

The main point of concern which I think you may be missing is that self.kwargs and self.args are set inside dispatch, so using other mixins that require self.kwargs and self.args to be set (most do) will not work, without doing:

def dispatch(self, request, *args, **kwargs):
    self.args = args;
    self.kwargs = kwargs
    self.init()
    return super(Class, self).dispatch(request, *args, **kwargs)

Which isn't very tidy, to me having self.args and self.kwargs be set twice (once in my overridden dispatch method, and once in the original dispatch) feels wrong. I can't give you a good reason for it, it just feels bad every time I do it. The only way to work around this is to override dispatch without calling the original, and essentially duplicate the original dispatch method with an init call added in.

Cheers,
Jordan

--
You received this message because you are subscribed to the Google Groups "Django developers" group.

Russell Keith-Magee

unread,
Nov 9, 2012, 1:05:01 AM11/9/12
to django-d...@googlegroups.com
Ok… so let's get this straight:

  * init() needs to have access to request, args, kwargs
  * That means your implementation of dispatch() needs to set them.

Sure. I'll pay that. If you assume that an init() method is required, then sure, you need to set up attributes to support it.

What I don't understand is why the need for an init() method isn't being challenged in the first place. 

Why on earth can't just just take the logic that you're putting in init(), and put it *in dispatch()*. The sequence of calls is *identical*, and since *args and **kwargs are locals, they don't need to be set anywhere. What's the problem with putting the init() logic in the dispatch() method in the way I described?

Yours,
Russ Magee %-)

Jordan Hagan

unread,
Nov 9, 2012, 1:34:11 AM11/9/12
to django-d...@googlegroups.com
Not quite, other Mixins, take for example SingleObjectMixin require access to request, args, kwargs so to utilize the functionality of existing mixins in init(), we need to set self.request, self.args and self.kwargs in our dispatch method before dropping down into init(), or do it inside init()

This is the same if we remove the init method entirely, and just try use the get_object() method on SingleObjectMixin in our overridden dispatch method.

Since authorization seems to be the main argument so far, I'll use it as an example - currently we have to do the following:

class MyModel(models.Model):
    name = models.CharField(max_length=100)

    def auth(self, user):
        # do auth

class MyView(SingleObjectMixin, View):
    model = MyModel

    def dispatch(self, request, *args, **kwargs):
        self.request = request
        self.args = args
        self.kwargs = kwargs
        
        object = self.get_object()
        if object.auth(request.user):
            return super(MyView, self).dispatch(request, *args, **kwargs)
        else:
            # fail somehow

    def get(self, request, *args, **kwargs):
        # behave normally

whereas with some kind of init method, this becomes:

class MyView(SingleObjectMixin, View):
    model = MyModel

    def init(self):
        object = self.get_object()
        if not object.auth(self.request.user):
            # fail somehow

    def get(self, request, *args, **kwargs):
        # behave normally

The example is a bit crude as I just whipped it up, but it gets the point across I think.

Sorry for dragging this out, I did attempt to explain this earlier however perhaps I didn't do the best job.

Cheers,
Jordan

Sebastian Goll

unread,
Nov 9, 2012, 1:43:33 AM11/9/12
to django-d...@googlegroups.com
On Fri, 9 Nov 2012 14:05:01 +0800
Russell Keith-Magee <rus...@keith-magee.com> wrote:

> Why on earth can't just just take the logic that you're putting in init(),
> and put it *in dispatch()*. The sequence of calls is *identical*, and since
> *args and **kwargs are locals, they don't need to be set anywhere. What's
> the problem with putting the init() logic in the dispatch() method in the
> way I described?

I think the problem is that order of initialization _is_ reversed when
inheritance comes into the game, in the case of handling everything at
the beginning of dispatch(). Consider:

class DepartmentMixin(object):
def initialize(self, request, *args, **kwargs):
super(DepartmentMixin, self).initialize(request, *args, **kwargs)
self.department = get_object_or_404(Department, slug=kwargs['department'])

class ShiftMixin(DepartmentMixin):
def initialize(self, request, *args, **kwargs):
super(ShiftMixin, self).initialize(request, *args, **kwargs)
self.shift = get_object_or_404(Shift, department=self.department, datetime__year=kwargs['year'])

This would not be possible in dispatch() because the super method must
be called at the end of the child method (tail recursion):

class DepartmentMixin(object):
def dispatch(self, request, *args, **kwargs):
self.department = get_object_or_404(Department, slug=kwargs['department'])
return super(DepartmentMixin, self).dispatch(request, *args, **kwargs)

class ShiftMixin(DepartmentMixin):
def initialize(self, request, *args, **kwargs):
# THIS DOESN'T WORK: self.department HAS NOT BEEN SET YET.
self.shift = get_object_or_404(Shift, department=self.department, datetime__year=kwargs['year'])
return super(ShiftMixin, self).dispatch(request, *args, **kwargs)

I think the control flow is easier to follow in case of initialize()
vs. dispatch(). This doesn't mean that I think initialize() should be
part of core, but the described situation is something I came across in
real life.

Best wishes,
Sebastian.

Daniel Sokolowski

unread,
Nov 9, 2012, 10:28:45 AM11/9/12
to django-d...@googlegroups.com
I’ve done the below in the past, the only issue with that is if you have side effects in parent’s dispatch you don’t want executed but you would also run that risk if you had an initialize() method work flow; in the end I find dispatch() is enough in my experience.
 
def dispatch(self, request, *args, **kwargs):
    parent_dispatch_return = super(Class, self).dispatch(request, *args, **kwargs)
    ...my code based on values based on the super call...
    return parent_dispatch_return
To unsubscribe from this group, send email to mailto:django-developers%2Bunsu...@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
 

Aaron Merriam

unread,
Nov 9, 2012, 3:12:36 PM11/9/12
to django-d...@googlegroups.com
That pattern has nasty side-effects.  It can be used in some cases but it fails in most.

Diederik van der Boor

unread,
Nov 12, 2012, 8:38:29 AM11/12/12
to django-d...@googlegroups.com
Op 9 nov. 2012, om 07:05 heeft Russell Keith-Magee het volgende geschreven:

What I don't understand is why the need for an init() method isn't being challenged in the first place. 

Why on earth can't just just take the logic that you're putting in init(), and put it *in dispatch()*. The sequence of calls is *identical*, and since *args and **kwargs are locals, they don't need to be set anywhere. What's the problem with putting the init() logic in the dispatch() method in the way I described?


It has to do with the order of execution, the derived class always runs first.
When a permission check is in dispatch(), anything that overrides dispatch() runs before the permission check.
The same also applies to any other initialization code, a derived class can never build on top of that because it should call


Please allow me to paraphrase my previous explanation (https://groups.google.com/d/msg/django-developers/7c7aI-slGNc/a-DYFrIM3ZgJ)


What is the problem with overriding dispatch()?
When overriding dispatch(), get() or post() the flow is always:

def dispatch(self, request, *args, **kwargs):
    # my code here.
    return super(…).dispatch(request, *args, **kwargs)

The same also applies to get() and post().
In other words, the last deriving class on top of the inheritance chain is always initializing first before it's base classes.
It can't rely on a base class to do some initialization.

With our permission check in the base class' dispatch() method, anything deriving from that effectively
couldn't override dispatch() anymore because that would run before the permission check.

How does the init method fix this?
By doing a self.init() in the top-most dispatch() method, each class in the inheritance chain has a chance to fetch the objects it needs to have.

That code can be written as:

def init(self):
    super(..).init()
    # my code here.

Now, the base class can initialize, then the deriving class.
With a larger inheritance chain, this behavior becomes crucial.
Each class can build upon what the other has prepared already.



Greetings,

Diederik

Daniel Sokolowski

unread,
Nov 14, 2012, 8:48:54 AM11/14/12
to django-d...@googlegroups.com
Can you elaborate the nasty side effects you are thinking of? I can’t think of any that that the base views do to warrant your statement.

To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
 

Alex Ogier

unread,
Nov 14, 2012, 10:35:52 AM11/14/12
to django-d...@googlegroups.com
For example, you miss Http404 and other error responses, which are implemented as exceptional control flow. In addition, you can't do any preprocessing of the request; for example, you can't set up any invariants before your actual view method is called.

Best,
Alex Ogier

Daniel Sokolowski

unread,
Nov 14, 2012, 10:58:43 AM11/14/12
to django-d...@googlegroups.com
Hmm, ok, so that is only and issue if you don’t know the side effects calling super when dealing with non django provided views - and you can still pre-process request before calling super. 
 
So at the moment I can understand the appeal of init() as shown in the example but fail to see where I couldn’t just use dispatch() instead. In the past when we needed to tie permission checks I opted into making a view mixin to override dispatch similar to this one: https://github.com/lukaszb/django-guardian/blob/master/guardian/mixins.py
 
Perhaps people that do find this init() method worthwhile should voice so. Thanks
From: Alex Ogier
Sent: Wednesday, November 14, 2012 10:35 AM
Subject: Re: Class based views: A standard hook for http-method-independent code
For example, you miss Http404 and other error responses, which are implemented as exceptional control flow. In addition, you can't do any preprocessing of the request; for example, you can't set up any invariants before your actual view method is called.
 
Best,
Alex Ogier


On Wed, Nov 14, 2012 at 8:48 AM, Daniel Sokolowski <daniel.s...@klinsight.com> wrote:
Can you elaborate the nasty side effects you are thinking of? I can’t think of any that that the base views do to warrant your statement.
 
Sent: Friday, November 09, 2012 3:12 PM
Subject: Re: Class based views: A standard hook for http-method-independent code
 
That pattern has nasty side-effects.  It can be used in some cases but it fails in most.

On Friday, November 9, 2012 8:28:47 AM UTC-7, Daniel Sokolowski wrote:
I’ve done the below in the past, the only issue with that is if you have side effects in parent’s dispatch you don’t want executed but you would also run that risk if you had an initialize() method work flow; in the end I find dispatch() is enough in my experience.
 
def dispatch(self, request, *args, **kwargs):
    parent_dispatch_return = super(Class, self).dispatch(request, *args, **kwargs)
    ...my code based on values based on the super call...
    return parent_dispatch_return
--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
 

Aaron Merriam

unread,
Nov 14, 2012, 11:18:34 PM11/14/12
to django-d...@googlegroups.com
If the super call changes any data then by the time you've run whatever code comes after the super call, the changes have already occured.  

  • If you wait to call super before running your own code, then request, args, and kwargs are not available on the request, so anything that depends on them being there (such as self.get_object()) will not work, so it must be re-implemented, 
  • Otherwise you have to set request, args, kwargs yourself which does not feel very DRY.
For me, the entire reason I would like this change, is so that I can do something before dispatch that uses self.request/args/kwargs.  Everything I want can be accomplished within dispatch, but not as cleanly, or as DRY as if this method hook existed.
Message has been deleted
Message has been deleted

Daniel Sokolowski

unread,
Nov 16, 2012, 9:09:20 AM11/16/12
to django-d...@googlegroups.com
I like this approach.
 
Sent: Thursday, November 15, 2012 7:27 AM
Subject: Re: Class based views: A standard hook for http-method-independent code
 
I have a slightly different proposal, one where we can avoid the extra hook but hopefully cover everyone's use cases too.
 
 
I've personally never liked the setting of args, kwargs & request from within dispatch since it seems like it's feature creep of the dispatch method. However I'm also in the same boat as many of the other posters here in needing to do permissions related checks before dispatch is called.
 
With my suggestion above you would be able to put your pre-dispatch code in a subclasses overridden dispatch before calling super while also depending on args, kwargs & request on self.

To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
 
 

Aaron Merriam

unread,
Nov 17, 2012, 12:02:14 AM11/17/12
to django-d...@googlegroups.com
This is great.  +1.  Better than an additional hook.

Jeremy Dunck

unread,
Nov 16, 2012, 1:52:44 PM11/16/12
to django-d...@googlegroups.com
+1, this looks like a good change anyway and doesn't smell to me.
Message has been deleted

Jordan Hagan

unread,
Nov 18, 2012, 4:51:53 PM11/18/12
to django-d...@googlegroups.com
+1, this looks like a better solution than the hook method.

Mike Fogel

unread,
Nov 21, 2012, 1:30:37 AM11/21/12
to django-d...@googlegroups.com
+1, this allows me what I want - to have http method-independent code that uses self.args, self.kwargs and self.request.

George Hickman

unread,
Feb 11, 2013, 12:16:14 PM2/11/13
to django-d...@googlegroups.com


On Mon, Feb 11, 2013 at 5:05 PM, Michael Bylstra <mbyl...@gmail.com> wrote:
The git link seems to be broken. I'd love to see this bit of code everyone is raving about!
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.

To post to this group, send email to django-d...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages