Merge pull request #5031 from edx/andya/merge-hotfix-2014-08-29

Merge hotfix-2014-08-29 to master

cms/djangoapps/contentstore/tests/utils.py

@@ -95,17 +95,23 @@ class CourseTestCase(ModuleStoreTestCase):
             nonstaff.is_authenticated = True
         return client, nonstaff
 
-    def populate_course(self):
+    def populate_course(self, branching=2):
         """
-        Add 2 chapters, 4 sections, 8 verticals, 16 problems to self.course (branching 2)
+        Add k chapters, k^2 sections, k^3 verticals, k^4 problems to self.course (where k = branching)
         """
         user_id = self.user.id
+        self.populated_usage_keys = {}
+
         def descend(parent, stack):
-            xblock_type = stack.pop(0)
-            for _ in range(2):
+            if not stack:
+                return
+
+            xblock_type = stack[0]
+            for _ in range(branching):
                 child = ItemFactory.create(category=xblock_type, parent_location=parent.location, user_id=user_id)
-                if stack:
-                    descend(child, stack)
+                print child.location
+                self.populated_usage_keys.setdefault(xblock_type, []).append(child.location)
+                descend(child, stack[1:])
 
         descend(self.course, ['chapter', 'sequential', 'vertical', 'problem'])
 

cms/djangoapps/contentstore/views/component.py

@@ -27,6 +27,7 @@ from opaque_keys.edx.keys import UsageKey
 
 from .access import has_course_access
 from django.utils.translation import ugettext as _
+from models.settings.course_grading import CourseGradingModel
 
 __all__ = ['OPEN_ENDED_COMPONENT_TYPES',
            'ADVANCED_COMPONENT_POLICY_KEY',

cms/djangoapps/contentstore/views/item.py

@@ -570,7 +570,7 @@ def _get_xblock(usage_key, user):
     """
     store = modulestore()
     try:
-        return store.get_item(usage_key)
+        return store.get_item(usage_key, depth=None)
     except ItemNotFoundError:
         if usage_key.category in CREATE_IF_NOT_FOUND:
             # Create a new one for certain categories only. Used for course info handouts.
@@ -595,6 +595,9 @@ def _get_module_info(xblock, rewrite_static_links=True):
             course_id=xblock.location.course_key
         )
 
+    # Pre-cache has changes for the entire course because we'll need it for the ancestor info
+    modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None))
+
     # Note that children aren't being returned until we have a use case.
     return create_xblock_info(xblock, data=data, metadata=own_metadata(xblock), include_ancestor_info=True)
 
@@ -755,7 +758,7 @@ def _compute_visibility_state(xblock, child_info, is_unit_with_changes):
         return VisibilityState.needs_attention
     is_unscheduled = xblock.start == DEFAULT_START_DATE
     is_live = datetime.now(UTC) > xblock.start
-    children = child_info and child_info['children']
+    children = child_info and child_info.get('children', [])
     if children and len(children) > 0:
         all_staff_only = True
         all_unscheduled = True

cms/djangoapps/contentstore/views/tests/test_item.py

 """Tests for items views."""
-import os
 import json
 from datetime import datetime, timedelta
 import ddt
-from unittest import skipUnless
 
 from mock import patch
 from pytz import UTC
@@ -26,7 +24,7 @@ from student.tests.factories import UserFactory
 from xmodule.capa_module import CapaDescriptor
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.tests.factories import ItemFactory
+from xmodule.modulestore.tests.factories import ItemFactory, check_mongo_calls
 from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW
 from xblock.exceptions import NoSuchHandlerError
 from opaque_keys.edx.keys import UsageKey, CourseKey
@@ -83,7 +81,8 @@ class ItemTest(CourseTestCase):
         return self.response_usage_key(resp)
 
 
-class GetItem(ItemTest):
+@ddt.ddt
+class GetItemTest(ItemTest):
     """Tests for '/xblock' GET url."""
 
     def _get_container_preview(self, usage_key):
@@ -100,6 +99,25 @@ class GetItem(ItemTest):
         self.assertIsNotNone(resources)
         return html, resources
 
+    @ddt.data(
+        (1, 21, 23, 35, 37),
+        (2, 22, 24, 38, 39),
+        (3, 23, 25, 41, 41),
+    )
+    @ddt.unpack
+    def test_get_query_count(self, branching_factor, chapter_queries, section_queries, unit_queries, problem_queries):
+        self.populate_course(branching_factor)
+        # Retrieve it
+        with check_mongo_calls(chapter_queries):
+            self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['chapter'][-1]))
+        with check_mongo_calls(section_queries):
+            self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['sequential'][-1]))
+        with check_mongo_calls(unit_queries):
+            self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['vertical'][-1]))
+        with check_mongo_calls(problem_queries):
+            self.client.get(reverse_usage_url('xblock_handler', self.populated_usage_keys['problem'][-1]))
+
+
     def test_get_vertical(self):
         # Add a vertical
         resp = self.create_xblock(category='vertical')

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

@@ -24,6 +24,7 @@ from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xblock.runtime import Mixologist
 from xblock.core import XBlock
+import functools
 
 log = logging.getLogger('edx.modulestore')
 
@@ -577,6 +578,36 @@ class ModuleStoreReadBase(ModuleStoreRead):
         """
         pass
 
+    @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):
+            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)
 
 class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
     '''

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

@@ -589,6 +589,7 @@ class DraftModuleStore(MongoModuleStore):
         _internal([root_usage.to_deprecated_son() for root_usage in root_usages])
         self.collection.remove({'_id': {'$in': to_be_deleted}}, safe=self.collection.safe)
 
+    @MongoModuleStore.memoize_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/tests/django_utils.py

@@ -10,6 +10,7 @@ from xmodule.modulestore.django import modulestore, clear_existing_modulestores
 from xmodule.modulestore import ModuleStoreEnum
 import datetime
 import pytz
+from request_cache.middleware import RequestCache
 from xmodule.tabs import CoursewareTab, CourseInfoTab, StaticTab, DiscussionTab, ProgressTab, WikiTab
 from xmodule.modulestore.tests.sample_courses import default_block_info_tree, TOY_BLOCK_INFO_TREE
 from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST
@@ -275,6 +276,8 @@ class ModuleStoreTestCase(TestCase):
         # the next time they are accessed.
         # We do this at *both* setup and teardown just to be safe.
         clear_existing_modulestores()
+        # clear RequestCache to emulate its clearance after each http request.
+        RequestCache().clear_request_cache()
 
         # Call superclass implementation
         super(ModuleStoreTestCase, self)._post_teardown()

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -354,8 +354,9 @@ class TestMixedModuleStore(unittest.TestCase):
             )
 
     # draft: 2 to look in draft and then published and then 5 for updating ancestors.
-    # split: 1 for the course index, 1 for the course structure before the change, 1 for the structure after the change
-    #  2 sends: insert structure, update index_entry
+    # split: 3 to get the course structure & the course definition (show_calculator is scope content)
+    #  before the change. 1 during change to refetch the definition. 3 afterward (b/c it calls get_item to return the "new" object).
+    #  2 sends to update index & structure (calculator is a setting field)
     @ddt.data(('draft', 11, 5), ('split', 3, 2))
     @ddt.unpack
     def test_update_item(self, default_ms, max_find, max_send):
@@ -438,7 +439,6 @@ class TestMixedModuleStore(unittest.TestCase):
         self.assertFalse(self.store.has_changes(component))
 
     # TODO: LMS-11220: Document why split find count is 4
-    # TODO: LMS-11220: Document why draft find count is 8
     # TODO: LMS-11220: Document why split send count is 3
     @ddt.data(('draft', 8, 2), ('split', 4, 3))
     @ddt.unpack
@@ -459,7 +459,7 @@ class TestMixedModuleStore(unittest.TestCase):
         with self.assertRaises(ItemNotFoundError):
             self.store.get_item(self.writable_chapter_location)
 
-    # TODO: LMS-11220: Document why draft find count is 9
+    # TODO: LMS-11220: Document why split find count is 4
     # TODO: LMS-11220: Document why split send count is 3
     @ddt.data(('draft', 9, 2), ('split', 4, 3))
     @ddt.unpack
@@ -506,8 +506,7 @@ class TestMixedModuleStore(unittest.TestCase):
         self.assertFalse(self.store.has_item(leaf_loc))
         self.assertNotIn(vert_loc, course.children)
 
-    # TODO: LMS-11220: Document why split send count is 2
-    # TODO: LMS-11220: Document why draft find count is 5
+    # TODO: LMS-11220: Document why split find count is 2
     @ddt.data(('draft', 5, 1), ('split', 2, 2))
     @ddt.unpack
     def test_delete_draft_vertical(self, default_ms, max_find, max_send):
@@ -712,9 +711,9 @@ class TestMixedModuleStore(unittest.TestCase):
     #        - load vertical
     #        - load inheritance data
 
-    # TODO: LMS-11220: Document why draft send count is 6
-    # TODO: LMS-11220: Document why draft find count is 18
-    # TODO: LMS-11220: Document why split find count is 16
+    # TODO: LMS-11220: Document why draft send count is 5
+    # TODO: LMS-11220: Document why draft find count is [19, 6]
+    # TODO: LMS-11220: Document why split find count is [2, 2]
     @ddt.data(('draft', [19, 6], 0), ('split', [2, 2], 0))
     @ddt.unpack
     def test_path_to_location(self, default_ms, num_finds, num_sends):
@@ -1045,6 +1044,7 @@ class TestMixedModuleStore(unittest.TestCase):
     # Sends:
     #    - insert structure
     #    - write index entry
+    # TODO: LMS-11220: Document why split find count is 3
     @ddt.data(('draft', 3, 6), ('split', 3, 2))
     @ddt.unpack
     def test_unpublish(self, default_ms, max_find, max_send):

lms/djangoapps/courseware/tests/test_module_render.py

@@ -326,15 +326,16 @@ class TestTOC(ModuleStoreTestCase):
         self.request = factory.get(chapter_url)
         self.request.user = UserFactory()
         self.modulestore = self.store._get_modulestore_for_courseid(self.course_key)
-        self.toy_course = self.store.get_course(self.toy_loc, depth=2)
         with check_mongo_calls(num_finds, num_sends):
+            self.toy_course = self.store.get_course(self.toy_loc, depth=2)
             self.field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
                 self.toy_loc, self.request.user, self.toy_course, depth=2
             )
 
 
-    # TODO: LMS-11220: Document why split find count is 21
-    @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 5, 0))
+    # TODO: LMS-11220: Document why split find count is 9
+    # TODO: LMS-11220: Document why mongo find count is 4
+    @ddt.data((ModuleStoreEnum.Type.mongo, 4, 0), (ModuleStoreEnum.Type.split, 9, 0))
     @ddt.unpack
     def test_toc_toy_from_chapter(self, default_ms, num_finds, num_sends):
         with self.store.default_store(default_ms):
@@ -361,8 +362,9 @@ class TestTOC(ModuleStoreTestCase):
         for toc_section in expected:
             self.assertIn(toc_section, actual)
 
-    # TODO: LMS-11220: Document why split find count is 21
-    @ddt.data((ModuleStoreEnum.Type.mongo, 1, 0), (ModuleStoreEnum.Type.split, 5, 0))
+    # TODO: LMS-11220: Document why split find count is 9
+    # TODO: LMS-11220: Document why mongo find count is 4
+    @ddt.data((ModuleStoreEnum.Type.mongo, 4, 0), (ModuleStoreEnum.Type.split, 9, 0))
     @ddt.unpack
     def test_toc_toy_from_section(self, default_ms, num_finds, num_sends):
         with self.store.default_store(default_ms):