Make RequestCache reusable

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -7,13 +7,11 @@ import logging
 import re
 import json
 import datetime
-from uuid import uuid4
 
 from pytz import UTC
-from collections import namedtuple, defaultdict
+from collections import defaultdict
 import collections
 from contextlib import contextmanager
-import functools
 import threading
 from operator import itemgetter
 from sortedcontainers import SortedListWithKey
@@ -27,8 +25,6 @@ from xmodule.errortracker import make_error_tracker
 from xmodule.assetstore import AssetMetadata
 from opaque_keys.edx.keys import CourseKey, UsageKey, AssetKey
 from opaque_keys.edx.locations import Location  # For import backwards compatibility
-from opaque_keys import InvalidKeyError
-from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xblock.runtime import Mixologist
 from xblock.core import XBlock
 
@@ -1195,41 +1191,6 @@ class ModuleStoreReadBase(BulkOperationsMixin, ModuleStoreRead):
             raise ValueError(u"Cannot set default store to type {}".format(store_type))
         yield
 
-    @staticmethod
-    def memoize_request_cache(func):
-        """
-        Memoize a function call results on the request_cache if there's one. Creates the cache key by
-        joining the unicode of all the args with &; so, if your arg may use the default &, it may
-        have false hits
-        """
-        @functools.wraps(func)
-        def wrapper(self, *args, **kwargs):
-            """
-            Wraps a method to memoize results.
-            """
-            if self.request_cache:
-                cache_key = '&'.join([hashvalue(arg) for arg in args])
-                if cache_key in self.request_cache.data.setdefault(func.__name__, {}):
-                    return self.request_cache.data[func.__name__][cache_key]
-
-                result = func(self, *args, **kwargs)
-
-                self.request_cache.data[func.__name__][cache_key] = result
-                return result
-            else:
-                return func(self, *args, **kwargs)
-        return wrapper
-
-
-def hashvalue(arg):
-    """
-    If arg is an xblock, use its location. otherwise just turn it into a string
-    """
-    if isinstance(arg, XBlock):
-        return unicode(arg.location)
-    else:
-        return unicode(arg)
-
 
 # pylint: disable=abstract-method
 class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -915,6 +915,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
                 services["user"] = self.user_service
             services["settings"] = SettingsService()
 
+            if self.request_cache:
+                services["request_cache"] = self.request_cache
+
             system = CachingDescriptorSystem(
                 modulestore=self,
                 course_key=course_key,

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -10,6 +10,7 @@ import pymongo
 import logging
 
 from opaque_keys.edx.locations import Location
+from openedx.core.lib.cache_utils import memoize_in_request_cache
 from xmodule.exceptions import InvalidVersionError
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.exceptions import (
@@ -634,7 +635,7 @@ class DraftModuleStore(MongoModuleStore):
             bulk_record.dirty = True
             self.collection.remove({'_id': {'$in': to_be_deleted}}, safe=self.collection.safe)
 
-    @MongoModuleStore.memoize_request_cache
+    @memoize_in_request_cache('request_cache')
     def has_changes(self, xblock):
         """
         Check if the subtree rooted at xblock has any drafts and thus may possibly have changes

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -671,6 +671,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         if user_service is not None:
             self.services["user"] = user_service
 
+        if self.request_cache is not None:
+            self.services["request_cache"] = self.request_cache
+
         self.signal_handler = signal_handler
 
     def close_connections(self):

openedx/core/lib/cache_utils.py

+"""
+Utilities related to caching.
+"""
+
+import functools
+from xblock.core import XBlock
+
+
+def memoize_in_request_cache(request_cache_attr_name=None):
+    """
+    Memoize a method call's results in the request_cache if there's one. Creates the cache key by
+    joining the unicode of all the args with &; so, if your arg may use the default &, it may
+    have false hits.
+
+    Arguments:
+        request_cache_attr_name - The name of the field or property in this method's containing
+         class that stores the request_cache.
+    """
+    def _decorator(func):
+        """Outer method decorator."""
+        @functools.wraps(func)
+        def _wrapper(self, *args, **kwargs):
+            """
+            Wraps a method to memoize results.
+            """
+            request_cache = getattr(self, request_cache_attr_name, None)
+            if request_cache:
+                cache_key = '&'.join([hashvalue(arg) for arg in args])
+                if cache_key in request_cache.data.setdefault(func.__name__, {}):
+                    return request_cache.data[func.__name__][cache_key]
+
+                result = func(self, *args, **kwargs)
+
+                request_cache.data[func.__name__][cache_key] = result
+                return result
+            else:
+                return func(self, *args, **kwargs)
+        return _wrapper
+    return _decorator
+
+
+def hashvalue(arg):
+    """
+    If arg is an xblock, use its location. otherwise just turn it into a string
+    """
+    if isinstance(arg, XBlock):
+        return unicode(arg.location)
+    else:
+        return unicode(arg)

openedx/core/lib/tests/test_cache_utils.py

+"""
+Tests for cache_utils.py
+"""
+import ddt
+from mock import MagicMock
+from unittest import TestCase
+
+from openedx.core.lib.cache_utils import memoize_in_request_cache
+
+
+@ddt.ddt
+class TestMemoizeInRequestCache(TestCase):
+    """
+    Test the memoize_in_request_cache helper function.
+    """
+    class TestCache(object):
+        """
+        A test cache that provides a data dict for caching values, analogous to the request_cache.
+        """
+        def __init__(self):
+            self.data = {}
+
+    def setUp(self):
+        super(TestMemoizeInRequestCache, self).setUp()
+        self.request_cache = self.TestCache()
+
+    @memoize_in_request_cache('request_cache')
+    def func_to_memoize(self, param):
+        """
+        A test function whose results are to be memoized in the request_cache.
+        """
+        return self.func_to_count(param)
+
+    @memoize_in_request_cache('request_cache')
+    def multi_param_func_to_memoize(self, param1, param2):
+        """
+        A test function with multiple parameters whose results are to be memoized in the request_cache.
+        """
+        return self.func_to_count(param1, param2)
+
+    def test_memoize_in_request_cache(self):
+        """
+        Tests the memoize_in_request_cache decorator for both single-param and multiple-param functions.
+        """
+        funcs_to_test = (
+            (self.func_to_memoize, ['foo'], ['bar']),
+            (self.multi_param_func_to_memoize, ['foo', 'foo2'], ['foo', 'foo3']),
+        )
+
+        for func_to_memoize, arg_list1, arg_list2 in funcs_to_test:
+            self.func_to_count = MagicMock()  # pylint: disable=attribute-defined-outside-init
+            self.assertFalse(self.func_to_count.called)
+
+            func_to_memoize(*arg_list1)
+            self.func_to_count.assert_called_once_with(*arg_list1)
+
+            func_to_memoize(*arg_list1)
+            self.func_to_count.assert_called_once_with(*arg_list1)
+
+            for _ in range(10):
+                func_to_memoize(*arg_list1)
+                func_to_memoize(*arg_list2)
+
+            self.assertEquals(self.func_to_count.call_count, 2)