From a41a5fd0606767c94fa2a2603b62dc911527fa83 Mon Sep 17 00:00:00 2001 From: purhan Date: Wed, 27 Jan 2021 20:03:17 +0530 Subject: [PATCH 01/11] [feature] Add geojson endpoints #360 Closes #360 --- openwisp_controller/geo/api/views.py | 56 ++++++++++++++++++++++++++++ openwisp_controller/geo/utils.py | 6 +++ 2 files changed, 62 insertions(+) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index 4a539a3ee..b8542a8e4 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -1,9 +1,16 @@ from django.core.exceptions import ObjectDoesNotExist +from django.db.models import Count +from django.urls import reverse from rest_framework import generics from rest_framework.permissions import BasePermission +from rest_framework.serializers import IntegerField, SerializerMethodField from rest_framework_gis import serializers as gis_serializers +from rest_framework_gis.pagination import GeoJsonPagination from swapper import load_model +from openwisp_users.api.mixins import FilterByOrganizationManaged, FilterByParentManaged +from openwisp_utils.api.serializers import ValidatedModelSerializer + Device = load_model('config', 'Device') Location = load_model('geo', 'Location') DeviceLocation = load_model('geo', 'DeviceLocation') @@ -22,6 +29,28 @@ class Meta: read_only_fields = ('name',) +class DeviceSerializer(ValidatedModelSerializer): + admin_edit_url = SerializerMethodField('get_admin_edit_url') + + def get_admin_edit_url(self, obj): + return self.context['request'].build_absolute_uri( + reverse(f'admin:{obj._meta.app_label}_device_change', args=(obj.id,)) + ) + + class Meta: + model = Device + fields = '__all__' + + +class GeoJsonLocationSerializer(gis_serializers.GeoFeatureModelSerializer): + device_count = IntegerField() + + class Meta: + model = Location + geo_field = 'geometry' + fields = '__all__' + + class DeviceLocationView(generics.RetrieveUpdateAPIView): serializer_class = LocationSerializer permission_classes = (DevicePermission,) @@ -58,4 +87,31 @@ def create_location(self, device): return location +class GeoJsonLocationList(FilterByOrganizationManaged, generics.ListAPIView): + GeoJsonPagination.page_size = 1000 + pagination_class = GeoJsonPagination + queryset = Location.objects.filter(devicelocation__isnull=False).annotate( + device_count=Count('devicelocation') + ) + serializer_class = GeoJsonLocationSerializer + + +class LocationDeviceList(FilterByParentManaged, generics.ListAPIView): + serializer_class = DeviceSerializer + queryset = Device.objects.none() + + def get_parent_queryset(self): + qs = Location.objects.filter(pk=self.kwargs['location_pk']) + return qs + + def get_queryset(self): + super().get_queryset() + qs = Device.objects.filter( + devicelocation__location_id=self.kwargs['location_pk'] + ) + return qs + + device_location = DeviceLocationView.as_view() +geojson = GeoJsonLocationList.as_view() +location_device_list = LocationDeviceList.as_view() diff --git a/openwisp_controller/geo/utils.py b/openwisp_controller/geo/utils.py index 737e32ec0..e65ebc287 100644 --- a/openwisp_controller/geo/utils.py +++ b/openwisp_controller/geo/utils.py @@ -8,4 +8,10 @@ def get_geo_urls(geo_views): geo_views.device_location, name='api_device_location', ), + url(r'^api/v1/device/geojson$', geo_views.geojson, name='api_geojson',), + url( + r'^api/v1/location/(?P[^/]+)/device/$', + geo_views.location_device_list, + name='api_location_device_list', + ), ] From 74553b0bb9ebde0df882c1184a233aed7df310f0 Mon Sep 17 00:00:00 2001 From: purhan Date: Tue, 2 Feb 2021 16:45:48 +0530 Subject: [PATCH 02/11] [qa] Add tests and requested changes --- openwisp_controller/geo/api/views.py | 6 +- openwisp_controller/geo/tests/test_api.py | 83 +++++++++++++++++++++++ openwisp_controller/geo/utils.py | 2 +- 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index b8542a8e4..060fdc803 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -101,14 +101,12 @@ class LocationDeviceList(FilterByParentManaged, generics.ListAPIView): queryset = Device.objects.none() def get_parent_queryset(self): - qs = Location.objects.filter(pk=self.kwargs['location_pk']) + qs = Location.objects.filter(pk=self.kwargs['pk']) return qs def get_queryset(self): super().get_queryset() - qs = Device.objects.filter( - devicelocation__location_id=self.kwargs['location_pk'] - ) + qs = Device.objects.filter(devicelocation__location_id=self.kwargs['pk']) return qs diff --git a/openwisp_controller/geo/tests/test_api.py b/openwisp_controller/geo/tests/test_api.py index 0242c7a66..2379a9c23 100644 --- a/openwisp_controller/geo/tests/test_api.py +++ b/openwisp_controller/geo/tests/test_api.py @@ -5,11 +5,16 @@ from django.urls import reverse from swapper import load_model +from openwisp_controller.config.tests.utils import CreateConfigTemplateMixin +from openwisp_users.tests.utils import TestOrganizationMixin +from openwisp_utils.tests import capture_any_output + from .utils import TestGeoMixin Device = load_model('config', 'Device') Location = load_model('geo', 'Location') DeviceLocation = load_model('geo', 'DeviceLocation') +OrganizationUser = load_model('openwisp_users', 'OrganizationUser') class TestApi(TestGeoMixin, TestCase): @@ -83,3 +88,81 @@ def test_put_update_coordinates(self): }, ) self.assertEqual(self.location_model.objects.count(), 1) + + +class TestMultitenantApi( + TestOrganizationMixin, TestGeoMixin, TestCase, CreateConfigTemplateMixin +): + object_location_model = DeviceLocation + location_model = Location + object_model = Device + + def setUp(self): + super().setUp() + # create 2 orgs + self._create_org(name='org_b', slug='org_b') + org_a = self._create_org(name='org_a', slug='org_a') + # create an operator for org_a + ou = OrganizationUser.objects.create( + user=self._create_operator(), organization=org_a + ) + ou.is_admin = True + ou.save() + # create a superuser + self._create_admin(is_superuser=True,) + + def _create_device_location(self, **kwargs): + options = dict() + options.update(kwargs) + device_location = self.object_location_model(**options) + device_location.full_clean() + device_location.save() + return device_location + + @capture_any_output() + def test_location_device_list(self): + url = 'geo:api_location_device_list' + # create 2 devices and 2 device location for each org + device_a = self._create_device(organization=self._get_org('org_a')) + device_b = self._create_device(organization=self._get_org('org_b')) + location_a = self._create_location(organization=self._get_org('org_a')) + location_b = self._create_location(organization=self._get_org('org_b')) + self._create_device_location(content_object=device_a, location=location_a) + self._create_device_location(content_object=device_b, location=location_b) + + with self.subTest('Test location device list for org operator'): + self.client.login(username='operator', password='tester') + r = self.client.get(reverse(url, args=[location_a.id])) + self.assertContains(r, str(device_a.id)) + r = self.client.get(reverse(url, args=[location_b.id])) + self.assertEqual(r.status_code, 404) + + with self.subTest('Test location device list for org superuser'): + self.client.login(username='admin', password='tester') + r = self.client.get(reverse(url, args=[location_a.id])) + self.assertContains(r, str(device_a.id)) + r = self.client.get(reverse(url, args=[location_b.id])) + self.assertContains(r, str(device_b.id)) + + @capture_any_output() + def test_geojson_list(self): + url = 'geo:api_geojson' + # create 2 devices and 2 device location for each org + device_a = self._create_device(organization=self._get_org('org_a')) + device_b = self._create_device(organization=self._get_org('org_b')) + location_a = self._create_location(organization=self._get_org('org_a')) + location_b = self._create_location(organization=self._get_org('org_b')) + self._create_device_location(content_object=device_a, location=location_a) + self._create_device_location(content_object=device_b, location=location_b) + + with self.subTest('Test geojson list for org operator'): + self.client.login(username='operator', password='tester') + r = self.client.get(reverse(url)) + self.assertContains(r, str(location_a.pk)) + self.assertNotContains(r, str(location_b.pk)) + + with self.subTest('Test geojson list for superuser'): + self.client.login(username='admin', password='tester') + r = self.client.get(reverse(url)) + self.assertContains(r, str(location_a.pk)) + self.assertContains(r, str(location_b.pk)) diff --git a/openwisp_controller/geo/utils.py b/openwisp_controller/geo/utils.py index e65ebc287..2cbc4ccb1 100644 --- a/openwisp_controller/geo/utils.py +++ b/openwisp_controller/geo/utils.py @@ -10,7 +10,7 @@ def get_geo_urls(geo_views): ), url(r'^api/v1/device/geojson$', geo_views.geojson, name='api_geojson',), url( - r'^api/v1/location/(?P[^/]+)/device/$', + r'^api/v1/location/(?P[^/]+)/device/$', geo_views.location_device_list, name='api_location_device_list', ), From cbe0ac3567c8ee274e3f120caf78dd51650f11c7 Mon Sep 17 00:00:00 2001 From: purhan Date: Tue, 2 Feb 2021 21:18:14 +0530 Subject: [PATCH 03/11] [dependency] Update openwisp-users --- requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 5ab97840f..cc0c453f3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,8 @@ django-x509~=0.9.2 django-taggit~=1.3.0 django-loci~=0.4.0 django-flat-json-widget~=0.1.2 -openwisp-users~=0.5.1 +# TODO: change this when next version of openwisp_users is released +openwisp-users @ https://github.com/purhan/openwisp-users/tarball/master openwisp-utils[rest]~=0.7.1 openwisp-notifications~=0.3 djangorestframework-gis>=0.12.0,<0.17.0 From eb234367c56dae5229ff29050c6cd29df69a059e Mon Sep 17 00:00:00 2001 From: purhan Date: Tue, 2 Feb 2021 21:59:58 +0530 Subject: [PATCH 04/11] [change] Add views in sample app to fix CI build --- tests/openwisp2/sample_geo/views.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/openwisp2/sample_geo/views.py b/tests/openwisp2/sample_geo/views.py index ce1b1cada..ec368c9dd 100644 --- a/tests/openwisp2/sample_geo/views.py +++ b/tests/openwisp2/sample_geo/views.py @@ -1,10 +1,26 @@ from openwisp_controller.geo.api.views import ( DeviceLocationView as BaseDeviceLocationView, ) +from openwisp_controller.geo.api.views import ( + GeoJsonLocationList as BaseGeoJsonLocationList, +) +from openwisp_controller.geo.api.views import ( + LocationDeviceList as BaseLocationDeviceList, +) class DeviceLocationView(BaseDeviceLocationView): pass +class GeoJsonLocationList(BaseGeoJsonLocationList): + pass + + +class LocationDeviceList(BaseLocationDeviceList): + pass + + device_location = DeviceLocationView.as_view() +geojson = GeoJsonLocationList.as_view() +location_device_list = LocationDeviceList.as_view() From b019b8b79f72b5b02b694cf144d3b6531f97a919 Mon Sep 17 00:00:00 2001 From: purhan Date: Wed, 3 Feb 2021 11:50:02 +0530 Subject: [PATCH 05/11] [change] Add user authentication --- openwisp_controller/geo/api/views.py | 4 +++- openwisp_controller/geo/tests/test_api.py | 10 ++++++++++ openwisp_controller/geo/utils.py | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index 060fdc803..ebeb06ee1 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -2,7 +2,7 @@ from django.db.models import Count from django.urls import reverse from rest_framework import generics -from rest_framework.permissions import BasePermission +from rest_framework.permissions import BasePermission, IsAuthenticated from rest_framework.serializers import IntegerField, SerializerMethodField from rest_framework_gis import serializers as gis_serializers from rest_framework_gis.pagination import GeoJsonPagination @@ -89,6 +89,7 @@ def create_location(self, device): class GeoJsonLocationList(FilterByOrganizationManaged, generics.ListAPIView): GeoJsonPagination.page_size = 1000 + permission_classes = (IsAuthenticated,) pagination_class = GeoJsonPagination queryset = Location.objects.filter(devicelocation__isnull=False).annotate( device_count=Count('devicelocation') @@ -98,6 +99,7 @@ class GeoJsonLocationList(FilterByOrganizationManaged, generics.ListAPIView): class LocationDeviceList(FilterByParentManaged, generics.ListAPIView): serializer_class = DeviceSerializer + permission_classes = (IsAuthenticated,) queryset = Device.objects.none() def get_parent_queryset(self): diff --git a/openwisp_controller/geo/tests/test_api.py b/openwisp_controller/geo/tests/test_api.py index 2379a9c23..b7c5cedde 100644 --- a/openwisp_controller/geo/tests/test_api.py +++ b/openwisp_controller/geo/tests/test_api.py @@ -144,6 +144,11 @@ def test_location_device_list(self): r = self.client.get(reverse(url, args=[location_b.id])) self.assertContains(r, str(device_b.id)) + with self.subTest('Test location device list for unauthenticated user'): + self.client.logout() + r = self.client.get(reverse(url, args=[location_a.id])) + self.assertEqual(r.status_code, 403) + @capture_any_output() def test_geojson_list(self): url = 'geo:api_geojson' @@ -166,3 +171,8 @@ def test_geojson_list(self): r = self.client.get(reverse(url)) self.assertContains(r, str(location_a.pk)) self.assertContains(r, str(location_b.pk)) + + with self.subTest('Test geojson list unauthenticated user'): + self.client.logout() + r = self.client.get(reverse(url)) + self.assertEqual(r.status_code, 403) diff --git a/openwisp_controller/geo/utils.py b/openwisp_controller/geo/utils.py index 2cbc4ccb1..f52b4c8a2 100644 --- a/openwisp_controller/geo/utils.py +++ b/openwisp_controller/geo/utils.py @@ -8,7 +8,7 @@ def get_geo_urls(geo_views): geo_views.device_location, name='api_device_location', ), - url(r'^api/v1/device/geojson$', geo_views.geojson, name='api_geojson',), + url(r'^api/v1/device/geojson/$', geo_views.geojson, name='api_geojson',), url( r'^api/v1/location/(?P[^/]+)/device/$', geo_views.location_device_list, From 71d5b36f29f8b32be77ea16ceb5a9c85a6b27491 Mon Sep 17 00:00:00 2001 From: purhan Date: Tue, 9 Feb 2021 20:56:41 +0530 Subject: [PATCH 06/11] [change] Update docs --- README.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.rst b/README.rst index 8623852bd..66f9fbabd 100755 --- a/README.rst +++ b/README.rst @@ -170,6 +170,19 @@ their coordinates. See below for further details: GET /api/v1/device/{id}/location/ PUT /api/v1/device/{id}/location/ +If a location has multiple devices, then the list of all such devices and related +information can be accessed via the following endpoint: + +.. code-block:: text + + GET /api/v1/location/{pk}/device/ + +To access all the locations that have devices deployed in them, use the following +endpoint, which provides a paginated list of such locations in GeoJSON format: + +.. code-block:: text + + GET /api/v1/device/geojson Settings -------- From b3d7dcf2bed675f844a7b87f6d9e5828af41d09a Mon Sep 17 00:00:00 2001 From: purhan Date: Sat, 13 Feb 2021 22:51:05 +0530 Subject: [PATCH 07/11] [change] Add pagination to LocationDeviceList --- openwisp_controller/geo/api/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index ebeb06ee1..3055ba472 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -1,7 +1,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.db.models import Count from django.urls import reverse -from rest_framework import generics +from rest_framework import generics, pagination from rest_framework.permissions import BasePermission, IsAuthenticated from rest_framework.serializers import IntegerField, SerializerMethodField from rest_framework_gis import serializers as gis_serializers @@ -51,6 +51,12 @@ class Meta: fields = '__all__' +class ListViewPagination(pagination.PageNumberPagination): + page_size = 10 + page_size_query_param = 'page_size' + max_page_size = 100 + + class DeviceLocationView(generics.RetrieveUpdateAPIView): serializer_class = LocationSerializer permission_classes = (DevicePermission,) @@ -100,6 +106,7 @@ class GeoJsonLocationList(FilterByOrganizationManaged, generics.ListAPIView): class LocationDeviceList(FilterByParentManaged, generics.ListAPIView): serializer_class = DeviceSerializer permission_classes = (IsAuthenticated,) + pagination_class = ListViewPagination queryset = Device.objects.none() def get_parent_queryset(self): From 5ff56bcb823213c9fe35969b0284a2145220e7a5 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Tue, 16 Feb 2021 21:12:41 -0500 Subject: [PATCH 08/11] [chores] Minor improvements --- openwisp_controller/geo/api/views.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index 3055ba472..14019242f 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -92,20 +92,21 @@ def create_location(self, device): dl.save() return location + +class GeoJsonLocationListPagination(GeoJsonPagination): + page_size = 1000 + class GeoJsonLocationList(FilterByOrganizationManaged, generics.ListAPIView): - GeoJsonPagination.page_size = 1000 - permission_classes = (IsAuthenticated,) - pagination_class = GeoJsonPagination queryset = Location.objects.filter(devicelocation__isnull=False).annotate( device_count=Count('devicelocation') ) serializer_class = GeoJsonLocationSerializer + pagination_class = GeoJsonLocationListPagination class LocationDeviceList(FilterByParentManaged, generics.ListAPIView): serializer_class = DeviceSerializer - permission_classes = (IsAuthenticated,) pagination_class = ListViewPagination queryset = Device.objects.none() From f24620698d27c6c9aadf6e9dd8c83eb9bdd83799 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Tue, 16 Feb 2021 21:16:13 -0500 Subject: [PATCH 09/11] [chores] Fixed whitespace --- openwisp_controller/geo/api/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index 14019242f..839d85c4a 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -92,7 +92,7 @@ def create_location(self, device): dl.save() return location - + class GeoJsonLocationListPagination(GeoJsonPagination): page_size = 1000 From 243dce3e76026dffbefa22de1662c510ab425a64 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Tue, 16 Feb 2021 21:20:33 -0500 Subject: [PATCH 10/11] [chores] Fixed qa --- openwisp_controller/geo/api/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openwisp_controller/geo/api/views.py b/openwisp_controller/geo/api/views.py index 839d85c4a..7f19b9070 100644 --- a/openwisp_controller/geo/api/views.py +++ b/openwisp_controller/geo/api/views.py @@ -2,7 +2,7 @@ from django.db.models import Count from django.urls import reverse from rest_framework import generics, pagination -from rest_framework.permissions import BasePermission, IsAuthenticated +from rest_framework.permissions import BasePermission from rest_framework.serializers import IntegerField, SerializerMethodField from rest_framework_gis import serializers as gis_serializers from rest_framework_gis.pagination import GeoJsonPagination From 771b14ed9fc90a6b48e7860d5630a6ddc3d72144 Mon Sep 17 00:00:00 2001 From: purhan Date: Wed, 17 Feb 2021 09:40:45 +0530 Subject: [PATCH 11/11] [chores] Minor improvements --- README.rst | 2 +- requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 66f9fbabd..817554ea8 100755 --- a/README.rst +++ b/README.rst @@ -182,7 +182,7 @@ endpoint, which provides a paginated list of such locations in GeoJSON format: .. code-block:: text - GET /api/v1/device/geojson + GET /api/v1/device/geojson/ Settings -------- diff --git a/requirements.txt b/requirements.txt index cc0c453f3..97d052a02 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ django-taggit~=1.3.0 django-loci~=0.4.0 django-flat-json-widget~=0.1.2 # TODO: change this when next version of openwisp_users is released -openwisp-users @ https://github.com/purhan/openwisp-users/tarball/master +openwisp-users @ https://github.com/openwisp/openwisp-users/tarball/master openwisp-utils[rest]~=0.7.1 openwisp-notifications~=0.3 djangorestframework-gis>=0.12.0,<0.17.0