From 41b09891c0c821d4d885b590f174e3808ce3aff2 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:52:30 -0500 Subject: [PATCH 01/19] Remove dynamic field and its setter after migrating to field location_type --- kolibri/core/discovery/models.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/kolibri/core/discovery/models.py b/kolibri/core/discovery/models.py index ed2b776ae44..1d8e093182f 100644 --- a/kolibri/core/discovery/models.py +++ b/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 From 8b4b872a5f39dbc098a7398221528989e1660e57 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:57:34 -0500 Subject: [PATCH 02/19] =?UTF-8?q?Update=20task=20helper=20to=20prevent=20e?= =?UTF-8?q?nqueuing=20new=20tasks=20when=C2=A0is=5Flocal=20is=20False?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- kolibri/core/discovery/tasks.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/kolibri/core/discovery/tasks.py b/kolibri/core/discovery/tasks.py index 8937177a798..0157b763bb2 100644 --- a/kolibri/core/discovery/tasks.py +++ b/kolibri/core/discovery/tasks.py @@ -184,6 +184,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( From 9704b4a43e33c86cc99c18e19502197a955aa28d Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:02:39 -0500 Subject: [PATCH 03/19] =?UTF-8?q?Use=20hardcoded=20ID=20to=20create=20rese?= =?UTF-8?q?rved=C2=A0network=20locations=C2=A0for=20KDP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- kolibri/core/discovery/well_known.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kolibri/core/discovery/well_known.py b/kolibri/core/discovery/well_known.py index aa67aab2564..4c66f53d311 100644 --- a/kolibri/core/discovery/well_known.py +++ b/kolibri/core/discovery/well_known.py @@ -1,5 +1,6 @@ from le_utils.uuidv5 import generate_ecosystem_namespaced_uuid +from kolibri.core.serializers import HexOnlyUUIDField from kolibri.utils import conf # AKA Kolibri Studio @@ -7,3 +8,7 @@ CENTRAL_CONTENT_BASE_INSTANCE_ID = generate_ecosystem_namespaced_uuid( CENTRAL_CONTENT_BASE_URL ).hex + +# AKA Kolibri Data Portal +DATA_PORTAL_SYNCING_BASE_URL = conf.OPTIONS["Urls"]["DATA_PORTAL_SYNCING_BASE_URL"] +DATA_PORTAL_INSTANCE_ID = HexOnlyUUIDField() From 8a96989c99c0ddf30bdb2e9d632b426d5e383abe Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:04:20 -0500 Subject: [PATCH 04/19] Update reset_connection_states to handle reserved locations for Studio and KDP --- kolibri/core/discovery/tasks.py | 53 ++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/kolibri/core/discovery/tasks.py b/kolibri/core/discovery/tasks.py index 0157b763bb2..05453b08533 100644 --- a/kolibri/core/discovery/tasks.py +++ b/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 @@ -340,17 +342,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", + }, ) @@ -380,11 +401,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() From b9eb8394cf2d0f2bb7c8d498bbd987f7d5f734e2 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:32:15 -0500 Subject: [PATCH 05/19] Update NetworkLocationSerializer to only allow static models to be created and set location_type to static --- kolibri/core/discovery/serializers.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/kolibri/core/discovery/serializers.py b/kolibri/core/discovery/serializers.py index cea9109d267..fabfb6f6a38 100644 --- a/kolibri/core/discovery/serializers.py +++ b/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 @@ -30,6 +31,7 @@ class Meta: "subset_of_users_device", "connection_status", "is_local", + "location_type", ) read_only_fields = ( "available", @@ -45,6 +47,7 @@ class Meta: "subset_of_users_device", "connection_status", "is_local", + "location_type", ) def validate(self, data): @@ -64,6 +67,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): """ From 9976dd65f6812018397424448e4543734c4499f6 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:35:54 -0500 Subject: [PATCH 06/19] Annotate dynamic attribute when location_type is dynamic within NetworkLocationViewSet --- kolibri/core/discovery/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kolibri/core/discovery/api.py b/kolibri/core/discovery/api.py index 79d0f9806f7..a69c9b7e828 100644 --- a/kolibri/core/discovery/api.py +++ b/kolibri/core/discovery/api.py @@ -47,6 +47,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 From 5ffd66c01b35f7ca1ab967141e38707fddc7ef64 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:41:33 -0500 Subject: [PATCH 07/19] Add syncable filter in NetworkLocationViewSet to include KDP and Studio reserved location, excluding both if syncable is not provided --- kolibri/core/discovery/api.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/kolibri/core/discovery/api.py b/kolibri/core/discovery/api.py index a69c9b7e828..442e91db9a6 100644 --- a/kolibri/core/discovery/api.py +++ b/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 From 5698dc948a178c10aacfaaee2d42ea8b62242760 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:58:00 -0500 Subject: [PATCH 08/19] Change DATA_PORTAL_INSTANCE_ID to DATA_PORTAL_BASE_INSTANCE_ID --- kolibri/core/discovery/well_known.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/discovery/well_known.py b/kolibri/core/discovery/well_known.py index 4c66f53d311..ff5b434177a 100644 --- a/kolibri/core/discovery/well_known.py +++ b/kolibri/core/discovery/well_known.py @@ -11,4 +11,4 @@ # AKA Kolibri Data Portal DATA_PORTAL_SYNCING_BASE_URL = conf.OPTIONS["Urls"]["DATA_PORTAL_SYNCING_BASE_URL"] -DATA_PORTAL_INSTANCE_ID = HexOnlyUUIDField() +DATA_PORTAL_BASE_INSTANCE_ID = HexOnlyUUIDField() From 4b785f0c7acec1f288a474b78deea02d18abf685 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:59:28 -0500 Subject: [PATCH 09/19] Add test for enqueue_network_location_update_with_backoff when not_local is false and update previous tests that used dynamic field --- kolibri/core/discovery/test/test_tasks.py | 24 +++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/kolibri/core/discovery/test/test_tasks.py b/kolibri/core/discovery/test/test_tasks.py index 6b5c7570b25..f0e53acbb31 100644 --- a/kolibri/core/discovery/test/test_tasks.py +++ b/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 + ) + ) From c1a5767e5671d49cd490d0d52e3f95064ba0b684 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Fri, 4 Oct 2024 09:04:19 -0500 Subject: [PATCH 10/19] Remove use of dynamic field for NetworkLocation model --- kolibri/core/discovery/serializers.py | 2 -- kolibri/core/discovery/tasks.py | 5 ++++- kolibri/core/discovery/test/test_connections.py | 6 +++--- kolibri/core/discovery/test/test_models.py | 8 -------- kolibri/core/discovery/test/test_network_utils.py | 8 ++++---- kolibri/core/discovery/utils/network/client.py | 3 ++- kolibri/core/discovery/utils/network/connections.py | 3 ++- 7 files changed, 15 insertions(+), 20 deletions(-) diff --git a/kolibri/core/discovery/serializers.py b/kolibri/core/discovery/serializers.py index fabfb6f6a38..b5b6075813d 100644 --- a/kolibri/core/discovery/serializers.py +++ b/kolibri/core/discovery/serializers.py @@ -17,7 +17,6 @@ class Meta: fields = ( "id", "available", - "dynamic", "nickname", "base_url", "device_name", @@ -35,7 +34,6 @@ class Meta: ) read_only_fields = ( "available", - "dynamic", "device_name", "instance_id", "added", diff --git a/kolibri/core/discovery/tasks.py b/kolibri/core/discovery/tasks.py index 05453b08533..bf03eba7a24 100644 --- a/kolibri/core/discovery/tasks.py +++ b/kolibri/core/discovery/tasks.py @@ -137,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, ) ) diff --git a/kolibri/core/discovery/test/test_connections.py b/kolibri/core/discovery/test/test_connections.py index a78f1f4107b..54f820a3940 100644 --- a/kolibri/core/discovery/test/test_connections.py +++ b/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) diff --git a/kolibri/core/discovery/test/test_models.py b/kolibri/core/discovery/test/test_models.py index 0d976c19741..976846072c9 100644 --- a/kolibri/core/discovery/test/test_models.py +++ b/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) diff --git a/kolibri/core/discovery/test/test_network_utils.py b/kolibri/core/discovery/test/test_network_utils.py index 710bced0dc6..d79978d4ef8 100644 --- a/kolibri/core/discovery/test/test_network_utils.py +++ b/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) diff --git a/kolibri/core/discovery/utils/network/client.py b/kolibri/core/discovery/utils/network/client.py index 1b2c31e1abb..7fd2915f194 100644 --- a/kolibri/core/discovery/utils/network/client.py +++ b/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) diff --git a/kolibri/core/discovery/utils/network/connections.py b/kolibri/core/discovery/utils/network/connections.py index e32b8edb278..4a25fee32d8 100644 --- a/kolibri/core/discovery/utils/network/connections.py +++ b/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 From d51a97d139d936bbf9575f0f21c51c14213d7ca3 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:02:23 -0500 Subject: [PATCH 11/19] Remove filtering for KDP network locations before enqueuing --- kolibri/core/discovery/tasks.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/kolibri/core/discovery/tasks.py b/kolibri/core/discovery/tasks.py index bf03eba7a24..27e540af682 100644 --- a/kolibri/core/discovery/tasks.py +++ b/kolibri/core/discovery/tasks.py @@ -404,11 +404,9 @@ def reset_connection_states(broadcast_id): connection_faults=0, ) - # 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) + # enqueue update tasks for all static locations + for static_location_id in StaticNetworkLocation.objects.all().values_list( + "id", flat=True ): perform_network_location_update.enqueue( job_id=generate_job_id(TYPE_CONNECT, static_location_id), From c4830a9a228520e3312f1206f80c6933ad02e1bc Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:19:22 -0500 Subject: [PATCH 12/19] Enforce NetworkLocationSerializer to use StaticNetworkLocation model --- kolibri/core/discovery/serializers.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/kolibri/core/discovery/serializers.py b/kolibri/core/discovery/serializers.py index b5b6075813d..d2db70c0361 100644 --- a/kolibri/core/discovery/serializers.py +++ b/kolibri/core/discovery/serializers.py @@ -3,9 +3,8 @@ from rest_framework.serializers import ValidationError from .models import ConnectionStatus -from .models import LocationTypes -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 @@ -13,7 +12,7 @@ class NetworkLocationSerializer(serializers.ModelSerializer): class Meta: - model = NetworkLocation + model = StaticNetworkLocation fields = ( "id", "available", @@ -65,11 +64,6 @@ 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): """ From 7d0de1b41352b3fa0a6d649196a3326034ff3a07 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:29:13 -0500 Subject: [PATCH 13/19] Add correct KDP instance_id --- kolibri/core/discovery/well_known.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kolibri/core/discovery/well_known.py b/kolibri/core/discovery/well_known.py index ff5b434177a..73019987b87 100644 --- a/kolibri/core/discovery/well_known.py +++ b/kolibri/core/discovery/well_known.py @@ -1,6 +1,5 @@ from le_utils.uuidv5 import generate_ecosystem_namespaced_uuid -from kolibri.core.serializers import HexOnlyUUIDField from kolibri.utils import conf # AKA Kolibri Studio @@ -11,4 +10,4 @@ # AKA Kolibri Data Portal DATA_PORTAL_SYNCING_BASE_URL = conf.OPTIONS["Urls"]["DATA_PORTAL_SYNCING_BASE_URL"] -DATA_PORTAL_BASE_INSTANCE_ID = HexOnlyUUIDField() +DATA_PORTAL_BASE_INSTANCE_ID = "2a824768819aa2bec5cecbc06a31ec1e" From 5dcb4562cb4b87fc9f776319be01259e24d2ed87 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:38:40 -0500 Subject: [PATCH 14/19] Return dynamic locations in addition to static ones --- kolibri/core/discovery/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/core/discovery/api.py b/kolibri/core/discovery/api.py index 442e91db9a6..7fa356c3761 100644 --- a/kolibri/core/discovery/api.py +++ b/kolibri/core/discovery/api.py @@ -54,7 +54,7 @@ def get_queryset(self): else: # Exclude both KDP and Studio queryset = NetworkLocation.objects.filter( - location_type=LocationTypes.Static + location_type__in=[LocationTypes.Static, LocationTypes.Dynamic] ) return queryset From 496947408d73d242fe8b5975df637e163eb11f93 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Wed, 13 Nov 2024 10:21:39 -0600 Subject: [PATCH 15/19] Remove duplicate KDP application setting --- kolibri/core/discovery/tasks.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/kolibri/core/discovery/tasks.py b/kolibri/core/discovery/tasks.py index 27e540af682..3e90d0ead7c 100644 --- a/kolibri/core/discovery/tasks.py +++ b/kolibri/core/discovery/tasks.py @@ -412,11 +412,3 @@ def reset_connection_states(broadcast_id): 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() From 2d2b5afdb1cb24ae6f3ea2de82a1f8bf5082909e Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Wed, 13 Nov 2024 15:38:36 -0600 Subject: [PATCH 16/19] Remove annotated dynamic attribute --- kolibri/core/discovery/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/kolibri/core/discovery/api.py b/kolibri/core/discovery/api.py index 7fa356c3761..8590dc1ff95 100644 --- a/kolibri/core/discovery/api.py +++ b/kolibri/core/discovery/api.py @@ -71,7 +71,6 @@ 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 From 9aa66e133a21370d6d95519f0e8701a734d8b002 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:34:59 -0600 Subject: [PATCH 17/19] Refactor NetworkLocationViewSet to return dynamic locations --- kolibri/core/discovery/api.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/kolibri/core/discovery/api.py b/kolibri/core/discovery/api.py index 8590dc1ff95..7025a60e1cc 100644 --- a/kolibri/core/discovery/api.py +++ b/kolibri/core/discovery/api.py @@ -1,4 +1,3 @@ -from django.db.models import Q from django_filters.rest_framework import DjangoFilterBackend from rest_framework import decorators from rest_framework import viewsets @@ -29,7 +28,6 @@ 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", @@ -39,23 +37,24 @@ class NetworkLocationViewSet(viewsets.ModelViewSet): 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 - queryset = NetworkLocation.objects.filter( - Q(location_type=LocationTypes.Static) - | Q(id=DATA_PORTAL_BASE_INSTANCE_ID) - ) + reserved_ids.append(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) + reserved_ids.append(CENTRAL_CONTENT_BASE_INSTANCE_ID) + if reserved_ids: + reserved_queryset = NetworkLocation.objects.filter( + id__in=reserved_ids, ) + queryset = base_queryset | reserved_queryset else: - # Exclude both KDP and Studio - queryset = NetworkLocation.objects.filter( - location_type__in=[LocationTypes.Static, LocationTypes.Dynamic] - ) + # By default, exclude KDP/Studio reserved locations + queryset = base_queryset return queryset def get_object(self, id_filter=None): From a425e4f62a806adf8c073a2fab48f1e837fb4c66 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:39:11 -0600 Subject: [PATCH 18/19] Remove NetworkLocationViewSet get_queryset redundancy --- kolibri/core/discovery/api.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kolibri/core/discovery/api.py b/kolibri/core/discovery/api.py index 7025a60e1cc..2c490d3a5cf 100644 --- a/kolibri/core/discovery/api.py +++ b/kolibri/core/discovery/api.py @@ -51,11 +51,8 @@ def get_queryset(self): reserved_queryset = NetworkLocation.objects.filter( id__in=reserved_ids, ) - queryset = base_queryset | reserved_queryset - else: - # By default, exclude KDP/Studio reserved locations - queryset = base_queryset - return queryset + return base_queryset | reserved_queryset + return base_queryset def get_object(self, id_filter=None): """ From b97e1d36537ec52a7d7a1a56c5a6d8d3efd8d879 Mon Sep 17 00:00:00 2001 From: Liana Harris <46411498+LianaHarris360@users.noreply.github.com> Date: Wed, 20 Nov 2024 07:36:58 -0600 Subject: [PATCH 19/19] Add API test cases for returned static, dynamic, and reserved network locations --- kolibri/core/discovery/test/test_api.py | 70 +++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/kolibri/core/discovery/test/test_api.py b/kolibri/core/discovery/test/test_api.py index 2ceb684960b..5f7df9b5fbf 100644 --- a/kolibri/core/discovery/test/test_api.py +++ b/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__"