Correct behavior for background initialization, only useful for pulls

kolibri_sync_extras_plugin/management/commands/sync_proceed_to.py

@@ -34,10 +34,15 @@ def handle(self, *args, **options):
         capabilities = options.pop("capabilities")
 
         context = BackgroundSessionContext(
-            transfer_session=transfer_session, is_push=True, capabilities=capabilities
+            transfer_session=transfer_session, capabilities=capabilities
         )
 
-        logger.info("Proceeding {} to {}".format(transfer_session.pk, target_stage))
+        sync_dir = "push" if transfer_session.push else "pull"
+        logger.info(
+            "Proceeding {} transfer session {} to {}".format(
+                sync_dir, transfer_session.pk, target_stage
+            )
+        )
         status = transfer_statuses.PENDING
         tries = 0
 

kolibri_sync_extras_plugin/sync/context.py

@@ -9,11 +9,10 @@ class BackgroundSessionContext(LocalSessionContext):
     """
 
     def __init__(self, *args, **kwargs):
+        is_server = kwargs.pop("is_server", None)
         super(BackgroundSessionContext, self).__init__(*args, **kwargs)
-        # for this use case, we're always going to be the server, and the receiver
-        self.is_server = True
-
-    @property
-    def is_receiver(self):
-        # for this use case, we're always going to be the server, and the receiver
-        return True
+        # base class sets `is_server` on the presence of a request object, which we don't have in bg,
+        # so we rely on it being passed in, deferring to the sync session if not provided
+        if is_server is None:
+            is_server = getattr(self.sync_session, "is_server", False)
+        self.is_server = is_server

kolibri_sync_extras_plugin/tasks.py

@@ -25,4 +25,4 @@ def set_job_id(context, job_id):
     job_key = get_job_key(context)
     extra_fields[job_key] = job_id
     context.sync_session.extra_fields = json.dumps(extra_fields)
-    context.sync_session.save()
+    context.sync_session.save(update_fields=["extra_fields"])

pyproject.toml

@@ -20,8 +20,7 @@ classifiers = [
     "Programming Language :: Python :: 3.9",
 ]
 dependencies = [
-    "kolibri>=0.15",
-    "morango>=0.6.13",
+    "kolibri>=0.19",
 ]
 
 [project.urls]
@@ -36,12 +35,13 @@ dev = [
     { include-group = "lint" },
     { include-group = "test" },
     "prek",
+    "morango>=0.8.15"
 ]
 lint = [
     "ruff",
 ]
 test = [
-    "mock",
+    "django==3.2.25",  # needed for pytest-django
     "pytest",
     "pytest-django",
     "pytest-env",

test/base.py

@@ -1,4 +1,5 @@
-import mock
+from unittest import mock
+
 from django.test import SimpleTestCase
 from morango.constants import transfer_stages
 from morango.models.core import SyncSession
@@ -11,7 +12,9 @@
 class BaseTestCase(SimpleTestCase):
     def setUp(self):
         super(BaseTestCase, self).setUp()
-        self.sync_session = mock.Mock(spec=SyncSession, id="abc123", extra_fields="{}")
+        self.sync_session = mock.Mock(
+            spec=SyncSession, id="abc123", extra_fields="{}", is_server=True
+        )
         self.transfer_session = mock.Mock(
             spec=TransferSession,
             id="def456",

test/management/commands/test_sync_proceed_to.py

@@ -1,4 +1,5 @@
-import mock
+from unittest import mock
+
 from morango.constants import capabilities
 from morango.constants import transfer_stages
 from morango.constants import transfer_statuses
@@ -41,8 +42,28 @@ def test_pass__async_capable(self):
         self.assertEqual(proceed_to_call.get("target_stage"), transfer_stages.CLEANUP)
         self.assertEqual(context.transfer_session, self.transfer_session)
         self.assertTrue(context.is_push)
+        self.assertTrue(context.is_server)
+        self.assertTrue(context.is_receiver)
         self.assertEqual(context.capabilities, {capabilities.ASYNC_OPERATIONS})
 
+    def test_pass__pull(self):
+        # a server servicing a pull should be the producer, not the receiver, unlike the
+        # previously hardcoded behavior which always forced push/receiver semantics
+        self.transfer_session.push = False
+        self.session_controller_mock.proceed_to.return_value = transfer_statuses.COMPLETED
+        self.cmd.handle(
+            id="abc123",
+            target_stage=transfer_stages.CLEANUP,
+            capabilities=[capabilities.ASYNC_OPERATIONS],
+        )
+
+        proceed_to_call = self.session_controller_mock.proceed_to.call_args_list[0][1]
+        context = proceed_to_call.get("context")
+        self.assertFalse(context.is_push)
+        self.assertTrue(context.is_server)
+        self.assertFalse(context.is_receiver)
+        self.assertTrue(context.is_producer)
+
     def test_failure__errored(self):
         def _proceed_to(target_stage=None, context=None):
             context.error = RuntimeError("test")

test/sync/test_context.py

@@ -1,3 +1,5 @@
+import unittest
+
 from morango.constants import transfer_stages
 from morango.constants import transfer_statuses
 
@@ -13,3 +15,50 @@ def test_background_context(self):
         self.assertTrue(context.is_receiver)
         context.update_state(stage=transfer_stages.QUEUING, stage_status=transfer_statuses.STARTED)
         self.transfer_session.update_state.assert_called_once()
+
+    def test_is_server__derived_from_sync_session(self):
+        self.sync_session.is_server = False
+        context = BackgroundSessionContext(transfer_session=self.transfer_session)
+        self.assertFalse(context.is_server)
+
+    def test_is_server__explicit_kwarg_overrides_sync_session(self):
+        self.sync_session.is_server = False
+        context = BackgroundSessionContext(transfer_session=self.transfer_session, is_server=True)
+        self.assertTrue(context.is_server)
+
+    def test_is_receiver__server_push(self):
+        # a server receiving a push is the receiver
+        self.sync_session.is_server = True
+        self.transfer_session.push = True
+        context = BackgroundSessionContext(transfer_session=self.transfer_session)
+        self.assertTrue(context.is_receiver)
+        self.assertFalse(context.is_producer)
+
+    def test_is_receiver__server_pull(self):
+        # a server servicing a pull is the producer, not the receiver
+        self.sync_session.is_server = True
+        self.transfer_session.push = False
+        context = BackgroundSessionContext(transfer_session=self.transfer_session)
+        self.assertFalse(context.is_receiver)
+        self.assertTrue(context.is_producer)
+
+    def test_is_receiver__client_push(self):
+        # a client pushing is the producer, not the receiver
+        self.sync_session.is_server = False
+        self.transfer_session.push = True
+        context = BackgroundSessionContext(transfer_session=self.transfer_session)
+        self.assertFalse(context.is_receiver)
+        self.assertTrue(context.is_producer)
+
+    @unittest.expectedFailure
+    def test_is_receiver__client_pull(self):
+        # a client pulling is the receiver, not the producer
+        # currently fails because morango's SessionContext resolves `is_push` via
+        # `transfer_session.push or self.is_push`, which collapses `False` (pull) down to
+        # `None` when no explicit `is_push` is passed, since nothing set `self.is_push` first.
+        # remove this xfail marker once that's fixed upstream in morango.
+        self.sync_session.is_server = False
+        self.transfer_session.push = False
+        context = BackgroundSessionContext(transfer_session=self.transfer_session)
+        self.assertTrue(context.is_receiver)
+        self.assertFalse(context.is_producer)

test/sync/test_operations.py

@@ -1,4 +1,5 @@
-import mock
+from unittest import mock
+
 from kolibri.core.tasks.job import State
 from morango.constants import capabilities
 from morango.constants import transfer_stages

test/test_tasks.py

@@ -16,4 +16,4 @@ def test_get_job_id(self):
     def test_set_job_id(self):
         set_job_id(self.context, "xyz789")
         self.assertEqual('{"def456_dequeuing_job_id": "xyz789"}', self.sync_session.extra_fields)
-        self.sync_session.save.assert_called_once()
+        self.sync_session.save.assert_called_once_with(update_fields=["extra_fields"])