Launchpad itself

Merge ~pelpsi/launchpad:new-API-endpoint-to-select-a-git-repository-by-id into launchpad:master

  1. Git
  2. lp:~pelpsi/launchpad
  3. new-API-endpoint-to-select-a-git-repository-by-id
  4. Merge into master
Proposed by Simone Pelosi
Status: Merged
Approved by: Simone Pelosi
Approved revision: 88b1d95bba505d5d1c26ef8f3f9e939408ef2031
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pelpsi/launchpad:new-API-endpoint-to-select-a-git-repository-by-id
Merge into: launchpad:master
Diff against target: 88 lines (+55/-0)
3 files modified
lib/lp/code/interfaces/gitrepository.py (+11/-0)
lib/lp/code/model/gitrepository.py (+11/-0)
lib/lp/code/model/tests/test_gitrepository.py (+33/-0)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+439326@code.launchpad.net

Commit message

Created API endpoint to select a git repository by id

Added new API to get repository by ID.
Added three test cases for the new API.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

Looks lovely, thanks.

Just a note on something you might want to fix somewhere in your setup: your git commit messages seem to be including literal "\n" sequences for some reason, in a context where it would make more sense for it to be an actual newline character (or indeed probably two newlines).

review: Approve
Revision history for this message
Simone Pelosi (pelpsi) wrote :

> Looks lovely, thanks.
>
> Just a note on something you might want to fix somewhere in your setup: your
> git commit messages seem to be including literal "\n" sequences for some
> reason, in a context where it would make more sense for it to be an actual
> newline character (or indeed probably two newlines).

Oh you're right! I'll fix it! Thank you Colin!

Revision history for this message
Jürgen Gmach (jugmac00) wrote :

Simone, thanks a lot for working on this! This will make it much easier to identify a problematic git repository in the following context ( https://wiki.canonical.com/Launchpad/SupportRotation/HighGitLoad ), which I will update in a min.

I would love to see some type information for "id", either as a type annotation or via the docstring.

The implemenation (int) varies from the spec (string) (https://warthogs.atlassian.net/browse/LP-1048), possibly for a very good reason.

Could you please elaborate why you did choose to diverge?

Revision history for this message
Colin Watson (cjwatson) wrote :

The spec was clearly wrong. IDs are ints.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
2index 453954f..31ed38b 100644
3--- a/lib/lp/code/interfaces/gitrepository.py
4+++ b/lib/lp/code/interfaces/gitrepository.py
5@@ -1356,6 +1356,17 @@ class IGitRepositorySet(Interface):
6 :param with_hosting: Create the repository on the hosting service.
7 """
8
9+ @call_with(user=REQUEST_USER)
10+ @operation_parameters(id=Int(title=_("Repository id"), required=True))
11+ @operation_returns_entry(IGitRepository)
12+ @export_read_operation()
13+ @operation_for_version("devel")
14+ def getByID(user, id):
15+ """Find a repository by its id.
16+
17+ Return None if no match was found.
18+ """
19+
20 # Marker for references to Git URL layouts: ##GITNAMESPACE##
21 @call_with(user=REQUEST_USER)
22 @operation_parameters(
23diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
24index 8021ac4..0ffae31 100644
25--- a/lib/lp/code/model/gitrepository.py
26+++ b/lib/lp/code/model/gitrepository.py
27@@ -2231,6 +2231,17 @@ class GitRepositorySet:
28 clone_from_repository=clone_from_repository,
29 )
30
31+ def getByID(self, user, id):
32+ """See `IGitRepositorySet`."""
33+ repository = getUtility(IGitLookup).get(id)
34+ if repository is None:
35+ return None
36+ # removeSecurityProxy is safe here since we're explicitly performing
37+ # a permission check.
38+ if removeSecurityProxy(repository).visibleByUser(user):
39+ return repository
40+ return None
41+
42 def getByPath(self, user, path):
43 """See `IGitRepositorySet`."""
44 repository, extra_path = getUtility(IGitLookup).getByPath(path)
45diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
46index 216d302..2b31131 100644
47--- a/lib/lp/code/model/tests/test_gitrepository.py
48+++ b/lib/lp/code/model/tests/test_gitrepository.py
49@@ -4801,6 +4801,39 @@ class TestGitRepositorySet(TestCaseWithFactory):
50 # GitRepositorySet instances provide IGitRepositorySet.
51 verifyObject(IGitRepositorySet, self.repository_set)
52
53+ def test_getByID(self):
54+ # getByID returns a repository matching the id that it's given.
55+ a = self.factory.makeGitRepository()
56+ self.factory.makeGitRepository()
57+ repository = self.repository_set.getByID(a.owner, a.id)
58+ self.assertEqual(a, repository)
59+
60+ def test_getByID_not_found(self):
61+ # If a repository cannot be found for a given id, then getById returns
62+ # None.
63+ a = self.factory.makeGitRepository()
64+ self.factory.makeGitRepository()
65+ repository = self.repository_set.getByID(a.owner, -1)
66+ self.assertIsNone(repository)
67+
68+ def test_getByID_inaccessible(self):
69+ # If the given user cannot view the matched repository, then
70+ # getByID returns None.
71+ owner = self.factory.makePerson()
72+ repository = self.factory.makeGitRepository(
73+ owner=owner, information_type=InformationType.USERDATA
74+ )
75+ with person_logged_in(owner):
76+ repository_id = repository.id
77+ self.assertEqual(
78+ repository, self.repository_set.getByID(owner, repository_id)
79+ )
80+ self.assertIsNone(
81+ self.repository_set.getByID(
82+ self.factory.makePerson(), repository_id
83+ )
84+ )
85+
86 def test_getByPath(self):
87 # getByPath returns a repository matching the path that it's given.
88 a = self.factory.makeGitRepository()

Subscribers

People subscribed via source and target branches

to status/vote changes: