Separate signal attachment between morango core and kolibri sync logic

kolibri/core/auth/apps.py

@@ -15,8 +15,14 @@ def ready(self):
         from .signals import cascade_delete_user  # noqa: F401
 
         from kolibri.core.auth.sync_event_hook_utils import (
-            register_sync_event_handlers,
+            pre_sync_transfer_handler,
+            post_sync_transfer_handler,
         )  # noqa: F401
         from morango.api.viewsets import session_controller  # noqa: F401
 
-        register_sync_event_handlers(session_controller.signals)
+        # attach to `initializing.completed` signal so that the context has all information needed
+        # for the handler and any hooks invoked by it
+        session_controller.signals.initializing.completed.connect(
+            pre_sync_transfer_handler
+        )
+        session_controller.signals.cleanup.completed.connect(post_sync_transfer_handler)

kolibri/core/auth/management/utils.py

@@ -26,7 +26,8 @@
 from kolibri.core.auth.models import dataset_cache
 from kolibri.core.auth.models import Facility
 from kolibri.core.auth.models import FacilityUser
-from kolibri.core.auth.sync_event_hook_utils import register_sync_event_handlers
+from kolibri.core.auth.sync_event_hook_utils import post_sync_transfer_handler
+from kolibri.core.auth.sync_event_hook_utils import pre_sync_transfer_handler
 from kolibri.core.device.models import DevicePermissions
 from kolibri.core.device.utils import device_provisioned
 from kolibri.core.device.utils import provision_device
@@ -403,7 +404,8 @@ def _sync(self, sync_session_client, **options):  # noqa: C901
         client_cert = sync_session_client.sync_session.client_certificate
         # we create a custom signals, so we can fire them outside of transaction blocks
         custom_signals = SessionControllerSignals()
-        register_sync_event_handlers(custom_signals)
+        custom_signals.initializing.started.connect(pre_sync_transfer_handler)
+        custom_signals.cleanup.completed.connect(post_sync_transfer_handler)
 
         filter_scope, scope_params = get_sync_filter_scope(client_cert, user_id=user_id)
         dataset_id = scope_params.get("dataset_id")

kolibri/core/auth/sync_event_hook_utils.py

@@ -97,7 +97,10 @@ def wrapper(context):
 
 
 @_local_event_handler
-def _pre_transfer_handler(**kwargs):
+def pre_sync_transfer_handler(**kwargs):
+    """
+    Used to attach to signals like those on morango.sync.controller.SessionControllerSignals
+    """
     for hook in FacilityDataSyncHook.registered_hooks:
         # we catch all errors because as a rule of thumb, we don't want hooks to fail
         try:
@@ -110,7 +113,10 @@ def _pre_transfer_handler(**kwargs):
 
 
 @_local_event_handler
-def _post_transfer_handler(**kwargs):
+def post_sync_transfer_handler(**kwargs):
+    """
+    Used to attach to signals like those on morango.sync.controller.SessionControllerSignals
+    """
     for hook in FacilityDataSyncHook.registered_hooks:
         # we catch all errors because as a rule of thumb, we don't want hooks to fail
         try:
@@ -120,14 +126,3 @@ def _post_transfer_handler(**kwargs):
                 "{}.post_transfer hook failed".format(hook.__class__.__name__),
                 exc_info=e,
             )
-
-
-def register_sync_event_handlers(signals):
-    """
-    Attaches the pre and post transfer handlers to the morango session controller signals.
-
-    :param signals: The signals object from the morango session controller
-    :type signals: morango.sync.controller.SessionControllerSignals
-    """
-    signals.initializing.started.connect(_pre_transfer_handler)
-    signals.cleanup.completed.connect(_post_transfer_handler)

kolibri/core/auth/test/test_sync_event_hook_utils.py

@@ -3,8 +3,8 @@
 from morango.sync.context import CompositeSessionContext
 from morango.sync.context import LocalSessionContext
 
-from kolibri.core.auth.sync_event_hook_utils import _post_transfer_handler
-from kolibri.core.auth.sync_event_hook_utils import _pre_transfer_handler
+from kolibri.core.auth.sync_event_hook_utils import post_sync_transfer_handler
+from kolibri.core.auth.sync_event_hook_utils import pre_sync_transfer_handler
 
 
 MODULE_NAME = "kolibri.core.auth.sync_event_hook_utils"
@@ -33,7 +33,7 @@ class TestHook(object):
     def test_pre_transfer(self, mock_hook_registry):
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.pre_transfer.assert_not_called()
-        _pre_transfer_handler(self.context)
+        pre_sync_transfer_handler(self.context)
         self.hook.pre_transfer.assert_called_once_with(
             context=self.local_context,
             dataset_id="123",
@@ -46,15 +46,15 @@ def test_pre_transfer__not_local(self, mock_hook_registry):
         self.context.children = [mock.Mock()]
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.pre_transfer.assert_not_called()
-        _pre_transfer_handler(self.context)
+        pre_sync_transfer_handler(self.context)
         self.hook.pre_transfer.assert_not_called()
 
     @mock.patch("kolibri.core.auth.sync_event_hook_utils.logger")
     def test_pre_transfer__failure(self, mock_logger, mock_hook_registry):
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.pre_transfer.assert_not_called()
         self.hook.pre_transfer.side_effect = RuntimeError()
-        _pre_transfer_handler(self.context)
+        pre_sync_transfer_handler(self.context)
         self.hook.pre_transfer.assert_called()
         mock_logger.error.assert_called_once_with(
             "TestHook.pre_transfer hook failed",
@@ -65,13 +65,13 @@ def test_pre_transfer__failure__logging(self, mock_hook_registry):
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.pre_transfer.assert_not_called()
         self.hook.pre_transfer.side_effect = RuntimeError()
-        _pre_transfer_handler(self.context)
+        pre_sync_transfer_handler(self.context)
         self.hook.pre_transfer.assert_called()
 
     def test_post_transfer(self, mock_hook_registry):
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.post_transfer.assert_not_called()
-        _post_transfer_handler(self.context)
+        post_sync_transfer_handler(self.context)
         self.hook.post_transfer.assert_called_once_with(
             context=self.local_context,
             dataset_id="123",
@@ -84,15 +84,15 @@ def test_post_transfer__not_local(self, mock_hook_registry):
         self.context.children = [mock.Mock()]
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.post_transfer.assert_not_called()
-        _post_transfer_handler(self.context)
+        post_sync_transfer_handler(self.context)
         self.hook.post_transfer.assert_not_called()
 
     @mock.patch("kolibri.core.auth.sync_event_hook_utils.logger")
     def test_post_transfer__failure(self, mock_logger, mock_hook_registry):
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.post_transfer.assert_not_called()
         self.hook.post_transfer.side_effect = RuntimeError()
-        _post_transfer_handler(self.context)
+        post_sync_transfer_handler(self.context)
         self.hook.post_transfer.assert_called()
         mock_logger.error.assert_called_once_with(
             "TestHook.post_transfer hook failed",
@@ -103,5 +103,5 @@ def test_post_transfer__failure__logging(self, mock_hook_registry):
         mock_hook_registry.registered_hooks = [self.hook]
         self.hook.post_transfer.assert_not_called()
         self.hook.post_transfer.side_effect = RuntimeError()
-        _post_transfer_handler(self.context)
+        post_sync_transfer_handler(self.context)
         self.hook.post_transfer.assert_called()