Merge lp:~edwin-grubbs/launchpad/bug-534462-project-index-timeout into lp:launchpad

[Edwin Grubbs]

Fixed timeout on gcc project index page due to the Packages in Ubuntu portlet.

Summary
-------

Fixed timeout issue caused by the "Packages in Ubuntu" portlet
on the project index page. I also made DistroSourcePackage.upstream_product
more efficient, even though that is no longer needed by the view,
since searchSourcePackages() can now do the filtering for us.

Implementation details
----------------------

Added has_packaging parameter to searchSourcePackages() so we
do not have to check whether the package has an upstream_product.
    lib/lp/registry/browser/product.py
    lib/lp/registry/doc/distribution.txt
    lib/lp/registry/interfaces/distribution.py
    lib/lp/registry/model/distribution.py

Optimized DistroSourcePackage.upstream_product:
    lib/lp/registry/model/distributionsourcepackage.py

The portlet would either have the title "Packages in Ubuntu" or
"Packages in distributions" depending on the content, which didn't
really make sense, so now there is a single title.
    lib/lp/registry/stories/product/xx-product-index.txt
    lib/lp/registry/templates/product-portlet-packages.pt

Tests
-----

./bin/test -vv -t 'doc/distribution.txt|xx-product-index.txt|distribution-sourcepackage.txt|xx-distribution-source-package.txt'

Demo and Q/A
------------

On launchpad.dev:
* Open http://launchpad.dev/firefox/+packages
  * Remove all the linked packages.
* Open http://launchpad.dev/firefox/+packages
  * The "Packages in Ubuntu" portlet should show
    the mozilla-firefox sourcepackage and a button
    to link to it. (If it doesn't show up, try
    running cronscripts/update-pkgcache.py)

On edge:
* Open https://edge.launchpad.net/gcc
  * This should not timeout.

[Brad Crittenden (bac)]

review: Approve (code)

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2010-03-08 01:51:58 +0000
+++ lib/lp/registry/browser/product.py	2010-03-09 17:46:07 +0000
@@ -982,14 +982,13 @@
         """See `LaunchpadFormView`."""
         super(ProductPackagesPortletView, self).setUpFields()
         ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        source_packages = ubuntu.searchSourcePackages(self.context.name)
+        source_packages = ubuntu.searchSourcePackages(
+            self.context.name, has_packaging=False)
         # Based upon the matches, create a new vocabulary with
         # term descriptions that include a link to the source package.
         self.suggestions = []
         vocab_terms = []
-        for package in source_packages:
-            if package.upstream_product is not None:
-                continue
+        for package in source_packages[:20]:
             self.suggestions.append(package)
             item_url = canonical_url(package)
             description = """<a href="%s">%s</a>""" % (

=== modified file 'lib/lp/registry/doc/distribution.txt'
--- lib/lp/registry/doc/distribution.txt	2010-02-15 12:59:55 +0000
+++ lib/lp/registry/doc/distribution.txt	2010-03-09 17:46:07 +0000
@@ -202,6 +202,23 @@
     DistributionSourcePackage: foobar
     DistributionSourcePackage: commercialpackage
 
+searchSourcePackages() also has a has_packaging parameter that
+it just passes on to searchSourcePackageCaches(), and it restricts
+the results based on whether the source package has an entry
+in the Packaging table linking it to an upstream project.
+
+    >>> packages = ubuntu.searchSourcePackages('a', has_packaging=True)
+    >>> for dsp in packages:
+    ...     print "%s: %s" % (dsp.__class__.__name__, dsp.name)
+    DistributionSourcePackage: mozilla-firefox
+    DistributionSourcePackage: netapplet
+    DistributionSourcePackage: alsa-utils
+    >>> packages = ubuntu.searchSourcePackages('a', has_packaging=False)
+    >>> for dsp in packages:
+    ...     print "%s: %s" % (dsp.__class__.__name__, dsp.name)
+    DistributionSourcePackage: foobar
+    DistributionSourcePackage: commercialpackage
+
 
 === Searching for binary packages ===
 

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2010-03-05 04:12:27 +0000
+++ lib/lp/registry/interfaces/distribution.py	2010-03-09 17:46:07 +0000
@@ -219,7 +219,8 @@
     # properties
     currentseries = exported(
         Reference(
-            Interface, # Really IDistroSeries, see _schema_circular_imports.py.
+            # Really IDistroSeries, see _schema_circular_imports.py.
+            Interface,
             title=_("Current series"),
             description=_(
                 "The current development series of this distribution. "
@@ -405,17 +406,21 @@
     # _schema_circular_imports.py.
     @operation_returns_collection_of(Interface)
     @export_read_operation()
-    def searchSourcePackages(text):
+    def searchSourcePackages(text, has_packaging=None):
         """Search for source packages that correspond to the given text.
 
         This method just decorates the result of searchSourcePackageCaches()
         to return DistributionSourcePackages.
         """
 
-    def searchSourcePackageCaches(text):
+    def searchSourcePackageCaches(text, has_packaging=None):
         """Search for source packages that correspond to the given text.
 
         :param text: The text that will be matched.
+        :param has_packaging: If True, it will filter out
+            packages with no packaging (i.e. no link to the upstream
+            project). False will do the reverse filtering, and None
+            will do no filtering on this field.
         :return: A result set containing
             (DistributionSourcePackageCache, SourcePackageName, rank) tuples
             ordered by rank.

=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py	2010-03-06 21:38:23 +0000
+++ lib/lp/registry/model/distribution.py	2010-03-09 17:46:07 +0000
@@ -927,7 +927,8 @@
         cache.binpkgdescriptions = ' '.join(sorted(binpkgdescriptions))
         cache.changelog = ' '.join(sorted(sprchangelog))
 
-    def searchSourcePackageCaches(self, text):
+    def searchSourcePackageCaches(
+        self, text, has_packaging=None):
         """See `IDistribution`."""
         # The query below tries exact matching on the source package
         # name as well; this is because source package names are
@@ -948,25 +949,40 @@
                 )
             ]
 
+
+        packaging_query = """
+            SELECT 1
+            FROM Packaging
+            WHERE Packaging.sourcepackagename = SourcePackageName.id
+            """
+        has_packaging_condition = ''
+        if has_packaging is True:
+            has_packaging_condition = 'AND EXISTS (%s)' % packaging_query
+        elif has_packaging is False:
+            has_packaging_condition = 'AND NOT EXISTS (%s)' % packaging_query
+
         # Note: When attempting to convert the query below into straight
         # Storm expressions, a 'tuple index out-of-range' error was always
         # raised.
-        dsp_caches_with_ranks = store.using(*origin).find(
-            find_spec,
-            """distribution = %s AND
+        condition = """
+            distribution = %s AND
             archive IN %s AND
             (fti @@ ftq(%s) OR
              DistributionSourcePackageCache.name ILIKE '%%' || %s || '%%')
+            %s
             """ % (quote(self), quote(self.all_distro_archive_ids),
-                   quote(text), quote_like(text))
+                   quote(text), quote_like(text), has_packaging_condition)
+        dsp_caches_with_ranks = store.using(*origin).find(
+            find_spec, condition
             ).order_by('rank DESC')
 
         return dsp_caches_with_ranks
 
-    def searchSourcePackages(self, text):
+    def searchSourcePackages(self, text, has_packaging=None):
         """See `IDistribution`."""
 
-        dsp_caches_with_ranks = self.searchSourcePackageCaches(text)
+        dsp_caches_with_ranks = self.searchSourcePackageCaches(
+            text, has_packaging=has_packaging)
 
         # Create a function that will decorate the resulting
         # DistributionSourcePackageCaches, converting

=== modified file 'lib/lp/registry/model/distributionsourcepackage.py'
--- lib/lp/registry/model/distributionsourcepackage.py	2010-03-08 11:22:05 +0000
+++ lib/lp/registry/model/distributionsourcepackage.py	2010-03-09 17:46:07 +0000
@@ -22,6 +22,8 @@
 
 from canonical.database.sqlbase import sqlvalues
 from canonical.launchpad.database.emailaddress import EmailAddress
+from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.packaging import Packaging
 from lp.registry.model.structuralsubscription import (
     StructuralSubscriptionTargetMixin)
 from canonical.launchpad.interfaces.lpstorm import IStore
@@ -315,12 +317,18 @@
 
     @property
     def upstream_product(self):
-        for distroseries in self.distribution.series:
-            source_package = distroseries.getSourcePackage(
-                self.sourcepackagename)
-            if source_package.direct_packaging is not None:
-                return source_package.direct_packaging.productseries.product
-        return None
+        store = Store.of(self.sourcepackagename)
+        condition = And(
+            Packaging.sourcepackagename == self.sourcepackagename,
+            Packaging.distroseriesID == DistroSeries.id,
+            DistroSeries.distribution == self.distribution
+            )
+        result = store.find(Packaging, condition)
+        result.order_by("debversion_sort_key(version) DESC")
+        if result.count() == 0:
+            return None
+        else:
+            return result[0].productseries.product
 
     # XXX kiko 2006-08-16: Bad method name, no need to be a property.
     @property
@@ -353,8 +361,6 @@
 
     def getReleasesAndPublishingHistory(self):
         """See `IDistributionSourcePackage`."""
-        # Local import of DistroSeries to avoid import loop.
-        from lp.registry.model.distroseries import DistroSeries
         store = Store.of(self.distribution)
         result = store.find(
             (SourcePackageRelease, SourcePackagePublishingHistory),

=== modified file 'lib/lp/registry/stories/product/xx-product-index.txt'
--- lib/lp/registry/stories/product/xx-product-index.txt	2010-03-01 21:48:15 +0000
+++ lib/lp/registry/stories/product/xx-product-index.txt	2010-03-09 17:46:07 +0000
@@ -359,7 +359,7 @@
     >>> print extract_text(
     ...     find_tag_by_id(user_browser.contents, 'portlet-packages'))
     All packages
-    Packages in distributions
+    Packages in Ubuntu
     “mozilla-firefox” source package in Hoary
     “mozilla-firefox” source package in Warty Version 0.9 uploaded on...
 

=== modified file 'lib/lp/registry/templates/product-portlet-packages.pt'
--- lib/lp/registry/templates/product-portlet-packages.pt	2010-03-01 21:48:15 +0000
+++ lib/lp/registry/templates/product-portlet-packages.pt	2010-03-09 17:46:07 +0000
@@ -6,14 +6,14 @@
   define="packages context/sourcepackages">
 
 <div class="portlet" id="portlet-packages">
+  <h2>
+    <span class="see-all"><a
+      tal:attributes="href context/menu:overview/packages/fmt:url">
+      All packages</a></span>
+    Packages in Ubuntu
+  </h2>
 
   <tal:has_packages condition="packages">
-    <h2>
-      <span class="see-all"><a
-        tal:attributes="href context/menu:overview/packages/fmt:url">All
-        packages</a></span>
-      Packages in distributions
-    </h2>
 
     <ul>
       <tal:pair tal:repeat="package packages">
@@ -35,12 +35,6 @@
   </tal:has_packages>
 
   <tal:has_no_packages condition="not:packages">
-    <h2>
-      <span class="see-all"><a
-        tal:attributes="href context/menu:overview/packages/fmt:url">All
-        packages</a></span>
-      Packages in Ubuntu
-    </h2>
 
     <p>
       Launchpad doesn't know which Ubuntu packages this project