From 41b420f31c598f441040bfe278d717f661389152 Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Tue, 14 May 2024 13:20:56 +0200 Subject: [PATCH 1/2] wip --- common/tests/factories/extensions.py | 13 +++- extensions/admin.py | 7 ++ .../0030_platform_version_platforms.py | 38 +++++++++++ extensions/models.py | 30 ++++++++- .../extensions/components/platforms.html | 10 +++ extensions/templates/extensions/detail.html | 5 +- .../templates/extensions/version_list.html | 5 +- extensions/tests/test_views.py | 12 ++++ extensions/views/api.py | 66 ++++++++++++------- extensions/views/manage.py | 1 + files/forms.py | 11 +++- files/utils.py | 58 +++++++++------- files/validators.py | 59 ++++++++++++++++- 13 files changed, 264 insertions(+), 51 deletions(-) create mode 100644 extensions/migrations/0030_platform_version_platforms.py create mode 100644 extensions/templates/extensions/components/platforms.html diff --git a/common/tests/factories/extensions.py b/common/tests/factories/extensions.py index 396c8076..1da75db1 100644 --- a/common/tests/factories/extensions.py +++ b/common/tests/factories/extensions.py @@ -6,7 +6,7 @@ from mdgen import MarkdownPostProvider import factory import factory.fuzzy -from extensions.models import Extension, Version, Tag, Preview +from extensions.models import Extension, Version, Tag, Preview, Platform from ratings.models import Rating fake_markdown = Faker() @@ -83,6 +83,17 @@ class VersionFactory(DjangoModelFactory): RatingFactory, size=lambda: random.randint(1, 50), factory_related_name='version' ) + @factory.post_generation + def platforms(self, create, extracted, **kwargs): + if not create: + return + + if not extracted: + return + + tags = Platform.objects.filter(slug__in=extracted) + self.platforms.add(*tags) + @factory.post_generation def tags(self, create, extracted, **kwargs): if not create: diff --git a/extensions/admin.py b/extensions/admin.py index f3b4006a..d5c57fe8 100644 --- a/extensions/admin.py +++ b/extensions/admin.py @@ -127,6 +127,7 @@ class VersionAdmin(admin.ModelAdmin): 'licenses', 'tags', 'permissions', + 'platforms', ) search_fields = ('id', 'extension__slug', 'extension__name') raw_id_fields = ('extension', 'file') @@ -156,6 +157,7 @@ class VersionAdmin(admin.ModelAdmin): 'tags', 'file', 'permissions', + 'platforms', ), }, ), @@ -192,6 +194,10 @@ class LicenseAdmin(admin.ModelAdmin): list_display = ('name', 'slug', 'url') +class PlatformAdmin(admin.ModelAdmin): + list_display = ('name', 'slug') + + class TagAdmin(admin.ModelAdmin): model = Tag list_display = ('name', 'slug', 'type') @@ -212,5 +218,6 @@ 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.Platform, PlatformAdmin) admin.site.register(models.Tag, TagAdmin) admin.site.register(models.VersionPermission, VersionPermissionAdmin) diff --git a/extensions/migrations/0030_platform_version_platforms.py b/extensions/migrations/0030_platform_version_platforms.py new file mode 100644 index 00000000..1bd26b33 --- /dev/null +++ b/extensions/migrations/0030_platform_version_platforms.py @@ -0,0 +1,38 @@ +# Generated by Django 4.2.11 on 2024-05-14 11:06 + +from django.db import migrations, models + + +def populate_platforms(apps, schema_editor): + Platform = apps.get_model('extensions', 'Platform') + for p in ["windows-amd64", "windows-arm64", "macos-x86_64", "macos-arm64", "linux-x86_64"]: + Platform(name=p, slug=p).save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('extensions', '0029_remove_extensionreviewerflags_extension_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='Platform', + 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)), + ('name', models.CharField(max_length=128, unique=True)), + ('slug', models.SlugField(help_text='A platform tag, see https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/', unique=True)), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddField( + model_name='version', + name='platforms', + field=models.ManyToManyField(blank=True, related_name='versions', to='extensions.platform'), + ), + migrations.RunPython(populate_platforms), + ] diff --git a/extensions/models.py b/extensions/models.py index e885f151..4e477b57 100644 --- a/extensions/models.py +++ b/extensions/models.py @@ -99,6 +99,23 @@ class License(CreatedModifiedMixin, models.Model): return cls.objects.filter(slug__startswith=slug).first() +class Platform(CreatedModifiedMixin, models.Model): + name = models.CharField(max_length=128, null=False, blank=False, unique=True) + slug = models.SlugField( + blank=False, + null=False, + help_text='A platform tag, see https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/', # noqa + unique=True, + ) + + def __str__(self) -> str: + return f'{self.name}' + + @classmethod + def get_by_slug(cls, slug: str): + return cls.objects.filter(slug__startswith=slug).first() + + class ExtensionManager(models.Manager): @property def listed(self): @@ -441,8 +458,9 @@ class VersionManager(models.Manager): def update_or_create(self, *args, **kwargs): # Stash the ManyToMany to be created after the Version has a valid ID already - permissions = kwargs.pop('permissions', []) licenses = kwargs.pop('licenses', []) + permissions = kwargs.pop('permissions', []) + platforms = kwargs.pop('platforms', []) tags = kwargs.pop('tags', []) version, result = super().update_or_create(*args, **kwargs) @@ -450,6 +468,7 @@ class VersionManager(models.Manager): # Add the ManyToMany to the already initialized Version version.set_initial_licenses(licenses) version.set_initial_permissions(permissions) + version.set_initial_platforms(platforms) version.set_initial_tags(tags) return version, result @@ -518,6 +537,7 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model ) permissions = models.ManyToManyField(VersionPermission, related_name='versions', blank=True) + platforms = models.ManyToManyField(Platform, related_name='versions', blank=True) release_notes = models.TextField(help_text=common.help_texts.markdown, blank=True) @@ -546,6 +566,14 @@ class Version(CreatedModifiedMixin, RatingMixin, TrackChangesMixin, models.Model permission = VersionPermission.get_by_slug(permission_name) self.permissions.add(permission) + def set_initial_platforms(self, _platforms): + if not _platforms: + return + + for slug in _platforms: + platform = Platform.get_by_slug(slug) + self.platforms.add(platform) + def set_initial_licenses(self, _licenses): if not _licenses: return diff --git a/extensions/templates/extensions/components/platforms.html b/extensions/templates/extensions/components/platforms.html new file mode 100644 index 00000000..037a84d1 --- /dev/null +++ b/extensions/templates/extensions/components/platforms.html @@ -0,0 +1,10 @@ +{% if version.platforms.all %} +
+ Supported platforms: + +
+{% endif %} diff --git a/extensions/templates/extensions/detail.html b/extensions/templates/extensions/detail.html index fdf4896f..dbc24767 100644 --- a/extensions/templates/extensions/detail.html +++ b/extensions/templates/extensions/detail.html @@ -180,7 +180,10 @@
{% trans 'Compatibility' %}
-
{% include "extensions/components/blender_version.html" with version=latest %}
+
+ {% include "extensions/components/blender_version.html" with version=latest %} + {% include "extensions/components/platforms.html" with version=latest %} +
diff --git a/extensions/templates/extensions/version_list.html b/extensions/templates/extensions/version_list.html index d8020f49..aac85e95 100644 --- a/extensions/templates/extensions/version_list.html +++ b/extensions/templates/extensions/version_list.html @@ -56,7 +56,10 @@
{% trans 'Compatibility' %}
-
{% include "extensions/components/blender_version.html" with version=version %}
+
+ {% include "extensions/components/blender_version.html" with version=version %} + {% include "extensions/components/platforms.html" with version=version %} +
diff --git a/extensions/tests/test_views.py b/extensions/tests/test_views.py index 4d84d2bd..459942b6 100644 --- a/extensions/tests/test_views.py +++ b/extensions/tests/test_views.py @@ -112,6 +112,18 @@ class ApiViewsTest(_BaseTestCase): ).json() self.assertEqual(len(json3['data']), 3) + def test_platform_filter(self): + create_approved_version(platforms=['windows-amd64']) + create_approved_version(platforms=['windows-arm64']) + create_approved_version() + url = reverse('extensions:api') + + json = self.client.get( + url + '?platform=windows-amd64', + HTTP_ACCEPT='application/json', + ).json() + self.assertEqual(len(json['data']), 2) + def test_blender_version_filter_latest_not_max_version(self): version = create_approved_version(blender_version_min='4.0.1') version.date_created diff --git a/extensions/views/api.py b/extensions/views/api.py index 9ea768c3..ebddfb48 100644 --- a/extensions/views/api.py +++ b/extensions/views/api.py @@ -7,7 +7,7 @@ from drf_spectacular.utils import OpenApiParameter, extend_schema from django.core.exceptions import ValidationError from common.compare import is_in_version_range, version -from extensions.models import Extension +from extensions.models import Extension, Platform from extensions.utils import clean_json_dictionary_from_optional_fields @@ -20,7 +20,10 @@ log = logging.getLogger(__name__) class ListedExtensionsSerializer(serializers.ModelSerializer): error_messages = { - "invalid_version": "Invalid version: use full semantic versioning like 4.2.0." + "invalid_blender_version": "Invalid blender_version: use full semantic versioning like " + "4.2.0.", + "invalid_platform": "Invalid platform: use notation specified in " + "https://developer.blender.org/docs/features/extensions/schema/1.0.0/", } class Meta: @@ -30,16 +33,22 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): def __init__(self, *args, **kwargs): self.request = kwargs.pop('request', None) self.blender_version = kwargs.pop('blender_version', None) + self.platform = kwargs.pop('platform', None) self._validate() super().__init__(*args, **kwargs) def _validate(self): - if self.blender_version is None: - return - try: - version(self.blender_version) - except ValidationError: - self.fail('invalid_version') + if self.blender_version: + try: + version(self.blender_version) + except ValidationError: + self.fail('invalid_blender_version') + if self.platform: + # FIXME change to an in-memory lookup? + try: + Platform.objects.get(slug=self.platform) + except Platform.DoesNotExist: + self.fail('invalid_platform') def to_representation(self, instance): matching_version = None @@ -52,18 +61,19 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): if not versions: return None versions = sorted(versions, key=lambda v: v.date_created, reverse=True) - if self.blender_version: - for v in versions: - if is_in_version_range( - self.blender_version, - v.blender_version_min, - v.blender_version_max, - ): - matching_version = v - break - else: - # same as latest_version, but without triggering a new queryset - matching_version = versions[0] + for v in versions: + if self.blender_version and not is_in_version_range( + self.blender_version, + v.blender_version_min, + v.blender_version_max, + ): + continue + platform_slugs = set(p.slug for p in v.platforms.all()) + # empty platforms field matches any platform filter + if self.platform and not (not platform_slugs or self.platform in platform_slugs): + continue + matching_version = v + break if not matching_version: return None @@ -85,6 +95,7 @@ class ListedExtensionsSerializer(serializers.ModelSerializer): 'maintainer': str(instance.authors.all()[0]), 'license': [license_iter.slug for license_iter in matching_version.licenses.all()], 'permissions': [permission.slug for permission in matching_version.permissions.all()], + 'platforms': [platform.slug for platform in matching_version.platforms.all()], # TODO: handle copyright 'tags': [str(tag) for tag in matching_version.tags.all()], } @@ -101,21 +112,32 @@ class ExtensionsAPIView(APIView): name="blender_version", description=("Blender version to check for compatibility"), type=str, - ) + ), + OpenApiParameter( + name="platform", + description=("Platform to check for compatibility"), + type=str, + ), ] ) def get(self, request): blender_version = request.GET.get('blender_version') + platform = request.GET.get('platform') qs = Extension.objects.listed.prefetch_related( 'authors', 'versions', 'versions__file', 'versions__licenses', 'versions__permissions', + 'versions__platforms', 'versions__tags', ).all() serializer = self.serializer_class( - qs, blender_version=blender_version, request=request, many=True + qs, + blender_version=blender_version, + platform=platform, + request=request, + many=True, ) data = [e for e in serializer.data if e is not None] return Response( diff --git a/extensions/views/manage.py b/extensions/views/manage.py index d47d0038..1a37298c 100644 --- a/extensions/views/manage.py +++ b/extensions/views/manage.py @@ -45,6 +45,7 @@ class ExtensionDetailView(ExtensionQuerysetMixin, DetailView): 'versions__file', 'versions__file__validation', 'versions__permissions', + 'versions__platforms', ) def get_object(self, queryset=None): diff --git a/files/forms.py b/files/forms.py index 50aac976..e9f7906b 100644 --- a/files/forms.py +++ b/files/forms.py @@ -39,6 +39,7 @@ class FileForm(forms.ModelForm): 'missing_or_multiple_theme_xml': _('A theme should have exactly one XML file.'), 'invalid_zip_archive': msg_only_zip_files, 'missing_manifest_toml': _('The manifest file is missing.'), + 'missing_wheel': _('A declared wheel is missing in the zip file, expected_path: {path}'), } class Meta: @@ -143,7 +144,15 @@ class FileForm(forms.ModelForm): manifest, error_codes = utils.read_manifest_from_zip(file_path) for code in error_codes: - errors.append(forms.ValidationError(self.error_messages[code])) + if isinstance(code, dict): + errors.append( + forms.ValidationError( + self.error_messages[code['code']], + params=code['params'], + ) + ) + else: + errors.append(forms.ValidationError(self.error_messages[code])) if errors: self.add_error('source', errors) diff --git a/files/utils.py b/files/utils.py index c82f77f5..41eb2d9d 100644 --- a/files/utils.py +++ b/files/utils.py @@ -108,6 +108,9 @@ def read_manifest_from_zip(archive_path): """ manifest_name = 'blender_manifest.toml' error_codes = [] + file_list = [] + manifest_content = None + try: with zipfile.ZipFile(archive_path) as myzip: bad_file = myzip.testzip() @@ -129,34 +132,45 @@ def read_manifest_from_zip(archive_path): error_codes.append('invalid_manifest_path') return None, error_codes - # Extract the file content with myzip.open(manifest_filepath) as file_content: - toml_content = toml.loads(file_content.read().decode()) - - # If manifest was parsed successfully, do additional type-specific validation - type_slug = toml_content['type'] - if type_slug == 'theme': - theme_xmls = filter_paths_by_ext(file_list, '.xml') - if len(list(theme_xmls)) != 1: - error_codes.append('missing_or_multiple_theme_xml') - elif type_slug == 'add-on': - # __init__.py is expected to be next to the manifest - expected_init_path = os.path.join(os.path.dirname(manifest_filepath), '__init__.py') - init_filepath = find_exact_path(file_list, expected_init_path) - if not init_filepath: - error_codes.append('invalid_missing_init') - - return toml_content, error_codes - - except toml.decoder.TomlDecodeError as e: - logger.error(f"Manifest Error: {e.msg}") - error_codes.append('invalid_manifest_toml') + manifest_content = file_content.read().decode() except Exception as e: logger.error(f"Error extracting from archive: {e}") error_codes.append('invalid_zip_archive') + return None, error_codes - return None, error_codes + try: + toml_content = toml.loads(manifest_content) + except toml.decoder.TomlDecodeError as e: + logger.error(f"Manifest Error: {e.msg}") + error_codes.append('invalid_manifest_toml') + return None, error_codes + + # If manifest was parsed successfully, do additional type-specific validation + type_slug = toml_content['type'] + if type_slug == 'theme': + theme_xmls = filter_paths_by_ext(file_list, '.xml') + if len(list(theme_xmls)) != 1: + error_codes.append('missing_or_multiple_theme_xml') + elif type_slug == 'add-on': + # __init__.py is expected to be next to the manifest + expected_init_path = os.path.join(os.path.dirname(manifest_filepath), '__init__.py') + init_filepath = find_exact_path(file_list, expected_init_path) + if not init_filepath: + error_codes.append('invalid_missing_init') + + wheels = toml_content.get('wheels') + if wheels: + for wheel in wheels: + expected_wheel_path = os.path.join(os.path.dirname(manifest_filepath), wheel) + wheel_filepath = find_exact_path(file_list, expected_wheel_path) + if not wheel_filepath: + error_codes.append( + {'code': 'missing_wheel', 'params': {'path': expected_wheel_path}} + ) + + return toml_content, error_codes def guess_mimetype_from_ext(file_name: str) -> str: diff --git a/files/validators.py b/files/validators.py index 04083b5c..42f5220e 100644 --- a/files/validators.py +++ b/files/validators.py @@ -7,7 +7,13 @@ from django.utils.deconstruct import deconstructible from django.utils.html import escape from django.utils.safestring import mark_safe -from extensions.models import Extension, License, VersionPermission, Tag +from extensions.models import ( + Extension, + License, + Platform, + Tag, + VersionPermission, +) from constants.base import EXTENSION_TYPE_SLUGS_SINGULAR, EXTENSION_TYPE_CHOICES from files.utils import guess_mimetype_from_ext, guess_mimetype_from_content @@ -361,6 +367,53 @@ class PermissionsValidator: return mark_safe(error_message) +class PlatformsValidator: + """See https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/""" + + example = ["windows-amd64", "linux-x86_64"] + + @classmethod + def validate(cls, *, name: str, value: list[str], manifest: dict) -> str: + """Return error message if there is any license that is not accepted by the site""" + is_error = False + error_message = "" + + unknown_value = None + if type(value) != list: + is_error = True + else: + for platform in value: + if Platform.get_by_slug(platform): + continue + is_error = True + unknown_value = platform + break + + if not is_error: + return + + error_message = mark_safe( + f'Manifest value error: platforms expects a list of ' + f'supported platforms. e.g., {cls.example}.' + ) + if unknown_value: + error_message += mark_safe(f' Unknown value: {escape(unknown_value)}.') + + return error_message + + +class WheelsValidator: + example = ["./wheels/mywheel-v1.0.0-py3-none-any.whl"] + + @classmethod + def validate(cls, *, name: str, value: list[str], manifest: dict) -> str: + if type(value) != list: + return mark_safe( + f'Manifest value error: wheels expects a list of ' + f'wheel files . e.g., {cls.example}.' + ) + + class VersionValidator: example = '1.0.0' @@ -489,10 +542,12 @@ class ManifestValidator: } optional_fields = { 'blender_version_max': VersionMaxValidator, - 'website': StringValidator, 'copyright': ListValidator, 'permissions': PermissionsValidator, + 'platforms': PlatformsValidator, 'tags': TagsValidator, + 'website': StringValidator, + 'wheels': WheelsValidator, } all_fields = {**mandatory_fields, **optional_fields} -- 2.30.2 From 77a1adc44d92ca36ada365c56bd34e423a78aa1d Mon Sep 17 00:00:00 2001 From: Oleg Komarov Date: Thu, 16 May 2024 16:06:53 +0200 Subject: [PATCH 2/2] tests --- extensions/tests/files/invalid-missing-wheels.zip | Bin 0 -> 741 bytes extensions/tests/test_submit.py | 9 +++++++-- files/forms.py | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 extensions/tests/files/invalid-missing-wheels.zip diff --git a/extensions/tests/files/invalid-missing-wheels.zip b/extensions/tests/files/invalid-missing-wheels.zip new file mode 100644 index 0000000000000000000000000000000000000000..76c124847cc83a995de279142ec88ea29645356c GIT binary patch literal 741 zcmWIWW@h1H00E7<4G~}llwf0!VMt6#$gTj!<=D(6)(JbiYk{d2^Ujb00W%vlwi5d2~PfpcG; z$z8daJjd{VSnt8mujhD{>P3i0@;!6Pt5|Zfip5mf_onK;w5eX2o7MHN*&o`?sVw?P zbRPfj>31JLTsiSh+S%UiJ(BlCYL{(E({tu7UKD%#k@@KvE|YiWrC+Pb`u$KWEK_e)-e68?hl z@QaVn%*!l^kJl@x1czGi28%QhMhmq7Z$>6LW?Yde0gYAxhQE#=CM;@MAyJFfEL;%_ zF$);S3|kr<;AY{7W_%Xl@;S`0APX9R_COqq5)MFPv4#OF8_+=v%s|Kj)OZ@?NCp4_ CJKl%@ literal 0 HcmV?d00001 diff --git a/extensions/tests/test_submit.py b/extensions/tests/test_submit.py index f4edfb44..b60ec00d 100644 --- a/extensions/tests/test_submit.py +++ b/extensions/tests/test_submit.py @@ -84,6 +84,11 @@ EXPECTED_VALIDATION_ERRORS = { }, 'invalid-manifest-toml.zip': {'source': ['Could not parse the manifest file.']}, 'invalid-theme-multiple-xmls.zip': {'source': ['A theme should have exactly one XML file.']}, + 'invalid-missing-wheels.zip': { + 'source': [ + 'A declared wheel is missing in the zip file, expected path: addon/./wheels/test-wheel-whatever.whl' + ] + }, } POST_DATA = { 'preview_set-TOTAL_FORMS': ['0'], @@ -169,13 +174,13 @@ class SubmitFileTest(TestCase): user = UserFactory() self.client.force_login(user) - for test_archive, extected_errors in EXPECTED_VALIDATION_ERRORS.items(): + for test_archive, expected_errors in EXPECTED_VALIDATION_ERRORS.items(): with self.subTest(test_archive=test_archive): with open(TEST_FILES_DIR / test_archive, 'rb') as fp: response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True}) self.assertEqual(response.status_code, 200) - self.assertDictEqual(response.context['form'].errors, extected_errors) + self.assertDictEqual(response.context['form'].errors, expected_errors) def test_addon_without_top_level_directory(self): self.assertEqual(Extension.objects.count(), 0) diff --git a/files/forms.py b/files/forms.py index e9f7906b..a2798787 100644 --- a/files/forms.py +++ b/files/forms.py @@ -39,7 +39,7 @@ class FileForm(forms.ModelForm): 'missing_or_multiple_theme_xml': _('A theme should have exactly one XML file.'), 'invalid_zip_archive': msg_only_zip_files, 'missing_manifest_toml': _('The manifest file is missing.'), - 'missing_wheel': _('A declared wheel is missing in the zip file, expected_path: {path}'), + 'missing_wheel': _('A declared wheel is missing in the zip file, expected path: %(path)s'), } class Meta: -- 2.30.2