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.

This includes smash updates that are included in a separate PR.

Requires PR: pulp/pulp-smash#1174

https://pulp.plan.io/issues/4283
closes pulp#4283

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')),
+            ],
+        ),
+    ]

pulpcore/app/models/__init__.py

@@ -18,6 +18,7 @@
     Repository,
     RepositoryContent,
     RepositoryVersion,
+    RepositoryVersionContentDetails,
 )
 
 from .task import CreatedResource, ReservedResource, Task, TaskReservedResource, Worker  # noqa

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,58 @@ 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>: <url>}
+        """
+        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)
+        if self.count_type == self.ADDED:
+            partial_url_str = "{base}?repository_version_added={rv_href}"
+        elif self.count_type == self.PRESENT:
+            partial_url_str = "{base}?repository_version={rv_href}"
+        elif self.count_type == self.REMOVED:
+            partial_url_str = "{base}?repository_version_removed={rv_href}"
+        full_url = partial_url_str.format(
+            base=ctype_url, rv_href=rv_href)
+        return full_url

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>: <count>}
-        """
-        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>: <count>}
-        """
-        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>: <count>}
-        """
-        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>: <url>}
+            dict: of {'added': {<_type>: {'count': <count>, 'href': <href>},
+                      'removed': {<_type>: {'count': <count>, 'href': <href>},
+                      'present': {<_type>: {'count': <count>, 'href': <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>: <url>}
-        """
-        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>: <url>}
-        """
-        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',
         )
 
 

pulpcore/tests/functional/api/using_plugin/constants.py

@@ -27,7 +27,7 @@
 FILE_FIXTURE_COUNT = 3
 """The number of packages available at :data:`FILE_FIXTURE_URL`."""
 
-FILE_FIXTURE_SUMMARY = {FILE_CONTENT_NAME: FILE_FIXTURE_COUNT}
+FILE_FIXTURE_SUMMARY = {FILE_CONTENT_NAME: {'count': FILE_FIXTURE_COUNT}}
 """The desired content summary after syncing :data:`FILE_FIXTURE_URL`."""
 
 FILE2_FIXTURE_URL = urljoin(PULP_FIXTURES_BASE_URL, 'file2/')

pulpcore/tests/functional/api/using_plugin/test_repo_versions.py

@@ -175,14 +175,14 @@ def test_03_remove_content(self):
 
         content_summary = get_content_summary(repo)
         self.assertDictEqual(
-            content_summary, {FILE_CONTENT_NAME: FILE_FIXTURE_COUNT - 1}
+            content_summary, {FILE_CONTENT_NAME: {'count': FILE_FIXTURE_COUNT - 1}}
         )
 
         content_added_summary = get_added_content_summary(repo)
         self.assertDictEqual(content_added_summary, {})
 
         content_removed_summary = get_removed_content_summary(repo)
-        self.assertDictEqual(content_removed_summary, {FILE_CONTENT_NAME: 1})
+        self.assertDictEqual(content_removed_summary, {FILE_CONTENT_NAME: {'count': 1}})
 
     @skip_if(bool, 'repo', False)
     def test_04_add_content(self):
@@ -220,7 +220,7 @@ def test_04_add_content(self):
         self.assertDictEqual(content_summary, FILE_FIXTURE_SUMMARY)
 
         content_added_summary = get_added_content_summary(repo)
-        self.assertDictEqual(content_added_summary, {FILE_CONTENT_NAME: 1})
+        self.assertDictEqual(content_added_summary, {FILE_CONTENT_NAME: {'count': 1}})
 
         content_removed_summary = get_removed_content_summary(repo)
         self.assertDictEqual(content_removed_summary, {})