fix: Return {} if no dataset is found by name, instead of HTTP 404 (w…

…as ObjectDoesNotExist)

- Return HTTP 400 Bad Request for missing request parameters when creating datasets
- Return HTTP 404 Not Found on status action for missing dataset, instead of returning {}
- Return HTTP 404 Not Found on metadata action, instead of raising ObjectDoesNotExist
- Return HTTP 404 Not Found on coverage action, instead of returning {}

backend/processor/views.py

@@ -6,7 +6,7 @@
 from django.views.decorators.http import require_POST
 from dqt.models import Dataset, FieldLevelCheck, ProgressMonitorDataset
 from psycopg2.sql import SQL, Identifier
-from rest_framework import status, viewsets
+from rest_framework import serializers, status, viewsets
 from rest_framework.decorators import action
 from rest_framework.response import Response
 
@@ -21,44 +21,87 @@ def create_dataset_filter(request):
     return JsonResponse({"status": "ok"})
 
 
+class CreateDatasetSerializer(serializers.Serializer):
+    name = serializers.CharField()
+    collection_id = serializers.IntegerField()
+
+
+class DeleteDatasetSerializer(serializers.Serializer):
+    dataset_id = serializers.IntegerField()
+
+
 class DatasetViewSet(viewsets.GenericViewSet):
     queryset = Dataset.objects.all()
     # ViewSet's don't allow typed paths like <int:pk>.
     # https://github.com/encode/django-rest-framework/pull/6789
     # https://github.com/encode/django-rest-framework/issues/6148#issuecomment-725297421
     lookup_value_regex = "[0-9]+"
 
+    def get_serializer_class(self):
+        if self.request.method == "DELETE":
+            return DeleteDatasetSerializer
+        return CreateDatasetSerializer
+
     def create(self, request):
-        message = {"name": request.data.get("name"), "collection_id": request.data.get("collection_id")}
-        publish(json.dumps(message), "ocds_kingfisher_extractor_init")
+        """
+        Publishes a message to RabbitMQ to create the dataset with the given ``name`` and ``collection_id``.
+        """
+        serializer = self.get_serializer(
+            data={"name": request.data.get("name"), "collection_id": request.data.get("collection_id")}
+        )
+        serializer.is_valid(raise_exception=True)
+        publish(json.dumps(serializer.data), "ocds_kingfisher_extractor_init")
         return Response(status=status.HTTP_202_ACCEPTED)
 
     def destroy(self, request, pk=None):
-        message = {"dataset_id": int(pk)}
-        publish(json.dumps(message), "wiper_init")
+        """
+        Publishes a message to RabbitMQ to wipe the dataset.
+        """
+        serializer = self.get_serializer(data={"dataset_id": pk})
+        serializer.is_valid(raise_exception=True)
+        publish(json.dumps(serializer.data), "wiper_init")
         return Response(status=status.HTTP_202_ACCEPTED)
 
     @action(detail=False)
     def find_by_name(self, request):
-        dataset = get_object_or_404(self.get_queryset(), name=request.GET.get("name"))
-        return Response({"id": dataset.id})
+        """
+        Returns the ID of the dataset with the ``name`` given in the query string, as an object like ``{"id": 123}``,
+        or ``{}`` if no name matches.
+        """
+        try:
+            dataset = self.get_queryset().get(name=request.GET.get("name"))
+            return Response({"id": dataset.pk})
+        except Dataset.DoesNotExist:
+            return Response({})
 
     @action(detail=True)
     def status(self, request, pk=None):
+        """
+        Returns the dataset's status, or ``{}`` if no status is set.
+        """
+        self.get_object()  # trigger 404 if no dataset
         try:
             monitor = ProgressMonitorDataset.objects.values("state", "phase").get(dataset__pk=pk)
+            return Response(monitor)
         except ProgressMonitorDataset.DoesNotExist:
-            monitor = {}
-        return Response(monitor)
+            return Response({})
 
     @action(detail=True)
     def metadata(self, request, pk=None):
-        meta = self.get_queryset().values_list("meta__collection_metadata", flat=True).get(pk=pk)
+        """
+        Returns the dataset's collection metadata.
+        """
+        meta = get_object_or_404(self.get_queryset().values_list("meta__collection_metadata", flat=True), pk=pk)
         return Response(meta or {})
 
     @action(detail=True)
     def coverage(self, request, pk=None):
-        map = {
+        """
+        Returns the dataset's coverage statistics.
+        """
+        self.get_object()  # trigger 404 if no dataset
+
+        mapping = {
             "parties": ["parties.id"],
             "plannings": ["planning.budget"],
             "tenders": ["tender.id"],
@@ -83,7 +126,11 @@ def coverage(self, request, pk=None):
                 "contracts.milestones.id",
                 "contracts.implementation.milestones.id",
             ],
-            "amendments": ["tender.amendments.id", "awards.amendments.id", "contract.amendments.id"],
+            "amendments": [
+                "tender.amendments.id",
+                "awards.amendments.id",
+                "contract.amendments.id",
+            ],
         }
 
         with connections["data"].cursor() as cursor:
@@ -98,13 +145,13 @@ def coverage(self, request, pk=None):
 
             cursor.execute(
                 SQL(statement).format(table=Identifier(FieldLevelCheck._meta.db_table)),
-                {"checks": tuple(j for i in map.values() for j in i), "dataset_id": pk},
+                {"checks": tuple(j for i in mapping.values() for j in i), "dataset_id": pk},
             )
 
             results = cursor.fetchall()
 
             counts = {}
-            for key, items in map.items():
+            for key, items in mapping.items():
                 counts[key] = 0
                 for i in items:
                     for r in results:

backend/tests/processor/test_views.py

@@ -32,6 +32,13 @@ def test_require_POST(self):
 
                 self.assertEqual(response.status_code, 405)
 
+    @patch("processor.views.publish")
+    def test_create_no_values(self, publish):
+        response = self.client.post("/datasets/", {}, "application/json")
+
+        self.assertEqual(response.status_code, 400)
+        publish.assert_not_called()
+
     @patch("processor.views.publish")
     def test_create(self, publish):
         response = self.client.post(
@@ -60,25 +67,36 @@ def test_destroy(self, publish):
         self.assertEqual(response.status_code, 202)
         publish.assert_called_once_with('{"dataset_id": 123}', "wiper_init")
 
+    def test_find_by_name_no_name(self):
+        self.create(Dataset, name="anything")
+        response = self.client.get("/datasets/find_by_name/")
+
+        self.assertEqual(response.status_code, 200)
+        self.assertJSONEqual(response.content, {})
+
+    def test_find_by_name_no_match(self):
+        self.create(Dataset, name="anything")
+        response = self.client.get("/datasets/find_by_name/?name=other")
+
+        self.assertEqual(response.status_code, 200)
+        self.assertJSONEqual(response.content, {})
+
     def test_find_by_name(self):
         dataset = self.create(Dataset, name="anything")
-
         response = self.client.get("/datasets/find_by_name/?name=anything")
 
         self.assertEqual(response.status_code, 200)
         self.assertJSONEqual(response.content, {"id": dataset.pk})
 
-    def test_metadata_no_meta(self):
+    def test_metadata_no_values(self):
         dataset = self.create(Dataset, name="anything")
-
         response = self.client.get(f"/datasets/{dataset.pk}/metadata/")
 
         self.assertEqual(response.status_code, 200)
         self.assertJSONEqual(response.content, {})
 
     def test_metadata(self):
         dataset = self.create(Dataset, name="anything", meta={"collection_metadata": {"ocid_prefix": "ocds-213czf"}})
-
         response = self.client.get(f"/datasets/{dataset.pk}/metadata/")
 
         self.assertEqual(response.status_code, 200)
@@ -112,15 +130,8 @@ def test_coverage(self):
             },
         )
 
-    def test_status_no_dataset(self):
-        response = self.client.get("/datasets/123/status/")
-
-        self.assertEqual(response.status_code, 200)
-        self.assertJSONEqual(response.content, {})
-
     def test_status_no_progress(self):
         dataset = self.create(Dataset, name="anything")
-
         response = self.client.get(f"/datasets/{dataset.pk}/status/")
 
         self.assertEqual(response.status_code, 200)
@@ -129,7 +140,6 @@ def test_status_no_progress(self):
     def test_status_no_values(self):
         dataset = self.create(Dataset, name="anything")
         self.create(ProgressMonitorDataset, dataset=dataset)
-
         response = self.client.get(f"/datasets/{dataset.pk}/status/")
 
         self.assertEqual(response.status_code, 200)
@@ -138,8 +148,19 @@ def test_status_no_values(self):
     def test_status(self):
         dataset = self.create(Dataset, name="anything")
         self.create(ProgressMonitorDataset, dataset=dataset, phase="CHECKED", state="OK")
-
         response = self.client.get(f"/datasets/{dataset.pk}/status/")
 
         self.assertEqual(response.status_code, 200)
         self.assertJSONEqual(response.content, {"phase": "CHECKED", "state": "OK"})
+
+    def test_status_no_dataset(self):
+        response = self.client.get("/datasets/123/status/")
+        self.assertEqual(response.status_code, 404)
+
+    def test_metadata_no_dataset(self):
+        response = self.client.get("/datasets/123/metadata/")
+        self.assertEqual(response.status_code, 404)
+
+    def test_coverage_no_dataset(self):
+        response = self.client.get("/datasets/123/coverage/")
+        self.assertEqual(response.status_code, 404)

docs/backend/reference/index.rst

@@ -0,0 +1,9 @@
+Reference
+=========
+
+.. toctree::
+   :maxdepth: 2
+
+   overview
+   settings
+   integration

docs/backend/reference/integration.rst

@@ -0,0 +1,18 @@
+Pelican backend integration
+===========================
+
+Pelican backend's database is treated as a read-only `legacy database <https://docs.djangoproject.com/en/3.2/howto/legacy-databases/>`__, with ``managed = False`` in all model's ``Meta`` class, and with a ``DATABASE_ROUTERS`` setting that routes queries to its database.
+
+To update ``backend/dqt/models.py`` following changes to Pelican backend's database schema:
+
+-  Run ``python backend/manage.py inspectdb > backend/dqt/models.py``
+-  Replace comments at top of file
+-  Replace ``models.DO_NOTHING`` with ``on_delete=models.CASCADE``
+-  ``Dataset``: Add methods
+-  ``Dataset.meta``: Add ``blank=True, default=dict``
+-  ``DatasetFilter.dataset_id_original``: Rename to ``dataset_original``, add ``related_name="dataset_filter_parent"``
+-  ``DatasetFilter.dataset_id_filtered``: Rename to ``dataset_filtered``, add ``related_name="dataset_filter_child"``
+-  ``ProgressMonitorDataset.dataset``: Add ``related_name="progress"``
+-  ``ProgressMonitorItem.item``: Rename to ``data_item``
+-  ``Report.type``: Change ``TextField`` to ``CharField``, add ``max_length=255``, and remove ``# This field type is a guess.``
+

docs/backend/reference/overview.rst

@@ -0,0 +1,8 @@
+Technical overview
+==================
+
+The Django project is made up of three apps:
+
+-  ``dqt``: Serves API requests for the :doc:`../../frontend/index`
+-  ``exporter``: Generates the exports to Google Docs
+-  ``processor``: Serves API requests for managing Pelican backend

docs/backend/reference.rst → docs/backend/reference/settings.rst

@@ -1,26 +1,5 @@
-Reference
-=========
-
-Pelican backend integration
----------------------------
-
-Pelican backend's database is treated as a read-only `legacy database <https://docs.djangoproject.com/en/3.2/howto/legacy-databases/>`__, with ``managed = False`` in all model's ``Meta`` class, and with a ``DATABASE_ROUTERS`` setting that routes queries to its database.
-
-To update ``backend/dqt/models.py`` following changes to Pelican backend's database schema:
-
--  Run ``python backend/manage.py inspectdb > backend/dqt/models.py``
--  Replace comments at top of file
--  Replace ``models.DO_NOTHING`` with ``on_delete=models.CASCADE``
--  ``Dataset``: Add methods
--  ``Dataset.meta``: Add ``blank=True, default=dict``
--  ``DatasetFilter.dataset_id_original``: Rename to ``dataset_original``, add ``related_name="dataset_filter_parent"``
--  ``DatasetFilter.dataset_id_filtered``: Rename to ``dataset_filtered``, add ``related_name="dataset_filter_child"``
--  ``ProgressMonitorDataset.dataset``: Add ``related_name="progress"``
--  ``ProgressMonitorItem.item``: Rename to ``data_item``
--  ``Report.type``: Change ``TextField`` to ``CharField``, add ``max_length=255``, and remove ``# This field type is a guess.``
-
 Environment variables
----------------------
+=====================
 
 See `OCP's approach to Django settings <https://ocp-software-handbook.readthedocs.io/en/latest/python/django.html#settings>`__. New variables are:
 

docs/changelog.rst

@@ -6,10 +6,26 @@ This changelog only notes major changes, to notify other developers.
 2021-11-09
 ----------
 
--  refactor: Rewrite processor API.
+-  fix: Return ``{}`` if no dataset is found by name, instead of raising ``ObjectDoesNotExist``.
+-  refactor: Rewrite the API endpoints for managing Pelican backend.
+
+   -  Requests:
+
+      -  Use the DELETE method to delete datasets.
+      -  Use the GET method for the safe operation of finding a dataset by name, instead of the POST method.
+      -  Accept the primary key in the URL path, instead of in the request body.
+
+   -  Responses:
+
+      -  Return a HTTP 2xx code, instead of ``"status": "ok"`` in the JSON response.
+      -  Return HTTP 202 Accepted for creating and deleting datasets asynchronously.
+      -  Return HTTP 400 Bad Request for missing request parameters when creating datasets.
+      -  Return HTTP 404 Not Found, instead of raising ``ObjectDoesNotExist``.
+      -  Return HTTP 405 Method Not Allowed for incorrect HTTP methods, instead of HTTP 200 with a JSON error message.
+      -  Return a JSON object from all endpoints, instead of sometimes null, a number or a string.
 
 2021-11-08
 ----------
 
--  refactor: Split Django applications from Django project. :commit:`df4b678` :commit:`fe94f41`
+-  refactor: Split Django applications from Django project. :commit:`df4b678` :commit:`fe94f41` :commit:`f01bcaf`
 -  refactor: Move static assets out of code directory. :commit:`80bbd09`