Merge lp:~lifeless/launchpad/soyuz into lp:launchpad

[Robert Collins]

Incremental improvement for +packages pages.

Incremental improvement for the +packages pages, eager loading the attribute triggering most DB queries and tuning the logic to do only one related call. We may still have DB access being triggered, but this should help.

[Steve Kowalik (stevenk)]

My only concern with this is what impact the caching will have on the publisher, but the code itself looks good.

review: Approve (code*)

[Robert Collins (lifeless)]

The publisher doesn't seem to use that attribute AFAICT. IMBW, tests will show.

[Tim Penhey (thumper)]

Since we want to make soyuz code more understandable, can you please just add to the docstring at the top of the setNewerDistroSeriesVersions as to what spphs (or spph objects) are?

It wasn't until much further down that change that I actually had any inkling as to what it was.

review: Approve (mentore)

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2010-11-02 06:21:58 +0000
+++ lib/lp/registry/model/distroseries.py	2010-11-03 17:51:56 +0000
@@ -12,9 +12,11 @@
     'DistroSeriesSet',
     ]
 
+import collections
 from cStringIO import StringIO
 import logging
 
+import apt_pkg
 from sqlobject import (
     BoolCol,
     ForeignKey,
@@ -119,7 +121,10 @@
 from lp.registry.model.structuralsubscription import (
     StructuralSubscriptionTargetMixin,
     )
-from lp.services.propertycache import cachedproperty
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 from lp.services.worlddata.model.language import Language
 from lp.soyuz.enums import (
     ArchivePurpose,
@@ -1506,6 +1511,32 @@
 
         return distro_sprs
 
+    @staticmethod
+    def setNewerDistroSeriesVersions(spphs):
+        """Set the newer_distroseries_version attribute on the spph entries.
+
+        :param spphs: The SourcePackagePublishingHistory objects to set the
+            newer_distroseries_version attribute on.
+        """
+        # Partition by distro series to use getCurrentSourceReleases
+        distro_series = collections.defaultdict(list)
+        for spph in spphs:
+            distro_series[spph.distroseries].append(spph)
+        for series, spphs in distro_series.items():
+            packagenames = set()
+            for spph in spphs:
+                packagenames.add(spph.sourcepackagerelease.sourcepackagename)
+            latest_releases = series.getCurrentSourceReleases(
+                packagenames)
+            for spph in spphs:
+                latest_release = latest_releases.get(spph.meta_sourcepackage, None)
+                if latest_release is not None and apt_pkg.VersionCompare(
+                    latest_release.version, spph.source_package_version) > 0:
+                    version = latest_release
+                else:
+                    version = None
+                get_property_cache(spph).newer_distroseries_version = version
+
     def createQueueEntry(self, pocket, changesfilename, changesfilecontent,
                          archive, signing_key=None):
         """See `IDistroSeries`."""

=== modified file 'lib/lp/soyuz/adapters/archivesourcepublication.py'
--- lib/lp/soyuz/adapters/archivesourcepublication.py	2010-08-20 20:31:18 +0000
+++ lib/lp/soyuz/adapters/archivesourcepublication.py	2010-11-03 17:51:56 +0000
@@ -19,6 +19,7 @@
 from zope.component import getUtility
 
 from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
+from lp.registry.model.distroseries import DistroSeries
 from lp.soyuz.interfaces.publishing import (
     IPublishingSet,
     ISourcePackagePublishingHistory,
@@ -163,6 +164,7 @@
         builds_by_source = self.getBuildsBySource()
         unpublished_builds_by_source = self.getUnpublishedBuildsBySource()
         changesfiles_by_source = self.getChangesFileBySource()
+        DistroSeries.setNewerDistroSeriesVersions(self._source_publications)
 
         # Build the decorated object with the information we have.
         for pub in self._source_publications:

=== modified file 'lib/lp/soyuz/doc/publishing.txt'
--- lib/lp/soyuz/doc/publishing.txt	2010-10-19 18:44:31 +0000
+++ lib/lp/soyuz/doc/publishing.txt	2010-11-03 17:51:56 +0000
@@ -135,9 +135,18 @@
     ...     distroseries=pub.distroseries, version="1.1",
     ...     sourcename='iceweasel')
 
+    >>> from lp.services.propertycache import get_property_cache
+    >>> del get_property_cache(pub).newer_distroseries_version
     >>> print pub.newer_distroseries_version.title
     "iceweasel" 1.1 source package in The Warty Warthog Release
 
+We can calculate the newer_distroseries_version for many spph objects at once.
+
+    >>> del get_property_cache(pub).newer_distroseries_version
+    >>> pub.distroseries.setNewerDistroSeriesVersions([pub])
+    >>> print get_property_cache(pub).newer_distroseries_version.title
+    "iceweasel" 1.1 source package in The Warty Warthog Release
+
 A helper is also included to create a summary of the build statuses for
 the spph's related builds, getStatusSummaryForBuilds(), which just
 augments the IBuildSet.getStatusSummaryForBuilds() method to include the
@@ -1530,9 +1539,15 @@
     >>> cprov_published_sources.count()
     4
 
-    >>> len(list(decorated_set))
+    >>> decorated_sources_list = list(decorated_set)
+    >>> len(decorated_sources_list)
     4
 
+The objects loaded have their newer_distroseries_version preloaded.
+
+    >>> actual_pub = decorated_sources_list[0].context
+    >>> get_property_cache(actual_pub).newer_distroseries_version
+
 The decorated objects are returned in the same order used in the given
 'source_publications'.
 

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2010-10-17 03:45:49 +0000
+++ lib/lp/soyuz/model/publishing.py	2010-11-03 17:51:56 +0000
@@ -71,6 +71,10 @@
 from lp.buildmaster.model.packagebuild import PackageBuild
 from lp.registry.interfaces.person import validate_public_person
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 from lp.services.worlddata.model.country import Country
 from lp.soyuz.enums import (
     BinaryPackageFormat,
@@ -445,18 +449,11 @@
             return self.sourcepackagerelease.dscsigningkey.owner
         return None
 
-    @property
+    @cachedproperty
     def newer_distroseries_version(self):
         """See `ISourcePackagePublishingHistory`."""
-        latest_releases = self.distroseries.getCurrentSourceReleases(
-            [self.sourcepackagerelease.sourcepackagename])
-        latest_release = latest_releases.get(self.meta_sourcepackage, None)
-
-        if latest_release is not None and apt_pkg.VersionCompare(
-            latest_release.version, self.source_package_version) > 0:
-            return latest_release
-        else:
-            return None
+        self.distroseries.setNewerDistroSeriesVersions([self])
+        return get_property_cache(self).newer_distroseries_version
 
     def getPublishedBinaries(self):
         """See `ISourcePackagePublishingHistory`."""