do the ORG filtering as part of the mongo query. Also, don't trigger metadata…

do the ORG filtering as part of the mongo query. Also, don't trigger metadata inheritence calculations when just trying to load 'about' information, which is not tied into the course tree anyhow (i.e. no parent/child relationship)

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

@@ -829,7 +829,9 @@ class ModuleStoreRead(ModuleStoreAssetBase):
     def get_courses(self, **kwargs):
         '''
         Returns a list containing the top level XModuleDescriptors of the courses
-        in this modulestore.
+        in this modulestore. This method can take an optional argument 'org' which
+        will efficiently apply a filter so that only the courses of the specified
+        ORG in the CourseKey will be fetched.
         '''
         pass
 

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

@@ -74,6 +74,8 @@ BLOCK_TYPES_WITH_CHILDREN = list(set(
 # at module level, cache one instance of OSFS per filesystem root.
 _OSFS_INSTANCE = {}
 
+_DETACHED_CATEGORIES = [name for name, __ in XBlock.load_tagged_classes("detached")]
+
 
 class MongoRevisionKey(object):
     """
@@ -933,17 +935,36 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
                 course_key,
                 item,
                 data_cache,
-                apply_cached_metadata=(item['location']['category'] != 'course' or depth != 0),
-                using_descriptor_system=using_descriptor_system
+                using_descriptor_system=using_descriptor_system,
+                apply_cached_metadata=self._should_apply_cached_metadata(item, depth)
             )
             for item in items
         ]
 
+    def _should_apply_cached_metadata(self, item, depth):
+        """
+        Returns a boolean whether a particular query should trigger an application
+        of inherited metadata onto the item
+        """
+        category = item['location']['category']
+        apply_cached_metadata = category not in _DETACHED_CATEGORIES and \
+            not (category == 'course' and depth == 0)
+        return apply_cached_metadata
+
     @autoretry_read()
     def get_courses(self, **kwargs):
         '''
-        Returns a list of course descriptors.
+        Returns a list of course descriptors. This accepts an optional parameter of 'org' which
+        will apply an efficient filter to only get courses with the specified ORG
         '''
+
+        course_org_filter = kwargs.get('org')
+
+        if course_org_filter:
+            course_records = self.collection.find({'_id.category': 'course', '_id.org': course_org_filter})
+        else:
+            course_records = self.collection.find({'_id.category': 'course'})
+
         base_list = sum(
             [
                 self._load_items(
@@ -953,7 +974,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
                 for course
                 # I tried to add '$and': [{'_id.org': {'$ne': 'edx'}}, {'_id.course': {'$ne': 'templates'}}]
                 # but it didn't do the right thing (it filtered all edx and all templates out)
-                in self.collection.find({'_id.category': 'course'})
+                in course_records
                 if not (  # TODO kill this
                     course['_id']['org'] == 'edx' and
                     course['_id']['course'] == 'templates'

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

@@ -191,7 +191,7 @@ class MongoConnection(object):
             }
         return self.course_index.find_one(query)
 
-    def find_matching_course_indexes(self, branch=None, search_targets=None):
+    def find_matching_course_indexes(self, branch=None, search_targets=None, org_target=None):
         """
         Find the course_index matching particular conditions.
 
@@ -199,6 +199,8 @@ class MongoConnection(object):
             branch: If specified, this branch must exist in the returned courses
             search_targets: If specified, this must be a dictionary specifying field values
                 that must exist in the search_targets of the returned courses
+            org_target: If specified, this is an ORG filter so that only course_indexs are
+                returned for the specified ORG
         """
         query = {}
         if branch is not None:
@@ -208,6 +210,9 @@ class MongoConnection(object):
             for key, value in search_targets.iteritems():
                 query['search_targets.{}'.format(key)] = value
 
+        if org_target:
+            query['org'] = org_target
+
         return self.course_index.find(query)
 
     def insert_course_index(self, course_index):

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

@@ -486,14 +486,16 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
             block_data.edit_info.original_usage = original_usage
             block_data.edit_info.original_usage_version = original_usage_version
 
-    def find_matching_course_indexes(self, branch=None, search_targets=None):
+    def find_matching_course_indexes(self, branch=None, search_targets=None, org_target=None):
         """
-        Find the course_indexes which have the specified branch and search_targets.
+        Find the course_indexes which have the specified branch and search_targets. An optional org_target
+        can be specified to apply an ORG filter to return only the courses that are part of
+        that ORG.
 
         Returns:
             a Cursor if there are no changes in flight or a list if some have changed in current bulk op
         """
-        indexes = self.db_connection.find_matching_course_indexes(branch, search_targets)
+        indexes = self.db_connection.find_matching_course_indexes(branch, search_targets, org_target)
 
         def _replace_or_append_index(altered_index):
             """
@@ -519,6 +521,13 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
                 ):
                     continue
 
+            # if we've specified a filter by org,
+            # make sure we've honored that filter when
+            # integrating in-transit records
+            if org_target:
+                if record.index['org'] != org_target:
+                    continue
+
             if not hasattr(indexes, 'append'):  # Just in time conversion to list from cursor
                 indexes = list(indexes)
 
@@ -830,11 +839,20 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         # add it in the envelope for the structure.
         return CourseEnvelope(course_key.replace(version_guid=version_guid), entry)
 
-    def _get_structures_for_branch(self, branch):
+    def _get_structures_for_branch(self, branch, **kwargs):
         """
         Internal generator for fetching lists of courses, libraries, etc.
         """
-        matching_indexes = self.find_matching_course_indexes(branch)
+
+        # if we pass in a 'org' parameter that means to
+        # only get the course which match the passed in
+        # ORG
+
+        matching_indexes = self.find_matching_course_indexes(
+            branch,
+            search_targets=None,
+            org_target=kwargs.get('org')
+        )
 
         # collect ids and then query for those
         version_guids = []
@@ -858,7 +876,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         :param type locator_factory: Factory to create locator from structure info and branch
         """
         result = []
-        for entry, structure_info in self._get_structures_for_branch(branch):
+        for entry, structure_info in self._get_structures_for_branch(branch, **kwargs):
             locator = locator_factory(structure_info, branch)
             envelope = CourseEnvelope(locator, entry)
             root = entry['root']

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

@@ -4,6 +4,7 @@ Unit tests for the Mongo modulestore
 # pylint: disable=no-member
 # pylint: disable=protected-access
 # pylint: disable=no-name-in-module
+# pylint: disable=bad-continuation
 from nose.tools import assert_equals, assert_raises, \
     assert_not_equals, assert_false, assert_true, assert_greater, assert_is_instance, assert_is_none
 # pylint: enable=E0611
@@ -145,6 +146,18 @@ class TestMongoModuleStoreBase(unittest.TestCase):
             verbose=True
         )
 
+        # also import a course under a different course_id (especially ORG)
+        import_course_from_xml(
+            draft_store,
+            999,
+            DATA_DIR,
+            ['test_import_course'],
+            static_content_store=content_store,
+            do_import_static=False,
+            verbose=True,
+            target_id=SlashSeparatedCourseKey('guestx', 'foo', 'bar')
+        )
+
         return content_store, draft_store
 
     @staticmethod
@@ -191,15 +204,29 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
     def test_get_courses(self):
         '''Make sure the course objects loaded properly'''
         courses = self.draft_store.get_courses()
-        assert_equals(len(courses), 6)
+
+        # note, the number of courses expected is really
+        # 6, but due to a lack of cache flushing between
+        # test case runs, we will get back 7.
+        # When we fix the caching issue, we should reduce this
+        # to 6 and remove the 'treexport_peer_component' course_id
+        # from the list below
+        assert_equals(len(courses), 7)  # pylint: disable=no-value-for-parameter
         course_ids = [course.id for course in courses]
+
         for course_key in [
 
             SlashSeparatedCourseKey(*fields)
             for fields in [
-                ['edX', 'simple', '2012_Fall'], ['edX', 'simple_with_draft', '2012_Fall'],
-                ['edX', 'test_import_course', '2012_Fall'], ['edX', 'test_unicode', '2012_Fall'],
-                ['edX', 'toy', '2012_Fall']
+                ['edX', 'simple', '2012_Fall'],
+                ['edX', 'simple_with_draft', '2012_Fall'],
+                ['edX', 'test_import_course', '2012_Fall'],
+                ['edX', 'test_unicode', '2012_Fall'],
+                ['edX', 'toy', '2012_Fall'],
+                ['guestx', 'foo', 'bar'],
+                # This course below is due to a caching issue in the modulestore
+                # which is not cleared between test runs. This means
+                ['edX', 'treeexport_peer_component', 'export_peer_component'],
             ]
         ]:
             assert_in(course_key, course_ids)
@@ -212,6 +239,48 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
             assert_false(self.draft_store.has_course(mix_cased))
             assert_true(self.draft_store.has_course(mix_cased, ignore_case=True))
 
+    def test_get_org_courses(self):
+        """
+        Make sure that we can query for a filtered list of courses for a given ORG
+        """
+
+        courses = self.draft_store.get_courses(org='guestx')
+        assert_equals(len(courses), 1)  # pylint: disable=no-value-for-parameter
+        course_ids = [course.id for course in courses]
+
+        for course_key in [
+            SlashSeparatedCourseKey(*fields)
+            for fields in [
+                ['guestx', 'foo', 'bar']
+            ]
+        ]:
+            assert_in(course_key, course_ids)  # pylint: disable=no-value-for-parameter
+
+        courses = self.draft_store.get_courses(org='edX')
+        # note, the number of courses expected is really
+        # 5, but due to a lack of cache flushing between
+        # test case runs, we will get back 6.
+        # When we fix the caching issue, we should reduce this
+        # to 6 and remove the 'treexport_peer_component' course_id
+        # from the list below
+        assert_equals(len(courses), 6)  # pylint: disable=no-value-for-parameter
+        course_ids = [course.id for course in courses]
+
+        for course_key in [
+            SlashSeparatedCourseKey(*fields)
+            for fields in [
+                ['edX', 'simple', '2012_Fall'],
+                ['edX', 'simple_with_draft', '2012_Fall'],
+                ['edX', 'test_import_course', '2012_Fall'],
+                ['edX', 'test_unicode', '2012_Fall'],
+                ['edX', 'toy', '2012_Fall'],
+                # This course below is due to a caching issue in the modulestore
+                # which is not cleared between test runs. This means
+                ['edX', 'treeexport_peer_component', 'export_peer_component'],
+            ]
+        ]:
+            assert_in(course_key, course_ids)  # pylint: disable=no-value-for-parameter
+
     def test_no_such_course(self):
         """
         Test get_course and has_course with ids which don't exist

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

@@ -617,6 +617,23 @@ class SplitModuleCourseTests(SplitModuleTest):
         self.assertEqual(course.edited_by, "testassist@edx.org")
         self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.45})
 
+    def test_get_org_courses(self):
+        courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='guestx')
+
+        # should have gotten 1 draft courses
+        self.assertEqual(len(courses), 1)
+
+        courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='testx')
+
+        # should have gotten 2 draft courses
+        self.assertEqual(len(courses), 2)
+
+        # although this is already covered in other tests, let's
+        # also not pass in org= parameter to make sure we get back
+        # 3 courses
+        courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT)
+        self.assertEqual(len(courses), 3)
+
     def test_branch_requests(self):
         # query w/ branch qualifier (both draft and published)
         def _verify_published_course(courses_published):
@@ -1232,6 +1249,37 @@ class TestItemCrud(SplitModuleTest):
             self.assertEqual(refetch_course.previous_version, course_block_update_version)
             self.assertEqual(refetch_course.update_version, transaction_guid)
 
+    def test_bulk_ops_org_filtering(self):
+        """
+        Make sure of proper filtering when using bulk operations and
+        calling get_courses with an 'org' filter
+        """
+
+        # start transaction w/ simple creation
+        user = random.getrandbits(32)
+        course_key = CourseLocator('test_org', 'test_transaction', 'test_run')
+        with modulestore().bulk_operations(course_key):
+            modulestore().create_course('test_org', 'test_transaction', 'test_run', user, BRANCH_NAME_DRAFT)
+
+            courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='test_org')
+            self.assertEqual(len(courses), 1)
+            self.assertEqual(courses[0].id.org, course_key.org)
+            self.assertEqual(courses[0].id.course, course_key.course)
+            self.assertEqual(courses[0].id.run, course_key.run)
+
+            courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='other_org')
+            self.assertEqual(len(courses), 0)
+
+        # re-assert after the end of the with scope
+        courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='test_org')
+        self.assertEqual(len(courses), 1)
+        self.assertEqual(courses[0].id.org, course_key.org)
+        self.assertEqual(courses[0].id.course, course_key.course)
+        self.assertEqual(courses[0].id.run, course_key.run)
+
+        courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='other_org')
+        self.assertEqual(len(courses), 0)
+
     def test_update_metadata(self):
         """
         test updating an items metadata ensuring the definition doesn't version but the course does if it should

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

@@ -274,9 +274,10 @@ class TestBulkWriteMixinFindMethods(TestBulkWriteMixin):
     def test_no_bulk_find_matching_course_indexes(self):
         branch = Mock(name='branch')
         search_targets = MagicMock(name='search_targets')
+        org_targets = None
         self.conn.find_matching_course_indexes.return_value = [Mock(name='result')]
         result = self.bulk.find_matching_course_indexes(branch, search_targets)
-        self.assertConnCalls(call.find_matching_course_indexes(branch, search_targets))
+        self.assertConnCalls(call.find_matching_course_indexes(branch, search_targets, org_targets))
         self.assertEqual(result, self.conn.find_matching_course_indexes.return_value)
         self.assertCacheNotCleared()
 

lms/djangoapps/branding/__init__.py

@@ -10,7 +10,10 @@ def get_visible_courses():
     """
     Return the set of CourseDescriptors that should be visible in this branded instance
     """
-    _courses = modulestore().get_courses()
+
+    filtered_by_org = microsite.get_value('course_org_filter')
+
+    _courses = modulestore().get_courses(org=filtered_by_org)
 
     courses = [c for c in _courses
                if isinstance(c, CourseDescriptor)]
@@ -25,8 +28,6 @@ def get_visible_courses():
     if hasattr(settings, 'COURSE_LISTINGS') and subdomain in settings.COURSE_LISTINGS and not settings.DEBUG:
         filtered_visible_ids = frozenset([SlashSeparatedCourseKey.from_deprecated_string(c) for c in settings.COURSE_LISTINGS[subdomain]])
 
-    filtered_by_org = microsite.get_value('course_org_filter')
-
     if filtered_by_org:
         return [course for course in courses if course.location.org == filtered_by_org]
     if filtered_visible_ids: