From 5ab2e869887eb4a7b1ce0471dd13e36e539d9f0d Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Thu, 11 Jun 2015 17:14:20 +0500 Subject: [PATCH] In bulk_operations() exit, emit signals at the end. PLAT-676 --- .../xmodule/xmodule/modulestore/__init__.py | 11 +++-- .../xmodule/xmodule/modulestore/mongo/base.py | 10 ++--- .../xmodule/modulestore/split_mongo/split.py | 6 +-- .../tests/test_mixed_modulestore.py | 43 +++++++++++++++++++ 4 files changed, 57 insertions(+), 13 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 6ed562ce34..81232c0b3d 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -225,7 +225,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): """ @@ -249,7 +250,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. @@ -273,10 +274,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 9f62ee5ef4..41bd27b3c5 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 722dbf8dcf..786f0b95a0 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: