CheckCodeOwner: Include annotations into the response
This REST endpoint is used to debug code ownerships. Since annotations
are important they should be returned as well.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I687494d8a1e1654809d09014a207c2361a4515de
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerCheckInfo.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerCheckInfo.java
index c96645c..19b02a0 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerCheckInfo.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerCheckInfo.java
@@ -131,6 +131,16 @@
/** Whether the the specified path in the branch is owned by all users (aka {@code *}). */
public boolean isOwnedByAllUsers;
+ /**
+ * Annotations that were set for the user.
+ *
+ * <p>Contains only supported annotations (unsupported annotations are reported in the {@link
+ * #debugLogs}).
+ *
+ * <p>Sorted alphabetically.
+ */
+ public List<String> annotations;
+
/** Debug logs that may help to understand why the user is or isn't a code owner. */
public List<String> debugLogs;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerAnnotations.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerAnnotations.java
index b3977ff..a9d3edb 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerAnnotations.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerAnnotations.java
@@ -14,6 +14,9 @@
package com.google.gerrit.plugins.codeowners.backend;
+import java.util.ArrayList;
+import java.util.List;
+
/** Class that defines all known/supported {@link CodeOwnerAnnotation}s on code owners. */
public class CodeOwnerAnnotations {
/**
@@ -23,6 +26,18 @@
public static final CodeOwnerAnnotation NEVER_SUGGEST_ANNOTATION =
CodeOwnerAnnotation.create("NEVER_SUGGEST");
+ private static final List<String> KEYS_ALL;
+
+ static {
+ KEYS_ALL = new ArrayList<>();
+ KEYS_ALL.add(NEVER_SUGGEST_ANNOTATION.key());
+ }
+
+ /** Whether the given annotation is known and supported. */
+ public static boolean isSupported(String annotation) {
+ return KEYS_ALL.contains(annotation);
+ }
+
/**
* Private constructor to prevent instantiation of this class.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
index 5b2ef36..93a36be 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
@@ -83,6 +83,13 @@
return annotations;
}
+ /** Gets the annotations for the given email. */
+ public ImmutableSet<String> getAnnotationsFor(String email) {
+ return getAnnotations().get(CodeOwnerReference.create(email)).stream()
+ .map(CodeOwnerAnnotation::key)
+ .collect(toImmutableSet());
+ }
+
/**
* Whether parent code owners should be ignored for the path.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
index f4e5c0f..4f65355 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
@@ -14,8 +14,12 @@
package com.google.gerrit.plugins.codeowners.restapi;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.stream.Collectors.toList;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -27,6 +31,7 @@
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerCheckInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwner;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerAnnotations;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigHierarchy;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerReference;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
@@ -57,8 +62,10 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
@@ -157,6 +164,7 @@
AtomicBoolean isDefaultCodeOwner = new AtomicBoolean(false);
AtomicBoolean hasRevelantCodeOwnerDefinitions = new AtomicBoolean(false);
AtomicBoolean parentCodeOwnersAreIgnored = new AtomicBoolean(false);
+ Set<String> annotations = new HashSet<>();
codeOwnerConfigHierarchy.visit(
branchResource.getBranchKey(),
ObjectId.fromString(branchResource.getRevision()),
@@ -196,6 +204,14 @@
"found email %s as code owner in %s", email, codeOwnerConfigFilePath));
codeOwnerConfigFilePaths.add(codeOwnerConfigFilePath);
}
+
+ ImmutableSet<String> localAnnotations =
+ pathCodeOwnersResult.get().getAnnotationsFor(email);
+ if (!localAnnotations.isEmpty()) {
+ messages.add(
+ String.format("email %s is annotated with %s", email, sort(localAnnotations)));
+ annotations.addAll(localAnnotations);
+ }
}
if (pathCodeOwnersResult.get().getPathCodeOwners().stream()
@@ -218,6 +234,16 @@
codeOwnerConfigFilePaths.add(codeOwnerConfigFilePath);
}
}
+
+ ImmutableSet<String> localAnnotations =
+ pathCodeOwnersResult.get().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)));
+ annotations.addAll(localAnnotations);
+ }
}
if (codeOwnerResolverProvider
@@ -282,6 +308,17 @@
}
}
+ ImmutableSet<String> unsupportedAnnotations =
+ annotations.stream()
+ .filter(annotation -> !CodeOwnerAnnotations.isSupported(annotation))
+ .collect(toImmutableSet());
+ if (!unsupportedAnnotations.isEmpty()) {
+ messages.add(
+ String.format(
+ "dropping unsupported annotations for %s: %s", email, sort(unsupportedAnnotations)));
+ annotations.removeAll(unsupportedAnnotations);
+ }
+
boolean isFallbackCodeOwner =
!isCodeOwnershipAssignedToEmail.get()
&& !isCodeOwnershipAssignedToAllUsers.get()
@@ -305,6 +342,7 @@
codeOwnerCheckInfo.isDefaultCodeOwner = isDefaultCodeOwner.get();
codeOwnerCheckInfo.isGlobalCodeOwner = isGlobalCodeOwner;
codeOwnerCheckInfo.isOwnedByAllUsers = isCodeOwnershipAssignedToAllUsers.get();
+ codeOwnerCheckInfo.annotations = sort(annotations);
codeOwnerCheckInfo.debugLogs = messages;
return Response.ok(codeOwnerCheckInfo);
}
@@ -408,4 +446,8 @@
}
return OptionalResultWithMessages.createEmpty(messages);
}
+
+ private ImmutableList<String> sort(Set<String> set) {
+ return set.stream().sorted().collect(toImmutableList());
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerCheckInfoSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerCheckInfoSubject.java
index 31fa4e0..3128b7f 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerCheckInfoSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerCheckInfoSubject.java
@@ -133,6 +133,10 @@
check("isOwnedByAllUsers").that(codeOwnerCheckInfo().isOwnedByAllUsers).isFalse();
}
+ public IterableSubject hasAnnotationsThat() {
+ return check("annotations").that(codeOwnerCheckInfo().annotations);
+ }
+
public void hasDebugLogsThatContainAllOf(String... expectedMessages) {
for (String expectedMessage : expectedMessages) {
check("debugLogs").that(codeOwnerCheckInfo().debugLogs).contains(expectedMessage);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
index 0f12736..582617a 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
@@ -23,6 +23,7 @@
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
@@ -44,6 +45,8 @@
import com.google.gerrit.plugins.codeowners.acceptance.testsuite.TestCodeOwnerConfigCreation;
import com.google.gerrit.plugins.codeowners.acceptance.testsuite.TestPathExpressions;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerCheckInfo;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerAnnotation;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerAnnotations;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigImportMode;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigReference;
@@ -144,6 +147,7 @@
assertThat(checkCodeOwnerInfo).isNotDefaultCodeOwner();
assertThat(checkCodeOwnerInfo).isNotGlobalCodeOwner();
assertThat(checkCodeOwnerInfo).isNotOwnedByAllUsers();
+ assertThat(checkCodeOwnerInfo).hasAnnotationsThat().isEmpty();
assertThat(checkCodeOwnerInfo)
.hasDebugLogsThatContainAllOf(
String.format(
@@ -1454,6 +1458,89 @@
assertThat(checkCodeOwnerInfo).cannotApproveChange();
}
+ @Test
+ public void checkCodeOwnerWithAnnotations() throws Exception {
+ skipTestIfAnnotationsNotSupportedByCodeOwnersBackend();
+
+ TestAccount codeOwner =
+ accountCreator.create(
+ "codeOwner", "codeOwner@example.com", "Code Owner", /* displayName= */ null);
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addCodeOwnerEmail(codeOwner.email())
+ .addAnnotation(codeOwner.email(), CodeOwnerAnnotations.NEVER_SUGGEST_ANNOTATION)
+ .build())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addCodeOwnerEmail(CodeOwnerResolver.ALL_USERS_WILDCARD)
+ .addAnnotation(
+ CodeOwnerResolver.ALL_USERS_WILDCARD, CodeOwnerAnnotation.create("ANNOTATION"))
+ .build())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addCodeOwnerEmail(codeOwner.email())
+ .addAnnotation(codeOwner.email(), CodeOwnerAnnotations.NEVER_SUGGEST_ANNOTATION)
+ .addAnnotation(codeOwner.email(), CodeOwnerAnnotation.create("OTHER_ANNOTATION"))
+ .build())
+ .create();
+
+ String path = "/foo/bar/baz.md";
+ CodeOwnerCheckInfo checkCodeOwnerInfo = checkCodeOwner(path, codeOwner.email());
+ assertThat(checkCodeOwnerInfo).isCodeOwner();
+ assertThat(checkCodeOwnerInfo)
+ .hasAnnotationsThat()
+ .containsExactly(CodeOwnerAnnotations.NEVER_SUGGEST_ANNOTATION.key())
+ .inOrder();
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatContainAllOf(
+ String.format(
+ "found email %s as code owner in %s",
+ codeOwner.email(), getCodeOwnerConfigFilePath("/foo/bar/")),
+ String.format(
+ "email %s is annotated with %s",
+ codeOwner.email(),
+ ImmutableSet.of(
+ CodeOwnerAnnotations.NEVER_SUGGEST_ANNOTATION.key(), "OTHER_ANNOTATION")),
+ String.format(
+ "found email %s as code owner in %s",
+ CodeOwnerResolver.ALL_USERS_WILDCARD, getCodeOwnerConfigFilePath("/foo/")),
+ String.format(
+ "found annotations for the all users wildcard ('%s') which apply to %s: %s",
+ CodeOwnerResolver.ALL_USERS_WILDCARD,
+ codeOwner.email(),
+ ImmutableSet.of("ANNOTATION")),
+ String.format(
+ "found email %s as code owner in %s",
+ codeOwner.email(), getCodeOwnerConfigFilePath("/")),
+ String.format(
+ "email %s is annotated with %s",
+ codeOwner.email(),
+ ImmutableSet.of(CodeOwnerAnnotations.NEVER_SUGGEST_ANNOTATION.key())),
+ String.format(
+ "dropping unsupported annotations for %s: %s",
+ codeOwner.email(), ImmutableSet.of("ANNOTATION", "OTHER_ANNOTATION")));
+ }
+
private CodeOwnerCheckInfo checkCodeOwner(String path, String email) throws RestApiException {
return checkCodeOwner(path, email, /* user= */ null);
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 0ab7345..90d6055 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -847,6 +847,7 @@
| `is_global_code_owner` | Whether the given email is configured as a global
code owner. Note that if the email is configured as global code owner, but the email is not resolvable (see `is_resolvable` field), the user is not a code owner.
| `is_owned_by_all_users` | Whether the the specified path in the branch is owned by all users (aka `*`).
+| `annotation` | Annotations that were set for the user. Contains only supported annotations (unsupported annotations are reported in the `debugs_logs`). Sorted alphabetically.
| `debug_logs` | List of debug logs that may help to understand why the user is or isn't a code owner.
---
