From 0e1ab3a9218202e00b2ac30f52ef04c7607686f8 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 6 May 2024 14:13:22 +0200 Subject: [PATCH 1/2] Don't treat permission name as a machine readable field, use slug instead This also fixes a discrepancy in behavior between sqlite (case-insensitive like) and postgresql (case-sensitive like), since we expect slugs to always be lowercase. --- extensions/admin.py | 5 +++ ...cense_slug_alter_versionpermission_slug.py | 23 ++++++++++++++ extensions/models.py | 31 +++---------------- 3 files changed, 32 insertions(+), 27 deletions(-) create mode 100644 extensions/migrations/0028_alter_license_slug_alter_versionpermission_slug.py diff --git a/extensions/admin.py b/extensions/admin.py index 37b99080..9813ca8f 100644 --- a/extensions/admin.py +++ b/extensions/admin.py @@ -202,8 +202,13 @@ class TagAdmin(admin.ModelAdmin): return () +class VersionPermissionAdmin(admin.ModelAdmin): + list_display = ('name', 'slug') + + admin.site.register(models.Extension, ExtensionAdmin) admin.site.register(models.Version, VersionAdmin) admin.site.register(models.Maintainer, MaintainerAdmin) admin.site.register(models.License, LicenseAdmin) admin.site.register(models.Tag, TagAdmin) +admin.site.register(models.VersionPermission, VersionPermissionAdmin) diff --git a/extensions/migrations/0028_alter_license_slug_alter_versionpermission_slug.py b/extensions/migrations/0028_alter_license_slug_alter_versionpermission_slug.py new file mode 100644 index 00000000..5bf90ee1 --- /dev/null +++ b/extensions/migrations/0028_alter_license_slug_alter_versionpermission_slug.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.11 on 2024-05-06 12:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('extensions', '0027_unique_preview_files'), + ] + + operations = [ + migrations.AlterField( + model_name='license', + name='slug', + field=models.SlugField(help_text='Should be taken from https://spdx.org/licenses/', unique=True), + ), + migrations.AlterField( + model_name='versionpermission', + name='slug', + field=models.SlugField(help_text='Permissions add-ons are expected to need.', unique=True), + ), + ] diff --git a/extensions/models.py b/extensions/models.py index 33d274c7..cbcd3095 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -19,8 +19,6 @@ from constants.base import ( EXTENSION_TYPE_SLUGS, FILE_STATUS_CHOICES, ) -from constants.licenses import ALL_LICENSES -from constants.version_permissions import ALL_VERSION_PERMISSIONS import common.help_texts import extensions.fields @@ -90,22 +88,13 @@ class License(CreatedModifiedMixin, models.Model): blank=False, null=False, help_text='Should be taken from https://spdx.org/licenses/', + unique=True, ) url = models.URLField(blank=False, null=False) def __str__(self) -> str: return f'{self.name}' - @classmethod - def generate(cls): - """Generate License records from constants.""" - licenses = [cls(id=li.id, name=li.name, slug=li.slug, url=li.url) for li in ALL_LICENSES] - cls.objects.bulk_create(licenses) - - @classmethod - def get_by_name(cls, name: str): - return cls.objects.filter(name__startswith=name).first() - @classmethod def get_by_slug(cls, slug: str): return cls.objects.filter(slug__startswith=slug).first() @@ -386,28 +375,16 @@ class VersionPermission(CreatedModifiedMixin, models.Model): blank=False, null=False, help_text='Permissions add-ons are expected to need.', + unique=True, ) help = models.CharField(max_length=128, null=False, blank=False, unique=True) def __str__(self) -> str: return f'{self.name}' - @classmethod - def generate(cls): - """Generate Permission records from constants.""" - permissions = [ - cls(id=li.id, name=li.name, slug=li.slug, help=li.help) - for li in ALL_VERSION_PERMISSIONS - ] - cls.objects.bulk_create(permissions) - - @classmethod - def get_by_name(cls, name: str): - return cls.objects.filter(name__startswith=name).first() - @classmethod def get_by_slug(cls, slug: str): - return cls.objects.filter(slug__startswith=slug).first() + return cls.objects.filter(slug=slug).first() class Tag(CreatedModifiedMixin, models.Model): @@ -537,7 +514,7 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model return for permission_name in _permissions: - permission = VersionPermission.get_by_name(permission_name) + permission = VersionPermission.get_by_slug(permission_name) # Just ignore versions that are incompatible. if not permission: -- 2.30.2 From c82e3522fd3ae6ccebed872eef6ed30d263f3346 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Mon, 6 May 2024 17:35:06 +0200 Subject: [PATCH 2/2] Don't fail silently in get_by_slug --- extensions/models.py | 7 +------ files/validators.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/extensions/models.py b/extensions/models.py index cbcd3095..8e94036e 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -384,7 +384,7 @@ class VersionPermission(CreatedModifiedMixin, models.Model): @classmethod def get_by_slug(cls, slug: str): - return cls.objects.filter(slug=slug).first() + return cls.objects.get(slug=slug) class Tag(CreatedModifiedMixin, models.Model): @@ -515,11 +515,6 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model for permission_name in _permissions: permission = VersionPermission.get_by_slug(permission_name) - - # Just ignore versions that are incompatible. - if not permission: - continue - self.permissions.add(permission) def set_initial_licenses(self, _licenses): diff --git a/files/validators.py b/files/validators.py index 0fb5c61f..c0d4f965 100644 --- a/files/validators.py +++ b/files/validators.py @@ -54,12 +54,11 @@ class FileMIMETypeValidator: class ExtensionIDManifestValidator: - """ - Make sure the extension id is valid: - * Extension id consists of Unicode letters, numbers or underscores. - * Neither hyphens nor spaces are supported. - * Each extension id most be unique across all extensions. - * All versions of an extension must have the same extension id. + """Make sure the extension id is valid: + * Extension id consists of Unicode letters, numbers or underscores. + * Neither hyphens nor spaces are supported. + * Each extension id most be unique across all extensions. + * All versions of an extension must have the same extension id. """ def __init__(self, manifest, extension_to_be_updated): @@ -307,10 +306,11 @@ class PermissionsValidator: is_error = True else: for permission in value: - if VersionPermission.get_by_slug(permission): - continue - is_error = True - logger.info(f'Permission unavailable: {permission}') + try: + VersionPermission.get_by_slug(permission) + except VersionPermission.DoesNotExist: + is_error = True + logger.info(f'Permission unavailable: {permission}') if not is_error: return -- 2.30.2