[owners] Add SuggestOwners to OwnersClient.

Gerrit API doesn't provide the score for
an owner of a path, so we can't use the
same algorithm when suggesting owners.

This change introduces a new algorithm to
select the smallest set of at least 2
owners that can approve the change.

Change-Id: If620073bdf63633f171c1480e345dbaf75e9f575
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2556479Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

owners_client.py

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import itertools
 import os
 import random
 
@@ -15,6 +16,29 @@ PENDING = 'PENDING'
 INSUFFICIENT_REVIEWERS = 'INSUFFICIENT_REVIEWERS'
 
 
+def _owner_combinations(owners, num_owners):
+  """Iterate owners combinations by decrasing score.
+
+  The score of an owner is its position on the owners list.
+  The score of a set of owners is the maximum score of all owners on the set.
+
+  Returns all combinations of up to `num_owners` sorted by decreasing score:
+    _owner_combinations(['0', '1', '2', '3'], 2) == [
+        # score 1
+        ('1', '0'),
+        # score 2
+        ('2', '0'),
+        ('2', '1'),
+        # score 3
+        ('3', '0'),
+        ('3', '1'),
+        ('3', '2'),
+    ]
+  """
+  return reversed(list(itertools.combinations(reversed(owners), num_owners)))
+
+
+
 class InvalidOwnersConfig(Exception):
   pass
 
@@ -80,6 +104,36 @@ class OwnersClient(object):
         status[path] = INSUFFICIENT_REVIEWERS
     return status
 
+  def SuggestOwners(self, project, branch, paths):
+    """Suggest a set of owners for the given paths."""
+    paths_by_owner = {}
+    score_by_owner = {}
+    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
+        # owners that appear first should be prefered.
+        # We define the score of an owner to be their minimum position in all
+        # paths.
+        score_by_owner[owner] = min(i, score_by_owner.get(owner, i))
+
+    # 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
+    # to top-level owners.
+    num_owners = 2
+    while True:
+      # Iterate all combinations of `num_owners` by decreasing score, and select
+      # the first one that covers all paths.
+      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 selected
+      num_owners += 1
+
 
 class DepotToolsClient(OwnersClient):
   """Implement OwnersClient using owners.py Database."""

tests/owners_client_test.py

@@ -14,12 +14,18 @@ else:
 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
 import gerrit_util
-import owners
 import owners_client
 
 from testing_support import filesystem_mock
 
 
+alice = 'alice@example.com'
+bob = 'bob@example.com'
+chris = 'chris@example.com'
+dave = 'dave@example.com'
+emily = 'emily@example.com'
+
+
 def _get_change():
   return {
     "labels": {
@@ -144,6 +150,74 @@ class OwnersClientTest(unittest.TestCase):
             'insufficient': owners_client.INSUFFICIENT_REVIEWERS,
         })
 
+  def test_owner_combinations(self):
+    owners = [alice, bob, chris, dave, emily]
+    self.assertEqual(
+        list(owners_client._owner_combinations(owners, 2)),
+        [(bob, alice),
+         (chris, alice),
+         (chris, bob),
+         (dave, alice),
+         (dave, bob),
+         (dave, chris),
+         (emily, alice),
+         (emily, bob),
+         (emily, chris),
+         (emily, dave)])
+
+  def testSuggestOwners(self):
+    self.client.owners_by_path = {'abcd': [alice, bob, chris, dave]}
+    self.assertEqual(
+        self.client.SuggestOwners('project', 'branch', ['abcd']),
+        (bob, alice))
+
+    self.client.owners_by_path = {
+        'ae': [alice, emily],
+        'be': [bob, emily],
+        'ce': [chris, emily],
+        'de': [dave, emily],
+    }
+    self.assertEqual(
+        self.client.SuggestOwners(
+            'project', 'branch', ['ae', 'be', 'ce', 'de']),
+        (emily, bob))
+
+    self.client.owners_by_path = {
+        'ad': [alice, dave],
+        'cad': [chris, alice, dave],
+        'ead': [emily, alice, dave],
+        'bd': [bob, dave],
+    }
+    self.assertEqual(
+        self.client.SuggestOwners(
+            'project', 'branch', ['ad', 'cad', 'ead', 'bd']),
+        (bob, alice))
+
+    self.client.owners_by_path = {
+        'a': [alice],
+        'b': [bob],
+        'c': [chris],
+        'ad': [alice, dave],
+    }
+    self.assertEqual(
+        self.client.SuggestOwners(
+            'project', 'branch', ['a', 'b', 'c', 'ad']),
+        (alice, chris, bob))
+
+    self.client.owners_by_path = {
+        'abc': [alice, bob, chris],
+        'acb': [alice, chris, bob],
+        'bac': [bob, alice, chris],
+        'bca': [bob, chris, alice],
+        'cab': [chris, alice, bob],
+        'cba': [chris, bob, alice]
+    }
+    self.assertEqual(
+        self.client.SuggestOwners(
+            'project', 'branch',
+            ['abc', 'acb', 'bac', 'bca', 'cab', 'cba']),
+        (chris, bob))
+
 
 if __name__ == '__main__':
   unittest.main()