Distinguish btw. messages that can be shown to all users vs. admins only
Calling the CheckCodeOwner REST endpoint requires the caller to be an
admin (have the 'Administrate Server' capability or the 'Check Code
Owner Capability'). Due to this normal users cannot debug issues with
OWNERS files on their own, but have to file tickets to find someone that
calls the REST endpoint and explains them the result. To reduce the
ticket load we intend to offer the CheckCodeOwner REST endpoint as a
self-service that every user can invoke. For this we must hide all
debug messages that contain information that requires admin permissions.
As a first step towards offering the CheckCodeOwner REST endpoint as a
self service we distinguish between messages that can be shown to any
calling user and messages that must only be shown to admin users:
* Most messages can be shown to the calling user (e.g. they contain
information about the code owner config files that they can access
through other APIs).
* Messages that explain why a code owner email is not resolvable reveal
information about whether an email exists or not and hence can be
shown only to admins (e.g. we must not reveal whether a non-visible
secondary email exists). Instead of the detailed message, for normal
users we just show a generic message that the email cannot be resolved
because it doesn't exist or because it is not visible.
* Messages that explain what another user can see must only be shown to
admins (when we offer the CheckCodeOwner REST endpoint as a self
service we will not allow to specify a user for whom the evalution
should be done, so these message will never be triggered for normal
users, but to be safe we suppress them).
On API level there is no change yet: Since for now debug messages are
only returned for admins we always return the admin messages.
Bug: Google b/345161989
Change-Id: Ib28802d38dc637dde82919f17b25014a174dbafc
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index f6718eb..2f55c21 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -281,18 +281,19 @@
ImmutableMultimap<CodeOwnerReference, CodeOwnerAnnotation> annotationsByCodeOwnerReference,
ImmutableList<CodeOwnerConfigImport> resolvedImports,
ImmutableList<CodeOwnerConfigImport> unresolvedImports,
- ImmutableList<String> pathCodeOwnersMessages) {
+ ImmutableList<DebugMessage> pathCodeOwnersMessages) {
requireNonNull(codeOwnerReferences, "codeOwnerReferences");
requireNonNull(resolvedImports, "resolvedImports");
requireNonNull(unresolvedImports, "unresolvedImports");
requireNonNull(pathCodeOwnersMessages, "pathCodeOwnersMessages");
try (Timer0.Context ctx = codeOwnerMetrics.resolveCodeOwnerReferences.start()) {
- ImmutableList.Builder<String> messageBuilder = ImmutableList.builder();
+ ImmutableList.Builder<DebugMessage> messageBuilder = ImmutableList.builder();
messageBuilder.addAll(pathCodeOwnersMessages);
unresolvedImports.forEach(
unresolvedImport ->
- messageBuilder.add(unresolvedImportFormatter.format(unresolvedImport)));
+ messageBuilder.add(
+ DebugMessage.createMessage(unresolvedImportFormatter.format(unresolvedImport))));
AtomicBoolean ownedByAllUsers = new AtomicBoolean(false);
AtomicBoolean hasUnresolvedCodeOwners = new AtomicBoolean(false);
@@ -356,12 +357,13 @@
if (CodeOwnerResolver.ALL_USERS_WILDCARD.equals(codeOwnerReference.email())) {
return OptionalResultWithMessages.createEmpty(
- String.format(
- "cannot resolve code owner email %s: no account with this email exists",
- CodeOwnerResolver.ALL_USERS_WILDCARD));
+ DebugMessage.createMessage(
+ String.format(
+ "cannot resolve code owner email %s: no account with this email exists",
+ CodeOwnerResolver.ALL_USERS_WILDCARD)));
}
- ImmutableList.Builder<String> messageBuilder = ImmutableList.builder();
+ ImmutableList.Builder<DebugMessage> messageBuilder = ImmutableList.builder();
AtomicBoolean ownedByAllUsers = new AtomicBoolean(false);
AtomicBoolean hasUnresolvedCodeOwners = new AtomicBoolean(false);
ImmutableMap<CodeOwner, ImmutableSet<CodeOwnerAnnotation>> codeOwnersWithAnnotations =
@@ -371,7 +373,7 @@
hasUnresolvedCodeOwners,
ImmutableSet.of(codeOwnerReference),
/* annotations= */ ImmutableMultimap.of());
- ImmutableList<String> messages = messageBuilder.build();
+ ImmutableList<DebugMessage> messages = messageBuilder.build();
if (codeOwnersWithAnnotations.isEmpty()) {
return OptionalResultWithMessages.createEmpty(messages);
}
@@ -397,7 +399,7 @@
* owners without annotations and Multimap doesn't store keys for which no values are stored)
*/
private ImmutableMap<CodeOwner, ImmutableSet<CodeOwnerAnnotation>> resolve(
- ImmutableList.Builder<String> messages,
+ ImmutableList.Builder<DebugMessage> messages,
AtomicBoolean ownedByAllUsers,
AtomicBoolean hasUnresolvedCodeOwners,
Set<CodeOwnerReference> codeOwnerReferences,
@@ -435,7 +437,7 @@
.filter(filterOutEmailsOfNonVisibleAccounts(messages))
.filter(filterOutNonVisibleSecondaryEmails(messages));
} else {
- messages.add("code owner visibility is not checked");
+ messages.add(DebugMessage.createMessage("code owner visibility is not checked"));
}
ImmutableMap<String, CodeOwner> codeOwnersByEmail =
@@ -501,7 +503,7 @@
* @param messages builder to which debug messages are added
*/
private Predicate<String> filterOutEmailsWithNonAllowedDomains(
- ImmutableList.Builder<String> messages) {
+ ImmutableList.Builder<DebugMessage> messages) {
return email -> {
boolean isEmailDomainAllowed = isEmailDomainAllowed(messages, email);
if (!isEmailDomainAllowed) {
@@ -525,7 +527,7 @@
* contains {@code false}
*/
public OptionalResultWithMessages<Boolean> isEmailDomainAllowed(String email) {
- ImmutableList.Builder<String> messages = ImmutableList.builder();
+ ImmutableList.Builder<DebugMessage> messages = ImmutableList.builder();
boolean isEmailDomainAllowed = isEmailDomainAllowed(messages, email);
return OptionalResultWithMessages.create(isEmailDomainAllowed, messages.build());
}
@@ -541,19 +543,19 @@
* @return {@code true} if the domain of the given email is allowed for code owners, otherwise
* {@code false}
*/
- private boolean isEmailDomainAllowed(ImmutableList.Builder<String> messages, String email) {
+ private boolean isEmailDomainAllowed(ImmutableList.Builder<DebugMessage> messages, String email) {
requireNonNull(messages, "messages");
requireNonNull(email, "email");
ImmutableSet<String> allowedEmailDomains =
codeOwnersPluginConfiguration.getGlobalConfig().getAllowedEmailDomains();
if (allowedEmailDomains.isEmpty()) {
- messages.add("all domains are allowed");
+ messages.add(DebugMessage.createMessage("all domains are allowed"));
return true;
}
if (email.equals(ALL_USERS_WILDCARD)) {
- messages.add("all users wildcard is allowed");
+ messages.add(DebugMessage.createMessage("all users wildcard is allowed"));
return true;
}
@@ -562,13 +564,14 @@
String emailDomain = email.substring(emailAtIndex + 1);
boolean isEmailDomainAllowed = allowedEmailDomains.contains(emailDomain);
messages.add(
- String.format(
- "domain %s of email %s is %s",
- emailDomain, email, isEmailDomainAllowed ? "allowed" : "not allowed"));
+ DebugMessage.createMessage(
+ String.format(
+ "domain %s of email %s is %s",
+ emailDomain, email, isEmailDomainAllowed ? "allowed" : "not allowed")));
return isEmailDomainAllowed;
}
- messages.add(String.format("email %s has no domain", email));
+ messages.add(DebugMessage.createMessage(String.format("email %s has no domain", email)));
return false;
}
@@ -583,7 +586,7 @@
* @return external IDs per email
*/
private ImmutableMap<String, Collection<ExternalId>> lookupExternalIds(
- ImmutableList.Builder<String> messages, ImmutableSet<String> emails) {
+ ImmutableList.Builder<DebugMessage> messages, ImmutableSet<String> emails) {
try {
ImmutableMap<String, Collection<ExternalId>> extIdsByEmail =
externalIdCache.byEmails(emails.toArray(new String[0])).asMap();
@@ -593,9 +596,11 @@
email -> {
transientCodeOwnerCache.cacheNonResolvable(email);
messages.add(
- String.format(
- "cannot resolve code owner email %s: no account with this email exists",
- email));
+ createDebugMessageForNonResolvableEmail(
+ email,
+ String.format(
+ "cannot resolve code owner email %s: no account with this email exists",
+ email)));
});
return extIdsByEmail;
} catch (IOException e) {
@@ -615,7 +620,7 @@
* @return account states per email
*/
private Stream<Pair<String, Collection<AccountState>>> lookupAccounts(
- ImmutableList.Builder<String> messages,
+ ImmutableList.Builder<DebugMessage> messages,
ImmutableMap<String, Collection<ExternalId>> externalIdsByEmail) {
ImmutableSet<Account.Id> accountIds =
externalIdsByEmail.values().stream()
@@ -635,10 +640,12 @@
AccountState accountState = accounts.get(accountId);
if (accountState == null) {
messages.add(
- String.format(
- "cannot resolve account %s for email %s: account does not"
- + " exists",
- accountId, e.getKey()));
+ createDebugMessageForNonResolvableEmail(
+ e.getKey(),
+ String.format(
+ "cannot resolve account %s for email %s: account does not"
+ + " exists",
+ accountId, e.getKey())));
}
return accountState;
})
@@ -656,7 +663,7 @@
* @param messages builder to which debug messages are added
*/
private Function<Pair<String, Collection<AccountState>>, Pair<String, Collection<AccountState>>>
- removeInactiveAccounts(ImmutableList.Builder<String> messages) {
+ removeInactiveAccounts(ImmutableList.Builder<DebugMessage> messages) {
return e -> Pair.of(e.key(), removeInactiveAccounts(messages, e.key(), e.value()));
}
@@ -669,7 +676,7 @@
* @return the account states that belong to active accounts
*/
private ImmutableSet<AccountState> removeInactiveAccounts(
- ImmutableList.Builder<String> messages,
+ ImmutableList.Builder<DebugMessage> messages,
String email,
Collection<AccountState> accountStates) {
return accountStates.stream()
@@ -677,9 +684,10 @@
accountState -> {
if (!accountState.account().isActive()) {
messages.add(
- String.format(
- "ignoring inactive account %s for email %s",
- accountState.account().id(), email));
+ DebugMessage.createMessage(
+ String.format(
+ "ignoring inactive account %s for email %s",
+ accountState.account().id(), email)));
return false;
}
return true;
@@ -696,15 +704,17 @@
* @param messages builder to which debug messages are added
*/
private Predicate<Pair<String, Collection<AccountState>>> filterOutEmailsWithoutAccounts(
- ImmutableList.Builder<String> messages) {
+ ImmutableList.Builder<DebugMessage> messages) {
return e -> {
if (e.value().isEmpty()) {
String email = e.key();
transientCodeOwnerCache.cacheNonResolvable(email);
messages.add(
- String.format(
- "cannot resolve code owner email %s: no active account with this email found",
- email));
+ createDebugMessageForNonResolvableEmail(
+ email,
+ String.format(
+ "cannot resolve code owner email %s: no active account with this email found",
+ email)));
return false;
}
return true;
@@ -721,13 +731,15 @@
* @param messages builder to which debug messages are added
*/
private Predicate<Pair<String, Collection<AccountState>>> filterOutAmbiguousEmails(
- ImmutableList.Builder<String> messages) {
+ ImmutableList.Builder<DebugMessage> messages) {
return e -> {
if (e.value().size() > 1) {
String email = e.key();
transientCodeOwnerCache.cacheNonResolvable(email);
messages.add(
- String.format("cannot resolve code owner email %s: email is ambiguous", email));
+ createDebugMessageForNonResolvableEmail(
+ email,
+ String.format("cannot resolve code owner email %s: email is ambiguous", email)));
return false;
}
return true;
@@ -745,12 +757,14 @@
* @param messages builder to which debug messages are added
*/
private Function<Pair<String, Collection<AccountState>>, Pair<String, AccountState>>
- mapToOnlyAccount(ImmutableList.Builder<String> messages) {
+ mapToOnlyAccount(ImmutableList.Builder<DebugMessage> messages) {
return e -> {
String email = e.key();
AccountState accountState = Iterables.getOnlyElement(e.value());
messages.add(
- String.format("resolved email %s to account %s", email, accountState.account().id()));
+ DebugMessage.createMessage(
+ String.format(
+ "resolved email %s to account %s", email, accountState.account().id())));
return Pair.of(email, accountState);
};
}
@@ -761,18 +775,20 @@
* @param messages builder to which debug messages are added
*/
private Predicate<Pair<String, AccountState>> filterOutEmailsOfNonVisibleAccounts(
- ImmutableList.Builder<String> messages) {
+ ImmutableList.Builder<DebugMessage> messages) {
return e -> {
String email = e.key();
AccountState accountState = e.value();
if (!canSee(accountState)) {
transientCodeOwnerCache.cacheNonResolvable(email);
messages.add(
- String.format(
- "cannot resolve code owner email %s: account %s is not visible to user %s",
+ createDebugMessageForNonResolvableEmail(
email,
- accountState.account().id(),
- user != null ? user.getLoggableName() : currentUser.get().getLoggableName()));
+ String.format(
+ "cannot resolve code owner email %s: account %s is not visible to user %s",
+ email,
+ accountState.account().id(),
+ user != null ? user.getLoggableName() : currentUser.get().getLoggableName())));
return false;
}
@@ -801,27 +817,29 @@
* @param messages builder to which debug messages are added
*/
private Predicate<Pair<String, AccountState>> filterOutNonVisibleSecondaryEmails(
- ImmutableList.Builder<String> messages) {
+ ImmutableList.Builder<DebugMessage> messages) {
return e -> {
String email = e.key();
AccountState accountState = e.value();
if (email.equals(accountState.account().preferredEmail())) {
// the email is a primary email of the account
messages.add(
- String.format(
- "account %s is visible to user %s",
- accountState.account().id(),
- user != null ? user.getLoggableName() : currentUser.get().getLoggableName()));
+ DebugMessage.createMessage(
+ String.format(
+ "account %s is visible to user %s",
+ accountState.account().id(),
+ user != null ? user.getLoggableName() : currentUser.get().getLoggableName())));
return true;
}
if (user != null) {
if (user.hasEmailAddress(email)) {
messages.add(
- String.format(
- "email %s is visible to user %s: email is a secondary email that is owned by this"
- + " user",
- email, user.getLoggableName()));
+ DebugMessage.createAdminOnlyMessage(
+ String.format(
+ "email %s is visible to user %s: email is a secondary email that is owned by this"
+ + " user",
+ email, user.getLoggableName())));
return true;
}
} else if (currentUser.get().isIdentifiedUser()
@@ -829,10 +847,11 @@
// it's a secondary email of the calling user, users can always see their own secondary
// emails
messages.add(
- String.format(
- "email %s is visible to the calling user %s: email is a secondary email that is"
- + " owned by this user",
- email, currentUser.get().getLoggableName()));
+ DebugMessage.createMessage(
+ String.format(
+ "email %s is visible to the calling user %s: email is a secondary email that is"
+ + " owned by this user",
+ email, currentUser.get().getLoggableName())));
return true;
}
@@ -843,32 +862,37 @@
if (!permissionBackend.user(user).test(GlobalPermission.VIEW_SECONDARY_EMAILS)) {
transientCodeOwnerCache.cacheNonResolvable(email);
messages.add(
- String.format(
- "cannot resolve code owner email %s: account %s is referenced by secondary"
- + " email but user %s cannot see secondary emails",
- email, accountState.account().id(), user.getLoggableName()));
+ DebugMessage.createAdminOnlyMessage(
+ String.format(
+ "cannot resolve code owner email %s: account %s is referenced by secondary"
+ + " email but user %s cannot see secondary emails",
+ email, accountState.account().id(), user.getLoggableName())));
return false;
}
messages.add(
- String.format(
- "resolved code owner email %s: account %s is referenced by secondary email"
- + " and user %s can see secondary emails",
- email, accountState.account().id(), user.getLoggableName()));
+ DebugMessage.createAdminOnlyMessage(
+ String.format(
+ "resolved code owner email %s: account %s is referenced by secondary email"
+ + " and user %s can see secondary emails",
+ email, accountState.account().id(), user.getLoggableName())));
return true;
} else if (!permissionBackend.currentUser().test(GlobalPermission.VIEW_SECONDARY_EMAILS)) {
transientCodeOwnerCache.cacheNonResolvable(email);
messages.add(
- String.format(
- "cannot resolve code owner email %s: account %s is referenced by secondary email"
- + " but the calling user %s cannot see secondary emails",
- email, accountState.account().id(), currentUser.get().getLoggableName()));
+ createDebugMessageForNonResolvableEmail(
+ email,
+ String.format(
+ "cannot resolve code owner email %s: account %s is referenced by secondary email"
+ + " but the calling user %s cannot see secondary emails",
+ email, accountState.account().id(), currentUser.get().getLoggableName())));
return false;
} else {
messages.add(
- String.format(
- "resolved code owner email %s: account %s is referenced by secondary email"
- + " and the calling user %s can see secondary emails",
- email, accountState.account().id(), currentUser.get().getLoggableName()));
+ DebugMessage.createMessage(
+ String.format(
+ "resolved code owner email %s: account %s is referenced by secondary email"
+ + " and the calling user %s can see secondary emails",
+ email, accountState.account().id(), currentUser.get().getLoggableName())));
return true;
}
} catch (PermissionBackendException ex) {
@@ -880,6 +904,15 @@
};
}
+ private DebugMessage createDebugMessageForNonResolvableEmail(String email, String adminMessage) {
+ // for non-admins we cannot reveal why a code owner email cannot be resolved so that they are
+ // not able to probe whether an email exists or not
+ return DebugMessage.createMessage(
+ adminMessage,
+ String.format(
+ "cannot resolve code owner email %s: email doesn't exist or is not visible", email));
+ }
+
/**
* Creates a map function that maps a {@code Pair<String, AccountState>} to a code owner.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
index 4db3a4c..a973c8e 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
@@ -64,7 +64,7 @@
}
/** Gets messages that were collected while resolving the code owners. */
- public abstract ImmutableList<String> messages();
+ public abstract ImmutableList<DebugMessage> messages();
/**
* Whether there are any code owners defined for the path, regardless of whether they can be
@@ -98,7 +98,7 @@
boolean hasUnresolvedCodeOwners,
ImmutableList<CodeOwnerConfigImport> resolvedImports,
ImmutableList<CodeOwnerConfigImport> unresolvedImports,
- List<String> messages) {
+ List<DebugMessage> messages) {
return new AutoValue_CodeOwnerResolverResult(
codeOwners,
annotations,
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/DebugMessage.java b/java/com/google/gerrit/plugins/codeowners/backend/DebugMessage.java
new file mode 100644
index 0000000..9baa07b
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/backend/DebugMessage.java
@@ -0,0 +1,68 @@
+// Copyright (C) 2024 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 com.google.auto.value.AutoValue;
+import java.util.Optional;
+
+/** A message to return debug information to callers. */
+@AutoValue
+public abstract class DebugMessage {
+ /**
+ * The debug message with all information.
+ *
+ * <p>Must be shown only to admin users (users with the 'Administrate Server' capability or the
+ * 'Check Code Owner' capability).
+ */
+ public abstract String adminMessage();
+
+ /**
+ * The debug message without information that require admin permissions.
+ *
+ * <p>Can be shown to the calling user.
+ *
+ * <p>Some messages are not available for the calling user. In this case {@link Optional#empty()}
+ * is returned.
+ */
+ public abstract Optional<String> userMessage();
+
+ /**
+ * Creates a debug message.
+ *
+ * @param adminMessage message that can only be shown to admins
+ */
+ public static DebugMessage createAdminOnlyMessage(String adminMessage) {
+ return new AutoValue_DebugMessage(adminMessage, Optional.empty());
+ }
+
+ /**
+ * Creates a debug message.
+ *
+ * @param adminMessage message that can only be shown to admins
+ * @param userMessage message that can be shown to the calling user
+ */
+ public static DebugMessage createMessage(String adminMessage, String userMessage) {
+ return new AutoValue_DebugMessage(adminMessage, Optional.of(userMessage));
+ }
+
+ /**
+ * Creates a debug message.
+ *
+ * @param userMessage message that can be shown to the calling user
+ */
+ public static DebugMessage createMessage(String userMessage) {
+ return new AutoValue_DebugMessage(userMessage, Optional.of(userMessage));
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OptionalResultWithMessages.java b/java/com/google/gerrit/plugins/codeowners/backend/OptionalResultWithMessages.java
index 6775d5b..efff089 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OptionalResultWithMessages.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OptionalResultWithMessages.java
@@ -47,7 +47,7 @@
}
/** Gets the messages. */
- public abstract ImmutableList<String> messages();
+ public abstract ImmutableList<DebugMessage> messages();
/** Creates a {@link OptionalResultWithMessages} instance without messages. */
public static <T> OptionalResultWithMessages<T> create(T result) {
@@ -55,26 +55,26 @@
}
/** Creates an empty {@link OptionalResultWithMessages} instance with a single message. */
- public static <T> OptionalResultWithMessages<T> createEmpty(String message) {
+ public static <T> OptionalResultWithMessages<T> createEmpty(DebugMessage message) {
requireNonNull(message, "message");
return createEmpty(ImmutableList.of(message));
}
/** Creates an empty {@link OptionalResultWithMessages} instance with messages. */
- public static <T> OptionalResultWithMessages<T> createEmpty(List<String> messages) {
+ public static <T> OptionalResultWithMessages<T> createEmpty(List<DebugMessage> messages) {
requireNonNull(messages, "messages");
return new AutoValue_OptionalResultWithMessages<>(
Optional.empty(), ImmutableList.copyOf(messages));
}
/** Creates a {@link OptionalResultWithMessages} instance with messages. */
- public static <T> OptionalResultWithMessages<T> create(T result, String message) {
+ public static <T> OptionalResultWithMessages<T> create(T result, DebugMessage message) {
requireNonNull(message, "message");
return create(result, ImmutableList.of(message));
}
/** Creates a {@link OptionalResultWithMessages} instance with messages. */
- public static <T> OptionalResultWithMessages<T> create(T result, List<String> messages) {
+ public static <T> OptionalResultWithMessages<T> create(T result, List<DebugMessage> messages) {
requireNonNull(result, "result");
requireNonNull(messages, "messages");
return new AutoValue_OptionalResultWithMessages<>(
@@ -83,7 +83,7 @@
/** Creates a {@link OptionalResultWithMessages} instance with messages. */
public static <T> OptionalResultWithMessages<T> create(
- Optional<T> result, List<String> messages) {
+ Optional<T> result, List<DebugMessage> messages) {
requireNonNull(result, "result");
requireNonNull(messages, "messages");
return new AutoValue_OptionalResultWithMessages<>(result, ImmutableList.copyOf(messages));
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
index 2083fc2..585c3e7 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
@@ -229,12 +229,13 @@
codeOwnerConfigFilePath);
pathCodeOwnersResultBuilder.addMessage(
- String.format(
- "resolve code owners for %s from code owner config %s:%s:%s",
- path,
- codeOwnerConfig.key().project(),
- codeOwnerConfig.key().shortBranchName(),
- codeOwnerConfigFilePath));
+ DebugMessage.createMessage(
+ String.format(
+ "resolve code owners for %s from code owner config %s:%s:%s",
+ path,
+ codeOwnerConfig.key().project(),
+ codeOwnerConfig.key().shortBranchName(),
+ codeOwnerConfigFilePath)));
// Add all data from the original code owner config that is relevant for the path
// (ignoreParentCodeOwners flag, global code owner sets and matching per-file code owner
@@ -246,9 +247,10 @@
getMatchingPerFileCodeOwnerSets(codeOwnerConfig).collect(toImmutableSet());
for (CodeOwnerSet codeOwnerSet : matchingPerFileCodeOwnerSets) {
pathCodeOwnersResultBuilder.addMessage(
- String.format(
- "per-file code owner set with path expressions %s matches",
- codeOwnerSet.pathExpressions()));
+ DebugMessage.createMessage(
+ String.format(
+ "per-file code owner set with path expressions %s matches",
+ codeOwnerSet.pathExpressions())));
pathCodeOwnersResultBuilder.addPerFileCodeOwnerSet(codeOwnerSet);
}
@@ -461,7 +463,7 @@
message = message.substring(0, message.length() - 1);
}
if (!message.isEmpty()) {
- pathCodeOwnersResultBuilder.addMessage(message);
+ pathCodeOwnersResultBuilder.addMessage(DebugMessage.createMessage(message));
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
index 6967763..b0ab171 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
@@ -60,7 +60,7 @@
return !unresolvedImports().isEmpty();
}
- public abstract ImmutableList<String> messages();
+ public abstract ImmutableList<DebugMessage> messages();
/**
* Gets the code owners from the code owner config that apply to the path.
@@ -184,10 +184,11 @@
ignoreGlobalCodeOwners(true);
addMessage(
- String.format(
- "found matching per-file code owner set (with path expressions = %s) that ignores"
- + " parent code owners, hence ignoring the folder code owners",
- perFileCodeOwnerSet.pathExpressions()));
+ DebugMessage.createMessage(
+ String.format(
+ "found matching per-file code owner set (with path expressions = %s) that ignores"
+ + " parent code owners, hence ignoring the folder code owners",
+ perFileCodeOwnerSet.pathExpressions())));
}
}
@@ -235,17 +236,17 @@
return this;
}
- abstract ImmutableList.Builder<String> messagesBuilder();
+ abstract ImmutableList.Builder<DebugMessage> messagesBuilder();
@CanIgnoreReturnValue
- Builder addMessage(String message) {
+ Builder addMessage(DebugMessage message) {
requireNonNull(message, "message");
messagesBuilder().add(message);
return this;
}
@CanIgnoreReturnValue
- Builder addAllMessages(ImmutableList<String> messages) {
+ Builder addAllMessages(ImmutableList<DebugMessage> messages) {
requireNonNull(messages, "messages");
messagesBuilder().addAll(messages);
return this;
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
index 4ffc179..0e97a25 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -50,6 +50,7 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScoring;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScorings;
+import com.google.gerrit.plugins.codeowners.backend.DebugMessage;
import com.google.gerrit.plugins.codeowners.backend.Pair;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
@@ -219,7 +220,7 @@
Set<CodeOwner> codeOwners = new HashSet<>();
ListMultimap<CodeOwner, CodeOwnerAnnotation> annotations = LinkedListMultimap.create();
AtomicBoolean ownedByAllUsers = new AtomicBoolean(false);
- ImmutableList.Builder<String> debugLogsBuilder = ImmutableList.builder();
+ ImmutableList.Builder<DebugMessage> debugLogsBuilder = ImmutableList.builder();
ImmutableList.Builder<CodeOwnerConfigFileInfo> codeOwnerConfigFileInfosBuilder =
ImmutableList.builder();
codeOwnerConfigHierarchy.visit(
@@ -283,7 +284,7 @@
if (!ownedByAllUsers.get()) {
CodeOwnerResolverResult globalCodeOwners = getGlobalCodeOwners(rsrc.getBranch().project());
- debugLogsBuilder.add("resolve global code owners");
+ debugLogsBuilder.add(DebugMessage.createMessage("resolve global code owners"));
debugLogsBuilder.addAll(globalCodeOwners.messages());
globalCodeOwners
@@ -339,15 +340,20 @@
sortedAndLimitedCodeOwners.stream()
.map(codeOwner -> Pair.of(codeOwner, codeOwnerScorings.getScoringsByScore(codeOwner)))
.collect(toImmutableMap(Pair::key, Pair::value));
- ImmutableList<CodeOwnerInfo> codeOwnersInfoList = codeOwnerJsonFactory.create(getFillOptions())
- .format(sortedAndLimitedCodeOwners, codeOwnerToScorings);
+ ImmutableList<CodeOwnerInfo> codeOwnersInfoList =
+ codeOwnerJsonFactory
+ .create(getFillOptions())
+ .format(sortedAndLimitedCodeOwners, codeOwnerToScorings);
CodeOwnersInfo codeOwnersInfo = new CodeOwnersInfo();
codeOwnersInfo.codeOwners = codeOwnersInfoList;
codeOwnersInfo.ownedByAllUsers = ownedByAllUsers.get() ? true : null;
codeOwnersInfo.codeOwnerConfigs = codeOwnerConfigFileInfosBuilder.build();
- ImmutableList<String> debugLogs = debugLogsBuilder.build();
- codeOwnersInfo.debugLogs = debug ? debugLogs : null;
+ ImmutableList<DebugMessage> debugLogs = debugLogsBuilder.build();
+ codeOwnersInfo.debugLogs =
+ debug
+ ? debugLogs.stream().map(DebugMessage::adminMessage).collect(toImmutableList())
+ : null;
logger.atFine().log("debug logs: %s", debugLogs);
return Response.ok(codeOwnersInfo);
@@ -400,7 +406,7 @@
R rsrc,
ImmutableMultimap<CodeOwner, CodeOwnerAnnotation> annotations,
ImmutableSet<CodeOwner> codeOwners,
- ImmutableList.Builder<String> debugLogs) {
+ ImmutableList.Builder<DebugMessage> debugLogs) {
return filterCodeOwners(rsrc, annotations, getVisibleCodeOwners(rsrc, codeOwners), debugLogs)
.collect(toImmutableSet());
}
@@ -418,7 +424,7 @@
R rsrc,
ImmutableMultimap<CodeOwner, CodeOwnerAnnotation> annotations,
Stream<CodeOwner> codeOwners,
- ImmutableList.Builder<String> debugLogs) {
+ ImmutableList.Builder<DebugMessage> debugLogs) {
return codeOwners;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
index b276c00..303f906 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
@@ -37,6 +37,7 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerReference;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
import com.google.gerrit.plugins.codeowners.backend.CodeOwners;
+import com.google.gerrit.plugins.codeowners.backend.DebugMessage;
import com.google.gerrit.plugins.codeowners.backend.FallbackCodeOwners;
import com.google.gerrit.plugins.codeowners.backend.OptionalResultWithMessages;
import com.google.gerrit.plugins.codeowners.backend.PathCodeOwners;
@@ -160,7 +161,7 @@
Path absolutePath = JgitPath.of(path).getAsAbsolutePath();
ImmutableList.Builder<CheckedCodeOwnerConfigFileInfo> checkedCodeOwnerConfigFileInfosBuilder =
ImmutableList.builder();
- List<String> messages = new ArrayList<>();
+ List<DebugMessage> messages = new ArrayList<>();
AtomicBoolean isCodeOwnershipAssignedToEmail = new AtomicBoolean(false);
AtomicBoolean isCodeOwnershipAssignedToAllUsers = new AtomicBoolean(false);
AtomicBoolean isDefaultCodeOwner = new AtomicBoolean(false);
@@ -177,8 +178,10 @@
boolean assignsCodeOwnershipToUser = false;
messages.add(
- String.format(
- "checking code owner config file %s", codeOwnerConfig.key().format(codeOwners)));
+ DebugMessage.createMessage(
+ String.format(
+ "checking code owner config file %s",
+ codeOwnerConfig.key().format(codeOwners))));
PathCodeOwnersResult pathCodeOwnersResult =
pathCodeOwnersFactory
.createWithoutCache(codeOwnerConfig, absolutePath)
@@ -189,7 +192,9 @@
.unresolvedImports()
.forEach(
unresolvedImport ->
- messages.add(unresolvedImportFormatter.format(unresolvedImport)));
+ messages.add(
+ DebugMessage.createMessage(
+ unresolvedImportFormatter.format(unresolvedImport))));
Optional<CodeOwnerReference> codeOwnerReference =
pathCodeOwnersResult.getPathCodeOwners().stream()
.filter(cor -> cor.email().equals(email))
@@ -201,20 +206,25 @@
if (RefNames.isConfigRef(codeOwnerConfig.key().ref())) {
messages.add(
- String.format(
- "found email %s as a code owner in the default code owner config", email));
+ DebugMessage.createMessage(
+ String.format(
+ "found email %s as a code owner in the default code owner config",
+ email)));
isDefaultCodeOwner.set(true);
} else {
Path codeOwnerConfigFilePath = codeOwners.getFilePath(codeOwnerConfig.key());
messages.add(
- String.format(
- "found email %s as a code owner in %s", email, codeOwnerConfigFilePath));
+ DebugMessage.createMessage(
+ String.format(
+ "found email %s as a code owner in %s", email, codeOwnerConfigFilePath)));
}
ImmutableSet<String> localAnnotations = pathCodeOwnersResult.getAnnotationsFor(email);
if (!localAnnotations.isEmpty()) {
messages.add(
- String.format("email %s is annotated with %s", email, sort(localAnnotations)));
+ DebugMessage.createMessage(
+ String.format(
+ "email %s is annotated with %s", email, sort(localAnnotations))));
annotations.addAll(localAnnotations);
}
}
@@ -226,27 +236,30 @@
if (RefNames.isConfigRef(codeOwnerConfig.key().ref())) {
messages.add(
- String.format(
- "found the all users wildcard ('%s') as a code owner in the default code"
- + " owner config which makes %s a code owner",
- CodeOwnerResolver.ALL_USERS_WILDCARD, email));
+ DebugMessage.createMessage(
+ String.format(
+ "found the all users wildcard ('%s') as a code owner in the default code"
+ + " owner config which makes %s a code owner",
+ CodeOwnerResolver.ALL_USERS_WILDCARD, email)));
isDefaultCodeOwner.set(true);
} else {
Path codeOwnerConfigFilePath = codeOwners.getFilePath(codeOwnerConfig.key());
messages.add(
- String.format(
- "found the all users wildcard ('%s') as a code owner in %s which makes %s a"
- + " code owner",
- CodeOwnerResolver.ALL_USERS_WILDCARD, codeOwnerConfigFilePath, email));
+ DebugMessage.createMessage(
+ String.format(
+ "found the all users wildcard ('%s') as a code owner in %s which makes %s a"
+ + " code owner",
+ CodeOwnerResolver.ALL_USERS_WILDCARD, codeOwnerConfigFilePath, email)));
}
ImmutableSet<String> localAnnotations =
pathCodeOwnersResult.getAnnotationsFor(CodeOwnerResolver.ALL_USERS_WILDCARD);
if (!localAnnotations.isEmpty()) {
messages.add(
- String.format(
- "found annotations for the all users wildcard ('%s') which apply to %s: %s",
- CodeOwnerResolver.ALL_USERS_WILDCARD, email, sort(localAnnotations)));
+ DebugMessage.createMessage(
+ String.format(
+ "found annotations for the all users wildcard ('%s') which apply to %s: %s",
+ CodeOwnerResolver.ALL_USERS_WILDCARD, email, sort(localAnnotations))));
annotations.addAll(localAnnotations);
}
}
@@ -259,7 +272,7 @@
}
if (pathCodeOwnersResult.ignoreParentCodeOwners()) {
- messages.add("parent code owners are ignored");
+ messages.add(DebugMessage.createMessage("parent code owners are ignored"));
parentCodeOwnersAreIgnored.set(true);
}
@@ -282,15 +295,17 @@
if (isGlobalCodeOwner(branchResource.getNameKey(), email)) {
isGlobalCodeOwner = true;
- messages.add(String.format("found email %s as global code owner", email));
+ messages.add(
+ DebugMessage.createMessage(String.format("found email %s as global code owner", email)));
isCodeOwnershipAssignedToEmail.set(true);
}
if (isGlobalCodeOwner(branchResource.getNameKey(), CodeOwnerResolver.ALL_USERS_WILDCARD)) {
isGlobalCodeOwner = true;
messages.add(
- String.format(
- "found email %s as global code owner", CodeOwnerResolver.ALL_USERS_WILDCARD));
+ DebugMessage.createMessage(
+ String.format(
+ "found email %s as global code owner", CodeOwnerResolver.ALL_USERS_WILDCARD)));
isCodeOwnershipAssignedToAllUsers.set(true);
}
@@ -331,8 +346,10 @@
.collect(toImmutableSet());
if (!unsupportedAnnotations.isEmpty()) {
messages.add(
- String.format(
- "dropping unsupported annotations for %s: %s", email, sort(unsupportedAnnotations)));
+ DebugMessage.createMessage(
+ String.format(
+ "dropping unsupported annotations for %s: %s",
+ email, sort(unsupportedAnnotations))));
annotations.removeAll(unsupportedAnnotations);
}
@@ -359,7 +376,8 @@
codeOwnerCheckInfo.isGlobalCodeOwner = isGlobalCodeOwner;
codeOwnerCheckInfo.isOwnedByAllUsers = isCodeOwnershipAssignedToAllUsers.get();
codeOwnerCheckInfo.annotations = sort(annotations);
- codeOwnerCheckInfo.debugLogs = messages;
+ codeOwnerCheckInfo.debugLogs =
+ messages.stream().map(DebugMessage::adminMessage).collect(toImmutableList());
return Response.ok(codeOwnerCheckInfo);
}
@@ -436,8 +454,8 @@
OptionalResultWithMessages<CodeOwner> resolveResult =
codeOwnerResolver.resolveWithMessages(CodeOwnerReference.create(email));
- List<String> messages = new ArrayList<>();
- messages.add(String.format("trying to resolve email %s", email));
+ List<DebugMessage> messages = new ArrayList<>();
+ messages.add(DebugMessage.createMessage(String.format("trying to resolve email %s", email)));
messages.addAll(resolveResult.messages());
if (resolveResult.isPresent()) {
return OptionalResultWithMessages.create(resolveResult.get(), messages);
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
index bcd0072..2ee456d 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
@@ -34,6 +34,7 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScoring;
+import com.google.gerrit.plugins.codeowners.backend.DebugMessage;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.server.account.AccountControl;
@@ -143,7 +144,7 @@
CodeOwnersInChangeCollection.PathResource rsrc,
ImmutableMultimap<CodeOwner, CodeOwnerAnnotation> annotations,
Stream<CodeOwner> codeOwners,
- ImmutableList.Builder<String> debugLogs) {
+ ImmutableList.Builder<DebugMessage> debugLogs) {
// The change owner and service users should never be suggested, hence filter them out.
ImmutableList<CodeOwner> filteredCodeOwners =
@@ -171,14 +172,17 @@
}
private Predicate<CodeOwner> filterOutChangeOwner(
- CodeOwnersInChangeCollection.PathResource rsrc, ImmutableList.Builder<String> debugLogs) {
+ CodeOwnersInChangeCollection.PathResource rsrc,
+ ImmutableList.Builder<DebugMessage> debugLogs) {
return codeOwner -> {
if (!codeOwner.accountId().equals(rsrc.getRevisionResource().getChange().getOwner())) {
// Returning true from the Predicate here means that the code owner should be kept.
return true;
}
debugLogs.add(
- String.format("filtering out %s because this code owner is the change owner", codeOwner));
+ DebugMessage.createMessage(
+ String.format(
+ "filtering out %s because this code owner is the change owner", codeOwner)));
// Returning false from the Predicate here means that the code owner should be filtered out.
return false;
};
@@ -187,7 +191,7 @@
private Predicate<CodeOwner> filterOutCodeOwnersThatAreAnnotatedWithLastResortSuggestion(
CodeOwnersInChangeCollection.PathResource rsrc,
ImmutableMultimap<CodeOwner, CodeOwnerAnnotation> annotations,
- ImmutableList.Builder<String> debugLogs) {
+ ImmutableList.Builder<DebugMessage> debugLogs) {
return codeOwner -> {
boolean lastResortSuggestion =
annotations.containsEntry(
@@ -198,9 +202,10 @@
if (isReviewer(rsrc, codeOwner)) {
if (lastResortSuggestion) {
debugLogs.add(
- String.format(
- "ignoring %s annotation for %s because this code owner is a reviewer",
- CodeOwnerAnnotations.LAST_RESORT_SUGGESTION_ANNOTATION.key(), codeOwner));
+ DebugMessage.createMessage(
+ String.format(
+ "ignoring %s annotation for %s because this code owner is a reviewer",
+ CodeOwnerAnnotations.LAST_RESORT_SUGGESTION_ANNOTATION.key(), codeOwner)));
}
// Returning true from the Predicate here means that the code owner should be kept.
@@ -211,9 +216,10 @@
return true;
}
debugLogs.add(
- String.format(
- "filtering out %s because this code owner is annotated with %s",
- codeOwner, CodeOwnerAnnotations.LAST_RESORT_SUGGESTION_ANNOTATION.key()));
+ DebugMessage.createMessage(
+ String.format(
+ "filtering out %s because this code owner is annotated with %s",
+ codeOwner, CodeOwnerAnnotations.LAST_RESORT_SUGGESTION_ANNOTATION.key())));
// Returning false from the Predicate here means that the code owner should be filtered out.
return false;
};
@@ -227,7 +233,8 @@
.contains(codeOwner.accountId());
}
- private Predicate<CodeOwner> filterOutServiceUsers(ImmutableList.Builder<String> debugLogs) {
+ private Predicate<CodeOwner> filterOutServiceUsers(
+ ImmutableList.Builder<DebugMessage> debugLogs) {
if (!cfg.getBoolean(
"suggest", "skipServiceUsers", SuggestReviewers.DEFAULT_SKIP_SERVICE_USERS)) {
// Returning true from the Predicate here means that the code owner should not be filtered
@@ -241,7 +248,9 @@
return true;
}
debugLogs.add(
- String.format("filtering out %s because this code owner is a service user", codeOwner));
+ DebugMessage.createMessage(
+ String.format(
+ "filtering out %s because this code owner is a service user", codeOwner)));
// Returning false from the Predicate here means that the code owner should be filtered out.
return false;
};
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/OptionalResultWithMessagesSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/OptionalResultWithMessagesSubject.java
index 8d46152..0f6cf34 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/OptionalResultWithMessagesSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/OptionalResultWithMessagesSubject.java
@@ -14,13 +14,16 @@
package com.google.gerrit.plugins.codeowners.testing;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertAbout;
import static com.google.gerrit.truth.OptionalSubject.optionals;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IterableSubject;
import com.google.common.truth.Subject;
+import com.google.gerrit.plugins.codeowners.backend.DebugMessage;
import com.google.gerrit.plugins.codeowners.backend.OptionalResultWithMessages;
+import java.util.Optional;
/** {@link Subject} for doing assertions on {@link OptionalResultWithMessages}s. */
public class OptionalResultWithMessagesSubject extends Subject {
@@ -57,8 +60,48 @@
check("result()").about(optionals()).that(optionalResultWithMessages().result()).isEmpty();
}
- public IterableSubject hasMessagesThat() {
- return check("messages()").that(optionalResultWithMessages().messages());
+ public OptionalResultWithMessagesSubject assertContainsAdminOnlyMessage(
+ String expectedAdminMessage) {
+ hasAdminMessagesThat().contains(expectedAdminMessage);
+ hasUserMessagesThat().doesNotContain(expectedAdminMessage);
+ return this;
+ }
+
+ public OptionalResultWithMessagesSubject assertContainsMessage(String expectedMessage) {
+ hasAdminMessagesThat().contains(expectedMessage);
+ hasUserMessagesThat().contains(expectedMessage);
+ return this;
+ }
+
+ public OptionalResultWithMessagesSubject assertContainsMessage(
+ String expectedAdminMessage, String expectedUserMessage) {
+ hasAdminMessagesThat().contains(expectedAdminMessage);
+ hasUserMessagesThat().contains(expectedUserMessage);
+ return this;
+ }
+
+ public OptionalResultWithMessagesSubject assertContainsExactlyMessage(String expectedMessage) {
+ hasAdminMessagesThat().containsExactly(expectedMessage);
+ hasUserMessagesThat().containsExactly(expectedMessage);
+ return this;
+ }
+
+ public IterableSubject hasAdminMessagesThat() {
+ return check("messages()")
+ .that(
+ optionalResultWithMessages().messages().stream()
+ .map(DebugMessage::adminMessage)
+ .collect(toImmutableList()));
+ }
+
+ public IterableSubject hasUserMessagesThat() {
+ return check("messages()")
+ .that(
+ optionalResultWithMessages().messages().stream()
+ .map(DebugMessage::userMessage)
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .collect(toImmutableList()));
}
private OptionalResultWithMessages<?> optionalResultWithMessages() {
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResultTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResultTest.java
index febdf6b..ed1d6c8 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResultTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResultTest.java
@@ -50,7 +50,7 @@
CodeOwnerConfig.Key.create(project, "master", "/bar/"),
CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"),
"test message")),
- ImmutableList.of("test message"));
+ ImmutableList.of(DebugMessage.createMessage("test message")));
assertThatToStringIncludesAllData(codeOwnerResolverResult, CodeOwnerResolverResult.class);
}
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
index ec97a36..7fc6cec 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
@@ -93,10 +93,12 @@
.resolveWithMessages(CodeOwnerReference.create(nonExistingEmail));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
- String.format(
+ .assertContainsMessage(
+ /* adminMessage= */ String.format(
"cannot resolve code owner email %s: no account with this email exists",
+ nonExistingEmail),
+ /* userMessage= */ String.format(
+ "cannot resolve code owner email %s: email doesn't exist or is not visible",
nonExistingEmail));
}
@@ -108,8 +110,8 @@
.resolveWithMessages(CodeOwnerReference.create(admin.email()));
assertThat(result.get()).hasAccountIdThat().isEqualTo(admin.id());
assertThat(result)
- .hasMessagesThat()
- .contains(String.format("account %s is visible to user %s", admin.id(), admin.username()));
+ .assertContainsMessage(
+ String.format("account %s is visible to user %s", admin.id(), admin.username()));
}
@Test
@@ -120,8 +122,7 @@
.resolveWithMessages(CodeOwnerReference.create(CodeOwnerResolver.ALL_USERS_WILDCARD));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
+ .assertContainsMessage(
String.format(
"cannot resolve code owner email %s: no account with this email exists",
CodeOwnerResolver.ALL_USERS_WILDCARD));
@@ -170,9 +171,12 @@
.resolveWithMessages(CodeOwnerReference.create(admin.email()));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
- String.format("cannot resolve code owner email %s: email is ambiguous", admin.email()));
+ .assertContainsMessage(
+ /* adminMessage= */ String.format(
+ "cannot resolve code owner email %s: email is ambiguous", admin.email()),
+ /* userMessage= */ String.format(
+ "cannot resolve code owner email %s: email doesn't exist or is not visible",
+ admin.email()));
}
@Test
@@ -191,11 +195,13 @@
codeOwnerResolverProvider.get().resolveWithMessages(CodeOwnerReference.create(email));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
- String.format(
+ .assertContainsMessage(
+ /* adminMessage= */ String.format(
"cannot resolve account %s for email %s: account does not exists",
- accountId, email));
+ accountId, email),
+ /* userMessage= */ String.format(
+ "cannot resolve code owner email %s: email doesn't exist or is not visible",
+ email));
}
@Test
@@ -207,8 +213,7 @@
.resolveWithMessages(CodeOwnerReference.create(user.email()));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
+ .assertContainsMessage(
String.format("ignoring inactive account %s for email %s", user.id(), user.email()));
}
@@ -228,11 +233,13 @@
.resolveWithMessages(CodeOwnerReference.create(admin.email()));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
- String.format(
+ .assertContainsMessage(
+ /* adminMessage= */ String.format(
"cannot resolve code owner email %s: account %s is not visible to user %s",
- admin.email(), admin.id(), user2.username()));
+ admin.email(), admin.id(), user2.username()),
+ /* userMessage= */ String.format(
+ "cannot resolve code owner email %s: email doesn't exist or is not visible",
+ admin.email()));
}
@Test
@@ -251,8 +258,7 @@
.resolveWithMessages(CodeOwnerReference.create(secondaryEmail));
assertThat(result.get()).hasAccountIdThat().isEqualTo(user.id());
assertThat(result)
- .hasMessagesThat()
- .contains(
+ .assertContainsMessage(
String.format(
"resolved code owner email %s: account %s is referenced by secondary email and the"
+ " calling user %s can see secondary emails",
@@ -268,8 +274,7 @@
.resolveWithMessages(CodeOwnerReference.create(secondaryEmail));
assertThat(result.get()).hasAccountIdThat().isEqualTo(user.id());
assertThat(result)
- .hasMessagesThat()
- .contains(
+ .assertContainsAdminOnlyMessage(
String.format(
"resolved code owner email %s: account %s is referenced by secondary email and user"
+ " %s can see secondary emails",
@@ -283,8 +288,7 @@
.resolveWithMessages(CodeOwnerReference.create(secondaryEmail));
assertThat(result.get()).hasAccountIdThat().isEqualTo(user.id());
assertThat(result)
- .hasMessagesThat()
- .contains(
+ .assertContainsMessage(
String.format(
"email %s is visible to the calling user %s: email is a secondary email that is"
+ " owned by this user",
@@ -299,8 +303,7 @@
.resolveWithMessages(CodeOwnerReference.create(secondaryEmail));
assertThat(result.get()).hasAccountIdThat().isEqualTo(user.id());
assertThat(result)
- .hasMessagesThat()
- .contains(
+ .assertContainsAdminOnlyMessage(
String.format(
"email %s is visible to user %s: email is a secondary email that is owned by this"
+ " user",
@@ -322,12 +325,14 @@
.resolveWithMessages(CodeOwnerReference.create(secondaryEmail));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
- String.format(
+ .assertContainsMessage(
+ /* adminMessage= */ String.format(
"cannot resolve code owner email %s: account %s is referenced by secondary email"
+ " but the calling user %s cannot see secondary emails",
- secondaryEmail, admin.id(), user.username()));
+ secondaryEmail, admin.id(), user.username()),
+ /* userMessage= */ String.format(
+ "cannot resolve code owner email %s: email doesn't exist or is not visible",
+ secondaryEmail));
// user doesn't have the "Modify Account" global capability and hence cannot see the secondary
// email of the admin account if another user is the calling user
@@ -339,8 +344,7 @@
.resolveWithMessages(CodeOwnerReference.create(secondaryEmail));
assertThat(result).isEmpty();
assertThat(result)
- .hasMessagesThat()
- .contains(
+ .assertContainsAdminOnlyMessage(
String.format(
"cannot resolve code owner email %s: account %s is referenced by secondary email"
+ " but user %s cannot see secondary emails",
@@ -678,7 +682,7 @@
OptionalResultWithMessages<Boolean> isEmailDomainAllowedResult =
codeOwnerResolverProvider.get().isEmailDomainAllowed(email);
assertThat(isEmailDomainAllowedResult.get()).isEqualTo(expectedResult);
- assertThat(isEmailDomainAllowedResult.messages()).containsExactly(expectedMessage);
+ assertThat(isEmailDomainAllowedResult).assertContainsExactlyMessage(expectedMessage);
}
@Test
