Refactor viewsets for non-detail use case

Author: banesullivan

README.md

@@ -27,9 +27,9 @@ to Django by providing a set of abstract, mixin API viewset classes that will
 handle tile serving, fetching metadata from images, and extracting regions of
 interest.
 
-`django-large-image` is an optionally installable Django app with
+`django-large-image` is an installable Django app with
 a few classes that can be mixed into a Django project (or application)'s
-drf-based views to provide tile serving endpoints out of the box. Notably,
+drf-based viewsets to provide tile serving endpoints out of the box. Notably,
 `django-large-image` is designed to work specifically with `FileField`
 interfaces with development being tailored to Kitware's
 [`S3FileField`](https://github.com/girder/django-s3-file-field). We are working
@@ -89,9 +89,8 @@ pip install \
 
 ## Usage
 
-Simply install the app and mixin the `LargeImageViewSetMixin` class to your
-existing `django-rest-framework` viewsets and specify the `FILE_FIELD_NAME` as
-the string name of the `FileField` in which your image data are saved.
+Simply install the app and mixin one of the mixing classes to your
+existing `django-rest-framework` viewset.
 
 ```py
 # settings.py
@@ -101,11 +100,24 @@ INSTALLED_APPS = [
 ]
 ```
 
+The following are the provided mixin classes and their use case:
+
+- `LargeImageMixin`: for use with a standard, non-detail `ViewSet`. Users must implement `get_path()`
+- `LargeImageDetailMixin`: for use with a detail viewset like `GenericViewSet`. Users must implement `get_path()`
+- `LargeImageFileDetailMixin`: (most commonly used) for use with a detail viewset like `GenericViewSet` where the associated model has a `FileField` storing the image data.
+- `LargeImageVSIFileDetailMixin`: (geospatial) for use with a detail viewset like `GenericViewSet` where the associated model has a `FileField` storing the image data that is intended to be read with GDAL. This will access the data over GDAL's Virtual File System interface (a VSI path).
+
+Most users will want to use `LargeImageFileDetailMixin` and so the following
+example demonstrate how to use it:
+
+Specify the `FILE_FIELD_NAME` as the string name of the `FileField` in which
+your image data are saved on the associated model.
+
 ```py
 # viewsets.py
-from django_large_image.rest import LargeImageViewSetMixin
+from django_large_image.rest import LargeImageFileDetailMixin
 
-class MyModelViewSet(viewsets.GenericViewSet, LargeImageViewSetMixin):
+class MyModelViewSet(viewsets.GenericViewSet, LargeImageFileDetailMixin):
   ...  # configuration for your model's viewset
   FILE_FIELD_NAME = 'field_name'
 ```
@@ -167,13 +179,13 @@ Then create the viewset, mixing in the `django-large-image` viewset class:
 from example.core import models
 from rest_framework import mixins, viewsets
 
-from django_large_image.rest import LargeImageViewSetMixin
+from django_large_image.rest import LargeImageFileDetailMixin
 
 
 class ImageFileDetailViewSet(
     mixins.ListModelMixin,
     viewsets.GenericViewSet,
-    LargeImageViewSetMixin,
+    LargeImageFileDetailMixin,
 ):
     queryset = models.ImageFile.objects.all()
     serializer_class = models.ImageFileSerializer
@@ -222,11 +234,11 @@ repository that shows how to use `django-large-image` in a `girder-4` project.
 
 ### Customization
 
-The `BaseLargeImageViewSetMixin` is modularly designed and able to be subclassed
-for your project's needs. While the provided `LargeImageViewSetMixin` handles
+The mixin classes modularly designed and able to be subclassed
+for your project's needs. While the provided `LargeImageFileDetailMixin` handles
 `FileField`-interfaces, you can easily extend its base class,
-`BaseLargeImageViewSetMixin`, to handle any mechanism of data storage in any
-APIView.
+`LargeImageDetailMixin`, to handle any mechanism of data storage in your
+detail-oriented viewset.
 
 In the following example, I will show how to use GDAL compatible VSI paths
 from a model that stores `s3://` or `https://` URLs.
@@ -254,23 +266,53 @@ class URLImageFileSerializer(serializers.ModelSerializer):
 from example.core import models
 from rest_framework import mixins, viewsets
 
-from django_large_image.rest import BaseLargeImageViewSetMixin
+from django_large_image.rest import LargeImageDetailMixin
 from django_large_image.utilities import make_vsi
 
 
-class URLLargeImageViewSetMixin(BaseLargeImageViewSetMixin):
-    def get_path(self, request, pk):
+class URLLargeImageMixin(LargeImageDetailMixin):
+    def get_path(self, request, pk=None):
         object = self.get_object()
         return make_vsi(object.url)
 
 
 class URLImageFileDetailViewSet(
     mixins.ListModelMixin,
     viewsets.GenericViewSet,
-    URLLargeImageViewSetMixin,
+    URLLargeImageMixin,
 ):
     queryset = models.URLImageFile.objects.all()
     serializer_class = models.URLImageFileSerializer
 ```
 
 Here is a good test image: https://oin-hotosm.s3.amazonaws.com/59c66c5223c8440011d7b1e4/0/7ad397c0-bba2-4f98-a08a-931ec3a6e943.tif
+
+
+#### Non-Detail ViewSets
+
+The `LargeImageMixin` provides a mixin interface for non-detail viewsets (no
+associated model or primary key required). This can be particularly useful if
+your viewset has custom logic to retrieve the desired data.
+
+For example, you may want a viewset that gets the data path as a URL embedded
+in the request's query parameters. To do this, you can make a standard ViewSet
+with the `LargeImageMixin` like so:
+
+```py
+# viewsets.py
+from rest_framework import viewsets
+from rest_framework.exceptions import ValidationError
+
+from django_large_image.rest import LargeImageMixin
+from django_large_image.utilities import make_vsi
+
+
+class URLLargeImageViewSet(viewsets.ViewSet, LargeImageMixin):
+    def get_path(self, request, pk=None):
+        try:
+            url = request.query_params.get('url')
+        except KeyError:
+            raise ValidationError('url must be defined as a query parameter.')
+        return make_vsi(url)
+
+```

django_large_image/rest/__init__.py

@@ -1,85 +1,8 @@
 # flake8: noqa: F401
-from functools import wraps
-
-from rest_framework.exceptions import APIException
-from rest_framework.request import Request
-from rest_framework.viewsets import ViewSet
-
-from django_large_image import utilities
-from django_large_image.rest.data import DataMixin
-from django_large_image.rest.metadata import MetaDataMixin
 from django_large_image.rest.standalone import ListColormapsView, ListTileSourcesView
-from django_large_image.rest.tiles import TilesMixin
-
-
-class BaseLargeImageViewSetMixin(DataMixin, MetaDataMixin, TilesMixin):
-    """Abstract mixin class for large-image endpoints in viewsets.
-
-    Subclasses must implement `get_path()`:
-
-    .. code:: python
-
-        def get_path(self, request: Request, pk: int = None):
-            instance = Model.objects.get(pk=pk)
-            return instance.file.name
-
-    """
-
-    pass
-
-
-class LargeImageViewSet(ViewSet, BaseLargeImageViewSetMixin):
-    """Abstract class for large-image endpoints as a viewset.
-
-    Subclasses must implement `get_path()`:
-
-    .. code:: python
-
-        def get_path(self, request: Request, pk: int = None):
-            instance = Model.objects.get(pk=pk)
-            return instance.file.name
-
-    """
-
-    pass
-
-
-class LargeImageViewSetMixin(BaseLargeImageViewSetMixin):
-    """Mixin specifically for ViewSets that have a file field on the mdoel.
-
-    Define `FILE_FIELD_NAME` as the string name of the file field on the
-    subclassed ViewSet's model.
-
-    This mixin assumes `get_object()` is present - thus for ViewSets
-    """
-
-    FILE_FIELD_NAME: str = None
-
-    def get_field_file(self):
-        """Get `FileField` using `FILE_FIELD_NAME`."""
-        try:
-            return getattr(self.get_object(), self.FILE_FIELD_NAME)
-        except (AttributeError, TypeError):
-            # Raise 500 server error
-            raise APIException('`FILE_FIELD_NAME` not properly set on viewset class.')
-
-    @wraps(BaseLargeImageViewSetMixin.get_path)
-    def get_path(self, request: Request, pk: int = None):
-        return utilities.field_file_to_local_path(self.get_field_file())
-
-
-class LargeImageVSIViewSetMixin(LargeImageViewSetMixin):
-    USE_VSI: bool = True
-
-    @wraps(LargeImageViewSetMixin.get_path)
-    def get_path(self, request: Request, pk: int = None):
-        """Wraps get_path with VSI support."""
-        field_file = self.get_field_file()
-        if self.USE_VSI:
-            with utilities.patch_internal_presign(field_file):
-                # Grab URL and pass back VSI path
-                # DO NOT return here to make sure this context is cleared
-                vsi = utilities.make_vsi(field_file.url)
-            return vsi
-        # Checkout file locally if no VSI
-        return LargeImageViewSetMixin.get_path(self, request, pk)
+from django_large_image.rest.viewsets import (
+    LargeImageDetailMixin,
+    LargeImageFileDetailMixin,
+    LargeImageMixin,
+    LargeImageVSIFileDetailMixin,
+)

django_large_image/rest/base.py

@@ -11,7 +11,7 @@
 CACHE_TIMEOUT = 60 * 60 * 2
 
 
-class LargeImageViewSetMixinBase:
+class LargeImageMixinBase:
     def get_path(self, request: Request, pk: int = None):
         """Return path on disk to image file (or VSI str).
 
@@ -23,7 +23,7 @@ def get_path(self, request: Request, pk: int = None):
         str : The local file path to pass to large_image
 
         """
-        raise NotImplementedError
+        raise NotImplementedError('You must implement `get_path` on your viewset.')
 
     def get_style(self, request: Request):
         data = utilities.get_request_body_as_dict(request)

django_large_image/rest/data.py

@@ -9,10 +9,10 @@
 
 from django_large_image import tilesource
 from django_large_image.rest import params
-from django_large_image.rest.base import CACHE_TIMEOUT, LargeImageViewSetMixinBase
+from django_large_image.rest.base import CACHE_TIMEOUT, LargeImageMixinBase
 
 
-class DataMixin(LargeImageViewSetMixinBase):
+class DataMixin(LargeImageMixinBase):
     def thumbnail(self, request: Request, pk: int = None, format: str = None) -> HttpResponse:
         encoding = tilesource.format_to_encoding(format)
         source = self.get_tile_source(request, pk, encoding=encoding)
@@ -25,7 +25,7 @@ def thumbnail(self, request: Request, pk: int = None, format: str = None) -> Htt
         operation_summary='Returns thumbnail of full image as PNG.',
         manual_parameters=[params.projection] + params.STYLE,
     )
-    @action(detail=True, url_path='thumbnail.png')
+    @action(detail=False, url_path='thumbnail.png')
     def thumbnail_png(self, request: Request, pk: int = None) -> HttpResponse:
         return self.thumbnail(request, pk, format='png')
 
@@ -35,7 +35,7 @@ def thumbnail_png(self, request: Request, pk: int = None) -> HttpResponse:
         operation_summary='Returns thumbnail of full image as JPEG.',
         manual_parameters=[params.projection] + params.STYLE,
     )
-    @action(detail=True, url_path='thumbnail.jpeg')
+    @action(detail=False, url_path='thumbnail.jpeg')
     def thumbnail_jpeg(self, request: Request, pk: int = None) -> HttpResponse:
         return self.thumbnail(request, pk, format='jpeg')
 
@@ -78,10 +78,7 @@ def region(self, request: Request, pk: int = None, format: str = None) -> HttpRe
         operation_summary='Returns region tile binary from world coordinates in given EPSG as a tiled tif image.',
         manual_parameters=[params.projection] + params.REGION,
     )
-    @action(
-        detail=True,
-        url_path=r'region.tif',
-    )
+    @action(detail=False, url_path=r'region.tif')
     def region_tif(self, request: Request, pk: int = None) -> HttpResponse:
         return self.region(request, pk, format='tif')
 
@@ -90,10 +87,7 @@ def region_tif(self, request: Request, pk: int = None) -> HttpResponse:
         operation_summary='Returns region tile binary from world coordinates in given EPSG as a png image.',
         manual_parameters=[params.projection] + params.REGION,
     )
-    @action(
-        detail=True,
-        url_path=r'region.png',
-    )
+    @action(detail=False, url_path=r'region.png')
     def region_png(self, request: Request, pk: int = None) -> HttpResponse:
         return self.region(request, pk, format='png')
 
@@ -102,10 +96,7 @@ def region_png(self, request: Request, pk: int = None) -> HttpResponse:
         operation_summary='Returns region tile binary from world coordinates in given EPSG as a jpeg image.',
         manual_parameters=[params.projection] + params.REGION,
     )
-    @action(
-        detail=True,
-        url_path=r'region.jpeg',
-    )
+    @action(detail=False, url_path=r'region.jpeg')
     def region_jpeg(self, request: Request, pk: int = None) -> HttpResponse:
         return self.region(request, pk, format='jpeg')
 
@@ -114,7 +105,7 @@ def region_jpeg(self, request: Request, pk: int = None) -> HttpResponse:
         operation_summary='Returns single pixel.',
         manual_parameters=[params.projection, params.left, params.top] + params.STYLE,
     )
-    @action(detail=True)
+    @action(detail=False)
     def pixel(self, request: Request, pk: int = None) -> Response:
         left = float(request.query_params.get('left'))
         top = float(request.query_params.get('top'))
@@ -127,7 +118,7 @@ def pixel(self, request: Request, pk: int = None) -> Response:
         operation_summary='Returns histogram',
         manual_parameters=[params.projection] + params.HISTOGRAM,
     )
-    @action(detail=True)
+    @action(detail=False)
     def histogram(self, request: Request, pk: int = None) -> Response:
         kwargs = dict(
             # TODO: add openapi params for these
@@ -147,3 +138,33 @@ def histogram(self, request: Request, pk: int = None) -> Response:
                 if key in entry:
                     entry[key] = float(entry[key])
         return Response(result)
+
+
+class DataDetailMixin(DataMixin):
+    @action(detail=True, url_path='thumbnail.png')
+    def thumbnail_png(self, request: Request, pk: int = None) -> HttpResponse:
+        return super().thumbnail_png(request, pk)
+
+    @action(detail=True, url_path='thumbnail.jpeg')
+    def thumbnail_jpeg(self, request: Request, pk: int = None) -> HttpResponse:
+        return super().thumbnail_jpeg(request, pk)
+
+    @action(detail=True, url_path=r'region.tif')
+    def region_tif(self, request: Request, pk: int = None) -> HttpResponse:
+        return super().region_tif(request, pk)
+
+    @action(detail=True, url_path=r'region.png')
+    def region_png(self, request: Request, pk: int = None) -> HttpResponse:
+        return super().region_png(request, pk)
+
+    @action(detail=True, url_path=r'region.jpeg')
+    def region_jpeg(self, request: Request, pk: int = None) -> HttpResponse:
+        return super().region_jpeg(request, pk)
+
+    @action(detail=True)
+    def pixel(self, request: Request, pk: int = None) -> Response:
+        return super().pixel(request, pk)
+
+    @action(detail=True)
+    def histogram(self, request: Request, pk: int = None) -> Response:
+        return super().histogram(request, pk)