Fix NPE when returning group owners without expansion
When using OWNERS with matchers and no group expansion,
the group owners were not applied to the final matching
list resulting in an NPE when returning the submit rule
or serving the GetFilesOwners REST-API.
Cover the OWNERS with matchers use-case in the integration
tests.
Change-Id: I973e6383d7db8c83e7efce0866e210282d24097e
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
index 33b9965..02c3fdf 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
@@ -98,7 +98,10 @@
.flatMap(o -> accounts.find(o).stream())
.collect(Collectors.toSet());
Set<String> groupOwners =
- flatten(getNode(node, "owners").map(ConfigurationParser::extractAsText))
+ flatten(
+ getNode(node, "owners")
+ .map(ConfigurationParser::extractAsText)
+ .map(owns -> owns.map(PathOwnersEntry::stripOwnerDomain)))
.collect(Collectors.toSet());
Set<Id> reviewers =
getNode(node, "reviewers")
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java
index 4ef8d13..321b588 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java
@@ -35,13 +35,25 @@
@Override
public String toString() {
- return "Matcher [path=" + path + ", owners=" + owners + ", reviewers=" + reviewers + "]";
+ return "Matcher [path="
+ + path
+ + ", owners="
+ + owners
+ + ", groupOwners="
+ + groupOwners
+ + ", reviewers="
+ + reviewers
+ + "]";
}
public Set<Account.Id> getOwners() {
return owners;
}
+ public Set<String> getGroupOwners() {
+ return groupOwners;
+ }
+
public void setOwners(Set<Account.Id> owners) {
this.owners = owners;
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
index 7e3a311..681b2ea 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
@@ -227,6 +227,7 @@
if (matcher.matches(path)) {
newMatchers.put(matcher.getPath(), matcher);
ownersMap.addFileOwners(path, matcher.getOwners());
+ ownersMap.addFileGroupOwners(path, matcher.getGroupOwners());
ownersMap.addFileReviewers(path, matcher.getReviewers());
}
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
index ee9c8dc..168d8bd 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
@@ -142,7 +142,7 @@
return this.matchers.containsKey(path);
}
- private static String stripOwnerDomain(String owner) {
+ public static String stripOwnerDomain(String owner) {
return Splitter.on('@').split(owner).iterator().next();
}
}
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
index 0c5967b..cd23d0d 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
@@ -90,6 +90,19 @@
}
@Test
+ @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
+ public void shouldReturnResponseWithUnexpandedFileMatchersOwners() throws Exception {
+ addOwnerFileWithMatchersToRoot(true);
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
+ }
+
+ @Test
@UseLocalDisk
public void shouldReturnInheritedOwnersFromProjectsOwners() throws Exception {
assertInheritFromProject(project);
@@ -174,4 +187,24 @@
String.format("inherited: %s\nowners:\n- %s\n", inherit, admin.email()),
""));
}
+
+ private void addOwnerFileWithMatchersToRoot(boolean inherit) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // matchers:
+ // - suffix: .txt
+ // owners:
+ // - admin@mail.com
+ merge(
+ createChange(
+ testRepo,
+ "master",
+ "Add OWNER file",
+ "OWNERS",
+ String.format(
+ "inherited: %s\nmatchers:\n" + "- suffix: .txt\n owners:\n - %s\n",
+ inherit, admin.email()),
+ ""));
+ }
}
