launchpad-reviewers team mailing list archive

[James Westby <jw+debian@xxxxxxxxxxxxxxx>]

James Westby has proposed merging lp:~james-w/launchpad/bpph-binary-file-urls into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #881509 in Launchpad itself: "Unable to get to the actual files related to a binaryPackagePublishingHistory"
  https://bugs.launchpad.net/launchpad/+bug/881509

For more details, see:
https://code.launchpad.net/~james-w/launchpad/bpph-binary-file-urls/+merge/86470

Hi,

This branch adds a binaryFileUrls method to BinaryPackagePublishingHistory to
parallel the one on SourcePackagePublishingHistory. You can now get the actual
files that you get after looking up some binaries.

Thanks,

James

-- 
https://code.launchpad.net/~james-w/launchpad/bpph-binary-file-urls/+merge/86470
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~james-w/launchpad/bpph-binary-file-urls into lp:launchpad.

=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py	2011-10-23 02:58:56 +0000
+++ lib/lp/soyuz/interfaces/publishing.py	2011-12-20 22:04:24 +0000
@@ -901,6 +901,14 @@
         :param end_date: The optional last date to return.
         """
 
+    @export_read_operation()
+    @operation_for_version("devel")
+    def binaryFileUrls():
+        """URLs for this binary publication's binary files.
+
+        :return: A collection of URLs for this binary.
+        """
+
 
 class IBinaryPackagePublishingHistory(IBinaryPackagePublishingHistoryPublic,
                                       IPublishingEdit):

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-12-19 23:38:16 +0000
+++ lib/lp/soyuz/model/publishing.py	2011-12-20 22:04:24 +0000
@@ -152,6 +152,12 @@
     return archive
 
 
+def proxied_urls(files, parent):
+    """Run the files passed through `ProxiedLibraryFileAlias`."""
+    return [
+        ProxiedLibraryFileAlias(file, parent).http_url for file in files]
+
+
 class FilePublishingBase:
     """Base class to publish files in the archive."""
 
@@ -577,14 +583,14 @@
         # Return a webapp-proxied LibraryFileAlias so that restricted
         # librarian files are accessible.  Non-restricted files will get
         # a 302 so that webapp threads are not tied up.
-        the_url = self._proxied_urls((changes_lfa,), self.archive)[0]
+        the_url = proxied_urls((changes_lfa,), self.archive)[0]
         return the_url
 
     def changelogUrl(self):
         """See `ISourcePackagePublishingHistory`."""
         lfa = self.sourcepackagerelease.changelog
         if lfa is not None:
-            return self._proxied_urls((lfa,), self)[0]
+            return proxied_urls((lfa,), self)[0]
         return None
 
     def _getAllowedArchitectures(self, available_archs):
@@ -870,14 +876,9 @@
         assert self.component in (
             self.archive.getComponentsForSeries(self.distroseries))
 
-    def _proxied_urls(self, files, parent):
-        """Run the files passed through `ProxiedLibraryFileAlias`."""
-        return [
-            ProxiedLibraryFileAlias(file, parent).http_url for file in files]
-
     def sourceFileUrls(self):
         """See `ISourcePackagePublishingHistory`."""
-        source_urls = self._proxied_urls(
+        source_urls = proxied_urls(
             [file.libraryfile for file in self.sourcepackagerelease.files],
              self.archive)
         return source_urls
@@ -887,7 +888,7 @@
         publishing_set = getUtility(IPublishingSet)
         binaries = publishing_set.getBinaryFilesForSources(
             self).config(distinct=True)
-        binary_urls = self._proxied_urls(
+        binary_urls = proxied_urls(
             [binary for _source, binary, _content in binaries], self.archive)
         return binary_urls
 
@@ -1322,6 +1323,12 @@
         """See `IPublishing`."""
         self.setDeleted(removed_by, removal_comment)
 
+    def binaryFileUrls(self):
+        """See `IBinaryPackagePublishingHistory`."""
+        binary_urls = proxied_urls(
+            [f.libraryfilealias for f in self.files], self.archive)
+        return binary_urls
+
 
 def expand_binary_requests(distroseries, binaries):
     """Architecture-expand a dict of binary publication requests.

=== modified file 'lib/lp/soyuz/tests/test_publishing_models.py'
--- lib/lp/soyuz/tests/test_publishing_models.py	2011-10-23 03:12:50 +0000
+++ lib/lp/soyuz/tests/test_publishing_models.py	2011-12-20 22:04:24 +0000
@@ -7,6 +7,8 @@
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.database.constants import UTC_NOW
+from canonical.launchpad.testing.pages import webservice_for_person
+from canonical.launchpad.webapp.interfaces import OAuthPermission
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.testing.layers import (
     LaunchpadFunctionalLayer,
@@ -18,8 +20,13 @@
     IPublishingSet,
     PackagePublishingStatus,
     )
+from lp.soyuz.enums import BinaryPackageFileType
 from lp.soyuz.tests.test_binarypackagebuild import BaseTestCaseWithThreeBuilds
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    api_url,
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 
 
 class TestPublishingSet(BaseTestCaseWithThreeBuilds):
@@ -136,3 +143,52 @@
     def test_getFileByName_unhandled_name(self):
         spph = self.factory.makeSourcePackagePublishingHistory()
         self.assertRaises(NotFoundError, spph.getFileByName, 'not-changelog')
+
+
+class TestBinaryPackagePublishingHistory(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def test_binaryFileUrls_no_binaries(self):
+        bpr = self.factory.makeBinaryPackageRelease()
+        bpph = self.factory.makeBinaryPackagePublishingHistory(
+            binarypackagerelease=bpr)
+        self.assertEqual(0, len(bpph.binaryFileUrls()))
+
+    def test_binaryFileUrls_one_binary(self):
+        bpr = self.factory.makeBinaryPackageRelease()
+        self.factory.makeBinaryPackageFile(binarypackagerelease=bpr)
+        bpph = self.factory.makeBinaryPackagePublishingHistory(
+            binarypackagerelease=bpr)
+        self.assertEqual(1, len(bpph.binaryFileUrls()))
+
+    def test_binaryFileUrls_two_binaries(self):
+        bpr = self.factory.makeBinaryPackageRelease()
+        self.factory.makeBinaryPackageFile(
+            binarypackagerelease=bpr, filetype=BinaryPackageFileType.DEB)
+        self.factory.makeBinaryPackageFile(
+            binarypackagerelease=bpr, filetype=BinaryPackageFileType.DDEB)
+        bpph = self.factory.makeBinaryPackagePublishingHistory(
+            binarypackagerelease=bpr)
+        self.assertEqual(2, len(bpph.binaryFileUrls()))
+
+
+class BinaryPackagePublishingHistoryWebserviceTests(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def test_binaryFileUrls(self):
+        bpr = self.factory.makeBinaryPackageRelease()
+        self.factory.makeBinaryPackageFile(binarypackagerelease=bpr)
+        bpph = self.factory.makeBinaryPackagePublishingHistory(
+            binarypackagerelease=bpr)
+        person = self.factory.makePerson()
+        webservice = webservice_for_person(
+            person, permission=OAuthPermission.READ_PUBLIC)
+        with person_logged_in(person):
+            bpph_url = api_url(bpph)
+        response = webservice.named_get(
+            bpph_url, 'binaryFileUrls', api_version='devel')
+        self.assertEqual(200, response.status)
+        urls = response.jsonBody()
+        self.assertEqual(1, len(urls))