Merge lp:~cjwatson/launchpad/git-get-by-unique-name-oops into lp:launchpad

Proposed by Colin Watson on 2018-07-23
Status: Merged
Merged at revision: 18735
Proposed branch: lp:~cjwatson/launchpad/git-get-by-unique-name-oops
Merge into: lp:launchpad
Diff against target: 76 lines (+28/-3)
2 files modified
lib/lp/code/model/gitlookup.py (+2/-2)
lib/lp/code/model/tests/test_gitlookup.py (+26/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-get-by-unique-name-oops
Reviewer Review Type Date Requested Status
William Grant code 2018-07-23 Approve on 2018-07-26
Review via email: mp+350447@code.launchpad.net

Commit message

Fix OOPS when trying to look up ~user/project:branch as a unique Git repository name.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/model/gitlookup.py'
2--- lib/lp/code/model/gitlookup.py 2015-07-09 12:18:51 +0000
3+++ lib/lp/code/model/gitlookup.py 2018-07-23 11:05:21 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2015 Canonical Ltd. This software is licensed under the
6+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Database implementation of the Git repository lookup utility."""
10@@ -347,7 +347,7 @@
11 if repository is None or trailing or list(segments):
12 raise InvalidNamespace(unique_name)
13 return repository
14- except (InvalidNamespace, NameLookupFailed):
15+ except (InvalidNamespace, InvalidProductName, NameLookupFailed):
16 pass
17 return None
18
19
20=== modified file 'lib/lp/code/model/tests/test_gitlookup.py'
21--- lib/lp/code/model/tests/test_gitlookup.py 2017-10-04 01:29:35 +0000
22+++ lib/lp/code/model/tests/test_gitlookup.py 2018-07-23 11:05:21 +0000
23@@ -1,4 +1,4 @@
24-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
25+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
26 # GNU Affero General Public License version 3 (see the file LICENSE).
27
28 """Tests for the IGitLookup implementation."""
29@@ -65,22 +65,47 @@
30 unused_name = self.factory.getUniqueString()
31 self.assertIsNone(self.lookup.getByUniqueName(unused_name))
32
33+ def test_invalid_name(self):
34+ # repository:branch forms are not valid as repository unique names.
35+ # Provoke various failure modes depending on where the invalid
36+ # component occurs in the traversal.
37+ repository = self.factory.makeGitRepository()
38+ self.assertIsNone(self.lookup.getByUniqueName(
39+ repository.unique_name + ":branch-name"))
40+ with person_logged_in(repository.owner):
41+ getUtility(IGitRepositorySet).setDefaultRepositoryForOwner(
42+ repository.owner, repository.target, repository,
43+ repository.owner)
44+ self.assertIsNone(self.lookup.getByUniqueName(
45+ repository.shortened_path + ":branch-name"))
46+ with person_logged_in(repository.target.owner):
47+ getUtility(IGitRepositorySet).setDefaultRepository(
48+ repository.target, repository)
49+ self.assertIsNone(self.lookup.getByUniqueName(
50+ repository.shortened_path + ":branch-name"))
51+
52 def test_project(self):
53 repository = self.factory.makeGitRepository()
54 self.assertEqual(
55 repository, self.lookup.getByUniqueName(repository.unique_name))
56+ self.assertIsNone(self.lookup.getByUniqueName(
57+ repository.unique_name + "-nonexistent"))
58
59 def test_package(self):
60 dsp = self.factory.makeDistributionSourcePackage()
61 repository = self.factory.makeGitRepository(target=dsp)
62 self.assertEqual(
63 repository, self.lookup.getByUniqueName(repository.unique_name))
64+ self.assertIsNone(self.lookup.getByUniqueName(
65+ repository.unique_name + "-nonexistent"))
66
67 def test_personal(self):
68 owner = self.factory.makePerson()
69 repository = self.factory.makeGitRepository(owner=owner, target=owner)
70 self.assertEqual(
71 repository, self.lookup.getByUniqueName(repository.unique_name))
72+ self.assertIsNone(self.lookup.getByUniqueName(
73+ repository.unique_name + "-nonexistent"))
74
75
76 class TestGetByPath(TestCaseWithFactory):