Convert edxmako to use django_crum to get the current request

cms/envs/common.py

@@ -350,9 +350,6 @@ MIDDLEWARE_CLASSES = (
 
     'codejail.django_integration.ConfigureCodeJailMiddleware',
 
-    # needs to run after locale middleware (or anything that modifies the request context)
-    'edxmako.middleware.MakoMiddleware',
-
     # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500
     'ratelimitbackend.middleware.RateLimitMiddleware',
 

common/djangoapps/edxmako/middleware.py → common/djangoapps/edxmako/request_context.py

@@ -11,29 +11,22 @@
 #   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 #   See the License for the specific language governing permissions and
 #   limitations under the License.
+#
+# This file has been modified by edX.org
+
+"""
+Methods for creating RequestContext for using with Mako templates.
+"""
+
 
-import threading
 from django.conf import settings
 from django.template import RequestContext
 from django.template.context import _builtin_context_processors
 from django.utils.module_loading import import_string
 from util.request import safe_get_host
+from crum import get_current_request
 
-from request_cache.middleware import RequestCache
-
-REQUEST_CONTEXT = threading.local()
-
-
-class MakoMiddleware(object):
-
-    def process_request(self, request):
-        """ Process the middleware request. """
-        REQUEST_CONTEXT.request = request
-
-    def process_response(self, __, response):
-        """ Process the middleware response. """
-        REQUEST_CONTEXT.request = None
-        return response
+from request_cache import get_request_cache
 
 
 def get_template_context_processors():
@@ -50,15 +43,17 @@ def get_template_request_context():
     Returns the template processing context to use for the current request,
     or returns None if there is not a current request.
     """
-    request = getattr(REQUEST_CONTEXT, "request", None)
-    if not request:
-        return None
 
-    request_cache_dict = RequestCache.get_request_cache().data
-    cache_key = "edxmako_request_context"
+    request_cache_dict = get_request_cache('edxmako')
+    cache_key = "request_context"
     if cache_key in request_cache_dict:
         return request_cache_dict[cache_key]
 
+    request = get_current_request()
+
+    if request is None:
+        return None
+
     context = RequestContext(request)
     context['is_secure'] = request.is_secure()
     context['site'] = safe_get_host(request)

common/djangoapps/edxmako/shortcuts.py

@@ -19,7 +19,7 @@ import logging
 from microsite_configuration import microsite
 
 from edxmako import lookup_template
-from edxmako.middleware import get_template_request_context
+from edxmako.request_context import get_template_request_context
 from django.conf import settings
 from django.core.urlresolvers import reverse
 log = logging.getLogger(__name__)

common/djangoapps/edxmako/template.py

@@ -15,7 +15,7 @@
 import edxmako
 
 from django.conf import settings
-from edxmako.middleware import get_template_request_context
+from edxmako.request_context import get_template_request_context
 from edxmako.shortcuts import marketing_link
 from mako.template import Template as MakoTemplate
 

common/djangoapps/edxmako/tests.py

@@ -3,14 +3,14 @@ from mock import patch, Mock
 import unittest
 import ddt
 
+from request_cache.middleware import RequestCache
 from django.conf import settings
 from django.http import HttpResponse
 from django.test import TestCase
 from django.test.utils import override_settings
 from django.test.client import RequestFactory
 from django.core.urlresolvers import reverse
-import edxmako.middleware
-from edxmako.middleware import get_template_request_context
+from edxmako.request_context import get_template_request_context
 from edxmako import add_lookup, LOOKUP
 from edxmako.shortcuts import (
     marketing_link,
@@ -81,52 +81,78 @@ class AddLookupTests(TestCase):
         self.assertTrue(dirs[0].endswith('management'))
 
 
-class MakoMiddlewareTest(TestCase):
+class MakoRequestContextTest(TestCase):
     """
     Test MakoMiddleware.
     """
 
     def setUp(self):
-        super(MakoMiddlewareTest, self).setUp()
-        self.middleware = edxmako.middleware.MakoMiddleware()
+        super(MakoRequestContextTest, self).setUp()
         self.user = UserFactory.create()
         self.url = "/"
         self.request = RequestFactory().get(self.url)
         self.request.user = self.user
         self.response = Mock(spec=HttpResponse)
 
-    def test_clear_request_context_variable(self):
+        self.addCleanup(RequestCache.clear_request_cache)
+
+    def test_with_current_request(self):
+        """
+        Test that if get_current_request returns a request, then get_template_request_context
+        returns a RequestContext.
+        """
+
+        with patch('edxmako.request_context.get_current_request', return_value=self.request):
+            # requestcontext should not be None.
+            self.assertIsNotNone(get_template_request_context())
+
+    def test_without_current_request(self):
         """
-        Test the global variable requestcontext is cleared correctly
-        when response middleware is called.
+        Test that if get_current_request returns None, then get_template_request_context
+        returns None.
         """
+        with patch('edxmako.request_context.get_current_request', return_value=None):
+            # requestcontext should be None.
+            self.assertIsNone(get_template_request_context())
+
+    def test_request_context_caching(self):
+        """
+        Test that the RequestContext is cached in the RequestCache.
+        """
+        with patch('edxmako.request_context.get_current_request', return_value=None):
+            # requestcontext should be None, because the cache isn't filled
+            self.assertIsNone(get_template_request_context())
+
+        with patch('edxmako.request_context.get_current_request', return_value=self.request):
+            # requestcontext should not be None, and should fill the cache
+            self.assertIsNotNone(get_template_request_context())
+
+        mock_get_current_request = Mock()
+        with patch('edxmako.request_context.get_current_request', mock_get_current_request):
+            # requestcontext should not be None, because the cache is filled
+            self.assertIsNotNone(get_template_request_context())
+        mock_get_current_request.assert_not_called()
 
-        self.middleware.process_request(self.request)
-        # requestcontext should not be None.
-        self.assertIsNotNone(get_template_request_context())
+        RequestCache.clear_request_cache()
 
-        self.middleware.process_response(self.request, self.response)
-        # requestcontext should be None.
-        self.assertIsNone(get_template_request_context())
+        with patch('edxmako.request_context.get_current_request', return_value=None):
+            # requestcontext should be None, because the cache isn't filled
+            self.assertIsNone(get_template_request_context())
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
-    @patch("edxmako.middleware.REQUEST_CONTEXT")
-    def test_render_to_string_when_no_global_context_lms(self, context_mock):
+    def test_render_to_string_when_no_global_context_lms(self):
         """
         Test render_to_string() when makomiddleware has not initialized
         the threadlocal REQUEST_CONTEXT.context. This is meant to run in LMS.
         """
-        del context_mock.context
         self.assertIn("this module is temporarily unavailable", render_to_string("courseware/error-message.html", None))
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'cms.urls', 'Test only valid in cms')
-    @patch("edxmako.middleware.REQUEST_CONTEXT")
-    def test_render_to_string_when_no_global_context_cms(self, context_mock):
+    def test_render_to_string_when_no_global_context_cms(self):
         """
         Test render_to_string() when makomiddleware has not initialized
         the threadlocal REQUEST_CONTEXT.context. This is meant to run in CMS.
         """
-        del context_mock.context
         self.assertIn("We're having trouble rendering your component", render_to_string("html_error.html", None))
 
 

common/djangoapps/external_auth/tests/test_ssl.py

@@ -5,6 +5,7 @@ of the external_auth app.
 import copy
 import unittest
 
+from contextlib import contextmanager
 from django.conf import settings
 from django.contrib.auth import SESSION_KEY
 from django.contrib.auth.models import AnonymousUser, User
@@ -13,10 +14,9 @@ from django.core.urlresolvers import reverse
 from django.test.client import Client
 from django.test.client import RequestFactory
 from django.test.utils import override_settings
-from edxmako.middleware import MakoMiddleware
 from external_auth.models import ExternalAuthMap
 import external_auth.views
-from mock import Mock
+from mock import Mock, patch
 
 from student.models import CourseEnrollment
 from student.roles import CourseStaffRole
@@ -48,6 +48,7 @@ class SSLClientTest(ModuleStoreTestCase):
     USER_EMAIL = 'test_user_ssl@EDX.ORG'
     MOCK_URL = '/'
 
+    @contextmanager
     def _create_ssl_request(self, url):
         """Creates a basic request for SSL use."""
         request = self.factory.get(url)
@@ -56,9 +57,11 @@ class SSLClientTest(ModuleStoreTestCase):
         middleware = SessionMiddleware()
         middleware.process_request(request)
         request.session.save()
-        MakoMiddleware().process_request(request)
-        return request
 
+        with patch('edxmako.request_context.get_current_request', return_value=request):
+            yield request
+
+    @contextmanager
     def _create_normal_request(self, url):
         """Creates sessioned request without SSL headers"""
         request = self.factory.get(url)
@@ -66,8 +69,9 @@ class SSLClientTest(ModuleStoreTestCase):
         middleware = SessionMiddleware()
         middleware.process_request(request)
         request.session.save()
-        MakoMiddleware().process_request(request)
-        return request
+
+        with patch('edxmako.request_context.get_current_request', return_value=request):
+            yield request
 
     def setUp(self):
         """Setup test case by adding primary user."""
@@ -82,8 +86,8 @@ class SSLClientTest(ModuleStoreTestCase):
         Validate that an SSL login creates an eamap user and
         redirects them to the signup page.
         """
-
-        response = external_auth.views.ssl_login(self._create_ssl_request('/'))
+        with self._create_ssl_request('/') as request:
+            response = external_auth.views.ssl_login(request)
 
         # Response should contain template for signup form, eamap should have user, and internal
         # auth should not have a user
@@ -122,8 +126,8 @@ class SSLClientTest(ModuleStoreTestCase):
         Test IMMEDIATE_SIGNUP feature flag and ensure the user account is automatically created
         and the user is redirected to slash.
         """
-
-        external_auth.views.ssl_login(self._create_ssl_request('/'))
+        with self._create_ssl_request('/') as request:
+            external_auth.views.ssl_login(request)
 
         # Assert our user exists in both eamap and Users, and that we are logged in
         try:
@@ -244,7 +248,9 @@ class SSLClientTest(ModuleStoreTestCase):
         and this should not fail.
         """
         # Create account, break internal password, and activate account
-        external_auth.views.ssl_login(self._create_ssl_request('/'))
+
+        with self._create_ssl_request('/') as request:
+            external_auth.views.ssl_login(request)
         user = User.objects.get(email=self.USER_EMAIL)
         user.set_password('not autogenerated')
         user.is_active = True
@@ -262,12 +268,13 @@ class SSLClientTest(ModuleStoreTestCase):
         """Make sure no external auth happens without SSL enabled"""
 
         dec_mock = external_auth.views.ssl_login_shortcut(self.mock)
-        request = self._create_normal_request(self.MOCK_URL)
-        request.user = AnonymousUser()
-        # Call decorated mock function to make sure it passes
-        # the call through without hitting the external_auth functions and
-        # thereby creating an external auth map object.
-        dec_mock(request)
+
+        with self._create_normal_request(self.MOCK_URL) as request:
+            request.user = AnonymousUser()
+            # Call decorated mock function to make sure it passes
+            # the call through without hitting the external_auth functions and
+            # thereby creating an external auth map object.
+            dec_mock(request)
         self.assertTrue(self.mock.called)
         self.assertEqual(0, len(ExternalAuthMap.objects.all()))
 
@@ -278,23 +285,23 @@ class SSLClientTest(ModuleStoreTestCase):
         dec_mock = external_auth.views.ssl_login_shortcut(self.mock)
 
         # Test that anonymous without cert doesn't create authmap
-        request = self._create_normal_request(self.MOCK_URL)
-        dec_mock(request)
+        with self._create_normal_request(self.MOCK_URL) as request:
+            dec_mock(request)
         self.assertTrue(self.mock.called)
         self.assertEqual(0, len(ExternalAuthMap.objects.all()))
 
         # Test valid user
         self.mock.reset_mock()
-        request = self._create_ssl_request(self.MOCK_URL)
-        dec_mock(request)
+        with self._create_ssl_request(self.MOCK_URL) as request:
+            dec_mock(request)
         self.assertFalse(self.mock.called)
         self.assertEqual(1, len(ExternalAuthMap.objects.all()))
 
         # Test logged in user gets called
         self.mock.reset_mock()
-        request = self._create_ssl_request(self.MOCK_URL)
-        request.user = UserFactory()
-        dec_mock(request)
+        with self._create_ssl_request(self.MOCK_URL) as request:
+            request.user = UserFactory()
+            dec_mock(request)
         self.assertTrue(self.mock.called)
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@@ -306,8 +313,9 @@ class SSLClientTest(ModuleStoreTestCase):
         """
 
         dec_mock = external_auth.views.ssl_login_shortcut(self.mock)
-        request = self._create_ssl_request(self.MOCK_URL)
-        dec_mock(request)
+        with self._create_ssl_request(self.MOCK_URL) as request:
+            dec_mock(request)
+
         # Assert our user exists in both eamap and Users
         try:
             ExternalAuthMap.objects.get(external_id=self.USER_EMAIL)
@@ -334,7 +342,8 @@ class SSLClientTest(ModuleStoreTestCase):
             display_name='Robot Super Course'
         )
 
-        external_auth.views.ssl_login(self._create_ssl_request('/'))
+        with self._create_ssl_request('/') as request:
+            external_auth.views.ssl_login(request)
         user = User.objects.get(email=self.USER_EMAIL)
         CourseEnrollment.enroll(user, course.id)
         course_private_url = '/courses/MITx/999/Robot_Super_Course/courseware'
@@ -364,7 +373,8 @@ class SSLClientTest(ModuleStoreTestCase):
             display_name='Robot Super Course'
         )
 
-        external_auth.views.ssl_login(self._create_ssl_request('/'))
+        with self._create_ssl_request('/') as request:
+            external_auth.views.ssl_login(request)
         user = User.objects.get(email=self.USER_EMAIL)
         CourseEnrollment.enroll(user, course.id)
 

common/djangoapps/request_cache/__init__.py

@@ -9,6 +9,7 @@ import logging
 from urlparse import urlparse
 
 from celery.signals import task_postrun
+import crum
 from django.conf import settings
 from django.test.client import RequestFactory
 
@@ -42,8 +43,10 @@ def get_cache(name):
 def get_request():
     """
     Return the current request.
+
+    Deprecated: Please use crum to retrieve current requests.
     """
-    return middleware.RequestCache.get_current_request()
+    return crum.get_current_request()
 
 
 def get_request_or_stub():
@@ -56,7 +59,7 @@ def get_request_or_stub():
     This is useful in cases where we need to pass in a request object
     but don't have an active request (for example, in test cases).
     """
-    request = get_request()
+    request = crum.get_current_request()
 
     if request is None:
         log.warning(

lms/djangoapps/ccx/tests/test_field_override_performance.py

@@ -14,7 +14,6 @@ from django.conf import settings
 from django.core.cache import caches
 from django.test.client import RequestFactory
 from django.test.utils import override_settings
-from edxmako.middleware import MakoMiddleware
 from nose.plugins.attrib import attr
 from pytz import UTC
 from request_cache.middleware import RequestCache
@@ -64,11 +63,13 @@ class FieldOverridePerformanceTestCase(ProceduralCourseTestMixin,
         self.student = UserFactory.create()
         self.request = self.request_factory.get("foo")
         self.request.user = self.student
+
+        patcher = mock.patch('edxmako.request_context.get_current_request', return_value=self.request)
+        patcher.start()
+        self.addCleanup(patcher.stop)
         self.course = None
         self.ccx = None
 
-        MakoMiddleware().process_request(self.request)
-
     def setup_course(self, size, enable_ccx, view_as_ccx):
         """
         Build a gradable course where each node has `size` children.

lms/envs/common.py

@@ -1151,8 +1151,6 @@ MIDDLEWARE_CLASSES = (
 
     # catches any uncaught RateLimitExceptions and returns a 403 instead of a 500
     'ratelimitbackend.middleware.RateLimitMiddleware',
-    # needs to run after locale middleware (or anything that modifies the request context)
-    'edxmako.middleware.MakoMiddleware',
 
     # for expiring inactive sessions
     'session_inactivity_timeout.middleware.SessionInactivityTimeout',