Merge pull request #8438 from jamalex/dont_sync_the_messenger

First pass at regression test and fix for SoUD FacilityUser sync conflict

kolibri/core/auth/kolibri_plugin.py

@@ -0,0 +1,26 @@
+from kolibri.core.auth.hooks import FacilityDataSyncHook
+from kolibri.core.auth.models import FacilityUser
+from kolibri.plugins.hooks import register_hook
+
+
+@register_hook
+class SingleFacilityUserChangeClearingHook(FacilityDataSyncHook):
+    def pre_transfer(
+        self,
+        dataset_id,
+        local_is_single_user,
+        remote_is_single_user,
+        single_user_id,
+        context,
+    ):
+
+        # If this is a single-user device, make sure we ignore any changes
+        # that have been made to FacilityUsers, to avoid conflicts.
+        if local_is_single_user and single_user_id is not None:
+            try:
+                user = FacilityUser.objects.get(
+                    id=single_user_id, _morango_dirty_bit=True
+                )
+                user.save(update_dirty_bit_to=False)
+            except FacilityUser.DoesNotExist:
+                pass

kolibri/core/auth/test/sync_utils.py

@@ -71,6 +71,17 @@ def create_model(self, model, **kwargs):
             )
         )
 
+    def update_model(self, model, pk, **kwargs):
+        kwarg_text = json.dumps(kwargs, default=str)
+        self.pipe_shell(
+            'import json; from {module_path} import {model_nm}; kwargs = json.loads("""{}"""); {model_nm}.objects.filter(pk="{pk}").update(**kwargs)'.format(
+                kwarg_text,
+                module_path=model.__module__,
+                model_nm=model.__name__,
+                pk=pk,
+            )
+        )
+
     def delete_model(self, model, **kwargs):
         kwarg_text = json.dumps(kwargs, default=str)
         self.pipe_shell(

kolibri/core/auth/test/test_morango_integration.py

@@ -1,6 +1,7 @@
 """
 Tests related specifically to integration with Morango.
 """
+import datetime
 import os
 import unittest
 import uuid
@@ -857,6 +858,57 @@ def test_single_user_assignment_sync(self, servers):
             == 0
         )
 
+    @multiple_kolibri_servers(2)
+    def test_facility_user_conflict_syncing_from_tablet(self, servers):
+        self._test_facility_user_conflict(servers, True)
+
+    @multiple_kolibri_servers(2)
+    def test_facility_user_conflict_syncing_from_laptop(self, servers):
+        self._test_facility_user_conflict(servers, False)
+
+    def _test_facility_user_conflict(self, servers, tablet_is_client):
+        """
+        This is a regression test to handle the case of a FacilityUser being changed
+        on a SoUD as a backend side effect, e.g. by Django updating the `last_login`
+        field, leading to a merge conflict when the user is updated on another device.
+        """
+
+        self.servers = servers
+        self.laptop = 0
+        self.tablet = 1
+
+        self.alias_laptop = servers[self.laptop].db_alias
+        self.alias_tablet = servers[self.tablet].db_alias
+
+        # create the original facility on the Laptop
+        servers[self.laptop].manage("loaddata", "content_test")
+        servers[self.laptop].manage(
+            "generateuserdata", "--no-onboarding", "--num-content-items", "1"
+        )
+        self.facility_id = Facility.objects.using(self.alias_laptop).get().id
+        self.learner = FacilityUser.objects.using(self.alias_laptop).filter(
+            roles__isnull=True
+        )[0]
+
+        self.sync_single_user(self.laptop)
+
+        servers[self.laptop].update_model(
+            FacilityUser, self.learner.id, full_name="NEW"
+        )
+
+        servers[self.tablet].update_model(
+            FacilityUser, self.learner.id, last_login=datetime.datetime.now()
+        )
+
+        self.sync_single_user(self.laptop, tablet_is_client=tablet_is_client)
+
+        assert (
+            FacilityUser.objects.using(self.alias_tablet)
+            .get(id=self.learner.id)
+            .full_name
+            == "NEW"
+        )
+
     def sync_full_facility_servers(self):
         """
         Perform a full sync between Laptop A and Laptop B.

kolibri/plugins/registry.py

@@ -32,6 +32,7 @@
 from __future__ import unicode_literals
 
 import logging
+from importlib import import_module
 
 from django.apps import AppConfig
 from django.conf import settings
@@ -56,6 +57,34 @@ class PluginExistsInApp(Exception):
     """
 
 
+def parse_installed_app_entry(app):
+    # In case we are registering non-plugins from INSTALLED_APPS (the usual use case)
+    # that could include Django AppConfig objects, or module paths to Django AppConfig objects.
+    if isinstance(app, AppConfig):
+        return app.name
+    # Check if this is a module path or an import path to an AppConfig object.
+    # Logic here modified from:
+    # https://github.com/django/django/blob/c669cf279ae7b3e02a61db4fb077030a4db80e4f/django/apps/config.py#L86
+    try:
+        # If import_module succeeds, entry is a path to an app module,
+        # so we carry on.
+        # Otherwise, entry is a path to an app config class or an error.
+        import_module(app)
+    except ImportError:
+        mod_path, _, cls_name = app.rpartition(".")
+        try:
+            module = import_module(mod_path)
+            cls = getattr(module, cls_name)
+            return cls.name
+        except (ImportError, AttributeError):
+            # If none of this works out, something has been misconfigured
+            # Django will be picking this up soon and give more detailed debugging
+            # so we just let this pass silently for now.
+            pass
+    # If we get to here, just return the original.
+    return app
+
+
 class Registry(object):
     __slots__ = ("_apps",)
 
@@ -111,12 +140,10 @@ class in their kolibri_plugin.py module - these cannot be enabled and disabled
         by the Kolibri plugin machinery, but may wish to still register Kolibri Hooks
         """
         for app in apps:
-            # In case we are registering non-plugins from INSTALLED_APPS (the usual use case)
-            # that could include Django AppConfig objects.
-            if isinstance(app, AppConfig):
-                app = app.name
+            app = parse_installed_app_entry(app)
             if app not in self._apps:
                 try:
+
                     initialize_kolibri_plugin(app)
                     # Raise an error here because non-plugins should raise a PluginDoesNotExist exception
                     # if they are properly configured.