Restrict block ID comparison slightly to avoid false positives

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

@@ -57,6 +57,7 @@ import copy
 import datetime
 import hashlib
 import logging
+import six
 from contracts import contract, new_contract
 from importlib import import_module
 from mongodb_proxy import autoretry_read
@@ -1202,9 +1203,14 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
             block_name = qualifiers.pop('name')
             block_ids = []
             for block_id, block in course.structure['blocks'].iteritems():
-                # Do an in comparison on the name qualifier
-                # so that a list can be used to filter on block_id
-                if block_id.id in block_name and _block_matches_all(block):
+                # Don't do an in comparison blindly; first check to make sure
+                # that the name qualifier we're looking at isn't a plain string;
+                # if it is a string, then it should match exactly.
+                if isinstance(block_name, six.string_types):
+                    name_matches = block_id.id == block_name
+                else:
+                    name_matches = block_id.id in block_name
+                if name_matches and _block_matches_all(block):
                     block_ids.append(block_id)
 
             return self._load_items(course, block_ids, **kwargs)

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

@@ -267,6 +267,15 @@ class SplitModuleTest(unittest.TestCase):
                             },
                         },
                         {
+                            "id": "chap",
+                            "parent": "head12345",
+                            "parent_type": "course",
+                            "category": "chapter",
+                            "fields": {
+                                "display_name": "Buffalo buffalo Buffalo buffalo buffalo buffalo Buffalo buffalo"
+                            },
+                        },
+                        {
                             "id": "chapter2",
                             "parent": "head12345",
                             "parent_type": "course",
@@ -1189,13 +1198,14 @@ class SplitModuleItemTests(SplitModuleTest):
         locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT)
         # get all modules
         matches = modulestore().get_items(locator)
-        self.assertEqual(len(matches), 7)
+        self.assertEqual(len(matches), 8)
         matches = modulestore().get_items(locator)
-        self.assertEqual(len(matches), 7)
+        self.assertEqual(len(matches), 8)
         matches = modulestore().get_items(locator, qualifiers={'category': 'chapter'})
-        self.assertEqual(len(matches), 3)
+        self.assertEqual(len(matches), 4)
         matches = modulestore().get_items(locator, qualifiers={'category': 'garbage'})
         self.assertEqual(len(matches), 0)
+        # Test that we don't accidentally get an item with a similar name.
         matches = modulestore().get_items(locator, qualifiers={'name': 'chapter1'})
         self.assertEqual(len(matches), 1)
         matches = modulestore().get_items(locator, qualifiers={'name': ['chapter1', 'chapter2']})
@@ -1209,7 +1219,7 @@ class SplitModuleItemTests(SplitModuleTest):
         matches = modulestore().get_items(locator, settings={'group_access': {'$exists': True}})
         self.assertEqual(len(matches), 1)
         matches = modulestore().get_items(locator, settings={'group_access': {'$exists': False}})
-        self.assertEqual(len(matches), 6)
+        self.assertEqual(len(matches), 7)
 
     def test_get_parents(self):
         '''