Reserved network locations for Studio and KDP

kolibri/core/discovery/api.py

@@ -1,3 +1,4 @@
+from django.db.models import Q
 from django_filters.rest_framework import DjangoFilterBackend
 from rest_framework import decorators
 from rest_framework import viewsets
@@ -20,6 +21,8 @@
 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
 
 
@@ -34,6 +37,27 @@ class NetworkLocationViewSet(viewsets.ModelViewSet):
         "instance_id",
     ]
 
+    def get_queryset(self):
+        syncable = self.request.query_params.get("syncable", None)
+        if syncable == "1":
+            # Include KDP's reserved location
+            queryset = NetworkLocation.objects.filter(
+                Q(location_type=LocationTypes.Static)
+                | Q(id=DATA_PORTAL_BASE_INSTANCE_ID)
+            )
+        elif syncable == "0":
+            # Include Studio's reserved location
+            queryset = NetworkLocation.objects.filter(
+                Q(location_type=LocationTypes.Static)
+                | Q(id=CENTRAL_CONTENT_BASE_INSTANCE_ID)
+            )
+        else:
+            # Exclude both KDP and Studio
+            queryset = NetworkLocation.objects.filter(
+                location_type=LocationTypes.Static
+            )
+        return queryset
+
     def get_object(self, id_filter=None):
         """
         Override get_object to use the unrestricted queryset for the detail view
@@ -47,6 +71,7 @@ def get_object(self, id_filter=None):
         for filter_key in ("id", "instance_id"):
             try:
                 obj = queryset.get(**{filter_key: id_filter})
+                obj.dynamic = obj.location_type == LocationTypes.Dynamic
                 break
             except NetworkLocation.DoesNotExist:
                 pass

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,6 +3,7 @@
 from rest_framework.serializers import ValidationError
 
 from .models import ConnectionStatus
+from .models import LocationTypes
 from .models import NetworkLocation
 from .models import PinnedDevice
 from .utils.network import errors
@@ -16,7 +17,6 @@ class Meta:
         fields = (
             "id",
             "available",
-            "dynamic",
             "nickname",
             "base_url",
             "device_name",
@@ -30,10 +30,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 +45,7 @@ class Meta:
             "subset_of_users_device",
             "connection_status",
             "is_local",
+            "location_type",
         )
 
     def validate(self, data):
@@ -64,6 +65,11 @@ def validate(self, data):
         data.update(info)
         return super(NetworkLocationSerializer, self).validate(data)
 
+    def create(self, validated_data):
+        # Ensure 'location_type' is set to 'static' when creating new instances
+        validated_data["location_type"] = LocationTypes.Static
+        return super(NetworkLocationSerializer, self).create(validated_data)
+
 
 class PinnedDeviceSerializer(ModelSerializer):
     """

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",
+        },
     )
 
 
@@ -372,11 +404,21 @@ def reset_connection_states(broadcast_id):
         connection_faults=0,
     )
 
-    # enqueue update tasks for all static locations
-    for static_location_id in StaticNetworkLocation.objects.all().values_list(
-        "id", flat=True
+    # enqueue update tasks for all static locations except KDP
+    for static_location_id in (
+        StaticNetworkLocation.objects.all()
+        .values_list("id", flat=True)
+        .exclude(id=DATA_PORTAL_BASE_INSTANCE_ID)
     ):
         perform_network_location_update.enqueue(
             job_id=generate_job_id(TYPE_CONNECT, static_location_id),
             args=(static_location_id,),
         )
+
+    # For KDP, set the application to 'Kolibri Data Portal' without enqueuing update task
+    kdp_location = NetworkLocation.objects.filter(
+        id=DATA_PORTAL_BASE_INSTANCE_ID
+    ).first()
+    if kdp_location:
+        kdp_location.application = "Kolibri Data Portal"
+        kdp_location.save()

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)

kolibri/core/discovery/test/test_tasks.py

@@ -7,6 +7,7 @@
 
 from ..models import ConnectionStatus
 from ..models import DynamicNetworkLocation
+from ..models import LocationTypes
 from ..models import NetworkLocation
 from ..models import StaticNetworkLocation
 from ..tasks import _dispatch_discovery_hooks
@@ -183,7 +184,7 @@ def setUp(self):
             kolibri_version="0.15.11",
             instance_id=mock_device_info.get("instance_id"),
             subset_of_users_device=False,
-            dynamic=True,
+            location_type=LocationTypes.Dynamic,
         )
         self.task = unwrap(unwrap(remove_dynamic_network_location))
 
@@ -194,7 +195,7 @@ def test_not_found(self, mock_dispatch):
 
     @mock.patch("kolibri.core.discovery.tasks._dispatch_discovery_hooks")
     def test_static_location(self, mock_dispatch):
-        self.network_location.dynamic = False
+        self.network_location.location_type = LocationTypes.Static
         self.network_location.save()
         self.task(self.broadcast_id, self.instance)
         mock_dispatch.assert_not_called()
@@ -479,3 +480,22 @@ def test_enqueue_network_location_update_with_backoff__non_zero_faults(
             next_attempt,
             priority=Priority.LOW,
         )
+
+    @mock.patch("kolibri.core.discovery.tasks.get_current_job")
+    def test_enqueue_network_location_update_with_backoff__not_local(
+        self, mock_get_current_job
+    ):
+        current_job_mock = mock.MagicMock()
+        mock_get_current_job.return_value = current_job_mock
+        self.network_location.is_local = False
+
+        with mock.patch("kolibri.core.discovery.tasks.logger") as mock_logger:
+            _enqueue_network_location_update_with_backoff(self.network_location)
+            # 'retry_in' should not be called since is_local is False
+            current_job_mock.retry_in.assert_not_called()
+            # Verify the function logged the appropriate message
+            mock_logger.info.assert_called_once_with(
+                "Network location {} is not local. Skipping enqueue.".format(
+                    self.network_location.id
+                )
+            )

kolibri/core/discovery/utils/network/client.py

@@ -9,6 +9,7 @@
 from .urls import HTTP_PORTS
 from .urls import HTTPS_PORTS
 from kolibri.core.discovery.models import ConnectionStatus
+from kolibri.core.discovery.models import LocationTypes
 from kolibri.core.tasks.utils import get_current_job
 from kolibri.core.utils.urls import join_url
 from kolibri.utils.server import get_urls
@@ -98,7 +99,7 @@ def build_from_network_location(cls, network_location, timeout=None):
         # expect that static network locations have an exact base_url, and only try different
         # variations if we haven't already
         if (
-            network_location.dynamic
+            network_location.location_type is LocationTypes.Dynamic
             and network_location.connection_status == ConnectionStatus.Unknown
         ):
             return cls.build_for_address(network_location.base_url, timeout=timeout)

kolibri/core/discovery/utils/network/connections.py

@@ -7,6 +7,7 @@
 from .client import NetworkClient
 from .urls import parse_address_into_components
 from kolibri.core.discovery.models import ConnectionStatus
+from kolibri.core.discovery.models import LocationTypes
 from kolibri.core.discovery.models import NetworkLocation
 
 
@@ -48,7 +49,7 @@ def capture_network_state(network_location, client):
     # having validated the base URL, we can save that
     network_location.base_url = client.base_url
     # save the IP address for static locations
-    if not network_location.dynamic:
+    if network_location.location_type is not LocationTypes.Dynamic:
         remote_ip = client.remote_ip
 
         network_location.last_known_ip = remote_ip