CodeOwnerApprovalCheck: Remove wrong shortcut for fallback code owners
getFileStatusesForAccount(...) returned all files as approved if project
owners were configured as fallback code owners and the given user was a
project owner. Since fallback code owners are only applied for files
for which no code owners have been defined, doing this was incorrect if
a file had explcit code owners defined, but the given user was not
amongst them.
Since this was only a shortcut, we can simply drop it and the full
computation will compute the correct status.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I686bf7340e59c6ff852459113d8a412903e82910
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index 65f4d78..0e34b9f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -408,9 +408,6 @@
FallbackCodeOwners fallbackCodeOwners = codeOwnersConfig.getFallbackCodeOwners();
logger.atFine().log(
"fallbackCodeOwner = %s, isProjectOwner = %s", fallbackCodeOwners, isProjectOwner);
- if (fallbackCodeOwners.equals(FallbackCodeOwners.PROJECT_OWNERS) && isProjectOwner) {
- return getAllPathsAsApproved(changeNotes, patchSet);
- }
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy = codeOwnerConfigHierarchyProvider.get();
return changedFiles.getOrCompute(changeNotes.getProjectName(), patchSet.commitId()).stream()
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckForAccountTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckForAccountTest.java
index 5693c57..1220c6c 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckForAccountTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckForAccountTest.java
@@ -257,6 +257,36 @@
.isEqualTo(CodeOwnerStatus.APPROVED);
}
+ @Test
+ @GerritConfig(name = "plugin.code-owners.fallbackCodeOwners", value = "PROJECT_OWNERS")
+ public void notApprovedByProjectOwner_projectOwnersAreFallbackCodeOwner_otherOwnerDefined()
+ throws Exception {
+ TestAccount codeOwner =
+ accountCreator.create(
+ "codeOwner", "codeOwner@example.com", "CodeOwner", /* displayName= */ null);
+ setAsRootCodeOwners(codeOwner);
+
+ Path path = Paths.get("/foo/bar.baz");
+ String changeId =
+ createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
+ ChangeNotes changeNotes = getChangeNotes(changeId);
+
+ // Verify that the file would be not approved by the 'admin' user. The 'admin' user is a
+ // project owner, but fallback code owners are not applied if code ownership was explicitly
+ // defined.
+ Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatusesForAccount(
+ changeNotes, changeNotes.getCurrentPatchSet(), admin.id());
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+ assertThatStream(fileCodeOwnerStatuses).onlyElement();
+ fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+ fileCodeOwnerStatusSubject
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ }
+
@GerritConfig(name = "plugin.code-owners.fallbackCodeOwners", value = "ALL_USERS")
@Test
public void approvedByFallbackCodeOwner() throws Exception {
