Merge pull request #87 from OpenGeoscience/testing-infrastructure

Improve Testing Infrastructure

.github/workflows/nightly_ci.yml

@@ -0,0 +1,53 @@
+name: nightly-ci
+on:
+  schedule:
+    # Run at 1:14 am every night, to avoid high load at common schedule times.
+    - cron: "14 1 * * *"
+
+jobs:
+  test:
+    runs-on: ubuntu-22.04
+    services:
+      postgres:
+        image: postgis/postgis:14-3.3
+        env:
+          POSTGRES_DB: django
+          POSTGRES_PASSWORD: postgres
+        ports:
+          - 5432:5432
+      rabbitmq:
+        image: rabbitmq:management
+        ports:
+          - 5672:5672
+      minio:
+        image: bitnami/minio:latest
+        env:
+          MINIO_ROOT_USER: minioAccessKey
+          MINIO_ROOT_PASSWORD: minioSecretKey
+        ports:
+          - 9000:9000
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.10"
+      - name: Install tox
+        run: |
+          pip install --upgrade pip
+          pip install tox
+      - name: Install GDAL
+        run: |
+          sudo apt-add-repository ppa:ubuntugis/ppa
+          sudo apt-get update
+          sudo apt-get install gdal-bin libgdal-dev
+          pip install GDAL==`gdal-config --version`
+      - name: Run tests
+        run: |
+          tox -e test -- -k "slow"
+        env:
+          DJANGO_DATABASE_URL: postgres://postgres:postgres@localhost:5432/django
+          DJANGO_MINIO_STORAGE_ENDPOINT: localhost:9000
+          DJANGO_MINIO_STORAGE_ACCESS_KEY: minioAccessKey
+          DJANGO_MINIO_STORAGE_SECRET_KEY: minioSecretKey
+          DJANGO_HOMEPAGE_REDIRECT_URL: http://localhost:8080/

dev/.env.docker-compose

@@ -7,3 +7,4 @@ DJANGO_MINIO_STORAGE_ACCESS_KEY=minioAccessKey
 DJANGO_MINIO_STORAGE_SECRET_KEY=minioSecretKey
 DJANGO_STORAGE_BUCKET_NAME=django-storage
 DJANGO_MINIO_STORAGE_MEDIA_URL=http://localhost:9000/django-storage
+DJANGO_HOMEPAGE_REDIRECT_URL=http://localhost:8080/

dev/.env.docker-compose-native

@@ -6,3 +6,4 @@ DJANGO_MINIO_STORAGE_ENDPOINT=localhost:9000
 DJANGO_MINIO_STORAGE_ACCESS_KEY=minioAccessKey
 DJANGO_MINIO_STORAGE_SECRET_KEY=minioSecretKey
 DJANGO_STORAGE_BUCKET_NAME=django-storage
+DJANGO_HOMEPAGE_REDIRECT_URL=http://localhost:8080/

setup.py

@@ -70,5 +70,11 @@
             'ipython==8.26.0',
             'tox==4.16.0',
         ],
+        'test': [
+            'factory-boy==3.3.1',
+            'pytest==8.3.3',
+            'pytest-django==4.9.0',
+            'pytest-mock==3.14.0',
+        ],
     },
 )

tox.ini

@@ -48,15 +48,10 @@ passenv =
     DJANGO_HOMEPAGE_REDIRECT_URL
 extras =
     dev
-deps =
-    factory-boy
-    pytest
-    pytest-django
-    pytest-factoryboy
-    pytest-mock
+    test
 commands =
     pip install large-image-converter --find-links https://girder.github.io/large_image_wheels
-    pytest {posargs}
+    pytest {posargs:-k "not slow"}
 
 [testenv:check-migrations]
 setenv =
@@ -94,6 +89,8 @@ exclude =
 DJANGO_SETTINGS_MODULE = uvdat.settings
 DJANGO_CONFIGURATION = TestingConfiguration
 addopts = --strict-markers --showlocals --verbose
+markers =
+    slow: mark test as slow
 filterwarnings =
     # https://github.com/jazzband/django-configurations/issues/190
     ignore:the imp module is deprecated in favour of importlib:DeprecationWarning:configurations

uvdat/core/models/project.py

@@ -1,8 +1,10 @@
+import typing
+
 from django.contrib.auth.models import User
 from django.contrib.gis.db import models as geo_models
 from django.db import models, transaction
 from guardian.models import UserObjectPermission
-from guardian.shortcuts import assign_perm
+from guardian.shortcuts import assign_perm, get_users_with_perms
 
 from .dataset import Dataset
 
@@ -13,27 +15,73 @@ class Project(models.Model):
     default_map_zoom = models.FloatField(default=10)
     datasets = models.ManyToManyField(Dataset, blank=True)
 
+    def owner(self) -> User:
+        users = typing.cast(
+            list[User], list(get_users_with_perms(self, only_with_perms_in=['owner']))
+        )
+        if len(users) != 1:
+            raise Exception('Project must have exactly one owner')
+
+        return users[0]
+
+    def collaborators(self) -> list[User]:
+        return typing.cast(
+            list[User], list(get_users_with_perms(self, only_with_perms_in=['collaborator']))
+        )
+
+    def followers(self):
+        return typing.cast(
+            list[User], list(get_users_with_perms(self, only_with_perms_in=['follower']))
+        )
+
     @transaction.atomic()
-    def set_permissions(
-        self,
-        owner: User,
-        collaborator: list[User] | None = None,
-        follower: list[User] | None = None,
-    ):
-        # Delete all existing first
+    def delete_users_perms(self, users: list[User]):
+        """Delete all permissions a user may have on this project."""
+        user_ids = [user.id for user in users]
+        UserObjectPermission.objects.filter(
+            content_type__app_label=self._meta.app_label,
+            content_type__model=self._meta.model_name,
+            object_pk=self.pk,
+            user_id__in=user_ids,
+        ).delete()
+
+    @transaction.atomic()
+    def set_owner(self, user: User):
+        # Remove existing owner
         UserObjectPermission.objects.filter(
             content_type__app_label=self._meta.app_label,
             content_type__model=self._meta.model_name,
             object_pk=self.pk,
+            permission__codename='owner',
         ).delete()
 
-        # Assign new perms
-        assign_perm('owner', owner, self)
-        for user in collaborator or []:
+        # Delete any existing user perms and set owner
+        self.delete_users_perms([user])
+        assign_perm('owner', user, self)
+
+    @transaction.atomic()
+    def add_collaborators(self, users: list[User]):
+        self.delete_users_perms(users)
+        for user in users:
             assign_perm('collaborator', user, self)
-        for user in follower or []:
+
+    @transaction.atomic()
+    def add_followers(self, users: list[User]):
+        self.delete_users_perms(users)
+        for user in users:
             assign_perm('follower', user, self)
 
+    @transaction.atomic()
+    def set_permissions(
+        self,
+        owner: User,
+        collaborator: list[User] | None = None,
+        follower: list[User] | None = None,
+    ):
+        self.set_owner(owner)
+        self.add_collaborators(collaborator or [])
+        self.add_followers(follower or [])
+
     class Meta:
         permissions = [
             ('owner', 'Can read, write, and delete'),

uvdat/core/rest/__init__.py

@@ -1,8 +1,6 @@
 from .chart import ChartViewSet
 from .dataset import DatasetViewSet
-from .file_item import FileItemViewSet
 from .map_layers import RasterMapLayerViewSet, VectorMapLayerViewSet
-from .network import NetworkEdgeViewSet, NetworkNodeViewSet, NetworkViewSet
 from .project import ProjectViewSet
 from .regions import SourceRegionViewSet
 from .simulations import SimulationViewSet
@@ -11,12 +9,8 @@
 __all__ = [
     ProjectViewSet,
     ChartViewSet,
-    FileItemViewSet,
     RasterMapLayerViewSet,
     VectorMapLayerViewSet,
-    NetworkViewSet,
-    NetworkNodeViewSet,
-    NetworkEdgeViewSet,
     DatasetViewSet,
     SourceRegionViewSet,
     SimulationViewSet,

uvdat/core/rest/access_control.py

@@ -8,12 +8,16 @@
 from uvdat.core.models.project import Project
 
 
-# TODO: Dataset permissions should be separated from Project permissions
 def filter_queryset_by_projects(queryset: QuerySet[Model], projects: QuerySet[models.Project]):
     model = queryset.model
+
+    # Dataset permissions not yet implemented, and as such, all datasets are visible to all users
+    if model == models.Dataset:
+        return queryset
+
     if model == models.Project:
         return queryset.filter(id__in=projects.values_list('id', flat=True))
-    if model in [models.Dataset, models.Chart, models.SimulationResult]:
+    if model in [models.Chart, models.SimulationResult]:
         return queryset.filter(project__in=projects)
     if model in [
         models.FileItem,

uvdat/core/rest/dataset.py

@@ -1,9 +1,8 @@
-import json
-
-from django.http import HttpResponse
+from drf_yasg.utils import swagger_auto_schema
+from rest_framework import serializers
 from rest_framework.decorators import action
 from rest_framework.response import Response
-from rest_framework.viewsets import ModelViewSet
+from rest_framework.viewsets import ReadOnlyModelViewSet
 
 from uvdat.core.models import Dataset, Network, NetworkEdge, NetworkNode
 from uvdat.core.rest.access_control import GuardianFilter, GuardianPermission
@@ -17,7 +16,12 @@
 from uvdat.core.tasks.chart import add_gcc_chart_datum
 
 
-class DatasetViewSet(ModelViewSet):
+class GCCQueryParamSerializer(serializers.Serializer):
+    project = serializers.IntegerField()
+    exclude_nodes = serializers.RegexField(r'^\d+(,\s?\d+)*$')
+
+
+class DatasetViewSet(ReadOnlyModelViewSet):
     queryset = Dataset.objects.all()
     serializer_class = DatasetSerializer
     permission_classes = [GuardianPermission]
@@ -49,12 +53,6 @@ def map_layers(self, request, **kwargs):
         # Return response with rendered data
         return Response(serializer.data, status=200)
 
-    @action(detail=True, methods=['get'])
-    def convert(self, request, **kwargs):
-        dataset = self.get_object()
-        dataset.spawn_conversion_task()
-        return HttpResponse(status=200)
-
     @action(detail=True, methods=['get'])
     def network(self, request, **kwargs):
         dataset = self.get_object()
@@ -72,15 +70,21 @@ def network(self, request, **kwargs):
                     ],
                 }
             )
-        return HttpResponse(json.dumps(networks), status=200)
+        return Response(networks, status=200)
 
+    @swagger_auto_schema(query_serializer=GCCQueryParamSerializer)
     @action(detail=True, methods=['get'])
     def gcc(self, request, **kwargs):
         dataset = self.get_object()
-        project_id = request.query_params.get('project')
-        exclude_nodes = request.query_params.get('exclude_nodes', [])
-        exclude_nodes = exclude_nodes.split(',')
-        exclude_nodes = [int(n) for n in exclude_nodes if len(n)]
+
+        # Validate and de-serialize query params
+        serializer = GCCQueryParamSerializer(data=request.query_params)
+        serializer.is_valid(raise_exception=True)
+        project_id = serializer.validated_data['project']
+        exclude_nodes = [int(n) for n in serializer.validated_data['exclude_nodes'].split(',')]
+
+        if not dataset.networks.exists():
+            return Response(data='No networks exist in selected dataset', status=400)
 
         # Find the GCC for each network in the dataset
         network_gccs: list[list[int]] = []

uvdat/core/rest/map_layers.py

@@ -3,9 +3,10 @@
 from django.db import connection
 from django.http import HttpResponse
 from django_large_image.rest import LargeImageFileDetailMixin
+from rest_framework import mixins
 from rest_framework.decorators import action
 from rest_framework.response import Response
-from rest_framework.viewsets import ModelViewSet
+from rest_framework.viewsets import GenericViewSet
 
 from uvdat.core.models import RasterMapLayer, VectorMapLayer
 from uvdat.core.rest.access_control import GuardianFilter, GuardianPermission
@@ -70,7 +71,7 @@
 """
 
 
-class RasterMapLayerViewSet(ModelViewSet, LargeImageFileDetailMixin):
+class RasterMapLayerViewSet(GenericViewSet, mixins.RetrieveModelMixin, LargeImageFileDetailMixin):
     queryset = RasterMapLayer.objects.select_related('dataset').all()
     serializer_class = RasterMapLayerSerializer
     permission_classes = [GuardianPermission]
@@ -90,7 +91,7 @@ def get_raster_data(self, request, resolution: str = '1', **kwargs):
         return HttpResponse(json.dumps(data), status=200)
 
 
-class VectorMapLayerViewSet(ModelViewSet):
+class VectorMapLayerViewSet(GenericViewSet, mixins.RetrieveModelMixin):
     queryset = VectorMapLayer.objects.select_related('dataset').all()
     serializer_class = VectorMapLayerSerializer
     permission_classes = [GuardianPermission]