From c48f866fcf32b4d7f95743060ce841c0d3c34ee9 Mon Sep 17 00:00:00 2001
From: Devon Loehr <dloehr@google.com>
Date: Tue, 4 Mar 2025 12:19:30 -0800
Subject: [PATCH] Select a minimal number of owners for a set of files

This CL adds a function which takes a set of files, and attempts to
select a single owner for all of them. If it cannot, it falls back to
the standard owner selection algorithm, which may result in more owners
being chosen than necessary, but guarantees that a valid set of owners
is always returned.

Bug: 389069356
Change-Id: I985804040f149a02bfb5b3c6b946602a81334e7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6321289
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Devon Loehr <dloehr@google.com>
---
 owners_client.py            | 35 +++++++++++++++++++++++++++++++++++
 tests/owners_client_test.py | 27 +++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/owners_client.py b/owners_client.py
index 4a0fdba67..1ea13d966 100644
--- a/owners_client.py
+++ b/owners_client.py
@@ -116,6 +116,41 @@ class OwnersClient(object):
 
         return selected
 
+    def SuggestMinimalOwners(self,
+                             paths: list[str],
+                             exclude: list[str] = None) -> list[str]:
+        """
+        Suggest a set of owners for the given paths. Never return an owner in
+        the |exclude| list.
+
+        Aims to provide only one, but will provide more if it's unable to
+        find a common owner.
+        """
+        exclude = exclude or []
+
+        owners_by_path = self.BatchListOwners(paths)
+        if not owners_by_path:
+            return []
+
+        common_owners = set(owners_by_path.popitem()[1]) - set(exclude)
+        for _, owners in owners_by_path.items():
+            common_owners = common_owners.intersection(set(owners))
+
+        if not common_owners:
+            # This likely means some of the files had `noparent` set.
+            # Fall back to the default suggestion algorithm, which accounts
+            # for noparent but is liable to return many different owners
+            return self.SuggestOwners(paths, exclude)
+
+        # Return an arbitrary common owner, preferring those with a good score
+        sorted_common_owners = [
+            owner for owner in self.ScoreOwners(paths, exclude=exclude)
+            if owner in common_owners
+        ]
+
+        # Return a singleton list so this function has a consistent return type
+        return sorted_common_owners[:1]
+
 
 class GerritClient(OwnersClient):
     """Implement OwnersClient using OWNERS REST API."""
diff --git a/tests/owners_client_test.py b/tests/owners_client_test.py
index 0513299e3..7efd71e7c 100755
--- a/tests/owners_client_test.py
+++ b/tests/owners_client_test.py
@@ -244,6 +244,33 @@ class OwnersClientTest(unittest.TestCase):
             self.client.BatchListOwners(
                 ['bar/everyone/foo.txt', 'bar/everyone/bar.txt', 'bar/foo/']))
 
+    def testSuggestMinimalOwners(self):
+        self.client.owners_by_path = {
+            'bar/everyone/foo.txt': [alice, bob, emily],
+            'bar/everyone/bar.txt': [bob, emily],
+            'bar/foo/': [bob, chris, emily],
+            'baz/baz/baz': [chris, dave, emily]
+        }
+
+        self.assertEqual([bob],
+                         self.client.SuggestMinimalOwners([
+                             'bar/everyone/foo.txt', 'bar/everyone/bar.txt',
+                             'bar/foo/'
+                         ]))
+
+        self.assertEqual([chris],
+                         self.client.SuggestMinimalOwners(
+                             ['bar/foo/', 'baz/baz/baz']))
+
+        # If no common owner exists, fallback to returning multiple owners
+        self.assertEqual([alice, bob, chris],
+                         self.client.SuggestMinimalOwners([
+                             'bar/everyone/foo.txt', 'bar/everyone/bar.txt',
+                             'bar/foo/', 'baz/baz/baz'
+                         ],
+                                                          exclude=[emily]))
+
+
 
 class GetCodeOwnersClientTest(unittest.TestCase):
     def setUp(self):