Merge lp:~stevenk/launchpad/populate-searchables-for-pu into lp:launchpad

[Steve Kowalik]

Populate the new searchable_names and searchable_versions columns on PackageUpload.

Populate the new searchable_names and searchable_versions columns on PackageUpload. The queries the garbo job are running are pretty hideous, but they're temporary and actually quite performant -- dogfood is averaging 840 rows each loop through.

LoC justification is the garbo job is temporary and will be dropped when it's done, and after that we can delete most of IPackageUploadSet.get_all() and the horrible 8 table joins it does today -- that's roughly a 140 line drop. By my calculations, we end up at -50 lines, even including the +36 from three or four database patches.

This can not land until https://code.launchpad.net/~stevenk/launchpad/db-searchableversions-for-pu/+merge/136843 has landed and been deployed.

[William Grant (wgrant)]

20 + searchable_names = None
21 + searchable_versions = None
22 + if package_copy_job is not None:
23 + searchable_names = package_copy_job.package_name
24 + searchable_versions = [package_copy_job.package_version]

Could you make it clear that the non-PCJ case is handled when the PU[SBC] are created? Wouldn't it also make more sense to set searchable_names and searchable_versions to '' and [] respectively in the constructor, and then use the normal add mechanism with the PCJ data?

74 + return self.store.find(
75 + (PackageUpload.id,), PackageUpload.searchable_names == None,
76 + PackageUpload.searchable_versions == None,
77 + PackageUpload.id >= self.start_at).order_by(PackageUpload.id)

I generally prefer to the start the conditions on the line after the findspec, so it's easier to scan.

443 + def setSearchableNames(self, names):
444 + self.searchable_names = ' '.join(
445 + self._appendSearchables(self.searchable_names, names))
446 +
447 + def setSearchableVersions(self, versions):
448 + self.searchable_versions = self._appendSearchables(
449 + self.searchable_versions, versions)

These look like they actually add, not set. It would also be nice to sort the sets (probably in _appendSearchables), so that this is all deterministic.

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-12-03 21:56:19 +0000
+++ database/schema/security.cfg	2012-12-14 00:18:23 +0000
@@ -2253,6 +2253,7 @@
 public.oauthnonce                       = SELECT, DELETE
 public.openidconsumerassociation        = SELECT, DELETE
 public.openidconsumernonce              = SELECT, DELETE
+public.packageupload                    = SELECT, UPDATE
 public.person                           = SELECT, DELETE
 public.product                          = SELECT, UPDATE
 public.pofiletranslator                 = SELECT, INSERT, UPDATE, DELETE
 
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2012-12-06 13:33:46 +0000
+++ lib/lp/registry/model/distroseries.py	2012-12-14 00:18:23 +0000
@@ -1342,9 +1342,8 @@
 
         return PackageUpload(
             distroseries=self, status=PackageUploadStatus.NEW,
-            pocket=pocket, archive=archive,
-            changesfile=changes_file_alias, signing_key=signing_key,
-            package_copy_job=package_copy_job)
+            pocket=pocket, archive=archive, changesfile=changes_file_alias,
+            signing_key=signing_key, package_copy_job=package_copy_job)
 
     def getPackageUploadQueue(self, state):
         """See `IDistroSeries`."""
 
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-12-04 16:34:40 +0000
+++ lib/lp/scripts/garbo.py	2012-12-14 00:18:23 +0000
@@ -18,6 +18,7 @@
     )
 import logging
 import multiprocessing
+from operator import itemgetter
 import os
 import threading
 import time
@@ -122,6 +123,7 @@
 from lp.services.verification.model.logintoken import LoginToken
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.publishing import SourcePackagePublishingHistory
+from lp.soyuz.model.queue import PackageUpload
 from lp.soyuz.model.reporting import LatestPersonSourcePackageReleaseCache
 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
 from lp.translations.interfaces.potemplate import IPOTemplateSet
@@ -1337,6 +1339,131 @@
         transaction.commit()
 
 
+class PopulatePackageUploadSearchables(TunableLoop):
+    """Populates PackageUpload.searchable_names and
+    PackageUpload.searchable_versions."""
+
+    maximum_chunk_size = 5000
+
+    def __init__(self, log, abort_time=None):
+        super(PopulatePackageUploadSearchables, self).__init__(log, abort_time)
+        self.start_at = 1
+        self.store = IMasterStore(PackageUpload)
+
+    def findPackageUploadIDs(self):
+        return self.store.find(
+            (PackageUpload.id,),
+            Or(PackageUpload.searchable_names == None,
+            PackageUpload.searchable_versions == None),
+            PackageUpload.id >= self.start_at).order_by(PackageUpload.id)
+
+    def isDone(self):
+        return self.findPackageUploadIDs().is_empty()
+
+    def __call__(self, chunk_size):
+        packageupload_ids = map(
+            itemgetter(0), list(self.findPackageUploadIDs()[:chunk_size]))
+        # The following SQL links from PU[SBC] to fetch all of the relevant
+        # source names, binary names, libraryfile filenames and their versions.
+        results = self.store.find(
+            (PackageUpload.id, SQL("""
+            (SELECT COALESCE(
+                string_agg(DISTINCT name, ' ' ORDER BY name), '') FROM (
+                (SELECT spn.name
+                    FROM
+                        packageuploadbuild
+                        JOIN binarypackagebuild AS bpb ON
+                        bpb.id = packageuploadbuild.build
+                        JOIN sourcepackagerelease AS spr ON
+                        spr.id = bpb.source_package_release
+                        JOIN sourcepackagename AS spn ON
+                        spn.id = spr.sourcepackagename
+                    WHERE packageuploadbuild.packageupload = packageupload.id
+                )
+                UNION
+                (SELECT bpn.name
+                    FROM
+                        packageuploadbuild
+                        JOIN binarypackagerelease ON
+                        binarypackagerelease.build = packageuploadbuild.build
+                        JOIN binarypackagename AS bpn ON
+                        bpn.id = binarypackagerelease.binarypackagename
+                    WHERE packageuploadbuild.packageupload = packageupload.id
+                )
+                UNION
+                (SELECT sourcepackagename.name
+                    FROM
+                        packageuploadsource
+                        JOIN sourcepackagerelease AS spr ON
+                        spr.id = packageuploadsource.sourcepackagerelease
+                        JOIN sourcepackagename ON
+                        sourcepackagename.id = spr.sourcepackagename
+                    WHERE packageuploadsource.packageupload = packageupload.id
+                )
+                UNION
+                (SELECT lfa.filename
+                    FROM
+                        packageuploadcustom
+                        JOIN libraryfilealias AS lfa ON
+                        lfa.id = packageuploadcustom.libraryfilealias
+                    WHERE packageuploadcustom.packageupload = packageupload.id
+                )
+                UNION
+                (SELECT package_name FROM packagecopyjob
+                WHERE packageupload.package_copy_job = packagecopyjob.id
+        )) AS names (name))
+        """), SQL("""
+        (SELECT COALESCE(array_agg(DISTINCT version ORDER BY version)::text[],
+            ARRAY[]::text[]) FROM (
+            (
+                SELECT spr.version
+                FROM packageuploadsource
+                    JOIN sourcepackagerelease AS spr ON
+                        spr.id = packageuploadsource.sourcepackagerelease
+                WHERE packageuploadsource.packageupload = packageupload.id
+            )
+            UNION
+            (
+                SELECT binarypackagerelease.version
+                FROM packageuploadbuild
+                    JOIN binarypackagerelease ON
+                        binarypackagerelease.build = packageuploadbuild.build
+                WHERE packageuploadbuild.packageupload = packageupload.id
+            )
+            UNION
+            (
+                SELECT (regexp_matches(json_data,
+                    '"package_version": "([^"]+)"')::debversion[])[1]
+                FROM packagecopyjob
+                WHERE packageupload.package_copy_job = packagecopyjob.id
+            )) AS versions (version))
+        """)), PackageUpload.id.is_in(packageupload_ids))
+        # Construct our cache data and populate our Values expression.
+        cache_data = ClassAlias(PackageUpload, "cache_data")
+        updated_columns = dict(
+            [(PackageUpload.searchable_names, cache_data.searchable_names),
+            (PackageUpload.searchable_versions,
+                cache_data.searchable_versions)])
+        values = [
+            [dbify_value(col, val)[0]
+            for (col, val) in zip(
+                (PackageUpload.id, PackageUpload.searchable_names,
+                PackageUpload.searchable_versions), data)]
+            for data in results]
+        cols = [
+            ('id', 'integer'), ('searchable_names', 'text'),
+            ('searchable_versions', 'text[]')]
+        cache_data_expr = Values('cache_data', cols, values)
+        # Using the PackageUpload table, and the pseudo-table Values, set
+        # updated_columns for every row in this loop.
+        self.store.execute(
+            BulkUpdate(
+                updated_columns, table=PackageUpload, values=cache_data_expr,
+                where=PackageUpload.id == cache_data.id))
+        self.start_at = packageupload_ids[-1] + 1
+        transaction.commit()
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1592,6 +1719,7 @@
         UnusedSessionPruner,
         DuplicateSessionPruner,
         BugHeatUpdater,
+        PopulatePackageUploadSearchables,
         ]
     experimental_tunable_loops = []
 
 
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-12-04 16:34:40 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-12-14 00:18:23 +0000
@@ -1275,6 +1275,55 @@
             'PopulateLatestPersonSourcePackageReleaseCache')
         self.assertEqual(spph_2.id, job_data['last_spph_id'])
 
+    def test_PopulatePackageUploadSearchables(self):
+        # PopulatePackageUploadSearchables sets searchable_names and
+        # searchable_versions for existing uploads correctly.
+        switch_dbuser('testadmin')
+        distroseries = self.factory.makeDistroSeries()
+        source = self.factory.makeSourcePackageUpload(distroseries)
+        binary = self.factory.makeBuildPackageUpload(distroseries)
+        build = self.factory.makeBinaryPackageBuild()
+        self.factory.makeBinaryPackageRelease(build=build)
+        binary.addBuild(build)
+        custom = self.factory.makeCustomPackageUpload(distroseries)
+        # They are all have searchable_{names,versions} set, so unset them.
+        for kind in (source, binary, custom):
+            removeSecurityProxy(kind).searchable_names = None
+            removeSecurityProxy(kind).searchable_versions = None
+        transaction.commit()
+        self.runHourly()
+        source_name = source.sources[0].sourcepackagerelease.name
+        binary_names = ' '.join(
+            [build.build.binarypackages[0].name for build in binary.builds] + [
+                build.build.source_package_release.name
+                    for build in binary.builds])
+        filename = custom.customfiles[0].libraryfilealias.filename
+        self.assertEqual(source.searchable_names, source_name)
+        self.assertEqual(binary.searchable_names, binary_names)
+        self.assertEqual(custom.searchable_names, filename)
+        source_version = [source.sources[0].sourcepackagerelease.version]
+        binary_versions = [
+            build.build.binarypackages[0].version for build in binary.builds]
+        self.assertContentEqual(source_version, source.searchable_versions)
+        self.assertContentEqual(binary_versions, binary.searchable_versions)
+        self.assertEqual([], custom.searchable_versions)
+
+    def test_PopulatePackageUploadSearchables_deduplication(self):
+        # When the SPN and the BPN are the same for a build, the
+        # searchable_names field is set to just one name.
+        switch_dbuser('testadmin')
+        distroseries = self.factory.makeDistroSeries()
+        spr = self.factory.makeSourcePackageRelease()
+        bpn = self.factory.makeBinaryPackageName(name=spr.name)
+        binary = self.factory.makeBuildPackageUpload(
+            distroseries=distroseries, binarypackagename=bpn,
+            source_package_release=spr)
+        removeSecurityProxy(binary).searchable_names = None
+        removeSecurityProxy(binary).searchable_versions = None
+        transaction.commit()
+        self.runHourly()
+        self.assertEqual(spr.name, binary.searchable_names)
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer
 
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2012-11-15 01:41:14 +0000
+++ lib/lp/soyuz/configure.zcml	2012-12-14 00:18:23 +0000
@@ -190,7 +190,9 @@
                 package_name
                 package_version
                 section_name
-                components"/>
+                components
+                searchable_names
+                searchable_versions"/>
         <require
             permission="launchpad.Edit"
             attributes="
 
=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2012-11-15 01:41:14 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2012-12-14 00:18:23 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0211,E0213
-
 """Queue interfaces."""
 
 __metaclass__ = type
@@ -212,6 +210,11 @@
     sourcepackagerelease = Attribute(
         "The source package release for this item")
 
+    searchable_names = TextLine(
+        title=_("Searchable names for this item"), readonly=True)
+    searchable_versions = List(
+        title=_("Searchable versions for this item"), readonly=True)
+
     package_name = exported(
         TextLine(
             title=_("Name of the uploaded source package"), readonly=True),
 
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2012-11-15 02:45:58 +0000
+++ lib/lp/soyuz/model/queue.py	2012-12-14 00:18:23 +0000
@@ -1,8 +1,6 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=E0611,W0212
-
 __metaclass__ = type
 __all__ = [
     'PackageUploadQueue',
@@ -23,6 +21,7 @@
     ForeignKey,
     SQLMultipleJoin,
     SQLObjectNotFound,
+    StringCol,
     )
 from storm.expr import LeftJoin
 from storm.locals import (
@@ -30,8 +29,10 @@
     Desc,
     Int,
     Join,
+    List,
     Or,
     Reference,
+    Unicode,
     )
 from storm.store import (
     EmptyResultSet,
@@ -216,30 +217,33 @@
 
     _defaultOrder = ['id']
 
-    status = EnumCol(dbName='status', unique=False, notNull=True,
-                     default=PackageUploadStatus.NEW,
-                     schema=PackageUploadStatus,
-                     storm_validator=validate_status)
+    status = EnumCol(
+        dbName='status', unique=False, notNull=True,
+        default=PackageUploadStatus.NEW, schema=PackageUploadStatus,
+        storm_validator=validate_status)
 
     date_created = UtcDateTimeCol(notNull=False, default=UTC_NOW)
 
-    distroseries = ForeignKey(dbName="distroseries",
-                               foreignKey='DistroSeries')
+    distroseries = ForeignKey(dbName="distroseries", foreignKey='DistroSeries')
 
-    pocket = EnumCol(dbName='pocket', unique=False, notNull=True,
-                     schema=PackagePublishingPocket)
+    pocket = EnumCol(
+        dbName='pocket', unique=False, notNull=True,
+        schema=PackagePublishingPocket)
 
     changesfile = ForeignKey(
         dbName='changesfile', foreignKey="LibraryFileAlias", notNull=False)
 
     archive = ForeignKey(dbName="archive", foreignKey="Archive", notNull=True)
 
-    signing_key = ForeignKey(foreignKey='GPGKey', dbName='signing_key',
-                             notNull=False)
+    signing_key = ForeignKey(
+        foreignKey='GPGKey', dbName='signing_key', notNull=False)
 
     package_copy_job_id = Int(name='package_copy_job', allow_none=True)
     package_copy_job = Reference(package_copy_job_id, 'PackageCopyJob.id')
 
+    searchable_names = StringCol(name='searchable_names', default='')
+    searchable_versions = List(type=Unicode(), default_factory=list)
+
     # XXX julian 2007-05-06:
     # Sources should not be SQLMultipleJoin, there is only ever one
     # of each at most.
@@ -252,6 +256,14 @@
     _builds = SQLMultipleJoin('PackageUploadBuild',
                               joinColumn='packageupload')
 
+    def __init__(self, *args, **kwargs):
+        super(PackageUpload, self).__init__(*args, **kwargs)
+        # searchable_{name,version}s are set for the other cases when
+        # add{Source,Build,Custom} are called.
+        if self.package_copy_job:
+            self.addSearchableNames([self.package_copy_job.package_name])
+            self.addSearchableVersions([self.package_copy_job.package_version])
+
     @cachedproperty
     def sources(self):
         return list(self._sources)
@@ -353,15 +365,13 @@
     def setNew(self):
         """See `IPackageUpload`."""
         if self.status == PackageUploadStatus.NEW:
-            raise QueueInconsistentStateError(
-                'Queue item already new')
+            raise QueueInconsistentStateError('Queue item already new')
         self.status = PassthroughStatusValue(PackageUploadStatus.NEW)
 
     def setUnapproved(self):
         """See `IPackageUpload`."""
         if self.status == PackageUploadStatus.UNAPPROVED:
-            raise QueueInconsistentStateError(
-                'Queue item already unapproved')
+            raise QueueInconsistentStateError('Queue item already unapproved')
         self.status = PassthroughStatusValue(PackageUploadStatus.UNAPPROVED)
 
     def setAccepted(self):
@@ -374,8 +384,7 @@
             self.pocket.name, self.distroseries.status.name))
 
         if self.status == PackageUploadStatus.ACCEPTED:
-            raise QueueInconsistentStateError(
-                'Queue item already accepted')
+            raise QueueInconsistentStateError('Queue item already accepted')
 
         for source in self.sources:
             source.verifyBeforeAccept()
@@ -461,15 +470,13 @@
     def setDone(self):
         """See `IPackageUpload`."""
         if self.status == PackageUploadStatus.DONE:
-            raise QueueInconsistentStateError(
-                'Queue item already done')
+            raise QueueInconsistentStateError('Queue item already done')
         self.status = PassthroughStatusValue(PackageUploadStatus.DONE)
 
     def setRejected(self):
         """See `IPackageUpload`."""
         if self.status == PackageUploadStatus.REJECTED:
-            raise QueueInconsistentStateError(
-                'Queue item already rejected')
+            raise QueueInconsistentStateError('Queue item already rejected')
         self.status = PassthroughStatusValue(PackageUploadStatus.REJECTED)
 
     def _closeBugs(self, changesfile_path, logger=None):
@@ -709,20 +716,17 @@
     @cachedproperty
     def contains_upgrader(self):
         """See `IPackageUpload`."""
-        return (PackageUploadCustomFormat.DIST_UPGRADER
-                in self._customFormats)
+        return PackageUploadCustomFormat.DIST_UPGRADER in self._customFormats
 
     @cachedproperty
     def contains_ddtp(self):
         """See `IPackageUpload`."""
-        return (PackageUploadCustomFormat.DDTP_TARBALL
-                in self._customFormats)
+        return PackageUploadCustomFormat.DDTP_TARBALL in self._customFormats
 
     @cachedproperty
     def contains_uefi(self):
         """See `IPackageUpload`."""
-        return (PackageUploadCustomFormat.UEFI
-                in self._customFormats)
+        return PackageUploadCustomFormat.UEFI in self._customFormats
 
     @property
     def package_name(self):
@@ -853,26 +857,43 @@
 
         return publishing_records
 
+    def _appendSearchables(self, existing, new):
+        return sorted(filter(None, set(existing) | set(new)))
+
+    def addSearchableNames(self, names):
+        self.searchable_names = ' '.join(
+            self._appendSearchables(self.searchable_names.split(' '), names))
+
+    def addSearchableVersions(self, versions):
+        self.searchable_versions = self._appendSearchables(
+            self.searchable_versions, versions)
+
     def addSource(self, spr):
         """See `IPackageUpload`."""
         del get_property_cache(self).sources
+        self.addSearchableNames([spr.name])
+        self.addSearchableVersions([spr.version])
         return PackageUploadSource(
-            packageupload=self,
-            sourcepackagerelease=spr.id)
+            packageupload=self, sourcepackagerelease=spr.id)
 
     def addBuild(self, build):
         """See `IPackageUpload`."""
         del get_property_cache(self).builds
-        return PackageUploadBuild(
-            packageupload=self,
-            build=build.id)
+        names = [build.source_package_release.name]
+        versions = []
+        for bpr in build.binarypackages:
+            names.append(bpr.name)
+            versions.append(bpr.version)
+        self.addSearchableNames(names)
+        self.addSearchableVersions(versions)
+        return PackageUploadBuild(packageupload=self, build=build.id)
 
     def addCustom(self, library_file, custom_type):
         """See `IPackageUpload`."""
         del get_property_cache(self).customfiles
+        self.addSearchableNames([library_file.filename])
         return PackageUploadCustom(
-            packageupload=self,
-            libraryfilealias=library_file.id,
+            packageupload=self, libraryfilealias=library_file.id,
             customformat=custom_type)
 
     def isPPA(self):
@@ -1361,15 +1382,14 @@
     _defaultOrder = ['id']
 
     packageupload = ForeignKey(
-        dbName='packageupload',
-        foreignKey='PackageUpload')
-
-    customformat = EnumCol(dbName='customformat', unique=False,
-                           notNull=True, schema=PackageUploadCustomFormat)
-
-    libraryfilealias = ForeignKey(dbName='libraryfilealias',
-                                  foreignKey="LibraryFileAlias",
-                                  notNull=True)
+        dbName='packageupload', foreignKey='PackageUpload')
+
+    customformat = EnumCol(
+        dbName='customformat', unique=False, notNull=True,
+        schema=PackageUploadCustomFormat)
+
+    libraryfilealias = ForeignKey(
+        dbName='libraryfilealias', foreignKey="LibraryFileAlias", notNull=True)
 
     def publish(self, logger=None):
         """See `IPackageUploadCustom`."""
@@ -1425,8 +1445,7 @@
         """See `IPackageUploadCustom`."""
         # XXX cprov 2005-03-03: We need to use the Zope Component Lookup
         # to instantiate the object in question and avoid circular imports
-        from lp.archivepublisher.dist_upgrader import (
-            process_dist_upgrader)
+        from lp.archivepublisher.dist_upgrader import process_dist_upgrader
 
         self._publishCustom(process_dist_upgrader, logger=logger)
 
@@ -1434,8 +1453,7 @@
         """See `IPackageUploadCustom`."""
         # XXX cprov 2005-03-03: We need to use the Zope Component Lookup
         # to instantiate the object in question and avoid circular imports
-        from lp.archivepublisher.ddtp_tarball import (
-            process_ddtp_tarball)
+        from lp.archivepublisher.ddtp_tarball import process_ddtp_tarball
 
         self._publishCustom(process_ddtp_tarball, logger=logger)
 
 
=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2012-11-15 01:41:14 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2012-12-14 00:18:23 +0000
@@ -116,6 +116,8 @@
         upload.addSource(spr)
         self.assertEqual(spr.sourcepackagename.name, upload.package_name)
         self.assertEqual(spr.version, upload.package_version)
+        self.assertEqual(spr.name, upload.searchable_names)
+        self.assertContentEqual([spr.version], upload.searchable_versions)
 
     def test_publish_sets_packageupload(self):
         # Publishing a PackageUploadSource will pass itself to the source
@@ -459,9 +461,45 @@
         upload, job = self.makeUploadWithPackageCopyJob()
         self.assertEqual(job.package_name, upload.package_name)
         self.assertEqual(job.package_version, upload.package_version)
+        self.assertEqual(job.package_name, upload.searchable_names)
+        self.assertContentEqual(
+            [job.package_version], upload.searchable_versions)
+
+    def test_searchables_for_builds(self):
+        distroseries = self.factory.makeDistroSeries()
+        upload = self.factory.makeBuildPackageUpload(distroseries)
+        build = self.factory.makeBinaryPackageBuild()
+        self.factory.makeBinaryPackageRelease(build=build)
+        upload.addBuild(build)
+        name_array = [
+            build.build.binarypackages[0].name for build in upload.builds]
+        name_array.extend(
+            [b.build.source_package_release.name for b in upload.builds])
+        names = ' '.join(sorted(name_array))
+        self.assertEqual(names, upload.searchable_names)
+        self.assertContentEqual(
+            [build.build.binarypackages[0].version for build in upload.builds],
+            upload.searchable_versions)
+
+    def test_searchables_for_builds_duplication(self):
+        distroseries = self.factory.makeDistroSeries()
+        spr = self.factory.makeSourcePackageRelease()
+        bpn = self.factory.makeBinaryPackageName(name=spr.name)
+        binary = self.factory.makeBuildPackageUpload(
+            distroseries=distroseries, binarypackagename=bpn,
+            source_package_release=spr)
+        self.assertEqual(spr.name, binary.searchable_names)
+
+    def test_searchables_for_custom(self):
+        distroseries = self.factory.makeDistroSeries()
+        upload = self.factory.makeCustomPackageUpload(distroseries)
+        self.assertEqual(
+            upload.searchable_names,
+            upload.customfiles[0].libraryfilealias.filename)
+        self.assertEqual([], upload.searchable_versions)
 
     def test_displayarchs_for_copy_job_is_sync(self):
-        # For copy jobs, displayarchs is "source."
+        # For copy jobs, displayarchs is "sync."
         upload, job = self.makeUploadWithPackageCopyJob()
         self.assertEqual('sync', upload.displayarchs)
 
 
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-12-06 02:03:50 +0000
+++ lib/lp/testing/factory.py	2012-12-14 00:18:23 +0000
@@ -3437,10 +3437,10 @@
             pocket=pocket)
         build = self.makeBinaryPackageBuild(
             source_package_release=source_package_release, pocket=pocket)
-        upload.addBuild(build)
         self.makeBinaryPackageRelease(
             binarypackagename=binarypackagename, build=build,
             component=component)
+        upload.addBuild(build)
         return upload
 
     def makeCustomPackageUpload(self, distroseries=None, archive=None,