Avoid loading all accounts when code ownership is assigned to all users
When a code ownership was assigned to all users by using the '*'
wildcard as email, CodeOwnerResolver was loading all accounts. Loading
all accounts requires parsing all user refs. Then we checked the
visibility of all accounts, which means that all accounts were retrieved
from the cache and, depending on the accounts visibility setting, groups
for all users had to be loaded. Doing this is too expensive and if
collection of performance logs is enabled this can lead to memory
exceeded errors because each account lookup creates performance logs
which are collected in memory.
To fix this, CodeOwnerResolver is no longer resolving '*' to all users.
Instead we return a CodeOwnerResolverResult that contains all resolved
code owners plus a flag telling whether all users are code owners.
Returning this as a flag, rather than returning all accounts, is much
cheaper, but it means that callers now must handle this case on their
own. However they can do something smarter than loading all accounts.
There are 2 callers for which this is relevant:
1. CodeOwnerApprovalCheck:
Here we need to determine whether a code owner approved the change,
or whether a code owner is a reviewer of the change. If a file is
owned by all users and the approver set is non-empty, we know that
the file is approved (since there is at least one approver and any
user is a code owner). If there is no approval but a reviewer, we
know that the status for the file is pending (since all users are a
code owner, the reviewer must be a code owner).
2. AbstractGetCodeOwnersForPath:
Here we list code owners, but listing of code owners is always done
with a limit (if no limit is specified, there is a default limit).
If all users are code owners we want to include random users until
the limit is reached. For this we asked for 2 times the number of
users that we still need to complete the suggestion list. We use the
factor 2 here because some users may still be filtered out (e.g.
because they are service users or because they cannot see the
branch). When selecting random users for the suggestion we must
ensure that the included users are visible to the caller. This means
the logic that picks the random users depends on the configured
account visibility:
* ALL:
If all accounts are visible, we can randomly pick any account. For
this we retrieve the list of all accounts (getting this should be
rather cheap) and select some random entries.
* NONE:
If no other account is visible, we return an empty list.
* SAME_GROUP / VISIBLE_GROUP:
These are tricky, because potentially we will again end up loading
all accounts and all groups until we find enough visible accounts.
To avoid this we do a best effort to find enough accounts. We pick
a random sample of users that is 3 times the limit and filter out
non-visible accounts, in the hope that the sample contained enough
visible accounts to reach our limit. If we didn't find enough
visible accounts, we return less code owners than the requested
limit, although there might be further code owners available. This
is a compromise that we have to accept in order to ensure good
performance.
In addition, if the calling user can view all accounts (via the
VIEW_ALL_ACCOUNTS global capability) we always apply the same logic as
for the ALL account visibility setting.
Change-Id: I896edd3a08230106316b327e84f18fab239f4503
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index 4ed19ce..a49620b 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -187,15 +187,13 @@
"patchSetUploader = %d, implicit approval from uploader is %s",
patchSetUploader.get(), enableImplicitApprovalFromUploader ? "enabled" : "disabled");
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds =
+ CodeOwnerResolverResult globalCodeOwners =
codeOwnerResolver
.get()
.enforceVisibility(false)
.resolve(
- codeOwnersPluginConfiguration.getGlobalCodeOwners(changeNotes.getProjectName()))
- .map(CodeOwner::accountId)
- .collect(toImmutableSet());
- logger.atFine().log("global code owner accounts = %s", globalCodeOwnerAccountIds);
+ codeOwnersPluginConfiguration.getGlobalCodeOwners(changeNotes.getProjectName()));
+ logger.atFine().log("global code owners = %s", globalCodeOwners);
// If the branch doesn't contain any code owner config file yet, we apply special logic
// (project
@@ -217,7 +215,7 @@
getFileStatus(
branch,
revision,
- globalCodeOwnerAccountIds,
+ globalCodeOwners,
enableImplicitApprovalFromUploader,
patchSetUploader,
reviewerAccountIds,
@@ -231,7 +229,7 @@
private FileCodeOwnerStatus getFileStatus(
BranchNameKey branch,
ObjectId revision,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
boolean enableImplicitApprovalFromUploader,
Account.Id patchSetUploader,
ImmutableSet<Account.Id> reviewerAccountIds,
@@ -250,7 +248,7 @@
getPathCodeOwnerStatus(
branch,
revision,
- globalCodeOwnerAccountIds,
+ globalCodeOwners,
enableImplicitApprovalFromUploader,
patchSetUploader,
reviewerAccountIds,
@@ -271,7 +269,7 @@
getPathCodeOwnerStatus(
branch,
revision,
- globalCodeOwnerAccountIds,
+ globalCodeOwners,
enableImplicitApprovalFromUploader,
patchSetUploader,
reviewerAccountIds,
@@ -290,7 +288,7 @@
private PathCodeOwnerStatus getPathCodeOwnerStatus(
BranchNameKey branch,
ObjectId revision,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
boolean enableImplicitApprovalFromUploader,
Account.Id patchSetUploader,
ImmutableSet<Account.Id> reviewerAccountIds,
@@ -310,7 +308,7 @@
return isBootstrapping
? getPathCodeOwnerStatusBootstrappingMode(
branch,
- globalCodeOwnerAccountIds,
+ globalCodeOwners,
enableImplicitApprovalFromUploader,
patchSetUploader,
reviewerAccountIds,
@@ -318,7 +316,7 @@
absolutePath)
: getPathCodeOwnerStatusRegularMode(
branch,
- globalCodeOwnerAccountIds,
+ globalCodeOwners,
enableImplicitApprovalFromUploader,
patchSetUploader,
revision,
@@ -336,7 +334,7 @@
*/
private PathCodeOwnerStatus getPathCodeOwnerStatusBootstrappingMode(
BranchNameKey branch,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
boolean enableImplicitApprovalFromUploader,
Account.Id patchSetUploader,
ImmutableSet<Account.Id> reviewerAccountIds,
@@ -348,13 +346,13 @@
if (isApprovedBootstrappingMode(
branch.project(),
absolutePath,
- globalCodeOwnerAccountIds,
+ globalCodeOwners,
approverAccountIds,
enableImplicitApprovalFromUploader,
patchSetUploader)) {
codeOwnerStatus = CodeOwnerStatus.APPROVED;
} else if (isPendingBootstrappingMode(
- branch.project(), absolutePath, globalCodeOwnerAccountIds, reviewerAccountIds)) {
+ branch.project(), absolutePath, globalCodeOwners, reviewerAccountIds)) {
codeOwnerStatus = CodeOwnerStatus.PENDING;
}
@@ -367,21 +365,21 @@
private boolean isApprovedBootstrappingMode(
Project.NameKey projectName,
Path absolutePath,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
ImmutableSet<Account.Id> approverAccountIds,
boolean enableImplicitApprovalFromUploader,
Account.Id patchSetUploader) {
return (enableImplicitApprovalFromUploader
&& isImplicitlyApprovedBootstrappingMode(
- projectName, absolutePath, globalCodeOwnerAccountIds, patchSetUploader))
+ projectName, absolutePath, globalCodeOwners, patchSetUploader))
|| isExplicitlyApprovedBootstrappingMode(
- projectName, absolutePath, globalCodeOwnerAccountIds, approverAccountIds);
+ projectName, absolutePath, globalCodeOwners, approverAccountIds);
}
private boolean isImplicitlyApprovedBootstrappingMode(
Project.NameKey projectName,
Path absolutePath,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
Account.Id patchSetUploader) {
if (isProjectOwner(projectName, patchSetUploader)) {
// The uploader of the patch set is a project owner and thus a code owner. This means there
@@ -393,7 +391,8 @@
return true;
}
- if (globalCodeOwnerAccountIds.contains(patchSetUploader)) {
+ if (globalCodeOwners.ownedByAllUsers()
+ || globalCodeOwners.codeOwnersAccountIds().contains(patchSetUploader)) {
// If the uploader of the patch set is a global code owner, there is an implicit code owner
// approval from the patch set uploader so that the path is automatically approved.
logger.atFine().log(
@@ -408,9 +407,10 @@
private boolean isExplicitlyApprovedBootstrappingMode(
Project.NameKey projectName,
Path absolutePath,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
ImmutableSet<Account.Id> approverAccountIds) {
- if (!Collections.disjoint(approverAccountIds, globalCodeOwnerAccountIds)) {
+ if (!Collections.disjoint(approverAccountIds, globalCodeOwners.codeOwnersAccountIds())
+ || (globalCodeOwners.ownedByAllUsers() && !approverAccountIds.isEmpty())) {
// At least one of the global code owners approved the change.
logger.atFine().log("%s was approved by a global code owner", absolutePath);
return true;
@@ -429,7 +429,7 @@
private boolean isPendingBootstrappingMode(
Project.NameKey projectName,
Path absolutePath,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
ImmutableSet<Account.Id> reviewerAccountIds) {
if (reviewerAccountIds.stream()
.anyMatch(reviewerAccountId -> isProjectOwner(projectName, reviewerAccountId))) {
@@ -438,7 +438,7 @@
return true;
}
- if (isPending(absolutePath, globalCodeOwnerAccountIds, reviewerAccountIds)) {
+ if (isPending(absolutePath, globalCodeOwners, reviewerAccountIds)) {
// At least one of the reviewers is a global code owner.
logger.atFine().log("%s is owned by a reviewer who is a global owner", absolutePath);
return true;
@@ -453,7 +453,7 @@
*/
private PathCodeOwnerStatus getPathCodeOwnerStatusRegularMode(
BranchNameKey branch,
- ImmutableSet<Account.Id> globalCodeOwnerAccountIds,
+ CodeOwnerResolverResult globalCodeOwners,
boolean enableImplicitApprovalFromUploader,
Account.Id patchSetUploader,
ObjectId revision,
@@ -467,7 +467,7 @@
if (isApproved(
absolutePath,
- globalCodeOwnerAccountIds,
+ globalCodeOwners,
approverAccountIds,
enableImplicitApprovalFromUploader,
patchSetUploader)) {
@@ -476,7 +476,7 @@
} else {
logger.atFine().log("%s was not approved by a global code owner", absolutePath);
- if (isPending(absolutePath, globalCodeOwnerAccountIds, reviewerAccountIds)) {
+ if (isPending(absolutePath, globalCodeOwners, reviewerAccountIds)) {
logger.atFine().log("%s is owned by a reviewer who is a global owner", absolutePath);
codeOwnerStatus.set(CodeOwnerStatus.PENDING);
}
@@ -486,23 +486,22 @@
revision,
absolutePath,
codeOwnerConfig -> {
- ImmutableSet<Account.Id> codeOwnerAccountIds =
- getCodeOwnerAccountIds(codeOwnerConfig, absolutePath);
+ CodeOwnerResolverResult codeOwners = getCodeOwners(codeOwnerConfig, absolutePath);
logger.atFine().log(
"code owners = %s (code owner config folder path = %s, file name = %s)",
- codeOwnerAccountIds,
+ codeOwners,
codeOwnerConfig.key().folderPath(),
codeOwnerConfig.key().fileName().orElse("<default>"));
if (isApproved(
absolutePath,
- codeOwnerAccountIds,
+ codeOwners,
approverAccountIds,
enableImplicitApprovalFromUploader,
patchSetUploader)) {
codeOwnerStatus.set(CodeOwnerStatus.APPROVED);
return false;
- } else if (isPending(absolutePath, codeOwnerAccountIds, reviewerAccountIds)) {
+ } else if (isPending(absolutePath, codeOwners, reviewerAccountIds)) {
codeOwnerStatus.set(CodeOwnerStatus.PENDING);
// We need to continue to check if any of the higher-level code owners approved the
@@ -524,18 +523,21 @@
private boolean isApproved(
Path absolutePath,
- ImmutableSet<Account.Id> codeOwnerAccountIds,
+ CodeOwnerResolverResult codeOwners,
ImmutableSet<Account.Id> approverAccountIds,
boolean enableImplicitApprovalFromUploader,
Account.Id patchSetUploader) {
- if (enableImplicitApprovalFromUploader && codeOwnerAccountIds.contains(patchSetUploader)) {
+ if (enableImplicitApprovalFromUploader
+ && (codeOwners.codeOwnersAccountIds().contains(patchSetUploader)
+ || codeOwners.ownedByAllUsers())) {
// If the uploader of the patch set owns the path, there is an implicit code owner
// approval from the patch set uploader so that the path is automatically approved.
logger.atFine().log("%s was implicitly approved by the patch set uploader", absolutePath);
return true;
}
- if (!Collections.disjoint(approverAccountIds, codeOwnerAccountIds)) {
+ if (!Collections.disjoint(approverAccountIds, codeOwners.codeOwnersAccountIds())
+ || (codeOwners.ownedByAllUsers() && !approverAccountIds.isEmpty())) {
// At least one of the global code owners approved the change.
logger.atFine().log("%s was explicitly approved by a code owner", absolutePath);
return true;
@@ -546,9 +548,10 @@
private boolean isPending(
Path absolutePath,
- ImmutableSet<Account.Id> codeOwnerAccountIds,
+ CodeOwnerResolverResult codeOwners,
ImmutableSet<Account.Id> reviewerAccountIds) {
- if (!Collections.disjoint(codeOwnerAccountIds, reviewerAccountIds)) {
+ if (!Collections.disjoint(codeOwners.codeOwnersAccountIds(), reviewerAccountIds)
+ || (codeOwners.ownedByAllUsers() && !reviewerAccountIds.isEmpty())) {
logger.atFine().log("%s is owned by a reviewer", absolutePath);
return true;
}
@@ -578,17 +581,17 @@
}
/**
- * Gets the IDs of the accounts that own the given path according to the given code owner config.
+ * Gets the code owners that own the given path according to the given code owner config.
*
* @param codeOwnerConfig the code owner config from which the code owners should be retrieved
* @param absolutePath the path for which the code owners should be retrieved
*/
- private ImmutableSet<Account.Id> getCodeOwnerAccountIds(
+ private CodeOwnerResolverResult getCodeOwners(
CodeOwnerConfig codeOwnerConfig, Path absolutePath) {
- return codeOwnerResolver.get().enforceVisibility(false)
- .resolvePathCodeOwners(codeOwnerConfig, absolutePath).stream()
- .map(CodeOwner::accountId)
- .collect(toImmutableSet());
+ return codeOwnerResolver
+ .get()
+ .enforceVisibility(false)
+ .resolvePathCodeOwners(codeOwnerConfig, absolutePath);
}
/**
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index b53afb2..fdd4da2 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -29,7 +29,6 @@
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountState;
-import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.logging.Metadata;
@@ -44,6 +43,7 @@
import java.nio.file.Path;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Stream;
/** Class to resolve {@link CodeOwnerReference}s to {@link CodeOwner}s. */
@@ -57,7 +57,6 @@
private final Provider<CurrentUser> currentUser;
private final ExternalIds externalIds;
private final AccountCache accountCache;
- private final Accounts accounts;
private final AccountControl.Factory accountControlFactory;
private final PathCodeOwners.Factory pathCodeOwnersFactory;
@@ -76,7 +75,6 @@
Provider<CurrentUser> currentUser,
ExternalIds externalIds,
AccountCache accountCache,
- Accounts accounts,
AccountControl.Factory accountControlFactory,
PathCodeOwners.Factory pathCodeOwnersFactory) {
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
@@ -84,7 +82,6 @@
this.currentUser = currentUser;
this.externalIds = externalIds;
this.accountCache = accountCache;
- this.accounts = accounts;
this.accountControlFactory = accountControlFactory;
this.pathCodeOwnersFactory = pathCodeOwnersFactory;
}
@@ -123,6 +120,12 @@
return this;
}
+ /** Whether the given code owner reference can be resolved. */
+ public boolean isResolvable(CodeOwnerReference codeOwnerReference) {
+ CodeOwnerResolverResult result = resolve(ImmutableSet.of(codeOwnerReference));
+ return !result.codeOwners().isEmpty() || result.ownedByAllUsers();
+ }
+
/**
* Resolves the code owners from the given code owner config for the given path from {@link
* CodeOwnerReference}s to a {@link CodeOwner}s.
@@ -135,7 +138,7 @@
* absolute; can be the path of a file or folder; the path may or may not exist
* @return the resolved code owners
*/
- public ImmutableSet<CodeOwner> resolvePathCodeOwners(
+ public CodeOwnerResolverResult resolvePathCodeOwners(
CodeOwnerConfig codeOwnerConfig, Path absolutePath) {
requireNonNull(codeOwnerConfig, "codeOwnerConfig");
requireNonNull(absolutePath, "absolutePath");
@@ -150,9 +153,7 @@
.filePath(codeOwnerConfig.key().fileName().orElse("<default>"))
.build())) {
logger.atFine().log("resolving path code owners for path %s", absolutePath);
- return pathCodeOwnersFactory.create(codeOwnerConfig, absolutePath).get().stream()
- .flatMap(this::resolve)
- .collect(toImmutableSet());
+ return resolve(pathCodeOwnersFactory.create(codeOwnerConfig, absolutePath).get());
}
}
@@ -163,9 +164,22 @@
* @return the {@link CodeOwner} for the given code owner references
* @see #resolve(CodeOwnerReference)
*/
- public Stream<CodeOwner> resolve(Set<CodeOwnerReference> codeOwnerReferences) {
+ public CodeOwnerResolverResult resolve(Set<CodeOwnerReference> codeOwnerReferences) {
requireNonNull(codeOwnerReferences, "codeOwnerReferences");
- return codeOwnerReferences.stream().flatMap(this::resolve);
+ AtomicBoolean ownedByAllUsers = new AtomicBoolean(false);
+ ImmutableSet<CodeOwner> codeOwners =
+ codeOwnerReferences.stream()
+ .filter(
+ codeOwnerReference -> {
+ if (ALL_USERS_WILDCARD.equals(codeOwnerReference.email())) {
+ ownedByAllUsers.set(true);
+ return false;
+ }
+ return true;
+ })
+ .flatMap(this::resolve)
+ .collect(toImmutableSet());
+ return CodeOwnerResolverResult.create(codeOwners, ownedByAllUsers.get());
}
/**
@@ -201,18 +215,18 @@
* all accounts
* </ul>
*
+ * <p>This method does not resolve {@link CodeOwnerReference}s that assign the code ownership to
+ * all user by using {@link #ALL_USERS_WILDCARD} as email.
+ *
* @param codeOwnerReference the code owner reference that should be resolved
* @return the {@link CodeOwner} for the code owner reference if it was resolved, otherwise {@link
* Optional#empty()}
*/
+ @VisibleForTesting
public Stream<CodeOwner> resolve(CodeOwnerReference codeOwnerReference) {
String email = requireNonNull(codeOwnerReference, "codeOwnerReference").email();
logger.atFine().log("resolving code owner reference %s", codeOwnerReference);
- if (ALL_USERS_WILDCARD.equals(email)) {
- return resolveAllUsersWildcard();
- }
-
if (!isEmailDomainAllowed(email)) {
logger.atFine().log("domain of email %s is not allowed", email);
return Stream.of();
@@ -239,18 +253,6 @@
return Stream.of(codeOwner);
}
- private Stream<CodeOwner> resolveAllUsersWildcard() {
- try (TraceTimer traceTimer =
- TraceContext.newTimer("Resolve all users wildcard", Metadata.builder().build())) {
- return accounts.all().stream()
- .filter(accountState -> !enforceVisibility || canSee(accountState))
- .map(accountState -> CodeOwner.create(accountState.account().id()));
- } catch (IOException e) {
- throw new StorageException(
- String.format("cannot resolve code owner email %s", ALL_USERS_WILDCARD), e);
- }
- }
-
/** Whether the given account can be seen. */
private boolean canSee(AccountState accountState) {
AccountControl accountControl =
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
new file mode 100644
index 0000000..226d541
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
@@ -0,0 +1,59 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.backend;
+
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Account;
+
+/** The result of resolving code owner references via {@link CodeOwnerResolver}. */
+@AutoValue
+public abstract class CodeOwnerResolverResult {
+ /**
+ * Returns the resolved code owners as stream.
+ *
+ * <p>Doesn't include code owners to which the code ownership was assigned by using the {@link
+ * CodeOwnerResolver#ALL_USERS_WILDCARD}.
+ */
+ public abstract ImmutableSet<CodeOwner> codeOwners();
+
+ /** Returns the account IDs of the resolved code owners as set. */
+ public ImmutableSet<Account.Id> codeOwnersAccountIds() {
+ return codeOwners().stream().map(CodeOwner::accountId).collect(toImmutableSet());
+ }
+
+ /**
+ * Whether the code ownership was assigned to all users by using the {@link
+ * CodeOwnerResolver#ALL_USERS_WILDCARD}.
+ */
+ public abstract boolean ownedByAllUsers();
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("codeOwners", codeOwners())
+ .add("ownedByAllUsers", ownedByAllUsers())
+ .toString();
+ }
+
+ /** Creates a {@link CodeOwnerResolverResult} instance. */
+ public static CodeOwnerResolverResult create(
+ ImmutableSet<CodeOwner> codeOwners, boolean ownedByAllUsers) {
+ return new AutoValue_CodeOwnerResolverResult(codeOwners, ownedByAllUsers);
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
index dd4fd6a..fcc5543 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -21,9 +21,12 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.client.ListAccountsOption;
import com.google.gerrit.extensions.client.ListOption;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
@@ -31,17 +34,21 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwner;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigHierarchy;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolverResult;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScoring;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.ServiceUserClassifier;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Provider;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
@@ -60,6 +67,9 @@
@VisibleForTesting public static final int DEFAULT_LIMIT = 10;
+ private final AccountVisibility accountVisibility;
+ private final Accounts accounts;
+ private final AccountControl.Factory accountControlFactory;
private final PermissionBackend permissionBackend;
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
private final CodeOwnerConfigHierarchy codeOwnerConfigHierarchy;
@@ -96,12 +106,18 @@
}
protected AbstractGetCodeOwnersForPath(
+ AccountVisibility accountVisibility,
+ Accounts accounts,
+ AccountControl.Factory accountControlFactory,
PermissionBackend permissionBackend,
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy,
Provider<CodeOwnerResolver> codeOwnerResolver,
ServiceUserClassifier serviceUserClassifier,
CodeOwnerJson.Factory codeOwnerJsonFactory) {
+ this.accountVisibility = accountVisibility;
+ this.accounts = accounts;
+ this.accountControlFactory = accountControlFactory;
this.permissionBackend = permissionBackend;
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.codeOwnerConfigHierarchy = codeOwnerConfigHierarchy;
@@ -134,12 +150,31 @@
rsrc.getRevision(),
rsrc.getPath(),
codeOwnerConfig -> {
- ImmutableSet<CodeOwner> pathCodeOwners =
+ CodeOwnerResolverResult pathCodeOwners =
codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, rsrc.getPath());
- codeOwners.addAll(filterCodeOwners(rsrc, pathCodeOwners));
+ codeOwners.addAll(filterCodeOwners(rsrc, pathCodeOwners.codeOwners()));
+
+ if (pathCodeOwners.ownedByAllUsers()) {
+ fillUpWithRandomUsers(rsrc, codeOwners, limit);
+
+ if (codeOwners.size() < limit) {
+ logger.atFine().log(
+ "tried to fill up the suggestion list with random users,"
+ + " but didn't find enough visible accounts"
+ + " (wanted number of suggestions = %d, got = %d",
+ limit, codeOwners.size());
+ }
+
+ // We already found that the path is owned by all users. Hence we do not need to check
+ // if there are further code owners in higher-level code owner configs.
+ return false;
+ }
+
int distance = rootDistance - codeOwnerConfig.key().folderPath().getNameCount();
- pathCodeOwners.forEach(
- localCodeOwner -> distanceScoring.putValueForCodeOwner(localCodeOwner, distance));
+ pathCodeOwners
+ .codeOwners()
+ .forEach(
+ localCodeOwner -> distanceScoring.putValueForCodeOwner(localCodeOwner, distance));
// If codeOwners.size() >= limit we have gathered enough code owners and do not need to
// look at further code owner configs.
@@ -150,10 +185,15 @@
});
if (codeOwners.size() < limit) {
- ImmutableSet<CodeOwner> globalCodeOwners = getGlobalCodeOwners(rsrc.getBranch().project());
- globalCodeOwners.forEach(
- codeOwner -> distanceScoring.putValueForCodeOwner(codeOwner, maxDistance));
- codeOwners.addAll(filterCodeOwners(rsrc, globalCodeOwners));
+ CodeOwnerResolverResult globalCodeOwners = getGlobalCodeOwners(rsrc.getBranch().project());
+ globalCodeOwners
+ .codeOwners()
+ .forEach(codeOwner -> distanceScoring.putValueForCodeOwner(codeOwner, maxDistance));
+ codeOwners.addAll(filterCodeOwners(rsrc, globalCodeOwners.codeOwners()));
+
+ if (globalCodeOwners.ownedByAllUsers()) {
+ fillUpWithRandomUsers(rsrc, codeOwners, limit);
+ }
}
return Response.ok(
@@ -162,12 +202,11 @@
.format(sortAndLimit(distanceScoring.build(), ImmutableSet.copyOf(codeOwners))));
}
- private ImmutableSet<CodeOwner> getGlobalCodeOwners(Project.NameKey projectName) {
- ImmutableSet<CodeOwner> globalCodeOwners =
+ private CodeOwnerResolverResult getGlobalCodeOwners(Project.NameKey projectName) {
+ CodeOwnerResolverResult globalCodeOwners =
codeOwnerResolver
.get()
- .resolve(codeOwnersPluginConfiguration.getGlobalCodeOwners(projectName))
- .collect(toImmutableSet());
+ .resolve(codeOwnersPluginConfiguration.getGlobalCodeOwners(projectName));
logger.atFine().log("including global code owners = %s", globalCodeOwners);
return globalCodeOwners;
}
@@ -288,14 +327,85 @@
}
/**
- * Returns the given code owners in a random order.
+ * Returns the entries from the given set in a random order.
*
- * @param codeOwners the code owners that should be returned in a random order
- * @return the given code owners in a random order
+ * @param set the set for which the entries should be returned in a random order
+ * @return the entries from the given set in a random order
*/
- private static Stream<CodeOwner> randomizeOrder(ImmutableSet<CodeOwner> codeOwners) {
- List<CodeOwner> randomlyOrderedCodeOwners = new ArrayList<>(codeOwners);
+ private static <T> Stream<T> randomizeOrder(Set<T> set) {
+ List<T> randomlyOrderedCodeOwners = new ArrayList<>(set);
Collections.shuffle(randomlyOrderedCodeOwners);
return randomlyOrderedCodeOwners.stream();
}
+
+ /**
+ * If the limit is not reached yet, add random visible users as code owners to the given code
+ * owner set.
+ *
+ * <p>Must be only used to complete the suggestion list when it is found that the path is owned by
+ * all user.
+ */
+ private void fillUpWithRandomUsers(
+ AbstractPathResource rsrc, Set<CodeOwner> codeOwners, int limit) {
+ if (codeOwners.size() >= limit) {
+ // limit is already reach, we don't need to add further suggestions
+ return;
+ }
+
+ logger.atFine().log("filling up with random users");
+ codeOwners.addAll(
+ filterCodeOwners(
+ rsrc,
+ // ask for 2 times the number of users that we need so that we still have enough
+ // suggestions when some users are removed by the filterCodeOwners call or if the
+ // returned users were already present in codeOwners
+ getRandomVisibleUsers(2 * limit - codeOwners.size())
+ .map(CodeOwner::create)
+ .collect(toImmutableSet())));
+ }
+
+ /**
+ * Returns random visible users, at most as many as specified by the limit.
+ *
+ * <p>It's possible that this method returns less users than the limit although further visible
+ * users exist. This is because we may inspect only a random set of users, instead of all users,
+ * for performance reasons.
+ *
+ * @param limit the max number of users that should be returned
+ * @return random visible users
+ */
+ private Stream<Account.Id> getRandomVisibleUsers(int limit) {
+ try {
+ if (permissionBackend.currentUser().test(GlobalPermission.VIEW_ALL_ACCOUNTS)) {
+ return getRandomUsers(limit);
+ }
+
+ switch (accountVisibility) {
+ case ALL:
+ return getRandomUsers(limit);
+ case SAME_GROUP:
+ case VISIBLE_GROUP:
+ // We cannot afford to inspect all relevant users and test their visibility for
+ // performance reasons, hence we use a random sample of users that is 3 times the limit.
+ return getRandomUsers(3 * limit)
+ .filter(accountId -> accountControlFactory.get().canSee(accountId))
+ .limit(limit);
+ case NONE:
+ return Stream.of();
+ }
+
+ throw new IllegalStateException("unknown account visibility setting: " + accountVisibility);
+ } catch (IOException | PermissionBackendException e) {
+ throw new StorageException("failed to get visible users", e);
+ }
+ }
+
+ /**
+ * Returns random users, at most as many as specified by the limit.
+ *
+ * <p>No visibility check is performed.
+ */
+ private Stream<Account.Id> getRandomUsers(int limit) throws IOException {
+ return randomizeOrder(accounts.allIds()).limit(limit);
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
index baf260d..6e3d054 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -26,6 +27,8 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigHierarchy;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.server.account.AccountControl;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.ServiceUserClassifier;
import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -65,6 +68,9 @@
@Inject
GetCodeOwnersForPathInBranch(
+ AccountVisibility accountVisibility,
+ Accounts accounts,
+ AccountControl.Factory accountControlFactory,
PermissionBackend permissionBackend,
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy,
@@ -73,6 +79,9 @@
CodeOwnerJson.Factory codeOwnerJsonFactory,
GitRepositoryManager repoManager) {
super(
+ accountVisibility,
+ accounts,
+ accountControlFactory,
permissionBackend,
codeOwnersPluginConfiguration,
codeOwnerConfigHierarchy,
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
index 43a5d21..3483614 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
@@ -14,6 +14,7 @@
package com.google.gerrit.plugins.codeowners.restapi;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -21,6 +22,8 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigHierarchy;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.server.account.AccountControl;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.ServiceUserClassifier;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -40,6 +43,9 @@
implements RestReadView<CodeOwnersInChangeCollection.PathResource> {
@Inject
GetCodeOwnersForPathInChange(
+ AccountVisibility accountVisibility,
+ Accounts accounts,
+ AccountControl.Factory accountControlFactory,
PermissionBackend permissionBackend,
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy,
@@ -47,6 +53,9 @@
ServiceUserClassifier serviceUserClassifier,
CodeOwnerJson.Factory codeOwnerJsonFactory) {
super(
+ accountVisibility,
+ accounts,
+ accountControlFactory,
permissionBackend,
codeOwnersPluginConfiguration,
codeOwnerConfigHierarchy,
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 4e883c5..f0023ff 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -674,7 +674,7 @@
}
// Check if the code owner reference is resolvable.
- if (codeOwnerResolver.resolve(codeOwnerReference).findAny().isPresent()) {
+ if (codeOwnerResolver.isResolvable(codeOwnerReference)) {
// The code owner reference was successfully resolved to at least one code owner.
return Optional.empty();
}
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 360e2a7..4048684 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
@@ -15,10 +15,12 @@
package com.google.gerrit.plugins.codeowners.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.assertThatList;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasAccountId;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasAccountName;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableList;
@@ -28,6 +30,7 @@
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.extensions.client.ListAccountsOption;
@@ -721,4 +724,178 @@
.update();
assertThat(queryCodeOwners("/foo/bar/baz.md")).isEmpty();
}
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "ALL")
+ public void getAllUsersAsCodeOwners_allVisible() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // Add a code owner config that makes all users code owners.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ List<CodeOwnerInfo> codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user.id(), user2.id(), admin.id());
+
+ // Query code owners with a limit.
+ codeOwnerInfos = queryCodeOwners(getCodeOwnersApi().query().withLimit(2), "/foo/bar/baz.md");
+ assertThat(codeOwnerInfos).hasSize(2);
+ assertThatList(codeOwnerInfos)
+ .element(0)
+ .hasAccountIdThat()
+ .isAnyOf(user.id(), user2.id(), admin.id());
+ assertThatList(codeOwnerInfos)
+ .element(1)
+ .hasAccountIdThat()
+ .isAnyOf(user.id(), user2.id(), admin.id());
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP")
+ public void getAllUsersAsCodeOwners_sameGroupVisibility() throws Exception {
+ // Create 2 accounts that share a group.
+ TestAccount user2 = accountCreator.user2();
+ TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3", null);
+ groupOperations.newGroup().addMember(user2.id()).addMember(user3.id()).create();
+
+ // Add a code owner config that makes all users code owners.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ // user can only see itself
+ requestScopeOperations.setApiUser(user.id());
+ List<CodeOwnerInfo> codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos).comparingElementsUsing(hasAccountId()).containsExactly(user.id());
+
+ // user2 can see user3 and itself
+ requestScopeOperations.setApiUser(user2.id());
+ codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user2.id(), user3.id());
+
+ // admin can see all users
+ requestScopeOperations.setApiUser(admin.id());
+ codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(admin.id(), user.id(), user2.id(), user3.id());
+
+ // Query code owners with a limit, user2 can see user3 and itself
+ requestScopeOperations.setApiUser(user2.id());
+ codeOwnerInfos = queryCodeOwners(getCodeOwnersApi().query().withLimit(1), "/foo/bar/baz.md");
+ assertThat(codeOwnerInfos).hasSize(1);
+ assertThatList(codeOwnerInfos).element(0).hasAccountIdThat().isAnyOf(user2.id(), user3.id());
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "VISIBLE_GROUP")
+ public void getAllUsersAsCodeOwners_visibleGroupVisibility() throws Exception {
+ // create a group that until contains user
+ AccountGroup.UUID userGroup = groupOperations.newGroup().addMember(user.id()).create();
+
+ // create user2 account and a group that only contains user2, but which is visible to user
+ // (since user owns the group)
+ TestAccount user2 = accountCreator.user2();
+ groupOperations.newGroup().addMember(user2.id()).ownerGroupUuid(userGroup).create();
+
+ // create user3 account and a group that only contains user3, but which is visible to all users
+ TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3", null);
+ groupOperations.newGroup().addMember(user3.id()).visibleToAll(true).create();
+
+ // Add a code owner config that makes all users code owners.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ // user can only see itself, user2 (because user is owner of a group that contains user2) and
+ // user3 (because user3 is member of a group that is visible to all users)
+ requestScopeOperations.setApiUser(user.id());
+ List<CodeOwnerInfo> codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user.id(), user2.id(), user3.id());
+
+ // user2 can see user3 and itself
+ requestScopeOperations.setApiUser(user2.id());
+ codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user2.id(), user3.id());
+
+ // admin can see all users
+ requestScopeOperations.setApiUser(admin.id());
+ codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(admin.id(), user.id(), user2.id(), user3.id());
+
+ // Query code owners with a limit, user2 can see user3 and itself
+ requestScopeOperations.setApiUser(user2.id());
+ codeOwnerInfos = queryCodeOwners(getCodeOwnersApi().query().withLimit(1), "/foo/bar/baz.md");
+ assertThat(codeOwnerInfos).hasSize(1);
+ assertThatList(codeOwnerInfos).element(0).hasAccountIdThat().isAnyOf(user2.id(), user3.id());
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "NONE")
+ public void getAllUsersAsCodeOwners_noneVisible() throws Exception {
+ accountCreator.user2();
+
+ // Add a code owner config that makes all users code owners.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ // Use user, since admin is allowed to view all accounts.
+ requestScopeOperations.setApiUser(user.id());
+ List<CodeOwnerInfo> codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "NONE")
+ public void getAllUsersAsCodeOwners_withViewAllAccounts() throws Exception {
+ // Allow all users to view all accounts.
+ projectOperations
+ .allProjectsForUpdate()
+ .add(allowCapability(GlobalCapability.VIEW_ALL_ACCOUNTS).group(REGISTERED_USERS))
+ .update();
+
+ TestAccount user2 = accountCreator.user2();
+
+ // Add a code owner config that makes all users code owners.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ List<CodeOwnerInfo> codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(admin.id(), user.id(), user2.id());
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/BUILD b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/BUILD
index 861a20f..4821121 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/BUILD
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/BUILD
@@ -25,8 +25,10 @@
srcs = glob(["Abstract*.java"]),
deps = [
"//java/com/google/gerrit/acceptance:framework-lib",
+ "//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/entities",
"//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/server",
"//lib:guava",
"//lib/guice",
"//plugins/code-owners:code-owners__plugin",
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
index 69e5f97..dc2549f 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
@@ -892,6 +892,136 @@
}
@Test
+ public void approvedByAnyoneWhenEveryoneIsCodeOwner() throws Exception {
+ // Create a code owner config file that makes everyone a code owner.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ // Create a change.
+ Path path = Paths.get("/foo/bar.baz");
+ String changeId =
+ createChange(user, "Change Adding A File", JgitPath.of(path).get(), "file content")
+ .getChangeId();
+
+ // Verify that the file is not approved yet (the change owner is a code owner, but
+ // implicit approvals are disabled).
+ Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+ assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add an approval by a user that is a code owner only through the global code ownership.
+ approve(changeId);
+
+ // Check that the file is approved now.
+ requestScopeOperations.setApiUser(admin.id());
+ fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ fileCodeOwnerStatusSubject = assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.APPROVED);
+ }
+
+ @Test
+ public void everyoneIsCodeOwner_noImplicitApproval() throws Exception {
+ testImplicitlyApprovedWhenEveryoneIsCodeOwner(false);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableImplicitApprovals", value = "true")
+ public void everyoneIsCodeOwner_withImplicitApproval() throws Exception {
+ testImplicitlyApprovedWhenEveryoneIsCodeOwner(true);
+ }
+
+ private void testImplicitlyApprovedWhenEveryoneIsCodeOwner(boolean implicitApprovalsEnabled)
+ throws Exception {
+ // Create a code owner config file that makes everyone a code owner.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ // Create a change as a user that is a code owner.
+ Path path = Paths.get("/foo/bar.baz");
+ String changeId =
+ createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
+
+ Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+ assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(
+ implicitApprovalsEnabled
+ ? CodeOwnerStatus.APPROVED
+ : CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ }
+
+ @Test
+ public void anyReviewerWhenEveryoneIsCodeOwner() throws Exception {
+ // Create a code owner config file that makes everyone a code owner.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .create();
+
+ // Create a change as a user that is a code owner.
+ Path path = Paths.get("/foo/bar.baz");
+ String changeId =
+ createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
+
+ // Verify that the status of the file is INSUFFICIENT_REVIEWERS (since there is no implicit
+ // approval by default).
+ Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+ assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add a user as reviewer that is a code owner.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ // Check that the status of the file is PENDING now.
+ fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ fileCodeOwnerStatusSubject = assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ }
+
+ @Test
@GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "bot@example.com")
public void approvedByGlobalCodeOwner() throws Exception {
testApprovedByGlobalCodeOwner(false);
@@ -1114,6 +1244,198 @@
}
@Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ public void approvedByAnyoneWhenEveryoneIsGlobalCodeOwner() throws Exception {
+ testApprovedByAnyoneWhenEveryoneIsGlobalCodeOwner(false);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ public void approvedByAnyoneWhenEveryoneIsGlobalCodeOwner_bootstrappingMode() throws Exception {
+ testApprovedByAnyoneWhenEveryoneIsGlobalCodeOwner(true);
+ }
+
+ private void testApprovedByAnyoneWhenEveryoneIsGlobalCodeOwner(boolean bootstrappingMode)
+ throws Exception {
+ if (!bootstrappingMode) {
+ // Create a code owner config file so that we are not in the bootstrapping mode.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+ }
+
+ // Create a change.
+ Path path = Paths.get("/foo/bar.baz");
+ String changeId =
+ createChange(user, "Change Adding A File", JgitPath.of(path).get(), "file content")
+ .getChangeId();
+
+ // Verify that the file is not approved yet (the change owner is a global code owner, but
+ // implicit approvals are disabled).
+ Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+ assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add an approval by a user that is a code owner only through the global code ownership.
+ approve(changeId);
+
+ // Check that the file is approved now.
+ requestScopeOperations.setApiUser(admin.id());
+ fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ fileCodeOwnerStatusSubject = assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.APPROVED);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ public void everyoneIsGlobalCodeOwner_noImplicitApproval() throws Exception {
+ testImplicitlyApprovedByGlobalCodeOwnerWhenEveryoneIsGlobalCodeOwner(
+ /** implicitApprovalsEnabled = */
+ false,
+ /** bootstrappingMode = */
+ false);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ @GerritConfig(name = "plugin.code-owners.enableImplicitApprovals", value = "true")
+ public void everyoneIsGlobalCodeOwner_withImplicitApproval() throws Exception {
+ testImplicitlyApprovedByGlobalCodeOwnerWhenEveryoneIsGlobalCodeOwner(
+ /** implicitApprovalsEnabled = */
+ true,
+ /** bootstrappingMode = */
+ false);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ public void everyoneIsGlobalCodeOwner_noImplicitApproval_bootstrappingMode() throws Exception {
+ testImplicitlyApprovedByGlobalCodeOwnerWhenEveryoneIsGlobalCodeOwner(
+ /** implicitApprovalsEnabled = */
+ false,
+ /** bootstrappingMode = */
+ true);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ @GerritConfig(name = "plugin.code-owners.enableImplicitApprovals", value = "true")
+ public void everyoneIsGlobalCodeOwner_withImplicitApproval_bootstrappingMode() throws Exception {
+ testImplicitlyApprovedByGlobalCodeOwnerWhenEveryoneIsGlobalCodeOwner(
+ /** implicitApprovalsEnabled = */
+ true,
+ /** bootstrappingMode = */
+ true);
+ }
+
+ private void testImplicitlyApprovedByGlobalCodeOwnerWhenEveryoneIsGlobalCodeOwner(
+ boolean implicitApprovalsEnabled, boolean bootstrappingMode) throws Exception {
+ if (!bootstrappingMode) {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+ }
+
+ // Create a change as a user that is a code owner only through the global code ownership.
+ Path path = Paths.get("/foo/bar.baz");
+ String changeId =
+ createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
+
+ Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+ assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(
+ implicitApprovalsEnabled
+ ? CodeOwnerStatus.APPROVED
+ : CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ public void anyReviewerWhenEveryoneIsGlobalCodeOwner() throws Exception {
+ testAnyReviewerWhenEveryoneIsGlobalCodeOwner(false);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "*")
+ public void anyReviewerWhenEveryoneIsGlobalCodeOwner_bootstrappingMode() throws Exception {
+ testAnyReviewerWhenEveryoneIsGlobalCodeOwner(true);
+ }
+
+ private void testAnyReviewerWhenEveryoneIsGlobalCodeOwner(boolean bootstrappingMode)
+ throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ if (!bootstrappingMode) {
+ // Create a code owner config file so that we are not in the bootstrapping mode.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user2.email())
+ .create();
+ }
+
+ // Create a change as a user that is a code owner only through the global code ownership.
+ Path path = Paths.get("/foo/bar.baz");
+ String changeId =
+ createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
+
+ // Verify that the status of the file is INSUFFICIENT_REVIEWERS (since there is no implicit
+ // approval by default).
+ Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+ assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add a user as reviewer that is a code owner only through the global code ownership.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ // Check that the status of the file is PENDING now.
+ fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+ fileCodeOwnerStatusSubject = assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ }
+
+ @Test
public void parentCodeOwnerConfigsAreConsidered() throws Exception {
TestAccount user2 = accountCreator.user2();
TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3", null);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
index 8737bc6..f390336 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerSubject.assertThat;
-import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerSubject.hasAccountId;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableSet;
@@ -90,53 +89,12 @@
}
@Test
- public void resolveCodeOwnerReferenceForStarAsEmail() throws Exception {
- TestAccount user2 = accountCreator.user2();
+ public void cannotResolveCodeOwnerReferenceForStarAsEmail() throws Exception {
Stream<CodeOwner> codeOwner =
codeOwnerResolver
.get()
.resolve(CodeOwnerReference.create(CodeOwnerResolver.ALL_USERS_WILDCARD));
- assertThat(codeOwner)
- .comparingElementsUsing(hasAccountId())
- .containsExactly(admin.id(), user.id(), user2.id());
- }
-
- @Test
- @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP")
- public void resolveCodeOwnerReferenceForStarAsEmailChecksAccountVisibility() throws Exception {
- // Create a new user that is not a member of any group. This means 'user' and 'admin' are not
- // visible to this user since they do not share any group.
- TestAccount user2 = accountCreator.user2();
-
- // Set user2 as current user.
- requestScopeOperations.setApiUser(user2.id());
-
- Stream<CodeOwner> codeOwner =
- codeOwnerResolver
- .get()
- .resolve(CodeOwnerReference.create(CodeOwnerResolver.ALL_USERS_WILDCARD));
- assertThat(codeOwner).comparingElementsUsing(hasAccountId()).containsExactly(user2.id());
- }
-
- @Test
- @GerritConfig(name = "accounts.visibility", value = "SAME_GROUP")
- public void nonVisibleCodeOwnerCanBeResolvedForStarAsEmailIfVisibilityIsNotEnforced()
- throws Exception {
- // Create a new user that is not a member of any group. This means 'user' and 'admin' are not
- // visible to this user since they do not share any group.
- TestAccount user2 = accountCreator.user2();
-
- // Set user2 as current user.
- requestScopeOperations.setApiUser(user2.id());
-
- Stream<CodeOwner> codeOwner =
- codeOwnerResolver
- .get()
- .enforceVisibility(false)
- .resolve(CodeOwnerReference.create(CodeOwnerResolver.ALL_USERS_WILDCARD));
- assertThat(codeOwner)
- .comparingElementsUsing(hasAccountId())
- .containsExactly(admin.id(), user.id(), user2.id());
+ assertThat(codeOwner).isEmpty();
}
@Test
@@ -222,9 +180,10 @@
CodeOwnerConfig codeOwnerConfig =
CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION)
.build();
- assertThat(
- codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md")))
- .isEmpty();
+ CodeOwnerResolverResult result =
+ codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
+ assertThat(result.codeOwners()).isEmpty();
+ assertThat(result.ownedByAllUsers()).isFalse();
}
@Test
@@ -233,10 +192,25 @@
CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION)
.addCodeOwnerSet(CodeOwnerSet.createWithoutPathExpressions(admin.email(), user.email()))
.build();
- assertThat(
- codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md")))
- .comparingElementsUsing(hasAccountId())
- .containsExactly(admin.id(), user.id());
+
+ CodeOwnerResolverResult result =
+ codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
+ assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id(), user.id());
+ assertThat(result.ownedByAllUsers()).isFalse();
+ }
+
+ @Test
+ public void resolvePathCodeOwnersWhenStarIsUsedAsEmail() throws Exception {
+ CodeOwnerConfig codeOwnerConfig =
+ CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION)
+ .addCodeOwnerSet(
+ CodeOwnerSet.createWithoutPathExpressions(CodeOwnerResolver.ALL_USERS_WILDCARD))
+ .build();
+
+ CodeOwnerResolverResult result =
+ codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
+ assertThat(result.codeOwnersAccountIds()).isEmpty();
+ assertThat(result.ownedByAllUsers()).isTrue();
}
@Test
@@ -247,10 +221,10 @@
CodeOwnerSet.createWithoutPathExpressions(
admin.email(), "non-existing@example.com"))
.build();
- assertThat(
- codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md")))
- .comparingElementsUsing(hasAccountId())
- .containsExactly(admin.id());
+ CodeOwnerResolverResult result =
+ codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
+ assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id());
+ assertThat(result.ownedByAllUsers()).isFalse();
}
@Test
@@ -376,14 +350,27 @@
@Test
public void resolveCodeOwnerReferences() throws Exception {
+ CodeOwnerResolverResult result =
+ codeOwnerResolver
+ .get()
+ .resolve(
+ ImmutableSet.of(
+ CodeOwnerReference.create(admin.email()),
+ CodeOwnerReference.create(user.email())));
+ assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id(), user.id());
+ assertThat(result.ownedByAllUsers()).isFalse();
+ }
+
+ @Test
+ public void isResolvable() throws Exception {
+ assertThat(codeOwnerResolver.get().isResolvable(CodeOwnerReference.create(admin.email())))
+ .isTrue();
+ }
+
+ @Test
+ public void isNotResolvable() throws Exception {
assertThat(
- codeOwnerResolver
- .get()
- .resolve(
- ImmutableSet.of(
- CodeOwnerReference.create(admin.email()),
- CodeOwnerReference.create(user.email()))))
- .comparingElementsUsing(hasAccountId())
- .containsExactly(admin.id(), user.id());
+ codeOwnerResolver.get().isResolvable(CodeOwnerReference.create("unknown@example.com")))
+ .isFalse();
}
}
