Merge lp:~cjwatson/launchpad/better-bzr-mp-diffs into lp:launchpad

Proposed by Colin Watson on 2018-05-16
Status: Merged
Merged at revision: 18707
Proposed branch: lp:~cjwatson/launchpad/better-bzr-mp-diffs
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/branch-hosting-client
Diff against target: 506 lines (+230/-34)
11 files modified
lib/lp/code/browser/branch.py (+48/-0)
lib/lp/code/browser/branchmergeproposal.py (+9/-12)
lib/lp/code/browser/configure.zcml (+5/-1)
lib/lp/code/browser/gitrepository.py (+12/-10)
lib/lp/code/browser/tests/test_branch.py (+72/-0)
lib/lp/code/browser/tests/test_branchmergeproposal.py (+19/-2)
lib/lp/code/browser/tests/test_gitrepository.py (+18/-2)
lib/lp/code/interfaces/branch.py (+10/-1)
lib/lp/code/model/branch.py (+6/-0)
lib/lp/code/tests/helpers.py (+16/-1)
lib/lp/services/webapp/publisher.py (+15/-5)
To merge this branch: bzr merge lp:~cjwatson/launchpad/better-bzr-mp-diffs
Reviewer Review Type Date Requested Status
William Grant code 2018-05-16 Approve on 2018-06-21
Review via email: mp+345705@code.launchpad.net

Commit message

Proxy loggerhead branch diffs through the webapp, allowing AJAX MP revision diffs to work for private branches.

Description of the change

This is the same approach as we already use for Git, built on top of my recent work to expose a suitable loggerhead API on a private port.

To post a comment you must log in.
18665. By Colin Watson on 2018-05-16

Update copyright.

18666. By Colin Watson on 2018-05-16

Minor simplification.

18667. By Colin Watson on 2018-06-14

Merge branch-hosting-client.

William Grant (wgrant) :
review: Approve (code)
18668. By Colin Watson on 2018-06-21

Merge branch-hosting-client, adjusting Branch.getDiff.

18669. By Colin Watson on 2018-06-21

Add a feature flag to disable the Bazaar diff proxy view.

18670. By Colin Watson on 2018-06-21

Quote filename properly in Content-Disposition header.

18671. By Colin Watson on 2018-06-22

Remove unnecessary u"" in unicode_literals file.

18672. By Colin Watson on 2018-06-22

Add branch/repository name to filename in proxied diff views.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py 2018-03-16 21:20:00 +0000
+++ lib/lp/code/browser/branch.py 2018-06-22 10:03:28 +0000
@@ -46,6 +46,7 @@
46 providedBy,46 providedBy,
47 )47 )
48from zope.publisher.interfaces import NotFound48from zope.publisher.interfaces import NotFound
49from zope.publisher.interfaces.browser import IBrowserPublisher
49from zope.schema import (50from zope.schema import (
50 Bool,51 Bool,
51 Choice,52 Choice,
@@ -136,6 +137,7 @@
136from lp.services.webapp.breadcrumb import NameBreadcrumb137from lp.services.webapp.breadcrumb import NameBreadcrumb
137from lp.services.webapp.escaping import structured138from lp.services.webapp.escaping import structured
138from lp.services.webapp.interfaces import ICanonicalUrlData139from lp.services.webapp.interfaces import ICanonicalUrlData
140from lp.services.webapp.publisher import DataDownloadView
139from lp.services.webhooks.browser import WebhookTargetNavigationMixin141from lp.services.webhooks.browser import WebhookTargetNavigationMixin
140from lp.snappy.browser.hassnaps import (142from lp.snappy.browser.hassnaps import (
141 HasSnapsMenuMixin,143 HasSnapsMenuMixin,
@@ -203,6 +205,22 @@
203 return None205 return None
204 return self.context.getMergeProposalByID(id)206 return self.context.getMergeProposalByID(id)
205207
208 @stepto("+diff")
209 def traverse_diff(self):
210 segments = list(self.request.getTraversalStack())
211 if len(segments) == 1:
212 new = segments.pop()
213 old = None
214 self.request.stepstogo.consume()
215 elif len(segments) == 2:
216 new = segments.pop()
217 old = segments.pop()
218 self.request.stepstogo.consume()
219 self.request.stepstogo.consume()
220 else:
221 return None
222 return BranchDiffView(self.context, self.request, new, old=old)
223
206 @stepto("+code-import")224 @stepto("+code-import")
207 def traverse_code_import(self):225 def traverse_code_import(self):
208 """Traverses to the `ICodeImport` for the branch."""226 """Traverses to the `ICodeImport` for the branch."""
@@ -1280,3 +1298,33 @@
1280 'target_branch',1298 'target_branch',
1281 "This branch is not mergeable into %s." %1299 "This branch is not mergeable into %s." %
1282 target_branch.bzr_identity)1300 target_branch.bzr_identity)
1301
1302
1303@implementer(IBrowserPublisher)
1304class BranchDiffView(DataDownloadView):
1305
1306 content_type = "text/x-patch"
1307
1308 def __init__(self, context, request, new, old=None):
1309 super(BranchDiffView, self).__init__(context, request)
1310 self.new = new
1311 self.old = old
1312
1313 def __call__(self):
1314 if getFeatureFlag(u"code.bzr.diff.disable_proxy"):
1315 self.request.response.setStatus(401)
1316 return "Proxying of branch diffs is disabled.\n"
1317 return super(BranchDiffView, self).__call__()
1318
1319 @property
1320 def filename(self):
1321 if self.old is None:
1322 return "%s_%s.diff" % (self.context.name, self.new)
1323 else:
1324 return "%s_%s_%s.diff" % (self.context.name, self.old, self.new)
1325
1326 def getBody(self):
1327 return self.context.getDiff(self.new, old=self.old)
1328
1329 def browserDefault(self, request):
1330 return self, ()
12831331
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2018-04-25 12:01:33 +0000
+++ lib/lp/code/browser/branchmergeproposal.py 2018-06-22 10:03:28 +0000
@@ -617,19 +617,16 @@
617 super(BranchMergeProposalView, self).initialize()617 super(BranchMergeProposalView, self).initialize()
618 cache = IJSONRequestCache(self.request)618 cache = IJSONRequestCache(self.request)
619 cache.objects['branch_name'] = self.context.merge_source.name619 cache.objects['branch_name'] = self.context.merge_source.name
620 if IBranch.providedBy(self.context.merge_source):620 if (IBranch.providedBy(self.context.merge_source) and
621 cache.objects.update({621 getFeatureFlag("code.bzr.diff.disable_proxy")):
622 'branch_diff_link':622 # This fallback works for public branches, but not private ones.
623 'https://%s/+loggerhead/%s/diff/' % (623 cache.objects['branch_diff_link'] = (
624 config.launchpad.code_domain,624 'https://%s/+loggerhead/%s/diff/' % (
625 self.context.source_branch.unique_name),625 config.launchpad.code_domain,
626 })626 self.context.source_branch.unique_name))
627 else:627 else:
628 cache.objects.update({628 cache.objects['branch_diff_link'] = (
629 'branch_diff_link':629 canonical_url(self.context.parent) + '/+diff/')
630 canonical_url(self.context.source_git_repository) +
631 '/+diff/',
632 })
633 if getFeatureFlag("longpoll.merge_proposals.enabled"):630 if getFeatureFlag("longpoll.merge_proposals.enabled"):
634 cache.objects['merge_proposal_event_key'] = subscribe(631 cache.objects['merge_proposal_event_key'] = subscribe(
635 self.context).event_key632 self.context).event_key
636633
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2017-09-27 13:50:02 +0000
+++ lib/lp/code/browser/configure.zcml 2018-06-22 10:03:28 +0000
@@ -1,4 +1,4 @@
1<!-- Copyright 2009-2017 Canonical Ltd. This software is licensed under the1<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
2 GNU Affero General Public License version 3 (see the file LICENSE).2 GNU Affero General Public License version 3 (see the file LICENSE).
3-->3-->
44
@@ -442,6 +442,10 @@
442 class="lp.code.browser.codeimport.CodeImportEditView"442 class="lp.code.browser.codeimport.CodeImportEditView"
443 permission="launchpad.Edit"443 permission="launchpad.Edit"
444 template="../../app/templates/generic-edit.pt"/>444 template="../../app/templates/generic-edit.pt"/>
445 <class class="lp.code.browser.branch.BranchDiffView">
446 <allow interface="zope.publisher.interfaces.browser.IBrowserPublisher"
447 attributes="__call__"/>
448 </class>
445 <browser:page449 <browser:page
446 name="+delete"450 name="+delete"
447 for="lp.code.interfaces.branch.IBranch"451 for="lp.code.interfaces.branch.IBranch"
448452
=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py 2016-10-15 01:14:32 +0000
+++ lib/lp/code/browser/gitrepository.py 2018-06-22 10:03:28 +0000
@@ -97,6 +97,7 @@
97from lp.services.webapp.breadcrumb import Breadcrumb97from lp.services.webapp.breadcrumb import Breadcrumb
98from lp.services.webapp.escaping import structured98from lp.services.webapp.escaping import structured
99from lp.services.webapp.interfaces import ICanonicalUrlData99from lp.services.webapp.interfaces import ICanonicalUrlData
100from lp.services.webapp.publisher import DataDownloadView
100from lp.services.webhooks.browser import WebhookTargetNavigationMixin101from lp.services.webhooks.browser import WebhookTargetNavigationMixin
101from lp.snappy.browser.hassnaps import HasSnapsViewMixin102from lp.snappy.browser.hassnaps import HasSnapsViewMixin
102103
@@ -638,23 +639,24 @@
638639
639640
640@implementer(IBrowserPublisher)641@implementer(IBrowserPublisher)
641class GitRepositoryDiffView:642class GitRepositoryDiffView(DataDownloadView):
643
644 content_type = "text/x-patch"
645 charset = "UTF-8"
642646
643 def __init__(self, context, request, old, new):647 def __init__(self, context, request, old, new):
648 super(GitRepositoryDiffView, self).__init__(context, request)
644 self.context = context649 self.context = context
645 self.request = request650 self.request = request
646 self.old = old651 self.old = old
647 self.new = new652 self.new = new
648653
649 def __call__(self):654 @property
650 content = self.context.getDiff(self.old, self.new)655 def filename(self):
651 filename = "%s_%s.diff" % (self.old, self.new)656 return "%s_%s_%s.diff" % (self.context.name, self.old, self.new)
652 self.request.response.setHeader(657
653 "Content-Type", 'text/x-patch; charset="UTF-8"')658 def getBody(self):
654 self.request.response.setHeader("Content-Length", str(len(content)))659 return self.context.getDiff(self.old, self.new)
655 self.request.response.setHeader(
656 "Content-Disposition", "attachment; filename=%s" % filename)
657 return content
658660
659 def browserDefault(self, request):661 def browserDefault(self, request):
660 return self, ()662 return self, ()
661663
=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py 2018-02-01 18:38:24 +0000
+++ lib/lp/code/browser/tests/test_branch.py 2018-06-22 10:03:28 +0000
@@ -12,10 +12,12 @@
1212
13from fixtures import FakeLogger13from fixtures import FakeLogger
14import pytz14import pytz
15from six.moves.urllib_error import HTTPError
15from storm.store import Store16from storm.store import Store
16from testtools.matchers import Equals17from testtools.matchers import Equals
17from zope.component import getUtility18from zope.component import getUtility
18from zope.publisher.interfaces import NotFound19from zope.publisher.interfaces import NotFound
20from zope.security.interfaces import Unauthorized
19from zope.security.proxy import removeSecurityProxy21from zope.security.proxy import removeSecurityProxy
2022
21from lp.app.enums import InformationType23from lp.app.enums import InformationType
@@ -32,12 +34,14 @@
32 RepositoryFormat,34 RepositoryFormat,
33 )35 )
34from lp.code.enums import BranchType36from lp.code.enums import BranchType
37from lp.code.tests.helpers import BranchHostingFixture
35from lp.registry.enums import BranchSharingPolicy38from lp.registry.enums import BranchSharingPolicy
36from lp.registry.interfaces.accesspolicy import IAccessPolicySource39from lp.registry.interfaces.accesspolicy import IAccessPolicySource
37from lp.registry.interfaces.person import PersonVisibility40from lp.registry.interfaces.person import PersonVisibility
38from lp.services.beautifulsoup import BeautifulSoup41from lp.services.beautifulsoup import BeautifulSoup
39from lp.services.config import config42from lp.services.config import config
40from lp.services.database.constants import UTC_NOW43from lp.services.database.constants import UTC_NOW
44from lp.services.features.testing import FeatureFixture
41from lp.services.helpers import truncate_text45from lp.services.helpers import truncate_text
42from lp.services.webapp.publisher import canonical_url46from lp.services.webapp.publisher import canonical_url
43from lp.services.webapp.servers import LaunchpadTestRequest47from lp.services.webapp.servers import LaunchpadTestRequest
@@ -1220,3 +1224,71 @@
1220 InformationType.USERDATA.description, description.renderContents())1224 InformationType.USERDATA.description, description.renderContents())
1221 self.assertIsNotNone(1225 self.assertIsNotNone(
1222 soup.find('a', id='privacy-link', attrs={'href': edit_url}))1226 soup.find('a', id='privacy-link', attrs={'href': edit_url}))
1227
1228
1229class TestBranchDiffView(BrowserTestCase):
1230
1231 layer = DatabaseFunctionalLayer
1232
1233 def test_feature_disabled(self):
1234 self.useFixture(FeatureFixture({"code.bzr.diff.disable_proxy": "on"}))
1235 hosting_fixture = self.useFixture(BranchHostingFixture())
1236 person = self.factory.makePerson()
1237 branch = self.factory.makeBranch(owner=person)
1238 e = self.assertRaises(
1239 HTTPError, self.getUserBrowser,
1240 canonical_url(branch) + "/+diff/2/1")
1241 self.assertEqual(401, e.code)
1242 self.assertEqual("Proxying of branch diffs is disabled.\n", e.read())
1243 self.assertEqual([], hosting_fixture.getDiff.calls)
1244
1245 def test_render(self):
1246 diff = b"A fake diff\n"
1247 hosting_fixture = self.useFixture(BranchHostingFixture(diff=diff))
1248 person = self.factory.makePerson()
1249 branch = self.factory.makeBranch(owner=person, name="some-branch")
1250 browser = self.getUserBrowser(
1251 canonical_url(branch) + "/+diff/2/1")
1252 with person_logged_in(person):
1253 self.assertEqual(
1254 [((branch.id, "2"), {"old": "1"})],
1255 hosting_fixture.getDiff.calls)
1256 self.assertEqual("text/x-patch", browser.headers["Content-Type"])
1257 self.assertEqual(str(len(diff)), browser.headers["Content-Length"])
1258 self.assertEqual(
1259 'attachment; filename="some-branch_1_2.diff"',
1260 browser.headers["Content-Disposition"])
1261 self.assertEqual(diff, browser.contents)
1262
1263 def test_security(self):
1264 # A user who can see a private branch can fetch diffs from it, but
1265 # other users cannot.
1266 diff = b"A fake diff\n"
1267 self.useFixture(BranchHostingFixture(diff=diff))
1268 person = self.factory.makePerson()
1269 project = self.factory.makeProduct(
1270 owner=person, information_type=InformationType.PROPRIETARY)
1271 with person_logged_in(person):
1272 branch = self.factory.makeBranch(
1273 owner=person, product=project,
1274 information_type=InformationType.PROPRIETARY)
1275 branch_url = canonical_url(branch)
1276 browser = self.getUserBrowser(branch_url + "/+diff/2/1", user=person)
1277 self.assertEqual(diff, browser.contents)
1278 self.useFixture(FakeLogger())
1279 self.assertRaises(
1280 Unauthorized, self.getUserBrowser, branch_url + "/+diff/2/1")
1281
1282 def test_filename_quoting(self):
1283 # If we construct revisions containing metacharacters and somehow
1284 # manage to get that past the hosting service, the Content-Disposition
1285 # header is quoted properly.
1286 diff = b"A fake diff\n"
1287 self.useFixture(BranchHostingFixture(diff=diff))
1288 person = self.factory.makePerson()
1289 branch = self.factory.makeBranch(owner=person, name="some-branch")
1290 browser = self.getUserBrowser(
1291 canonical_url(branch) + '/+diff/foo"/"bar')
1292 self.assertEqual(
1293 r'attachment; filename="some-branch_\"bar_foo\".diff"',
1294 browser.headers["Content-Disposition"])
12231295
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2018-04-25 12:01:33 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2018-06-22 10:03:28 +0000
@@ -89,6 +89,7 @@
89 PersonVisibility,89 PersonVisibility,
90 TeamMembershipPolicy,90 TeamMembershipPolicy,
91 )91 )
92from lp.services.features.testing import FeatureFixture
92from lp.services.librarian.interfaces.client import LibrarianServerError93from lp.services.librarian.interfaces.client import LibrarianServerError
93from lp.services.messages.model.message import MessageSet94from lp.services.messages.model.message import MessageSet
94from lp.services.timeout import TimeoutError95from lp.services.timeout import TimeoutError
@@ -1634,8 +1635,24 @@
1634 "on 2015-01-01" % sha1, extract_text(tag))1635 "on 2015-01-01" % sha1, extract_text(tag))
16351636
1636 def test_client_cache_bzr(self):1637 def test_client_cache_bzr(self):
1637 # For Bazaar, the client cache contains the branch name and a1638 # For Bazaar, the client cache contains the branch name and a link
1638 # loggerhead-based diff link.1639 # to the Loggerhead diff via the webapp.
1640 bmp = self.factory.makeBranchMergeProposal()
1641 view = create_initialized_view(bmp, '+index')
1642 cache = IJSONRequestCache(view.request)
1643 self.assertThat(cache.objects, ContainsDict({
1644 'branch_name': Equals(bmp.source_branch.name),
1645 'branch_diff_link': Equals(
1646 'http://code.launchpad.dev/%s/+diff/' %
1647 bmp.source_branch.unique_name),
1648 }))
1649
1650 def test_client_cache_bzr_proxy_feature_disabled(self):
1651 # For Bazaar, a feature flag can be set to cause the client cache to
1652 # contain a direct link to the Loggerhead diff rather than via the
1653 # webapp. (This only works for public branches.)
1654 self.useFixture(
1655 FeatureFixture({u'code.bzr.diff.disable_proxy': u'on'}))
1639 bmp = self.factory.makeBranchMergeProposal()1656 bmp = self.factory.makeBranchMergeProposal()
1640 view = create_initialized_view(bmp, '+index')1657 view = create_initialized_view(bmp, '+index')
1641 cache = IJSONRequestCache(view.request)1658 cache = IJSONRequestCache(view.request)
16421659
=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py 2018-02-01 18:38:24 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py 2018-06-22 10:03:28 +0000
@@ -875,7 +875,8 @@
875 hosting_fixture = self.useFixture(GitHostingFixture(875 hosting_fixture = self.useFixture(GitHostingFixture(
876 diff={"patch": diff}))876 diff={"patch": diff}))
877 person = self.factory.makePerson()877 person = self.factory.makePerson()
878 repository = self.factory.makeGitRepository(owner=person)878 repository = self.factory.makeGitRepository(
879 owner=person, name="some-repository")
879 browser = self.getUserBrowser(880 browser = self.getUserBrowser(
880 canonical_url(repository) + "/+diff/0123456/0123456^")881 canonical_url(repository) + "/+diff/0123456/0123456^")
881 with person_logged_in(person):882 with person_logged_in(person):
@@ -886,7 +887,7 @@
886 'text/x-patch;charset=UTF-8', browser.headers["Content-Type"])887 'text/x-patch;charset=UTF-8', browser.headers["Content-Type"])
887 self.assertEqual(str(len(diff)), browser.headers["Content-Length"])888 self.assertEqual(str(len(diff)), browser.headers["Content-Length"])
888 self.assertEqual(889 self.assertEqual(
889 "attachment; filename=0123456^_0123456.diff",890 'attachment; filename="some-repository_0123456^_0123456.diff"',
890 browser.headers["Content-Disposition"])891 browser.headers["Content-Disposition"])
891 self.assertEqual(diff, browser.contents)892 self.assertEqual(diff, browser.contents)
892893
@@ -911,6 +912,21 @@
911 Unauthorized, self.getUserBrowser,912 Unauthorized, self.getUserBrowser,
912 repository_url + "/+diff/0123456/0123456^")913 repository_url + "/+diff/0123456/0123456^")
913914
915 def test_filename_quoting(self):
916 # If we construct revisions containing metacharacters and somehow
917 # manage to get that past the hosting service, the
918 # Content-Disposition header is quoted properly.
919 diff = "A fake diff\n"
920 self.useFixture(GitHostingFixture(diff={"patch": diff}))
921 person = self.factory.makePerson()
922 repository = self.factory.makeGitRepository(
923 owner=person, name="some-repository")
924 browser = self.getUserBrowser(
925 canonical_url(repository) + '/+diff/foo"/"bar')
926 self.assertEqual(
927 r'attachment; filename="some-repository_\"bar_foo\".diff"',
928 browser.headers["Content-Disposition"])
929
914930
915class TestGitRepositoryDeletionView(BrowserTestCase):931class TestGitRepositoryDeletionView(BrowserTestCase):
916932
917933
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2018-03-06 00:59:06 +0000
+++ lib/lp/code/interfaces/branch.py 2018-06-22 10:03:28 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2016 Canonical Ltd. This software is licensed under the1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Branch interfaces."""4"""Branch interfaces."""
@@ -765,6 +765,15 @@
765 :param launchbag: `ILaunchBag`.765 :param launchbag: `ILaunchBag`.
766 """766 """
767767
768 def getDiff(new, old):
769 """Get the diff between two revisions in this branch.
770
771 :param new: The new revno or revision ID.
772 :param old: The old revno or revision ID. Defaults to the parent
773 revision of `new`.
774 :return: The diff as a byte string.
775 """
776
768 @export_read_operation()777 @export_read_operation()
769 @operation_for_version('beta')778 @operation_for_version('beta')
770 def canBeDeleted():779 def canBeDeleted():
771780
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2018-03-06 00:59:06 +0000
+++ lib/lp/code/model/branch.py 2018-06-22 10:03:28 +0000
@@ -105,6 +105,7 @@
105 WrongNumberOfReviewTypeArguments,105 WrongNumberOfReviewTypeArguments,
106 )106 )
107from lp.code.interfaces.branchcollection import IAllBranches107from lp.code.interfaces.branchcollection import IAllBranches
108from lp.code.interfaces.branchhosting import IBranchHostingClient
108from lp.code.interfaces.branchlookup import IBranchLookup109from lp.code.interfaces.branchlookup import IBranchLookup
109from lp.code.interfaces.branchmergeproposal import (110from lp.code.interfaces.branchmergeproposal import (
110 BRANCH_MERGE_PROPOSAL_FINAL_STATES,111 BRANCH_MERGE_PROPOSAL_FINAL_STATES,
@@ -782,6 +783,11 @@
782 RevisionAuthor, revisions, ['revision_author_id'])783 RevisionAuthor, revisions, ['revision_author_id'])
783 return DecoratedResultSet(result, pre_iter_hook=eager_load)784 return DecoratedResultSet(result, pre_iter_hook=eager_load)
784785
786 def getDiff(self, new, old=None):
787 """See `IBranch`."""
788 hosting_client = getUtility(IBranchHostingClient)
789 return hosting_client.getDiff(self.id, new, old=old)
790
785 def canBeDeleted(self):791 def canBeDeleted(self):
786 """See `IBranch`."""792 """See `IBranch`."""
787 if ((len(self.deletionRequirements()) != 0) or not793 if ((len(self.deletionRequirements()) != 0) or not
788794
=== modified file 'lib/lp/code/tests/helpers.py'
--- lib/lp/code/tests/helpers.py 2017-12-19 17:16:38 +0000
+++ lib/lp/code/tests/helpers.py 2018-06-22 10:03:28 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2017 Canonical Ltd. This software is licensed under the1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Helper functions for code testing live here."""4"""Helper functions for code testing live here."""
@@ -8,6 +8,7 @@
8__metaclass__ = type8__metaclass__ = type
9__all__ = [9__all__ = [
10 'add_revision_to_branch',10 'add_revision_to_branch',
11 'BranchHostingFixture',
11 'get_non_existant_source_package_branch_unique_name',12 'get_non_existant_source_package_branch_unique_name',
12 'GitHostingFixture',13 'GitHostingFixture',
13 'make_erics_fooix_project',14 'make_erics_fooix_project',
@@ -35,6 +36,7 @@
35 )36 )
3637
37from lp.app.enums import InformationType38from lp.app.enums import InformationType
39from lp.code.interfaces.branchhosting import IBranchHostingClient
38from lp.code.interfaces.branchmergeproposal import (40from lp.code.interfaces.branchmergeproposal import (
39 IBranchMergeProposalJobSource,41 IBranchMergeProposalJobSource,
40 )42 )
@@ -301,6 +303,19 @@
301 c.execute('delete from branch')303 c.execute('delete from branch')
302304
303305
306class BranchHostingFixture(fixtures.Fixture):
307 """A fixture that temporarily registers a fake Bazaar hosting client."""
308
309 def __init__(self, diff=None, inventory=None, blob=None):
310 self.create = FakeMethod()
311 self.getDiff = FakeMethod(result=diff or {})
312 self.getInventory = FakeMethod(result=inventory or {})
313 self.getBlob = FakeMethod(result=blob)
314
315 def _setUp(self):
316 self.useFixture(ZopeUtilityFixture(self, IBranchHostingClient))
317
318
304class GitHostingFixture(fixtures.Fixture):319class GitHostingFixture(fixtures.Fixture):
305 """A fixture that temporarily registers a fake Git hosting client."""320 """A fixture that temporarily registers a fake Git hosting client."""
306321
307322
=== modified file 'lib/lp/services/webapp/publisher.py'
--- lib/lp/services/webapp/publisher.py 2017-11-22 13:05:11 +0000
+++ lib/lp/services/webapp/publisher.py 2018-06-22 10:03:28 +0000
@@ -29,6 +29,7 @@
29from cgi import FieldStorage29from cgi import FieldStorage
30import httplib30import httplib
31import re31import re
32from wsgiref.headers import Headers
3233
33from lazr.restful import (34from lazr.restful import (
34 EntryResource,35 EntryResource,
@@ -233,9 +234,12 @@
233class DataDownloadView:234class DataDownloadView:
234 """Download data without templating.235 """Download data without templating.
235236
236 Subclasses must provide getBody, content_type and filename.237 Subclasses must provide getBody, content_type and filename, and may
238 provide charset.
237 """239 """
238240
241 charset = None
242
239 def __init__(self, context, request):243 def __init__(self, context, request):
240 self.context = context244 self.context = context
241 self.request = request245 self.request = request
@@ -246,10 +250,16 @@
246 It is not necessary to supply Content-length, because this is added by250 It is not necessary to supply Content-length, because this is added by
247 the caller.251 the caller.
248 """252 """
249 self.request.response.setHeader('Content-Type', self.content_type)253 headers = Headers([])
250 self.request.response.setHeader(254 content_type_params = {}
251 'Content-Disposition', 'attachment; filename="%s"' % (255 if self.charset is not None:
252 self.filename))256 content_type_params['charset'] = self.charset
257 headers.add_header(
258 'Content-Type', self.content_type, **content_type_params)
259 headers.add_header(
260 'Content-Disposition', 'attachment', filename=self.filename)
261 for key, value in headers.items():
262 self.request.response.setHeader(key, value)
253 return self.getBody()263 return self.getBody()
254264
255265