mirror of
https://github.com/suitenumerique/meet.git
synced 2026-09-29 14:09:17 +00:00
Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| a8cf005090 | |||
| 67bd09af07 | |||
| 738f263c2b | |||
| 06d3a9922e | |||
| b723b7bb62 |
@@ -22,9 +22,11 @@ and this project adheres to
|
||||
- ⬆️(frontend) upgrade posthog-js from 1.414.0 to 1.418.10
|
||||
- ⬆️(addons) upgrade i18next from 26.3.6 to 26.4.0
|
||||
- ⬆️(frontend) upgrade humanize-duration from 3.33.2 to 3.34.1
|
||||
- ♻️(backend) delete files synchronously
|
||||
|
||||
### Fixed
|
||||
|
||||
- 🐛(backend) remove the temporary upload object when a file is deleted
|
||||
- 🐛(backend) acknowledge unknown LiveKit webhook events instead of 422
|
||||
- 🔒️(backend) enforce display name setting on rename API
|
||||
- 🔒️(backend) reject inactive users in resource server backend
|
||||
|
||||
@@ -36,14 +36,9 @@ DB_PORT = 5432
|
||||
|
||||
# -- Docker
|
||||
# Get the current user ID to use for docker run and docker exec commands
|
||||
ifneq ($(findstring podman,$(DOCKER_HOST)),)
|
||||
DOCKER_UID = 0
|
||||
DOCKER_GID = 0
|
||||
else
|
||||
DOCKER_UID = $(shell id -u)
|
||||
DOCKER_GID = $(shell id -g)
|
||||
endif
|
||||
DOCKER_USER ?= $(DOCKER_UID):$(DOCKER_GID)
|
||||
DOCKER_USER = $(DOCKER_UID):$(DOCKER_GID)
|
||||
COMPOSE = DOCKER_USER=$(DOCKER_USER) docker compose
|
||||
COMPOSE_EXEC = $(COMPOSE) exec
|
||||
COMPOSE_EXEC_APP = $(COMPOSE_EXEC) app-dev
|
||||
|
||||
+2
-5
@@ -25,11 +25,8 @@ function _set_user() {
|
||||
return
|
||||
fi
|
||||
|
||||
# USER_ID = USER_ID or the engine-appropriate default if USER_ID is not set.
|
||||
case "${DOCKER_HOST:-}" in
|
||||
*podman*) USER_ID=${USER_ID:-0} ;;
|
||||
*) USER_ID=${USER_ID:-$(id -u)} ;;
|
||||
esac
|
||||
# USER_ID = USER_ID or `id -u` if USER_ID is not set
|
||||
USER_ID=${USER_ID:-$(id -u)}
|
||||
|
||||
echo "🙋(user) ID: ${USER_ID}"
|
||||
}
|
||||
|
||||
+5
-1
@@ -40,7 +40,11 @@ services:
|
||||
minio:
|
||||
condition: service_healthy
|
||||
restart: true
|
||||
entrypoint: ["/bin/sh", "-c", "mc alias set meet http://minio:9000 meet password && mc mb --ignore-existing meet/meet-media-storage"]
|
||||
entrypoint: >
|
||||
sh -c "
|
||||
/usr/bin/mc alias set meet http://minio:9000 meet password && \
|
||||
/usr/bin/mc mb meet/meet-media-storage && \
|
||||
exit 0;"
|
||||
|
||||
app-dev:
|
||||
build:
|
||||
|
||||
@@ -3,48 +3,25 @@
|
||||
from django import forms
|
||||
from django.contrib import admin, messages
|
||||
from django.contrib.auth import admin as auth_admin
|
||||
from django.db import transaction
|
||||
from django.utils.html import format_html
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
|
||||
from core.recording.event import notification
|
||||
|
||||
from . import models
|
||||
from .tasks.file import process_file_deletion
|
||||
from .utils import generate_download_s3_url
|
||||
|
||||
|
||||
def hard_delete_file(file):
|
||||
"""Hard delete a file, soft deleting it first when needed."""
|
||||
if file.deleted_at is None:
|
||||
file.soft_delete()
|
||||
file.hard_delete()
|
||||
transaction.on_commit(lambda: process_file_deletion.delay(file.id))
|
||||
|
||||
|
||||
class FileInlineFormSet(forms.BaseInlineFormSet):
|
||||
"""Inline formset overriding delete behavior for files."""
|
||||
|
||||
def delete_existing(self, obj, commit=True):
|
||||
"""Hard delete files instead of calling model.delete()."""
|
||||
hard_delete_file(obj)
|
||||
|
||||
|
||||
class FileInline(admin.TabularInline):
|
||||
"""Inline class for the File model."""
|
||||
|
||||
model = models.File
|
||||
formset = FileInlineFormSet
|
||||
fk_name = "creator"
|
||||
extra = 0
|
||||
fields = ("id", "title", "type", "upload_state", "created_at")
|
||||
readonly_fields = ("id", "created_at", "upload_state", "type")
|
||||
show_change_link = True
|
||||
|
||||
def get_queryset(self, request):
|
||||
"""Hide hard deleted files in the inline."""
|
||||
return super().get_queryset(request).filter(hard_deleted_at__isnull=True)
|
||||
|
||||
|
||||
@admin.register(models.User)
|
||||
class UserAdmin(auth_admin.UserAdmin):
|
||||
@@ -146,7 +123,6 @@ class FileAdmin(admin.ModelAdmin):
|
||||
"creator",
|
||||
"upload_state",
|
||||
"deleted_at",
|
||||
"hard_deleted_at",
|
||||
"created_at",
|
||||
"updated_at",
|
||||
)
|
||||
@@ -156,7 +132,6 @@ class FileAdmin(admin.ModelAdmin):
|
||||
"created_at",
|
||||
"updated_at",
|
||||
"deleted_at",
|
||||
"hard_deleted_at",
|
||||
)
|
||||
search_fields = (
|
||||
"id",
|
||||
@@ -174,7 +149,6 @@ class FileAdmin(admin.ModelAdmin):
|
||||
"created_at",
|
||||
"updated_at",
|
||||
"deleted_at",
|
||||
"hard_deleted_at",
|
||||
"description",
|
||||
"malware_detection_info",
|
||||
"is_ready",
|
||||
@@ -213,15 +187,7 @@ class FileAdmin(admin.ModelAdmin):
|
||||
)
|
||||
},
|
||||
),
|
||||
(
|
||||
_("Deletion"),
|
||||
{
|
||||
"fields": (
|
||||
"deleted_at",
|
||||
"hard_deleted_at",
|
||||
)
|
||||
},
|
||||
),
|
||||
(_("Deletion"), {"fields": ("deleted_at",)}),
|
||||
(
|
||||
_("Derived info"),
|
||||
{
|
||||
@@ -248,18 +214,10 @@ class FileAdmin(admin.ModelAdmin):
|
||||
'<a href="{}" target="_blank" rel="noopener noreferrer">Open File</a>', url
|
||||
)
|
||||
|
||||
def get_queryset(self, request):
|
||||
"""Hide hard deleted files in admin listing and lookups."""
|
||||
return super().get_queryset(request).filter(hard_deleted_at__isnull=True)
|
||||
|
||||
def delete_model(self, request, obj):
|
||||
"""Hard delete instead of calling model.delete()."""
|
||||
hard_delete_file(obj)
|
||||
|
||||
def delete_queryset(self, request, queryset):
|
||||
"""Hard delete all selected files."""
|
||||
"""Delete one by one so storage is cleaned up too."""
|
||||
for file in queryset:
|
||||
hard_delete_file(file)
|
||||
file.delete()
|
||||
|
||||
def has_add_permission(self, request):
|
||||
return False
|
||||
|
||||
@@ -134,7 +134,7 @@ class FilePermission(IsAuthenticated):
|
||||
Return a 404 on deleted files or if the user is not the owner
|
||||
"""
|
||||
|
||||
if obj.deleted_at is not None or obj.hard_deleted_at is not None:
|
||||
if obj.is_deleted:
|
||||
raise Http404
|
||||
|
||||
if obj.creator != request.user:
|
||||
|
||||
@@ -456,7 +456,6 @@ class ListFileSerializer(serializers.ModelSerializer):
|
||||
"type",
|
||||
"creator",
|
||||
"deleted_at",
|
||||
"hard_deleted_at",
|
||||
"filename",
|
||||
"upload_state",
|
||||
"mimetype",
|
||||
@@ -471,7 +470,6 @@ class ListFileSerializer(serializers.ModelSerializer):
|
||||
"updated_at",
|
||||
"creator",
|
||||
"deleted_at",
|
||||
"hard_deleted_at",
|
||||
"filename",
|
||||
"upload_state",
|
||||
"mimetype",
|
||||
|
||||
@@ -82,7 +82,6 @@ from core.services.room_roles import (
|
||||
)
|
||||
from core.services.subtitle import SubtitleException, SubtitleService
|
||||
from core.tasks.connection_test import delete_connection_test_room
|
||||
from core.tasks.file import process_file_deletion
|
||||
from core.utils import generate_token
|
||||
|
||||
from ..authentication.livekit import LiveKitTokenAuthentication
|
||||
@@ -1199,7 +1198,7 @@ class FileViewSet(
|
||||
permission_classes = [
|
||||
permissions.FilePermission,
|
||||
]
|
||||
queryset = models.File.objects.filter(hard_deleted_at__isnull=True)
|
||||
queryset = models.File.objects.all()
|
||||
default_serializer_class = serializers.FileSerializer
|
||||
serializer_classes = {
|
||||
"list": serializers.ListFileSerializer,
|
||||
@@ -1353,7 +1352,7 @@ class FileViewSet(
|
||||
)
|
||||
|
||||
if validation_error is not None:
|
||||
self._complete_file_deletion(file)
|
||||
file.delete()
|
||||
else:
|
||||
file.upload_state = models.FileUploadStateChoices.READY
|
||||
file.mimetype = mimetype
|
||||
@@ -1395,12 +1394,6 @@ class FileViewSet(
|
||||
|
||||
return drf_response.Response(serializer.data, status=drf_status.HTTP_200_OK)
|
||||
|
||||
def _complete_file_deletion(self, file):
|
||||
"""Delete a file completely."""
|
||||
file.soft_delete()
|
||||
file.hard_delete()
|
||||
transaction.on_commit(lambda: process_file_deletion.delay(file.id))
|
||||
|
||||
def _authorize_subrequest(self, request, pattern):
|
||||
"""
|
||||
Authorize access based on the original URL of an Nginx subrequest
|
||||
|
||||
@@ -6,7 +6,6 @@ from django.core.management.base import BaseCommand, CommandError
|
||||
from django.utils import timezone
|
||||
|
||||
from core.models import File, FileUploadStateChoices
|
||||
from core.tasks.file import process_file_deletion
|
||||
|
||||
|
||||
class Command(BaseCommand):
|
||||
@@ -32,16 +31,19 @@ class Command(BaseCommand):
|
||||
files = File.objects.filter(
|
||||
upload_state=FileUploadStateChoices.PENDING,
|
||||
created_at__lt=threshold,
|
||||
hard_deleted_at__isnull=True,
|
||||
)
|
||||
|
||||
count = 0
|
||||
failed = []
|
||||
for file in files.iterator():
|
||||
# This check shouldn't happen, but just in case we do it to avoid an error
|
||||
if not file.deleted_at:
|
||||
file.soft_delete()
|
||||
file.hard_delete()
|
||||
process_file_deletion(file.id)
|
||||
count += 1
|
||||
try:
|
||||
file.delete()
|
||||
count += 1
|
||||
except Exception as exc: # noqa: BLE001 # pylint: disable=broad-exception-caught
|
||||
failed.append(file.pk)
|
||||
self.stderr.write(f"[ERROR] Failed to clean file '{file.pk}': {exc}")
|
||||
|
||||
self.stdout.write(f"Cleaned {count} stale pending file(s).")
|
||||
|
||||
if failed:
|
||||
raise CommandError(f"Failed to clean {len(failed)} file(s).")
|
||||
|
||||
@@ -3,38 +3,33 @@
|
||||
from datetime import timedelta
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.management.base import BaseCommand
|
||||
from django.db.models import Q
|
||||
from django.core.management.base import BaseCommand, CommandError
|
||||
from django.utils import timezone
|
||||
|
||||
from core.models import File
|
||||
from core.tasks.file import process_file_deletion
|
||||
|
||||
|
||||
class Command(BaseCommand):
|
||||
"""
|
||||
Purge deleted files (object storage and database object):
|
||||
- files marked as hard deleted in database
|
||||
- files marked as soft deleted and for which the trashbin retention period has expired
|
||||
"""
|
||||
"""Purge files (object storage and database object) whose trash bin retention has expired."""
|
||||
|
||||
help = "Purge deleted files"
|
||||
|
||||
def handle(self, *args, **options):
|
||||
"""Browse purgeable files and queue them through the file deletion task."""
|
||||
"""Delete files soft deleted for longer than the grace period."""
|
||||
|
||||
is_hard_deleted = Q(hard_deleted_at__isnull=False)
|
||||
is_purgeable = Q(
|
||||
deleted_at__lte=timezone.now()
|
||||
- timedelta(days=settings.FILE_PURGE_GRACE_DAYS)
|
||||
)
|
||||
threshold = timezone.now() - timedelta(days=settings.FILE_PURGE_GRACE_DAYS)
|
||||
|
||||
count = 0
|
||||
for file in File.objects.filter(is_hard_deleted | is_purgeable).iterator():
|
||||
if file.hard_deleted_at is None:
|
||||
file.hard_delete()
|
||||
|
||||
process_file_deletion.delay(file.id)
|
||||
count += 1
|
||||
failed = []
|
||||
for file in File.objects.filter(deleted_at__lte=threshold).iterator():
|
||||
try:
|
||||
file.delete()
|
||||
count += 1
|
||||
except Exception as exc: # noqa: BLE001 # pylint: disable=broad-exception-caught
|
||||
failed.append(file.pk)
|
||||
self.stderr.write(f"[ERROR] Failed to purge file '{file.pk}': {exc}")
|
||||
|
||||
self.stdout.write(f"Purged {count} deleted file(s).")
|
||||
|
||||
if failed:
|
||||
raise CommandError(f"Failed to purge {len(failed)} file(s).")
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
from django.db import migrations
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
("core", "0022_user_default_room_access_level_and_more"),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.RemoveField(
|
||||
model_name="file",
|
||||
name="hard_deleted_at",
|
||||
),
|
||||
]
|
||||
+22
-50
@@ -17,7 +17,8 @@ from django.contrib.auth.base_user import AbstractBaseUser
|
||||
from django.contrib.postgres.fields import ArrayField
|
||||
from django.core import mail, validators
|
||||
from django.core.exceptions import PermissionDenied, ValidationError
|
||||
from django.db import models, transaction
|
||||
from django.core.files.storage import default_storage
|
||||
from django.db import models
|
||||
from django.utils import timezone
|
||||
from django.utils.text import capfirst, slugify
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
@@ -916,7 +917,6 @@ class File(BaseModel):
|
||||
null=True,
|
||||
)
|
||||
deleted_at = models.DateTimeField(null=True, blank=True)
|
||||
hard_deleted_at = models.DateTimeField(null=True, blank=True)
|
||||
|
||||
filename = models.CharField(max_length=255, null=False, blank=False)
|
||||
|
||||
@@ -954,11 +954,10 @@ class File(BaseModel):
|
||||
|
||||
return super().save(*args, **kwargs)
|
||||
|
||||
def delete(self, using=None, keep_parents=False):
|
||||
if self.deleted_at is None:
|
||||
raise RuntimeError("The file must be soft deleted before being deleted.")
|
||||
|
||||
return super().delete(using, keep_parents)
|
||||
@property
|
||||
def is_deleted(self):
|
||||
"""Return whether the file is in the trash bin."""
|
||||
return self.deleted_at is not None
|
||||
|
||||
@property
|
||||
def is_ready(self):
|
||||
@@ -1018,60 +1017,33 @@ class File(BaseModel):
|
||||
"""
|
||||
Compute and return abilities for a given user on the file.
|
||||
"""
|
||||
# Characteristics that are based only on specific access
|
||||
is_creator = user == self.creator
|
||||
retrieve = is_creator
|
||||
is_deleted = self.deleted_at is not None
|
||||
can_update = is_creator and not is_deleted and user.is_authenticated
|
||||
can_hard_delete = is_creator and user.is_authenticated
|
||||
can_destroy = can_hard_delete and not is_deleted
|
||||
can_edit = is_creator and not self.is_deleted
|
||||
|
||||
return {
|
||||
"destroy": can_destroy,
|
||||
"hard_delete": can_hard_delete,
|
||||
"retrieve": retrieve,
|
||||
"media_auth": retrieve and not is_deleted,
|
||||
"partial_update": can_update,
|
||||
"update": can_update,
|
||||
"upload_ended": can_update and user.is_authenticated,
|
||||
"destroy": can_edit,
|
||||
"retrieve": is_creator,
|
||||
"media_auth": can_edit,
|
||||
"partial_update": can_edit,
|
||||
"update": can_edit,
|
||||
"upload_ended": can_edit,
|
||||
}
|
||||
|
||||
@transaction.atomic
|
||||
def soft_delete(self):
|
||||
"""
|
||||
Soft delete the file.
|
||||
We still keep the .delete() method untouched for programmatic purposes.
|
||||
"""
|
||||
"""Move the file to the trash bin."""
|
||||
if self.deleted_at:
|
||||
raise RuntimeError("This file is already deleted.")
|
||||
|
||||
self.deleted_at = timezone.now()
|
||||
self.save(update_fields=["deleted_at"])
|
||||
|
||||
def hard_delete(self):
|
||||
def delete(self, using=None, keep_parents=False):
|
||||
"""
|
||||
Hard delete the file.
|
||||
We still keep the .delete() method untouched for programmatic purposes.
|
||||
Remove the file's objects from storage, then its row from the database.
|
||||
|
||||
Storage is removed first so that a storage failure leaves the row in place
|
||||
and the periodic purge commands retry it on their next run.
|
||||
"""
|
||||
if self.hard_deleted_at:
|
||||
raise ValidationError(
|
||||
{
|
||||
"hard_deleted_at": ValidationError(
|
||||
_("This file is already hard deleted."),
|
||||
code="file_hard_delete_already_effective",
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
if self.deleted_at is None:
|
||||
raise ValidationError(
|
||||
{
|
||||
"hard_deleted_at": ValidationError(
|
||||
_("To hard delete a file, it must first be soft deleted."),
|
||||
code="file_hard_delete_should_soft_delete_first",
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
self.hard_deleted_at = timezone.now()
|
||||
self.save(update_fields=["hard_deleted_at"])
|
||||
default_storage.delete(self.temporary_file_key) # Pending
|
||||
default_storage.delete(self.file_key) # Final
|
||||
return super().delete(using, keep_parents)
|
||||
|
||||
@@ -1,9 +1,5 @@
|
||||
"""Celery tasks for the core app."""
|
||||
|
||||
from core.tasks.connection_test import delete_connection_test_room
|
||||
from core.tasks.file import process_file_deletion
|
||||
|
||||
__all__ = (
|
||||
"delete_connection_test_room",
|
||||
"process_file_deletion",
|
||||
)
|
||||
__all__ = ("delete_connection_test_room",)
|
||||
|
||||
@@ -1,36 +0,0 @@
|
||||
"""
|
||||
Tasks related to files.
|
||||
"""
|
||||
|
||||
import logging
|
||||
|
||||
from django.core.files.storage import default_storage
|
||||
|
||||
from core.models import File
|
||||
from core.tasks._task import task
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@task
|
||||
def process_file_deletion(file_id):
|
||||
"""
|
||||
Process the deletion of a file.
|
||||
Definitely delete it in the database.
|
||||
Delete the files from the storage.
|
||||
"""
|
||||
logger.info("Processing item deletion for %s", file_id)
|
||||
try:
|
||||
file = File.objects.get(id=file_id)
|
||||
except File.DoesNotExist:
|
||||
logger.error("Item %s does not exist", file_id)
|
||||
return
|
||||
|
||||
if file.hard_deleted_at is None:
|
||||
logger.error("To process an item deletion, it must be hard deleted first.")
|
||||
return
|
||||
|
||||
logger.info("Deleting file %s", file.file_key)
|
||||
default_storage.delete(file.file_key)
|
||||
|
||||
file.delete()
|
||||
@@ -1,9 +1,11 @@
|
||||
"""Tests for the clean_pending_files management command."""
|
||||
|
||||
from datetime import timedelta
|
||||
from io import BytesIO, StringIO
|
||||
from unittest.mock import patch
|
||||
|
||||
from django.core.files.storage import default_storage
|
||||
from django.core.management import call_command
|
||||
from django.core.management import CommandError, call_command
|
||||
from django.utils import timezone
|
||||
|
||||
import pytest
|
||||
@@ -23,31 +25,32 @@ def test_clean_pending_files_recent_pending_not_deleted():
|
||||
file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
update_upload_state=models.FileUploadStateChoices.PENDING,
|
||||
upload_bytes=b"hello",
|
||||
)
|
||||
# A pending upload only lives under the temporary key
|
||||
default_storage.save(file.temporary_file_key, BytesIO(b"hello"))
|
||||
|
||||
call_command("clean_pending_files")
|
||||
|
||||
file.refresh_from_db()
|
||||
assert file.deleted_at is None
|
||||
assert default_storage.exists(file.file_key)
|
||||
assert default_storage.exists(file.temporary_file_key)
|
||||
|
||||
|
||||
def test_clean_pending_files_old_pending_deleted():
|
||||
"""Pending files older than the threshold should be deleted."""
|
||||
"""Pending files older than the threshold should be deleted, temporary object included."""
|
||||
old_date = timezone.now() - timedelta(hours=49)
|
||||
file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
update_upload_state=models.FileUploadStateChoices.PENDING,
|
||||
upload_bytes=b"hello",
|
||||
)
|
||||
assert default_storage.exists(file.file_key)
|
||||
# A pending upload only lives under the temporary key
|
||||
default_storage.save(file.temporary_file_key, BytesIO(b"hello"))
|
||||
models.File.objects.filter(pk=file.pk).update(created_at=old_date)
|
||||
|
||||
call_command("clean_pending_files")
|
||||
|
||||
assert not models.File.objects.filter(pk=file.pk).exists()
|
||||
assert not default_storage.exists(file.file_key)
|
||||
assert not default_storage.exists(file.temporary_file_key)
|
||||
|
||||
|
||||
def test_clean_pending_files_old_non_pending_not_deleted():
|
||||
@@ -63,7 +66,6 @@ def test_clean_pending_files_old_non_pending_not_deleted():
|
||||
|
||||
file.refresh_from_db()
|
||||
assert file.deleted_at is None
|
||||
assert file.hard_deleted_at is None
|
||||
|
||||
|
||||
def test_clean_pending_files_custom_hours():
|
||||
@@ -72,8 +74,8 @@ def test_clean_pending_files_custom_hours():
|
||||
file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
update_upload_state=models.FileUploadStateChoices.PENDING,
|
||||
upload_bytes=b"hello",
|
||||
)
|
||||
default_storage.save(file.temporary_file_key, BytesIO(b"hello"))
|
||||
models.File.objects.filter(pk=file.pk).update(created_at=old_date)
|
||||
|
||||
# Default 24h threshold -> file not deleted
|
||||
@@ -81,10 +83,58 @@ def test_clean_pending_files_custom_hours():
|
||||
|
||||
file.refresh_from_db()
|
||||
assert file.deleted_at is None
|
||||
assert default_storage.exists(file.file_key)
|
||||
assert default_storage.exists(file.temporary_file_key)
|
||||
|
||||
# 8h threshold -> file deleted
|
||||
call_command("clean_pending_files", "--hours=8")
|
||||
|
||||
assert not models.File.objects.filter(pk=file.pk).exists()
|
||||
assert not default_storage.exists(file.file_key)
|
||||
assert not default_storage.exists(file.temporary_file_key)
|
||||
|
||||
|
||||
def test_clean_pending_files_storage_failure_keeps_row_and_continues():
|
||||
"""
|
||||
A storage failure on one file leaves its row in place for the next run,
|
||||
does not stop the other files from being cleaned, and exits non-zero.
|
||||
"""
|
||||
out = StringIO()
|
||||
err = StringIO()
|
||||
old_date = timezone.now() - timedelta(hours=49)
|
||||
|
||||
failing_file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
update_upload_state=models.FileUploadStateChoices.PENDING,
|
||||
)
|
||||
default_storage.save(failing_file.temporary_file_key, BytesIO(b"hello"))
|
||||
stale_file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
update_upload_state=models.FileUploadStateChoices.PENDING,
|
||||
)
|
||||
default_storage.save(stale_file.temporary_file_key, BytesIO(b"hello"))
|
||||
models.File.objects.filter(pk__in=[failing_file.pk, stale_file.pk]).update(
|
||||
created_at=old_date
|
||||
)
|
||||
|
||||
original_delete = default_storage.delete
|
||||
|
||||
def flaky_delete(name):
|
||||
if name == failing_file.temporary_file_key:
|
||||
raise OSError("boom")
|
||||
return original_delete(name)
|
||||
|
||||
with (
|
||||
patch.object(default_storage, "delete", side_effect=flaky_delete),
|
||||
pytest.raises(CommandError, match="Failed to clean 1 file"),
|
||||
):
|
||||
call_command("clean_pending_files", stdout=out, stderr=err)
|
||||
|
||||
assert "Cleaned 1 stale pending file(s)." in out.getvalue()
|
||||
assert f"Failed to clean file '{failing_file.pk}': boom" in err.getvalue()
|
||||
|
||||
# The failing file is retried on the next run
|
||||
assert models.File.objects.filter(pk=failing_file.pk).exists()
|
||||
assert default_storage.exists(failing_file.temporary_file_key)
|
||||
|
||||
# The other file was cleaned despite the earlier failure
|
||||
assert not models.File.objects.filter(pk=stale_file.pk).exists()
|
||||
assert not default_storage.exists(stale_file.temporary_file_key)
|
||||
|
||||
@@ -6,13 +6,12 @@ from random import randint
|
||||
from unittest.mock import patch
|
||||
|
||||
from django.core.files.storage import default_storage
|
||||
from django.core.management import call_command
|
||||
from django.core.management import CommandError, call_command
|
||||
from django.utils import timezone
|
||||
|
||||
import pytest
|
||||
|
||||
from core import factories, models
|
||||
from core.tasks.file import process_file_deletion
|
||||
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
||||
@@ -25,11 +24,7 @@ def test_purge_deleted_files_no_deleted_files(django_assert_num_queries):
|
||||
|
||||
@pytest.mark.django_db(transaction=True)
|
||||
def test_purge_deleted_files_success(settings):
|
||||
"""
|
||||
Queue deletion for:
|
||||
- hard-deleted files
|
||||
- soft-deleted files past retention period + grace period.
|
||||
"""
|
||||
"""Only soft deleted files past the grace period are purged."""
|
||||
out = StringIO()
|
||||
|
||||
settings.FILE_PURGE_GRACE_DAYS = grace = randint(1, 20)
|
||||
@@ -56,30 +51,63 @@ def test_purge_deleted_files_success(settings):
|
||||
)
|
||||
purgeable_file.soft_delete()
|
||||
|
||||
hard_deleted_file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
upload_bytes=b"hello",
|
||||
)
|
||||
hard_deleted_file.soft_delete()
|
||||
hard_deleted_file.hard_delete()
|
||||
call_command("purge_deleted_files", stdout=out)
|
||||
|
||||
with patch(
|
||||
"core.management.commands.purge_deleted_files.process_file_deletion.delay",
|
||||
side_effect=process_file_deletion,
|
||||
) as mock_delay:
|
||||
call_command("purge_deleted_files", stdout=out)
|
||||
|
||||
assert "Purged 2 deleted file(s)." in out.getvalue()
|
||||
assert mock_delay.call_count == 2
|
||||
called_ids = {call.args[0] for call in mock_delay.call_args_list}
|
||||
assert called_ids == {purgeable_file.id, hard_deleted_file.id}
|
||||
assert "Purged 1 deleted file(s)." in out.getvalue()
|
||||
|
||||
assert models.File.objects.filter(id=not_deleted_file.id).exists()
|
||||
assert models.File.objects.filter(id=not_purgeable_file.id).exists()
|
||||
assert not models.File.objects.filter(id=purgeable_file.id).exists()
|
||||
assert not models.File.objects.filter(id=hard_deleted_file.id).exists()
|
||||
|
||||
assert default_storage.exists(not_deleted_file.file_key)
|
||||
assert default_storage.exists(not_purgeable_file.file_key)
|
||||
assert not default_storage.exists(purgeable_file.file_key)
|
||||
assert not default_storage.exists(hard_deleted_file.file_key)
|
||||
|
||||
|
||||
@pytest.mark.django_db(transaction=True)
|
||||
def test_purge_deleted_files_storage_failure_keeps_row_and_continues(settings):
|
||||
"""
|
||||
A storage failure on one file leaves its row in place for the next run,
|
||||
does not stop the other files from being purged, and exits non-zero.
|
||||
"""
|
||||
out = StringIO()
|
||||
err = StringIO()
|
||||
|
||||
settings.FILE_PURGE_GRACE_DAYS = 1
|
||||
purge_now = timezone.now() - timedelta(days=2)
|
||||
|
||||
with patch("django.utils.timezone.now", return_value=purge_now):
|
||||
failing_file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
upload_bytes=b"hello",
|
||||
)
|
||||
failing_file.soft_delete()
|
||||
purgeable_file = factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
upload_bytes=b"hello",
|
||||
)
|
||||
purgeable_file.soft_delete()
|
||||
|
||||
original_delete = default_storage.delete
|
||||
|
||||
def flaky_delete(name):
|
||||
if name == failing_file.file_key:
|
||||
raise OSError("boom")
|
||||
return original_delete(name)
|
||||
|
||||
with (
|
||||
patch.object(default_storage, "delete", side_effect=flaky_delete),
|
||||
pytest.raises(CommandError, match="Failed to purge 1 file"),
|
||||
):
|
||||
call_command("purge_deleted_files", stdout=out, stderr=err)
|
||||
|
||||
assert "Purged 1 deleted file(s)." in out.getvalue()
|
||||
assert f"Failed to purge file '{failing_file.pk}': boom" in err.getvalue()
|
||||
|
||||
# The failing file is retried on the next run
|
||||
assert models.File.objects.filter(id=failing_file.id).exists()
|
||||
assert default_storage.exists(failing_file.file_key)
|
||||
|
||||
# The other file was purged despite the earlier failure
|
||||
assert not models.File.objects.filter(id=purgeable_file.id).exists()
|
||||
assert not default_storage.exists(purgeable_file.file_key)
|
||||
|
||||
@@ -4,8 +4,6 @@ Tests for files API endpoint in meet's core app: list
|
||||
|
||||
from unittest import mock
|
||||
|
||||
from django.utils import timezone
|
||||
|
||||
import pytest
|
||||
from faker import Faker
|
||||
from rest_framework.pagination import PageNumberPagination
|
||||
@@ -56,14 +54,6 @@ def test_api_files_list_format():
|
||||
title="item 2",
|
||||
)
|
||||
|
||||
# hard deleted item should not appear
|
||||
factories.FileFactory(
|
||||
type=models.FileTypeChoices.BACKGROUND_IMAGE,
|
||||
hard_deleted_at=timezone.now(),
|
||||
title="hard deleted item",
|
||||
creator=user,
|
||||
)
|
||||
|
||||
response = client.get("/api/v1.0/files/")
|
||||
|
||||
assert response.status_code == 200
|
||||
@@ -94,10 +84,8 @@ def test_api_files_list_format():
|
||||
"size": None,
|
||||
"description": None,
|
||||
"deleted_at": None,
|
||||
"hard_deleted_at": None,
|
||||
"abilities": {
|
||||
"destroy": True,
|
||||
"hard_delete": True,
|
||||
"media_auth": True,
|
||||
"partial_update": True,
|
||||
"retrieve": True,
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import logging
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from io import BytesIO
|
||||
from unittest import mock
|
||||
|
||||
from django.core.files.storage import default_storage
|
||||
|
||||
@@ -136,6 +137,40 @@ def test_api_file_upload_ended_mimetype_not_allowed(settings, caplog):
|
||||
|
||||
assert not models.File.objects.filter(id=file.id).exists()
|
||||
assert not default_storage.exists(file.file_key)
|
||||
assert not default_storage.exists(file.temporary_file_key)
|
||||
|
||||
|
||||
def test_api_file_upload_ended_rejected_storage_failure_keeps_row(settings):
|
||||
"""
|
||||
When a rejected upload cannot be removed from storage, the row must survive
|
||||
(reverted to pending) so the cleanup can be retried, and no phantom file
|
||||
must be re-inserted under a new id.
|
||||
"""
|
||||
settings.FILE_UPLOAD_RESTRICTIONS = {
|
||||
"background_image": {
|
||||
**settings.FILE_UPLOAD_RESTRICTIONS["background_image"],
|
||||
"allowed_mimetypes": ["application/pdf"],
|
||||
}
|
||||
}
|
||||
|
||||
user = factories.UserFactory()
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
|
||||
file = factories.FileFactory(
|
||||
type=FileTypeChoices.BACKGROUND_IMAGE, filename="my_file.txt", creator=user
|
||||
)
|
||||
default_storage.save(file.temporary_file_key, BytesIO(b"my prose"))
|
||||
|
||||
with (
|
||||
mock.patch.object(default_storage, "delete", side_effect=OSError("boom")),
|
||||
pytest.raises(OSError, match="boom"),
|
||||
):
|
||||
client.post(f"/api/v1.0/files/{file.id!s}/upload-ended/")
|
||||
|
||||
assert models.File.objects.count() == 1
|
||||
file.refresh_from_db()
|
||||
assert file.upload_state == FileUploadStateChoices.PENDING
|
||||
|
||||
|
||||
def test_api_file_upload_ended_mimetype_not_allowed_not_checking_mimetype(settings):
|
||||
@@ -273,6 +308,7 @@ def test_api_upload_ended_file_size_exceeded(settings, caplog):
|
||||
|
||||
assert not models.File.objects.filter(id=file.id).exists()
|
||||
assert not default_storage.exists(file.file_key)
|
||||
assert not default_storage.exists(file.temporary_file_key)
|
||||
|
||||
|
||||
@pytest.mark.django_db(transaction=True)
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
"""
|
||||
Unit tests for the File model deletion flow
|
||||
"""
|
||||
|
||||
from io import BytesIO
|
||||
from unittest import mock
|
||||
|
||||
from django.core.files.storage import default_storage
|
||||
from django.utils import timezone
|
||||
|
||||
import pytest
|
||||
|
||||
from core.factories import FileFactory
|
||||
from core.models import File
|
||||
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
||||
|
||||
def test_models_files_soft_delete():
|
||||
"""Soft deleting should only set the deletion timestamp."""
|
||||
file = FileFactory()
|
||||
assert file.is_deleted is False
|
||||
|
||||
file.soft_delete()
|
||||
|
||||
file.refresh_from_db()
|
||||
assert file.is_deleted is True
|
||||
assert FileFactory(deleted_at=timezone.now()).is_deleted is True
|
||||
|
||||
|
||||
def test_models_files_soft_delete_twice():
|
||||
"""Soft deleting an already soft deleted file should fail."""
|
||||
file = FileFactory()
|
||||
file.soft_delete()
|
||||
|
||||
with pytest.raises(RuntimeError, match="already deleted"):
|
||||
file.soft_delete()
|
||||
|
||||
|
||||
def test_models_files_delete():
|
||||
"""Deleting should remove the row and both the final and temporary objects."""
|
||||
file = FileFactory(upload_bytes=b"hello")
|
||||
default_storage.save(file.temporary_file_key, BytesIO(b"hello"))
|
||||
# Captured up front: Django nulls the pk after delete, and the keys depend on it
|
||||
pk, key, temporary_key = file.pk, file.file_key, file.temporary_file_key
|
||||
|
||||
file.delete()
|
||||
|
||||
assert not File.objects.filter(pk=pk).exists()
|
||||
assert not default_storage.exists(key)
|
||||
assert not default_storage.exists(temporary_key)
|
||||
|
||||
|
||||
def test_models_files_delete_without_storage_object():
|
||||
"""Deleting a file that has nothing in storage should still remove the row."""
|
||||
file = FileFactory()
|
||||
pk = file.pk
|
||||
|
||||
file.delete()
|
||||
|
||||
assert not File.objects.filter(pk=pk).exists()
|
||||
|
||||
|
||||
def test_models_files_delete_storage_failure_keeps_row():
|
||||
"""A storage failure must leave the row in place so the deletion can be retried."""
|
||||
file = FileFactory(upload_bytes=b"hello")
|
||||
|
||||
with (
|
||||
mock.patch.object(default_storage, "delete", side_effect=OSError("boom")),
|
||||
pytest.raises(OSError, match="boom"),
|
||||
):
|
||||
file.delete()
|
||||
|
||||
assert File.objects.filter(pk=file.pk).exists()
|
||||
assert default_storage.exists(file.file_key)
|
||||
@@ -15,7 +15,6 @@ export type ApiFileItem = {
|
||||
type: ApiFileType
|
||||
creator: ApiFileCreator
|
||||
deleted_at: string | null
|
||||
hard_deleted_at: string | null
|
||||
filename: string
|
||||
upload_state: ApiFileUploadState
|
||||
mimetype: string // e.g. "image/png"
|
||||
|
||||
Reference in New Issue
Block a user