From 904fd7c91d42515d51ef698dfa4ce332efbb187c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 27 Apr 2018 01:41:19 -0500 Subject: [PATCH 1/5] Revert teardown solution --- readthedocs/rtd_tests/tests/test_subprojects.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index 5e86fad0ef1..cffbdcdee48 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -3,6 +3,7 @@ import mock import django_dynamic_fixture as fixture from django.contrib.auth.models import User +from django.core.urlresolvers import reverse, set_urlconf from django.test import TestCase from django.test.utils import override_settings @@ -185,6 +186,12 @@ def setUp(self): relation.save() fixture.get(Project, slug='sub_alias', language='ya') + def tearDown(self): + # The urlconf value is preserved for the current thread, + # (is modified to ``readthedocs.core.urls.subdomain``) + # this causes errors when using the reverse function. + # This method call deletes that setting. + # set_urlconf(None) @override_settings( PRODUCTION_DOMAIN='readthedocs.org', From 31bdebf0f388bd619be2271a15d8228f72ed0f56 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 27 Apr 2018 01:41:51 -0500 Subject: [PATCH 2/5] Restore urlconf on middleware on process_response --- readthedocs/core/middleware.py | 10 ++++++++++ readthedocs/rtd_tests/tests/test_subprojects.py | 1 + 2 files changed, 11 insertions(+) diff --git a/readthedocs/core/middleware.py b/readthedocs/core/middleware.py index a6034ccd047..09f0a4758a0 100644 --- a/readthedocs/core/middleware.py +++ b/readthedocs/core/middleware.py @@ -9,6 +9,7 @@ from django.contrib.sessions.middleware import SessionMiddleware from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned +from django.core.urlresolvers import set_urlconf, get_urlconf from django.http import Http404, HttpResponseBadRequest from readthedocs.core.utils import cname_to_slug @@ -53,6 +54,9 @@ def process_request(self, request): 'PRODUCTION_DOMAIN', 'readthedocs.org' ) + # Django sets the urlconf for the current thread + # so we need to set this again later. + self.current_urlconf = get_urlconf() if public_domain is None: public_domain = production_domain @@ -130,6 +134,12 @@ def process_request(self, request): # Normal request. return None + def process_response(self, request, response): + # Reset URLconf for this thread + # to the original one. + set_urlconf(self.current_urlconf) + return response + class SingleVersionMiddleware(object): diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index cffbdcdee48..d77b1eb12b8 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -192,6 +192,7 @@ def tearDown(self): # this causes errors when using the reverse function. # This method call deletes that setting. # set_urlconf(None) + pass @override_settings( PRODUCTION_DOMAIN='readthedocs.org', From 00391b81f366cb0ac00b953e59616bc23c5a43bf Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 27 Apr 2018 01:53:22 -0500 Subject: [PATCH 3/5] More general solution --- readthedocs/core/middleware.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/readthedocs/core/middleware.py b/readthedocs/core/middleware.py index 09f0a4758a0..732e93a86d1 100644 --- a/readthedocs/core/middleware.py +++ b/readthedocs/core/middleware.py @@ -54,9 +54,6 @@ def process_request(self, request): 'PRODUCTION_DOMAIN', 'readthedocs.org' ) - # Django sets the urlconf for the current thread - # so we need to set this again later. - self.current_urlconf = get_urlconf() if public_domain is None: public_domain = production_domain @@ -137,7 +134,7 @@ def process_request(self, request): def process_response(self, request, response): # Reset URLconf for this thread # to the original one. - set_urlconf(self.current_urlconf) + set_urlconf(None) return response From 3ea856b3e2352af78fc247f347c17350c142f2cb Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 27 Apr 2018 01:55:37 -0500 Subject: [PATCH 4/5] Remove teardown completely --- readthedocs/rtd_tests/tests/test_subprojects.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_subprojects.py b/readthedocs/rtd_tests/tests/test_subprojects.py index d77b1eb12b8..83932a76e50 100644 --- a/readthedocs/rtd_tests/tests/test_subprojects.py +++ b/readthedocs/rtd_tests/tests/test_subprojects.py @@ -3,7 +3,6 @@ import mock import django_dynamic_fixture as fixture from django.contrib.auth.models import User -from django.core.urlresolvers import reverse, set_urlconf from django.test import TestCase from django.test.utils import override_settings @@ -186,14 +185,6 @@ def setUp(self): relation.save() fixture.get(Project, slug='sub_alias', language='ya') - def tearDown(self): - # The urlconf value is preserved for the current thread, - # (is modified to ``readthedocs.core.urls.subdomain``) - # this causes errors when using the reverse function. - # This method call deletes that setting. - # set_urlconf(None) - pass - @override_settings( PRODUCTION_DOMAIN='readthedocs.org', USE_SUBDOMAIN=False, From 05d2c27b3f91587950041d505d23f4b80a14eaea Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 1 May 2018 01:09:09 -0500 Subject: [PATCH 5/5] Add test --- .../rtd_tests/tests/test_middleware.py | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_middleware.py b/readthedocs/rtd_tests/tests/test_middleware.py index 1a312e1a130..fbedd4cb16b 100644 --- a/readthedocs/rtd_tests/tests/test_middleware.py +++ b/readthedocs/rtd_tests/tests/test_middleware.py @@ -1,11 +1,13 @@ from __future__ import absolute_import + from django.http import Http404 from django.core.cache import cache +from django.core.urlresolvers import get_urlconf, set_urlconf from django.test import TestCase from django.test.client import RequestFactory from django.test.utils import override_settings -from django_dynamic_fixture import get, new +from django_dynamic_fixture import get from corsheaders.middleware import CorsMiddleware from mock import patch @@ -40,6 +42,29 @@ def test_proper_subdomain(self): self.assertEqual(request.subdomain, True) self.assertEqual(request.slug, 'pip') + @override_settings(PRODUCTION_DOMAIN='readthedocs.org') + def test_restore_urlconf_after_request(self): + """ + The urlconf attribute for the current thread + should remain intact after each request, + When is set to None it means 'use default from settings'. + """ + set_urlconf(None) + urlconf = get_urlconf() + self.assertIsNone(urlconf) + + self.client.get(self.url, HTTP_HOST='pip.readthedocs.org') + urlconf = get_urlconf() + self.assertIsNone(urlconf) + + self.client.get(self.url) + urlconf = get_urlconf() + self.assertIsNone(urlconf) + + self.client.get(self.url, HTTP_HOST='pip.readthedocs.org') + urlconf = get_urlconf() + self.assertIsNone(urlconf) + @override_settings(PRODUCTION_DOMAIN='prod.readthedocs.org') def test_subdomain_different_length(self): request = self.factory.get(self.url, HTTP_HOST='pip.prod.readthedocs.org')