From 3afc125ecb3ef1b061f09640de3ad0ac72e336fd Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Thu, 28 Aug 2014 14:31:46 -0400 Subject: [PATCH] Add test to verify bad query behavior of xblock_handler --- .../contentstore/tests/test_import.py | 6 ++--- cms/djangoapps/contentstore/tests/utils.py | 18 +++++++++----- cms/djangoapps/contentstore/views/item.py | 2 +- .../contentstore/views/tests/test_item.py | 24 +++++++++++++++++-- .../xmodule/modulestore/tests/django_utils.py | 3 +++ .../xmodule/modulestore/tests/factories.py | 4 ++-- 6 files changed, 43 insertions(+), 14 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index fe6b1bbbb2..c6d0a12f04 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/cms/djangoapps/contentstore/tests/test_import.py @@ -157,15 +157,15 @@ class ContentStoreImportTest(ModuleStoreTestCase): store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) # we try to refresh the inheritance tree for each update_item in the import - with check_exact_number_of_calls(store, store.refresh_cached_metadata_inheritance_tree, 28): + with check_exact_number_of_calls(store, "refresh_cached_metadata_inheritance_tree", 28): # _get_cached_metadata_inheritance_tree should be called only once - with check_exact_number_of_calls(store, store._get_cached_metadata_inheritance_tree, 1): + with check_exact_number_of_calls(store, "_get_cached_metadata_inheritance_tree", 1): # with bulk-edit in progress, the inheritance tree should be recomputed only at the end of the import # NOTE: On Jenkins, with memcache enabled, the number of calls here is only 1. # Locally, without memcache, the number of calls is actually 2 (once more during the publish step) - with check_number_of_calls(store, store._compute_metadata_inheritance_tree, 2): + with check_number_of_calls(store, "_compute_metadata_inheritance_tree", 2): self.load_test_import_course() def test_rewrite_reference_list(self): diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index fca7d8b2a2..359bc6fa96 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -98,17 +98,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']) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 2959ba1391..381f016de6 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -756,7 +756,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 diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index c613ab59d6..bffbfda7bd 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -26,7 +26,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 +83,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 +101,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') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 15bf4282ff..3f1afda31c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/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() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index e59b30aec8..b394df3ac5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -217,12 +217,12 @@ class ItemFactory(XModuleFactory): @contextmanager -def check_exact_number_of_calls(object_with_method, method, num_calls, method_name=None): +def check_exact_number_of_calls(object_with_method, method_name, num_calls): """ Instruments the given method on the given object to verify the number of calls to the method is exactly equal to 'num_calls'. """ - with check_number_of_calls(object_with_method, method, num_calls, num_calls, method_name): + with check_number_of_calls(object_with_method, method_name, num_calls, num_calls): yield