diff --git a/gerrit_util.py b/gerrit_util.py index 6ad84a994..d97b73bcd 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -26,6 +26,7 @@ import time import urllib import urlparse from cStringIO import StringIO +from multiprocessing.pool import ThreadPool import auth import gclient_utils @@ -932,11 +933,34 @@ def GetAccountDetails(host, account_id='self'): Documentation: https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#get-account + + Returns None if account is not found (i.e., Gerrit returned 404). """ - if account_id != 'self': - account_id = int(account_id) conn = CreateHttpConn(host, '/accounts/%s' % account_id) - return ReadHttpJsonResponse(conn) + return ReadHttpJsonResponse(conn, accept_statuses=[200, 404]) + + +def ValidAccounts(host, accounts, max_threads=10): + """Returns a mapping from valid account to its details. + + Invalid accounts, either not existing or without unique match, + are not present as returned dictionary keys. + """ + assert not isinstance(accounts, basestring), type(accounts) + accounts = list(set(accounts)) + if not accounts: + return {} + def get_one(account): + try: + return account, GetAccountDetails(host, account) + except GerritError: + return None, None + valid = {} + with contextlib.closing(ThreadPool(min(max_threads, len(accounts)))) as pool: + for account, details in pool.map(get_one, accounts): + if account and details: + valid[account] = details + return valid def PercentEncodeForGitRef(original): diff --git a/git_cl.py b/git_cl.py index 1fc3139bb..24be0eb59 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3122,7 +3122,6 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): change_desc.update_reviewers(options.reviewers, options.tbrs, options.add_owners_to, change) - # TODO(tandrii): process reviewers and ccs into refspec. reviewers = sorted(change_desc.get_reviewers()) # Add cc's from the CC_LIST and --cc flag (if any). if not options.private and not options.no_autocc: @@ -3134,6 +3133,11 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): cc = filter(None, [email.strip() for email in cc]) if change_desc.get_cced(): cc.extend(change_desc.get_cced()) + valid_accounts = gerrit_util.ValidAccounts( + self._GetGerritHost(), reviewers + cc) + logging.debug('accounts %s are valid, %s invalid', sorted(valid_accounts), + set(reviewers + cc).difference(set(valid_accounts))) + # TODO(tandrii): add valid reviwers and ccs to push option. # Extra options that can be specified at push time. Doc: # https://gerrit-review.googlesource.com/Documentation/user-upload.html diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index b198f7cce..57b3e25ae 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -458,6 +458,43 @@ class TestGitClBasic(unittest.TestCase): finally: git_cl.gerrit_util._GERRIT_MIRROR_PREFIXES = origMirrors + def test_valid_accounts(self): + mock_per_account = { + 'u1': None, # 404, doesn't exist. + 'u2': { + '_account_id': 123124, + 'avatars': [], + 'email': 'u2@example.com', + 'name': 'User Number 2', + 'status': 'OOO', + }, + 'u3': git_cl.gerrit_util.GerritError(500, 'retries didn\'t help :('), + } + def GetAccountDetailsMock(_, account): + # Poor-man's mock library's side_effect. + v = mock_per_account.pop(account) + if isinstance(v, Exception): + raise v + return v + + original = git_cl.gerrit_util.GetAccountDetails + try: + git_cl.gerrit_util.GetAccountDetails = GetAccountDetailsMock + actual = git_cl.gerrit_util.ValidAccounts( + 'host', ['u1', 'u2', 'u3'], max_threads=1) + finally: + git_cl.gerrit_util.GetAccountDetails = original + self.assertEqual(actual, { + 'u2': { + '_account_id': 123124, + 'avatars': [], + 'email': 'u2@example.com', + 'name': 'User Number 2', + 'status': 'OOO', + }, + }) + + class TestParseIssueURL(unittest.TestCase): def _validate(self, parsed, issue=None, patchset=None, hostname=None, @@ -692,6 +729,9 @@ class TestGitCl(TestCase): staticmethod(lambda: False)) self.mock(git_cl.gerrit_util.GceAuthenticator, 'is_gce', classmethod(lambda _: False)) + self.mock(git_cl.gerrit_util, 'ValidAccounts', + lambda host, accounts: + self._mocked_call('ValidAccounts', host, accounts)) self.mock(git_cl, 'DieWithError', lambda msg, change=None: self._mocked_call(['DieWithError', msg])) # It's important to reset settings to not have inter-tests interference. @@ -1262,7 +1302,13 @@ class TestGitCl(TestCase): ref_suffix += ',m=' + title calls += [ - ((['git', 'config', 'rietveld.cc'],), ''), + ((['git', 'config', 'rietveld.cc'],), ''), + (('ValidAccounts', 'chromium-review.googlesource.com', + sorted(reviewers) + ['joe@example.com', + 'chromium-reviews+test-more-cc@chromium.org'] + cc), + { + # TODO(tandrii): add here some valid accounts and make use of them. + }), ] calls.append((