Multi-platform: support multiple files per version #201

Merged
Oleg-Komarov merged 43 commits from multi-os into main 2024-07-09 16:27:46 +02:00
33 changed files with 782 additions and 265 deletions

View File

@ -95,20 +95,19 @@
// Create function copyInstallUrl
function copyInstallUrl() {
function init() {
// Create variables
// Create variables single
const btnInstall = document.querySelector('.js-btn-install');
const btnInstallAction = document.querySelector('.js-btn-install-action');
const btnInstallGroup = document.querySelector('.js-btn-install-group');
const btnInstallDrag = document.querySelector('.js-btn-install-drag');
const btnInstallDragGroup = document.querySelector('.js-btn-install-drag-group');
// Create variables multiple
const btnInstallActionItem = document.querySelectorAll('.js-btn-install-action-item');
if (btnInstall == null) {
return;
}
// Get data install URL
const btnInstallUrl = btnInstall.getAttribute('data-install-url');
// Show btnInstallAction
btnInstall.addEventListener('click', function() {
// Hide btnInstallGroup
btnInstallGroup.classList.add('d-none');
@ -117,19 +116,28 @@
btnInstallAction.classList.add('show');
});
// Drag btnInstallUrl
btnInstallDrag.addEventListener('dragstart', function(e) {
// Set data install URL to be transferred during drag
e.dataTransfer.setData('text/plain', btnInstallUrl);
btnInstallActionItem.forEach(function(item) {
// Create variables in function scope
const btnInstallDrag = item.querySelector('.js-btn-install-drag');
const btnInstallDragGroup = item.querySelectorAll('.js-btn-install-drag-group');
// Set drag area active
btnInstallDragGroup.classList.add('opacity-50');
});
// Get data install URL
const btnInstallUrl = item.getAttribute('data-install-url');
// Undrag btnInstallUrl
btnInstallDrag.addEventListener('dragend', function() {
// Set drag area inactive
btnInstallDragGroup.classList.remove('opacity-50');
// Drag btnInstallUrl
btnInstallDrag.addEventListener('dragstart', function(e) {
// Set data install URL to be transferred during drag
e.dataTransfer.setData('text/plain', btnInstallUrl);
// Set drag area active
btnInstallDragGroup.classList.add('opacity-50');
});
// Undrag btnInstallUrl
btnInstallDrag.addEventListener('dragend', function() {
// Set drag area inactive
btnInstallDragGroup.classList.remove('opacity-50');
});
});
}

View File

@ -164,6 +164,12 @@
.btn-accent
box-shadow: .2rem .8rem .8rem rgba(0,82,255,.102), 1.6rem 1.0rem 1.8rem rgba(0,82,255,.102), 3.2rem 1.0rem 3.2rem rgba(0,82,255,.349)
.btn-install-action-item
+margin(3, bottom)
&:last-child
+margin(0, bottom)
.btn-install-drag,
.btn-install-drag:active
transform: initial

View File

@ -7,7 +7,6 @@ from django.core.exceptions import ObjectDoesNotExist, BadRequest
from django.db import models, transaction
from django.db.models import Q, Count
from django.urls import reverse
from django.utils.functional import cached_property
from common.fields import FilterableManyToManyField
from common.model_mixins import CreatedModifiedMixin, RecordDeletionMixin, TrackChangesMixin
@ -253,7 +252,9 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
def update_metadata_from_version(self, version):
update_fields = set()
metadata = version.file.metadata
files = list(version.files.all())
# picking the last uploaded file
metadata = files[-1].metadata
# check if we can also update name
# if we are uploading a new version, we have just validated and don't expect a clash,
# but if a version is being deleted, we want to rollback to a name from an older version,
@ -276,9 +277,9 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
# TODO: check that latest version is currently in review (whatever that means in data?)
latest_version = self.latest_version
file = latest_version.file
file.status = FILE_STATUS_CHOICES.APPROVED
file.save()
for file in latest_version.files.all():
file.status = FILE_STATUS_CHOICES.APPROVED
file.save()
# TODO: check that this extension is currently in review (whatever that means in data?)
self.previews.update(status=FILE_STATUS_CHOICES.APPROVED)
@ -422,8 +423,12 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
)
def suspicious_files(self):
versions = self.versions.all()
return [v.file for v in versions if not v.file.validation.is_ok]
files = []
for version in self.versions.all():
for file in version.files.all():
if not file.validation.is_ok:
files.append(file)
return files
@classmethod
def get_lookup_field(cls, identifier):
@ -439,7 +444,8 @@ class Extension(CreatedModifiedMixin, TrackChangesMixin, models.Model):
for version in versions:
if skip_version and version == skip_version:
continue
if version.file.status not in self.valid_file_statuses:
files = version.files.all()
if not any([file.status in self.valid_file_statuses for file in files]):
continue
latest_version = version
break
@ -667,25 +673,71 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
def __str__(self) -> str:
return f'{self.extension} v{self.version}'
# TODO get rid of this once all places are aware of multi-file reality
#
# the fact that it is cached is important, this allows to use version.file as if it still was a
# model field, i.e. write things like
# version.file.status = ...
# version.file.save()
# if it isn't cached, then the save call is applied to a different, newly fetched File instance
@cached_property
def file(self):
files = list(self.files.all())
if files:
return files[0]
else:
return None
@property
def has_single_file(self):
return len(self.files.all()) == 1
def add_file(self, file: File):
current_platforms = set([p.slug for p in self.platforms.all()])
if not current_platforms:
Oleg-Komarov marked this conversation as resolved Outdated

s/get_available_platforms/get_missing_platforms/?

`s/get_available_platforms/get_missing_platforms/`?
raise ValueError(
f'add_file failed: Version pk={self.pk} is not platform-specific, '
f'no more files allowed'
)
file_platforms = set(file.get_platforms() or [])
if overlap := current_platforms & file_platforms:
raise ValueError(
f'add_file failed: File pk={file.pk} and Version pk={self.pk} have '
f'overlapping platforms: {overlap}'
)
self.files.add(file)
self.update_platforms()
@transaction.atomic
def update_platforms(self):
current_platforms = set([p.slug for p in self.platforms.all()])
files_platforms = set()
for file in self.files.all():
if file_platforms := file.get_platforms():
files_platforms.update(file_platforms)
else:
# we have a cross-platform file - version must not specify platforms
# other files should not exist (validation takes care of that),
# but here we won't double-check it (?)
if len(current_platforms):
self.platforms.delete()
return
Oleg-Komarov marked this conversation as resolved Outdated

or s/get_available_platforms/get_remaining_platforms/

in the sense that these are the platforms which don't yet have files uploaded for them, if any platform-specific file were uploaded

or `s/get_available_platforms/get_remaining_platforms/` in the sense that these are the platforms which don't yet have files uploaded for them, if any platform-specific file were uploaded
if to_create := files_platforms - current_platforms:
for p in to_create:
self.platforms.add(Platform.objects.get(slug=p))
if to_delete := current_platforms - files_platforms:
for p in to_delete:
self.platforms.remove(Platform.objects.get(slug=p))
def get_remaining_platforms(self):
all_platforms = set(p.slug for p in Platform.objects.all())
for file in self.files.all():
platforms = file.get_platforms()
if not platforms:
# no platforms means any platform
return set()
all_platforms -= set(platforms)
return all_platforms
def get_file_for_platform(self, platform):
for file in self.files.all():
platforms = file.get_platforms()
# not platform-specific, matches all platforms
if not platforms:
return file
if platform is None or platform in platforms:
return file
return None
@property
def is_listed(self):
# To be public, version file must have a public status.
return self.file.status == self.file.STATUSES.APPROVED
# To be public, at least one version file must have a public status.
return any([file.status == File.STATUSES.APPROVED for file in self.files.all()])
@property
def cannot_be_deleted_reasons(self) -> List[str]:
@ -699,35 +751,41 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
@property
def permissions_with_reasons(self) -> List[dict]:
"""Returns permissions with reasons, slugs, and names to be shown in templates."""
if 'permissions' not in self.file.metadata:
return []
permissions = []
all_permission_names = {p.slug: p.name for p in VersionPermission.objects.all()}
for slug, reason in self.file.metadata['permissions'].items():
permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]})
"""Returns permissions with reasons, slugs, and names to be shown in templates.
It might make sense to not aggregate at the Version level, and query directly from File.
If we ever restructure multi-platform UI, this method should be revisited.
Normally we can expect that all files have the same permissions, but checking all files,
just in case.
Oleg-Komarov marked this conversation as resolved Outdated

The filename below
filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip'

looks like a better name: doesn't rely on self.__str__.

It also looks like these file names should be the same in both places.

The `filename` below `filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip'` looks like a better name: doesn't rely on `self.__str__`. It also looks like these file names should be the same in both places.
"""
slug2reason = {}
Oleg-Komarov marked this conversation as resolved Outdated

to avoid making platform into file ext, maybe -?

to avoid making platform into file ext, maybe `-`?
for file in self.files.all():
if 'permissions' in file.metadata:
slug2reason.update(file.metadata['permissions'])
if not slug2reason:
return []
all_permission_names = {p.slug: p.name for p in VersionPermission.objects.all()}
permissions = []
for slug, reason in slug2reason.items():
permissions.append({'slug': slug, 'reason': reason, 'name': all_permission_names[slug]})
return permissions
@property
def download_name(self) -> str:
def _get_download_name(self, file) -> str:
"""Return a file name for downloads."""
replace_char = f'{self}'.replace('.', '-')
return f'{utils.slugify(replace_char)}{self.file.suffix}'
parts = [self.extension.type_slug_singular, self.extension.slug, f'v{self.version}']
if platforms := file.get_platforms():
parts.extend(platforms)
return f'{"-".join(parts)}.zip'
@property
def downloadable_signed_url(self) -> str:
# TODO: actual signed URLs?
return self.file.source.url
def download_url(self, append_repository_and_compatibility=True) -> str:
filename = f'{self.extension.type_slug_singular}-{self.extension.slug}-v{self.version}.zip'
def get_download_url(self, file, append_repository_and_compatibility=True) -> str:
filename = self._get_download_name(file)
download_url = reverse(
'extensions:version-download',
'extensions:download',
kwargs={
'type_slug': self.extension.type_slug,
'slug': self.extension.slug,
'version': self.version,
'filehash': file.hash,
'filename': filename,
},
)
@ -738,12 +796,57 @@ class Version(CreatedModifiedMixin, TrackChangesMixin, models.Model):
}
if self.blender_version_max:
params['blender_version_max'] = self.blender_version_max
if platforms := self.platforms.all():
params['platforms'] = ','.join([p.slug for p in platforms])
if platforms := file.get_platforms():
params['platforms'] = ','.join(platforms)
Oleg-Komarov marked this conversation as resolved Outdated

Would it make sense to use file hash in the URL?
This would allow skipping the file-selecting logic in the public view altogether by using the hash to retrieve the file directly.

Would it make sense to use file hash in the URL? This would allow skipping the file-selecting logic in the `public` view altogether by using the hash to retrieve the file directly.
query_string = urlencode(params)
download_url += f'?{query_string}'
return download_url
def get_download_list(self) -> List[dict]:
files = list(self.files.all())
if len(files) == 1:
file = files[0]
return [
{
'name': self._get_download_name(file),
'size': file.size_bytes,
'url': self.get_download_url(file),
}
]
platform2file = {}
for file in files:
platforms = file.get_platforms()
if not platforms:
log.warning(
f'data error: Version pk={self.pk} has multiple files, but File pk={file.pk} '
f'is not platform-specific'
)
for platform in platforms:
platform2file[platform] = file
return [
{
'name': self._get_download_name(file),
'platform': p,
'size': file.size_bytes,
'url': self.get_download_url(file),
}
for p, file in platform2file.items()
]
def get_build_list(self) -> List[dict]:
build_list = []
for file in self.files.all():
platforms = file.get_platforms() or []
build_list.append(
{
'name': self._get_download_name(file),
'platforms': platforms,
'size': file.size_bytes,
'url': self.get_download_url(file),
}
)
return build_list
def get_delete_url(self) -> str:
return reverse(
'extensions:version-delete',

View File

@ -43,10 +43,19 @@ def _delete_versionfiles_file(
logger.info('Deleting File pk=%s of VersionFile pk=%s', file.pk, instance.pk)
file.delete()
if instance.version.files.count() == 0:
# this was the last file, clean up the version
logger.info('Deleting Version pk=%s because its last file was deleted', instance.version.pk)
instance.version.delete()
# this code is already quite convoluted :double-facepalm:
# TODO? maybe find some way to have a predictable order of deletion
try:
instance.version.update_platforms()
if instance.version.files.count() == 0:
# this was the last file, clean up the version
logger.info(
'Deleting Version pk=%s because its last file was deleted',
instance.version.pk,
)
instance.version.delete()
except extensions.models.Version.DoesNotExist:
pass
@receiver(pre_save, sender=extensions.models.Extension)

View File

@ -69,7 +69,13 @@
</div>
<div class="dl-col">
<dt>{% trans 'Size' %}</dt>
<dd>{{ version.file.size_bytes|filesizeformat }}</dd>
{% if version.has_single_file %}
<dd>{{ version.files.first.size_bytes|filesizeformat }}</dd>
{% else %}
{% for file in version.files.all %}
<dd>{{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})</dd>
{% endfor %}
{% endif %}
</div>
</div>

View File

@ -1,10 +1,16 @@
{% load i18n %}
{% if version.platforms.all %}
<div>
Supported platforms:
<ul>
{% for p in version.platforms.all %}
<li>{{p.name}}</li>
{% endfor %}
</ul>
</div>
<div class="dl-row">
<div class="dl-col">
<dt>{% trans "Supported Platforms" %}</dt>
<dd>
<ul class="mb-0 ps-3">
{% for p in version.platforms.all %}
<li>{{p.name}}</li>
{% endfor %}
</ul>
</dd>
</div>
</div>
{% endif %}

View File

@ -187,7 +187,20 @@
</div>
<div class="dl-col">
<dt>{% trans 'Size' %}</dt>
<dd>{{ latest.file.size_bytes|filesizeformat }}</dd>
{% if latest.has_single_file %}
<dd>{{ latest.files.first.size_bytes|filesizeformat }}</dd>
{% else %}
<dd>
<ul class="ps-3">
{% for file in latest.files.all %}
<li class="lh-sm">
{{ file.size_bytes|filesizeformat }}<br>
<span class="fs-sm text-muted">({{ file.get_platforms|join:", " }})</span>
</li>
{% endfor %}
</ul>
</dd>
{% endif %}
</div>
</div>
@ -196,11 +209,12 @@
<dt>{% trans 'Compatibility' %}</dt>
<dd>
{% include "extensions/components/blender_version.html" with version=latest %}
{% include "extensions/components/platforms.html" with version=latest %}
</dd>
</div>
</div>
{% include "extensions/components/platforms.html" with version=latest %}
{% if extension.website %}
<div class="dl-row">
<div class="dl-col">
@ -250,37 +264,45 @@
{% block extension_download %}
<section class="ext-detail-download mt-3">
{% if extension.is_approved %}
{% with download_list=latest.get_download_list %}
<div class="btn-group js-btn-install-group">
<button class="btn btn-flex btn-accent js-btn-install" data-install-url="{{ request.scheme }}://{{ request.get_host }}{{ latest.download_url }}">
<button class="btn btn-flex btn-accent js-btn-install">
<span>{% trans 'Get' %} {{ extension.get_type_display }}</span>
</button>
</div>
<div class="fade js-btn-install-action">
<div class="btn-install-drag-group js-btn-install-drag-group mb-2 rounded">
<button class="btn btn-flex btn-primary btn-install-drag cursor-move js-btn-install-drag w-100" draggable="true">
<i class="i-move"></i>
<span>{% trans 'Drag and Drop into Blender' %}</span>
</button>
</div>
<small class="d-block text-center w-100">
{# TODO @front-end: Replace URL of the manual /dev/ with /latest/. #}
...or <a class="text-underline text-primary" href="{{ request.scheme }}://{{ request.get_host }}{{ latest.download_url }}" download="{{ latest.download_name }}">download</a>
and <a class="text-underline text-primary" href="https://docs.blender.org/manual/en/dev/editors/preferences/extensions.html#install" target="_blank">Install from Disk</a>
</small>
{% for download_item in download_list %}
<div class="btn-install-action-item js-btn-install-action-item" data-install-url="{{ request.scheme }}://{{ request.get_host }}{{ download_item.url }}" download="{{ download_item.name }}">
<div class="btn-install-drag-group js-btn-install-drag-group mb-2 rounded">
<button class="btn btn-flex btn-primary btn-install-drag cursor-move js-btn-install-drag w-100" draggable="true">
<i class="i-move"></i>
<span>{% trans 'Drag and Drop into Blender' %} <span class="fs-sm">{{ download_item.platform }}</span></span>
</button>
</div>
<small class="d-block text-center w-100">
{# TODO @front-end: Replace URL of the manual /dev/ with /latest/. #}
...or <a class="text-underline text-primary" href="{{ request.scheme }}://{{ request.get_host }}{{ download_item.url }}" download="{{ download_item.name }}">download</a>
and <a class="text-underline text-primary" href="https://docs.blender.org/manual/en/dev/editors/preferences/extensions.html#install" target="_blank">Install from Disk</a>
</small>
</div>
{% endfor %}
</div>
{# If JavaScript is disabled. #}
<noscript>
<style>.js-btn-install-group { display: none;}</style>
<div class="btn-col text-center">
<a class="btn btn-flex btn-accent" href="{{ request.scheme }}://{{ request.get_host }}{{ latest.download_url }}" download="{{ latest.download_name }}">
<i class="i-download"></i><span>{% trans 'Download' %} {{ extension.get_type_display }}</span>
{% for download_item in download_list %}
<a class="btn btn-flex btn-accent" href="{{ request.scheme }}://{{ request.get_host }}{{ download_item.url }}" download="{{ download_item.name }}">
<i class="i-download"></i><span>{% trans 'Download' %} {{ extension.get_type_display }} {{ download_item.platform }}</span>
</a>
<small class="mt-3">...and <a class="text-underline text-primary text-center" href="https://docs.blender.org/manual/en/dev/editors/preferences/extensions.html#install" target="_blank">Install from Disk</a></small>
{% endfor %}
</div>
</noscript>
{% else %}
{% endwith %}
{% else %}
<div class="card p-3 mt-3">
<p class="text-info">This {{ extension.get_type_display|lower }} is currently under review.</p>
<p>
@ -290,7 +312,7 @@
<span>{% trans 'Review Extension' %}</span>
</a>
</div>
{% endif %}
{% endif %}
</section>
{% endblock extension_download %}
</aside>

View File

@ -3,7 +3,7 @@
{% block page_title %}{{ extension.name }}{% endblock page_title %}
{% block content %}
{% with author=extension.latest_version.file.user form=form|add_form_classes %}
{% with form=form|add_form_classes %}
<div class="row">
<div class="col-md-8">
<h2>{{ extension.get_type_display }} {% trans 'details' %}</h2>

View File

@ -16,11 +16,7 @@
{% csrf_token %}
{% with form=form|add_form_classes %}
<h2>
{% blocktranslate with version=form.instance.version %}
v{{ version }}
{% endblocktranslate %}
</h2>
<h2>v{{ form.instance.version }}</h2>
<section class="card p-3">
<div class="row">
@ -39,6 +35,21 @@
</div>
{% endif %}
</section>
<section class="card p-3 mt-4">
{% trans 'Builds' %}:
{% with build_list=form.instance.get_build_list %}
<div class="btn-col">
{% for build in build_list %}
<a href="{{ build.url }}" download="{{ build.name }}" class="btn btn-danger btn-block">
<i class="i-download"></i>
<span>
{% trans 'Download' %} v{{ form.instance.version }} {{ build.platforms|join:", " }}
</span>
</a>
{% endfor %}
{% endwith %}
</div>
</section>
</div>
<div class="col-md-4">
<div class="is-sticky">

View File

@ -23,9 +23,11 @@
{% trans 'Upload Extension' %}
{% endif %}
</h1>
{% if extension %}
<p>
<a href="{{ extension.get_absolute_url }}"><strong>{{ extension.name }}</strong></a>
</p>
{% endif %}
<hr>
{% if not extension and drafts %}
<div>

View File

@ -21,7 +21,7 @@
<div class="row ext-version-history pb-3">
<div class="col">
{% for version in extension.versions.all %}
{% for version in object_list %}
{% if version.is_listed or is_maintainer %}
<details {% if forloop.counter == 1 %}open{% endif %} id="v{{ version.version|slugify }}">
<summary>
@ -30,7 +30,13 @@
{% include "extensions/components/blender_version.html" with version=version %}
</span>
<ul class="ms-auto">
<li class="show-on-collapse">{{ version.file.size_bytes|filesizeformat }}</li>
{% if version.has_single_file %}
<li class="show-on-collapse">{{ version.files.first.size_bytes|filesizeformat }}</li>
{% else %}
{% for file in version.files.all %}
<li class="show-on-collapse">{{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})</li>
{% endfor %}
{% endif %}
<li class="show-on-collapse"><i class="i-download"></i> {{ version.download_count }}</li>
<li>
<a href="#v{{ version.version|slugify }}" title="{% firstof version.date_approved|date:'l jS, F Y - H:i' version.date_created|date:'l jS, F Y - H:i' %}">
@ -58,10 +64,12 @@
<dt>{% trans 'Compatibility' %}</dt>
<dd>
{% include "extensions/components/blender_version.html" with version=version %}
{% include "extensions/components/platforms.html" with version=version %}
</dd>
</div>
</div>
{% include "extensions/components/platforms.html" with version=version %}
<div class="dl-row">
<div class="dl-col">
<dt>Downloads</dt>
@ -69,7 +77,13 @@
</div>
<div class="dl-col">
<dt>Size</dt>
<dd>{{ version.file.size_bytes|filesizeformat }}</dd>
{% if version.has_single_file %}
<dd>{{ version.files.first.size_bytes|filesizeformat }}</dd>
{% else %}
{% for file in version.files.all %}
<dd>{{ file.size_bytes|filesizeformat }} ({{ file.get_platforms|join:", " }})</dd>
{% endfor %}
{% endif %}
</div>
</div>
<div class="dl-row">
@ -91,17 +105,22 @@
<div class="dl-row">
<div class="dl-col">
<dt>Status</dt>
<dd>{% include "common/components/status.html" with object=version.file %}</dd>
{# all files of a version get approved together #}
<dd>{% include "common/components/status.html" with object=version.files.first %}</dd>
</div>
</div>
{% endif %}
</section>
<div class="btn-col">
<a href="{{ version.download_url }}" download="{{ version.download_name }}" class="btn btn-primary btn-block">
{% with download_list=version.get_download_list %}
{% for download_item in download_list %}
<a href="{{ download_item.url }}" download="{{ download_item.name }}" class="btn btn-primary btn-block">
<i class="i-download"></i>
<span>{% trans 'Download' %} v{{ version.version }}</span>
<span>{% trans 'Download' %} v{{ version.version }} {{ download_item.platform }}</span>
</a>
{% endfor %}
{% endwith %}
{% if is_maintainer %}
<a href="{{ version.update_url }}" class="btn">
<i class="i-edit"></i><span>Edit</span>

View File

@ -5,8 +5,9 @@ from django.urls import reverse
from rest_framework import status
from rest_framework.test import APITestCase, APIClient
from common.tests.factories.users import UserFactory
from common.tests.factories.extensions import create_approved_version, create_version
from common.tests.factories.files import FileFactory
from common.tests.factories.users import UserFactory
from common.tests.utils import create_user_token
from extensions.models import Extension, Version
@ -48,8 +49,9 @@ class ListedExtensionsTest(APITestCase):
self.assertEqual(self._listed_extensions_count(), 0)
def test_moderate_only_version(self):
self.version.file.status = File.STATUSES.DISABLED
self.version.file.save()
for file in self.version.files.all():
file.status = File.STATUSES.DISABLED
file.save()
self.assertEqual(self._listed_extensions_count(), 0)
@ -150,6 +152,66 @@ class FiltersTest(APITestCase):
).json()
self.assertEqual(len(json['data']), 1)
def test_platform_filter_same_extension(self):
version = create_approved_version(
metadata__platforms=['linux-x64'],
metadata__version='1.0.0',
)
extension = version.extension
version = create_approved_version(
extension=extension,
metadata__platforms=['windows-x64'],
metadata__version='1.0.1',
)
url = reverse('extensions:api')
json = self.client.get(
url + '?platform=linux-x64',
HTTP_ACCEPT='application/json',
).json()
self.assertEqual(len(json['data']), 1)
json = self.client.get(
url + '?platform=windows-x64',
HTTP_ACCEPT='application/json',
).json()
self.assertEqual(len(json['data']), 1)
def test_platform_filter_same_version(self):
version = create_approved_version(
metadata__id='filter_test',
metadata__platforms=['linux-x64'],
metadata__version='1.0.0',
)
version = version.add_file(
FileFactory(
metadata__id='filter_test',
metadata__platforms=['windows-x64'],
metadata__version='1.0.0',
)
)
url = reverse('extensions:api')
json = self.client.get(
url + '?platform=linux-x64',
HTTP_ACCEPT='application/json',
).json()
self.assertEqual(len(json['data']), 1)
json = self.client.get(
url + '?platform=windows-x64',
HTTP_ACCEPT='application/json',
).json()
self.assertEqual(len(json['data']), 1)
json = self.client.get(
url,
HTTP_ACCEPT='application/json',
).json()
self.assertEqual(len(json['data']), 2)
# all archive_url values should be unique
self.assertEqual(len(json['data']), len(set(item['archive_url'] for item in json['data'])))
def test_blender_version_filter_latest_not_max_version(self):
version = create_approved_version(metadata__blender_version_min='4.0.1')
date_created = version.date_created
@ -238,7 +300,8 @@ class VersionUploadAPITest(APITestCase):
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(
response.data['message'],
f'Extension "{other_extension.extension_id}" not maintained by user "{self.user.username}"',
f'Extension "{other_extension.extension_id}" not maintained by user '
f'"{self.user.username}"',
)
def test_version_upload_extension_does_not_exist(self):
@ -288,3 +351,59 @@ class VersionUploadAPITest(APITestCase):
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
self.token.refresh_from_db()
self.assertIsNotNone(self.token.date_last_access)
def test_multiplatform_upload(self):
extension = create_version(
metadata__id="some_addon",
metadata__version="0.0.1",
user=self.user,
).extension
file_linux = TEST_FILES_DIR / 'addon-with-split-platforms-linux.zip'
file_windows = TEST_FILES_DIR / 'addon-with-split-platforms-windows.zip'
with open(file_linux, 'rb') as version_file:
response = self.client.post(
self._get_upload_url('some_addon'),
{
'version_file': version_file,
'release_notes': 'only linux',
},
format='multipart',
HTTP_AUTHORIZATION=f'Bearer {self.token_key}',
)
self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.json())
extension.refresh_from_db()
self.assertEqual(extension.latest_version.files.count(), 1)
self.assertEqual(extension.latest_version.platforms.count(), 1)
with open(file_windows, 'rb') as version_file:
response = self.client.post(
self._get_upload_url('some_addon'),
{
'version_file': version_file,
'release_notes': 'linux and windows',
},
format='multipart',
HTTP_AUTHORIZATION=f'Bearer {self.token_key}',
)
self.assertEqual(response.status_code, status.HTTP_201_CREATED, response.json())
extension.refresh_from_db()
self.assertEqual(extension.latest_version.release_notes, 'linux and windows')
self.assertEqual(extension.latest_version.files.count(), 2)
self.assertEqual(extension.latest_version.platforms.count(), 2)
with open(file_windows, 'rb') as version_file:
response = self.client.post(
self._get_upload_url('some_addon'),
{
'version_file': version_file,
'release_notes': 'linux and windows',
},
format='multipart',
HTTP_AUTHORIZATION=f'Bearer {self.token_key}',
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST, response.json())
self.assertEqual(
response.data['message']['version_file'],
[f'{extension.latest_version} already has files for windows-x64'],
)

View File

@ -26,7 +26,7 @@ class ApproveExtensionTest(TestCase):
# latest_version of approved extension must be listed
# check that we rollback latest_version when file is not approved
file = new_version.file
file = new_version.files.first()
file.status = File.STATUSES.AWAITING_REVIEW
file.save(update_fields={'status'})
new_version.refresh_from_db()
@ -36,8 +36,9 @@ class ApproveExtensionTest(TestCase):
self.assertTrue(extension.is_listed)
# break the first_version, check that nothing is listed anymore
first_version.file.status = File.STATUSES.AWAITING_REVIEW
first_version.file.save()
file = first_version.files.first()
file.status = File.STATUSES.AWAITING_REVIEW
file.save()
self.assertFalse(first_version.is_listed)
extension.refresh_from_db()
self.assertFalse(extension.is_listed)

View File

@ -43,7 +43,7 @@ class DeleteTest(TestCase):
source='images/b0/b03fa981527593fbe15b28cf37c020220c3d83021999eab036b87f3bca9c9168.png',
)
)
version_file = version.file
version_file = version.files.first()
icon = extension.icon
featured_image = extension.featured_image
self.assertEqual(version_file.get_status_display(), 'Awaiting Review')
@ -216,3 +216,14 @@ class DeleteTest(TestCase):
self.assertEqual(response.status_code, 403)
extension.refresh_from_db()
def test_multi_file_version_delete(self):
version = create_version(metadata__platforms=['linux-x64'])
version.add_file(
FileFactory(
metadata__id=version.extension.extension_id,
metadata__version=version.version,
metadata__platforms=['windows-x64'],
)
)
version.extension.delete()

View File

@ -186,7 +186,7 @@ class ValidateManifestTest(CreateFileTest):
self.assertEqual(Extension.objects.count(), 1)
# The same author is to send a new version to thte same extension
self.client.force_login(version.file.user)
self.client.force_login(version.files.first().user)
non_kitsu_1_6 = {
"name": "Blender Kitsu with a different ID",
@ -213,7 +213,7 @@ class ValidateManifestTest(CreateFileTest):
self.assertEqual(Extension.objects.count(), 1)
# The same author is to send a new version to thte same extension
self.client.force_login(version.file.user)
self.client.force_login(version.files.first().user)
kitsu_version_clash = {
"name": "Change the name for lols",
@ -254,7 +254,7 @@ class ValidateManifestTest(CreateFileTest):
)
# The same author is to send a new version to thte same extension
self.client.force_login(version.file.user)
self.client.force_login(version.files.first().user)
updated_kitsu = {
"name": version.extension.name,
@ -692,7 +692,7 @@ class VersionPermissionsTest(CreateFileTest):
self.assertEqual(version_original, extension.latest_version)
# The same author is to send a new version to the same extension but with a new permission
self.client.force_login(version_original.file.user)
self.client.force_login(version_original.files.first().user)
new_kitsu = {
"id": "blender_kitsu",

View File

@ -5,7 +5,9 @@ from django.test import TestCase
from common.admin import get_admin_change_path
from common.log_entries import entries_for
from common.tests.factories.extensions import create_version
from common.tests.factories.files import FileFactory
from common.tests.factories.users import UserFactory
from extensions.models import Platform
class ExtensionTest(TestCase):
@ -70,19 +72,16 @@ class VersionTest(TestCase):
maxDiff = None
fixtures = ['dev', 'licenses']
def setUp(self):
super().setUp()
self.version = create_version(
def test_admin_change_view(self):
version = create_version(
metadata__blender_version_min='2.83.1',
metadata__name='Extension name',
metadata__support='https://example.com/',
metadata__version='1.1.2',
metadata__website='https://example.com/',
)
self.assertEqual(entries_for(self.version).count(), 0)
def test_admin_change_view(self):
path = get_admin_change_path(obj=self.version)
self.assertEqual(entries_for(version).count(), 0)
path = get_admin_change_path(obj=version)
self.assertEqual(path, '/admin/extensions/version/1/change/')
admin_user = UserFactory(is_staff=True, is_superuser=True)
@ -91,6 +90,77 @@ class VersionTest(TestCase):
self.assertEqual(response.status_code, 200, path)
def test_get_remaining_platforms(self):
version_platforms_none = create_version(metadata__platforms=None)
self.assertEqual(version_platforms_none.get_remaining_platforms(), set())
version_platforms_empty = create_version(metadata__platforms=[])
self.assertEqual(version_platforms_empty.get_remaining_platforms(), set())
version_platforms_some = create_version(metadata__platforms=['linux-x64', 'windows-x64'])
self.assertTrue(version_platforms_some.get_remaining_platforms())
version_two_files = create_version(metadata__platforms=['linux-x64'])
file = FileFactory(metadata__platforms=['windows-x64'])
version_two_files.files.add(file)
self.assertTrue(version_two_files.get_remaining_platforms())
version_platforms_all = create_version(
metadata__platforms=[p.slug for p in Platform.objects.all()]
)
self.assertEqual(version_platforms_all.get_remaining_platforms(), set())
skip_platforms = set(['linux-x64', 'windows-x64'])
version_platforms_without_some = create_version(
metadata__platforms=[
p.slug for p in Platform.objects.all() if p.slug not in skip_platforms
]
)
self.assertEqual(version_platforms_without_some.get_remaining_platforms(), skip_platforms)
def test_collect_platforms_across_files(self):
version = create_version(metadata__platforms=['linux-x64'])
file = FileFactory(metadata__platforms=['windows-x64'])
version.add_file(file)
self.assertQuerysetEqual(
version.platforms.order_by('slug'),
Platform.objects.filter(slug__in=['linux-x64', 'windows-x64']).order_by('slug'),
)
file.delete()
version.refresh_from_db()
self.assertQuerysetEqual(
version.platforms.order_by('slug'),
Platform.objects.filter(slug__in=['linux-x64']).order_by('slug'),
)
def test_add_file(self):
version = create_version(metadata__platforms=['linux-x64'])
file = FileFactory(metadata__platforms=['linux-x64'])
with self.assertRaises(ValueError):
version.add_file(file)
all_platforms_version = create_version(metadata__platforms=[])
file = FileFactory(metadata__platforms=['linux-x64'])
with self.assertRaises(ValueError):
all_platforms_version.add_file(file)
def test_get_file_for_platform(self):
version = create_version(metadata__platforms=['linux-x64'])
file = FileFactory(metadata__platforms=['windows-x64'])
version.add_file(file)
self.assertIsNotNone(version.get_file_for_platform(None))
self.assertIsNotNone(version.get_file_for_platform('linux-x64'))
self.assertIsNotNone(version.get_file_for_platform('windows-x64'))
self.assertIsNone(version.get_file_for_platform('windows-arm64'))
file2 = FileFactory(metadata__platforms=['macos-x64', 'macos-arm64'])
version.add_file(file2)
self.assertIsNotNone(version.get_file_for_platform('macos-x64'))
version2 = create_version(metadata__platforms=[])
self.assertIsNotNone(version2.get_file_for_platform(None))
self.assertIsNotNone(version2.get_file_for_platform('macos-x64'))
class UpdateMetadataTest(TestCase):
fixtures = ['dev', 'licenses']

View File

@ -80,7 +80,7 @@ EXPECTED_EXTENSION_DATA = {
'version_str': '0.1.0',
'slug': 'some-addon',
},
'addon-with-split-platforms.zip': {
'addon-with-split-platforms-linux.zip': {
'metadata': {
'tagline': 'Some add-on tag line',
'name': 'Some Add-on',
@ -118,12 +118,14 @@ EXPECTED_VALIDATION_ERRORS = {
'source': ['The manifest file is missing.'],
},
'invalid-manifest-toml.zip': {'source': ['Manifest file contains invalid code.']},
'invalid-theme-multiple-xmls.zip': {'source': ['Themes can only contain <strong>one XML file</strong>.']},
'invalid-theme-multiple-xmls-macosx.zip': {'source': ['Archive contains forbidden files or directories: __MACOSX/']},
'invalid-theme-multiple-xmls.zip': {
'source': ['Themes can only contain <strong>one XML file</strong>.']
},
'invalid-theme-multiple-xmls-macosx.zip': {
'source': ['Archive contains forbidden files or directories: __MACOSX/']
},
'invalid-missing-wheels.zip': {
'source': [
'Python wheel missing: addon/wheels/test-wheel-whatever.whl'
]
'source': ['Python wheel missing: addon/wheels/test-wheel-whatever.whl']
},
}
POST_DATA = {
@ -435,11 +437,11 @@ class SubmitFinaliseTest(CheckFilePropertiesMixin, TestCase):
self.assertEqual(version.release_notes, data['release_notes'])
# Check version file properties
self._test_file_properties(
version.file,
version.files.first(),
content_type='application/zip',
get_status_display='Awaiting Review',
get_type_display='Add-on',
hash=version.file.original_hash,
hash=version.files.first().original_hash,
original_hash='sha256:2831385',
)
# We cannot check for the ManyToMany yet (tags, licences, permissions)
@ -515,7 +517,7 @@ class NewVersionTest(TestCase):
self.assertTrue(response['Location'].startswith('/oauth/login'))
def test_validation_errors_agreed_with_terms_required(self):
self.client.force_login(self.version.file.user)
self.client.force_login(self.version.files.first().user)
with open(TEST_FILES_DIR / 'amaranth-1.0.8.zip', 'rb') as fp:
response = self.client.post(self.url, {'source': fp})
@ -527,7 +529,7 @@ class NewVersionTest(TestCase):
)
def test_validation_errors_invalid_extension(self):
self.client.force_login(self.version.file.user)
self.client.force_login(self.version.files.first().user)
with open(TEST_FILES_DIR / 'empty.txt', 'rb') as fp:
response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True})
@ -539,7 +541,7 @@ class NewVersionTest(TestCase):
)
def test_validation_errors_empty_file(self):
self.client.force_login(self.version.file.user)
self.client.force_login(self.version.files.first().user)
with open(TEST_FILES_DIR / 'empty.zip', 'rb') as fp:
response = self.client.post(self.url, {'source': fp, 'agreed_with_terms': True})
@ -551,7 +553,7 @@ class NewVersionTest(TestCase):
)
def test_upload_new_file_and_finalise_new_version(self):
self.client.force_login(self.version.file.user)
self.client.force_login(self.version.files.first().user)
self.extension.approve()
# Check step 1: upload a new file
@ -570,7 +572,7 @@ class NewVersionTest(TestCase):
self.assertEqual(new_version.version, '1.0.8')
self.assertEqual(new_version.blender_version_min, '4.2.0')
self.assertEqual(new_version.schema_version, '1.0.0')
self.assertEqual(new_version.file.get_status_display(), 'Approved')
self.assertEqual(new_version.files.first().get_status_display(), 'Approved')
self.assertEqual(new_version.release_notes, '')
self.assertEqual(
ApprovalActivity.objects.filter(
@ -595,6 +597,60 @@ class NewVersionTest(TestCase):
self.assertEqual(new_version.release_notes, 'new version')
class MultiPlatformUploadTest(TestCase):
def test_upload_more_files(self):
extension = create_version(
metadata__id="some_addon",
metadata__version="0.0.1",
).extension
file_linux = TEST_FILES_DIR / 'addon-with-split-platforms-linux.zip'
file_windows = TEST_FILES_DIR / 'addon-with-split-platforms-windows.zip'
file_no_platforms = TEST_FILES_DIR / 'addon-without-platforms-for-split-test.zip'
self.client.force_login(extension.authors.all()[0])
with open(file_linux, 'rb') as fp:
response = self.client.post(
extension.get_new_version_url(),
{'source': fp, 'agreed_with_terms': True},
)
self.assertEqual(response.status_code, 302)
extension.refresh_from_db()
self.assertEqual(extension.latest_version.files.count(), 1)
self.assertEqual(extension.latest_version.platforms.count(), 1)
with open(file_windows, 'rb') as fp:
response = self.client.post(
extension.get_new_version_url(),
{'source': fp, 'agreed_with_terms': True},
)
self.assertEqual(response.status_code, 302)
extension.refresh_from_db()
self.assertEqual(extension.latest_version.files.count(), 2)
self.assertEqual(extension.latest_version.platforms.count(), 2)
# can't upload the same file a second time
with open(file_windows, 'rb') as fp:
response = self.client.post(
extension.get_new_version_url(),
{'source': fp, 'agreed_with_terms': True},
)
self.assertEqual(response.status_code, 200)
extension.refresh_from_db()
self.assertEqual(extension.latest_version.files.count(), 2)
self.assertEqual(extension.latest_version.platforms.count(), 2)
# can't upload a platform-agnostic file
with open(file_no_platforms, 'rb') as fp:
response = self.client.post(
extension.get_new_version_url(),
{'source': fp, 'agreed_with_terms': True},
)
self.assertEqual(response.status_code, 200)
extension.refresh_from_db()
self.assertEqual(extension.latest_version.files.count(), 2)
self.assertEqual(extension.latest_version.platforms.count(), 2)
class DraftsWarningTest(TestCase):
fixtures = ['licenses']

View File

@ -216,7 +216,7 @@ class ExtensionManageViewTest(_BaseTestCase):
class UpdateVersionViewTest(_BaseTestCase):
def test_update_view_access(self):
extension = _create_extension()
extension_owner = extension.latest_version.file.user
extension_owner = extension.latest_version.files.first().user
extension.authors.add(extension_owner)
random_user = UserFactory()
@ -246,7 +246,7 @@ class UpdateVersionViewTest(_BaseTestCase):
def test_blender_max_version(self):
extension = _create_extension()
extension_owner = extension.latest_version.file.user
extension_owner = extension.latest_version.files.first().user
extension.authors.add(extension_owner)
self.client.force_login(extension_owner)
url = reverse(

View File

@ -17,7 +17,7 @@ urlpatterns = [
# API
path('api/v1/extensions/', api.ExtensionsAPIView.as_view(), name='api'),
path(
'api/v1/extensions/<str:extension_id>/versions/new/',
'api/v1/extensions/<str:extension_id>/versions/upload/',
api.UploadExtensionVersionView.as_view(),
name='upload-extension-version',
),
@ -28,6 +28,7 @@ urlpatterns = [
path('search/', public.SearchView.as_view(), name='search'),
path('tag/<slug:tag_slug>/', public.SearchView.as_view(), name='by-tag'),
path('team/<slug:team_slug>/', public.SearchView.as_view(), name='by-team'),
path('download/<filehash>/<filename>', public.file_download, name='download'),
re_path(
rf'^(?P<type_slug>{EXTENSION_SLUGS_PATH})/',
include(

View File

@ -12,7 +12,6 @@ from django.db import transaction
from common.compare import is_in_version_range, version
from extensions.models import Extension, Platform
from extensions.utils import clean_json_dictionary_from_optional_fields
from extensions.views.manage import NewVersionView
from files.forms import FileFormSkipAgreed
@ -55,17 +54,16 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
except Platform.DoesNotExist:
self.platform = self.UNKNOWN_PLATFORM
def to_representation(self, instance):
matching_version = None
def find_matching_files_and_version(self, instance):
# avoid triggering additional db queries, reuse the prefetched queryset
versions = [
v
for v in instance.versions.all()
if v.file and v.file.status in instance.valid_file_statuses
]
versions = sorted(
[v for v in instance.versions.all() if v.is_listed],
key=lambda v: v.date_created,
reverse=True,
)
if not versions:
return None
versions = sorted(versions, key=lambda v: v.date_created, reverse=True)
return ([], None)
for v in versions:
if self.blender_version and not is_in_version_range(
self.blender_version,
@ -73,42 +71,48 @@ class ListedExtensionsSerializer(serializers.ModelSerializer):
v.blender_version_max,
):
continue
platform_slugs = set(p.slug for p in v.platforms.all())
# empty platforms field matches any platform filter
# UNKNOWN_PLATFORM matches only empty platforms field
if self.platform and (platform_slugs and self.platform not in platform_slugs):
continue
matching_version = v
break
if self.platform:
if file := v.get_file_for_platform(self.platform):
return ([file], v)
else:
return (v.files.all(), v)
Oleg-Komarov marked this conversation as resolved Outdated

this looks shared with the logic in public view that selects which file to pick from storage, and should probable become a utility or method somewhere.

this looks shared with the logic in `public` view that selects which file to pick from storage, and should probable become a utility or method somewhere.
if not matching_version:
return None
return ([], None)
Oleg-Komarov marked this conversation as resolved Outdated

needs another break here

needs another `break` here
data = {
'id': instance.extension_id,
'schema_version': matching_version.schema_version,
'name': instance.name,
'version': matching_version.version,
'tagline': matching_version.tagline,
'archive_hash': matching_version.file.original_hash,
'archive_size': matching_version.file.size_bytes,
'archive_url': self.request.build_absolute_uri(
matching_version.download_url(append_repository_and_compatibility=False)
),
'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type),
'blender_version_min': matching_version.blender_version_min,
'blender_version_max': matching_version.blender_version_max,
'website': self.request.build_absolute_uri(instance.get_absolute_url()),
# avoid triggering additional db queries, reuse the prefetched queryset
'maintainer': instance.team and instance.team.name or str(instance.authors.all()[0]),
'license': [license_iter.slug for license_iter in matching_version.licenses.all()],
'permissions': matching_version.file.metadata.get('permissions'),
'platforms': [platform.slug for platform in matching_version.platforms.all()],
# TODO: handle copyright
'tags': [str(tag) for tag in matching_version.tags.all()],
}
return clean_json_dictionary_from_optional_fields(data)
def to_representation(self, instance):
matching_files, matching_version = self.find_matching_files_and_version(instance)
result = []
for file in matching_files:
data = {
'id': instance.extension_id,
'schema_version': matching_version.schema_version,
'name': instance.name,
'version': matching_version.version,
'tagline': matching_version.tagline,
'archive_hash': file.original_hash,
'archive_size': file.size_bytes,
'archive_url': self.request.build_absolute_uri(
matching_version.get_download_url(
file,
append_repository_and_compatibility=False,
)
),
'type': EXTENSION_TYPE_SLUGS_SINGULAR.get(instance.type),
'blender_version_min': matching_version.blender_version_min,
'blender_version_max': matching_version.blender_version_max,
'website': self.request.build_absolute_uri(instance.get_absolute_url()),
# avoid triggering additional db queries, reuse the prefetched queryset
'maintainer': (
instance.team and instance.team.name or str(instance.authors.all()[0])
),
'license': [license_iter.slug for license_iter in matching_version.licenses.all()],
'permissions': file.metadata.get('permissions'),
'platforms': file.get_platforms(),
# TODO: handle copyright
'tags': [str(tag) for tag in matching_version.tags.all()],
}
result.append(clean_json_dictionary_from_optional_fields(data))
return result
class ExtensionsAPIView(APIView):
@ -153,7 +157,10 @@ class ExtensionsAPIView(APIView):
request=request,
many=True,
)
data = [e for e in serializer.data if e is not None]
data = []
for entry in serializer.data:
data.extend(entry)
return Response(
{
'blocklist': Extension.objects.blocklisted.values_list('extension_id', flat=True),
@ -201,14 +208,16 @@ class UploadExtensionVersionView(APIView):
status=status.HTTP_403_FORBIDDEN,
)
# Create a NewVersionView instance to handle file creation
new_version_view = NewVersionView(request=request, extension=extension)
# Pass the version_file to the form
form = new_version_view.get_form(FileFormSkipAgreed)
form = FileFormSkipAgreed(
data={},
extension=extension,
request=request,
)
form.fields['source'].initial = version_file
if not form.is_valid():
if 'source' in form.errors:
form.errors['version_file'] = form.errors.pop('source')
return Response({'message': form.errors}, status=status.HTTP_400_BAD_REQUEST)
with transaction.atomic():
@ -217,10 +226,16 @@ class UploadExtensionVersionView(APIView):
file_instance.user = user
file_instance.save()
version = extension.create_version_from_file(
file=file_instance,
release_notes=release_notes,
)
manifest_version = file_instance.metadata['version']
if version := extension.versions.filter(version=manifest_version).first():
version.add_file(file_instance)
version.release_notes = release_notes
version.save(update_fields={'release_notes'})
else:
version = extension.create_version_from_file(
file=file_instance,
release_notes=release_notes,
)
return Response(
{

View File

@ -30,17 +30,9 @@ class VersionsView(ExtensionQuerysetMixin, ListView):
paginate_by = 15
def get_queryset(self):
"""Allow logged in users to view unlisted versions in certain conditions.
* maintainers should be able to preview their yet unlisted versions;
* staff should be able to preview yet unlisted versions;
"""
self.extension_queryset = self.get_extension_queryset()
self.extension = get_object_or_404(self.extension_queryset, slug=self.kwargs['slug'])
queryset = self.extension.versions
if self.request.user.is_staff or self.extension.has_maintainer(self.request.user):
return queryset.all()
return queryset.listed
return self.extension.versions.prefetch_related('files', 'platforms', 'permissions')
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
@ -222,7 +214,11 @@ class NewVersionView(
@transaction.atomic
def form_valid(self, form):
response = super().form_valid(form)
self.extension.create_version_from_file(self.object)
manifest_version = self.object.metadata['version']
if version := self.extension.versions.filter(version=manifest_version).first():
version.add_file(self.object)
else:
version = self.extension.create_version_from_file(self.object)
return response
def get_success_url(self):

View File

@ -8,16 +8,15 @@ from django.shortcuts import get_object_or_404, redirect
from django.utils.translation import gettext_lazy as _
from django.views.generic import DetailView, ListView
from extensions.models import Extension, Version, Tag
from constants.base import (
EXTENSION_TYPE_SLUGS,
EXTENSION_TYPE_PLURAL,
EXTENSION_TYPE_CHOICES,
)
from stats.models import ExtensionView
from extensions.models import Extension, Version, Tag
from files.models import File
from stats.models import ExtensionDownload, ExtensionView, VersionDownload
import ratings.models
from stats.models import ExtensionDownload, VersionDownload
import teams.models
User = get_user_model()
@ -52,16 +51,32 @@ class HomeView(ListedExtensionsView):
def extension_version_download(request, type_slug, slug, version, filename):
"""A backward-compatible url for downloads.
Assuming only a single file for a given version.
"""
extension_version = get_object_or_404(Version, extension__slug=slug, version=version)
file = extension_version.files.first()
return file_download(request, file.hash, filename)
def file_download(request, filehash, filename):
"""Download an extension version and count downloads.
This method processes urls constructed by Version.get_download_list.
The `filename` parameter is used to pass a file name ending with `.zip`.
This is a convention Blender uses to initiate an extension installation on an HTML anchor
drag&drop.
Also see $arg_filename usage in playbooks/templates/nginx/http.conf
"""
extension_version = get_object_or_404(Version, extension__slug=slug, version=version)
# TODO check file status
file = get_object_or_404(File, hash=filehash, version__isnull=False)
extension_version = file.version.first()
ExtensionDownload.create_from_request(request, object_id=extension_version.extension_id)
VersionDownload.create_from_request(request, object_id=extension_version.pk)
return redirect(extension_version.downloadable_signed_url + f'?filename={filename}')
url = file.source.url
return redirect(f'{url}?filename={filename}')
class SearchView(ListedExtensionsView):
@ -227,6 +242,9 @@ class ExtensionDetailView(DetailView):
queryset = Extension.objects.listed.prefetch_related(
'authors',
'latest_version__files',
'latest_version__files__validation',
'latest_version__permissions',
'latest_version__platforms',
'latest_version__tags',
'preview_set',
'preview_set__file',
@ -235,11 +253,6 @@ class ExtensionDetailView(DetailView):
'ratings__ratingreply__user',
'ratings__user',
'team',
'versions',
'versions__files',
'versions__files__validation',
'versions__permissions',
'versions__platforms',
).distinct()
context_object_name = 'extension'
template_name = 'extensions/detail.html'

View File

@ -10,6 +10,7 @@ from django.utils.translation import gettext_lazy as _
from .validators import (
ExtensionIDManifestValidator,
ExtensionNameManifestValidator,
ExtensionVersionManifestValidator,
FileMIMETypeValidator,
ManifestValidator,
)
@ -36,11 +37,15 @@ class FileForm(forms.ModelForm):
# TODO: surface TOML parsing errors?
'invalid_manifest_toml': _('Manifest file contains invalid code.'),
'invalid_missing_init': mark_safe(_('Add-on file missing: <strong>__init__.py</strong>.')),
'missing_or_multiple_theme_xml': mark_safe(_('Themes can only contain <strong>one XML file</strong>.')),
'missing_or_multiple_theme_xml': mark_safe(
_('Themes can only contain <strong>one XML file</strong>.')
),
'invalid_zip_archive': msg_only_zip_files,
'missing_manifest_toml': _('The manifest file is missing.'),
'missing_wheel': _('Python wheel missing: %(path)s'), # TODO <strong>%(path)s</strong>
'forbidden_filepaths': _('Archive contains forbidden files or directories: %(paths)s'), #TODO <strong>%(paths)s</strong>
'missing_wheel': _('Python wheel missing: %(path)s'), # TODO <strong>%(path)s</strong>
'forbidden_filepaths': _(
'Archive contains forbidden files or directories: %(paths)s'
), # TODO <strong>%(paths)s</strong>
}
class Meta:
@ -69,8 +74,8 @@ class FileForm(forms.ModelForm):
)
def __init__(self, *args, **kwargs):
self.request = kwargs.pop('request')
self.extension = kwargs.pop('extension', None)
self.request = kwargs.pop('request')
for field in self.base_fields:
if field not in {'source', 'agreed_with_terms'}:
self.base_fields[field].required = False
@ -160,6 +165,10 @@ class FileForm(forms.ModelForm):
ManifestValidator(manifest)
ExtensionIDManifestValidator(manifest, self.extension)
ExtensionNameManifestValidator(manifest, self.extension)
ExtensionVersionManifestValidator(
manifest,
self.extension,
)
self.cleaned_data['metadata'] = manifest
self.cleaned_data['type'] = EXTENSION_SLUG_TYPES[manifest['type']]

View File

@ -177,15 +177,20 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'website': data.get('website'),
}
@property
def parsed_version_fields(self) -> Dict[str, Any]:
"""Return Version-related data that was parsed from file's content."""
# Currently, the content of the manifest file is the only
# kind of file metadata that is supported.
data = self.metadata
def get_platforms(self):
return self.parse_platforms_from_manifest(self.metadata)
@classmethod
def parse_platforms_from_manifest(self, data):
build_generated_platforms = None
if 'build' in data and 'generated' in data['build']:
build_generated_platforms = data['build']['generated'].get('platforms')
return build_generated_platforms or data.get('platforms')
@property
def parsed_version_fields(self) -> Dict[str, Any]:
"""Return Version-related data that was parsed from file's content."""
data = self.metadata
return {
'version': data.get('version'),
'tagline': data.get('tagline'),
@ -193,7 +198,7 @@ class File(CreatedModifiedMixin, TrackChangesMixin, models.Model):
'schema_version': data.get('schema_version'),
'licenses': data.get('license'),
'permissions': data.get('permissions'),
'platforms': build_generated_platforms or data.get('platforms'),
'platforms': self.get_platforms(),
Oleg-Komarov marked this conversation as resolved Outdated

if keeping it a method is preferable, then get_platforms() might be cleaner,
otherwise making it a platforms property would make sense.

if keeping it a method is preferable, then `get_platforms()` might be cleaner, otherwise making it a `platforms` property would make sense.
'tags': data.get('tags'),
}

View File

@ -16,6 +16,7 @@ from extensions.models import (
)
from constants.base import EXTENSION_TYPE_SLUGS_SINGULAR, EXTENSION_TYPE_CHOICES
from files.utils import guess_mimetype_from_ext, guess_mimetype_from_content
import files.models
logger = logging.getLogger(__name__)
@ -164,6 +165,52 @@ class ExtensionNameManifestValidator:
)
class ExtensionVersionManifestValidator:
"""Validates version existence in db and available platforms."""
def __init__(self, manifest, extension_to_be_updated):
# If the extension wasn't created yet, any version is valid
if not extension_to_be_updated:
return
manifest_version = manifest.get('version')
version = extension_to_be_updated.versions.filter(version=manifest_version).first()
if not version:
return
# for existing versions only submissions for remaining platfroms are valid
remaining_platforms = version.get_remaining_platforms()
if platforms := files.models.File.parse_platforms_from_manifest(manifest):
if diff := set(platforms) - remaining_platforms:
raise ValidationError(
{
'source': [f'{version} already has files for {", ".join(diff)}'],
},
code='invalid',
)
else:
if remaining_platforms:
raise ValidationError(
{
'source': [
f'File upload for {version} is allowed only for remaining platforms: '
f'{", ".join(remaining_platforms)}'
],
},
code='invalid',
)
else:
raise ValidationError(
{
'source': [
f'The version {escape(manifest_version)} was already uploaded for this '
f'extension ({extension_to_be_updated.name})'
],
},
code='invalid',
)
class ManifestFieldValidator:
@classmethod
def validate(cls, *, name: str, value: object, manifest: dict) -> str:
@ -511,30 +558,6 @@ class SchemaVersionValidator(VersionValidator):
)
class VersionVersionValidator(VersionValidator):
example = '1.0.0'
@classmethod
def validate(cls, *, name: str, value: str, manifest: dict) -> str:
"""Return error message if cannot validate, otherwise returns nothing"""
if err_message := super().validate(name=name, value=value, manifest=manifest):
return err_message
extension = Extension.objects.filter(extension_id=manifest.get("id")).first()
# If the extension wasn't created yet, any version is valid
if not extension:
return
version = extension.versions.filter(version=value).first()
if version:
return mark_safe(
f'The version {value} was already uploaded for this extension '
f'({extension.name})'
)
class VersionMinValidator(VersionValidator):
example = '4.2.0'
@ -639,7 +662,7 @@ class ManifestValidator:
'schema_version': SchemaVersionValidator,
'tagline': TaglineValidator,
'type': TypeValidator,
'version': VersionVersionValidator,
'version': VersionValidator,
}
optional_fields = {
'blender_version_max': VersionMaxValidator,

View File

@ -4,7 +4,7 @@
{% block page_title %}{{ extension.name }}{% endblock page_title %}
{% block content %}
{% with latest=extension.latest_version author=extension.latest_version.file.user %}
{% with latest=extension.latest_version %}
<div class="d-flex mt-4">
<h2>
{% with text_ratings_count=extension.text_ratings_count %}

View File

@ -180,12 +180,16 @@
<strong>Try at your own risk.</strong>
</p>
<div class="btn-col">
<a href="{{ extension.latest_version.download_url }}" download="{{ extension.latest_version.download_name }}" class="btn btn-danger btn-block">
{% with build_list=extension.latest_version.get_build_list %}
{% for build in build_list %}
<a href="{{ build.url }}" download="{{ build.name }}" class="btn btn-danger btn-block">
<i class="i-download"></i>
<span>
{% trans 'Download' %} v{{ extension.latest_version.version }}
{% trans 'Download' %} v{{ extension.latest_version.version }} {{ build.platforms|join:", " }}
</span>
</a>
{% endfor %}
{% endwith %}
</div>
</div>
{% endif %}

View File

@ -14,7 +14,7 @@ class CommentsViewTest(TestCase):
self.default_version = version
ApprovalActivity(
type=ApprovalActivity.ActivityType.COMMENT,
user=version.file.user,
user=version.files.first().user,
extension=version.extension,
message='test comment',
).save()

View File

@ -77,9 +77,13 @@ class ExtensionsApprovalDetailView(DetailView):
def get_queryset(self):
return self.model.objects.prefetch_related(
'authors',
'latest_version__files',
'latest_version__files__validation',
'latest_version__permissions',
'latest_version__platforms',
'latest_version__tags',
'preview_set',
'preview_set__file',
'versions',
).all()
def get_context_data(self, **kwargs):

View File

@ -10,7 +10,6 @@ from common.tests.factories.extensions import RatingFactory, create_approved_ver
from common.tests.factories.reviewers import ApprovalActivityFactory
from common.tests.factories.users import OAuthUserFactory, create_moderator, UserFactory
import extensions.models
import files.models
import users.tasks as tasks
User = get_user_model()
@ -83,11 +82,11 @@ class TestTasks(TestCase):
self.assertEqual(response.status_code, 200)
response = self.client.get(version.extension.get_versions_url())
self.assertEqual(response.status_code, 200)
response = self.client.get(version.download_url())
response = self.client.get(version.get_download_url(version.files.first()))
self.assertEqual(response.status_code, 302)
self.assertEqual(
response['Location'],
f'/media/{version.file.original_name}'
f'/media/{version.files.first().original_name}'
f'?filename=add-on-{version.extension.slug}-v{version.version}.zip',
)
# Check that ratings remained publicly accessible
@ -146,10 +145,9 @@ class TestTasks(TestCase):
user.refresh_from_db()
# Check that unlisted extension file and version were deleted
with self.assertRaises(files.models.File.DoesNotExist):
self.authored_unlisted_extension.latest_version.file.refresh_from_db()
with self.assertRaises(extensions.models.Version.DoesNotExist):
self.authored_unlisted_extension.latest_version.refresh_from_db()
self.assertFalse(self.authored_unlisted_extension.latest_version.files.all())
# Check that reports made by this account remained accessible
self.report1.refresh_from_db()