Merge lp:~cjwatson/launchpad/git-mail-fix-delta into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 17445
Proposed branch: lp:~cjwatson/launchpad/git-mail-fix-delta
Merge into: lp:launchpad
Diff against target: 311 lines (+157/-17)
6 files modified
lib/lp/code/adapters/gitrepository.py (+6/-4)
lib/lp/code/adapters/tests/test_gitrepository.py (+53/-0)
lib/lp/code/interfaces/gitrepository.py (+12/-3)
lib/lp/code/model/gitnamespace.py (+10/-6)
lib/lp/code/model/gitrepository.py (+4/-0)
lib/lp/code/model/tests/test_gitrepository.py (+72/-4)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-mail-fix-delta
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+257015@code.launchpad.net

Commit message

Allow modifying Git repository names; fix sending mail for repository deltas.

Description of the change

While Git repository subscriptions work now, there's no way to actually cause any mail to be sent due to various bugs elsewhere. This makes it possible to change repository names, and fixes a missing part of GitRepositoryDelta's interface that caused the mailer to crash. The whole assemblage is now tested.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/adapters/gitrepository.py'
--- lib/lp/code/adapters/gitrepository.py 2015-04-16 14:35:17 +0000
+++ lib/lp/code/adapters/gitrepository.py 2015-04-21 23:25:53 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the1# Copyright 2015 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"""Components related to Git repositories."""4"""Components related to Git repositories."""
@@ -22,16 +22,18 @@
2222
23 implements(IGitRepositoryDelta)23 implements(IGitRepositoryDelta)
2424
25 delta_values = ('name', 'identity')25 delta_values = ('name', 'git_identity')
26
27 new_values = ()
2628
27 interface = IGitRepository29 interface = IGitRepository
2830
29 def __init__(self, repository, user, name=None, identity=None):31 def __init__(self, repository, user, name=None, git_identity=None):
30 self.repository = repository32 self.repository = repository
31 self.user = user33 self.user = user
3234
33 self.name = name35 self.name = name
34 self.identity = identity36 self.git_identity = git_identity
3537
36 @classmethod38 @classmethod
37 def construct(klass, old_repository, new_repository, user):39 def construct(klass, old_repository, new_repository, user):
3840
=== added file 'lib/lp/code/adapters/tests/test_gitrepository.py'
--- lib/lp/code/adapters/tests/test_gitrepository.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/adapters/tests/test_gitrepository.py 2015-04-21 23:25:53 +0000
@@ -0,0 +1,53 @@
1# Copyright 2015 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4from lazr.lifecycle.snapshot import Snapshot
5from testtools.matchers import MatchesStructure
6from zope.interface import providedBy
7
8from lp.code.adapters.gitrepository import GitRepositoryDelta
9from lp.code.interfaces.gitrepository import GIT_FEATURE_FLAG
10from lp.services.features.testing import FeatureFixture
11from lp.testing import (
12 person_logged_in,
13 TestCaseWithFactory,
14 )
15from lp.testing.layers import LaunchpadFunctionalLayer
16
17
18class TestGitRepositoryDelta(TestCaseWithFactory):
19
20 layer = LaunchpadFunctionalLayer
21
22 def setUp(self):
23 super(TestGitRepositoryDelta, self).setUp()
24 self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
25
26 def test_no_modification(self):
27 # If there are no modifications, no delta is returned.
28 repository = self.factory.makeGitRepository(name=u"foo")
29 old_repository = Snapshot(repository, providing=providedBy(repository))
30 delta = GitRepositoryDelta.construct(
31 old_repository, repository, repository.owner)
32 self.assertIsNone(delta)
33
34 def test_modification(self):
35 # If there are modifications, the delta reflects them.
36 owner = self.factory.makePerson(name="person")
37 project = self.factory.makeProduct(name="project")
38 repository = self.factory.makeGitRepository(
39 owner=owner, target=project, name=u"foo")
40 old_repository = Snapshot(repository, providing=providedBy(repository))
41 with person_logged_in(repository.owner):
42 repository.setName(u"bar", repository.owner)
43 delta = GitRepositoryDelta.construct(old_repository, repository, owner)
44 self.assertIsNotNone(delta)
45 self.assertThat(delta, MatchesStructure.byEquality(
46 name={
47 "old": u"foo",
48 "new": u"bar",
49 },
50 git_identity={
51 "old": u"lp:~person/project/+git/foo",
52 "new": u"lp:~person/project/+git/bar",
53 }))
054
=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py 2015-04-17 00:01:15 +0000
+++ lib/lp/code/interfaces/gitrepository.py 2015-04-21 23:25:53 +0000
@@ -200,9 +200,9 @@
200 def isPersonTrustedReviewer(reviewer):200 def isPersonTrustedReviewer(reviewer):
201 """Return true if the `reviewer` is a trusted reviewer.201 """Return true if the `reviewer` is a trusted reviewer.
202202
203 The reviewer is trusted if they either own the repository, or are in the203 The reviewer is trusted if they either own the repository, or are in
204 team that owns the repository, or they are in the review team for the204 the team that owns the repository, or they are in the review team
205 repository.205 for the repository.
206 """206 """
207207
208 git_identity = exported(Text(208 git_identity = exported(Text(
@@ -490,6 +490,15 @@
490class IGitRepositoryEdit(Interface):490class IGitRepositoryEdit(Interface):
491 """IGitRepository methods that require launchpad.Edit permission."""491 """IGitRepository methods that require launchpad.Edit permission."""
492492
493 @mutator_for(IGitRepositoryView["name"])
494 @call_with(user=REQUEST_USER)
495 @operation_parameters(
496 new_name=TextLine(title=_("The new name of the repository.")))
497 @export_write_operation()
498 @operation_for_version("devel")
499 def setName(new_name, user):
500 """Set the name of the repository to be `new_name`."""
501
493 @mutator_for(IGitRepositoryView["owner"])502 @mutator_for(IGitRepositoryView["owner"])
494 @call_with(user=REQUEST_USER)503 @call_with(user=REQUEST_USER)
495 @operation_parameters(504 @operation_parameters(
496505
=== modified file 'lib/lp/code/model/gitnamespace.py'
--- lib/lp/code/model/gitnamespace.py 2015-04-16 23:56:21 +0000
+++ lib/lp/code/model/gitnamespace.py 2015-04-21 23:25:53 +0000
@@ -152,9 +152,10 @@
152 def moveRepository(self, repository, mover, new_name=None,152 def moveRepository(self, repository, mover, new_name=None,
153 rename_if_necessary=False):153 rename_if_necessary=False):
154 """See `IGitNamespace`."""154 """See `IGitNamespace`."""
155 # Check to see if the repository is already in this namespace.155 # Check to see if the repository is already in this namespace with
156 # this name.
156 old_namespace = repository.namespace157 old_namespace = repository.namespace
157 if self.name == old_namespace.name:158 if self.name == old_namespace.name and new_name is None:
158 return159 return
159 if new_name is None:160 if new_name is None:
160 new_name = repository.name161 new_name = repository.name
@@ -164,10 +165,13 @@
164 # Remove the security proxy of the repository as the owner and165 # Remove the security proxy of the repository as the owner and
165 # target attributes are read-only through the interface.166 # target attributes are read-only through the interface.
166 naked_repository = removeSecurityProxy(repository)167 naked_repository = removeSecurityProxy(repository)
167 naked_repository.owner = self.owner168 if self.owner != repository.owner:
168 self._retargetRepository(naked_repository)169 naked_repository.owner = self.owner
169 del get_property_cache(naked_repository).target170 if self.target != repository.target:
170 naked_repository.name = new_name171 self._retargetRepository(naked_repository)
172 del get_property_cache(naked_repository).target
173 if new_name != repository.name:
174 naked_repository.name = new_name
171175
172 def getRepositories(self):176 def getRepositories(self):
173 """See `IGitNamespace`."""177 """See `IGitNamespace`."""
174178
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2015-04-21 11:39:09 +0000
+++ lib/lp/code/model/gitrepository.py 2015-04-21 23:25:53 +0000
@@ -622,6 +622,10 @@
622 # subscriptions.622 # subscriptions.
623 getUtility(IRemoveArtifactSubscriptionsJobSource).create(user, [self])623 getUtility(IRemoveArtifactSubscriptionsJobSource).create(user, [self])
624624
625 def setName(self, new_name, user):
626 """See `IGitRepository`."""
627 self.namespace.moveRepository(self, user, new_name=new_name)
628
625 def setOwner(self, new_owner, user):629 def setOwner(self, new_owner, user):
626 """See `IGitRepository`."""630 """See `IGitRepository`."""
627 new_namespace = get_git_namespace(self.target, new_owner)631 new_namespace = get_git_namespace(self.target, new_owner)
628632
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2015-04-21 11:39:09 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2015-04-21 23:25:53 +0000
@@ -6,11 +6,14 @@
6__metaclass__ = type6__metaclass__ = type
77
8from datetime import datetime8from datetime import datetime
9import email
9from functools import partial10from functools import partial
10import hashlib11import hashlib
11import json12import json
1213
13from lazr.lifecycle.event import ObjectModifiedEvent14from lazr.lifecycle.event import ObjectModifiedEvent
15from lazr.lifecycle.snapshot import Snapshot
16import transaction
14import pytz17import pytz
15from testtools.matchers import (18from testtools.matchers import (
16 EndsWith,19 EndsWith,
@@ -19,6 +22,8 @@
19 )22 )
20from zope.component import getUtility23from zope.component import getUtility
21from zope.event import notify24from zope.event import notify
25from zope.interface import providedBy
26from zope.security.interfaces import Unauthorized
22from zope.security.proxy import removeSecurityProxy27from zope.security.proxy import removeSecurityProxy
2328
24from lp.app.enums import (29from lp.app.enums import (
@@ -37,6 +42,7 @@
37 GitFeatureDisabled,42 GitFeatureDisabled,
38 GitRepositoryCreatorNotMemberOfOwnerTeam,43 GitRepositoryCreatorNotMemberOfOwnerTeam,
39 GitRepositoryCreatorNotOwner,44 GitRepositoryCreatorNotOwner,
45 GitRepositoryExists,
40 GitTargetError,46 GitTargetError,
41 )47 )
42from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository48from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository
@@ -68,6 +74,7 @@
68from lp.registry.tests.test_accesspolicy import get_policies_for_artifact74from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
69from lp.services.database.constants import UTC_NOW75from lp.services.database.constants import UTC_NOW
70from lp.services.features.testing import FeatureFixture76from lp.services.features.testing import FeatureFixture
77from lp.services.mail import stub
71from lp.services.webapp.authorization import check_permission78from lp.services.webapp.authorization import check_permission
72from lp.services.webapp.interfaces import OAuthPermission79from lp.services.webapp.interfaces import OAuthPermission
73from lp.testing import (80from lp.testing import (
@@ -288,16 +295,16 @@
288 repository.getRepositoryIdentities())295 repository.getRepositoryIdentities())
289296
290297
291class TestGitRepositoryDateLastModified(TestCaseWithFactory):298class TestGitRepositoryModifications(TestCaseWithFactory):
292 """Exercise the situations where date_last_modified is updated."""299 """Tests for Git repository modification notifications."""
293300
294 layer = DatabaseFunctionalLayer301 layer = DatabaseFunctionalLayer
295302
296 def setUp(self):303 def setUp(self):
297 super(TestGitRepositoryDateLastModified, self).setUp()304 super(TestGitRepositoryModifications, self).setUp()
298 self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))305 self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
299306
300 def test_initial_value(self):307 def test_date_last_modified_initial_value(self):
301 # The initial value of date_last_modified is date_created.308 # The initial value of date_last_modified is date_created.
302 repository = self.factory.makeGitRepository()309 repository = self.factory.makeGitRepository()
303 self.assertEqual(310 self.assertEqual(
@@ -314,6 +321,34 @@
314 self.assertSqlAttributeEqualsDate(321 self.assertSqlAttributeEqualsDate(
315 repository, "date_last_modified", UTC_NOW)322 repository, "date_last_modified", UTC_NOW)
316323
324 def test_sends_notifications(self):
325 # Attribute modifications send mail to subscribers.
326 self.assertEqual(0, len(stub.test_emails))
327 repository = self.factory.makeGitRepository(name=u"foo")
328 repository_before_modification = Snapshot(
329 repository, providing=providedBy(repository))
330 with person_logged_in(repository.owner):
331 repository.subscribe(
332 repository.owner,
333 BranchSubscriptionNotificationLevel.ATTRIBUTEONLY,
334 BranchSubscriptionDiffSize.NODIFF,
335 CodeReviewNotificationLevel.NOEMAIL,
336 repository.owner)
337 repository.setName(u"bar", repository.owner)
338 notify(ObjectModifiedEvent(
339 repository, repository_before_modification, ["name"],
340 user=repository.owner))
341 transaction.commit()
342 self.assertEqual(1, len(stub.test_emails))
343 message = email.message_from_string(stub.test_emails[0][2])
344 body = message.get_payload(decode=True)
345 self.assertIn("Name: foo => bar\n", body)
346 self.assertIn(
347 "Git identity: lp:~{person}/{project}/+git/foo => "
348 "lp:~{person}/{project}/+git/bar\n".format(
349 person=repository.owner.name, project=repository.target.name),
350 body)
351
317 # XXX cjwatson 2015-02-04: This will need to be expanded once Launchpad352 # XXX cjwatson 2015-02-04: This will need to be expanded once Launchpad
318 # actually notices any interesting kind of repository modifications.353 # actually notices any interesting kind of repository modifications.
319354
@@ -892,6 +927,39 @@
892 self.assertNotTrustedReviewer(repository, reviewer)927 self.assertNotTrustedReviewer(repository, reviewer)
893928
894929
930class TestGitRepositorySetName(TestCaseWithFactory):
931 """Test `IGitRepository.setName`."""
932
933 layer = DatabaseFunctionalLayer
934
935 def setUp(self):
936 super(TestGitRepositorySetName, self).setUp()
937 self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
938
939 def test_not_owner(self):
940 # A non-owner non-admin user cannot rename a repository.
941 repository = self.factory.makeGitRepository()
942 with person_logged_in(self.factory.makePerson()):
943 self.assertRaises(Unauthorized, getattr, repository, "setName")
944
945 def test_name_clash(self):
946 # Name clashes are refused.
947 repository = self.factory.makeGitRepository(name=u"foo")
948 self.factory.makeGitRepository(
949 owner=repository.owner, target=repository.target, name=u"bar")
950 with person_logged_in(repository.owner):
951 self.assertRaises(
952 GitRepositoryExists, repository.setName,
953 u"bar", repository.owner)
954
955 def test_rename(self):
956 # A non-clashing rename request works.
957 repository = self.factory.makeGitRepository(name=u"foo")
958 with person_logged_in(repository.owner):
959 repository.setName(u"bar", repository.owner)
960 self.assertEqual(u"bar", repository.name)
961
962
895class TestGitRepositorySetOwner(TestCaseWithFactory):963class TestGitRepositorySetOwner(TestCaseWithFactory):
896 """Test `IGitRepository.setOwner`."""964 """Test `IGitRepository.setOwner`."""
897965