Commit 900a99b3 authored by Raphaël Hertzog's avatar Raphaël Hertzog

Merge branch 'table-performance-improvements' into 'master'

Table performance improvements See merge request !42
parents f402fe3d 08103f78
Pipeline #13411 failed with stage
in 240 minutes 5 seconds
......@@ -11,9 +11,9 @@
import logging
import importlib
from django.utils.functional import cached_property
from django.db.models import Prefetch
from django.conf import settings
from django.template.loader import get_template
from distro_tracker import vendor
from django.core.exceptions import ObjectDoesNotExist
......@@ -39,11 +39,7 @@ class BaseTableField(metaclass=PluginRegistry):
``tracker_package_tables`` module at the top level of a Django app.
"""
def __init__(self, package):
self.package = package
@property
def context(self):
def context(self, package):
"""
Should return a dictionary representing context variables necessary for
the package table field.
......@@ -67,22 +63,15 @@ class BaseTableField(metaclass=PluginRegistry):
"""
return None
@property
def html_output(self):
"""
If the field does not want to use a template, it can return rendered
HTML in this property. The HTML needs to be marked safe or else it will
be escaped in the final output.
"""
return None
@property
def has_content(self):
def render(self, package, context=None, request=None):
"""
Returns a bool indicating whether the table actually has any content to
display for the package.
Render the field's HTML output for the given package.
"""
return True
if not hasattr(self, '_template'):
self._template = get_template(self.template_name)
if context is None:
context = self.context(package)
return self._template.render(context, request)
@property
def prefetch_related_lookups():
......@@ -124,25 +113,24 @@ class GeneralInformationTableField(BaseTableField):
),
]
@cached_property
def context(self):
def context(self, package):
try:
info = self.package.general_data[0]
info = package.general_data[0]
except IndexError:
# There is no general info for the package
return {
'url': self.package.get_absolute_url,
'name': self.package.name
'url': package.get_absolute_url,
'name': package.name
}
general = info.value
general['url'] = self.package.get_absolute_url
general['url'] = package.get_absolute_url
# Add developer information links and any other vendor-specific extras
general = add_developer_extras(general)
general = add_developer_extras(general, url_only=True)
try:
info = self.package.binaries_data[0]
info = package.binaries_data[0]
general['binaries'] = info.value
except IndexError:
general['binaries'] = []
......@@ -187,7 +175,7 @@ class VcsTableField(BaseTableField):
Prefetch(
'data',
queryset=PackageData.objects.filter(key='general'),
to_attr='general_vcs_data'
to_attr='general_data'
)
]
......@@ -198,22 +186,22 @@ class VcsTableField(BaseTableField):
'DISTRO_TRACKER_VCS_TABLE_FIELD_TEMPLATE',
self._default_template_name)
@cached_property
def context(self):
def context(self, package):
try:
info = self.package.general_vcs_data[0]
info = package.general_data[0]
except IndexError:
# There is no general info for the package
return
general = info.value
# Map the VCS type to its name.
if 'vcs' in general:
general = {}
if 'vcs' in info.value:
general['vcs'] = info.value['vcs']
# Map the VCS type to its name.
shorthand = general['vcs'].get('type', 'Unknown')
general['vcs']['full_name'] = get_vcs_name(shorthand)
result, implemented = vendor.call(
'get_vcs_data', self.package)
'get_vcs_data', package)
if implemented:
general.update(result)
......@@ -234,7 +222,7 @@ class ArchiveTableField(BaseTableField):
Prefetch(
'data',
queryset=PackageData.objects.filter(key='general'),
to_attr='general_archive_data'
to_attr='general_data'
),
Prefetch(
'data',
......@@ -243,18 +231,19 @@ class ArchiveTableField(BaseTableField):
)
]
@cached_property
def context(self):
def context(self, package):
try:
info = self.package.general_archive_data[0]
info = package.general_data[0]
except IndexError:
# There is no general info for the package
return
general = info.value
general = {}
if 'version' in info.value:
general['version'] = info.value['version']
try:
info = self.package.versions[0].value
info = package.versions[0].value
general['default_pool_url'] = info['default_pool_url']
except IndexError:
# There is no versions info for the package
......@@ -271,11 +260,10 @@ class BugStatsTableField(BaseTableField):
template_name = 'core/package-table-fields/bugs.html'
prefetch_related_lookups = ['bug_stats']
@cached_property
def context(self):
def context(self, package):
stats = {}
try:
stats['bugs'] = self.package.bug_stats.stats
stats['bugs'] = package.bug_stats.stats
except ObjectDoesNotExist:
stats['all'] = 0
return stats
......@@ -317,7 +305,6 @@ class BasePackageTable(metaclass=PluginRegistry):
self.scope = scope
self.limit = limit
@property
def context(self):
"""
Should return a dictionary representing context variables necessary for
......@@ -354,9 +341,15 @@ class BasePackageTable(metaclass=PluginRegistry):
Returns the list of packages with prefetched relationships defined by
table fields
"""
attributes_name = set()
package_query_set = self.packages
for field in self.table_fields:
for l in field.prefetch_related_lookups:
if isinstance(l, Prefetch):
if l.to_attr in attributes_name:
continue
else:
attributes_name.add(l.to_attr)
package_query_set = package_query_set.prefetch_related(l)
additional_data, implemented = vendor.call(
......@@ -419,10 +412,18 @@ class BasePackageTable(metaclass=PluginRegistry):
if self.limit:
packages = packages[:self.limit]
fields = [f() for f in self.table_fields]
context = {
'field': {
'context': ''
},
}
for package in packages:
context['package'] = package
row = []
for field_class in self.table_fields:
row.append(field_class(package))
for field in fields:
context['field']['context'] = field.context(package)
row.append(field.render(package, context))
rows_list.append(row)
return rows_list
......
{% if field.context.all == 0 %}
<b class='center'>{{field.context.all}}</b>
{% else %}
<span class="popover-hover cursor-pointer text-primary" id="bugs-field" data-toggle="popover" data-placement="left" title="All bugs" data-content='{% include "core/package-table-fields/popovers/bugs.html" with field=field %}'>
<span class="popover-hover cursor-pointer text-primary" id="bugs-field" data-toggle="popover" data-placement="left" title="All bugs" data-content='
<p>
{% for bug in field.context.bugs %}
<span class="font-weight-bold">{{ bug.category_name}}:</span> {{ bug.bug_count }}
<br>
{% endfor %}
'>
<b class='center'>{{field.context.all}}</b>
</span>
{% endif %}
......
<span class="popover-hover cursor-pointer text-primary" id="general-field" data-toggle="popover" data-placement="right" title="<a href='{{ field.context.url }}'>{{ field.context.name }}</a>" data-content='{% include "core/package-table-fields/popovers/general.html" with field=field %}'>
<span class="popover-hover cursor-pointer text-primary" id="general-field" data-toggle="popover" data-placement="right" title="<a href='{{ field.context.url }}'>{{ field.context.name }}</a>" data-content='
<p>
<span class="font-weight-bold">version:</span> {{ field.context.version }} <br>
{% if field.context.maintainer %}
<span class="font-weight-bold">maintainer:</span>
{% with mailto="mailto:"|add:field.context.maintainer.email %}
{% with url=field.context.maintainer.developer_info_url|default:mailto %}
<a href="{{ url }}">{{ field.context.maintainer.name }}</a>
{% endwith %}{% endwith %}
<br>
{% endif %}
{% if field.context.uploaders %}
<span class="font-weight-bold">uploaders:</span>
{% for uploader in field.context.uploaders %}
{% with mailto="mailto:"|add:uploader.email %}
{% with url=uploader.developer_info_url|default:mailto %}
<br><a href="{{ url }}">{{ uploader.name }}</a>
{% endwith %}{% endwith %}
{% endfor %}
<br>
{% endif %}
{% if field.context.architectures %}
<span class="font-weight-bold">arch:</span>
{{ field.context.architectures|join:", " }}
<br>
{% endif %}
{% if field.context.standards_version %}
<span class="font-weight-bold">std-ver:</span>
{{ field.context.standards_version }}
<br>
{% endif %}
</p>
<p>
{% with items=field.context.binaries|dictsort:"name"|slice:":5" %}
<span class="font-weight-bold">Binaries ({{field.context.binaries|length}}):</span>
{% for item in items %}
<br>
{% with url=item.url|default:"#" %}
<a href="{{ url }}" title="{{ item.repository_name }}">{{ item.name }}</a>
{% endwith %}
{% endfor %}
{% with count=field.context.binaries|length %}
{% if count > 5 %}
<br>
<a href="{{ field.context.url }}"><small>More...</small></a>
{% endif %}
{% endwith %}
{% endwith %}
</p>
'>
<a href='{{ field.context.url }}'>{{ field.context.name }}</a>
</span>
<p>
{% for bug in field.context.bugs %}
<span class="font-weight-bold">{{ bug.category_name}}:</span> {{ bug.bug_count }}
<br>
{% endfor %}
<p>
<span class="font-weight-bold">version:</span> {{ field.context.version }} <br>
{% if field.context.maintainer %}
<span class="font-weight-bold">maintainer:</span>
{% with mailto="mailto:"|add:field.context.maintainer.email %}
{% with url=field.context.maintainer.developer_info_url|default:mailto %}
<a href="{{ url }}">{{ field.context.maintainer.name }}</a>
{% endwith %}{% endwith %}
<br>
{% endif %}
{% if field.context.uploaders %}
<span class="font-weight-bold">uploaders:</span>
{% for uploader in field.context.uploaders %}
{% with mailto="mailto:"|add:uploader.email %}
{% with url=uploader.developer_info_url|default:mailto %}
<br><a href="{{ url }}">{{ uploader.name }}</a>
{% endwith %}{% endwith %}
{% endfor %}
<br>
{% endif %}
{% if field.context.architectures %}
<span class="font-weight-bold">arch:</span>
{{ field.context.architectures|join:', ' }}
<br>
{% endif %}
{% if field.context.standards_version %}
<span class="font-weight-bold">std-ver:</span>
{{ field.context.standards_version }}
<br>
{% endif %}
</p>
<p>
{% with items=field.context.binaries|dictsort:'name'|slice:":5" %}
<span class="font-weight-bold">Binaries ({{field.context.binaries|length}}):</span>
{% for item in items %}
<br>
{% with url=item.url|default:"#" %}
<a href="{{ url }}" title="{{ item.repository_name }}">{{ item.name }}</a>
{% endwith %}
{% endfor %}
{% with count=field.context.binaries|length %}
{% if count > 5 %}
<br>
<a href="{{ field.context.url }}"><small>More...</small></a>
{% endif %}
{% endwith %}
{% endwith %}
</p>
......@@ -13,13 +13,9 @@
{% with rows=table.rows %}
{% for row in rows %}
<tr scope='row'>
{% for field in row %}
{% for cell in row %}
<td class='center' scope='col'>
{% if field.template_name %}
{% include field.template_name with field=field %}
{% else %}
{{ field.html_output }}
{% endif %}
{{ cell }}
</td>
{% endfor %}
</tr>
......
......@@ -82,13 +82,13 @@ class GeneralInformationTableFieldTests(TestCase):
self.package = create_source_package_with_data('dummy-package')
self.package.general_data = self.package.data.filter(key='general')
self.package.binaries_data = self.package.data.filter(key='binaries')
self.field = GeneralInformationTableField(self.package)
self.field = GeneralInformationTableField()
def test_field_context(self):
"""
Tests field context content
"""
context = self.field.context
context = self.field.context(self.package)
self.assertEqual(context['url'], self.package.get_absolute_url)
self.assertTrue(context['vcs'])
self.assertIn('type', context['vcs'])
......@@ -103,8 +103,6 @@ class GeneralInformationTableFieldTests(TestCase):
Tests field specific properties
"""
self.assertEqual(self.field.column_name, 'Package')
self.assertTrue(self.field.has_content)
self.assertIsNone(self.field.html_output)
self.assertEqual(
self.field.template_name, 'core/package-table-fields/general.html')
self.assertEqual(len(self.field.prefetch_related_lookups), 2)
......@@ -113,14 +111,14 @@ class GeneralInformationTableFieldTests(TestCase):
class VcsTableFieldTests(TestCase):
def setUp(self):
self.package = create_source_package_with_data('dummy-package')
self.package.general_vcs_data = self.package.data.all()
self.field = VcsTableField(self.package)
self.package.general_data = self.package.data.all()
self.field = VcsTableField()
def test_field_context(self):
"""
Tests field context content
"""
context = self.field.context
context = self.field.context(self.package)
self.assertTrue(context['vcs'])
self.assertIn('type', context['vcs'])
self.assertIn('url', context['vcs'])
......@@ -132,8 +130,6 @@ class VcsTableFieldTests(TestCase):
Tests field specific properties
"""
self.assertEqual(self.field.column_name, 'VCS')
self.assertTrue(self.field.has_content)
self.assertIsNone(self.field.html_output)
self.assertEqual(
self.field.template_name,
'core/package-table-fields/vcs.html')
......@@ -143,17 +139,17 @@ class VcsTableFieldTests(TestCase):
class ArchiveTableFieldTests(TestCase):
def setUp(self):
self.package = create_source_package_with_data('dummy-package')
self.package.general_archive_data = self.package.data.filter(
self.package.general_data = self.package.data.filter(
key='general')
self.package.versions = self.package.data.filter(
key='versions')
self.field = ArchiveTableField(self.package)
self.field = ArchiveTableField()
def test_field_context(self):
"""
Tests field context content
"""
context = self.field.context
context = self.field.context(self.package)
self.assertTrue(context['version'])
self.assertTrue(context['default_pool_url'])
......@@ -162,8 +158,6 @@ class ArchiveTableFieldTests(TestCase):
Tests field specific properties
"""
self.assertEqual(self.field.column_name, 'Archive')
self.assertTrue(self.field.has_content)
self.assertIsNone(self.field.html_output)
self.assertEqual(
self.field.template_name,
'core/package-table-fields/archive.html')
......@@ -174,13 +168,13 @@ class BugStatsTableFieldTests(TestCase):
def setUp(self):
self.package = create_source_package_with_data('dummy-package')
create_package_bug_stats(self.package)
self.field = BugStatsTableField(self.package)
self.field = BugStatsTableField()
def test_field_context(self):
"""
Tests field context content
"""
context = self.field.context
context = self.field.context(self.package)
self.assertTrue(context['all'])
self.assertEqual(context['all'], 11)
self.assertEqual(len(context['bugs']), 3)
......@@ -193,8 +187,6 @@ class BugStatsTableFieldTests(TestCase):
Tests field specific properties
"""
self.assertEqual(self.field.column_name, 'Bugs')
self.assertTrue(self.field.has_content)
self.assertIsNone(self.field.html_output)
self.assertEqual(
self.field.template_name,
'core/package-table-fields/bugs.html')
......@@ -227,14 +219,11 @@ class GeneralTeamPackageTableTests(TestCase, TemplateTestsMixin):
return table
return False
def assert_number_of_queries(self, table):
number_of_queries = 1 + sum(
len(f.prefetch_related_lookups) for f in table.table_fields)
def assert_number_of_queries(self, table, number_of_queries):
with self.assertNumQueries(number_of_queries):
for row in table.rows:
for field in row:
field.context
for cell in row:
self.assertIsNotNone(cell)
def test_table_displayed(self):
"""
......@@ -293,12 +282,12 @@ class GeneralTeamPackageTableTests(TestCase, TemplateTestsMixin):
queries regardless of the number of packages
"""
table = GeneralTeamPackageTable(self.team)
self.assert_number_of_queries(table)
self.assert_number_of_queries(table, 5)
new_package = create_source_package_with_data('another-dummy-package')
create_package_bug_stats(new_package)
self.team.packages.add(new_package)
self.assert_number_of_queries(table)
self.assert_number_of_queries(table, 5)
def test_table_limit_of_packages(self):
"""
......@@ -312,12 +301,14 @@ class GeneralTeamPackageTableTests(TestCase, TemplateTestsMixin):
self.assertEqual(len(table.rows), 1)
# Get the first column from the first row
table_field = table.rows[0][0]
self.assertEqual(self.package, table_field.package)
self.assertIn(self.package.name, table_field)
table.limit = 2
# Get the first column from the first row
table_field = table.rows[0][0]
self.assertEqual(self.package, table_field.package)
self.assertIn(self.package.name, table_field)
self.assertNotIn(new_package.name, table_field)
# Get the first column from the second row
table_field = table.rows[1][0]
self.assertEqual(new_package, table_field.package)
self.assertIn(new_package.name, table_field)
# Get the first column from the second row
......@@ -266,7 +266,7 @@ def get_developer_information_url(email):
return info_url
def add_developer_extras(general):
def add_developer_extras(general, url_only=False):
"""
Receives a general dict with package data and add to it more data
regarding that package's developers
......@@ -276,20 +276,23 @@ def add_developer_extras(general):
url = get_developer_information_url(maintainer_email)
if url:
general['maintainer']['developer_info_url'] = url
extra, implemented = vendor.call(
'get_maintainer_extra', maintainer_email, general['name'])
general['maintainer']['extra'] = extra
if not url_only:
extra, implemented = vendor.call(
'get_maintainer_extra', maintainer_email, general['name'])
general['maintainer']['extra'] = extra
uploaders = general.get('uploaders', None)
if uploaders:
for uploader in uploaders:
url = get_developer_information_url(uploader['email'])
if url:
uploader['developer_info_url'] = url
if url_only:
continue
# Vendor specific extras.
extra, implemented = vendor.call(
'get_uploader_extra', uploader['email'], general['name'])
if implemented and extra:
uploader['extra'] = extra
url = get_developer_information_url(uploader['email'])
if url:
uploader['developer_info_url'] = url
return general
......@@ -840,7 +840,8 @@ def additional_prefetch_related_lookups():
Prefetch(
'action_items',
queryset=ActionItem.objects.filter(
item_type__type_name='vcswatch-warnings-and-errors'),
item_type__type_name='vcswatch-warnings-and-errors'
).prefetch_related('item_type'),
),
Prefetch(
'data',
......@@ -857,10 +858,10 @@ def get_vcs_data(package):
<distro_tracker.project.local_settings.DISTRO_TRACKER_VCS_TABLE_FIELD_TEMPLATE>`
settings.
"""
data = {}
try:
data = {}
item = package.vcswatch_data[0]
data['vcs_extra_data'] = item.value
data['changelog_version'] = item.value['changelog_version']
except IndexError:
# There is no vcs extra data for the package
pass
......
{{ field.context.action_item.short_description }}
<hr>
<a href="{{ field.context.action_item.url }}" class="text-primary">More details</a>
<div id='vcs-field'>
<a href='{{ field.context.vcs.browser }}'>{{ field.context.vcs_extra_data.changelog_version }} <small>({{ field.context.vcs.full_name }})</small></a>
<a href='{{ field.context.vcs.browser }}'>{{ field.context.changelog_version }} <small>({{ field.context.vcs.full_name }})</small></a>
{% if field.context.action_item != None %}
<span class="popover-hover cursor-pointer label label-{{field.context.action_item.severity.label_type}}" data-toggle="popover" data-placement="left" title="Issue with <b>{{ field.context.action_item.severity.name }}</b> severity" data-content='{% include "debian/package-table-fields/popovers/vcs.html" with field=field %}'>
<span class="popover-hover cursor-pointer label label-{{field.context.action_item.severity.label_type}}" data-toggle="popover" data-placement="left" title="Issue with <b>{{ field.context.action_item.severity.name }}</b> severity" data-content='
{{ field.context.action_item.short_description }}
<hr>
<a href="{{ field.context.action_item.url }}" class="text-primary">More details</a>
'>
action needed
</span>
{% endif %}
......
......@@ -6010,15 +6010,15 @@ class UpstreamTableFieldTests(TestCase):
}
}
self.data.save()
self.package.general_upstream_data = self.package.data.filter(
self.package.general_data = self.package.data.filter(
key='general')
self.field = UpstreamTableField(self.package)
self.field = UpstreamTableField()
def test_field_context(self):
"""
Tests field contex content
"""
context = self.field.context
context = self.field.context(self.package)
self.assertTrue(context['version'])
self.assertTrue(context['url'])
......@@ -6027,8 +6027,6 @@ class UpstreamTableFieldTests(TestCase):
Tests field specific properties
"""
self.assertEqual(self.field.column_name, 'Upstream')
self.assertTrue(self.field.has_content)
self.assertIsNone(self.field.html_output)
self.assertEqual(
self.field.template_name,
'debian/package-table-fields/upstream.html')
......
......@@ -10,7 +10,6 @@
# including this file, may be copied, modified, propagated, or distributed
# except according to the terms contained in the LICENSE file.
from django.utils.functional import cached_property
from django.db.models import Prefetch
from distro_tracker.core.models import (
PackageData,
......@@ -31,14 +30,13 @@ class UpstreamTableField(BaseTableField):
Prefetch(
'data',
queryset=PackageData.objects.filter(key='general'),
to_attr='general_upstream_data'
to_attr='general_data'
),
]
@cached_property
def context(self):
def context(self, package):
try:
info = self.package.general_upstream_data[0]
info = package.general_data[0]
if 'upstream' in info.value:
return info.value['upstream']
except IndexError:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment