Add information about whether a file is minified to the review… (#13661)
* Add information about whether a file is minified to the reviewers API. Fixes #13589 * Add documentation * experiment, don't delete all validation objects * Fix how we're fetching the FileValidation, use parent.current_file.pk * Increase num queries check for now * Avoid that extra query
This commit is contained in:
Christopher Grebs
GitHub
Родитель
4b740c4bd9
Коммит
e3e846a086
|
|
@ -169,6 +169,7 @@ This endpoint allows you to browse through the contents of an Add-on version.
|
|||
:>json string file.content: Raw content of the requested file.
|
||||
:>json string file.selected_file: The selected file, either from the ``file`` parameter or the default (manifest.json, install.rdf or package.json for Add-ons as well as the XML file for search engines).
|
||||
:>json string|null file.download_url: The download url of the selected file or ``null`` in case of a directory.
|
||||
:>json boolean uses_unknown_minified_code: Indicates that the selected file could be using minified code.
|
||||
:>json array file.entries[]: The complete file-tree of the extracted XPI.
|
||||
:>json int file.entries[].depth: Level of folder-tree depth, starting with 0.
|
||||
:>json string file.entries[].filename: The filename of the file.
|
||||
|
|
@ -214,6 +215,7 @@ This endpoint allows you to compare two Add-on versions with each other.
|
|||
:>json int|null file.entries[].size: The size in bytes.
|
||||
:>json string|null file.entries[].modified: The exact time of the commit, should be equivalent with ``created``.
|
||||
:>json object|null diff: See the following output with inline comments for a complete description.
|
||||
:>json boolean uses_unknown_minified_code: Indicates that the selected file in either the current or the parent version could be using minified code.
|
||||
|
||||
Git patch we're talking about:
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import io
|
|||
import os
|
||||
import mimetypes
|
||||
import pathlib
|
||||
import json
|
||||
from collections import OrderedDict
|
||||
from datetime import datetime
|
||||
|
||||
|
|
@ -27,7 +28,7 @@ from olympia.addons.models import AddonReviewerFlags
|
|||
from olympia.api.fields import ReverseChoiceField, SplitField
|
||||
from olympia.users.models import UserProfile
|
||||
from olympia.files.utils import get_sha256
|
||||
from olympia.files.models import File
|
||||
from olympia.files.models import File, FileValidation
|
||||
from olympia.reviewers.models import CannedResponse
|
||||
from olympia.versions.models import Version
|
||||
from olympia.lib.git import AddonGitRepository, get_mime_type_for_blob
|
||||
|
|
@ -50,13 +51,15 @@ class AddonReviewerFlagsSerializer(serializers.ModelSerializer):
|
|||
|
||||
class FileEntriesSerializer(FileSerializer):
|
||||
content = serializers.SerializerMethodField()
|
||||
uses_unknown_minified_code = serializers.SerializerMethodField()
|
||||
download_url = serializers.SerializerMethodField()
|
||||
entries = serializers.SerializerMethodField()
|
||||
selected_file = serializers.SerializerMethodField()
|
||||
|
||||
class Meta:
|
||||
fields = FileSerializer.Meta.fields + (
|
||||
'content', 'entries', 'selected_file', 'download_url'
|
||||
'content', 'entries', 'selected_file', 'download_url',
|
||||
'uses_unknown_minified_code'
|
||||
)
|
||||
model = File
|
||||
|
||||
|
|
@ -213,6 +216,20 @@ class FileEntriesSerializer(FileSerializer):
|
|||
# more explanation.
|
||||
return ''
|
||||
|
||||
def get_uses_unknown_minified_code(self, obj):
|
||||
try:
|
||||
validation = obj.validation
|
||||
except FileValidation.DoesNotExist:
|
||||
# We don't have any idea about whether it could be minified or not
|
||||
# so let's assume it's not for now.
|
||||
return False
|
||||
|
||||
validation_data = json.loads(validation.validation)
|
||||
|
||||
prop = 'unknownMinifiedFiles'
|
||||
minified_files = validation_data.get('metadata', {}).get(prop, [])
|
||||
return self.get_selected_file(obj) in minified_files
|
||||
|
||||
def get_download_url(self, obj):
|
||||
commit = self._get_commit(obj)
|
||||
tree = self.repo.get_root_tree(commit)
|
||||
|
|
@ -285,10 +302,12 @@ class FileEntriesDiffSerializer(FileEntriesSerializer):
|
|||
entries = serializers.SerializerMethodField()
|
||||
selected_file = serializers.SerializerMethodField()
|
||||
download_url = serializers.SerializerMethodField()
|
||||
uses_unknown_minified_code = serializers.SerializerMethodField()
|
||||
|
||||
class Meta:
|
||||
fields = FileSerializer.Meta.fields + (
|
||||
'diff', 'entries', 'selected_file', 'download_url'
|
||||
'diff', 'entries', 'selected_file', 'download_url',
|
||||
'uses_unknown_minified_code'
|
||||
)
|
||||
model = File
|
||||
|
||||
|
|
@ -384,6 +403,22 @@ class FileEntriesDiffSerializer(FileEntriesSerializer):
|
|||
|
||||
return entries
|
||||
|
||||
def get_uses_unknown_minified_code(self, obj):
|
||||
parent = self.context['parent_version']
|
||||
selected_file = self.get_selected_file(obj)
|
||||
|
||||
for file in [parent.current_file, obj]:
|
||||
try:
|
||||
data = json.loads(file.validation.validation)
|
||||
except FileValidation.DoesNotExist:
|
||||
continue
|
||||
|
||||
prop = 'unknownMinifiedFiles'
|
||||
minified_files = data.get('metadata', {}).get(prop, [])
|
||||
if selected_file in minified_files:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
class AddonCompareVersionSerializer(AddonBrowseVersionSerializer):
|
||||
file = FileEntriesDiffSerializer(source='current_file')
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
import json
|
||||
|
||||
from datetime import datetime
|
||||
|
||||
from django.core.cache import cache
|
||||
|
|
@ -11,6 +13,7 @@ from olympia import amo
|
|||
from olympia.amo.templatetags.jinja_helpers import absolutify
|
||||
from olympia.amo.tests import TestCase, addon_factory, version_factory
|
||||
from olympia.amo.urlresolvers import reverse
|
||||
from olympia.files.models import FileValidation
|
||||
from olympia.lib.git import AddonGitRepository
|
||||
from olympia.lib.tests.test_git import apply_changes
|
||||
from olympia.reviewers.models import CannedResponse
|
||||
|
|
@ -68,6 +71,7 @@ class TestFileEntriesSerializer(TestCase):
|
|||
'filename': 'manifest.json'
|
||||
}
|
||||
))
|
||||
assert not data['uses_unknown_minified_code']
|
||||
|
||||
assert set(data['entries'].keys()) == {
|
||||
'README.md',
|
||||
|
|
@ -211,6 +215,24 @@ class TestFileEntriesSerializer(TestCase):
|
|||
'71d4122c0f2f78e089136602f88dbf590f2fa04bb5bc417454bf21446d6cb4f0')
|
||||
assert data['entries']['icons/LICENSE']['sha256'] is None
|
||||
|
||||
def test_uses_unknown_minified_code(self):
|
||||
validation_data = {
|
||||
'metadata': {
|
||||
'unknownMinifiedFiles': ['content-script.js']
|
||||
}
|
||||
}
|
||||
|
||||
fobj = self.addon.current_version.current_file
|
||||
|
||||
FileValidation.objects.create(
|
||||
file=fobj, validation=json.dumps(validation_data))
|
||||
|
||||
data = self.serialize(fobj, file='content-script.js')
|
||||
assert data['uses_unknown_minified_code']
|
||||
|
||||
data = self.serialize(fobj, file='background-script.js')
|
||||
assert not data['uses_unknown_minified_code']
|
||||
|
||||
|
||||
class TestFileEntriesDiffSerializer(TestCase):
|
||||
def setUp(self):
|
||||
|
|
@ -294,6 +316,7 @@ class TestFileEntriesDiffSerializer(TestCase):
|
|||
'filename': 'manifest.json'
|
||||
}
|
||||
))
|
||||
assert not data['uses_unknown_minified_code']
|
||||
|
||||
assert set(data['entries'].keys()) == {
|
||||
'manifest.json', 'README.md', 'test.txt'}
|
||||
|
|
@ -301,6 +324,8 @@ class TestFileEntriesDiffSerializer(TestCase):
|
|||
# The API always renders a diff, even for unmodified files.
|
||||
assert data['diff'] is not None
|
||||
|
||||
assert not data['uses_unknown_minified_code']
|
||||
|
||||
# Unmodified file
|
||||
manifest_data = data['entries']['manifest.json']
|
||||
assert manifest_data['depth'] == 0
|
||||
|
|
@ -531,6 +556,54 @@ class TestFileEntriesDiffSerializer(TestCase):
|
|||
|
||||
assert data['diff'] is not None
|
||||
|
||||
def test_uses_unknown_minified_code(self):
|
||||
parent_version = self.addon.current_version
|
||||
|
||||
new_version = version_factory(
|
||||
addon=self.addon, file_kw={
|
||||
'filename': 'webextension_no_id.xpi',
|
||||
'is_webextension': True,
|
||||
}
|
||||
)
|
||||
AddonGitRepository.extract_and_commit_from_version(new_version)
|
||||
|
||||
file = self.addon.current_version.current_file
|
||||
|
||||
validation_data = {
|
||||
'metadata': {
|
||||
'unknownMinifiedFiles': ['README.md']
|
||||
}
|
||||
}
|
||||
|
||||
# Let's create a validation for the parent but not the current file
|
||||
# which will result in us notifying the frontend of a minified file
|
||||
# as well
|
||||
current_validation = FileValidation.objects.create(
|
||||
file=file, validation=json.dumps(validation_data))
|
||||
|
||||
data = self.serialize(
|
||||
file, parent_version=parent_version, file='README.md')
|
||||
assert data['uses_unknown_minified_code']
|
||||
|
||||
data = self.serialize(
|
||||
file, parent_version=parent_version, file='manifest.json')
|
||||
assert not data['uses_unknown_minified_code']
|
||||
|
||||
current_validation.delete()
|
||||
|
||||
# Creating a validation object for the current one works as well
|
||||
FileValidation.objects.create(
|
||||
file=parent_version.current_file,
|
||||
validation=json.dumps(validation_data))
|
||||
|
||||
data = self.serialize(
|
||||
file, parent_version=parent_version, file='README.md')
|
||||
assert data['uses_unknown_minified_code']
|
||||
|
||||
data = self.serialize(
|
||||
file, parent_version=parent_version, file='manifest.json')
|
||||
assert not data['uses_unknown_minified_code']
|
||||
|
||||
|
||||
class TestAddonBrowseVersionSerializer(TestCase):
|
||||
def setUp(self):
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче