diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index e35cabc149..c167c25422 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -221,7 +221,8 @@ class BulkOperationsMixin(object): """ Clear the record for this course """ - del self._active_bulk_ops.records[course_key.for_branch(None)] + if course_key.for_branch(None) in self._active_bulk_ops.records: + del self._active_bulk_ops.records[course_key.for_branch(None)] def _start_outermost_bulk_operation(self, bulk_ops_record, course_key): """ @@ -245,7 +246,7 @@ class BulkOperationsMixin(object): if bulk_ops_record.is_root: self._start_outermost_bulk_operation(bulk_ops_record, course_key) - def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key, emit_signals=True): + def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key): """ The outermost nested bulk_operation call: do the actual end of the bulk operation. @@ -269,10 +270,14 @@ class BulkOperationsMixin(object): if bulk_ops_record.active: return - self._end_outermost_bulk_operation(bulk_ops_record, structure_key, emit_signals) + dirty = self._end_outermost_bulk_operation(bulk_ops_record, structure_key) self._clear_bulk_ops_record(structure_key) + if emit_signals and dirty: + self.send_bulk_published_signal(bulk_ops_record, structure_key) + self.send_bulk_library_updated_signal(bulk_ops_record, structure_key) + def _is_in_bulk_operation(self, course_key, ignore_case=False): """ Return whether a bulk operation is active on `course_key`. diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index dd37d47feb..39df96df05 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -476,19 +476,17 @@ class MongoBulkOpsMixin(BulkOperationsMixin): # ensure it starts clean bulk_ops_record.dirty = False - def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key, emit_signals=True): + def _end_outermost_bulk_operation(self, bulk_ops_record, structure_key): """ Restart updating the meta-data inheritance cache for the given course or library. Refresh the meta-data inheritance cache now since it was temporarily disabled. """ + dirty = False if bulk_ops_record.dirty: self.refresh_cached_metadata_inheritance_tree(structure_key) - - if emit_signals: - self.send_bulk_published_signal(bulk_ops_record, structure_key) - self.send_bulk_library_updated_signal(bulk_ops_record, structure_key) - + dirty = True bulk_ops_record.dirty = False # brand spanking clean now + return dirty def _is_in_bulk_operation(self, course_id, ignore_case=False): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 4da2282c8c..1dedc2f394 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -231,7 +231,7 @@ class SplitBulkWriteMixin(BulkOperationsMixin): bulk_write_record.index = copy.deepcopy(bulk_write_record.initial_index) bulk_write_record.course_key = course_key - def _end_outermost_bulk_operation(self, bulk_write_record, structure_key, emit_signals=True): + def _end_outermost_bulk_operation(self, bulk_write_record, structure_key): """ End the active bulk write operation on structure_key (course or library key). """ @@ -273,9 +273,7 @@ class SplitBulkWriteMixin(BulkOperationsMixin): course_context=bulk_write_record.course_key ) - if dirty and emit_signals: - self.send_bulk_published_signal(bulk_write_record, structure_key) - self.send_bulk_library_updated_signal(bulk_write_record, structure_key) + return dirty def get_course_index(self, course_key, ignore_case=False): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 7840548a09..b41bf268d6 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1989,6 +1989,49 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): dest_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.split) self.assertCoursesEqual(source_store, source_course_key, dest_store, dest_course_id) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_bulk_operations_signal_firing(self, default): + """ Signals should be fired right before bulk_operations() exits. """ + with MongoContentstoreBuilder().build() as contentstore: + self.store = MixedModuleStore( + contentstore=contentstore, + create_modulestore_instance=create_modulestore_instance, + mappings={}, + signal_handler=SignalHandler(MixedModuleStore), + **self.OPTIONS + ) + self.addCleanup(self.store.close_all_connections) + + with self.store.default_store(default): + + with mock_signal_receiver(SignalHandler.course_published) as receiver: + self.assertEqual(receiver.call_count, 0) + + # Course creation and publication should fire the signal + course = self.store.create_course('org_x', 'course_y', 'run_z', self.user_id) + self.assertEqual(receiver.call_count, 1) + receiver.reset_mock() + + course_key = course.id + + def _clear_bulk_ops_record(course_key): # pylint: disable=unused-argument + """ Check if the signal has been fired. """ + self.assertEqual(receiver.call_count, 0) + + with patch.object( + self.store.thread_cache.default_store, '_clear_bulk_ops_record', wraps=_clear_bulk_ops_record + ) as mock_clear_bulk_ops_record: + + with self.store.bulk_operations(course_key): + categories = DIRECT_ONLY_CATEGORIES + for block_type in categories: + self.store.create_item(self.user_id, course_key, block_type) + self.assertEqual(receiver.call_count, 0) + + self.assertEqual(mock_clear_bulk_ops_record.call_count, 1) + + self.assertEqual(receiver.call_count, 1) + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_course_publish_signal_direct_firing(self, default): with MongoContentstoreBuilder().build() as contentstore: