Merge pull request #5933 from edx/dhm/bug_tnl-764

Remove over optimization which didn't work correctly

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

@@ -20,7 +20,6 @@ import re
 from uuid import uuid4
 
 from bson.son import SON
-from contracts import contract, new_contract
 from datetime import datetime
 from fs.osfs import OSFS
 from mongodb_proxy import MongoProxy, autoretry_read
@@ -1237,10 +1236,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
 
             # update subtree edited info for ancestors
             # don't update the subtree info for descendants of the publish root for efficiency
-            if (
-                (not isPublish or (isPublish and is_publish_root)) and
-                not self._is_in_bulk_operation(xblock.location.course_key)
-            ):
+            if not isPublish or (isPublish and is_publish_root):
                 ancestor_payload = {
                     'edit_info.subtree_edited_on': now,
                     'edit_info.subtree_edited_by': user_id

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

@@ -1432,25 +1432,26 @@ class TestMixedModuleStore(CourseComparisonTest):
             self.assertLess(node.subtree_edited_on, subtree_before)
             self.assertEqual(node.subtree_edited_by, subtree_by)
 
-        # Create a dummy vertical & html to test against
-        component = self.store.create_child(
-            self.user_id,
-            test_course.location,
-            'vertical',
-            block_id='test_vertical'
-        )
-        child = self.store.create_child(
-            self.user_id,
-            component.location,
-            'html',
-            block_id='test_html'
-        )
-        sibling = self.store.create_child(
-            self.user_id,
-            component.location,
-            'html',
-            block_id='test_html_no_change'
-        )
+        with self.store.bulk_operations(test_course.id):
+            # Create a dummy vertical & html to test against
+            component = self.store.create_child(
+                self.user_id,
+                test_course.location,
+                'vertical',
+                block_id='test_vertical'
+            )
+            child = self.store.create_child(
+                self.user_id,
+                component.location,
+                'html',
+                block_id='test_html'
+            )
+            sibling = self.store.create_child(
+                self.user_id,
+                component.location,
+                'html',
+                block_id='test_html_no_change'
+            )
 
         after_create = datetime.datetime.now(UTC)
         # Verify that all nodes were last edited in the past by create_user
@@ -1461,7 +1462,8 @@ class TestMixedModuleStore(CourseComparisonTest):
         component.display_name = 'Changed Display Name'
 
         editing_user = self.user_id - 2
-        component = self.store.update_item(component, editing_user)
+        with self.store.bulk_operations(test_course.id):  # TNL-764 bulk ops disabled ancestor updates
+            component = self.store.update_item(component, editing_user)
         after_edit = datetime.datetime.now(UTC)
         check_node(component.location, after_create, after_edit, editing_user, after_create, after_edit, editing_user)
         # but child didn't change

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

@@ -24,20 +24,21 @@ class TestPublish(SplitWMongoCourseBoostrapper):
 
         # with bulk will delay all inheritance computations which won't be added into the mongo_calls
         with self.draft_mongo.bulk_operations(self.old_course_key):
-            # finds: 1 for parent to add child
+            # finds: 1 for parent to add child and 2 to get ancestors
             # sends: 1 for insert, 1 for parent (add child)
-            with check_mongo_calls(1, 2):
+            with check_mongo_calls(3, 2):
                 self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', split=False)
 
-            with check_mongo_calls(2, 2):
+            with check_mongo_calls(4, 2):
                 self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', split=False)
             # For each vertical (2) created:
             #   - load draft
             #   - load non-draft
             #   - get last error
             #   - load parent
+            #   - get ancestors
             #   - load inheritable data
-            with check_mongo_calls(7, 4):
+            with check_mongo_calls(15, 6):
                 self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', split=False)
                 self._create_item('vertical', 'Vert2', {}, {'display_name': 'Vertical 2'}, 'chapter', 'Chapter1', split=False)
             # For each (4) item created
@@ -48,8 +49,9 @@ class TestPublish(SplitWMongoCourseBoostrapper):
             #   - load parent
             #   - load inheritable data
             #   - load parent
+            #   - load ancestors
             # count for updates increased to 16 b/c of edit_info updating
-            with check_mongo_calls(16, 8):
+            with check_mongo_calls(40, 16):
                 self._create_item('html', 'Html1', "<p>Goodbye</p>", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', split=False)
                 self._create_item(
                     'discussion', 'Discussion1',
@@ -77,7 +79,7 @@ class TestPublish(SplitWMongoCourseBoostrapper):
                     split=False
                 )
 
-            with check_mongo_calls(0, 2):
+            with check_mongo_calls(2, 2):
                 # 2 finds b/c looking for non-existent parents
                 self._create_item('static_tab', 'staticuno', "<p>tab</p>", {'display_name': 'Tab uno'}, None, None, split=False)
                 self._create_item('course_info', 'updates', "<ol><li><h2>Sep 22</h2><p>test</p></li></ol>", {}, None, None, split=False)