common/tests/factories/extensions.py

@@ -84,6 +84,18 @@ class VersionFactory(DjangoModelFactory):
         RatingFactory, size=lambda: random.randint(0, 5), factory_related_name='version'
     )
 
+    @factory.post_generation
+    def files(self, create, extracted, **kwargs):
+        if not create:
+            return
+
+        if not extracted:
+            self.files.add(self.file)
+            return
+
+        for file in extracted:
+            self.files.add(file)
+
     @factory.post_generation
     def platforms(self, create, extracted, **kwargs):
         if not create:

extensions/admin.py

@@ -5,7 +5,7 @@ from django.contrib import admin
 
 from . import models
 from common.admin import NoAddDeleteMixin
-from extensions.models import Extension, Maintainer, Version, Tag
+from extensions.models import Extension, Maintainer, Tag, Version, VersionFiles
 
 
 log = logging.getLogger(__name__)
@@ -241,6 +241,12 @@ class TagAdmin(admin.ModelAdmin):
             return ()
 
 
+class VersionFilesAdmin(admin.ModelAdmin):
+    model = VersionFiles
+    list_display = 'file'
+    readonly_fields = 'file'
+
+
 class VersionPermissionAdmin(admin.ModelAdmin):
     list_display = ('name', 'slug')
 

extensions/migrations/0040_version_add_files_field.py

@@ -0,0 +1,45 @@
+# Generated by Django 4.2.11 on 2024-06-17 10:13
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+def populate_files(apps, schema_editor):
+    Version = apps.get_model('extensions', 'Version')
+    for v in Version.objects.all():
+        v.files.add(v.file)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('files', '0010_remove_file_extension_alter_file_hash_and_more'),
+        ('extensions', '0039_alter_extension_status'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='version',
+            name='file',
+            field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='legacy_version', to='files.file'),
+        ),
+        migrations.CreateModel(
+            name='VersionFiles',
+            fields=[
+                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('date_created', models.DateTimeField(auto_now_add=True)),
+                ('date_modified', models.DateTimeField(auto_now=True)),
+                ('file', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='files.file')),
+                ('version', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='extensions.version')),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+        migrations.AddField(
+            model_name='version',
+            name='files',
+            field=models.ManyToManyField(related_name='version', through='extensions.VersionFiles', to='files.file'),
+        ),
+        migrations.RunPython(populate_files, migrations.RunPython.noop)
+    ]

extensions/models.py

@@ -496,10 +496,12 @@ class VersionManager(models.Manager):
             permissions = list(permissions.keys())
         platforms = kwargs.pop('platforms', [])
         tags = kwargs.pop('tags', [])
+        file = kwargs.get('file')  # FIXME legacy_version: replace get with pop
 
         version, result = super().update_or_create(*args, **kwargs)
 
         # Add the ManyToMany to the already initialized Version
+        version.files.add(file)
         version.set_initial_licenses(licenses)
         version.set_initial_permissions(permissions)
         version.set_initial_platforms(platforms)
@@ -515,14 +517,25 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
         'version',
         'licenses',
         'file',
+        'files',
         'tagline',
     }
 
     extension = models.ForeignKey(
         'extensions.Extension', related_name='versions', on_delete=models.CASCADE
     )
+    # TODO drop once data is migrated to `files`
     file = models.OneToOneField(
-        'files.File', related_name='version', on_delete=models.CASCADE, null=False, blank=False
+        'files.File',
+        related_name='legacy_version',
+        on_delete=models.CASCADE,
+        null=False,
+        blank=False,
+    )
+    files = models.ManyToManyField(
+        'files.File',
+        through='VersionFiles',
+        related_name='version',
     )
 
     version = extensions.fields.VersionStringField(
@@ -746,6 +759,11 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
         super().delete(*args, **kwargs)
 
 
+class VersionFiles(CreatedModifiedMixin, models.Model):
+    version = models.ForeignKey(Version, on_delete=models.CASCADE)
+    file = models.OneToOneField(File, on_delete=models.CASCADE)
+
+
 class Maintainer(CreatedModifiedMixin, models.Model):
     extension = models.ForeignKey(Extension, on_delete=models.CASCADE)
     user = models.ForeignKey(User, on_delete=models.CASCADE)

extensions/signals.py

@@ -43,6 +43,23 @@ def _delete_version_file(
     version_file.delete()
 
 
+@receiver(post_delete, sender=extensions.models.VersionFiles)
+def _delete_versionfiles_file(
+    sender: object, instance: extensions.models.VersionFiles, **kwargs: object
+) -> None:
+    # **N.B.**: this isn't part of an overloaded `VersionFiles.delete()` method because
+    # that method isn't called when `Extension.delete()` cascades to deleting the versions:
+    #
+    # delete() method for an object is not necessarily called ... as a result of a cascading delete
+    # https://docs.djangoproject.com/en/4.2/topics/db/models/#overriding-predefined-model-methods
+
+    # FIXME: uncomment when the signal above is cleaned up
+    return
+    version_file = instance.file
+    logger.info('Deleting file pk=%s of VersionFiles pk=%s', version_file.pk, instance.pk)
+    version_file.delete()
+
+
 @receiver(pre_save, sender=extensions.models.Extension)
 def _record_changes(
     sender: object,
@@ -69,8 +86,8 @@ def _update_version(
     if raw:
         return
 
-    if hasattr(instance, 'version'):
-        extension = instance.version.extension
+    if hasattr(instance, 'legacy_version'):
+        extension = instance.legacy_version.extension
         with transaction.atomic():
             # it's important to update is_listed before computing latest_version
             # because latest_version for listed and unlisted extensions are defined differently

extensions/tests/test_delete.py

@@ -73,10 +73,10 @@ class DeleteTest(TestCase):
                     version_file,
                     file_validation,
                     extension,
+                    version,
                     approval_activity,
                     preview_file.preview_set.first(),
                     reused_image.preview_set.first(),
-                    version,
                 ],
             )
         )

extensions/tests/test_manifest.py

@@ -330,7 +330,7 @@ class ValidateManifestTest(CreateFileTest):
 
         self.assertEqual(response.status_code, 302)
         file = File.objects.first()
-        extension = file.version.extension
+        extension = file.legacy_version.extension
         self.assertEqual(extension.slug, 'an-id')
         self.assertEqual(extension.name, 'Name. - With  Extra spaces and other characters Ж')
 

extensions/tests/test_models.py

@@ -98,6 +98,8 @@ class VersionTest(TestCase):
             extension__support='https://example.com/',
         )
         self.assertEqual(entries_for(self.version).count(), 0)
+        # FIXME remove when dropping Version.file field
+        self.assertEqual(self.version.file, self.version.files.first())
 
     def _check_change_message(self):
         entries = entries_for(self.version)

extensions/tests/test_submit.py

@@ -154,8 +154,8 @@ class SubmitFileTest(TestCase):
         self.assertEqual(response.status_code, 302)
         self.assertEqual(File.objects.count(), 1)
         file = File.objects.first()
-        self.assertEqual(response['Location'], file.version.extension.get_draft_url())
-        extension = file.version.extension
+        self.assertEqual(response['Location'], file.legacy_version.extension.get_draft_url())
+        extension = file.legacy_version.extension
         self.assertEqual(extension.slug, slug)
         self.assertEqual(extension.name, name)
         self.assertEqual(file.original_name, file_name)
@@ -166,7 +166,8 @@ class SubmitFileTest(TestCase):
         self.assertEqual(file.metadata['version'], version_str)
         self.assertEqual(file.metadata.get('permissions'), other_metadata.get('permissions'))
         self.assertEqual(
-            [p.slug for p in file.version.platforms.all()], other_metadata.get('platforms', [])
+            [p.slug for p in file.legacy_version.platforms.all()],
+            other_metadata.get('platforms', []),
         )
 
     def test_not_allowed_anonymous(self):
@@ -224,7 +225,7 @@ class SubmitFileTest(TestCase):
         self.assertEqual(response.status_code, 302, _get_all_form_errors(response))
         self.assertEqual(File.objects.count(), 1)
         file = File.objects.first()
-        self.assertEqual(response['Location'], file.version.extension.get_draft_url())
+        self.assertEqual(response['Location'], file.legacy_version.extension.get_draft_url())
         self.assertEqual(file.user, user)
         self.assertEqual(file.original_name, 'theme.zip')
         self.assertEqual(file.size_bytes, 5895)
@@ -305,19 +306,19 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
         )
 
     def test_get_finalise_addon_redirects_if_anonymous(self):
-        response = self.client.post(self.file.version.extension.get_draft_url(), {})
+        response = self.client.post(self.file.legacy_version.extension.get_draft_url(), {})
 
         self.assertEqual(response.status_code, 302)
         self.assertEqual(
             response['Location'],
-            f'/oauth/login?next=/add-ons/{self.file.version.extension.slug}/draft/',
+            f'/oauth/login?next=/add-ons/{self.file.legacy_version.extension.slug}/draft/',
         )
 
     def test_get_finalise_addon_not_allowed_if_different_user(self):
         user = UserFactory()
         self.client.force_login(user)
 
-        response = self.client.post(self.file.version.extension.get_draft_url(), {})
+        response = self.client.post(self.file.legacy_version.extension.get_draft_url(), {})
 
         # Technically this could (should) be a 403, but changing this means changing
         # the MaintainedExtensionMixin which is used in multiple places.
@@ -326,7 +327,7 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
     def test_post_finalise_addon_validation_errors(self):
         self.client.force_login(self.file.user)
         data = {**POST_DATA, 'submit_draft': ''}
-        response = self.client.post(self.file.version.extension.get_draft_url(), data)
+        response = self.client.post(self.file.legacy_version.extension.get_draft_url(), data)
 
         self.assertEqual(response.status_code, 200)
         self.assertDictEqual(
@@ -393,7 +394,7 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
                 'featured-image-source': fp4,
             }
             response = self.client.post(
-                self.file.version.extension.get_draft_url(), {**data, **files}
+                self.file.legacy_version.extension.get_draft_url(), {**data, **files}
             )
 
         self.assertEqual(response.status_code, 302, _get_all_form_errors(response))

files/admin.py

@@ -76,7 +76,7 @@ class FileAdmin(admin.ModelAdmin):
         'date_created',
         'date_modified',
         'date_status_changed',
-        ('version', admin.EmptyFieldListFilter),
+        ('legacy_version', admin.EmptyFieldListFilter),
         ('icon_of', admin.EmptyFieldListFilter),
         ('featured_image_of', admin.EmptyFieldListFilter),
         ('preview', admin.EmptyFieldListFilter),
@@ -87,7 +87,7 @@ class FileAdmin(admin.ModelAdmin):
         'date_created',
         'type',
         'status',
-        link_to('version'),
+        link_to('legacy_version'),
         link_to('icon_of'),
         link_to('featured_image_of'),
         link_to('preview_set.extension', 'preview of'),
@@ -95,9 +95,9 @@ class FileAdmin(admin.ModelAdmin):
     )
 
     list_select_related = (
-        'version__extension',
+        'legacy_version__extension',
         'user',
-        'version',
+        'legacy_version',
         'validation',
     )
     list_prefetch_related = (
@@ -123,8 +123,8 @@ class FileAdmin(admin.ModelAdmin):
         'content_type',
     )
     search_fields = (
-        'version__extension__slug',
-        'version__extension__name',
+        'legacy_version__extension__slug',
+        'legacy_version__extension__name',
         'extensions__slug',
         'extensions__name',
         'original_name',