Get code owners REST endpoints: Add option whether to resolve all users
If code ownership is assigned to all users, we currently fill up the
response with random users until the requested limit is reached. Add a
new request option that allows callers to control whether all users
should be resolved to random users.
Since the response now contains an explicit flag telling callers if a
path is owned by all users, resolving all users to random users may not
be wanted anymore.
By default all users are resolved to random users so that the current
behaviour is kept.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I8d05e936a32d2ad173dc8a868d352571cecdde9f
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java
index 674d4a5..5bc89e4 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwners.java
@@ -47,6 +47,7 @@
private Integer limit;
private String revision;
private Long seed;
+ private Boolean resolveAllUsers;
/**
* Lists the code owners for the given path.
@@ -103,6 +104,18 @@
}
/**
+ * Sets whether code ownerships that are assigned to all users should be resolved to random
+ * users.
+ *
+ * @param resolveAllUsers whether code ownerships that are assigned to all users should be
+ * resolved to random users
+ */
+ public QueryRequest setResolveAllUsers(boolean resolveAllUsers) {
+ this.resolveAllUsers = resolveAllUsers;
+ 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.
@@ -129,6 +142,13 @@
return Optional.ofNullable(seed);
}
+ /**
+ * Whether code ownerships that are assigned to all users should be resolved to random users.
+ */
+ public Optional<Boolean> getResolveAllUsers() {
+ return Optional.ofNullable(resolveAllUsers);
+ }
+
/** 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 da27858..9fba599 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
@@ -58,6 +58,7 @@
getOptions().forEach(getCodeOwners::addOption);
getLimit().ifPresent(getCodeOwners::setLimit);
getSeed().ifPresent(getCodeOwners::setSeed);
+ getResolveAllUsers().ifPresent(getCodeOwners::setResolveAllUsers);
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 16bbd78..ed2bad9 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
@@ -63,6 +63,7 @@
getOptions().forEach(getCodeOwners::addOption);
getLimit().ifPresent(getCodeOwners::setLimit);
getSeed().ifPresent(getCodeOwners::setSeed);
+ getResolveAllUsers().ifPresent(getCodeOwners::setResolveAllUsers);
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 1464ecb..d66ef51 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -85,6 +85,7 @@
private int limit = DEFAULT_LIMIT;
private Optional<Long> seed = Optional.empty();
+ private boolean resolveAllUsers = true;
@Option(
name = "-o",
@@ -117,6 +118,15 @@
this.seed = Optional.of(seed);
}
+ @Option(
+ name = "--resolve-all-users",
+ usage =
+ "whether code ownerships that are assigned to all users should be resolved to random"
+ + " users")
+ public void setResolveAllUsers(boolean resolveAllUsers) {
+ this.resolveAllUsers = resolveAllUsers;
+ }
+
protected AbstractGetCodeOwnersForPath(
AccountVisibility accountVisibility,
Accounts accounts,
@@ -381,10 +391,13 @@
*
* <p>Must be only used to complete the suggestion list when it is found that the path is owned by
* all user.
+ *
+ * <p>No-op if code ownership for all users should not be resolved.
*/
private void fillUpWithRandomUsers(R rsrc, Set<CodeOwner> codeOwners, int limit) {
- if (codeOwners.size() >= limit) {
- // limit is already reach, we don't need to add further suggestions
+ if (!resolveAllUsers || codeOwners.size() >= limit) {
+ // code ownership for all users should not be resolved or the limit has already been reached
+ // so that we don't need to add further suggestions
return;
}
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 cd57b24..fb51e53 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
@@ -653,6 +653,12 @@
.hasAccountIdThat()
.isAnyOf(user.id(), user2.id(), admin.id());
assertThat(codeOwnersInfo).hasOwnedByAllUsersThat().isTrue();
+
+ // Query code owners without resolving all users.
+ codeOwnersInfo =
+ queryCodeOwners(getCodeOwnersApi().query().setResolveAllUsers(false), "/foo/bar/baz.md");
+ assertThat(codeOwnersInfo).hasCodeOwnersThat().isEmpty();
+ assertThat(codeOwnersInfo).hasOwnedByAllUsersThat().isTrue();
}
@Test
@@ -1120,6 +1126,37 @@
}
@Test
+ public void getAllUsersAsCodeOwners_withoutResolvingAllUsers() throws Exception {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(user.email())
+ .addCodeOwnerEmail("*")
+ .create();
+
+ // If resolveAllUsers = false, only 'user' should be returned as code owner.
+ CodeOwnersInfo codeOwnersInfo =
+ queryCodeOwners(getCodeOwnersApi().query().setResolveAllUsers(false), "/foo/bar/baz.md");
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user.id());
+ assertThat(codeOwnersInfo).hasOwnedByAllUsersThat().isTrue();
+
+ // If resolveAllUsers = true, the result includes 'admin' in addition to 'user' which has code
+ // ownership explicitly assigned.
+ codeOwnersInfo =
+ queryCodeOwners(getCodeOwnersApi().query().setResolveAllUsers(true), "/foo/bar/baz.md");
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user.id(), admin.id());
+ assertThat(codeOwnersInfo).hasOwnedByAllUsersThat().isTrue();
+ }
+
+ @Test
public void getAllUsersAsCodeOwners_explicitlyMentionedCodeOwnersArePreferred() throws Exception {
TestAccount user2 = accountCreator.user2();
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index df7d90e..dd9dbcf 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -419,6 +419,7 @@
| `O` | optional | [Account option](../../../Documentation/rest-api-accounts.html#query-options) in hex. For the explanation see `o` parameter.
| `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, `true` 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
