Fix sorting in code owner suggestion if file is owned by all users
If an OWNERS file assigns code ownership to all users and a concrete
user, we didn't include any distance score for the concrete user. This
was because we returned early from the callback when a file is owned by
all users and the step to add distance scores for the concrete user was
skipped.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I80e99fa175411a2ce11bf409600115fc69c142ee
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
index 0644b4c..490343a 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -179,6 +179,15 @@
codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, rsrc.getPath());
codeOwners.addAll(filterCodeOwners(rsrc, pathCodeOwners.codeOwners()));
+ int distance =
+ codeOwnerConfig.key().branchNameKey().branch().equals(RefNames.REFS_CONFIG)
+ ? defaultOwnersDistance
+ : rootDistance - codeOwnerConfig.key().folderPath().getNameCount();
+ pathCodeOwners
+ .codeOwners()
+ .forEach(
+ localCodeOwner -> distanceScoring.putValueForCodeOwner(localCodeOwner, distance));
+
if (pathCodeOwners.ownedByAllUsers()) {
ownedByAllUsers.set(true);
fillUpWithRandomUsers(rsrc, codeOwners, limit);
@@ -190,19 +199,8 @@
+ " (wanted number of suggestions = %d, got = %d",
limit, codeOwners.size());
}
-
- return true;
}
- int distance =
- codeOwnerConfig.key().branchNameKey().branch().equals(RefNames.REFS_CONFIG)
- ? defaultOwnersDistance
- : rootDistance - codeOwnerConfig.key().folderPath().getNameCount();
- pathCodeOwners
- .codeOwners()
- .forEach(
- localCodeOwner -> distanceScoring.putValueForCodeOwner(localCodeOwner, distance));
-
return true;
});
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInChangeIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInChangeIT.java
index e0802a1..1feefc6 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInChangeIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnersForPathInChangeIT.java
@@ -36,6 +36,7 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.api.CodeOwners;
import com.google.gerrit.plugins.codeowners.api.CodeOwnersInfo;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.inject.Inject;
import java.util.List;
@@ -389,4 +390,43 @@
.containsExactly(user2.id(), user.id(), admin.id())
.inOrder();
}
+
+ @Test
+ public void codeOwnersSortedByDistance_fileOwnedByAllUsers() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(CodeOwnerResolver.ALL_USERS_WILDCARD)
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ // None of the code owners is a reviewer, hence the sorting is done only by distance.
+ CodeOwnersInfo codeOwnersInfo = queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user2.id(), user.id(), admin.id())
+ .inOrder();
+ assertThat(codeOwnersInfo).hasOwnedByAllUsersThat().isTrue();
+ }
}
