diff --git a/src/core/management/commands/backfill_sound_filenames.py b/src/core/management/commands/backfill_sound_filenames.py new file mode 100644 index 00000000..6c5ae2da --- /dev/null +++ b/src/core/management/commands/backfill_sound_filenames.py @@ -0,0 +1,125 @@ +"""Move sound files stored under their uploader's filename to generated names. + +See #991. New uploads already land at `sounds/.` (#982); this walks +the rows that predate that and brings them into line. +""" + +import logging +import re + +from django.core.files.base import ContentFile +from django.core.management.base import BaseCommand + +from core.models import Sound, sound_file_path + +log = logging.getLogger(__name__) + +# What `sound_file_path` produces: sounds/<32 hex chars>[.ext] +GENERATED_NAME = re.compile(r"^sounds/[0-9a-f]{32}(\.[A-Za-z0-9]+)?$") + + +class Command(BaseCommand): + help = ( + "Rename existing Sound files from the uploader's filename to a " + "generated one. Safe to re-run: rows already renamed are skipped, so " + "an interrupted run resumes by running it again." + ) + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Report what would change without touching storage or the database.", + ) + parser.add_argument( + "--limit", + type=int, + default=None, + help="Process at most this many rows, so a first batch can be small.", + ) + + def handle(self, *args, **options): + dry_run = options["dry_run"] + limit = options["limit"] + + pending = [ + sound + for sound in Sound.objects.exclude(file="").order_by("pk") + if not GENERATED_NAME.match(sound.file.name or "") + ] + if limit is not None: + pending = pending[:limit] + + if not pending: + self.stdout.write( + self.style.SUCCESS( + "Nothing to do: every sound file already has a generated name." + ) + ) + return + + self.stdout.write( + f"{len(pending)} sound file(s) still stored under a user-supplied name." + ) + + renamed = failed = 0 + for sound in pending: + old_name = sound.file.name + try: + if dry_run: + self.stdout.write( + f" would rename {old_name} " + f"-> {sound_file_path(sound, old_name)}" + ) + continue + new_name = self._rename(sound, old_name) + except Exception: + # One unreadable or unwritable object must not abort the run: + # the whole point of a command over a migration is that the + # rest of the batch still gets done, and the key is recorded. + failed += 1 + log.exception("Could not rename sound %s (%s)", sound.pk, old_name) + self.stderr.write( + self.style.ERROR(f" failed: {old_name} (sound {sound.pk})") + ) + continue + renamed += 1 + self.stdout.write(f" {old_name} -> {new_name}") + + if dry_run: + self.stdout.write(self.style.WARNING("Dry run: nothing was changed.")) + return + + self.stdout.write(self.style.SUCCESS(f"Renamed {renamed} file(s).")) + if failed: + self.stdout.write( + self.style.ERROR( + f"{failed} file(s) failed and were left as they were; " + "re-run to retry them." + ) + ) + + @staticmethod + def _rename(sound, old_name): + """Write the new object, point the row at it, then drop the old one. + + The order matters. Interrupted between any two steps, the worst + outcome is an orphaned old object, never a row pointing at a key that + no longer exists. + """ + storage = sound.file.storage + with sound.file.open("rb") as handle: + content = handle.read() + + if not sound.file_name_original: + sound.file_name_original = old_name.rsplit("/", 1)[-1] + + # FileField.save writes through storage and resets the cached handle. + sound.file.save( + sound_file_path(sound, old_name), ContentFile(content), save=True + ) + new_name = sound.file.name + + if old_name != new_name and storage.exists(old_name): + storage.delete(old_name) + return new_name diff --git a/src/core/tests/test_backfill_sound_filenames.py b/src/core/tests/test_backfill_sound_filenames.py new file mode 100644 index 00000000..5a0d87e2 --- /dev/null +++ b/src/core/tests/test_backfill_sound_filenames.py @@ -0,0 +1,127 @@ +import re +from io import StringIO +from unittest import mock + +from django.core.files.base import ContentFile +from django.core.management import call_command +from django.test import TestCase + +from core.models import Sound, SoundExtensions +from users.tests.factory import ProfileFactory + +GENERATED = re.compile(r"^sounds/[0-9a-f]{32}\.mp3$") + + +def legacy_sound(owner, filename="Música #1 (final).mp3"): + """A row as it looks before #982: stored under the uploader's own name.""" + sound = Sound.objects.create( + title=filename, + author="A", + owner=owner, + file_extension=SoundExtensions.MP3, + file_name_original="", + ) + # Bypass upload_to so the old layout can be reproduced verbatim. + sound.file.name = f"sounds/{filename}" + sound.file.storage.save(sound.file.name, ContentFile(b"audio-bytes")) + sound.save(update_fields=["file"]) + return sound + + +class TestBackfillSoundFilenames(TestCase): + def setUp(self): + self.owner = ProfileFactory() + + def run_command(self, **kwargs): + out = StringIO() + call_command("backfill_sound_filenames", stdout=out, stderr=out, **kwargs) + return out.getvalue() + + def test_a_legacy_file_is_moved_to_a_generated_name(self): + sound = legacy_sound(self.owner) + + self.run_command() + sound.refresh_from_db() + + assert GENERATED.match(sound.file.name), sound.file.name + assert "Música" not in sound.file.name + + def test_the_content_survives_the_move(self): + sound = legacy_sound(self.owner) + + self.run_command() + sound.refresh_from_db() + + with sound.file.open("rb") as handle: + assert handle.read() == b"audio-bytes" + + def test_the_old_object_is_removed_from_storage(self): + sound = legacy_sound(self.owner) + old_name = sound.file.name + + self.run_command() + + assert not sound.file.storage.exists(old_name) + + def test_the_original_filename_is_preserved_when_it_was_empty(self): + sound = legacy_sound(self.owner) + + self.run_command() + sound.refresh_from_db() + + assert sound.file_name_original == "Música #1 (final).mp3" + + def test_an_already_generated_name_is_left_alone(self): + sound = legacy_sound(self.owner) + self.run_command() + sound.refresh_from_db() + settled = sound.file.name + + output = self.run_command() + sound.refresh_from_db() + + assert sound.file.name == settled, "a second run must not rename again" + assert "Nothing to do" in output + + def test_dry_run_changes_nothing(self): + sound = legacy_sound(self.owner) + before = sound.file.name + + output = self.run_command(dry_run=True) + sound.refresh_from_db() + + assert sound.file.name == before + assert "would rename" in output + assert "Dry run" in output + + def test_limit_processes_only_the_first_rows(self): + first = legacy_sound(self.owner, "one.mp3") + second = legacy_sound(self.owner, "two.mp3") + + self.run_command(limit=1) + first.refresh_from_db() + second.refresh_from_db() + + assert GENERATED.match(first.file.name) + assert second.file.name == "sounds/two.mp3" + + def test_one_failure_does_not_abort_the_batch(self): + broken = legacy_sound(self.owner, "broken.mp3") + healthy = legacy_sound(self.owner, "healthy.mp3") + real_open = Sound.file.field.attr_class.open + + def fail_for_broken(self_file, *args, **kwargs): + if "broken" in (self_file.name or ""): + raise OSError("cannot read") + return real_open(self_file, *args, **kwargs) + + with mock.patch.object(Sound.file.field.attr_class, "open", fail_for_broken): + output = self.run_command() + + broken.refresh_from_db() + healthy.refresh_from_db() + + assert broken.file.name == "sounds/broken.mp3", "the failure stays untouched" + assert GENERATED.match(healthy.file.name), "the rest of the batch proceeds" + assert "failed" in output + assert "re-run to retry" in output