launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19709
[Merge] lp:~cjwatson/launchpad/sourcefileurls-include-meta into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/sourcefileurls-include-meta into lp:launchpad.
Commit message:
Add include_meta option to SPPH.sourceFileUrls, paralleling BPPH.binaryFileUrls.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/sourcefileurls-include-meta/+merge/276928
Add include_meta option to SPPH.sourceFileUrls, paralleling BPPH.binaryFileUrls.
This is mainly to support dgit, which needs to be able to fetch .dsc files from Launchpad and verify their contents without having to fetch a full Sources file (even if one were available for the .dsc in question) and its trust chain.
The include_meta option is rather weird, in that it causes the method in question to return a completely different type, and had I been doing this from scratch I would probably have added a new getSourceFiles method instead and deprecated sourceFileUrls over time. However, BPPH.binaryFileUrls(include_meta=True) already existed (https://code.launchpad.net/~michael.nelson/launchpad/include-binary-size/+merge/150831), and other things being equal I prefer new interfaces to look like existing ones. I tried to make the implementations a little more in line with each other as well.
While I was here, I also added sha256 to BPPH.binaryFileUrls(include_meta=True), to drag things a little further into the modern world. I didn't see any point in exporting sha1 in the new SPPH.sourceFileUrls(include_meta=True) interface.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/sourcefileurls-include-meta into lp:launchpad.
=== modified file 'lib/lp/soyuz/browser/tests/test_publishing_webservice.py'
--- lib/lp/soyuz/browser/tests/test_publishing_webservice.py 2015-09-29 01:38:34 +0000
+++ lib/lp/soyuz/browser/tests/test_publishing_webservice.py 2015-11-08 01:32:11 +0000
@@ -1,22 +1,86 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test webservice methods related to the publisher."""
-from testtools.matchers import IsInstance
+from functools import partial
-from lp.services.database.sqlbase import flush_database_caches
+from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.webapp.interfaces import OAuthPermission
from lp.testing import (
api_url,
+ login_person,
person_logged_in,
- RequestTimelineCollector,
+ record_two_runs,
TestCaseWithFactory,
)
from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.matchers import HasQueryCount
from lp.testing.pages import webservice_for_person
+class SourcePackagePublishingHistoryWebserviceTests(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def make_spph_for(self, person):
+ with person_logged_in(person):
+ spr = self.factory.makeSourcePackageRelease()
+ self.factory.makeSourcePackageReleaseFile(sourcepackagerelease=spr)
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagerelease=spr)
+ return spph, api_url(spph)
+
+ def test_sourceFileUrls(self):
+ person = self.factory.makePerson()
+ webservice = webservice_for_person(
+ person, permission=OAuthPermission.READ_PUBLIC)
+ spph, url = self.make_spph_for(person)
+
+ response = webservice.named_get(
+ url, 'sourceFileUrls', api_version='devel')
+
+ self.assertEqual(200, response.status)
+ urls = response.jsonBody()
+ with person_logged_in(person):
+ sprf = spph.sourcepackagerelease.files[0]
+ expected_urls = [
+ ProxiedLibraryFileAlias(
+ sprf.libraryfile, spph.archive).http_url]
+ self.assertEqual(expected_urls, urls)
+
+ def test_sourceFileUrls_include_meta(self):
+ person = self.factory.makePerson()
+ webservice = webservice_for_person(
+ person, permission=OAuthPermission.READ_PUBLIC)
+ spph, url = self.make_spph_for(person)
+
+ def create_file():
+ self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=spph.sourcepackagerelease)
+
+ def get_urls():
+ return webservice.named_get(
+ url, 'sourceFileUrls', include_meta=True, api_version='devel')
+
+ recorder1, recorder2 = record_two_runs(
+ get_urls, create_file, 2,
+ login_method=partial(login_person, person), record_request=True)
+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
+ response = get_urls()
+ self.assertEqual(200, response.status)
+ info = response.jsonBody()
+ with person_logged_in(person):
+ expected_info = [{
+ "url": ProxiedLibraryFileAlias(
+ sprf.libraryfile, spph.archive).http_url,
+ "size": sprf.libraryfile.content.filesize,
+ "sha256": sprf.libraryfile.content.sha256,
+ } for sprf in spph.sourcepackagerelease.files]
+ self.assertContentEqual(expected_info, info)
+
+
class BinaryPackagePublishingHistoryWebserviceTests(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
@@ -33,36 +97,48 @@
person = self.factory.makePerson()
webservice = webservice_for_person(
person, permission=OAuthPermission.READ_PUBLIC)
+ bpph, url = self.make_bpph_for(person)
response = webservice.named_get(
- self.make_bpph_for(person)[1], 'binaryFileUrls',
- api_version='devel')
+ url, 'binaryFileUrls', api_version='devel')
self.assertEqual(200, response.status)
urls = response.jsonBody()
- self.assertEqual(1, len(urls))
- self.assertTrue(urls[0], IsInstance(unicode))
+ with person_logged_in(person):
+ bpf = bpph.binarypackagerelease.files[0]
+ expected_urls = [
+ ProxiedLibraryFileAlias(
+ bpf.libraryfile, bpph.archive).http_url]
+ self.assertEqual(expected_urls, urls)
def test_binaryFileUrls_include_meta(self):
person = self.factory.makePerson()
webservice = webservice_for_person(
person, permission=OAuthPermission.READ_PUBLIC)
-
bpph, url = self.make_bpph_for(person)
- query_counts = []
- for i in range(3):
- flush_database_caches()
- with RequestTimelineCollector() as collector:
- response = webservice.named_get(
- url, 'binaryFileUrls', include_meta=True,
- api_version='devel')
- query_counts.append(collector.count)
- with person_logged_in(person):
- self.factory.makeBinaryPackageFile(
- binarypackagerelease=bpph.binarypackagerelease)
- self.assertEqual(query_counts[0] - 1, query_counts[-1])
-
+
+ def create_file():
+ self.factory.makeBinaryPackageFile(
+ binarypackagerelease=bpph.binarypackagerelease)
+
+ def get_urls():
+ return webservice.named_get(
+ url, 'binaryFileUrls', include_meta=True, api_version='devel')
+
+ recorder1, recorder2 = record_two_runs(
+ get_urls, create_file, 2,
+ login_method=partial(login_person, person), record_request=True)
+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
+ response = get_urls()
self.assertEqual(200, response.status)
- urls = response.jsonBody()
- self.assertEqual(3, len(urls))
- self.assertThat(urls[0], IsInstance(dict))
+ info = response.jsonBody()
+ with person_logged_in(person):
+ expected_info = [{
+ "url": ProxiedLibraryFileAlias(
+ bpf.libraryfile, bpph.archive).http_url,
+ "size": bpf.libraryfile.content.filesize,
+ "sha1": bpf.libraryfile.content.sha1,
+ "sha256": bpf.libraryfile.content.sha256,
+ } for bpf in bpph.binarypackagerelease.files]
+ self.assertContentEqual(expected_info, info)
=== modified file 'lib/lp/soyuz/interfaces/publishing.py'
--- lib/lp/soyuz/interfaces/publishing.py 2015-04-08 10:35:22 +0000
+++ lib/lp/soyuz/interfaces/publishing.py 2015-11-08 01:32:11 +0000
@@ -578,9 +578,13 @@
"""
@export_read_operation()
- def sourceFileUrls():
+ @operation_parameters(
+ include_meta=Bool(title=_("Include Metadata"), required=False))
+ def sourceFileUrls(include_meta=False):
"""URLs for this source publication's uploaded source files.
+ :param include_meta: Return a list of dicts with keys url, size, and
+ sha256 for each URL instead of a simple list.
:return: A collection of URLs for this source.
"""
@@ -869,8 +873,8 @@
def binaryFileUrls(include_meta=False):
"""URLs for this binary publication's binary files.
- :param include_meta: Return a list of dicts with keys url, size
- and sha1 for each url instead of a simple list.
+ :param include_meta: Return a list of dicts with keys url, size,
+ sha1, and sha256 for each URL instead of a simple list.
:return: A collection of URLs for this binary.
"""
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2015-07-08 16:05:11 +0000
+++ lib/lp/soyuz/model/publishing.py 2015-11-08 01:32:11 +0000
@@ -643,11 +643,23 @@
return getUtility(
IPublishingSet).getBuildStatusSummaryForSourcePublication(self)
- def sourceFileUrls(self):
+ def sourceFileUrls(self, include_meta=False):
"""See `ISourcePackagePublishingHistory`."""
+ sources = Store.of(self).find(
+ (LibraryFileAlias, LibraryFileContent),
+ LibraryFileContent.id == LibraryFileAlias.contentID,
+ LibraryFileAlias.id == SourcePackageReleaseFile.libraryfileID,
+ SourcePackageReleaseFile.sourcepackagerelease ==
+ SourcePackageRelease.id,
+ SourcePackageRelease.id == self.sourcepackagereleaseID)
source_urls = proxied_urls(
- [file.libraryfile for file in self.sourcepackagerelease.files],
- self.archive)
+ [source for source, _ in sources], self.archive)
+ if include_meta:
+ meta = [
+ (content.filesize, content.sha256) for _, content in sources]
+ return [
+ dict(url=url, size=size, sha256=sha256)
+ for url, (size, sha256) in zip(source_urls, meta)]
return source_urls
def binaryFileUrls(self):
@@ -1064,9 +1076,10 @@
[binary for binary, _ in binaries], self.archive)
if include_meta:
meta = [
- (content.filesize, content.sha1) for _, content in binaries]
- return [dict(url=url, size=size, sha1=sha1)
- for url, (size, sha1) in zip(binary_urls, meta)]
+ (content.filesize, content.sha1, content.sha256)
+ for _, content in binaries]
+ return [dict(url=url, size=size, sha1=sha1, sha256=sha256)
+ for url, (size, sha1, sha256) in zip(binary_urls, meta)]
return binary_urls
=== modified file 'lib/lp/soyuz/tests/test_publishing_models.py'
--- lib/lp/soyuz/tests/test_publishing_models.py 2015-05-14 13:23:47 +0000
+++ lib/lp/soyuz/tests/test_publishing_models.py 2015-11-08 01:32:11 +0000
@@ -8,6 +8,7 @@
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import BuildStatus
+from lp.registry.interfaces.sourcepackage import SourcePackageFileType
from lp.services.database.constants import UTC_NOW
from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.webapp.publisher import canonical_url
@@ -154,12 +155,68 @@
spph = self.factory.makeSourcePackagePublishingHistory()
self.assertRaises(NotFoundError, spph.getFileByName, 'not-changelog')
+ def getURLsForSPPH(self, spph, include_meta=False):
+ spr = spph.sourcepackagerelease
+ archive = spph.archive
+ urls = [ProxiedLibraryFileAlias(f.libraryfile, archive).http_url
+ for f in spr.files]
+
+ if include_meta:
+ meta = [(
+ f.libraryfile.content.filesize,
+ f.libraryfile.content.sha256,
+ ) for f in spr.files]
+
+ return [dict(url=url, size=size, sha256=sha256)
+ for url, (size, sha256) in zip(urls, meta)]
+ return urls
+
+ def makeSPPH(self, num_files=1):
+ archive = self.factory.makeArchive(private=False)
+ spr = self.factory.makeSourcePackageRelease(archive=archive)
+ filetypes = [
+ SourcePackageFileType.DSC, SourcePackageFileType.ORIG_TARBALL]
+ for count in range(num_files):
+ self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=spr, filetype=filetypes[count % 2])
+ return self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagerelease=spr, archive=archive)
+
+ def test_sourceFileUrls_no_files(self):
+ spph = self.makeSPPH(num_files=0)
+
+ urls = spph.sourceFileUrls()
+
+ self.assertContentEqual([], urls)
+
+ def test_sourceFileUrls_one_file(self):
+ spph = self.makeSPPH(num_files=1)
+
+ urls = spph.sourceFileUrls()
+
+ self.assertContentEqual(self.getURLsForSPPH(spph), urls)
+
+ def test_sourceFileUrls_two_files(self):
+ spph = self.makeSPPH(num_files=2)
+
+ urls = spph.sourceFileUrls()
+
+ self.assertContentEqual(self.getURLsForSPPH(spph), urls)
+
+ def test_sourceFileUrls_include_meta(self):
+ spph = self.makeSPPH(num_files=2)
+
+ urls = spph.sourceFileUrls(include_meta=True)
+
+ self.assertContentEqual(
+ self.getURLsForSPPH(spph, include_meta=True), urls)
+
class TestBinaryPackagePublishingHistory(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
- def get_urls_for_bpph(self, bpph, include_meta=False):
+ def getURLsForBPPH(self, bpph, include_meta=False):
bpr = bpph.binarypackagerelease
archive = bpph.archive
urls = [ProxiedLibraryFileAlias(f.libraryfile, archive).http_url
@@ -169,13 +226,14 @@
meta = [(
f.libraryfile.content.filesize,
f.libraryfile.content.sha1,
+ f.libraryfile.content.sha256,
) for f in bpr.files]
- return [dict(url=url, size=size, sha1=sha1)
- for url, (size, sha1) in zip(urls, meta)]
+ return [dict(url=url, size=size, sha1=sha1, sha256=sha256)
+ for url, (size, sha1, sha256) in zip(urls, meta)]
return urls
- def make_bpph(self, num_binaries=1):
+ def makeBPPH(self, num_binaries=1):
archive = self.factory.makeArchive(private=False)
bpr = self.factory.makeBinaryPackageRelease()
filetypes = [BinaryPackageFileType.DEB, BinaryPackageFileType.DDEB]
@@ -186,40 +244,40 @@
binarypackagerelease=bpr, archive=archive)
def test_binaryFileUrls_no_binaries(self):
- bpph = self.make_bpph(num_binaries=0)
+ bpph = self.makeBPPH(num_binaries=0)
urls = bpph.binaryFileUrls()
self.assertContentEqual([], urls)
def test_binaryFileUrls_one_binary(self):
- bpph = self.make_bpph(num_binaries=1)
+ bpph = self.makeBPPH(num_binaries=1)
urls = bpph.binaryFileUrls()
- self.assertContentEqual(self.get_urls_for_bpph(bpph), urls)
+ self.assertContentEqual(self.getURLsForBPPH(bpph), urls)
def test_binaryFileUrls_two_binaries(self):
- bpph = self.make_bpph(num_binaries=2)
+ bpph = self.makeBPPH(num_binaries=2)
urls = bpph.binaryFileUrls()
- self.assertContentEqual(self.get_urls_for_bpph(bpph), urls)
+ self.assertContentEqual(self.getURLsForBPPH(bpph), urls)
def test_binaryFileUrls_include_meta(self):
- bpph = self.make_bpph(num_binaries=2)
+ bpph = self.makeBPPH(num_binaries=2)
urls = bpph.binaryFileUrls(include_meta=True)
self.assertContentEqual(
- self.get_urls_for_bpph(bpph, include_meta=True), urls)
+ self.getURLsForBPPH(bpph, include_meta=True), urls)
def test_binaryFileUrls_removed(self):
# binaryFileUrls returns URLs even if the files have been removed
# from the published archive.
- bpph = self.make_bpph(num_binaries=2)
- expected_urls = self.get_urls_for_bpph(bpph)
- expected_urls_meta = self.get_urls_for_bpph(bpph, include_meta=True)
+ bpph = self.makeBPPH(num_binaries=2)
+ expected_urls = self.getURLsForBPPH(bpph)
+ expected_urls_meta = self.getURLsForBPPH(bpph, include_meta=True)
self.assertContentEqual(expected_urls, bpph.binaryFileUrls())
self.assertContentEqual(
expected_urls_meta, bpph.binaryFileUrls(include_meta=True))
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2015-10-04 01:28:19 +0000
+++ lib/lp/testing/__init__.py 2015-11-08 01:32:11 +0000
@@ -425,7 +425,8 @@
def record_two_runs(tested_method, item_creator, first_round_number,
- second_round_number=None, login_method=None):
+ second_round_number=None, login_method=None,
+ record_request=False):
"""A helper that returns the two storm statement recorders
obtained when running tested_method after having run the
method {item_creator} {first_round_number} times and then
@@ -435,9 +436,17 @@
If {login_method} is not None, it is called before each batch of
{item_creator} calls.
+ If {record_request} is True, `RequestTimelineCollector` is used to get
+ the query counts, so {tested_method} should make a web request.
+ Otherwise, `StormStatementRecorder` is used to get the query count.
+
:return: a tuple containing the two recorders obtained by the successive
runs.
"""
+ if record_request:
+ recorder_factory = RequestTimelineCollector
+ else:
+ recorder_factory = StormStatementRecorder
if login_method is not None:
login_method()
for i in range(first_round_number):
@@ -449,7 +458,7 @@
if queryInteraction() is not None:
clear_permission_cache()
getUtility(ILaunchpadCelebrities).clearCache()
- with StormStatementRecorder() as recorder1:
+ with recorder_factory() as recorder1:
tested_method()
# Run {item_creator} {second_round_number} more times.
if second_round_number is None:
@@ -463,7 +472,7 @@
if queryInteraction() is not None:
clear_permission_cache()
getUtility(ILaunchpadCelebrities).clearCache()
- with StormStatementRecorder() as recorder2:
+ with recorder_factory() as recorder2:
tested_method()
return recorder1, recorder2
Follow ups
