feat(api): implement file deletion (#2960)

This implements the file delete to the Django API. Previously, the code was only manipulating the database while leaving the file in place.

Co-authored-by: jo <ljonas@riseup.net>

api/libretime_api/schedule/models/schedule.py

@@ -1,4 +1,5 @@
 from django.db import models
+from django.utils.timezone import now
 
 
 class Schedule(models.Model):
@@ -115,6 +116,14 @@ class Schedule(models.Model):
             return self.instance.ends_at
         return self.ends_at
 
+    @staticmethod
+    def is_file_scheduled_in_the_future(file_id):
+        count = Schedule.objects.filter(
+            file_id=file_id,
+            ends_at__gt=now(),
+        ).count()
+        return count > 0
+
     class Meta:
         managed = False
         db_table = "cc_schedule"

api/libretime_api/storage/tests/views/test_file.py

@@ -1,35 +1,63 @@
+import os
+from unittest.mock import patch
+
 from django.conf import settings
 from model_bakery import baker
 from rest_framework.test import APITestCase
 
 from ...._fixtures import AUDIO_FILENAME
+from ...models import File
 
 
 class TestFileViewSet(APITestCase):
     @classmethod
     def setUpTestData(cls):
-        cls.path = "/api/v2/files/{id}/download"
         cls.token = settings.CONFIG.general.api_key
 
-    def test_invalid(self):
-        path = self.path.format(id="a")
+    def test_download_invalid(self):
         self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}")
-        response = self.client.get(path)
-        self.assertEqual(response.status_code, 400)
-
-    def test_does_not_exist(self):
-        path = self.path.format(id="1")
-        self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}")
-        response = self.client.get(path)
+        file_id = "1"
+        response = self.client.get(f"/api/v2/files/{file_id}/download")
         self.assertEqual(response.status_code, 404)
 
-    def test_exists(self):
-        file = baker.make(
+    def test_download(self):
+        self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}")
+        file: File = baker.make(
             "storage.File",
             mime="audio/mp3",
             filepath=AUDIO_FILENAME,
         )
-        path = self.path.format(id=str(file.pk))
-        self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}")
-        response = self.client.get(path)
+        response = self.client.get(f"/api/v2/files/{file.id}/download")
         self.assertEqual(response.status_code, 200)
+
+    def test_destroy(self):
+        self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}")
+        file: File = baker.make(
+            "storage.File",
+            mime="audio/mp3",
+            filepath=AUDIO_FILENAME,
+        )
+
+        with patch("libretime_api.storage.views.file.remove") as remove_mock:
+            response = self.client.delete(f"/api/v2/files/{file.id}")
+
+        self.assertEqual(response.status_code, 204)
+        remove_mock.assert_called_with(
+            os.path.join(settings.CONFIG.storage.path, file.filepath)
+        )
+
+    def test_destroy_no_file(self):
+        self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}")
+        file = baker.make(
+            "storage.File",
+            mime="audio/mp3",
+            filepath="invalid.mp3",
+        )
+        response = self.client.delete(f"/api/v2/files/{file.id}")
+        self.assertEqual(response.status_code, 204)
+
+    def test_destroy_invalid(self):
+        self.client.credentials(HTTP_AUTHORIZATION=f"Api-Key {self.token}")
+        file_id = "1"
+        response = self.client.delete(f"/api/v2/files/{file_id}")
+        self.assertEqual(response.status_code, 404)

api/libretime_api/storage/views/file.py

@@ -1,31 +1,59 @@
+import logging
 import os
+from os import remove
 
+from django.conf import settings
 from django.http import HttpResponse
-from django.shortcuts import get_object_or_404
 from django.utils.encoding import filepath_to_uri
-from rest_framework import viewsets
+from rest_framework import status, viewsets
 from rest_framework.decorators import action
-from rest_framework.serializers import IntegerField
+from rest_framework.exceptions import APIException
 
+from ...schedule.models import Schedule
 from ..models import File
 from ..serializers import FileSerializer
 
+logger = logging.getLogger(__name__)
+
+
+class FileInUse(APIException):
+    status_code = status.HTTP_409_CONFLICT
+    default_detail = "The file is currently used"
+    default_code = "file_in_use"
+
 
 class FileViewSet(viewsets.ModelViewSet):
     queryset = File.objects.all()
     serializer_class = FileSerializer
     model_permission_name = "file"
 
+    # pylint: disable=invalid-name,unused-argument
     @action(detail=True, methods=["GET"])
-    def download(self, request, pk=None):  # pylint: disable=invalid-name
-        pk = IntegerField().to_internal_value(data=pk)
-
-        file = get_object_or_404(File, pk=pk)
+    def download(self, request, pk=None):
+        instance: File = self.get_object()
 
         response = HttpResponse()
-
         # HTTP headers must be USASCII encoded, or Nginx might not find the file and
         # will return a 404.
-        redirect_uri = filepath_to_uri(os.path.join("/api/_media", file.filepath))
+        redirect_uri = filepath_to_uri(os.path.join("/api/_media", instance.filepath))
         response["X-Accel-Redirect"] = redirect_uri
         return response
+
+    def perform_destroy(self, instance: File):
+        if Schedule.is_file_scheduled_in_the_future(file_id=instance.id):
+            raise FileInUse("file is scheduled in the future")
+
+        try:
+            if instance.filepath is None:
+                logger.warning("file does not have a filepath: %d", instance.id)
+                return
+
+            path = os.path.join(settings.CONFIG.storage.path, instance.filepath)
+
+            if not os.path.isfile(path):
+                logger.warning("file does not exist in storage: %d", instance.id)
+                return
+
+            remove(path)
+        except OSError as exception:
+            raise APIException("could not delete file from storage") from exception