Revert "[owners] Refactor OwnersClient."

This reverts commit 9c7f6c25c0.

Reason for revert:
Not working
https://logs.chromium.org/logs/infra-internal/buildbucket/cr-buildbucket.appspot.com/8858807889610091152/+/steps/franky/0/steps/git_cl_upload/0/stdout

Original change's description:
> [owners] Refactor OwnersClient.
>
> - Remove GetChangeApprovalStatus and GetFilesApprovalStatus.
> - Make host, project and branch part of GerritClient.
> - Rename ListOwnersForFile to ListOwners.
>
> Change-Id: I1f610a64f0217ce54e5ce4a210026535adc1c2e5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2587268
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Gavin Mak <gavinmak@google.com>

TBR=ehmaldonado@chromium.org,gavinmak@google.com,infra-scoped@luci-project-accounts.iam.gserviceaccount.com,sokcevic@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I4928df90856526210a4fd4d104e6cc96aa005bc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2613925
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>

owners_client.py

@@ -5,7 +5,6 @@
 import itertools
 import os
 import random
-import threading
 
 import gerrit_util
 import git_common
@@ -40,6 +39,10 @@ def _owner_combinations(owners, num_owners):
   return reversed(list(itertools.combinations(reversed(owners), num_owners)))
 
 
+class InvalidOwnersConfig(Exception):
+  pass
+
+
 class OwnersClient(object):
   """Interact with OWNERS files in a repository.
 
@@ -47,30 +50,46 @@ class OwnersClient(object):
   Gerrit Code-Owners plugin REST API, and the owners database implemented by
   Depot Tools in owners.py:
 
-   - List all the owners for a group of files.
-   - Check if files have been approved.
-   - Suggest owners for a group of files.
+   - List all the owners for a change.
+   - Check if a change has been approved.
+   - Check if the OWNERS configuration in a change is valid.
 
   All code should use this class to interact with OWNERS files instead of the
   owners database in owners.py
   """
-  def ListOwners(self, path):
+  def __init__(self, host):
+    self._host = host
+
+  def ListOwnersForFile(self, project, branch, path):
     """List all owners for a file.
 
     The returned list is sorted so that better owners appear first.
     """
     raise Exception('Not implemented')
 
-  def BatchListOwners(self, paths):
-    """List all owners for a group of files.
-
-    Returns a dictionary {path: [owners]}.
-    """
+  def BatchListOwners(self, project, branch, paths):
+    """Returns a dictionary {path: [owners]}."""
     with git_common.ScopedPool(kind='threads') as pool:
       return dict(pool.imap_unordered(
-          lambda p: (p, self.ListOwners(p)), paths))
+          lambda p: (p, self.ListOwnersForFile(project, branch, p)), paths))
 
-  def GetFilesApprovalStatus(self, paths, approvers, reviewers):
+  def GetChangeApprovalStatus(self, change_id):
+    """Check the approval status for the latest revision_id in a change.
+
+    Returns a map of path to approval status, where the status can be one of:
+    - APPROVED: An owner of the file has reviewed the change.
+    - PENDING:  An owner of the file has been added as a reviewer, but no owner
+      has approved.
+    - INSUFFICIENT_REVIEWERS: No owner of the file has been added as a reviewer.
+    """
+    raise Exception('Not implemented')
+
+  def ValidateOwnersConfig(self, change_id):
+    """Check if the owners configuration in a change is valid."""
+    raise Exception('Not implemented')
+
+  def GetFilesApprovalStatus(
+      self, project, branch, paths, approvers, reviewers):
     """Check the approval status for the given paths.
 
     Utility method to check for approval status when a change has not yet been
@@ -81,23 +100,22 @@ class OwnersClient(object):
     approvers = set(approvers)
     reviewers = set(reviewers)
     status = {}
-    owners_by_path = self.BatchListOwners(paths)
-    for path, owners in owners_by_path.items():
-      owners = set(owners)
-      if owners.intersection(approvers):
+    for path in paths:
+      path_owners = set(self.ListOwnersForFile(project, branch, path))
+      if path_owners.intersection(approvers):
         status[path] = APPROVED
-      elif owners.intersection(reviewers):
+      elif path_owners.intersection(reviewers):
         status[path] = PENDING
       else:
         status[path] = INSUFFICIENT_REVIEWERS
     return status
 
-  def SuggestOwners(self, paths):
+  def SuggestOwners(self, project, branch, paths):
     """Suggest a set of owners for the given paths."""
     paths_by_owner = {}
     score_by_owner = {}
-    owners_by_path = self.BatchListOwners(paths)
-    for path, owners in owners_by_path.items():
+    for path in paths:
+      owners = self.ListOwnersForFile(project, branch, path)
       for i, owner in enumerate(owners):
         paths_by_owner.setdefault(owner, set()).add(path)
         # Gerrit API lists owners of a path sorted by an internal score, so
@@ -106,10 +124,8 @@ class OwnersClient(object):
         # paths.
         score_by_owner[owner] = min(i, score_by_owner.get(owner, i))
 
-    # Sort owners by their score. Randomize order of owners with same score.
-    owners = sorted(
-        score_by_owner,
-        key=lambda o: (score_by_owner[o], random.random()))
+    # Sort owners by their score.
+    owners = sorted(score_by_owner, key=lambda o: score_by_owner[o])
 
     # Select the minimum number of owners that can approve all paths.
     # We start at 2 to avoid sending all changes that require multiple reviewers
@@ -123,22 +139,19 @@ class OwnersClient(object):
       for selected in _owner_combinations(owners, num_owners):
         covered = set.union(*(paths_by_owner[o] for o in selected))
         if len(covered) == len(paths):
-          return list(selected)
+          return selected
 
 
 class DepotToolsClient(OwnersClient):
   """Implement OwnersClient using owners.py Database."""
-  def __init__(self, root, branch, fopen=open, os_path=os.path):
-    super(DepotToolsClient, self).__init__()
-
+  def __init__(self, host, root, branch, fopen=open, os_path=os.path):
+    super(DepotToolsClient, self).__init__(host)
     self._root = root
-    self._branch = branch
     self._fopen = fopen
     self._os_path = os_path
-
+    self._branch = branch
     self._db = owners_db.Database(root, fopen, os_path)
     self._db.override_files = self._GetOriginalOwnersFiles()
-    self._db_lock = threading.Lock()
 
   def _GetOriginalOwnersFiles(self):
     return {
@@ -147,33 +160,57 @@ class DepotToolsClient(OwnersClient):
       if os.path.basename(f) == 'OWNERS'
     }
 
-  def ListOwners(self, path):
-    # all_possible_owners is not thread safe.
-    with self._db_lock:
-      # all_possible_owners returns a dict {owner: [(path, distance)]}. We want
-      # to return a list of owners sorted by increasing distance.
-      distance_by_owner = self._db.all_possible_owners([path], None)
-      # We add a small random number to the distance, so that owners at the same
-      # distance are returned in random order to avoid overloading those who
-      # would appear first.
-      return sorted(
-          distance_by_owner,
-          key=lambda o: distance_by_owner[o][0][1] + random.random())
+  def ListOwnersForFile(self, _project, _branch, path):
+    # all_possible_owners returns a dict {owner: [(path, distance)]}. We want to
+    # return a list of owners sorted by increasing distance.
+    distance_by_owner = self._db.all_possible_owners([path], None)
+    # We add a small random number to the distance, so that owners at the same
+    # distance are returned in random order to avoid overloading those who would
+    # appear first.
+    return sorted(
+        distance_by_owner,
+        key=lambda o: distance_by_owner[o][0][1] + random.random())
+
+  def GetChangeApprovalStatus(self, change_id):
+    data = gerrit_util.GetChange(
+        self._host, change_id,
+        ['DETAILED_ACCOUNTS', 'DETAILED_LABELS', 'CURRENT_FILES',
+         'CURRENT_REVISION'])
+
+    reviewers = [r['email'] for r in data['reviewers']['REVIEWER']]
+
+    # Get reviewers that have approved this change
+    label = data['labels']['Code-Review']
+    max_value = max(int(v) for v in label['values'])
+    approvers = [v['email'] for v in label['all'] if v['value'] == max_value]
+
+    files = data['revisions'][data['current_revision']]['files']
+    return self.GetFilesApprovalStatus(None, None, files, approvers, reviewers)
+
+  def ValidateOwnersConfig(self, change_id):
+    data = gerrit_util.GetChange(
+        self._host, change_id,
+        ['DETAILED_ACCOUNTS', 'DETAILED_LABELS', 'CURRENT_FILES',
+         'CURRENT_REVISION'])
+
+    files = data['revisions'][data['current_revision']]['files']
+
+    db = owners_db.Database(self._root, self._fopen, self._os_path)
+    try:
+      db.load_data_needed_for(
+          [f for f in files if os.path.basename(f) == 'OWNERS'])
+    except Exception as e:
+      raise InvalidOwnersConfig('Error parsing OWNERS files:\n%s' % e)
 
 
 class GerritClient(OwnersClient):
   """Implement OwnersClient using OWNERS REST API."""
-  def __init__(self, host, project, branch):
-    super(GerritClient, self).__init__()
+  def __init__(self, host):
+    super(GerritClient, self).__init__(host)
 
-    self._host = host
-    self._project = project
-    self._branch = branch
-
-  def ListOwners(self, path):
+  def ListOwnersForFile(self, project, branch, path):
     # GetOwnersForFile returns a list of account details sorted by order of
     # best reviewer for path. If code owners have the same score, the order is
     # random.
-    data = gerrit_util.GetOwnersForFile(
-        self._host, self._project, self._branch, path)
+    data = gerrit_util.GetOwnersForFile(self._host, project, branch, path)
     return [d['account']['email'] for d in data]