Merge ~pappacena/launchpad:git-fork-backend into launchpad:master
- Git
- lp:~pappacena/launchpad
- git-fork-backend
- Merge into master
Proposed by
Thiago F. Pappacena
Status: | Merged |
---|---|
Approved by: | Thiago F. Pappacena |
Approved revision: | 9867039b20fc26412e770d5474d53ad45edefac6 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~pappacena/launchpad:git-fork-backend |
Merge into: | launchpad:master |
Diff against target: |
412 lines (+138/-24) 13 files modified
lib/lp/code/interfaces/githosting.py (+4/-2) lib/lp/code/interfaces/gitnamespace.py (+10/-2) lib/lp/code/interfaces/gitrepository.py (+8/-0) lib/lp/code/model/githosting.py (+9/-2) lib/lp/code/model/gitjob.py (+2/-1) lib/lp/code/model/gitnamespace.py (+14/-2) lib/lp/code/model/gitrepository.py (+20/-4) lib/lp/code/model/tests/test_githosting.py (+10/-2) lib/lp/code/model/tests/test_gitjob.py (+1/-1) lib/lp/code/model/tests/test_gitrepository.py (+34/-1) lib/lp/code/tests/helpers.py (+1/-1) lib/lp/code/xmlrpc/git.py (+9/-3) lib/lp/code/xmlrpc/tests/test_git.py (+16/-3) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+387146@code.launchpad.net |
Commit message
Fork method on GitRepository to allow users to asynchronously create a copy of a repository
Description of the change
To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote : | # |
Revision history for this message
Colin Watson (cjwatson) wrote : | # |
I haven't completely gone through this yet, but here are a few high-level/
review:
Needs Information
- 4774c21... by Thiago F. Pappacena
-
Merge branch 'master' into git-fork-backend
- 249af52... by Thiago F. Pappacena
-
Running confirmation job with existing db user
- c313a77... by Thiago F. Pappacena
-
separating user that requested a fork and the new repository owner
Revision history for this message
Thiago F. Pappacena (pappacena) wrote : | # |
I'm pushing some of the requested changes, and I could use your opinion on one of the comments, cjwatson.
Revision history for this message
Colin Watson (cjwatson) : | # |
Revision history for this message
Thiago F. Pappacena (pappacena) : | # |
- f56198c... by Thiago F. Pappacena
-
Remove polling job to confirm repo creation
- 2eff6c9... by Thiago F. Pappacena
-
Loose permission check for confirm/abort repo creation
- 730a9dc... by Thiago F. Pappacena
-
Avoiding race condition on create/confirm API calls
- 6fdbd8c... by Thiago F. Pappacena
-
Fixing test
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
- ded71d0... by Thiago F. Pappacena
-
Merge branch 'master' into git-fork-backend
- 9867039... by Thiago F. Pappacena
-
Fixing repository name clash when forking
Revision history for this message
Thiago F. Pappacena (pappacena) wrote : | # |
Pushed the requested changes. This should be good to go now.
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/code/interfaces/githosting.py b/lib/lp/code/interfaces/githosting.py | |||
2 | index 378930a..d854174 100644 | |||
3 | --- a/lib/lp/code/interfaces/githosting.py | |||
4 | +++ b/lib/lp/code/interfaces/githosting.py | |||
5 | @@ -1,4 +1,4 @@ | |||
7 | 1 | # Copyright 2015-2018 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
8 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
9 | 3 | 3 | ||
10 | 4 | """Interface for communication with the Git hosting service.""" | 4 | """Interface for communication with the Git hosting service.""" |
11 | @@ -14,13 +14,15 @@ from zope.interface import Interface | |||
12 | 14 | class IGitHostingClient(Interface): | 14 | class IGitHostingClient(Interface): |
13 | 15 | """Interface for the internal API provided by the Git hosting service.""" | 15 | """Interface for the internal API provided by the Git hosting service.""" |
14 | 16 | 16 | ||
16 | 17 | def create(path, clone_from=None): | 17 | def create(path, clone_from=None, async_create=False): |
17 | 18 | """Create a Git repository. | 18 | """Create a Git repository. |
18 | 19 | 19 | ||
19 | 20 | :param path: Physical path of the new repository on the hosting | 20 | :param path: Physical path of the new repository on the hosting |
20 | 21 | service. | 21 | service. |
21 | 22 | :param clone_from: If not None, clone the new repository from this | 22 | :param clone_from: If not None, clone the new repository from this |
22 | 23 | other physical path. | 23 | other physical path. |
23 | 24 | :param async_create: Do not block the call until the repository is | ||
24 | 25 | actually created. | ||
25 | 24 | """ | 26 | """ |
26 | 25 | 27 | ||
27 | 26 | def getProperties(path): | 28 | def getProperties(path): |
28 | diff --git a/lib/lp/code/interfaces/gitnamespace.py b/lib/lp/code/interfaces/gitnamespace.py | |||
29 | index 9c6886b..3a6e600 100644 | |||
30 | --- a/lib/lp/code/interfaces/gitnamespace.py | |||
31 | +++ b/lib/lp/code/interfaces/gitnamespace.py | |||
32 | @@ -40,8 +40,16 @@ class IGitNamespace(Interface): | |||
33 | 40 | def createRepository(repository_type, registrant, name, | 40 | def createRepository(repository_type, registrant, name, |
34 | 41 | information_type=None, date_created=None, | 41 | information_type=None, date_created=None, |
35 | 42 | target_default=False, owner_default=False, | 42 | target_default=False, owner_default=False, |
38 | 43 | with_hosting=False, status=None): | 43 | with_hosting=False, async_hosting=False, status=None): |
39 | 44 | """Create and return an `IGitRepository` in this namespace.""" | 44 | """Create and return an `IGitRepository` in this namespace. |
40 | 45 | |||
41 | 46 | :param with_hosting: If True, also creates the repository on git | ||
42 | 47 | hosting service. | ||
43 | 48 | :param async_hosting: If with_hosting is True, this controls if the | ||
44 | 49 | call to create repository on hosting service will be done | ||
45 | 50 | asynchronously, or it will block until the service creates the | ||
46 | 51 | repository. | ||
47 | 52 | """ | ||
48 | 45 | 53 | ||
49 | 46 | def isNameUsed(name): | 54 | def isNameUsed(name): |
50 | 47 | """Is 'name' already used in this namespace?""" | 55 | """Is 'name' already used in this namespace?""" |
51 | diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py | |||
52 | index 344bc43..201c9a4 100644 | |||
53 | --- a/lib/lp/code/interfaces/gitrepository.py | |||
54 | +++ b/lib/lp/code/interfaces/gitrepository.py | |||
55 | @@ -968,6 +968,14 @@ class IGitRepositorySet(Interface): | |||
56 | 968 | :param with_hosting: Create the repository on the hosting service. | 968 | :param with_hosting: Create the repository on the hosting service. |
57 | 969 | """ | 969 | """ |
58 | 970 | 970 | ||
59 | 971 | def fork(origin, requester, new_owner): | ||
60 | 972 | """Fork a repository to the given user's account. | ||
61 | 973 | |||
62 | 974 | :param origin: The original GitRepository. | ||
63 | 975 | :param requester: The IPerson performing this fork. | ||
64 | 976 | :param new_owner: The IPerson that will own the forked repository. | ||
65 | 977 | :return: The newly created GitRepository.""" | ||
66 | 978 | |||
67 | 971 | # Marker for references to Git URL layouts: ##GITNAMESPACE## | 979 | # Marker for references to Git URL layouts: ##GITNAMESPACE## |
68 | 972 | @call_with(user=REQUEST_USER) | 980 | @call_with(user=REQUEST_USER) |
69 | 973 | @operation_parameters( | 981 | @operation_parameters( |
70 | diff --git a/lib/lp/code/model/githosting.py b/lib/lp/code/model/githosting.py | |||
71 | index 94d6538..60dff43 100644 | |||
72 | --- a/lib/lp/code/model/githosting.py | |||
73 | +++ b/lib/lp/code/model/githosting.py | |||
74 | @@ -1,4 +1,4 @@ | |||
76 | 1 | # Copyright 2015-2019 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
77 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
78 | 3 | 3 | ||
79 | 4 | """Communication with the Git hosting service.""" | 4 | """Communication with the Git hosting service.""" |
80 | @@ -90,13 +90,20 @@ class GitHostingClient: | |||
81 | 90 | def _delete(self, path, **kwargs): | 90 | def _delete(self, path, **kwargs): |
82 | 91 | return self._request("delete", path, **kwargs) | 91 | return self._request("delete", path, **kwargs) |
83 | 92 | 92 | ||
85 | 93 | def create(self, path, clone_from=None): | 93 | def create(self, path, clone_from=None, async_create=False): |
86 | 94 | """See `IGitHostingClient`.""" | 94 | """See `IGitHostingClient`.""" |
87 | 95 | try: | 95 | try: |
88 | 96 | if clone_from: | 96 | if clone_from: |
89 | 97 | request = {"repo_path": path, "clone_from": clone_from} | 97 | request = {"repo_path": path, "clone_from": clone_from} |
90 | 98 | else: | 98 | else: |
91 | 99 | request = {"repo_path": path} | 99 | request = {"repo_path": path} |
92 | 100 | if async_create: | ||
93 | 101 | # XXX pappacena 2020-07-02: async forces to clone_refs | ||
94 | 102 | # because it's only used in situations where this is | ||
95 | 103 | # desirable for now. We might need to add "clone_refs" as | ||
96 | 104 | # parameter in the future. | ||
97 | 105 | request['async'] = True | ||
98 | 106 | request['clone_refs'] = clone_from is not None | ||
99 | 100 | self._post("/repo", json=request) | 107 | self._post("/repo", json=request) |
100 | 101 | except requests.RequestException as e: | 108 | except requests.RequestException as e: |
101 | 102 | raise GitRepositoryCreationFault( | 109 | raise GitRepositoryCreationFault( |
102 | diff --git a/lib/lp/code/model/gitjob.py b/lib/lp/code/model/gitjob.py | |||
103 | index 3c041da..ab37d2b 100644 | |||
104 | --- a/lib/lp/code/model/gitjob.py | |||
105 | +++ b/lib/lp/code/model/gitjob.py | |||
106 | @@ -1,4 +1,4 @@ | |||
108 | 1 | # Copyright 2015-2018 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
109 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
110 | 3 | 3 | ||
111 | 4 | __metaclass__ = type | 4 | __metaclass__ = type |
112 | @@ -12,6 +12,7 @@ __all__ = [ | |||
113 | 12 | 'ReclaimGitRepositorySpaceJob', | 12 | 'ReclaimGitRepositorySpaceJob', |
114 | 13 | ] | 13 | ] |
115 | 14 | 14 | ||
116 | 15 | |||
117 | 15 | from lazr.delegates import delegate_to | 16 | from lazr.delegates import delegate_to |
118 | 16 | from lazr.enum import ( | 17 | from lazr.enum import ( |
119 | 17 | DBEnumeratedType, | 18 | DBEnumeratedType, |
120 | diff --git a/lib/lp/code/model/gitnamespace.py b/lib/lp/code/model/gitnamespace.py | |||
121 | index f9ac2f5..22e9d8f 100644 | |||
122 | --- a/lib/lp/code/model/gitnamespace.py | |||
123 | +++ b/lib/lp/code/model/gitnamespace.py | |||
124 | @@ -14,6 +14,7 @@ __all__ = [ | |||
125 | 14 | 14 | ||
126 | 15 | from lazr.lifecycle.event import ObjectCreatedEvent | 15 | from lazr.lifecycle.event import ObjectCreatedEvent |
127 | 16 | from storm.locals import And | 16 | from storm.locals import And |
128 | 17 | import transaction | ||
129 | 17 | from zope.component import getUtility | 18 | from zope.component import getUtility |
130 | 18 | from zope.event import notify | 19 | from zope.event import notify |
131 | 19 | from zope.interface import implementer | 20 | from zope.interface import implementer |
132 | @@ -33,6 +34,7 @@ from lp.code.enums import ( | |||
133 | 33 | BranchSubscriptionDiffSize, | 34 | BranchSubscriptionDiffSize, |
134 | 34 | BranchSubscriptionNotificationLevel, | 35 | BranchSubscriptionNotificationLevel, |
135 | 35 | CodeReviewNotificationLevel, | 36 | CodeReviewNotificationLevel, |
136 | 37 | GitRepositoryStatus, | ||
137 | 36 | ) | 38 | ) |
138 | 37 | from lp.code.errors import ( | 39 | from lp.code.errors import ( |
139 | 38 | GitDefaultConflict, | 40 | GitDefaultConflict, |
140 | @@ -75,7 +77,8 @@ class _BaseGitNamespace: | |||
141 | 75 | reviewer=None, information_type=None, | 77 | reviewer=None, information_type=None, |
142 | 76 | date_created=DEFAULT, description=None, | 78 | date_created=DEFAULT, description=None, |
143 | 77 | target_default=False, owner_default=False, | 79 | target_default=False, owner_default=False, |
145 | 78 | with_hosting=False, status=None): | 80 | with_hosting=False, async_hosting=False, |
146 | 81 | status=GitRepositoryStatus.AVAILABLE): | ||
147 | 79 | """See `IGitNamespace`.""" | 82 | """See `IGitNamespace`.""" |
148 | 80 | repository_set = getUtility(IGitRepositorySet) | 83 | repository_set = getUtility(IGitRepositorySet) |
149 | 81 | 84 | ||
150 | @@ -119,11 +122,20 @@ class _BaseGitNamespace: | |||
151 | 119 | 122 | ||
152 | 120 | # Flush to make sure that repository.id is populated. | 123 | # Flush to make sure that repository.id is populated. |
153 | 121 | IStore(repository).flush() | 124 | IStore(repository).flush() |
154 | 125 | if async_hosting: | ||
155 | 126 | # If we are going to run async creation, we need to be sure | ||
156 | 127 | # the transaction is committed. | ||
157 | 128 | # Async creation will run a callback on Launchpad, and if | ||
158 | 129 | # the creation is quick enough, it might try to confirm on | ||
159 | 130 | # Launchpad (in another transaction) the creation of this | ||
160 | 131 | # repo before this transaction is actually committed. | ||
161 | 132 | transaction.commit() | ||
162 | 122 | assert repository.id is not None | 133 | assert repository.id is not None |
163 | 123 | 134 | ||
164 | 124 | clone_from_repository = repository.getClonedFrom() | 135 | clone_from_repository = repository.getClonedFrom() |
165 | 125 | repository._createOnHostingService( | 136 | repository._createOnHostingService( |
167 | 126 | clone_from_repository=clone_from_repository) | 137 | clone_from_repository=clone_from_repository, |
168 | 138 | async_create=async_hosting) | ||
169 | 127 | 139 | ||
170 | 128 | return repository | 140 | return repository |
171 | 129 | 141 | ||
172 | diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py | |||
173 | index 3d2408e..726e9f1 100644 | |||
174 | --- a/lib/lp/code/model/gitrepository.py | |||
175 | +++ b/lib/lp/code/model/gitrepository.py | |||
176 | @@ -359,7 +359,8 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin): | |||
177 | 359 | self.owner_default = False | 359 | self.owner_default = False |
178 | 360 | self.target_default = False | 360 | self.target_default = False |
179 | 361 | 361 | ||
181 | 362 | def _createOnHostingService(self, clone_from_repository=None): | 362 | def _createOnHostingService( |
182 | 363 | self, clone_from_repository=None, async_create=False): | ||
183 | 363 | """Create this repository on the hosting service.""" | 364 | """Create this repository on the hosting service.""" |
184 | 364 | hosting_path = self.getInternalPath() | 365 | hosting_path = self.getInternalPath() |
185 | 365 | if clone_from_repository is not None: | 366 | if clone_from_repository is not None: |
186 | @@ -367,7 +368,8 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin): | |||
187 | 367 | else: | 368 | else: |
188 | 368 | clone_from_path = None | 369 | clone_from_path = None |
189 | 369 | getUtility(IGitHostingClient).create( | 370 | getUtility(IGitHostingClient).create( |
191 | 370 | hosting_path, clone_from=clone_from_path) | 371 | hosting_path, clone_from=clone_from_path, |
192 | 372 | async_create=async_create) | ||
193 | 371 | 373 | ||
194 | 372 | def getClonedFrom(self): | 374 | def getClonedFrom(self): |
195 | 373 | """See `IGitRepository`""" | 375 | """See `IGitRepository`""" |
196 | @@ -1729,13 +1731,27 @@ class GitRepositorySet: | |||
197 | 1729 | 1731 | ||
198 | 1730 | def new(self, repository_type, registrant, owner, target, name, | 1732 | def new(self, repository_type, registrant, owner, target, name, |
199 | 1731 | information_type=None, date_created=DEFAULT, description=None, | 1733 | information_type=None, date_created=DEFAULT, description=None, |
201 | 1732 | with_hosting=False): | 1734 | with_hosting=False, async_hosting=False, |
202 | 1735 | status=GitRepositoryStatus.AVAILABLE): | ||
203 | 1733 | """See `IGitRepositorySet`.""" | 1736 | """See `IGitRepositorySet`.""" |
204 | 1734 | namespace = get_git_namespace(target, owner) | 1737 | namespace = get_git_namespace(target, owner) |
205 | 1735 | return namespace.createRepository( | 1738 | return namespace.createRepository( |
206 | 1736 | repository_type, registrant, name, | 1739 | repository_type, registrant, name, |
207 | 1737 | information_type=information_type, date_created=date_created, | 1740 | information_type=information_type, date_created=date_created, |
209 | 1738 | description=description, with_hosting=with_hosting) | 1741 | description=description, with_hosting=with_hosting, |
210 | 1742 | async_hosting=async_hosting, status=status) | ||
211 | 1743 | |||
212 | 1744 | def fork(self, origin, requester, new_owner): | ||
213 | 1745 | namespace = get_git_namespace(origin.target, new_owner) | ||
214 | 1746 | name = namespace.findUnusedName(origin.name) | ||
215 | 1747 | repository = self.new( | ||
216 | 1748 | repository_type=GitRepositoryType.HOSTED, | ||
217 | 1749 | registrant=requester, owner=new_owner, target=origin.target, | ||
218 | 1750 | name=name, information_type=origin.information_type, | ||
219 | 1751 | date_created=UTC_NOW, description=origin.description, | ||
220 | 1752 | with_hosting=True, async_hosting=True, | ||
221 | 1753 | status=GitRepositoryStatus.CREATING) | ||
222 | 1754 | return repository | ||
223 | 1739 | 1755 | ||
224 | 1740 | def getByPath(self, user, path): | 1756 | def getByPath(self, user, path): |
225 | 1741 | """See `IGitRepositorySet`.""" | 1757 | """See `IGitRepositorySet`.""" |
226 | diff --git a/lib/lp/code/model/tests/test_githosting.py b/lib/lp/code/model/tests/test_githosting.py | |||
227 | index d966dbe..b35fdd7 100644 | |||
228 | --- a/lib/lp/code/model/tests/test_githosting.py | |||
229 | +++ b/lib/lp/code/model/tests/test_githosting.py | |||
230 | @@ -2,7 +2,7 @@ | |||
231 | 2 | # NOTE: The first line above must stay first; do not move the copyright | 2 | # NOTE: The first line above must stay first; do not move the copyright |
232 | 3 | # notice to the top. See http://www.python.org/dev/peps/pep-0263/. | 3 | # notice to the top. See http://www.python.org/dev/peps/pep-0263/. |
233 | 4 | # | 4 | # |
235 | 5 | # Copyright 2016-2019 Canonical Ltd. This software is licensed under the | 5 | # Copyright 2016-2020 Canonical Ltd. This software is licensed under the |
236 | 6 | # GNU Affero General Public License version 3 (see the file LICENSE). | 6 | # GNU Affero General Public License version 3 (see the file LICENSE). |
237 | 7 | 7 | ||
238 | 8 | """Unit tests for `GitHostingClient`. | 8 | """Unit tests for `GitHostingClient`. |
239 | @@ -23,7 +23,6 @@ from lazr.restful.utils import get_current_browser_request | |||
240 | 23 | import responses | 23 | import responses |
241 | 24 | from six.moves.urllib.parse import ( | 24 | from six.moves.urllib.parse import ( |
242 | 25 | parse_qsl, | 25 | parse_qsl, |
243 | 26 | urljoin, | ||
244 | 27 | urlsplit, | 26 | urlsplit, |
245 | 28 | ) | 27 | ) |
246 | 29 | from testtools.matchers import ( | 28 | from testtools.matchers import ( |
247 | @@ -130,6 +129,15 @@ class TestGitHostingClient(TestCase): | |||
248 | 130 | "repo", method="POST", | 129 | "repo", method="POST", |
249 | 131 | json_data={"repo_path": "123", "clone_from": "122"}) | 130 | json_data={"repo_path": "123", "clone_from": "122"}) |
250 | 132 | 131 | ||
251 | 132 | def test_create_async(self): | ||
252 | 133 | with self.mockRequests("POST"): | ||
253 | 134 | self.client.create("123", clone_from="122", async_create=True) | ||
254 | 135 | self.assertRequest( | ||
255 | 136 | "repo", method="POST", | ||
256 | 137 | json_data={ | ||
257 | 138 | "repo_path": "123", "clone_from": "122", "async": True, | ||
258 | 139 | "clone_refs": True}) | ||
259 | 140 | |||
260 | 133 | def test_create_failure(self): | 141 | def test_create_failure(self): |
261 | 134 | with self.mockRequests("POST", status=400): | 142 | with self.mockRequests("POST", status=400): |
262 | 135 | self.assertRaisesWithContent( | 143 | self.assertRaisesWithContent( |
263 | diff --git a/lib/lp/code/model/tests/test_gitjob.py b/lib/lp/code/model/tests/test_gitjob.py | |||
264 | index 5964944..d4bfe20 100644 | |||
265 | --- a/lib/lp/code/model/tests/test_gitjob.py | |||
266 | +++ b/lib/lp/code/model/tests/test_gitjob.py | |||
267 | @@ -1,4 +1,4 @@ | |||
269 | 1 | # Copyright 2015-2019 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
270 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
271 | 3 | 3 | ||
272 | 4 | """Tests for `GitJob`s.""" | 4 | """Tests for `GitJob`s.""" |
273 | diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py | |||
274 | index 62bf865..7888d2f 100644 | |||
275 | --- a/lib/lp/code/model/tests/test_gitrepository.py | |||
276 | +++ b/lib/lp/code/model/tests/test_gitrepository.py | |||
277 | @@ -2729,6 +2729,38 @@ class TestGitRepositoryMarkSnapsStale(TestCaseWithFactory): | |||
278 | 2729 | self.assertFalse(snap.is_stale) | 2729 | self.assertFalse(snap.is_stale) |
279 | 2730 | 2730 | ||
280 | 2731 | 2731 | ||
281 | 2732 | class TestGitRepositoryFork(TestCaseWithFactory): | ||
282 | 2733 | |||
283 | 2734 | layer = DatabaseFunctionalLayer | ||
284 | 2735 | |||
285 | 2736 | def setUp(self): | ||
286 | 2737 | super(TestGitRepositoryFork, self).setUp() | ||
287 | 2738 | self.hosting_fixture = self.useFixture(GitHostingFixture()) | ||
288 | 2739 | |||
289 | 2740 | def test_fork(self): | ||
290 | 2741 | repo = self.factory.makeGitRepository() | ||
291 | 2742 | another_person = self.factory.makePerson() | ||
292 | 2743 | another_team = self.factory.makeTeam(members=[another_person]) | ||
293 | 2744 | |||
294 | 2745 | forked_repo = getUtility(IGitRepositorySet).fork( | ||
295 | 2746 | repo, another_person, another_team) | ||
296 | 2747 | self.assertEqual(another_team, forked_repo.owner) | ||
297 | 2748 | self.assertEqual(another_person, forked_repo.registrant) | ||
298 | 2749 | self.assertEqual(1, self.hosting_fixture.create.call_count) | ||
299 | 2750 | |||
300 | 2751 | def test_fork_same_name(self): | ||
301 | 2752 | repo = self.factory.makeGitRepository() | ||
302 | 2753 | |||
303 | 2754 | person = self.factory.makePerson() | ||
304 | 2755 | same_name_repo = self.factory.makeGitRepository( | ||
305 | 2756 | owner=person, registrant=person, | ||
306 | 2757 | name=repo.name, target=repo.target) | ||
307 | 2758 | |||
308 | 2759 | forked_repo = getUtility(IGitRepositorySet).fork(repo, person, person) | ||
309 | 2760 | self.assertEqual(forked_repo.target, repo.target) | ||
310 | 2761 | self.assertEqual(forked_repo.name, "%s-1" % same_name_repo.name) | ||
311 | 2762 | |||
312 | 2763 | |||
313 | 2732 | class TestGitRepositoryDetectMerges(TestCaseWithFactory): | 2764 | class TestGitRepositoryDetectMerges(TestCaseWithFactory): |
314 | 2733 | 2765 | ||
315 | 2734 | layer = ZopelessDatabaseLayer | 2766 | layer = ZopelessDatabaseLayer |
316 | @@ -3271,7 +3303,8 @@ class TestGitRepositorySet(TestCaseWithFactory): | |||
317 | 3271 | self.assertThat(repository, MatchesStructure.byEquality( | 3303 | self.assertThat(repository, MatchesStructure.byEquality( |
318 | 3272 | registrant=owner, owner=owner, target=target, name=name)) | 3304 | registrant=owner, owner=owner, target=target, name=name)) |
319 | 3273 | self.assertEqual( | 3305 | self.assertEqual( |
321 | 3274 | [((repository.getInternalPath(),), {"clone_from": None})], | 3306 | [((repository.getInternalPath(),), |
322 | 3307 | {'async_create': False, "clone_from": None})], | ||
323 | 3275 | hosting_fixture.create.calls) | 3308 | hosting_fixture.create.calls) |
324 | 3276 | 3309 | ||
325 | 3277 | def test_provides_IGitRepositorySet(self): | 3310 | def test_provides_IGitRepositorySet(self): |
326 | diff --git a/lib/lp/code/tests/helpers.py b/lib/lp/code/tests/helpers.py | |||
327 | index a4f8416..4ef843d 100644 | |||
328 | --- a/lib/lp/code/tests/helpers.py | |||
329 | +++ b/lib/lp/code/tests/helpers.py | |||
330 | @@ -355,7 +355,7 @@ class GitHostingFixture(fixtures.Fixture): | |||
331 | 355 | merges=None, blob=None, disable_memcache=True): | 355 | merges=None, blob=None, disable_memcache=True): |
332 | 356 | self.create = FakeMethod() | 356 | self.create = FakeMethod() |
333 | 357 | self.getProperties = FakeMethod( | 357 | self.getProperties = FakeMethod( |
335 | 358 | result={"default_branch": default_branch}) | 358 | result={"default_branch": default_branch, "is_available": True}) |
336 | 359 | self.setProperties = FakeMethod() | 359 | self.setProperties = FakeMethod() |
337 | 360 | self.getRefs = FakeMethod(result=({} if refs is None else refs)) | 360 | self.getRefs = FakeMethod(result=({} if refs is None else refs)) |
338 | 361 | self.getCommits = FakeMethod( | 361 | self.getCommits = FakeMethod( |
339 | diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py | |||
340 | index 0c3a2f1..c148acc 100644 | |||
341 | --- a/lib/lp/code/xmlrpc/git.py | |||
342 | +++ b/lib/lp/code/xmlrpc/git.py | |||
343 | @@ -627,6 +627,15 @@ class GitAPI(LaunchpadXMLRPCView): | |||
344 | 627 | if requester == LAUNCHPAD_ANONYMOUS: | 627 | if requester == LAUNCHPAD_ANONYMOUS: |
345 | 628 | requester = None | 628 | requester = None |
346 | 629 | 629 | ||
347 | 630 | if naked_repo.status != GitRepositoryStatus.CREATING: | ||
348 | 631 | raise faults.Unauthorized() | ||
349 | 632 | |||
350 | 633 | if requester == LAUNCHPAD_SERVICES and "macaroon" not in auth_params: | ||
351 | 634 | # For repo creation management operations, we trust | ||
352 | 635 | # LAUNCHPAD_SERVICES, since it should be just an internal call | ||
353 | 636 | # to confirm/abort repository creation. | ||
354 | 637 | return | ||
355 | 638 | |||
356 | 630 | verified = self._verifyAuthParams(requester, repository, auth_params) | 639 | verified = self._verifyAuthParams(requester, repository, auth_params) |
357 | 631 | if verified is not None and verified.user is NO_USER: | 640 | if verified is not None and verified.user is NO_USER: |
358 | 632 | # For internal-services authentication, we check if its using a | 641 | # For internal-services authentication, we check if its using a |
359 | @@ -642,9 +651,6 @@ class GitAPI(LaunchpadXMLRPCView): | |||
360 | 642 | if requester != naked_repo.registrant: | 651 | if requester != naked_repo.registrant: |
361 | 643 | raise faults.Unauthorized() | 652 | raise faults.Unauthorized() |
362 | 644 | 653 | ||
363 | 645 | if naked_repo.status != GitRepositoryStatus.CREATING: | ||
364 | 646 | raise faults.Unauthorized() | ||
365 | 647 | |||
366 | 648 | def _confirmRepoCreation(self, requester, translated_path, auth_params): | 654 | def _confirmRepoCreation(self, requester, translated_path, auth_params): |
367 | 649 | naked_repo = removeSecurityProxy( | 655 | naked_repo = removeSecurityProxy( |
368 | 650 | getUtility(IGitLookup).getByHostingPath(translated_path)) | 656 | getUtility(IGitLookup).getByHostingPath(translated_path)) |
369 | diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py | |||
370 | index c3b8126..9fb9b2e 100644 | |||
371 | --- a/lib/lp/code/xmlrpc/tests/test_git.py | |||
372 | +++ b/lib/lp/code/xmlrpc/tests/test_git.py | |||
373 | @@ -401,9 +401,10 @@ class TestGitAPIMixin: | |||
374 | 401 | expected_status = GitRepositoryStatus.AVAILABLE | 401 | expected_status = GitRepositoryStatus.AVAILABLE |
375 | 402 | expected_hosting_calls = 1 | 402 | expected_hosting_calls = 1 |
376 | 403 | expected_hosting_call_args = [(repository.getInternalPath(),)] | 403 | expected_hosting_call_args = [(repository.getInternalPath(),)] |
380 | 404 | expected_hosting_call_kwargs = [ | 404 | expected_hosting_call_kwargs = [{ |
381 | 405 | {"clone_from": (cloned_from.getInternalPath() | 405 | "clone_from": (cloned_from.getInternalPath() |
382 | 406 | if cloned_from else None)}] | 406 | if cloned_from else None), |
383 | 407 | "async_create": False}] | ||
384 | 407 | 408 | ||
385 | 408 | self.assertEqual(GitRepositoryType.HOSTED, repository.repository_type) | 409 | self.assertEqual(GitRepositoryType.HOSTED, repository.repository_type) |
386 | 409 | self.assertEqual(expected_translation, translation) | 410 | self.assertEqual(expected_translation, translation) |
387 | @@ -780,6 +781,12 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory): | |||
388 | 780 | repo.status = GitRepositoryStatus.CREATING | 781 | repo.status = GitRepositoryStatus.CREATING |
389 | 781 | self.assertConfirmsRepoCreation(owner, repo) | 782 | self.assertConfirmsRepoCreation(owner, repo) |
390 | 782 | 783 | ||
391 | 784 | def test_launchpad_service_confirm_git_repository_creation(self): | ||
392 | 785 | owner = self.factory.makePerson() | ||
393 | 786 | repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) | ||
394 | 787 | repo.status = GitRepositoryStatus.CREATING | ||
395 | 788 | self.assertConfirmsRepoCreation(LAUNCHPAD_SERVICES, repo) | ||
396 | 789 | |||
397 | 783 | def test_only_requester_can_confirm_git_repository_creation(self): | 790 | def test_only_requester_can_confirm_git_repository_creation(self): |
398 | 784 | requester = self.factory.makePerson() | 791 | requester = self.factory.makePerson() |
399 | 785 | repo = removeSecurityProxy(self.factory.makeGitRepository()) | 792 | repo = removeSecurityProxy(self.factory.makeGitRepository()) |
400 | @@ -961,6 +968,12 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory): | |||
401 | 961 | repo.status = GitRepositoryStatus.CREATING | 968 | repo.status = GitRepositoryStatus.CREATING |
402 | 962 | self.assertAbortsRepoCreation(requester, repo) | 969 | self.assertAbortsRepoCreation(requester, repo) |
403 | 963 | 970 | ||
404 | 971 | def test_launchpad_service_abort_git_repository_creation(self): | ||
405 | 972 | owner = self.factory.makePerson() | ||
406 | 973 | repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) | ||
407 | 974 | repo.status = GitRepositoryStatus.CREATING | ||
408 | 975 | self.assertAbortsRepoCreation(LAUNCHPAD_SERVICES, repo) | ||
409 | 976 | |||
410 | 964 | def test_only_requester_can_abort_git_repository_creation(self): | 977 | def test_only_requester_can_abort_git_repository_creation(self): |
411 | 965 | requester = self.factory.makePerson() | 978 | requester = self.factory.makePerson() |
412 | 966 | repo = removeSecurityProxy(self.factory.makeGitRepository()) | 979 | repo = removeSecurityProxy(self.factory.makeGitRepository()) |
This MP depends on https:/ /code.launchpad .net/~pappacena /turnip/ +git/turnip/ +merge/ 386461 being approved and landed in production.