Unable to use urlresolvers (url,reverse ) feature in redirect argument of decorator user_passes_test

[anonymous]

trying something like :

@user_passes_test(lambda u: my_function(u),login_url = reverse('my_reverse_key'))
def my view(request)
   pass
   ...

yields an error when importing "urls.py" (i.e.really quickly)

[yml]

One other side effect of the issue describes above is that it breaks the url resolution for reverse, {%url%}, permalink and company.
I have the case in one of my application where some views are protected by :
"""
@user_passes_test(lambda u: Group.objects.get(name='admin') in u.groups.all(), login_url=reverse("auth_login"))
"""
This lead to some nasty bugs in the url resolution of an other view.

I hope this description will help.

[Eratosfen]

It looks like problem concerns not only user_passes_test. reverse() can't be used at all decorators of views.
Since the decorators of your views are evaluated during parsing urls.py you have an 'chicken - egg' problem. The method reverse() can't be used since urls.py is not read.

The solution, I see, is to evaluate the reverse() lazy.

[yml]

I do not know if this information can help. I have observed that @cache (decorator) does not trigger any error message.

[Thomas Güttler <hv@…>]

I have this problem, too.

Here is my solution:

# file lazy.py
from django.core import urlresolvers

class lazy_string(object):
    def __init__(self, function, *args, **kwargs):
        self.function=function
        self.args=args
        self.kwargs=kwargs
        
    def __str__(self):
        if not hasattr(self, 'str'):
            self.str=self.function(*self.args, **self.kwargs)
        return self.str

def reverse(*args, **kwargs):
    return lazy_string(urlresolvers.reverse, *args, **kwargs)

Just use

from lazy import reverse

instead of

from django.core.urlresovlers import reverse

[Malcolm Tredinnick]

Should be possible to solve this using our existing django.utils.functional.lazy() function to create lazy_reverse. Reinventing that wheel is something to avoid. reverse() is a function that returns a unicode object (we should ensure that it does return unicode and not str). So there are a few things to do here: auditing existing reverse, hooking up reverse_lazy, documentation, tests, etc, but that sounds like a plan. At least in my head.

[Thomas Güttler]

I think it is better to alter reverse() than to invent a new method lazy_reverse(). The recursive import error results
in very strange Exceptions: Modules are missing methods/attributes that are in the source code. It took me some time
to find out what's wrong, although I am using Python since seven years.

[Antti Kaihola]

This urls.py works as expected (redirects from / to /comehere/), so django.utils.functional.lazy() seems to work as mtredinnick suggested:

from django.conf.urls.defaults import *
from django.core.urlresolvers import reverse
from django.utils.functional import lazy
from django.http import HttpResponse

reverse_lazy = lazy(reverse, unicode)

urlpatterns = patterns('',
    url(r'^comehere/', lambda request: HttpResponse('Welcome!'), name='comehere'),
    url(r'^$', 'django.views.generic.simple.redirect_to',
        {'url': reverse_lazy('comehere')}, name='root')
)

[Chris Beaven]

Patch provides a new reverse_lazy function (with documentation), and changes the user_passes_test decorator function to not use the login_url at runtime.

I'm not quite sure how you'd test this, since it'd require testing it before the URL names map is loaded.

[Chris Beaven]

PS: for anyone code reviewing, yes str is the right type instead of unicode, since reverse passes through iri_to_uri which converts it to a string type.

[Chris Beaven]

If anyone needs this functionality currently, here's a stand-alone object I created which works well enough:

from django.core.urlresolvers import reverse
from django.utils.functional import LazyObject


class ReverseLazy(LazyObject):
    """
    A lazily evaluated URL reversing object.

    Takes identical arguments to the ``reverse`` method in
    ``django.core.urlresolvers``.

    """

    def __init__(self, viewname, *args, **kwargs):
        self.__dict__['_reverse_args'] = (viewname,) + args
        self.__dict__['_reverse_kwargs'] = kwargs
        super(ReverseLazy, self).__init__()

    def _setup(self):
        self._wrapped = reverse(*self._reverse_args, **self._reverse_kwargs)

    def __nonzero__(self):
        """
        Always assuming a boolean check passes.

        This avoids one test in the ``user_passes_test`` decorator generator 
        in ``django.contrib.auth``.

        """
        return True

    def __str__(self):
        self._setup()
        return str(self._wrapped)

[Eric Holscher]

Lets fix this for 1.3, it has a patch from SmileyChris, who's now a core dev, so that should make it a bit easier :)

refs: ​http://twitter.com/bkmontgomery/status/28421302314

[Sean Brant]

I needed something like this for generic pagination templates that take want to use {% url %} but have variable viewnames and args. Probably not the best way to handle it but this is how I did it, not sure if it helps.

Here is the LazyReverse class::

    from django.core.urlresolvers import reverse

    class LazyReverse(object):
        def __init__(self, viewname, urlconf=None, args=None,
                kwargs=None, prefix=None, current_app=None):
            self.viewname = viewname
            self.urlconf = urlconf
            self.args = args or []
            self.kwargs = kwargs or {}
            self.prefix = prefix
            self.current_app = current_app

        def reverse(self, *extra_args, **extra_kwargs):
            args = []
            args.extend(self.args)
            args.extend(extra_args)
            kwargs = {}
            kwargs.update(self.kwargs)
            kwargs.update(extra_kwargs)
            return reverse(self.viewname, self.urlconf, args,
                kwargs, self.prefix, self.current_app)

In your view you would do::

    context['reverse_obj'] = LazyReverse('myview', key=val)

In your template::

    {% lazy_url reverse_obj extra_arg %}

[Chris Beaven]

(In [14733]) Fixes #11025 -- ability to specify LOGIN_URL as full qualified absolute URL.

auth.views.login now allows for login redirections for different schemes
with the same host (or no host even, e.g. '​https:///login/')

auth.decorators.login_required can now use lazy urls (refs #5925)

[Chris Beaven]

Well, the "fix" is now as simple as a one line code change and documentation. I'm going to bump to a design decision as to whether or not we should provide the lazy_reverse method.

I think we should, since it may not be obvious that it should be lazy(reverse, str) rather than .., unicode

[Simon Charette]

Is this going to make 1.3? It's really useful when extending ModelAdmin and defining named urls within get_urls.

[Preston Timmons]

Attached is a patch updated for Django 1.3. We also added tests. We removed the changes that Chris made to the auth decorators, which didn't apply cleanly, and didn't seem to effect this patch.

[Preston Timmons]

The patch 5925.2.diff is corrupt. The second patch was meant to replace it.

[Julien Phalip]

I agree with SmileyChris that reverse_lazy() is the right approach as it is more explicit, thus marking this ticket as accepted. I've updated the patch with tests using class-based views instead of the deprecated "simple" ones, also tweaked the doc to reflect that, and added release notes. I'm hoping this gets checked in as it would help tickets like #15294 move forward. Not marking RFC yet as I'd like to have another pair of eyes review this patch.

[Julien Phalip]

Added more tests for the original use case of this ticket (i.e. user_passes_test)

[Jannis Leidel]

In [16121]:

(The changeset message doesn't reference this ticket)

[Jannis Leidel]

In [16122]:

Added file forgotten in r16121. Refs #5925. Thanks, Julien.

[tomv2]

It's only a name in a test but in [16121] was LazyRedictView intending to be spelt LazyRedirectView?

[Julien Phalip]

Replying to tomv2:

It's only a name in a test but in [16121] was LazyRedictView intending to be spelt LazyRedirectView?

Yes, this is a minor typo.

[Jannis Leidel]

In [16217]:

Fixed minor typo in tests introduced in r16121. Refs #5925.

[Jacob]

Milestone 1.3 deleted