36

I am trying to decorate a Django view by two decorators, one for checking login, and one for checking is_active.

The first one is the built-in @login_required, and the second one is the following:

def active_required(function):
    dec = user_passes_test(lambda u: u.is_active, '/notallowed', '')
    return dec(function)

Now, the decorators in Python work inside out, however the following does not work:

@active_required
@login_required
def foo(request):
    ...

I want to first check if the user is logged in, and redirect to login page if not, and if he or she is logged in, I want to check whether he or she is active, and if not, perform redirect to '/notallowed'.

What happens is that if the login_required fails, the user is not redirected to the login page, but @active_required is executed, and since the user is null in that case, @active_required decorator fails and the user is redirected to /notallowed.

Changing the order seems to work,

@login_required
@active_required
def foo(request):
    ...

but I suspect there is something wrong with this approach too.

What is the proper way to combine two decorators, and why does the execution order differ from simple Python decorators?

arogachev
  • 33,150
  • 7
  • 114
  • 117
ustun
  • 6,941
  • 5
  • 44
  • 57

4 Answers4

40

Now, the decorators in Python work from the inside out

Well I guess that depends on your definition of inside out. In your case, you want @login_required to execute first, and so it should be the "outermost" (top) decorator.

As you noted, your last example works, and is indeed the correct way to do this.

edit

The confusion might be how these particular decorators work.

@login_required(@original_view) returns a new view, which first checks if you are logged in, and then calls original_view

so


    @login_required(
        @active_required(
            @my_view
        )
    )

first checks if you are logged in, then
    first(second) checks if you are active, then
        runs my_view
Fidelis Maina
  • 55
  • 1
  • 11
second
  • 28,029
  • 7
  • 75
  • 76
  • 2
    Hmm, I am still slightly confused about the order: http://stackoverflow.com/a/739665/72436 and http://stackoverflow.com/a/8715839/72436 suggest otherwise. – ustun Jan 03 '12 at 17:18
  • 1
    OK, I think you nailed it, the difference lies in returning a function vs calling it. – ustun Jan 03 '12 at 17:31
18

Decorators are applied in the order they appear in the source. Thus, your second example:

@login_required
@active_required
def foo(request):
    ...

is equivalent to the following:

def foo(request):
    ...
foo = login_required(active_required(foo))

Thus, if the code of one decorator depends on something set by (or ensured by) another, you have to put the dependent decorator "inside" the depdended-on decorator.

However, as Chris Pratt notes, you should avoid having decorator dependencies; when necessary, create a single new decorator that calls both in the right order.

dcrosta
  • 26,009
  • 8
  • 71
  • 83
  • OK, but I want just the reverse: First, login_required should be applied, then active_required. So, shouldn't it be as in the first example I gave? – ustun Jan 03 '12 at 17:18
  • In this case, `login_required` is applied first -- one way of thinking about it is that the call to `foo` first passes through the function returned by `login_required`, then through the function returned by `active_required`. You can step through it with PDB, or add debugging `print`s to see what I mean. – dcrosta Jan 03 '12 at 18:46
12

It only really makes sense to stack decorators if they have truly unique functionality. Based on your description, there's never going to be a scenario where you will want to use active_required but not login_required. Therefore, it makes more sense to have a login_and_active_required decorator that checks both and branches accordingly. Less to type, less to document, and negates the problem.

Chris Pratt
  • 232,153
  • 36
  • 385
  • 444
  • OK, I supposed Django builtins would have been more reliable than custom code. It is a little strange that Django doesn't have this decorator, though, it should be pretty common. I have seen some bug reports that were marked as WONTFIX. – ustun Jan 03 '12 at 17:28
  • Agreed. Since `is_active` is built-in, and pretty much negates `login_required` in most scenarios, the devs should have accounted for this out-of-the-box, but c'est la vis. – Chris Pratt Jan 03 '12 at 19:17
5

To explain it a bit more (I was also confused at first): active_required is applied first in a sense that it takes my_view and wraps it in some code. Then login_required is applied and wraps the result in some more code.

But when this wrapped version of my_view is actually invoked, first the code added by login_required is executed (checking that you're logged in), then the code added by active_required is executed (checking that you're active) and then finally my_view is executed.

Dennis Golomazov
  • 16,269
  • 5
  • 73
  • 81
  • That's a good way to differentiate to clear out the confusion in the comments here : http://stackoverflow.com/a/8715821/781695 – user Jun 15 '14 at 08:17