Opened 6 years ago

Closed 6 years ago

#29673 closed Bug (fixed)

Thread urlconf isn't reset after response complete

Reported by: tpict Owned by: Matthew Power
Component: Core (URLs) Version: dev
Severity: Normal Keywords:
Cc: Keryn Knight, Herbert Fortes, Adam Johnson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When setting the urlconf on a request (e.g. in middleware for handling multiple domains pointing to the same Django app), it's not reset until the start of the next request. Since urlconf is threadlocal, this causes problems when running a suite of tests, even if the tests pass when ran individually. For example:

  • Django test client makes a request that triggers a middleware to change the urlconf
  • reverse is called with no urlconf kwarg, expecting to be given the urlconf specified by ROOT_URLCONF
  • test throws NoReverseMatch

I took this problem to the IRC and found that another person recently messaged about the same thing: https://botbot.me/freenode/django/2018-08-07/?msg=103000008&page=3

Change History (13)

comment:1 by tpict, 6 years ago

Version: 2.12.0

comment:2 by Carlton Gibson, 6 years ago

Any chance you could put this into a test case?

comment:3 by Keryn Knight, 6 years ago

Cc: Keryn Knight added

comment:4 by Tim Graham, 6 years ago

There are a number of places in Django's test suite that set request.urlconf so I think a test is indeed needed to demonstrate the issue.

comment:5 by Tim Graham, 6 years ago

Resolution: needsinfo
Status: newclosed

in reply to:  5 comment:6 by tpict, 6 years ago

Replying to Tim Graham:
Sorry about the delay. I've just written a test case: https://github.com/tpict/django-29673-test-case

comment:7 by tpict, 6 years ago

Resolution: needsinfo
Status: closednew

comment:8 by Carlton Gibson, 6 years ago

Triage Stage: UnreviewedAccepted
Version: 2.0master

Hi Tom.

Thanks for the example project. It's a curious one:

(tmp-d8650c87792fd55) ~/Downloads/django-29673-test-case-master $ ./manage.py test
..E.
======================================================================
ERROR: test_primary (django29673.secondary.tests.SecondaryTestCase)
Do stuff with the primary site.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/carlton/Downloads/django-29673-test-case-master/django29673/secondary/tests.py", line 12, in test_primary
    response = self.client.get(reverse("primary-site"))
  File "/Users/carlton/ve/tmp-d8650c87792fd55/lib/python3.6/site-packages/django/urls/base.py", line 90, in reverse
    return iri_to_uri(resolver._reverse_with_prefix(view, prefix, *args, **kwargs))
  File "/Users/carlton/ve/tmp-d8650c87792fd55/lib/python3.6/site-packages/django/urls/resolvers.py", line 622, in _reverse_with_prefix
    raise NoReverseMatch(msg)
django.urls.exceptions.NoReverseMatch: Reverse for 'primary-site' not found. 'primary-site' is not a valid view function or pattern name.

----------------------------------------------------------------------
Ran 4 tests in 0.017s

FAILED (errors=1)

(tmp-d8650c87792fd55) ~/Downloads/django-29673-test-case-master $ ./manage.py test django29673.secondary.tests.SecondaryTestCase
..
----------------------------------------------------------------------
Ran 2 tests in 0.009s

OK

The workaround you demonstrate in the the middleware.py file is interesting:

        # To make tests pass, comment the return statement and uncomment this
        # block:
        try:
            return self.get_response(request)
        finally:
            if getattr(request, "urlconf", None) is not None:
                set_urlconf(None)

If you'd like to push this forward, that would be great. Next step I guess I would be to bring the test case into the Django test suite in a PR.

comment:9 by Herbert Fortes, 6 years ago

Cc: Herbert Fortes added

comment:10 by Matthew Power, 6 years ago

Owner: changed from nobody to Matthew Power
Status: newassigned

comment:11 by Adam Johnson, 6 years ago

Cc: Adam Johnson added

comment:12 by Adam Johnson, 6 years ago

Has patch: set
Last edited 6 years ago by Tim Graham (previous) (diff)

comment:13 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 18098d26:

Fixed #29673 -- Reset the URLconf at the end of each request.

Co-authored-by: Ross Thorne <rmwthorne@…>

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