CodeOwnerResolverResult: Include flag if there are unresolved code owners
Android wants a configuration option to make all users code owners of
paths for which no code owners are defined:
1. if for a path no code owners are defined, the path is owned by all
users
2. if for a path code owners are defined, only these users own the path
3. if for a path code owners are defined, but none of them is resolvable
the path is owned by no one and changes require an override for
submission
As a first step for adding such a configuration option we extend
CodeOwnerResolverResult with a flag that is set if there are unresolved
code owners. This allows callers to differentiate the cases above:
1. codeOwners is empty, ownedByAllUsers = false,
hasUnresolvedCodeOwners = false
2. codeOwner is non-empty or ownedByAllUsers = true
3. codeOwners is empty, ownedByAllUsers = false,
hasUnresolvedCodeOwners = true
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iab1abf7a139dcead9e8ccad55b989a00d53ff8fc
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index c6b5f55..205e931 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -177,6 +177,7 @@
public CodeOwnerResolverResult resolve(Set<CodeOwnerReference> codeOwnerReferences) {
requireNonNull(codeOwnerReferences, "codeOwnerReferences");
AtomicBoolean ownedByAllUsers = new AtomicBoolean(false);
+ AtomicBoolean hasUnresolvedCodeOwners = new AtomicBoolean(false);
ImmutableSet<CodeOwner> codeOwners =
codeOwnerReferences.stream()
.filter(
@@ -188,10 +189,18 @@
return true;
})
.map(this::resolve)
- .filter(Optional::isPresent)
+ .filter(
+ codeOwner -> {
+ if (!codeOwner.isPresent()) {
+ hasUnresolvedCodeOwners.set(true);
+ return false;
+ }
+ return true;
+ })
.map(Optional::get)
.collect(toImmutableSet());
- return CodeOwnerResolverResult.create(codeOwners, ownedByAllUsers.get());
+ return CodeOwnerResolverResult.create(
+ codeOwners, ownedByAllUsers.get(), hasUnresolvedCodeOwners.get());
}
/**
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
index 226d541..cd11262 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverResult.java
@@ -43,17 +43,24 @@
*/
public abstract boolean ownedByAllUsers();
+ /** Whether there are code owner references which couldn't be resolved. */
+ public abstract boolean hasUnresolvedCodeOwners();
+
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("codeOwners", codeOwners())
.add("ownedByAllUsers", ownedByAllUsers())
+ .add("hasUnresolvedCodeOwners", hasUnresolvedCodeOwners())
.toString();
}
/** Creates a {@link CodeOwnerResolverResult} instance. */
public static CodeOwnerResolverResult create(
- ImmutableSet<CodeOwner> codeOwners, boolean ownedByAllUsers) {
- return new AutoValue_CodeOwnerResolverResult(codeOwners, ownedByAllUsers);
+ ImmutableSet<CodeOwner> codeOwners,
+ boolean ownedByAllUsers,
+ boolean hasUnresolvedCodeOwners) {
+ return new AutoValue_CodeOwnerResolverResult(
+ codeOwners, ownedByAllUsers, hasUnresolvedCodeOwners);
}
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
index cefe297..86652b2 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
@@ -192,6 +192,7 @@
codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
assertThat(result.codeOwners()).isEmpty();
assertThat(result.ownedByAllUsers()).isFalse();
+ assertThat(result.hasUnresolvedCodeOwners()).isFalse();
}
@Test
@@ -205,6 +206,7 @@
codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id(), user.id());
assertThat(result.ownedByAllUsers()).isFalse();
+ assertThat(result.hasUnresolvedCodeOwners()).isFalse();
}
@Test
@@ -219,6 +221,7 @@
codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
assertThat(result.codeOwnersAccountIds()).isEmpty();
assertThat(result.ownedByAllUsers()).isTrue();
+ assertThat(result.hasUnresolvedCodeOwners()).isFalse();
}
@Test
@@ -233,6 +236,23 @@
codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id());
assertThat(result.ownedByAllUsers()).isFalse();
+ assertThat(result.hasUnresolvedCodeOwners()).isTrue();
+ }
+
+ @Test
+ public void resolvePathCodeOwnersNonResolvableCodeOwnersAreFilteredOutIfOwnedByAllUsers()
+ throws Exception {
+ CodeOwnerConfig codeOwnerConfig =
+ CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION)
+ .addCodeOwnerSet(
+ CodeOwnerSet.createWithoutPathExpressions(
+ "*", admin.email(), "non-existing@example.com"))
+ .build();
+ CodeOwnerResolverResult result =
+ codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
+ assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id());
+ assertThat(result.ownedByAllUsers()).isTrue();
+ assertThat(result.hasUnresolvedCodeOwners()).isTrue();
}
@Test
@@ -386,6 +406,21 @@
CodeOwnerReference.create(user.email())));
assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id(), user.id());
assertThat(result.ownedByAllUsers()).isFalse();
+ assertThat(result.hasUnresolvedCodeOwners()).isFalse();
+ }
+
+ @Test
+ public void resolveCodeOwnerReferencesNonResolveableCodeOwnersAreFilteredOut() throws Exception {
+ CodeOwnerResolverResult result =
+ codeOwnerResolver
+ .get()
+ .resolve(
+ ImmutableSet.of(
+ CodeOwnerReference.create(admin.email()),
+ CodeOwnerReference.create("non-existing@example.com")));
+ assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id());
+ assertThat(result.ownedByAllUsers()).isFalse();
+ assertThat(result.hasUnresolvedCodeOwners()).isTrue();
}
@Test
