Reduce sql queries for groupname tests.

cms/djangoapps/auth/authz.py

@@ -63,7 +63,7 @@ def get_all_course_role_groupnames(location, role, use_filter=True):
     # filter to the ones which exist
     default = groupnames[0]
     if use_filter:
-        groupnames = [group for group in groupnames if Group.objects.filter(name=group).exists()]
+        groupnames = [group.name for group in Group.objects.filter(name__in=groupnames)]
     return groupnames, default
 
 
@@ -203,12 +203,9 @@ def remove_user_from_course_group(caller, user, location, role):
 
     # see if the user is actually in that role, if not then we don't have to do anything
     groupnames, _ = get_all_course_role_groupnames(location, role)
-    for groupname in groupnames:
-        groups = user.groups.filter(name=groupname)
-        if groups:
-            # will only be one with that name
-            user.groups.remove(groups[0])
-            user.save()
+    for group in user.groups.filter(name__in=groupnames):
+        user.groups.remove(group)
+    user.save()
 
 
 def remove_user_from_creator_group(caller, user):
@@ -243,7 +240,7 @@ def is_user_in_course_group_role(user, location, role, check_staff=True):
         if check_staff and user.is_staff:
             return True
         groupnames, _ = get_all_course_role_groupnames(location, role)
-        return any(user.groups.filter(name=groupname).exists() for groupname in groupnames)
+        return user.groups.filter(name__in=groupnames).exists()
 
     return False
 
@@ -266,7 +263,7 @@ def is_user_in_creator_group(user):
 
     # Feature flag for using the creator group setting. Will be removed once the feature is complete.
     if settings.MITX_FEATURES.get('ENABLE_CREATOR_GROUP', False):
-        return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).count() > 0
+        return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).exists()
 
     return True
 

cms/djangoapps/contentstore/views/course.py

@@ -116,7 +116,8 @@ def course_listing(request):
     """
     List all courses available to the logged in user
     """
-    courses = modulestore('direct').get_items(['i4x', None, None, 'course', None])
+    # there's an index on category which will be used if none of its antecedents are set
+    courses = modulestore('direct').get_items([None, None, None, 'course', None])
 
     # filter out courses that we don't have access too
     def course_filter(course):

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -4,10 +4,10 @@
 from random import randint
 import re
 import pymongo
+import bson.son
 
 from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, DuplicateItemError
 from xmodule.modulestore.locator import BlockUsageLocator
-from xmodule.modulestore.mongo import draft
 from xmodule.modulestore import Location
 import urllib
 
@@ -91,9 +91,10 @@ def create_map_entry(self, course_location, course_id=None, draft_branch='draft'
             else:
                 course_id = "{0.org}.{0.course}".format(course_location)
         # very like _interpret_location_id but w/o the _id
-        location_id = {'org': course_location.org, 'course': course_location.course}
-        if course_location.category == 'course':
-            location_id['name'] = course_location.name
+        location_id = self._construct_location_son(
+            course_location.org, course_location.course, 
+            course_location.name if course_location.category == 'course' else None
+        )
 
         self.location_map.insert({
             '_id': location_id,
@@ -128,20 +129,25 @@ def translate_location(self, old_style_course_id, location, published=True, add_
         """
         location_id = self._interpret_location_course_id(old_style_course_id, location)
 
-        maps = self.location_map.find(location_id).sort('_id.name', pymongo.ASCENDING)
-        if maps.count() == 0:
+        maps = self.location_map.find(location_id)
+        maps = list(maps)
+        if len(maps) == 0:
             if add_entry_if_missing:
                 # create a new map
-                course_location = location.replace(category='course', name=location_id['_id.name'])
+                course_location = location.replace(category='course', name=location_id['_id']['name'])
                 self.create_map_entry(course_location)
                 entry = self.location_map.find_one(location_id)
             else:
                 raise ItemNotFoundError()
-        elif maps.count() > 1:
-            # if more than one, prefer the one w/o a name if that exists. Otherwise, choose the first (alphabetically)
+        elif len(maps) == 1:
             entry = maps[0]
         else:
+            # find entry w/o name, if any; otherwise, pick arbitrary
             entry = maps[0]
+            for item in maps:
+                if 'name' not in item['_id']:
+                    entry = item
+                    break
 
         if published:
             branch = entry['prod_branch']
@@ -242,11 +248,11 @@ def add_block_location_translator(self, location, old_course_id=None, usage_id=N
         location_id = self._interpret_location_course_id(old_course_id, location)
 
         maps = self.location_map.find(location_id)
-        if maps.count() == 0:
-            raise ItemNotFoundError()
-
         # turn maps from cursor to list
         map_list = list(maps)
+        if len(map_list) == 0:
+            raise ItemNotFoundError()
+
         encoded_location_name = self._encode_for_mongo(location.name)
         # check whether there's already a usage_id for this location (and it agrees w/ any passed in or found)
         for map_entry in map_list:
@@ -279,7 +285,10 @@ def add_block_location_translator(self, location, old_course_id=None, usage_id=N
                         )
 
                 map_entry['block_map'].setdefault(encoded_location_name, {})[location.category] = computed_usage_id
-                self.location_map.update({'_id': map_entry['_id']}, {'$set': {'block_map': map_entry['block_map']}})
+                self.location_map.update(
+                    {'_id': self._construct_location_son(**map_entry['_id'])}, 
+                    {'$set': {'block_map': map_entry['block_map']}}
+                )
 
         return computed_usage_id
 
@@ -317,7 +326,10 @@ def update_block_location_translator(self, location, usage_id, old_course_id=Non
 
             if location.category in map_entry['block_map'].setdefault(encoded_location_name, {}):
                 map_entry['block_map'][encoded_location_name][location.category] = usage_id
-                self.location_map.update({'_id': map_entry['_id']}, {'$set': {'block_map': map_entry['block_map']}})
+                self.location_map.update(
+                    {'_id': self._construct_location_son(**map_entry['_id'])}, 
+                    {'$set': {'block_map': map_entry['block_map']}}
+                )
 
         return usage_id
 
@@ -338,7 +350,10 @@ def delete_block_location_translator(self, location, old_course_id=None):
                     del map_entry['block_map'][encoded_location_name]
                 else:
                     del map_entry['block_map'][encoded_location_name][location.category]
-                self.location_map.update({'_id': map_entry['_id']}, {'$set': {'block_map': map_entry['block_map']}})
+                self.location_map.update(
+                    {'_id': self._construct_location_son(**map_entry['_id'])},
+                    {'$set': {'block_map': map_entry['block_map']}}
+                )
 
     def _add_to_block_map(self, location, location_id, block_map):
         '''add the given location to the block_map and persist it'''
@@ -357,7 +372,7 @@ def _add_to_block_map(self, location, location_id, block_map):
 
     def _interpret_location_course_id(self, course_id, location):
         """
-        Take the old style course id (org/course/run) and return a dict for querying the mapping table.
+        Take the old style course id (org/course/run) and return a dict w/ a SON for querying the mapping table.
         If the course_id is empty, it uses location, but this may result in an inadequate id.
 
         :param course_id: old style 'org/course/run' id from Location.course_id where Location.category = 'course'
@@ -367,12 +382,21 @@ def _interpret_location_course_id(self, course_id, location):
         if course_id:
             # re doesn't allow ?P<_id.org> and ilk
             matched = re.match(r'([^/]+)/([^/]+)/([^/]+)', course_id)
-            return dict(zip(['_id.org', '_id.course', '_id.name'], matched.groups()))
+            return {'_id': self._construct_location_son(*matched.groups())}
 
-        location_id = {'_id.org': location.org, '_id.course': location.course}
         if location.category == 'course':
-            location_id['_id.name'] = location.name
-        return location_id
+            return {'_id': self._construct_location_son(location.org, location.course, location.name)}
+        else:
+            return bson.son.SON([('_id.org', location.org), ('_id.course', location.course)])
+
+    def _construct_location_son(self, org, course, name=None):
+        """
+        Construct the SON needed to repr the location for either a query or an insertion
+        """
+        if name:
+            return bson.son.SON([('org', org), ('course', course), ('name', name)])
+        else:
+            return bson.son.SON([('org', org), ('course', course)])
 
     def _block_id_is_guid(self, name):
         """

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

@@ -36,8 +36,9 @@ def test_create_map(self):
         org = 'foo_org'
         course = 'bar_course'
         loc_mapper().create_map_entry(Location('i4x', org, course, 'course', 'baz_run'))
+        # pylint: disable=protected-access
         entry = loc_mapper().location_map.find_one({
-            '_id': {'org': org, 'course': course, 'name': 'baz_run'}
+            '_id': loc_mapper()._construct_location_son(org, course, 'baz_run')
         })
         self.assertIsNotNone(entry, "Didn't find entry")
         self.assertEqual(entry['course_id'], '{}.{}.baz_run'.format(org, course))
@@ -48,8 +49,9 @@ def test_create_map(self):
         # ensure create_entry does the right thing when not given a course (creates org/course
         # rather than org/course/run course_id)
         loc_mapper().create_map_entry(Location('i4x', org, course, 'vertical', 'baz_vert'))
+        # find the one which has no name
         entry = loc_mapper().location_map.find_one({
-            '_id': {'org': org, 'course': course}
+            '_id' : loc_mapper()._construct_location_son(org, course, None)
         })
         self.assertIsNotNone(entry, "Didn't find entry")
         self.assertEqual(entry['course_id'], '{}.{}'.format(org, course))
@@ -63,9 +65,7 @@ def test_create_map(self):
             'wip',
             'live',
             block_map)
-        entry = loc_mapper().location_map.find_one({
-            '_id': {'org': org, 'course': course}
-        })
+        entry = loc_mapper().location_map.find_one({'_id.org': org, '_id.course': course})
         self.assertIsNotNone(entry, "Didn't find entry")
         self.assertEqual(entry['course_id'], 'foo_org.geek_dept.quux_course.baz_run')
         self.assertEqual(entry['draft_branch'], 'wip')

lms/djangoapps/courseware/roles.py

@@ -4,7 +4,6 @@
 """
 
 from abc import ABCMeta, abstractmethod
-from functools import partial
 
 from django.contrib.auth.models import User, Group
 

mongo_indexes.md

@@ -0,0 +1,21 @@
+These are the indexes each mongo db should have in order to perform well.
+Each section states the collection name and then the indexes. To create an index,
+you'll typically either use the mongohq type web interface or a standard terminal console.
+If a terminal, this assumes you've logged in and gotten to the mongo prompt 
+```
+mongo mydatabasename
+```
+
+If using the terminal, to add an index to a collection, you'll need to prefix ```ensureIndex``` with
+```
+db.collection_name
+```
+as in ```db.location_map.ensureIndex({'course_id': 1}{background: true})```
+
+location_map:
+=============
+
+```
+ensureIndex({​'_id.org': 1, '_id.course': 1})
+ensureIndex({​'course_id': 1})
+```