Reserved network locations for Studio and KDP

kolibri/core/discovery/api.py

@@ -20,20 +20,40 @@
 from kolibri.core.api import BaseValuesViewset
 from kolibri.core.api import ValuesViewset
 from kolibri.core.device.permissions import NotProvisionedHasPermission
+from kolibri.core.discovery.well_known import CENTRAL_CONTENT_BASE_INSTANCE_ID
+from kolibri.core.discovery.well_known import DATA_PORTAL_BASE_INSTANCE_ID
 from kolibri.core.utils.urls import reverse_path
 
 
 class NetworkLocationViewSet(viewsets.ModelViewSet):
     permission_classes = [NetworkLocationPermissions | NotProvisionedHasPermission]
     serializer_class = NetworkLocationSerializer
-    queryset = NetworkLocation.objects.exclude(location_type=LocationTypes.Reserved)
     filter_backends = [DjangoFilterBackend]
     filterset_fields = [
         "id",
         "subset_of_users_device",
         "instance_id",
     ]
 
+    def get_queryset(self):
+        syncable = self.request.query_params.get("syncable", None)
+        base_queryset = NetworkLocation.objects.filter(
+            location_type__in=[LocationTypes.Static, LocationTypes.Dynamic]
+        )
+        reserved_ids = []
+        if syncable == "1":
+            # Include KDP's reserved location
+            reserved_ids.append(DATA_PORTAL_BASE_INSTANCE_ID)
+        elif syncable == "0":
+            # Include Studio's reserved location
+            reserved_ids.append(CENTRAL_CONTENT_BASE_INSTANCE_ID)
+        if reserved_ids:
+            reserved_queryset = NetworkLocation.objects.filter(
+                id__in=reserved_ids,
+            )
+            return base_queryset | reserved_queryset
+        return base_queryset
+
     def get_object(self, id_filter=None):
         """
         Override get_object to use the unrestricted queryset for the detail view

kolibri/core/discovery/models.py

@@ -131,17 +131,6 @@ def available(self):
 
         return self.connection_status == ConnectionStatus.Okay
 
-    @property
-    def dynamic(self):
-        return self.location_type == LocationTypes.Dynamic
-
-    @dynamic.setter
-    def dynamic(self, value):
-        """
-        TODO: remove this setter once we've migrated to the new location_type field
-        """
-        self.location_type = LocationTypes.Dynamic if value else LocationTypes.Static
-
     @property
     def reserved(self):
         return self.location_type == LocationTypes.Reserved

kolibri/core/discovery/serializers.py

@@ -3,20 +3,19 @@
 from rest_framework.serializers import ValidationError
 
 from .models import ConnectionStatus
-from .models import NetworkLocation
 from .models import PinnedDevice
+from .models import StaticNetworkLocation
 from .utils.network import errors
 from .utils.network.client import NetworkClient
 from kolibri.core.serializers import HexOnlyUUIDField
 
 
 class NetworkLocationSerializer(serializers.ModelSerializer):
     class Meta:
-        model = NetworkLocation
+        model = StaticNetworkLocation
         fields = (
             "id",
             "available",
-            "dynamic",
             "nickname",
             "base_url",
             "device_name",
@@ -30,10 +29,10 @@ class Meta:
             "subset_of_users_device",
             "connection_status",
             "is_local",
+            "location_type",
         )
         read_only_fields = (
             "available",
-            "dynamic",
             "device_name",
             "instance_id",
             "added",
@@ -45,6 +44,7 @@ class Meta:
             "subset_of_users_device",
             "connection_status",
             "is_local",
+            "location_type",
         )
 
     def validate(self, data):

kolibri/core/discovery/tasks.py

@@ -21,6 +21,8 @@
 from kolibri.core.discovery.utils.network.connections import update_network_location
 from kolibri.core.discovery.well_known import CENTRAL_CONTENT_BASE_INSTANCE_ID
 from kolibri.core.discovery.well_known import CENTRAL_CONTENT_BASE_URL
+from kolibri.core.discovery.well_known import DATA_PORTAL_BASE_INSTANCE_ID
+from kolibri.core.discovery.well_known import DATA_PORTAL_SYNCING_BASE_URL
 from kolibri.core.tasks.decorators import register_task
 from kolibri.core.tasks.job import Priority
 from kolibri.core.tasks.main import job_storage
@@ -135,7 +137,10 @@ def _update_connection_status(network_location):
         logger.error(e)
         logger.warning(
             "Failed to update connection status for {} location {}".format(
-                "dynamic" if network_location.dynamic else "static", network_location.id
+                "dynamic"
+                if network_location.location_type is LocationTypes.Dynamic
+                else "static",
+                network_location.id,
             )
         )
 
@@ -184,6 +189,14 @@ def _enqueue_network_location_update_with_backoff(network_location):
     dependent on how many connection faults have occurred
     :type network_location: NetworkLocation
     """
+    # Check if the network location is local before proceeding
+    if not network_location.is_local:
+        logger.info(
+            "Network location {} is not local. Skipping enqueue.".format(
+                network_location.id
+            )
+        )
+        return
     # exponential backoff depending on how many faults/attempts we've had
     next_attempt_minutes = 2 ** network_location.connection_faults
     logger.debug(
@@ -332,17 +345,36 @@ def dispatch_broadcast_hooks(hook_type, instance):
 
 def _refresh_reserved_locations():
     """
-    TODO handle this a bit smarter with: https://github.com/learningequality/kolibri/issues/10431
+    Refreshes the reserved network locations for Studio and Kolibri Data Portal
     """
+    # Delete existing reserved locations
     NetworkLocation.objects.filter(location_type=LocationTypes.Reserved).delete()
-    NetworkLocation.objects.create(
+
+    # Create or update Studio reserved location
+    NetworkLocation.objects.update_or_create(
         id=CENTRAL_CONTENT_BASE_INSTANCE_ID,
-        instance_id=CENTRAL_CONTENT_BASE_INSTANCE_ID,
-        nickname="Kolibri Studio",
-        base_url=CENTRAL_CONTENT_BASE_URL,
-        location_type=LocationTypes.Reserved,
-        is_local=False,
-        kolibri_version="0.16.0",
+        defaults={
+            "instance_id": CENTRAL_CONTENT_BASE_INSTANCE_ID,
+            "nickname": "Kolibri Studio",
+            "base_url": CENTRAL_CONTENT_BASE_URL,
+            "location_type": LocationTypes.Reserved,
+            "is_local": False,
+            "kolibri_version": "0.16.0",
+        },
+    )
+
+    # Create or update Kolibri Data Portal reserved location
+    NetworkLocation.objects.update_or_create(
+        id=DATA_PORTAL_BASE_INSTANCE_ID,
+        defaults={
+            "instance_id": DATA_PORTAL_BASE_INSTANCE_ID,
+            "nickname": "Kolibri Data Portal",
+            "base_url": DATA_PORTAL_SYNCING_BASE_URL,
+            "location_type": LocationTypes.Reserved,
+            "is_local": False,
+            "application": "Kolibri Data Portal",
+            "kolibri_version": "0.16.0",
+        },
     )
 
 

kolibri/core/discovery/test/test_api.py

@@ -18,6 +18,10 @@
 from kolibri.core.auth.test.helpers import provision_device
 from kolibri.core.auth.test.test_api import FacilityFactory
 from kolibri.core.auth.test.test_api import FacilityUserFactory
+from kolibri.core.discovery.well_known import CENTRAL_CONTENT_BASE_INSTANCE_ID
+from kolibri.core.discovery.well_known import CENTRAL_CONTENT_BASE_URL
+from kolibri.core.discovery.well_known import DATA_PORTAL_BASE_INSTANCE_ID
+from kolibri.core.discovery.well_known import DATA_PORTAL_SYNCING_BASE_URL
 
 
 @mock.patch.object(requests.Session, "request", mock_request)
@@ -40,12 +44,37 @@ def setUpTestData(cls):
         cls.existing_sad_netloc = models.NetworkLocation.objects.create(
             base_url="https://sadurl.qqq/"
         )
+        cls.kdp_reserved_location = models.NetworkLocation.objects.create(
+            id=DATA_PORTAL_BASE_INSTANCE_ID,
+            base_url=DATA_PORTAL_SYNCING_BASE_URL,
+            location_type=models.LocationTypes.Reserved,
+        )
+        cls.studio_reserved_location = models.NetworkLocation.objects.create(
+            id=CENTRAL_CONTENT_BASE_INSTANCE_ID,
+            base_url=CENTRAL_CONTENT_BASE_URL,
+            location_type=models.LocationTypes.Reserved,
+        )
+        cls.dynamic_location = models.DynamicNetworkLocation.objects.create(
+            id="a" * 32,
+            base_url="http://dynamiclocation.qqq",
+            instance_id="a" * 32,
+        )
 
     def login(self, user):
         self.client.login(
             username=user.username, password=DUMMY_PASSWORD, facility=user.facility
         )
 
+    def assert_network_location_list(self, syncable_value, expected_ids):
+        params = {"syncable": syncable_value} if syncable_value is not None else {}
+        response = self.client.get(
+            reverse("kolibri:core:networklocation-list"),
+            params,
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        location_ids = [location["id"] for location in response.data]
+        self.assertCountEqual(location_ids, expected_ids)
+
     def test_get__pk(self):
         self.login(self.superuser)
         response = self.client.get(
@@ -132,6 +161,47 @@ def test_reading_network_location_list_filter_soud(self):
         for location in response.data:
             self.assertFalse(location["subset_of_users_device"])
 
+    def test_return_kdp_reserved_location(self):
+        """
+        Tests the API for fetching dynamic, static, and KDP reserved network locations
+        """
+        self.login(self.superuser)
+        expected_ids = [
+            self.existing_happy_netloc.id,
+            self.existing_nonkolibri_netloc.id,
+            self.existing_sad_netloc.id,
+            self.dynamic_location.id,
+            self.kdp_reserved_location.id,
+        ]
+        self.assert_network_location_list("1", expected_ids)
+
+    def test_return_studio_reserved_location(self):
+        """
+        Tests the API for fetching dynamic, static, and Studio reserved network locations
+        """
+        self.login(self.superuser)
+        expected_ids = [
+            self.existing_happy_netloc.id,
+            self.existing_nonkolibri_netloc.id,
+            self.existing_sad_netloc.id,
+            self.dynamic_location.id,
+            self.studio_reserved_location.id,
+        ]
+        self.assert_network_location_list("0", expected_ids)
+
+    def test_return_no_reserved_locations(self):
+        """
+        Tests the API for fetching only dynamic and static network locations
+        """
+        self.login(self.superuser)
+        expected_ids = [
+            self.existing_happy_netloc.id,
+            self.existing_nonkolibri_netloc.id,
+            self.existing_sad_netloc.id,
+            self.dynamic_location.id,
+        ]
+        self.assert_network_location_list(None, expected_ids)
+
 
 class PinnedDeviceAPITestCase(APITestCase):
     databases = "__all__"

kolibri/core/discovery/test/test_connections.py

@@ -2,6 +2,7 @@
 from django.test import TestCase
 
 from ..models import ConnectionStatus
+from ..models import LocationTypes
 from ..models import NetworkLocation
 from ..utils.network import errors
 from ..utils.network.client import NetworkClient
@@ -19,7 +20,6 @@ def setUp(self):
         self.mock_location = mock.MagicMock(
             spec=NetworkLocation(),
             id="mock_location_id",
-            dynamic=False,
             instance_id=None,
             connection_status=ConnectionStatus.Unknown,
             connection_faults=0,
@@ -141,12 +141,12 @@ def test_okay(self):
         self.assertEqual(self.mock_location.connection_faults, 0)
 
     def test_okay__dynamic(self):
-        self.mock_location.dynamic = True
+        self.mock_location.location_type = LocationTypes.Dynamic
         update_network_location(self.mock_location)
         self.assertNotEqual(self.mock_location.last_known_ip, "192.168.101.101")
 
     def test_okay__static(self):
-        self.mock_location.dynamic = False
+        self.mock_location.location_type = LocationTypes.Static
         update_network_location(self.mock_location)
         self.assertEqual(self.mock_location.last_known_ip, "192.168.101.101")
         self.assertTrue(self.mock_location.is_local)

kolibri/core/discovery/test/test_models.py

@@ -28,14 +28,6 @@ def test_property__is_kolibri(self):
         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)

kolibri/core/discovery/test/test_network_utils.py

@@ -4,6 +4,7 @@
 
 import kolibri
 from ..models import ConnectionStatus
+from ..models import LocationTypes
 from ..models import NetworkLocation
 from ..utils.network import errors
 from ..utils.network.client import NetworkClient
@@ -190,7 +191,7 @@ def test_build_for_network_location__previously_not_okay(self):
             spec=NetworkLocation(),
             base_url="url.qqq",
             connection_status=ConnectionStatus.Unknown,
-            dynamic=True,
+            location_type=LocationTypes.Dynamic,
         )
         client = NetworkClient.build_from_network_location(network_loc)
         # should have resolved the base url to something different
@@ -205,7 +206,7 @@ def test_build_for_network_location__failure(self):
             spec=NetworkLocation(),
             base_url="url.qqq",
             connection_status=ConnectionStatus.Unknown,
-            dynamic=True,
+            location_type=LocationTypes.Dynamic,
         )
         with self.assertRaises(errors.NetworkLocationNotFound):
             NetworkClient.build_from_network_location(network_loc)
@@ -218,7 +219,7 @@ def test_build_for_network_location__no_raise(self):
             spec=NetworkLocation(),
             base_url="url.qqq",
             connection_status=ConnectionStatus.ConnectionFailure,
-            dynamic=True,
+            location_type=LocationTypes.Dynamic,
         )
         try:
             NetworkClient.build_from_network_location(network_loc)
@@ -233,7 +234,6 @@ def test_build_for_network_location__static(self):
             spec=NetworkLocation(),
             base_url="url.qqq",
             connection_status=ConnectionStatus.Unknown,
-            dynamic=False,
         )
         try:
             NetworkClient.build_from_network_location(network_loc)