refactor: Start rewrite of processor views

- fix: Return HTTP error code instead of HTTP 200 with a JSON error message
- fix: Use GET method for idempotent dataset_id endpoint, instead of POST method
- fix: Return JSON response from all app endpoints, instead of plain-text response
- chore: Use Django's require_POST decorator instead of custom code (HTTP 405 instead of HTTP 400)
- chore: Remove unnecessary safe=False argument to JsonResponse

backend/dqt/views.py

@@ -30,7 +30,7 @@ def dataset_filter_items(request):
     dataset_id_original = input_message["dataset_id_original"]
     filter_message = input_message["filter_message"]
 
-    # building query in a safely manner
+    # See similar code in dataset_filter.py in pelican-backend.
     try:
         variables = {"dataset_id_original": dataset_id_original}
         parts = ["SELECT count(*) FROM data_item WHERE dataset_id = %(dataset_id_original)s"]

backend/processor/views.py

@@ -1,88 +1,65 @@
 import simplejson as json
 from django.db import connections
-from django.http import HttpResponse, HttpResponseBadRequest, JsonResponse
+from django.http import JsonResponse
 from django.views.decorators.csrf import csrf_exempt
+from django.views.decorators.http import require_POST
 from dqt.models import Dataset, FieldLevelCheck, ProgressMonitorDataset
 from psycopg2.sql import SQL, Identifier
 
 from .rabbitmq import publish
 
 
 @csrf_exempt
+@require_POST
 def create_dataset_filter(request):
-    if request.method == "GET":
-        return HttpResponseBadRequest(reason="Only post method is accepted.")
-
     publish(request.body, "dataset_filter_extractor_init")
 
-    return HttpResponse("done")
+    return JsonResponse({"status": "ok"})
 
 
 @csrf_exempt
+@require_POST
 def dataset_start(request):
-    if request.method == "GET":
-        return JsonResponse({"status": "error", "data": {"reason": "Only post method is accepted."}})
-
-    routing_key = "ocds_kingfisher_extractor_init"
-
     body = json.loads(request.body.decode("utf-8"))
 
-    dataset_name = body.get("name")
-
     message = {
-        "name": dataset_name,
+        "name": body.get("name"),
         "collection_id": body.get("collection_id"),
-        # "ancestor_id": ancestor_id,
-        # "max_items": max_items,
     }
-
-    publish(json.dumps(message), routing_key)
+    publish(json.dumps(message), "ocds_kingfisher_extractor_init")
 
     return JsonResponse(
-        {"status": "ok", "data": {"message": f"Dataset {dataset_name} on Pelican started"}}, safe=False
+        {"status": "ok", "data": {"message": "Started dataset '%(name)s' for collection %(collection_id)s" % message}}
     )
 
 
 @csrf_exempt
+@require_POST
 def dataset_wipe(request):
-    if request.method == "GET":
-        return JsonResponse({"status": "error", "data": {"reason": "Only post method is accepted."}})
-
-    routing_key = "wiper_init"
-
     body = json.loads(request.body.decode("utf-8"))
 
     message = {
         "dataset_id": body.get("dataset_id"),
     }
+    publish(json.dumps(message), "wiper_init")
 
-    publish(json.dumps(message), routing_key)
-
-    return JsonResponse(
-        {"status": "ok", "data": {"message": f"Dataset id {body.get('dataset_id')} on Pelican will be wiped"}},
-        safe=False,
-    )
+    return JsonResponse({"status": "ok", "data": {"message": "Wiping dataset %(dataset_id)s" % message}})
 
 
 @csrf_exempt
 def dataset_progress(request, dataset_id):
     try:
         monitor = ProgressMonitorDataset.objects.values("state", "phase").get(dataset__id=dataset_id)
-        return JsonResponse({"status": "ok", "data": monitor}, safe=False)
+        return JsonResponse({"status": "ok", "data": monitor})
     except ProgressMonitorDataset.DoesNotExist:
-        return JsonResponse({"status": "ok", "data": None}, safe=False)
+        return JsonResponse({"status": "ok", "data": None})
 
 
 @csrf_exempt
 def dataset_id(request):
-    if request.method == "GET":
-        return JsonResponse({"status": "error", "data": {"reason": "Only post method is accepted."}})
-
-    body = json.loads(request.body.decode("utf-8"))
-    dataset_name = body.get("name")
+    dataset = Dataset.objects.get(name=request.GET.get("name"))
 
-    dataset = Dataset.objects.get(name=dataset_name)
-    return JsonResponse({"status": "ok", "data": dataset.id if dataset else None}, safe=False)
+    return JsonResponse({"status": "ok", "data": dataset.id if dataset else None})
 
 
 @csrf_exempt
@@ -140,11 +117,11 @@ def dataset_availability(request, dataset_id):
                     if r[0] == i:
                         counts[key] += int(r[1])
 
-    return JsonResponse({"status": "ok", "data": counts}, safe=False)
+    return JsonResponse({"status": "ok", "data": counts})
 
 
 @csrf_exempt
 def dataset_metadata(request, dataset_id):
     meta = Dataset.objects.values_list("meta__collection_metadata", flat=True).get(id=dataset_id)
 
-    return JsonResponse({"status": "ok", "data": meta}, safe=False)
+    return JsonResponse({"status": "ok", "data": meta})

backend/tests/processor/test_views.py

@@ -26,11 +26,11 @@ def create(self, model, **kwargs):
         return obj
 
     def test_require_POST(self):
-        for path in ("create_dataset_filter",):
+        for path in ("create_dataset_filter", "dataset_start", "dataset_wipe"):
             with self.subTest(path=path):
                 response = self.client.get(f"/api/{path}")
 
-                self.assertEqual(response.status_code, 400)
+                self.assertEqual(response.status_code, 405)
 
     @patch("processor.views.publish")
     def test_dataset_start(self, publish):
@@ -40,7 +40,7 @@ def test_dataset_start(self, publish):
 
         self.assertEqual(response.status_code, 200)
         self.assertJSONEqual(
-            response.content, {"status": "ok", "data": {"message": "Dataset anything on Pelican started"}}
+            response.content, {"status": "ok", "data": {"message": "Started dataset 'anything' for collection 123"}}
         )
         publish.assert_called_once_with('{"name": "anything", "collection_id": 123}', "ocds_kingfisher_extractor_init")
 
@@ -51,7 +51,7 @@ def test_create_dataset_filter(self, publish):
         )
 
         self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.content, b"done")
+        self.assertJSONEqual(response.content, {"status": "ok"})
         publish.assert_called_once_with(
             b'{"dataset_id_original": 123, "filter_message": {}}', "dataset_filter_extractor_init"
         )
@@ -61,15 +61,13 @@ def test_dataset_wipe(self, publish):
         response = self.client.post("/api/dataset_wipe", {"dataset_id": 123}, "application/json")
 
         self.assertEqual(response.status_code, 200)
-        self.assertJSONEqual(
-            response.content, {"status": "ok", "data": {"message": "Dataset id 123 on Pelican will be wiped"}}
-        )
+        self.assertJSONEqual(response.content, {"status": "ok", "data": {"message": "Wiping dataset 123"}})
         publish.assert_called_once_with('{"dataset_id": 123}', "wiper_init")
 
     def test_dataset_id(self):
         dataset = self.create(Dataset, name="anything")
 
-        response = self.client.post("/api/dataset_id", {"name": "anything"}, "application/json")
+        response = self.client.get("/api/dataset_id?name=anything")
 
         self.assertEqual(response.status_code, 200)
         self.assertJSONEqual(response.content, {"status": "ok", "data": dataset.pk})

docs/changelog.rst

@@ -3,6 +3,11 @@ Changelog
 
 This changelog only notes major changes, to notify other developers.
 
+2021-11-09
+----------
+
+-  refactor: Start rewrite of processor API.
+
 2021-11-08
 ----------