git cl: find which reviewers and ccs are OK for Gerrit before upload.

This adds O(reviewers+cc) RPC to Gerrit before upload,
though done concurrently in up to 10 threads. This information isn't
used yet, but will be in follow up CLs.

R=ehmaldonado

Bug: 877717
Change-Id: I2374f249ee874a71089244309e50e8e88a3dee7d
Reviewed-on: https://chromium-review.googlesource.com/c/1242847
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

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):

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

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((