404 for None and 400 for invalid pk

kolibri/core/content/api.py

@@ -5,6 +5,7 @@
 from collections import OrderedDict
 from functools import reduce
 from random import sample
+from uuid import UUID
 
 from django.core.cache import cache
 from django.core.exceptions import ValidationError
@@ -863,6 +864,13 @@ def retrieve(self, request, pk=None):
         if pk is None:
             raise Http404
 
+        try:
+            UUID(pk)
+        except ValueError:
+            return Response(
+                {"error": "Invalid UUID format."}, status=status.HTTP_400_BAD_REQUEST
+            )
+
         if self._should_proxy_request(request):
             if self.get_queryset().filter(admin_imported=True, pk=pk).exists():
                 # Used in the update method for remote request retrieval

kolibri/core/content/public_api.py

@@ -2,8 +2,8 @@
 
 from django.db import connection
 from django.db.models import Q
-from django.http import Http404
 from django.http import HttpResponseBadRequest
+from django.shortcuts import get_object_or_404
 from rest_framework import status
 from rest_framework.response import Response
 from rest_framework.serializers import Serializer
@@ -52,16 +52,13 @@ def retrieve(self, request, pk=None):
         :param pk: id parent node
         :return: an object with keys for each content metadata table and a schema_version key
         """
-        if pk is None:
-            raise Http404
-        else:
-            try:
-                UUID(pk)
-            except ValueError:
-                return Response(
-                    {"error": "Invalid UUID format."},
-                    status=status.HTTP_400_BAD_REQUEST,
-                )
+        try:
+            UUID(pk)
+        except ValueError:
+            return Response(
+                {"error": "Invalid UUID format."},
+                status=status.HTTP_400_BAD_REQUEST,
+            )
 
         content_schema = request.query_params.get(
             "schema_version", self.default_content_schema
@@ -84,7 +81,7 @@ def retrieve(self, request, pk=None):
 
         # Get the model for the target node here - we do this so that we trigger a 404 immediately if the node
         # does not exist.
-        node = self.get_object()
+        node = get_object_or_404(models.ContentNode.objects.all(), pk=pk)
 
         nodes = node.get_ancestors(include_self=True)
 

kolibri/core/content/test/test_content_app.py

@@ -515,6 +515,15 @@ def test_contentnode_tree_none_pk(self):
         )
         self.assertEqual(response.status_code, 404)
 
+    def test_contentnode_tree_bad_pk(self):
+        response = self.client.get(
+            reverse(
+                "kolibri:core:contentnode_tree-detail",
+                kwargs={"pk": "this is not UUID"},
+            )
+        )
+        self.assertEqual(response.status_code, 400)
+
     @unittest.skipIf(
         getattr(settings, "DATABASES")["default"]["ENGINE"]
         == "django.db.backends.postgresql",