Merge ~pappacena/launchpad:repo-creation-on-git-push into launchpad:master

Proposed by Thiago F. Pappacena on 2020-06-05
Status: Merged
Approved by: Thiago F. Pappacena on 2020-07-09
Approved revision: c19e6ff5878497acc8dfeb205537a5e45f5613dc
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:repo-creation-on-git-push
Merge into: launchpad:master
Prerequisite: ~pappacena/launchpad:gitrepo-status
Diff against target: 383 lines (+143/-40)
7 files modified
lib/lp/code/interfaces/gitnamespace.py (+1/-1)
lib/lp/code/interfaces/gitrepository.py (+3/-0)
lib/lp/code/model/gitnamespace.py (+3/-21)
lib/lp/code/model/gitrepository.py (+25/-0)
lib/lp/code/xmlrpc/git.py (+39/-6)
lib/lp/code/xmlrpc/tests/test_git.py (+64/-11)
lib/lp/xmlrpc/faults.py (+8/-1)
Reviewer Review Type Date Requested Status
Colin Watson Approve on 2020-07-08
Ioana Lasc 2020-06-05 Approve on 2020-06-22
Review via email: mp+385231@code.launchpad.net

Commit message

Preventing repo creation HTTP call to code hosting, and returning on translatePath call the information that the requested git path should be created on code hosting.

To post a comment you must log in.
Thiago F. Pappacena (pappacena) wrote :

This MP depends on https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/385208.

Also, this MP can only be landed to production after Turnip is updated and accepting this new information, and https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/385301 is also landed in production.

Ioana Lasc (ilasc) :
review: Approve
Colin Watson (cjwatson) :
review: Approve
Thiago F. Pappacena (pappacena) wrote :

Pushed the requested changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/interfaces/gitnamespace.py b/lib/lp/code/interfaces/gitnamespace.py
2index 48e6314..fd7e3e7 100644
3--- a/lib/lp/code/interfaces/gitnamespace.py
4+++ b/lib/lp/code/interfaces/gitnamespace.py
5@@ -40,7 +40,7 @@ class IGitNamespace(Interface):
6 def createRepository(repository_type, registrant, name,
7 information_type=None, date_created=None,
8 target_default=False, owner_default=False,
9- with_hosting=False):
10+ with_hosting=False, status=None):
11 """Create and return an `IGitRepository` in this namespace."""
12
13 def isNameUsed(name):
14diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
15index b09af62..189b426 100644
16--- a/lib/lp/code/interfaces/gitrepository.py
17+++ b/lib/lp/code/interfaces/gitrepository.py
18@@ -213,6 +213,9 @@ class IGitRepositoryView(IHasRecipes):
19 shortened_path = Attribute(
20 "The shortest reasonable version of the path to this repository.")
21
22+ def getClonedFrom():
23+ """Returns from which repository the given repo is a clone from."""
24+
25 @operation_parameters(
26 reviewer=Reference(
27 title=_("A person for which the reviewer status is in question."),
28diff --git a/lib/lp/code/model/gitnamespace.py b/lib/lp/code/model/gitnamespace.py
29index 155aa9a..2a358f6 100644
30--- a/lib/lp/code/model/gitnamespace.py
31+++ b/lib/lp/code/model/gitnamespace.py
32@@ -40,7 +40,6 @@ from lp.code.errors import (
33 GitRepositoryCreatorNotMemberOfOwnerTeam,
34 GitRepositoryCreatorNotOwner,
35 GitRepositoryExists,
36- GitTargetError,
37 )
38 from lp.code.interfaces.gitcollection import IAllGitRepositories
39 from lp.code.interfaces.gitnamespace import (
40@@ -76,7 +75,7 @@ class _BaseGitNamespace:
41 reviewer=None, information_type=None,
42 date_created=DEFAULT, description=None,
43 target_default=False, owner_default=False,
44- with_hosting=False):
45+ with_hosting=False, status=None):
46 """See `IGitNamespace`."""
47 repository_set = getUtility(IGitRepositorySet)
48
49@@ -91,7 +90,7 @@ class _BaseGitNamespace:
50 repository = GitRepository(
51 repository_type, registrant, self.owner, self.target, name,
52 information_type, date_created, reviewer=reviewer,
53- description=description)
54+ description=description, status=status)
55 repository._reconcileAccess()
56
57 # The owner of the repository should also be automatically subscribed
58@@ -122,24 +121,7 @@ class _BaseGitNamespace:
59 IStore(repository).flush()
60 assert repository.id is not None
61
62- # If repository has target_default, clone from default.
63- clone_from_repository = None
64- try:
65- default = repository_set.getDefaultRepository(
66- repository.target)
67- if default is not None and default.visibleByUser(registrant):
68- clone_from_repository = default
69- else:
70- default = repository_set.getDefaultRepositoryForOwner(
71- repository.owner, repository.target)
72- if (default is not None and
73- default.visibleByUser(registrant)):
74- clone_from_repository = default
75- except GitTargetError:
76- pass # Ignore Personal repositories.
77- if clone_from_repository == repository:
78- clone_from_repository = None
79-
80+ clone_from_repository = repository.getClonedFrom()
81 repository._createOnHostingService(
82 clone_from_repository=clone_from_repository)
83
84diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
85index c05b43b..77a4bbd 100644
86--- a/lib/lp/code/model/gitrepository.py
87+++ b/lib/lp/code/model/gitrepository.py
88@@ -369,6 +369,31 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
89 getUtility(IGitHostingClient).create(
90 hosting_path, clone_from=clone_from_path)
91
92+ def getClonedFrom(self):
93+ """See `IGitRepository`"""
94+ repository_set = getUtility(IGitRepositorySet)
95+ registrant = self.registrant
96+
97+ # If repository has target_default, clone from default.
98+ clone_from_repository = None
99+ try:
100+ default = repository_set.getDefaultRepository(
101+ self.target)
102+ if default is not None and default.visibleByUser(registrant):
103+ clone_from_repository = default
104+ else:
105+ default = repository_set.getDefaultRepositoryForOwner(
106+ self.owner, self.target)
107+ if (default is not None and
108+ default.visibleByUser(registrant)):
109+ clone_from_repository = default
110+ except GitTargetError:
111+ pass # Ignore Personal repositories.
112+ if clone_from_repository == self:
113+ clone_from_repository = None
114+
115+ return clone_from_repository
116+
117 @property
118 def valid_webhook_event_types(self):
119 return ["git:push:0.1", "merge-proposal:0.1"]
120diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
121index 1e00b78..0c3a2f1 100644
122--- a/lib/lp/code/xmlrpc/git.py
123+++ b/lib/lp/code/xmlrpc/git.py
124@@ -70,6 +70,7 @@ from lp.registry.interfaces.product import (
125 NoSuchProduct,
126 )
127 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
128+from lp.services.features import getFeatureFlag
129 from lp.services.macaroons.interfaces import (
130 IMacaroonIssuer,
131 NO_USER,
132@@ -84,6 +85,9 @@ from lp.xmlrpc import faults
133 from lp.xmlrpc.helpers import return_fault
134
135
136+GIT_ASYNC_CREATE_REPO = 'git.codehosting.async-create.enabled'
137+
138+
139 def _get_requester_id(auth_params):
140 """Get the requester ID from authentication parameters.
141
142@@ -215,9 +219,13 @@ class GitAPI(LaunchpadXMLRPCView):
143 raise faults.Unauthorized()
144
145 def _performLookup(self, requester, path, auth_params):
146+ """Perform a translation path lookup.
147+
148+ :return: A tuple with the repository object and a dict with
149+ translation information."""
150 repository, extra_path = getUtility(IGitLookup).getByPath(path)
151 if repository is None:
152- return None
153+ return None, None
154
155 verified = self._verifyAuthParams(requester, repository, auth_params)
156 naked_repository = removeSecurityProxy(repository)
157@@ -237,7 +245,7 @@ class GitAPI(LaunchpadXMLRPCView):
158 try:
159 hosting_path = repository.getInternalPath()
160 except Unauthorized:
161- return None
162+ return naked_repository, None
163 writable = (
164 repository.repository_type == GitRepositoryType.HOSTED and
165 check_permission("launchpad.Edit", repository))
166@@ -250,7 +258,7 @@ class GitAPI(LaunchpadXMLRPCView):
167 writable = True
168 private = repository.private
169
170- return {
171+ return naked_repository, {
172 "path": hosting_path,
173 "writable": writable,
174 "trailing": extra_path,
175@@ -348,10 +356,20 @@ class GitAPI(LaunchpadXMLRPCView):
176
177 try:
178 try:
179+ # Creates the repository on the database, but do not create
180+ # it on hosting service. It should be created by the hosting
181+ # service as a result of pathTranslate call indicating that
182+ # the repo was just created in Launchpad.
183+ if getFeatureFlag(GIT_ASYNC_CREATE_REPO):
184+ with_hosting = False
185+ status = GitRepositoryStatus.CREATING
186+ else:
187+ with_hosting = True
188+ status = GitRepositoryStatus.AVAILABLE
189 namespace.createRepository(
190 GitRepositoryType.HOSTED, requester, repository_name,
191 target_default=target_default, owner_default=owner_default,
192- with_hosting=True)
193+ with_hosting=with_hosting, status=status)
194 except LaunchpadValidationError as e:
195 # Despite the fault name, this just passes through the
196 # exception text so there's no need for a new Git-specific
197@@ -378,11 +396,26 @@ class GitAPI(LaunchpadXMLRPCView):
198 if requester == LAUNCHPAD_ANONYMOUS:
199 requester = None
200 try:
201- result = self._performLookup(requester, path, auth_params)
202+ repo, result = self._performLookup(requester, path, auth_params)
203+ if repo and repo.status == GitRepositoryStatus.CREATING:
204+ raise faults.GitRepositoryBeingCreated(path)
205+
206 if (result is None and requester is not None and
207 permission == "write"):
208 self._createRepository(requester, path)
209- result = self._performLookup(requester, path, auth_params)
210+ repo, result = self._performLookup(
211+ requester, path, auth_params)
212+
213+ # If the recently-created repo is in "CREATING" status,
214+ # it should be created asynchronously by hosting service
215+ # receiving this response. So, we must include the extra
216+ # parameter instructing code hosting service to do so.
217+ if repo.status == GitRepositoryStatus.CREATING:
218+ clone_from = repo.getClonedFrom()
219+ result["creation_params"] = {
220+ "clone_from": (clone_from.getInternalPath()
221+ if clone_from else None)
222+ }
223 if result is None:
224 raise faults.GitRepositoryNotFound(path)
225 if permission != "read" and not result["writable"]:
226diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
227index e2983a3..c3b8126 100644
228--- a/lib/lp/code/xmlrpc/tests/test_git.py
229+++ b/lib/lp/code/xmlrpc/tests/test_git.py
230@@ -46,6 +46,7 @@ from lp.code.interfaces.gitrepository import (
231 IGitRepositorySet,
232 )
233 from lp.code.tests.helpers import GitHostingFixture
234+from lp.code.xmlrpc.git import GIT_ASYNC_CREATE_REPO
235 from lp.registry.enums import TeamMembershipPolicy
236 from lp.services.config import config
237 from lp.services.features.testing import FeatureFixture
238@@ -157,6 +158,7 @@ class TestGitAPIMixin:
239 transport=XMLRPCTestTransport())
240 self.hosting_fixture = self.useFixture(GitHostingFixture())
241 self.repository_set = getUtility(IGitRepositorySet)
242+ self.useFixture(FeatureFixture({GIT_ASYNC_CREATE_REPO: True}))
243
244 def assertFault(self, expected_fault, request_id, func_name,
245 *args, **kwargs):
246@@ -366,7 +368,7 @@ class TestGitAPIMixin:
247 macaroon_raw)
248
249 def assertCreates(self, requester, path, can_authenticate=False,
250- private=False):
251+ private=False, async_create=True):
252 auth_params = _make_auth_params(
253 requester, can_authenticate=can_authenticate)
254 request_id = auth_params["request-id"]
255@@ -377,22 +379,49 @@ class TestGitAPIMixin:
256 requester, path.lstrip("/"))
257 self.assertIsNotNone(repository)
258 self.assertEqual(requester, repository.registrant)
259+
260+ cloned_from = repository.getClonedFrom()
261+ expected_translation = {
262+ "path": repository.getInternalPath(),
263+ "writable": True,
264+ "trailing": "",
265+ "private": private}
266+
267+ # This should be the case if GIT_ASYNC_CREATE_REPO feature flag is
268+ # enabled.
269+ if async_create:
270+ expected_translation["creation_params"] = {
271+ "clone_from": (cloned_from.getInternalPath() if cloned_from
272+ else None)}
273+ expected_status = GitRepositoryStatus.CREATING
274+ expected_hosting_calls = 0
275+ expected_hosting_call_args = []
276+ expected_hosting_call_kwargs = []
277+ else:
278+ expected_status = GitRepositoryStatus.AVAILABLE
279+ expected_hosting_calls = 1
280+ expected_hosting_call_args = [(repository.getInternalPath(),)]
281+ expected_hosting_call_kwargs = [
282+ {"clone_from": (cloned_from.getInternalPath()
283+ if cloned_from else None)}]
284+
285+ self.assertEqual(GitRepositoryType.HOSTED, repository.repository_type)
286+ self.assertEqual(expected_translation, translation)
287 self.assertEqual(
288- {"path": repository.getInternalPath(), "writable": True,
289- "trailing": "", "private": private},
290- translation)
291+ expected_hosting_calls, self.hosting_fixture.create.call_count)
292 self.assertEqual(
293- (repository.getInternalPath(),),
294- self.hosting_fixture.create.extract_args()[0])
295- self.assertEqual(GitRepositoryType.HOSTED, repository.repository_type)
296+ expected_hosting_call_args,
297+ self.hosting_fixture.create.extract_args())
298+ self.assertEqual(
299+ expected_hosting_call_kwargs,
300+ self.hosting_fixture.create.extract_kwargs())
301+ self.assertEqual(expected_status, repository.status)
302 return repository
303
304 def assertCreatesFromClone(self, requester, path, cloned_from,
305 can_authenticate=False):
306 self.assertCreates(requester, path, can_authenticate)
307- self.assertEqual(
308- {"clone_from": cloned_from.getInternalPath()},
309- self.hosting_fixture.create.extract_kwargs()[0])
310+ self.assertEqual(0, self.hosting_fixture.create.call_count)
311
312 def assertHasRefPermissions(self, requester, repository, ref_paths,
313 permissions, macaroon_raw=None):
314@@ -1182,7 +1211,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
315 path = u"/%s" % repository.target.name
316 self.assertTranslates(requester, path, repository, False)
317
318- def test_translatePath_create_project(self):
319+ def test_translatePath_create_project_async(self):
320 # translatePath creates a project repository that doesn't exist, if
321 # it can.
322 requester = self.factory.makePerson()
323@@ -1190,6 +1219,29 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
324 self.assertCreates(
325 requester, u"/~%s/%s/+git/random" % (requester.name, project.name))
326
327+ def test_translatePath_create_project_sync(self):
328+ self.useFixture(FeatureFixture({GIT_ASYNC_CREATE_REPO: ''}))
329+ requester = self.factory.makePerson()
330+ project = self.factory.makeProduct()
331+ self.assertCreates(
332+ requester, u"/~%s/%s/+git/random" % (requester.name, project.name),
333+ async_create=False)
334+
335+ def test_translatePath_create_project_blocks_duplicate_calls(self):
336+ # translatePath creates a project repository that doesn't exist,
337+ # but blocks any further request to create the same repository.
338+ requester = self.factory.makePerson()
339+ project = self.factory.makeProduct()
340+ path = u"/~%s/%s/+git/random" % (requester.name, project.name)
341+ self.assertCreates(requester, path)
342+
343+ auth_params = _make_auth_params(
344+ requester, can_authenticate=True)
345+ request_id = auth_params["request-id"]
346+ self.assertFault(
347+ faults.GitRepositoryBeingCreated,
348+ request_id, "translatePath", path, "write", auth_params)
349+
350 def test_translatePath_create_project_clone_from_target_default(self):
351 # translatePath creates a project repository cloned from the target
352 # default if it exists.
353@@ -1511,6 +1563,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
354 def test_translatePath_create_broken_hosting_service(self):
355 # If the hosting service is down, trying to create a repository
356 # fails and doesn't leave junk around in the Launchpad database.
357+ self.useFixture(FeatureFixture({GIT_ASYNC_CREATE_REPO: ''}))
358 self.hosting_fixture.create.failure = GitRepositoryCreationFault(
359 "nothing here", path="123")
360 requester = self.factory.makePerson()
361diff --git a/lib/lp/xmlrpc/faults.py b/lib/lp/xmlrpc/faults.py
362index 7211377..53491c3 100644
363--- a/lib/lp/xmlrpc/faults.py
364+++ b/lib/lp/xmlrpc/faults.py
365@@ -1,4 +1,4 @@
366-# Copyright 2009 Canonical Ltd. This software is licensed under the
367+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
368 # GNU Affero General Public License version 3 (see the file LICENSE).
369
370 """Launchpad XMLRPC faults."""
371@@ -387,6 +387,13 @@ class GitRepositoryNotFound(PathTranslationError):
372 msg_template = "Repository '%(path)s' not found."
373
374
375+class GitRepositoryBeingCreated(PathTranslationError):
376+ """Raised when a Git repository path is currently being created on
377+ hosting service."""
378+
379+ msg_template = "Repository '%(path)s' creation is in progress."
380+
381+
382 class InvalidPath(LaunchpadFault):
383 """Raised when `translatePath` is passed something that's not a path."""
384

Subscribers

People subscribed via source and target branches