From 581d37ff59b1e8262fae6313ce68093737cb3912 Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Tue, 31 Oct 2023 12:49:27 -0700 Subject: [PATCH] Avoid syncing with non-kolibris --- kolibri/core/device/soud.py | 4 +- kolibri/core/device/test/test_soud.py | 43 +++++++++++++++++++++ kolibri/core/discovery/models.py | 30 +++++++++++---- kolibri/core/discovery/test/test_models.py | 45 ++++++++++++++++++++++ kolibri/plugins/learn/kolibri_plugin.py | 2 +- kolibri/plugins/learn/test/test_hooks.py | 7 ++++ kolibri/plugins/user_profile/tasks.py | 13 +++++-- 7 files changed, 131 insertions(+), 13 deletions(-) create mode 100644 kolibri/core/device/test/test_soud.py create mode 100644 kolibri/core/discovery/test/test_models.py diff --git a/kolibri/core/device/soud.py b/kolibri/core/device/soud.py index 63df3d115b1..9cf020bbfcc 100644 --- a/kolibri/core/device/soud.py +++ b/kolibri/core/device/soud.py @@ -64,7 +64,9 @@ def sync_queue(self): @cached_property def network_location(self): return NetworkLocation.objects.filter( - instance_id=self.instance_id, connection_status=ConnectionStatus.Okay + instance_id=self.instance_id, + connection_status=ConnectionStatus.Okay, + application="kolibri", ).first() @property diff --git a/kolibri/core/device/test/test_soud.py b/kolibri/core/device/test/test_soud.py new file mode 100644 index 00000000000..d92048713a6 --- /dev/null +++ b/kolibri/core/device/test/test_soud.py @@ -0,0 +1,43 @@ +""" +Subset of Users Device (SOUD) tests +""" +import uuid + +from django.test import TestCase + +from ..soud import Context +from kolibri.core.discovery.models import ConnectionStatus +from kolibri.core.discovery.models import StaticNetworkLocation + + +class SoudContextTestCase(TestCase): + def setUp(self): + super(SoudContextTestCase, self).setUp() + self.context = Context(uuid.uuid4().hex, uuid.uuid4().hex) + + def test_property__network_location(self): + netloc = StaticNetworkLocation.objects.create( + base_url="https://kolibrihappyurl.qqq/", + connection_status=ConnectionStatus.Okay, + application="kolibri", + instance_id=self.context.instance_id, + ) + self.assertEqual(self.context.network_location, netloc) + + def test_property__network_location__not_kolibri(self): + StaticNetworkLocation.objects.create( + base_url="https://kolibrihappyurl.qqq/", + connection_status=ConnectionStatus.Okay, + application="studio", + instance_id=self.context.instance_id, + ) + self.assertIsNone(self.context.network_location) + + def test_property__network_location__not_connected(self): + StaticNetworkLocation.objects.create( + base_url="https://kolibrihappyurl.qqq/", + connection_status=ConnectionStatus.ConnectionFailure, + application="kolibri", + instance_id=self.context.instance_id, + ) + self.assertIsNone(self.context.network_location) diff --git a/kolibri/core/discovery/models.py b/kolibri/core/discovery/models.py index 6595fea1d69..e0a405772e3 100644 --- a/kolibri/core/discovery/models.py +++ b/kolibri/core/discovery/models.py @@ -101,6 +101,18 @@ class Meta: # Determines whether device is local or on the internet is_local = models.BooleanField(default=False) + def __init__(self, *args, **kwargs): + super(NetworkLocation, self).__init__(*args, **kwargs) + self._set_fields_for_type() + + def save(self, *args, **kwargs): + self._set_fields_for_type() + return super(NetworkLocation, self).save(*args, **kwargs) + + def _set_fields_for_type(self): + """Abstract method to set fields for type""" + pass + @property def since_last_accessed(self): """ @@ -133,6 +145,10 @@ def dynamic(self, value): def reserved(self): return self.location_type == LocationTypes.Reserved + @property + def is_kolibri(self): + return self.application == "kolibri" + @classmethod def has_field(cls, field): try: @@ -161,13 +177,12 @@ def get_queryset(self): class StaticNetworkLocation(NetworkLocation): objects = StaticNetworkLocationManager() + def _set_fields_for_type(self): + self.location_type = LocationTypes.Static + class Meta: proxy = True - def save(self, *args, **kwargs): - self.location_type = LocationTypes.Static - return super(StaticNetworkLocation, self).save(*args, **kwargs) - class DynamicNetworkLocationManager(models.Manager): def get_queryset(self): @@ -189,13 +204,14 @@ def update_or_create(self, defaults, **kwargs): class DynamicNetworkLocation(NetworkLocation): objects = DynamicNetworkLocationManager() + def _set_fields_for_type(self): + self.location_type = LocationTypes.Dynamic + self.is_local = True # all dynamic locations are local + class Meta: proxy = True def save(self, *args, **kwargs): - self.location_type = LocationTypes.Dynamic - self.is_local = True - if self.id and self.instance_id and self.id != self.instance_id: raise ValidationError( {"instance_id": "`instance_id` and `id` must be the same"} diff --git a/kolibri/core/discovery/test/test_models.py b/kolibri/core/discovery/test/test_models.py new file mode 100644 index 00000000000..98b61cbba72 --- /dev/null +++ b/kolibri/core/discovery/test/test_models.py @@ -0,0 +1,45 @@ +from django.test import TestCase + +from ..models import ConnectionStatus +from ..models import DynamicNetworkLocation +from ..models import LocationTypes +from ..models import NetworkLocation +from ..models import StaticNetworkLocation + + +class NetworkLocationTestCase(TestCase): + multi_db = True + + def test_property__available(self): + location = NetworkLocation() + self.assertFalse(location.available) + location.connection_status = ConnectionStatus.Okay + self.assertFalse(location.available) + location.base_url = "https://kolibrihappyurl.qqq/" + self.assertTrue(location.available) + location.connection_status = ConnectionStatus.ConnectionFailure + self.assertFalse(location.available) + + def test_property__is_kolibri(self): + location = NetworkLocation() + self.assertFalse(location.is_kolibri) + location.application = "kdp" + self.assertFalse(location.is_kolibri) + location.application = "kolibri" + self.assertTrue(location.is_kolibri) + + def test_property__dynamic(self): + static = StaticNetworkLocation() + self.assertFalse(static.dynamic) + dynamic = DynamicNetworkLocation() + self.assertTrue(dynamic.dynamic) + reserved = NetworkLocation(location_type=LocationTypes.Reserved) + self.assertFalse(reserved.dynamic) + + def test_property__reserved(self): + static = StaticNetworkLocation() + self.assertFalse(static.reserved) + dynamic = DynamicNetworkLocation() + self.assertFalse(dynamic.reserved) + reserved = NetworkLocation(location_type=LocationTypes.Reserved) + self.assertTrue(reserved.reserved) diff --git a/kolibri/plugins/learn/kolibri_plugin.py b/kolibri/plugins/learn/kolibri_plugin.py index 7f4d49d45ad..7a0ecb41f6f 100644 --- a/kolibri/plugins/learn/kolibri_plugin.py +++ b/kolibri/plugins/learn/kolibri_plugin.py @@ -116,7 +116,7 @@ def request_soud_sync(network_location): from kolibri.core.auth.tasks import enqueue_soud_sync_processing from kolibri.core.device.soud import request_sync_hook - if not network_location.subset_of_users_device: + if not network_location.subset_of_users_device and network_location.is_kolibri: request_sync_hook(network_location) enqueue_soud_sync_processing() diff --git a/kolibri/plugins/learn/test/test_hooks.py b/kolibri/plugins/learn/test/test_hooks.py index 69dd45f51f9..3a9d55e819f 100644 --- a/kolibri/plugins/learn/test/test_hooks.py +++ b/kolibri/plugins/learn/test/test_hooks.py @@ -12,6 +12,7 @@ def setUp(self): "kolibri.plugins.learn.NetworkDiscoveryForSoUDHook" ) self.mock_location = mock.MagicMock(spec=NetworkLocation()) + self.mock_location.is_kolibri = True begin_patcher = mock.patch("kolibri.core.device.soud.request_sync_hook") self.mock_begin = begin_patcher.start() self.addCleanup(begin_patcher.stop) @@ -32,6 +33,12 @@ def test_on_connect__location_is_not_soud(self): self.hook.on_connect(self.mock_location) self.mock_begin.assert_called_once_with(self.mock_location) + def test_on_connect__location_is_not_soud__but_not_kolibri(self): + self.mock_location.subset_of_users_device = False + self.mock_location.is_kolibri = False + self.hook.on_connect(self.mock_location) + self.mock_begin.assert_not_called() + def test_on_connect__self_not_soud__location_not_soud(self): self.mock_get_setting.return_value = False self.mock_location.subset_of_users_device = False diff --git a/kolibri/plugins/user_profile/tasks.py b/kolibri/plugins/user_profile/tasks.py index 49a61b66bb9..20328894490 100644 --- a/kolibri/plugins/user_profile/tasks.py +++ b/kolibri/plugins/user_profile/tasks.py @@ -13,6 +13,7 @@ from kolibri.core.auth.utils.migrate import merge_users from kolibri.core.device.models import DevicePermissions from kolibri.core.device.utils import set_device_settings +from kolibri.core.discovery.models import ConnectionStatus from kolibri.core.discovery.models import NetworkLocation from kolibri.core.tasks.decorators import register_task from kolibri.core.tasks.job import JobStatus @@ -88,10 +89,14 @@ def start_soud_sync(user_id): # This user would not previously have been included in any syncs # triggered by a device appearing on the network, so request syncs for them now - network_locations = NetworkLocation.objects.filter(subset_of_users_device=False) - for network_location in network_locations: - soud.request_sync(soud.Context(user_id, network_location.instance_id)) - if network_locations: + instance_ids = NetworkLocation.objects.filter( + subset_of_users_device=False, + connection_status=ConnectionStatus.Okay, + application="kolibri", + ).values_list("instance_id", flat=True) + for instance_id in instance_ids: + soud.request_sync(soud.Context(user_id, instance_id)) + if instance_ids: enqueue_soud_sync_processing()