Merge lp:~wgrant/launchpad/trim-asp into lp:launchpad

[William Grant]

[r=stevenk][no-qa] Drop getBuilds and getUnpublishedBuilds preloading from ArchiveSourcePublications. It no longer benefits getBuildStatusSummariesForSourceIdsAndArchive.

Several Archive views (primarily +index and +packages, but also +copy-packages and +delete-packages) decorate SPPH collections with ArchiveSourcePublications, which preloads .changes files and builds and build summaries. The build preloading was intended to make build summary calculation faster, but it no longer helps. This branch drops it, saving a couple of slow queries on each load.

[Steve Kowalik (stevenk)]

review: Approve

=== modified file 'lib/lp/soyuz/adapters/archivesourcepublication.py'
--- lib/lp/soyuz/adapters/archivesourcepublication.py	2010-12-02 16:13:51 +0000
+++ lib/lp/soyuz/adapters/archivesourcepublication.py	2011-03-21 10:51:01 +0000
@@ -56,10 +56,8 @@
     """
     delegates(ISourcePackagePublishingHistory)
 
-    def __init__(self, context, unpublished_builds, builds, changesfile, status_summary):
+    def __init__(self, context, changesfile, status_summary):
         self.context = context
-        self._unpublished_builds = unpublished_builds
-        self._builds = builds
         self._changesfile = changesfile
         self._status_summary = status_summary
 
@@ -73,19 +71,6 @@
         return ArchiveSourcePackageRelease(
             self.context.sourcepackagerelease, changesfile)
 
-    def getUnpublishedBuilds(self, build_state='ignored'):
-        """See `ISourcePackagePublishingHistory`.
-
-        In this cached implementation, we ignore the build_state argument
-        and simply return the unpublished builds with which we were
-        created.
-        """
-        return self._unpublished_builds
-
-    def getBuilds(self):
-        """See `ISourcePackagePublishingHistory`."""
-        return self._builds
-
     def getStatusSummaryForBuilds(self):
         """See `ISourcePackagePublishingHistory`."""
         return self._status_summary
@@ -103,39 +88,6 @@
         """Whether or not there are sources to be processed."""
         return len(self._source_publications) > 0
 
-    def groupBySource(self, source_and_value_list):
-        """Group the give list of tuples as a dictionary.
-
-        This is a common internal task for this class, it groups the given
-        list of tuples, (source, related_object), as a dictionary keyed by
-        distinct sources and pointing to a list of `relates_object`s.
-
-        :return: a dictionary keyed by the distinct sources and pointing to
-            a list of `related_object`s in their original order.
-        """
-        source_and_values = {}
-        for source, value in source_and_value_list:
-            values = source_and_values.setdefault(source, [])
-            values.append(value)
-        return source_and_values
-
-    def getBuildsBySource(self):
-        """Builds for all source publications."""
-        build_set = getUtility(IPublishingSet).getBuildsForSources(
-            self._source_publications)
-        source_and_builds = [
-            (source, build) for source, build, arch in build_set]
-        return self.groupBySource(source_and_builds)
-
-    def getUnpublishedBuildsBySource(self):
-        """Unpublished builds for sources."""
-        publishing_set = getUtility(IPublishingSet)
-        build_set = publishing_set.getUnpublishedBuildsForSources(
-            self._source_publications)
-        source_and_builds = [
-            (source, build) for source, build, arch in build_set]
-        return self.groupBySource(source_and_builds)
-
     def getChangesFileBySource(self):
         """Map changesfiles by their corresponding source publications."""
         publishing_set = getUtility(IPublishingSet)
@@ -160,8 +112,6 @@
         # Load the extra-information for all source publications.
         # All of this code would be better on an object representing a set of
         # publications.
-        builds_by_source = self.getBuildsBySource()
-        unpublished_builds_by_source = self.getUnpublishedBuildsBySource()
         changesfiles_by_source = self.getChangesFileBySource()
         # Source package names are used by setNewerDistroSeriesVersions:
         # batch load the used source package names.
@@ -184,13 +134,10 @@
 
         # Build the decorated object with the information we have.
         for pub in self._source_publications:
-            builds = builds_by_source.get(pub, [])
-            unpublished_builds = unpublished_builds_by_source.get(pub, [])
             changesfile = changesfiles_by_source.get(pub, None)
             status_summary = status_summaries[pub.id]
             complete_pub = ArchiveSourcePublication(
-                pub, unpublished_builds=unpublished_builds, builds=builds,
-                changesfile=changesfile, status_summary=status_summary)
+                pub, changesfile=changesfile, status_summary=status_summary)
             results.append(complete_pub)
 
         return iter(results)

=== modified file 'lib/lp/soyuz/browser/tests/test_archive_packages.py'
--- lib/lp/soyuz/browser/tests/test_archive_packages.py	2011-03-04 05:54:40 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_packages.py	2011-03-21 10:51:01 +0000
@@ -187,7 +187,7 @@
         self.assertIs(None, view.specified_name_filter)
 
     def test_source_query_counts(self):
-        query_baseline = 47
+        query_baseline = 43
         # Assess the baseline.
         collector = QueryCollector()
         collector.register()
@@ -228,7 +228,7 @@
         self.assertThat(collector, HasQueryCount(LessThan(expected_count)))
 
     def test_binary_query_counts(self):
-        query_baseline = 43
+        query_baseline = 40
         # Assess the baseline.
         collector = QueryCollector()
         collector.register()
@@ -245,8 +245,7 @@
                 archive=ppa)
             url = canonical_url(ppa) + "/+packages"
         browser.open(url)
-        self.assertThat(collector, HasQueryCount(
-            MatchesAny(LessThan(query_baseline), Equals(query_baseline))))
+        self.assertThat(collector, HasQueryCount(LessThan(query_baseline)))
         expected_count = collector.count
         # Use all new objects - avoids caching issues invalidating the
         # gathered metrics.
@@ -260,5 +259,4 @@
                     archive=ppa, distroarchseries=pkg.distroarchseries)
             url = canonical_url(ppa) + "/+packages"
         browser.open(url)
-        self.assertThat(collector, HasQueryCount(
-            MatchesAny(Equals(expected_count), LessThan(expected_count))))
+        self.assertThat(collector, HasQueryCount(Equals(expected_count)))

=== modified file 'lib/lp/soyuz/doc/publishing.txt'
--- lib/lp/soyuz/doc/publishing.txt	2011-03-03 00:43:44 +0000
+++ lib/lp/soyuz/doc/publishing.txt	2011-03-21 10:51:01 +0000
@@ -1587,11 +1587,9 @@
     Mismatch: ...
 
 We will check a little bit of the `ArchiveSourcePublications`
-internals. There are three essential methods to fetch distinct
+internals. There is one essential method to fetch distinct
 information to be cached in the decorated objects:
 
-  * getBuildsBySource;
-  * getUnpublishedBuildsBySource;
   * getChangesFileBySources
 
 They exclude the extra references ('prejoins') returned  from the
@@ -1601,24 +1599,6 @@
 
     >>> real_pub = cprov_published_sources[1]
 
-Builds by source.
-
-    >>> builds_by_source = decorated_set.getBuildsBySource()
-
-    >>> compare_ids(
-    ...     real_pub.getBuilds(), builds_by_source.get(real_pub, []))
-    Matches
-
-Unpublished builds by source.
-
-    >>> unpublished_builds_by_source = (
-    ...     decorated_set.getUnpublishedBuildsBySource())
-
-    >>> compare_ids(
-    ...     real_pub.getUnpublishedBuilds(),
-    ...     unpublished_builds_by_source.get(real_pub, []))
-    Matches
-
 getChangesFileBySources() returns a dictionary mapping each individual
 source package publication to its corresponding .changes file (as a
 LibraryFileAlias).