← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/queue-api-readonly into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/queue-api-readonly into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1006173 in Launchpad itself: "Queue tool requires direct DB access"
  https://bugs.launchpad.net/launchpad/+bug/1006173

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/queue-api-readonly/+merge/113202

== Summary ==

Replace the queue tool in Launchpad with an API client (part four).

== Proposed fix ==

This branch exports the necessary read-only properties and methods on PackageUpload.

== Pre-implementation notes ==

I've gone round a few times with various people, particularly William Grant, on the exact way to export all of this stuff, because I gather that we want to avoid exposing the current data model in order that it can be rearranged in the future.  This has led to the following design choices:

 * Everything is on devel.  The only clients for this should be tools such as those in lp:ubuntu-archive-tools, which can be kept up to date if there's a need to change these interfaces.
 * Even though some of the underlying methods are on other objects, all the new exported methods are on PackageUpload rather than exporting anything else.
 * There are source packages with lots of binaries that sometimes need to be overridden individually (e.g. linux) and API requests aren't especially fast.  I've therefore arranged for properties (including overrides) of all binaries in an upload to come back as a list of dicts in a single JSON response.

I extracted this branch from https://code.launchpad.net/~cjwatson/launchpad/queue-api/+merge/108967 at Benji's request.

== LOC Rationale ==

+347.  As with https://code.launchpad.net/~cjwatson/launchpad/queue-api/+merge/108967, this arc will be LoC-negative.

== Tests ==

bin/test -vvct distroseriesqueue.txt -t xx-packageupload.txt -t test_distroseriesqueue -t test_packageupload

== Demo and Q/A ==

http://paste.ubuntu.com/1072964/ is the current version of my client.  With this branch, it should be possible to get full information on all uploads equivalent to that provided by the queue tool, and it should be possible to use "queue fetch" to download them.
-- 
https://code.launchpad.net/~cjwatson/launchpad/queue-api-readonly/+merge/113202
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/queue-api-readonly into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/tests/nascentupload.txt'
--- lib/lp/archiveuploader/tests/nascentupload.txt	2012-01-20 15:42:44 +0000
+++ lib/lp/archiveuploader/tests/nascentupload.txt	2012-07-03 12:23:23 +0000
@@ -534,7 +534,7 @@
     >>> multibar_src_queue.status.name
     'NEW'
 
-    >>> multibar_src_queue.sources.count()
+    >>> len(multibar_src_queue.sources)
     1
     >>> multibar_spr = multibar_src_queue.sources[0].sourcepackagerelease
     >>> multibar_spr.title
@@ -596,7 +596,7 @@
     >>> multibar_bin_queue = multibar_bin_upload.queue_root
     >>> multibar_bin_queue.status.name
     'NEW'
-    >>> multibar_bin_queue.builds.count()
+    >>> len(multibar_bin_queue.builds)
     1
 
 The build considered as 'producer' of the upload binaries is the same

=== modified file 'lib/lp/archiveuploader/tests/test_buildduploads.py'
--- lib/lp/archiveuploader/tests/test_buildduploads.py	2011-12-30 06:14:56 +0000
+++ lib/lp/archiveuploader/tests/test_buildduploads.py	2012-07-03 12:23:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test buildd uploads use-cases."""
@@ -136,7 +136,7 @@
         self.assertTrue(
             queue_item is not None,
             "Binary Upload Failed\nGot: %s" % self.log.getLogBuffer())
-        self.assertEqual(queue_item.builds.count(), 1)
+        self.assertEqual(1, len(queue_item.builds))
         return queue_item.builds[0].build
 
     def _createBuild(self, archtag):

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2012-06-22 05:25:35 +0000
+++ lib/lp/soyuz/configure.zcml	2012-07-03 12:23:23 +0000
@@ -157,18 +157,24 @@
                 distroseries
                 pocket
                 changesfile
+                changes_file_url
                 signing_key
                 archive
                 sources
+                sourceFileUrls
                 builds
+                binaryFileUrls
                 customfiles
                 custom_file_urls
+                customFileUrls
+                getBinaryProperties
                 date_created
                 sourcepackagerelease
                 component_name
                 concrete_package_copy_job
                 contains_source
                 contains_build
+                contains_copy
                 contains_translation
                 contains_installer
                 contains_upgrader

=== modified file 'lib/lp/soyuz/interfaces/binarypackagerelease.py'
--- lib/lp/soyuz/interfaces/binarypackagerelease.py	2011-12-24 16:54:44 +0000
+++ lib/lp/soyuz/interfaces/binarypackagerelease.py	2012-07-03 12:23:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# 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
@@ -98,6 +98,13 @@
         description=_("True if there binary version was never published for "
                       "the architeture it was built for. False otherwise."))
 
+    def properties():
+        """Returns the properties of this binary.
+
+        For fast retrieval over the webservice, this is returned as a
+        dictionary.
+        """
+
     def addFile(file):
         """Create a BinaryPackageFile record referencing this build
         and attach the provided library file alias (file).

=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2012-06-30 17:41:37 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2012-07-03 12:23:23 +0000
@@ -28,6 +28,7 @@
 from lazr.restful.declarations import (
     error_status,
     export_as_webservice_entry,
+    export_read_operation,
     export_write_operation,
     exported,
     operation_for_version,
@@ -38,6 +39,7 @@
     Interface,
     )
 from zope.schema import (
+    Bool,
     Choice,
     Datetime,
     Int,
@@ -141,6 +143,14 @@
 
     changesfile = Attribute("The librarian alias for the changes file "
                             "associated with this upload")
+    changes_file_url = exported(
+        TextLine(
+            title=_("Changes file URL"),
+            description=_("Librarian URL for the changes file associated with "
+                          "this upload. Will be None if the upload was copied "
+                          "from another series."),
+            required=False, readonly=True),
+        as_of="devel")
 
     signing_key = Attribute("Changesfile Signing Key.")
 
@@ -162,17 +172,18 @@
             title=_("Archive"), required=True, readonly=True))
     sources = Attribute("The queue sources associated with this queue item")
     builds = Attribute("The queue builds associated with the queue item")
+
     customfiles = Attribute("Custom upload files associated with this "
                             "queue item")
-
     custom_file_urls = exported(
         List(
-            title=_("Custom File URLs"),
+            title=_("Custom file URLs"),
             description=_("Librarian URLs for all the custom files attached "
                           "to this upload."),
             value_type=TextLine(),
             required=False,
-            readonly=True))
+            readonly=True),
+        ("devel", dict(exported=False)), exported=True)
 
     displayname = exported(
         TextLine(
@@ -191,17 +202,39 @@
     sourcepackagerelease = Attribute(
         "The source package release for this item")
 
-    package_name = TextLine(
-        title=_("Name of the uploaded source package"), readonly=True)
-
-    package_version = TextLine(
-        title=_("Source package version"), readonly=True)
-
-    component_name = TextLine(
-        title=_("Source package component name"), readonly=True)
-
-    contains_source = Attribute("whether or not this upload contains sources")
-    contains_build = Attribute("whether or not this upload contains binaries")
+    package_name = exported(
+        TextLine(
+            title=_("Name of the uploaded source package"), readonly=True),
+        as_of="devel")
+
+    package_version = exported(
+        TextLine(title=_("Source package version"), readonly=True),
+        as_of="devel")
+
+    component_name = exported(
+        TextLine(title=_("Source package component name"), readonly=True),
+        as_of="devel")
+
+    section_name = exported(
+        TextLine(title=_("Source package section name"), readonly=True),
+        as_of="devel")
+
+    contains_source = exported(
+        Bool(
+            title=_("Whether or not this upload contains sources"),
+            readonly=True),
+        as_of="devel")
+    contains_build = exported(
+        Bool(
+            title=_("Whether or not this upload contains binaries"),
+            readonly=True),
+        as_of="devel")
+    contains_copy = exported(
+        Bool(
+            title=_("Whether or not this upload contains a copy from another "
+                    "series."),
+            readonly=True),
+        as_of="devel")
     contains_installer = Attribute(
         "whether or not this upload contains installers images")
     contains_translation = Attribute(
@@ -223,8 +256,38 @@
         on all the binarypackagerelease records arising from the build.
         """)
 
-    section_name = TextLine(
-        title=_("Source package sectio name"), readonly=True)
+    @export_read_operation()
+    @operation_for_version("devel")
+    def sourceFileUrls():
+        """URLs for all the source files attached to this upload.
+
+        :return: A collection of URLs for this upload.
+        """
+
+    @export_read_operation()
+    @operation_for_version("devel")
+    def binaryFileUrls():
+        """URLs for all the binary files attached to this upload.
+
+        :return: A collection of URLs for this upload.
+        """
+
+    @export_read_operation()
+    @operation_for_version("devel")
+    def customFileUrls():
+        """URLs for all the custom files attached to this upload.
+
+        :return: A collection of URLs for this upload.
+        """
+
+    @export_read_operation()
+    @operation_for_version("devel")
+    def getBinaryProperties():
+        """The properties of the binaries associated with this queue item.
+
+        :return: A list of dictionaries, each containing the properties of a
+            single binary.
+        """
 
     def setNew():
         """Set queue state to NEW."""
@@ -382,13 +445,20 @@
 
     packageupload = Int(
             title=_("PackageUpload"), required=True,
-            readonly=False,
+            readonly=True,
             )
 
     build = Int(
             title=_("The related build"), required=True, readonly=False,
             )
 
+    def binaries():
+        """Returns the properties of the binaries in this build.
+
+        For fast retrieval over the webservice, these are returned as a list
+        of dictionaries, one per binary.
+        """
+
     def publish(logger=None):
         """Publish this queued source in the distroseries referred to by
         the parent queue item.

=== modified file 'lib/lp/soyuz/model/binarypackagerelease.py'
--- lib/lp/soyuz/model/binarypackagerelease.py	2012-04-16 23:02:44 +0000
+++ lib/lp/soyuz/model/binarypackagerelease.py	2012-07-03 12:23:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# 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
@@ -131,6 +131,18 @@
             self.binarypackagename)
         return distroarchseries_binary_package.currentrelease is None
 
+    @property
+    def properties(self):
+        return {
+            "name": self.name,
+            "version": self.version,
+            "is_new": self.is_new,
+            "architecture": self.build.arch_tag,
+            "component": self.component.name,
+            "section": self.section.name,
+            "priority": self.priority.name,
+            }
+
     @cachedproperty
     def files(self):
         return list(
@@ -201,4 +213,3 @@
     def binary_package_version(self):
         """See `IBinaryPackageReleaseDownloadCount`."""
         return self.binary_package_release.version
-

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2012-07-03 08:04:35 +0000
+++ lib/lp/soyuz/model/queue.py	2012-07-03 12:23:23 +0000
@@ -13,6 +13,7 @@
     'PackageUploadSet',
     ]
 
+from itertools import chain
 import os
 import shutil
 import StringIO
@@ -50,8 +51,10 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.services.config import config
+from lp.services.database.bulk import load_referencing
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.datetimecol import UtcDateTimeCol
+from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.lpstorm import (
     IMasterStore,
@@ -61,11 +64,15 @@
     SQLBase,
     sqlvalues,
     )
+from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.librarian.interfaces.client import DownloadFailed
 from lp.services.librarian.model import LibraryFileAlias
 from lp.services.librarian.utils import copy_and_close
 from lp.services.mail.signedmessage import strip_pgp_signature
-from lp.services.propertycache import cachedproperty
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 from lp.soyuz.adapters.notification import notify
 from lp.soyuz.adapters.overrides import SourceOverride
 from lp.soyuz.enums import (
@@ -230,11 +237,44 @@
 
     # Join this table to the PackageUploadBuild and the
     # PackageUploadSource objects which are related.
-    sources = SQLMultipleJoin('PackageUploadSource',
+    _sources = SQLMultipleJoin('PackageUploadSource',
+                               joinColumn='packageupload')
+    # Does not include source builds.
+    _builds = SQLMultipleJoin('PackageUploadBuild',
                               joinColumn='packageupload')
-    # Does not include source builds.
-    builds = SQLMultipleJoin('PackageUploadBuild',
-                             joinColumn='packageupload')
+
+    @cachedproperty
+    def sources(self):
+        return list(self._sources)
+
+    def sourceFileUrls(self):
+        """See `IPackageUpload`."""
+        if self.contains_source:
+            return [
+                ProxiedLibraryFileAlias(
+                    file.libraryfile, self.archive).http_url
+                for file in self.sourcepackagerelease.files]
+        else:
+            return []
+
+    @cachedproperty
+    def builds(self):
+        return list(self._builds)
+
+    def binaryFileUrls(self):
+        """See `IPackageUpload`."""
+        return [
+            ProxiedLibraryFileAlias(file.libraryfile, self.archive).http_url
+            for build in self.builds
+            for bpr in build.build.binarypackages
+            for file in bpr.files]
+
+    @property
+    def changes_file_url(self):
+        if self.changesfile is not None:
+            return self.changesfile.getURL()
+        else:
+            return None
 
     def getSourceBuild(self):
         #avoid circular import
@@ -250,8 +290,12 @@
             PackageUploadSource.packageupload == self.id).one()
 
     # Also the custom files associated with the build.
-    customfiles = SQLMultipleJoin('PackageUploadCustom',
-                                  joinColumn='packageupload')
+    _customfiles = SQLMultipleJoin('PackageUploadCustom',
+                                   joinColumn='packageupload')
+
+    @cachedproperty
+    def customfiles(self):
+        return list(self._customfiles)
 
     @property
     def custom_file_urls(self):
@@ -259,6 +303,18 @@
         return tuple(
             file.libraryfilealias.getURL() for file in self.customfiles)
 
+    def customFileUrls(self):
+        """See `IPackageUpload`."""
+        return [
+            ProxiedLibraryFileAlias(
+                file.libraryfilealias, self.archive).http_url
+            for file in self.customfiles]
+
+    def getBinaryProperties(self):
+        """See `IPackageUpload`."""
+        return list(chain.from_iterable(
+            build.binaries for build in self.builds))
+
     def setNew(self):
         """See `IPackageUpload`."""
         if self.status == PackageUploadStatus.NEW:
@@ -543,7 +599,7 @@
     def acceptFromCopy(self):
         """See `IPackageUpload`."""
         assert self.is_delayed_copy, 'Can only process delayed-copies.'
-        assert self.sources.count() == 1, (
+        assert self._sources.count() == 1, (
             'Source is mandatory for delayed copies.')
         self.setAccepted()
 
@@ -580,7 +636,7 @@
 
     def _isSingleSourceUpload(self):
         """Return True if this upload contains only a single source."""
-        return ((self.sources.count() == 1) and
+        return ((self._sources.count() == 1) and
                 (not bool(self.builds)) and
                 (not bool(self.customfiles)))
 
@@ -589,12 +645,17 @@
     @cachedproperty
     def contains_source(self):
         """See `IPackageUpload`."""
-        return self.sources
+        return bool(self.sources)
 
     @cachedproperty
     def contains_build(self):
         """See `IPackageUpload`."""
-        return self.builds
+        return bool(self.builds)
+
+    @cachedproperty
+    def contains_copy(self):
+        """See `IPackageUpload`."""
+        return self.package_copy_job_id is not None
 
     @cachedproperty
     def from_build(self):
@@ -802,18 +863,21 @@
 
     def addSource(self, spr):
         """See `IPackageUpload`."""
+        del get_property_cache(self).sources
         return PackageUploadSource(
             packageupload=self,
             sourcepackagerelease=spr.id)
 
     def addBuild(self, build):
         """See `IPackageUpload`."""
+        del get_property_cache(self).builds
         return PackageUploadBuild(
             packageupload=self,
             build=build.id)
 
     def addCustom(self, library_file, custom_type):
         """See `IPackageUpload`."""
+        del get_property_cache(self).customfiles
         return PackageUploadCustom(
             packageupload=self,
             libraryfilealias=library_file.id,
@@ -1012,6 +1076,12 @@
 
     build = ForeignKey(dbName='build', foreignKey='BinaryPackageBuild')
 
+    @property
+    def binaries(self):
+        """See `IPackageUploadBuild`."""
+        for binary in self.build.binarypackages:
+            yield binary.properties
+
     def checkComponentAndSection(self):
         """See `IPackageUploadBuild`."""
         distroseries = self.packageupload.distroseries
@@ -1619,7 +1689,30 @@
             PackageUpload.distroseries == distroseries,
             *conditions)
         query = query.order_by(Desc(PackageUpload.id))
-        return query.config(distinct=True)
+        query = query.config(distinct=True)
+
+        def preload_hook(rows):
+            puses = load_referencing(
+                PackageUploadSource, rows, ["packageuploadID"])
+            pubs = load_referencing(
+                PackageUploadBuild, rows, ["packageuploadID"])
+            pucs = load_referencing(
+                PackageUploadCustom, rows, ["packageuploadID"])
+
+            for pu in rows:
+                cache = get_property_cache(pu)
+                cache.sources = []
+                cache.builds = []
+                cache.customfiles = []
+
+            for pus in puses:
+                get_property_cache(pus.packageupload).sources.append(pus)
+            for pub in pubs:
+                get_property_cache(pub.packageupload).builds.append(pub)
+            for puc in pucs:
+                get_property_cache(puc.packageupload).customfiles.append(puc)
+
+        return DecoratedResultSet(query, pre_iter_hook=preload_hook)
 
     def getBuildByBuildIDs(self, build_ids):
         """See `IPackageUploadSet`."""

=== modified file 'lib/lp/soyuz/scripts/queue.py'
--- lib/lp/soyuz/scripts/queue.py	2012-06-29 08:40:05 +0000
+++ lib/lp/soyuz/scripts/queue.py	2012-07-03 12:23:23 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# 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=W0231
@@ -260,8 +260,8 @@
             False: '-',
         }
         return (
-            source_tag[bool(queue_item.contains_source)] +
-            binary_tag[bool(queue_item.contains_build)])
+            source_tag[queue_item.contains_source] +
+            binary_tag[queue_item.contains_build])
 
     def displayItem(self, queue_item):
         """Display one line summary of the queue item provided."""

=== modified file 'lib/lp/soyuz/tests/test_distroseriesqueue_debian_installer.py'
--- lib/lp/soyuz/tests/test_distroseriesqueue_debian_installer.py	2012-05-28 12:50:34 +0000
+++ lib/lp/soyuz/tests/test_distroseriesqueue_debian_installer.py	2012-07-03 12:23:23 +0000
@@ -49,7 +49,7 @@
 
     def test_accepts_correct_upload(self):
         upload = self.uploadTestData()
-        self.assertEqual(1, upload.queue_root.customfiles.count())
+        self.assertEqual(1, len(upload.queue_root.customfiles))
 
     def test_generates_mail(self):
         # Two e-mail messages were generated (acceptance and announcement).

=== modified file 'lib/lp/soyuz/tests/test_distroseriesqueue_dist_upgrader.py'
--- lib/lp/soyuz/tests/test_distroseriesqueue_dist_upgrader.py	2012-05-25 13:27:41 +0000
+++ lib/lp/soyuz/tests/test_distroseriesqueue_dist_upgrader.py	2012-07-03 12:23:23 +0000
@@ -107,7 +107,7 @@
         # Make sure that we can use the librarian files.
         transaction.commit()
         self.assertFalse(upload.queue_root.realiseUpload(self.logger))
-        self.assertEqual(1, upload.queue_root.customfiles.count())
+        self.assertEqual(1, len(upload.queue_root.customfiles))
         self.assertRaises(
             DistUpgraderBadVersion, upload.queue_root.customfiles[0].publish,
             self.logger)

=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2012-06-07 16:43:28 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2012-07-03 12:23:23 +0000
@@ -12,9 +12,11 @@
     BadRequest,
     Unauthorized,
     )
+from testtools.matchers import Equals
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
+from zope.schema import getFields
 
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
 from lp.archiveuploader.tests import datadir
@@ -25,6 +27,7 @@
 from lp.services.config import config
 from lp.services.database.lpstorm import IStore
 from lp.services.job.interfaces.job import JobStatus
+from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.log.logger import BufferLogger
 from lp.services.mail import stub
 from lp.soyuz.adapters.overrides import SourceOverride
@@ -37,6 +40,7 @@
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.queue import (
+    IPackageUpload,
     IPackageUploadSet,
     QueueInconsistentStateError,
     )
@@ -48,6 +52,7 @@
     api_url,
     launchpadlib_for,
     person_logged_in,
+    StormStatementRecorder,
     TestCaseWithFactory,
     )
 from lp.testing.dbuser import switch_dbuser
@@ -55,7 +60,10 @@
     LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,
     )
-from lp.testing.matchers import Provides
+from lp.testing.matchers import (
+    HasQueryCount,
+    Provides,
+    )
 
 
 class PackageUploadTestCase(TestCaseWithFactory):
@@ -862,6 +870,20 @@
             list(reversed(ordered_uploads)),
             list(getUtility(IPackageUploadSet).getAll(series)))
 
+    def test_getAll_can_preload_exported_properties(self):
+        # getAll preloads everything exported on the webservice.
+        distroseries = self.factory.makeDistroSeries()
+        self.factory.makeSourcePackageUpload(distroseries=distroseries)
+        self.factory.makeBuildPackageUpload(distroseries=distroseries)
+        self.factory.makeCustomPackageUpload(distroseries=distroseries)
+        uploads = list(getUtility(IPackageUploadSet).getAll(distroseries))
+        with StormStatementRecorder() as recorder:
+            for name, field in getFields(IPackageUpload).items():
+                if field.queryTaggedValue("lazr.restful.exported") is not None:
+                    for upload in uploads:
+                        getattr(upload, name)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+
     def test_rejectFromQueue_no_changes_file(self):
         # If the PackageUpload has no changesfile, we can still reject it.
         pu = self.factory.makePackageUpload()
@@ -878,12 +900,13 @@
     def setUp(self):
         super(TestPackageUploadWebservice, self).setUp()
         self.webservice = None
-
-    def makeDistroSeries(self):
         self.distroseries = self.factory.makeDistroSeries()
         self.main = self.factory.makeComponent("main")
         self.factory.makeComponentSelection(
             distroseries=self.distroseries, component=self.main)
+        self.universe = self.factory.makeComponent("universe")
+        self.factory.makeComponentSelection(
+            distroseries=self.distroseries, component=self.universe)
 
     def makeQueueAdmin(self, components):
         person = self.factory.makePerson()
@@ -900,14 +923,52 @@
             self.webservice = launchpadlib_for("testing", person)
         return self.webservice.load(api_url(obj))
 
+    def makeSourcePackageUpload(self, person, **kwargs):
+        with person_logged_in(person):
+            upload = self.factory.makeSourcePackageUpload(
+                distroseries=self.distroseries, **kwargs)
+            transaction.commit()
+            spr = upload.sourcepackagerelease
+            for extension in ("dsc", "tar.gz"):
+                filename = "%s_%s.%s" % (spr.name, spr.version, extension)
+                lfa = self.factory.makeLibraryFileAlias(filename=filename)
+                spr.addFile(lfa)
+        transaction.commit()
+        return upload, self.load(upload, person)
+
+    def makeBinaryPackageUpload(self, person, binarypackagename=None,
+                                component=None):
+        with person_logged_in(person):
+            upload = self.factory.makeBuildPackageUpload(
+                distroseries=self.distroseries,
+                binarypackagename=binarypackagename, component=component)
+            self.factory.makeBinaryPackageRelease(
+                build=upload.builds[0].build, component=component)
+            transaction.commit()
+            for build in upload.builds:
+                for bpr in build.build.binarypackages:
+                    filename = "%s_%s_%s.deb" % (
+                        bpr.name, bpr.version, bpr.build.arch_tag)
+                    lfa = self.factory.makeLibraryFileAlias(filename=filename)
+                    bpr.addFile(lfa)
+        transaction.commit()
+        return upload, self.load(upload, person)
+
+    def makeCustomPackageUpload(self, person, **kwargs):
+        with person_logged_in(person):
+            upload = self.factory.makeCustomPackageUpload(
+                distroseries=self.distroseries, **kwargs)
+        transaction.commit()
+        return upload, self.load(upload, person)
+
     def assertRequiresEdit(self, method_name, **kwargs):
         """Test that a web service queue method requires launchpad.Edit."""
         with admin_logged_in():
             upload = self.factory.makeSourcePackageUpload()
         transaction.commit()
         ws_upload = self.load(upload)
-        self.assertRaises(Unauthorized, getattr(ws_upload, method_name),
-                          **kwargs)
+        self.assertRaises(
+            Unauthorized, getattr(ws_upload, method_name), **kwargs)
 
     def test_edit_permissions(self):
         self.assertRequiresEdit("acceptFromQueue")
@@ -915,42 +976,135 @@
 
     def test_acceptFromQueue_archive_admin(self):
         # acceptFromQueue as an archive admin accepts the upload.
-        self.makeDistroSeries()
         person = self.makeQueueAdmin([self.main])
-        with person_logged_in(person):
-            upload = self.factory.makeSourcePackageUpload(
-                distroseries=self.distroseries, component=self.main)
-        transaction.commit()
+        upload, ws_upload = self.makeSourcePackageUpload(
+            person, component=self.main)
 
-        ws_upload = self.load(upload, person)
         self.assertEqual("New", ws_upload.status)
         ws_upload.acceptFromQueue()
         self.assertEqual("Done", ws_upload.status)
 
     def test_double_accept_raises_BadRequest(self):
         # Trying to accept an upload twice returns 400 instead of OOPSing.
-        self.makeDistroSeries()
         person = self.makeQueueAdmin([self.main])
+        upload, _ = self.makeSourcePackageUpload(person, component=self.main)
+
         with person_logged_in(person):
-            upload = self.factory.makeSourcePackageUpload(
-                distroseries=self.distroseries, component=self.main)
             upload.setAccepted()
-        transaction.commit()
-
         ws_upload = self.load(upload, person)
         self.assertEqual("Accepted", ws_upload.status)
         self.assertRaises(BadRequest, ws_upload.acceptFromQueue)
 
     def test_rejectFromQueue_archive_admin(self):
         # rejectFromQueue as an archive admin rejects the upload.
-        self.makeDistroSeries()
         person = self.makeQueueAdmin([self.main])
-        with person_logged_in(person):
-            upload = self.factory.makeSourcePackageUpload(
-                distroseries=self.distroseries, component=self.main)
-        transaction.commit()
+        upload, ws_upload = self.makeSourcePackageUpload(
+            person, component=self.main)
 
-        ws_upload = self.load(upload, person)
         self.assertEqual("New", ws_upload.status)
         ws_upload.rejectFromQueue()
         self.assertEqual("Rejected", ws_upload.status)
+
+    def test_source_info(self):
+        # API clients can inspect properties of source uploads.
+        person = self.makeQueueAdmin([self.universe])
+        upload, ws_upload = self.makeSourcePackageUpload(
+            person, sourcepackagename="hello", component=self.universe)
+
+        self.assertTrue(ws_upload.contains_source)
+        self.assertFalse(ws_upload.contains_build)
+        self.assertFalse(ws_upload.contains_copy)
+        self.assertEqual("hello", ws_upload.display_name)
+        self.assertEqual(upload.package_version, ws_upload.display_version)
+        self.assertEqual("source", ws_upload.display_arches)
+        self.assertEqual("hello", ws_upload.package_name)
+        self.assertEqual(upload.package_version, ws_upload.package_version)
+        self.assertEqual("universe", ws_upload.component_name)
+        self.assertEqual(upload.section_name, ws_upload.section_name)
+
+    def test_source_fetch(self):
+        # API clients can fetch files attached to source uploads.
+        person = self.makeQueueAdmin([self.universe])
+        upload, ws_upload = self.makeSourcePackageUpload(
+            person, component=self.universe)
+        ws_source_file_urls = ws_upload.sourceFileUrls()
+        self.assertNotEqual(0, len(ws_source_file_urls))
+        with person_logged_in(person):
+            source_file_urls = [
+                ProxiedLibraryFileAlias(
+                    file.libraryfile, upload.archive).http_url
+                for file in upload.sourcepackagerelease.files]
+        self.assertContentEqual(source_file_urls, ws_source_file_urls)
+
+    def test_binary_info(self):
+        # API clients can inspect properties of binary uploads.
+        person = self.makeQueueAdmin([self.universe])
+        upload, ws_upload = self.makeBinaryPackageUpload(
+            person, component=self.universe)
+        with person_logged_in(person):
+            arch = upload.builds[0].build.arch_tag
+            bprs = upload.builds[0].build.binarypackages
+
+        self.assertFalse(ws_upload.contains_source)
+        self.assertTrue(ws_upload.contains_build)
+        ws_binaries = ws_upload.getBinaryProperties()
+        self.assertEqual(len(list(bprs)), len(ws_binaries))
+        for bpr, binary in zip(bprs, ws_binaries):
+            expected_binary = {
+                "is_new": True,
+                "name": bpr.name,
+                "version": bpr.version,
+                "architecture": arch,
+                "component": "universe",
+                "section": bpr.section.name,
+                "priority": bpr.priority.name,
+                }
+            self.assertContentEqual(expected_binary.keys(), binary.keys())
+            for key, value in expected_binary.items():
+                self.assertEqual(value, binary[key])
+
+    def test_binary_fetch(self):
+        # API clients can fetch files attached to binary uploads.
+        person = self.makeQueueAdmin([self.universe])
+        upload, ws_upload = self.makeBinaryPackageUpload(
+            person, component=self.universe)
+
+        ws_binary_file_urls = ws_upload.binaryFileUrls()
+        self.assertNotEqual(0, len(ws_binary_file_urls))
+        with person_logged_in(person):
+            binary_file_urls = [
+                ProxiedLibraryFileAlias(
+                    file.libraryfile, upload.archive).http_url
+                for bpr in upload.builds[0].build.binarypackages
+                for file in bpr.files]
+        self.assertContentEqual(binary_file_urls, ws_binary_file_urls)
+
+    def test_custom_info(self):
+        # API clients can inspect properties of custom uploads.
+        person = self.makeQueueAdmin([self.universe])
+        upload, ws_upload = self.makeCustomPackageUpload(
+            person, custom_type=PackageUploadCustomFormat.DEBIAN_INSTALLER,
+            filename="debian-installer-images_1.tar.gz")
+
+        self.assertFalse(ws_upload.contains_source)
+        self.assertFalse(ws_upload.contains_build)
+        self.assertFalse(ws_upload.contains_copy)
+        self.assertEqual(
+            "debian-installer-images_1.tar.gz", ws_upload.display_name)
+        self.assertEqual("-", ws_upload.display_version)
+        self.assertEqual("raw-installer", ws_upload.display_arches)
+
+    def test_custom_fetch(self):
+        # API clients can fetch files attached to custom uploads.
+        person = self.makeQueueAdmin([self.universe])
+        upload, ws_upload = self.makeCustomPackageUpload(
+            person, custom_type=PackageUploadCustomFormat.DEBIAN_INSTALLER,
+            filename="debian-installer-images_1.tar.gz")
+        ws_custom_file_urls = ws_upload.customFileUrls()
+        self.assertNotEqual(0, len(ws_custom_file_urls))
+        with person_logged_in(person):
+            custom_file_urls = [
+                ProxiedLibraryFileAlias(
+                    file.libraryfilealias, upload.archive).http_url
+                for file in upload.customfiles]
+        self.assertContentEqual(custom_file_urls, ws_custom_file_urls)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-06-22 17:26:53 +0000
+++ lib/lp/testing/factory.py	2012-07-03 12:23:23 +0000
@@ -3495,7 +3495,7 @@
 
     def makeBuildPackageUpload(self, distroseries=None, pocket=None,
                                binarypackagename=None,
-                               source_package_release=None):
+                               source_package_release=None, component=None):
         """Make a `PackageUpload` with a `PackageUploadBuild` attached."""
         if distroseries is None:
             distroseries = self.makeDistroSeries()
@@ -3506,7 +3506,8 @@
             source_package_release=source_package_release, pocket=pocket)
         upload.addBuild(build)
         self.makeBinaryPackageRelease(
-            binarypackagename=binarypackagename, build=build)
+            binarypackagename=binarypackagename, build=build,
+            component=component)
         return upload
 
     def makeCustomPackageUpload(self, distroseries=None, pocket=None,

=== modified file 'lib/lp/testing/tests/test_factory.py'
--- lib/lp/testing/tests/test_factory.py	2012-06-22 17:26:53 +0000
+++ lib/lp/testing/tests/test_factory.py	2012-07-03 12:23:23 +0000
@@ -780,16 +780,21 @@
     def test_makeBuildPackageUpload_passes_on_args(self):
         distroseries = self.factory.makeDistroSeries()
         bpn = self.factory.makeBinaryPackageName()
+        spr = self.factory.makeSourcePackageRelease()
+        component = self.factory.makeComponent()
         pu = self.factory.makeBuildPackageUpload(
             distroseries=distroseries, pocket=PackagePublishingPocket.PROPOSED,
-            binarypackagename=bpn)
+            binarypackagename=bpn, source_package_release=spr,
+            component=component)
         build = list(pu.builds)[0].build
         self.assertEqual(distroseries, pu.distroseries)
         self.assertEqual(distroseries.distribution, pu.archive.distribution)
         self.assertEqual(PackagePublishingPocket.PROPOSED, pu.pocket)
+        self.assertEqual(spr, build.source_package_release)
         release = IStore(distroseries).find(
             BinaryPackageRelease, BinaryPackageRelease.build == build).one()
         self.assertEqual(bpn, release.binarypackagename)
+        self.assertEqual(component, release.component)
 
     # makeCustomPackageUpload
     def test_makeCustomPackageUpload_makes_proxied_IPackageUpload(self):


Follow ups