List code owners: Add option to only get code owners with the highest score
Some callers are only interested in the code owners with the highest
score. Since the score is not exposed to callers they cannot do the
filtering on their own.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ibebb6b55e5a459b38dda9169548a56f11b7549d0
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java
index 578c1c3..655ef66 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java
@@ -48,6 +48,7 @@
private String revision;
private Long seed;
private Boolean resolveAllUsers;
+ private Boolean highestScoreOnly;
/**
* Lists the code owners for the given path.
@@ -116,6 +117,17 @@
}
/**
+ * Sets whether only the code owners with the highest score should be returned.
+ *
+ * @param highestScoreOnly whether only the code owners with the highest score should be
+ * returned
+ */
+ public QueryRequest withHighestScoreOnly(boolean highestScoreOnly) {
+ this.highestScoreOnly = highestScoreOnly;
+ return this;
+ }
+
+ /**
* Sets the branch revision from which the code owner configs should be read.
*
* <p>Not supported for querying code owners for a path in a change.
@@ -149,6 +161,11 @@
return Optional.ofNullable(resolveAllUsers);
}
+ /** Whether only the code owners with the highest score should be returned. */
+ public Optional<Boolean> getHighestScoreOnly() {
+ return Optional.ofNullable(highestScoreOnly);
+ }
+
/** Returns the branch revision from which the code owner configs should be read. */
public Optional<String> getRevision() {
return Optional.ofNullable(revision);
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
index 9fba599..107fa61 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
@@ -59,6 +59,7 @@
getLimit().ifPresent(getCodeOwners::setLimit);
getSeed().ifPresent(getCodeOwners::setSeed);
getResolveAllUsers().ifPresent(getCodeOwners::setResolveAllUsers);
+ getHighestScoreOnly().ifPresent(getCodeOwners::setHighestScoreOnly);
getRevision().ifPresent(getCodeOwners::setRevision);
CodeOwnersInBranchCollection.PathResource pathInBranchResource =
codeOwnersInBranchCollection.parse(
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
index ed2bad9..b185172 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
@@ -64,6 +64,7 @@
getLimit().ifPresent(getCodeOwners::setLimit);
getSeed().ifPresent(getCodeOwners::setSeed);
getResolveAllUsers().ifPresent(getCodeOwners::setResolveAllUsers);
+ getHighestScoreOnly().ifPresent(getCodeOwners::setHighestScoreOnly);
CodeOwnersInChangeCollection.PathResource pathInChangeResource =
codeOwnersInChangeCollection.parse(
revisionResource, IdString.fromDecoded(path.toString()));
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
index acb327e..d1e6cd2 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -89,6 +89,7 @@
private int limit = DEFAULT_LIMIT;
private Optional<Long> seed = Optional.empty();
private boolean resolveAllUsers;
+ private boolean highestScoreOnly;
@Option(
name = "-o",
@@ -130,6 +131,13 @@
this.resolveAllUsers = resolveAllUsers;
}
+ @Option(
+ name = "--highest-score-only",
+ usage = "whether only code owners with the highest score should be returned")
+ public void setHighestScoreOnly(boolean highestScoreOnly) {
+ this.highestScoreOnly = highestScoreOnly;
+ }
+
protected AbstractGetCodeOwnersForPath(
AccountVisibility accountVisibility,
Accounts accounts,
@@ -236,9 +244,22 @@
ImmutableMap<CodeOwner, Double> scoredCodeOwners =
codeOwnerScorings.getScorings(immutableCodeOwners);
+ ImmutableList<CodeOwner> sortedAndLimitedCodeOwners = sortAndLimit(rsrc, scoredCodeOwners);
+
+ if (highestScoreOnly) {
+ Optional<Double> highestScore =
+ scoredCodeOwners.values().stream().max(Comparator.naturalOrder());
+ if (highestScore.isPresent()) {
+ sortedAndLimitedCodeOwners =
+ sortedAndLimitedCodeOwners.stream()
+ .filter(codeOwner -> scoredCodeOwners.get(codeOwner).equals(highestScore.get()))
+ .collect(toImmutableList());
+ }
+ }
+
CodeOwnersInfo codeOwnersInfo = new CodeOwnersInfo();
codeOwnersInfo.codeOwners =
- codeOwnerJsonFactory.create(getFillOptions()).format(sortAndLimit(rsrc, scoredCodeOwners));
+ codeOwnerJsonFactory.create(getFillOptions()).format(sortedAndLimitedCodeOwners);
codeOwnersInfo.ownedByAllUsers = ownedByAllUsers.get() ? true : null;
return Response.ok(codeOwnersInfo);
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
index 743bef0..1a659ae 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
@@ -1385,4 +1385,77 @@
.inOrder();
assertThat(codeOwnersInfo).hasOwnedByAllUsersThat().isTrue();
}
+
+ @Test
+ public void getCodeOwnersWithHighestScoreOnly() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create some code owner configs
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail(user.email())
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ CodeOwnersInfo codeOwnersInfo =
+ queryCodeOwners(
+ getCodeOwnersApi().query().withHighestScoreOnly(/* highestScoreOnly= */ false),
+ "/foo/bar/baz.md");
+ assertThat(codeOwnersInfo).hasCodeOwnersThat().hasSize(3);
+ // the first 2 code owners have the same scoring, so their order is random and we don't know
+ // which of them we get get first and second
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .element(0)
+ .hasAccountIdThat()
+ .isAnyOf(user.id(), user2.id());
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .element(1)
+ .hasAccountIdThat()
+ .isAnyOf(user.id(), user2.id());
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .element(2)
+ .hasAccountIdThat()
+ .isEqualTo(admin.id());
+
+ codeOwnersInfo =
+ queryCodeOwners(
+ getCodeOwnersApi().query().withHighestScoreOnly(/* highestScoreOnly= */ true),
+ "/foo/bar/baz.md");
+ assertThat(codeOwnersInfo).hasCodeOwnersThat().hasSize(2);
+ // the first 2 code owners have the same scoring, so their order is random and we don't know
+ // which of them we get get first and second
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .element(0)
+ .hasAccountIdThat()
+ .isAnyOf(user.id(), user2.id());
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .element(1)
+ .hasAccountIdThat()
+ .isAnyOf(user.id(), user2.id());
+ }
+
+ @Test
+ public void getCodeOwnersWithHighestScoreOnly_noCodeOwners() throws Exception {
+ CodeOwnersInfo codeOwnersInfo =
+ queryCodeOwners(
+ getCodeOwnersApi().query().withHighestScoreOnly(/* highestScoreOnly= */ true),
+ "/foo/bar/baz.md");
+ assertThat(codeOwnersInfo).hasCodeOwnersThat().isEmpty();
+ }
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 9c0245c..c937cb2 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -459,6 +459,7 @@
| `limit`\|`n` | optional | Limit defining how many code owners should be returned at most. By default 10.
| `seed` | optional | Seed, as a long value, that should be used to shuffle code owners that have the same score. Can be used to make the sort order stable across several requests, e.g. to get the same set of random code owners for different file paths that have the same code owners. Important: the sort order is only stable if the requests use the same seed **and** the same limit. In addition, the sort order is not guaranteed to be stable if new accounts are created in between the requests, or if the account visibility is changed.
| `resolve-all-users` | optional | Whether code ownerships that are assigned to all users should be resolved to random users. If not set, `false` by default. Also see the [sorting example](#sortingExample) below to see how this parameter affects the returned code owners.
+| `highest-score-only` | optional | Whether only code owners with the highest score should be returned. If not set, `false` by default.
| `revision` | optional | Revision from which the code owner configs should be read as commit SHA1. Can be used to read historic code owners from this branch, but imports from other branches or repositories as well as default and global code owners from `refs/meta/config` are still read from the current revisions. If not specified the code owner configs are read from the HEAD revision of the branch. Not supported for getting code owners for a path in a change.
As a response a [CodeOwnersInfo](#code-owners-info) entity is returned that
