diff --git a/kolibri/core/discovery/api.py b/kolibri/core/discovery/api.py index 79d0f9806f7..2c490d3a5cf 100644 --- a/kolibri/core/discovery/api.py +++ b/kolibri/core/discovery/api.py @@ -20,13 +20,14 @@ 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", @@ -34,6 +35,25 @@ class NetworkLocationViewSet(viewsets.ModelViewSet): "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 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 diff --git a/kolibri/core/discovery/serializers.py b/kolibri/core/discovery/serializers.py index cea9109d267..d2db70c0361 100644 --- a/kolibri/core/discovery/serializers.py +++ b/kolibri/core/discovery/serializers.py @@ -3,8 +3,8 @@ 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 @@ -12,11 +12,10 @@ 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): diff --git a/kolibri/core/discovery/tasks.py b/kolibri/core/discovery/tasks.py index 8937177a798..3e90d0ead7c 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 @@ -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", + }, ) 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__" 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/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 + ) + ) 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 diff --git a/kolibri/core/discovery/well_known.py b/kolibri/core/discovery/well_known.py index aa67aab2564..73019987b87 100644 --- a/kolibri/core/discovery/well_known.py +++ b/kolibri/core/discovery/well_known.py @@ -7,3 +7,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_BASE_INSTANCE_ID = "2a824768819aa2bec5cecbc06a31ec1e"