launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-repository-getRule into lp:launchpad.

Commit message:
Add IGitRepository.getRule.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1517559 in Launchpad itself: "git fine-grained permissions"
  https://bugs.launchpad.net/launchpad/+bug/1517559

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-repository-getRule/+merge/357625

This may be handy for the UI (GitRef:+permissions will want to distinguish a rule that matches the ref path exactly), but it also seems like a reasonable thing to have in general.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-repository-getRule into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2018-10-22 10:37:03 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2018-10-22 12:52:35 +0000
@@ -622,6 +622,13 @@
         :return: The diff as a binary string.
         """
 
+    def getRule(ref_pattern):
+        """Get the access rule for this repository with a given pattern.
+
+        :param ref_pattern: The reference pattern that the rule should have.
+        :return: An `IGitRule`, or None.
+        """
+
     def getActivity(changed_after=None):
         """Get activity log entries for this repository.
 

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2018-10-16 15:29:37 +0000
+++ lib/lp/code/model/gitref.py	2018-10-22 12:52:35 +0000
@@ -434,9 +434,7 @@
 
     def setGrants(self, grants, user):
         """See `IGitRef`."""
-        rule = Store.of(self).find(
-            GitRule, GitRule.repository_id == self.repository_id,
-            GitRule.ref_pattern == self.path).one()
+        rule = self.repository.getRule(self.path)
         if rule is None:
             # We don't need to worry about position, since this is an
             # exact-match rule and therefore has a canonical position.

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2018-10-22 10:37:03 +0000
+++ lib/lp/code/model/gitrepository.py	2018-10-22 12:52:35 +0000
@@ -1156,6 +1156,10 @@
             if rule.position != position:
                 removeSecurityProxy(rule).position = position
 
+    def getRule(self, ref_pattern):
+        """See `IGitRepository`."""
+        return self.rules.find(GitRule.ref_pattern == ref_pattern).one()
+
     def addRule(self, ref_pattern, creator, position=None):
         """See `IGitRepository`."""
         rules = list(self.rules)

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2018-10-22 10:37:03 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2018-10-22 12:52:35 +0000
@@ -2375,6 +2375,20 @@
                 ref_pattern="refs/heads/stable/*"),
             ]))
 
+    def test_getRule(self):
+        repository = self.factory.makeGitRepository()
+        self.factory.makeGitRefs(
+            repository=repository, paths=["refs/heads/master"])
+        other_repository = self.factory.makeGitRepository()
+        master_rule = self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/master")
+        self.factory.makeGitRule(
+            repository=repository, ref_pattern="refs/heads/*")
+        self.factory.makeGitRule(
+            repository=other_repository, ref_pattern="refs/heads/master")
+        self.assertEqual(master_rule, repository.getRule("refs/heads/master"))
+        self.assertIsNone(repository.getRule("refs/heads/other"))
+
     def test_addRule_append(self):
         repository = self.factory.makeGitRepository()
         initial_rule = self.factory.makeGitRule(