From bb2c192deebc1eb2be38a0aec954df168dd7dcaf Mon Sep 17 00:00:00 2001 From: Brian Bouterse Date: Mon, 25 Feb 2019 17:56:56 -0500 Subject: [PATCH] Adopts new content_summary layout The performance issue causes us to introduce a new model named RepositoryVersionContentDetails. This is also a great opportunity to update the content_summary of the RepositoryVersion serializer to match the recent API changes. Most of this code was inspired from a patch from @dalley. Requires PR: https://github.com/PulpQE/pulp-smash/pull/1174 https://pulp.plan.io/issues/4283 closes #4283 --- .../0003_repositoryversioncontentdetails.py | 24 +++ pulpcore/app/models/__init__.py | 1 + pulpcore/app/models/repository.py | 84 ++++++++- pulpcore/app/serializers/repository.py | 167 ++---------------- 4 files changed, 121 insertions(+), 155 deletions(-) create mode 100644 pulpcore/app/migrations/0003_repositoryversioncontentdetails.py diff --git a/pulpcore/app/migrations/0003_repositoryversioncontentdetails.py b/pulpcore/app/migrations/0003_repositoryversioncontentdetails.py new file mode 100644 index 0000000000..1608fc9854 --- /dev/null +++ b/pulpcore/app/migrations/0003_repositoryversioncontentdetails.py @@ -0,0 +1,24 @@ +# Generated by Django 2.1.7 on 2019-02-25 22:56 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('pulp_app', '0002_task_name'), + ] + + operations = [ + migrations.CreateModel( + name='RepositoryVersionContentDetails', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('count_type', models.CharField(choices=[('A', 'added'), ('P', 'present'), ('R', 'removed')], max_length=1)), + ('content_type', models.TextField()), + ('count', models.IntegerField()), + ('repository_version', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='counts', to='pulp_app.RepositoryVersion')), + ], + ), + ] diff --git a/pulpcore/app/models/__init__.py b/pulpcore/app/models/__init__.py index e611266ab3..c74706c96b 100644 --- a/pulpcore/app/models/__init__.py +++ b/pulpcore/app/models/__init__.py @@ -18,6 +18,7 @@ Repository, RepositoryContent, RepositoryVersion, + RepositoryVersionContentDetails, ) from .task import CreatedResource, ReservedResource, Task, TaskReservedResource, Worker # noqa diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index 85bf82253c..565a521ff7 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -1,15 +1,19 @@ """ Repository related Django models. """ +import django + from contextlib import suppress from django.db import models from django.db import transaction +from django.urls import reverse from .base import Model, MasterModel from .content import Content from .task import CreatedResource from pulpcore.app.models.storage import get_tls_path +from pulpcore.app.util import get_view_name_for_model from pulpcore.exceptions import ResourceImmutableError @@ -285,6 +289,7 @@ def contains(self, content): def create(cls, repository, base_version=None): """ Create a new RepositoryVersion + Creation of a RepositoryVersion should be done in a RQ Job. Args: @@ -295,7 +300,6 @@ def create(cls, repository, base_version=None): Returns: pulpcore.app.models.RepositoryVersion: The Created RepositoryVersion """ - with transaction.atomic(): version = cls( repository=repository, @@ -456,6 +460,35 @@ def delete(self, **kwargs): self.repository.save() super().delete(**kwargs) + def compute_counts(self): + """ + Compute and save content unit counts by type. + + Count records are stored as :class:`~pulpcore.app.models.RepositoryVersionContentDetails`. + This method deletes existing :class:`~pulpcore.app.models.RepositoryVersionContentDetails` + objects and makes new ones with each call. + """ + with transaction.atomic(): + counts_list = [] + for value, name in RepositoryVersionContentDetails.COUNT_TYPE_CHOICES: + RepositoryVersionContentDetails.objects.filter(repository_version=self).delete() + if value == RepositoryVersionContentDetails.ADDED: + qs = self.added() + elif value == RepositoryVersionContentDetails.PRESENT: + qs = self.content + elif value == RepositoryVersionContentDetails.REMOVED: + qs = self.removed() + annotated = qs.values('_type').annotate(count=models.Count('_type')) + for item in annotated: + count_obj = RepositoryVersionContentDetails( + content_type=item['_type'], + repository_version=self, + count=item['count'], + count_type=value, + ) + counts_list.append(count_obj) + RepositoryVersionContentDetails.objects.bulk_create(counts_list) + def __enter__(self): """ Create the repository version @@ -474,3 +507,52 @@ def __exit__(self, exc_type, exc_value, traceback): else: self.complete = True self.save() + self.compute_counts() + + +class RepositoryVersionContentDetails(models.Model): + ADDED = 'A' + PRESENT = 'P' + REMOVED = 'R' + COUNT_TYPE_CHOICES = ( + (ADDED, 'added'), + (PRESENT, 'present'), + (REMOVED, 'removed'), + ) + + count_type = models.CharField( + max_length=1, + choices=COUNT_TYPE_CHOICES, + ) + content_type = models.TextField() + repository_version = models.ForeignKey('RepositoryVersion', related_name='counts', + on_delete=models.CASCADE) + count = models.IntegerField() + + @property + def content_href(self): + """ + Generate URLs for the content types present in the RepositoryVersion. + + For each content type present in the RepositoryVersion, create the URL of the viewset of + that variety of content along with a query parameter which filters it by presence in this + RepositoryVersion. + + Args: + obj (pulpcore.app.models.RepositoryVersion): The RepositoryVersion being serialized. + Returns: + dict: {<_type>: } + """ + ctype_model = Content.objects.filter(_type=self.content_type).first().cast().__class__ + ctype_view = get_view_name_for_model(ctype_model, 'list') + try: + ctype_url = reverse(ctype_view) + except django.urls.exceptions.NoReverseMatch: + # We've hit a content type for which there is no viewset. + # There's nothing we can do here, except to skip it. + return + rv_href = "/pulp/api/v3/repositories/{repo}/versions/{version}/".format( + repo=self.repository_version.repository.pk, version=self.repository_version.number) + full_url = "{base}?repository_version={rv_href}".format( + base=ctype_url, rv_href=rv_href) + return full_url diff --git a/pulpcore/app/serializers/repository.py b/pulpcore/app/serializers/repository.py index 2476702f8e..3f205cc01a 100644 --- a/pulpcore/app/serializers/repository.py +++ b/pulpcore/app/serializers/repository.py @@ -1,9 +1,5 @@ from gettext import gettext as _ -import django -from django.db.models import Count -from django.urls import reverse - from rest_framework import serializers, fields from rest_framework.validators import UniqueValidator from rest_framework_nested.serializers import NestedHyperlinkedModelSerializer @@ -19,7 +15,6 @@ ModelSerializer, ) from pulpcore.app.serializers import validate_unknown_fields -from pulpcore.app.util import get_view_name_for_model class RepositorySerializer(ModelSerializer): @@ -261,31 +256,9 @@ class RepositoryVersionSerializer(ModelSerializer, NestedHyperlinkedModelSeriali lookup_field='number', parent_lookup_kwargs={'repository_pk': 'repository__pk'}, ) - content_hrefs = serializers.SerializerMethodField( - help_text=_('A mapping of the types of content in this version, and the HREF to view ' - 'them.'), - read_only=True, - ) - content_added_hrefs = serializers.SerializerMethodField( - help_text=_('A mapping of the types of content added in this version, and the HREF to ' - 'view them.'), - read_only=True, - ) - content_removed_hrefs = serializers.SerializerMethodField( - help_text=_('A mapping of the types of content removed from this version, and the HREF ' - 'to view them.'), - read_only=True, - ) content_summary = serializers.SerializerMethodField( - help_text=_('A list of counts of each type of content in this version.'), - read_only=True, - ) - content_added_summary = serializers.SerializerMethodField( - help_text=_('A list of counts of each type of content added in this version.'), - read_only=True, - ) - content_removed_summary = serializers.SerializerMethodField( - help_text=_('A list of counts of each type of content removed in this version.'), + help_text=_('Various count summaries of the content in the version and the HREF to view ' + 'them.'), read_only=True, ) @@ -294,136 +267,22 @@ def get_content_summary(self, obj): The summary of contained content. Returns: - dict: of {<_type>: } - """ - annotated = obj.content.values('_type').annotate(count=Count('_type')) - return {c['_type']: c['count'] for c in annotated} - - def get_content_added_summary(self, obj): - """ - The summary of added content. - - Returns: - dict: of {<_type>: } - """ - annotated = obj.added().values('_type').annotate(count=Count('_type')) - return {c['_type']: c['count'] for c in annotated} - - def get_content_removed_summary(self, obj): - """ - The summary of removed content. - - Returns: - dict: of {<_type>: } - """ - annotated = obj.removed().values('_type').annotate(count=Count('_type')) - return {c['_type']: c['count'] for c in annotated} - - def get_content_hrefs(self, obj): - """ - Generate URLs for the content types present in the RepositoryVersion. - - For each content type present in the RepositoryVersion, create the URL of the viewset of - that variety of content along with a query parameter which filters it by presence in this - RepositoryVersion. - - Args: - obj (pulpcore.app.models.RepositoryVersion): The RepositoryVersion being serialized. - - Returns: - dict: {<_type>: } + dict: of {'added': {<_type>: {'count': , 'href': }, + 'removed': {<_type>: {'count': , 'href': }, + 'present': {<_type>: {'count': , 'href': }, + } """ - content_urls = {} - - for ctype in obj.content.values_list('_type', flat=True).distinct(): - ctype_model = obj.content.filter(_type=ctype).first().cast().__class__ - ctype_view = get_view_name_for_model(ctype_model, 'list') - try: - ctype_url = reverse(ctype_view) - except django.urls.exceptions.NoReverseMatch: - # We've hit a content type for which there is no viewset. - # There's nothing we can do here, except to skip it. - continue - rv_href = "/pulp/api/v3/repositories/{repo}/versions/{version}/".format( - repo=obj.repository.pk, version=obj.number) - full_url = "{base}?repository_version={rv_href}".format( - base=ctype_url, rv_href=rv_href) - content_urls[ctype] = full_url - - return content_urls - - def get_content_added_hrefs(self, obj): - """ - Generate URLs for the content types added in the RepositoryVersion. - - For each content type added in the RepositoryVersion, create the URL of the viewset of - that variety of content along with a query parameter which filters it by addition in this - RepositoryVersion. - - Args: - obj (pulpcore.app.models.RepositoryVersion): The RepositoryVersion being serialized. - - Returns: - dict: {<_type>: } - """ - content_urls = {} - - for ctype in obj.added().values_list('_type', flat=True).distinct(): - ctype_model = obj.content.filter(_type=ctype).first().cast().__class__ - ctype_view = get_view_name_for_model(ctype_model, 'list') - try: - ctype_url = reverse(ctype_view) - except django.urls.exceptions.NoReverseMatch: - # We've hit a content type for which there is no viewset. - # There's nothing we can do here, except to skip it. - continue - rv_href = "/pulp/api/v3/repositories/{repo}/versions/{version}/".format( - repo=obj.repository.pk, version=obj.number) - full_url = "{base}?repository_version_added={rv_href}".format( - base=ctype_url, rv_href=rv_href) - content_urls[ctype] = full_url - - return content_urls - - def get_content_removed_hrefs(self, obj): - """ - Generate URLs for the content types removed in the RepositoryVersion. - - For each content type removed in the RepositoryVersion, create the URL of the viewset of - that variety of content along with a query parameter which filters it by removal in this - RepositoryVersion. - - Args: - obj (pulpcore.app.models.RepositoryVersion): The RepositoryVersion being serialized. - - Returns: - dict: {<_type>: } - """ - content_urls = {} - - for ctype in obj.removed().values_list('_type', flat=True).distinct(): - ctype_model = obj.content.filter(_type=ctype).first().cast().__class__ - ctype_view = get_view_name_for_model(ctype_model, 'list') - try: - ctype_url = reverse(ctype_view) - except django.urls.exceptions.NoReverseMatch: - # We've hit a content type for which there is no viewset. - # There's nothing we can do here, except to skip it. - continue - rv_href = "/pulp/api/v3/repositories/{repo}/versions/{version}/".format( - repo=obj.repository.pk, version=obj.number) - full_url = "{base}?repository_version_removed={rv_href}".format( - base=ctype_url, rv_href=rv_href) - content_urls[ctype] = full_url - - return content_urls + to_return = {'added': {}, 'removed': {}, 'present': {}} + for count_detail in obj.counts.all(): + count_type = count_detail.get_count_type_display() + item_dict = {'count': count_detail.count, 'href': count_detail.content_href} + to_return[count_type][count_detail.content_type] = item_dict + return to_return class Meta: model = models.RepositoryVersion fields = ModelSerializer.Meta.fields + ( - '_href', 'number', 'base_version', - 'content_hrefs', 'content_added_hrefs', 'content_removed_hrefs', - 'content_summary', 'content_added_summary', 'content_removed_summary', + '_href', 'number', 'base_version', 'content_summary', )