diff --git a/gerrit_util.py b/gerrit_util.py index 0be70e8e7a..7704e23879 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -779,13 +779,13 @@ def IsCodeOwnersEnabled(host): def GetOwnersForFile(host, project, branch, path, limit=100, - o_params=('DETAILS',)): + resolve_all_users=True, o_params=('DETAILS',)): """Gets information about owners attached to a file.""" path = 'projects/%s/branches/%s/code_owners/%s' % ( urllib.parse.quote(project, ''), urllib.parse.quote(branch, ''), urllib.parse.quote(path, '')) - q = [] + q = ['resolve-all-users=%s' % json.dumps(resolve_all_users)] if limit: q.append('n=%d' % limit) if o_params: diff --git a/owners_client.py b/owners_client.py index 14c1bf5ef0..cdb42bd115 100644 --- a/owners_client.py +++ b/owners_client.py @@ -215,11 +215,17 @@ class GerritClient(OwnersClient): # best reviewer for path. If owners have the same score, the order is # random. data = gerrit_util.GetOwnersForFile( - self._host, self._project, self._branch, path) + self._host, self._project, self._branch, path, + resolve_all_users=False) self._owners_cache[path] = [ d['account']['email'] for d in data['code_owners'] + if 'account' in d and 'email' in d['account'] ] + # If owned_by_all_users is true, add everyone as an owner at the end of + # the owners list. + if data.get('owned_by_all_users', False): + self._owners_cache[path].append(self.EVERYONE) return self._owners_cache[path] diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py index 7695f9ccd0..40ee8214d8 100644 --- a/tests/owners_client_test.py +++ b/tests/owners_client_test.py @@ -58,6 +58,9 @@ class DepotToolsClientTest(unittest.TestCase): class GerritClientTest(unittest.TestCase): def setUp(self): self.client = owners_client.GerritClient('host', 'project', 'branch') + self.addCleanup(mock.patch.stopall) + + def testListOwners(self): mock.patch( 'gerrit_util.GetOwnersForFile', return_value={ @@ -76,12 +79,12 @@ class GerritClientTest(unittest.TestCase): "account": { "email": 'missing@example.com' }, + }, + { + "account": {}, } ] }).start() - self.addCleanup(mock.patch.stopall) - - def testListOwners(self): self.assertEquals( ['approver@example.com', 'reviewer@example.com', 'missing@example.com'], self.client.ListOwners(os.path.join('bar', 'everyone', 'foo.txt'))) @@ -92,7 +95,41 @@ class GerritClientTest(unittest.TestCase): self.client.ListOwners(os.path.join('bar', 'everyone', 'foo.txt'))) # Always use slashes as separators. gerrit_util.GetOwnersForFile.assert_called_once_with( - 'host', 'project', 'branch', 'bar/everyone/foo.txt') + 'host', 'project', 'branch', 'bar/everyone/foo.txt', + resolve_all_users=False) + + def testListOwnersOwnedByAll(self): + mock.patch( + 'gerrit_util.GetOwnersForFile', + side_effect=[ + { + "code_owners": [ + { + "account": { + "email": 'foo@example.com' + }, + }, + ], + "owned_by_all_users": True, + }, + { + "code_owners": [ + { + "account": { + "email": 'bar@example.com' + }, + }, + ], + "owned_by_all_users": False, + }, + ] + ).start() + self.assertEquals( + ['foo@example.com', self.client.EVERYONE], + self.client.ListOwners('foo.txt')) + self.assertEquals( + ['bar@example.com'], + self.client.ListOwners('bar.txt')) class TestClient(owners_client.OwnersClient):