Merge "Add code owner scorings to CodeOwnersInfo"
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerInfo.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerInfo.java
index c141bea..795313b 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerInfo.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerInfo.java
@@ -15,6 +15,7 @@
package com.google.gerrit.plugins.codeowners.api;
import com.google.gerrit.extensions.common.AccountInfo;
+import java.util.Map;
/**
* Representation of a code owner in the REST API.
@@ -24,4 +25,10 @@
public class CodeOwnerInfo {
/** The account of the code owner. */
public AccountInfo account;
+
+ /**
+ * The scorings for code owners on a particular scores that express how good the code owners are
+ * considered as reviewers.
+ */
+ public Map<String, Integer> scorings;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerScorings.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerScorings.java
index 84bddff..7a283cd 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerScorings.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerScorings.java
@@ -17,8 +17,11 @@
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import java.util.HashMap;
+import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -60,4 +63,14 @@
.collect(Collectors.summingDouble(Double::doubleValue));
return sum;
}
+
+ /** Returns the map of all unweighted scorings that available for the given code owner. */
+ public ImmutableMap<CodeOwnerScore, Integer> getScoringsByScore(CodeOwner codeOwner) {
+ ImmutableMap.Builder<CodeOwnerScore, Integer> builder = ImmutableMap.builder();
+ for (CodeOwnerScoring scoring : scorings()) {
+ scoring.bestValue(codeOwner)
+ .ifPresent(value -> builder.put(scoring.score(), value));
+ }
+ return builder.build();
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
index 1ddaa9b..4ffc179 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -15,6 +15,7 @@
package com.google.gerrit.plugins.codeowners.restapi;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore.IS_EXPLICITLY_MENTIONED_SCORING_VALUE;
import static com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore.NOT_EXPLICITLY_MENTIONED_SCORING_VALUE;
@@ -39,6 +40,7 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerConfigFileInfo;
+import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnersInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwner;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerAnnotation;
@@ -48,6 +50,7 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScoring;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScorings;
+import com.google.gerrit.plugins.codeowners.backend.Pair;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.server.account.AccountControl;
@@ -332,10 +335,15 @@
.collect(toImmutableList());
}
}
+ ImmutableMap<CodeOwner, ImmutableMap<CodeOwnerScore, Integer>> codeOwnerToScorings =
+ sortedAndLimitedCodeOwners.stream()
+ .map(codeOwner -> Pair.of(codeOwner, codeOwnerScorings.getScoringsByScore(codeOwner)))
+ .collect(toImmutableMap(Pair::key, Pair::value));
+ ImmutableList<CodeOwnerInfo> codeOwnersInfoList = codeOwnerJsonFactory.create(getFillOptions())
+ .format(sortedAndLimitedCodeOwners, codeOwnerToScorings);
CodeOwnersInfo codeOwnersInfo = new CodeOwnersInfo();
- codeOwnersInfo.codeOwners =
- codeOwnerJsonFactory.create(getFillOptions()).format(sortedAndLimitedCodeOwners);
+ codeOwnersInfo.codeOwners = codeOwnersInfoList;
codeOwnersInfo.ownedByAllUsers = ownedByAllUsers.get() ? true : null;
codeOwnersInfo.codeOwnerConfigs = codeOwnerConfigFileInfosBuilder.build();
ImmutableList<String> debugLogs = debugLogsBuilder.build();
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJson.java
index 085717c..7739500 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJson.java
@@ -16,11 +16,15 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwner;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
+import com.google.gerrit.plugins.codeowners.backend.Pair;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -58,14 +62,17 @@
* Formats the provided {@link CodeOwner}s as {@link CodeOwnerInfo}s.
*
* @param codeOwners the code owners that should be formatted as {@link CodeOwnerInfo}s
+ * @param scorings provides the scorings data that should be populated in {@link CodeOwnerInfo}s
* @return the provided code owners as {@link CodeOwnerInfo}s
*/
- ImmutableList<CodeOwnerInfo> format(ImmutableList<CodeOwner> codeOwners)
+ ImmutableList<CodeOwnerInfo> format(
+ ImmutableList<CodeOwner> codeOwners,
+ ImmutableMap<CodeOwner, ImmutableMap<CodeOwnerScore, Integer>> scorings)
throws PermissionBackendException {
AccountLoader accountLoader = accountLoaderFactory.create(accountOptions);
ImmutableList<CodeOwnerInfo> codeOwnerInfos =
requireNonNull(codeOwners, "codeOwners").stream()
- .map(codeOwner -> format(accountLoader, codeOwner))
+ .map(codeOwner -> format(accountLoader, codeOwner, scorings.get(codeOwner)))
.collect(toImmutableList());
accountLoader.fill();
return codeOwnerInfos;
@@ -77,11 +84,18 @@
* @param accountLoader the account loader instance that should be used to create the {@link
* com.google.gerrit.extensions.common.AccountInfo} in the code owner info
* @param codeOwner the code owner that should be formatted as {@link CodeOwnerInfo}
+ * @param scorings the scorings data that should be populated in {@link CodeOwnerInfo}s
* @return the provided code owner as {@link CodeOwnerInfo}
*/
- private static CodeOwnerInfo format(AccountLoader accountLoader, CodeOwner codeOwner) {
+ private static CodeOwnerInfo format(AccountLoader accountLoader, CodeOwner codeOwner,
+ ImmutableMap<CodeOwnerScore, Integer> scorings) {
CodeOwnerInfo info = new CodeOwnerInfo();
info.account = accountLoader.get(requireNonNull(codeOwner, "codeOwner").accountId());
+ if (scorings != null) {
+ info.scorings = scorings.entrySet().stream()
+ .map(e -> Pair.of(e.getKey().name(), e.getValue()))
+ .collect(toImmutableMap(Pair::key, Pair::value));
+ }
return info;
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerInfoSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerInfoSubject.java
index 33a19aa..de3d37a 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerInfoSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerInfoSubject.java
@@ -21,6 +21,7 @@
import com.google.common.truth.Subject;
import com.google.gerrit.entities.Account;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.truth.NullAwareCorrespondence;
/** {@link Subject} for doing assertions on {@link CodeOwnerInfo}s. */
@@ -40,6 +41,17 @@
codeOwnerInfo -> codeOwnerInfo.account.name, "has account name");
}
+ /** Constructs a {@link Correspondence} that maps {@link CodeOwnerInfo}s to scoring. */
+ public static final Correspondence<CodeOwnerInfo, Integer> hasScoring(CodeOwnerScore score) {
+ return NullAwareCorrespondence.transforming(
+ codeOwnerInfo -> codeOwnerInfo.scorings.entrySet().stream()
+ .filter(factor -> factor.getKey().equals(score.name()))
+ .findFirst()
+ .orElseThrow()
+ .getValue(),
+ "has scoring");
+ }
+
public static Factory<CodeOwnerInfoSubject, CodeOwnerInfo> codeOwnerInfos() {
return CodeOwnerInfoSubject::new;
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
index 708cb5d..a9c96a8 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerConfigFileInfoSubject.assertThat;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasAccountId;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasAccountName;
+import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasScoring;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnersInfoSubject.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -54,6 +55,7 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigImportMode;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigReference;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerSet;
import com.google.gerrit.plugins.codeowners.backend.config.BackendConfig;
import com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerCapability;
@@ -389,8 +391,8 @@
@Test
public void
- cannotGetCodeOwnersWithSecondaryEmailsWithoutViewSecondaryEmailAndWithoutModifyAccountCapability()
- throws Exception {
+ cannotGetCodeOwnersWithSecondaryEmailsWithoutViewSecondaryEmailAndWithoutModifyAccountCapability()
+ throws Exception {
// create a code owner config
codeOwnerConfigOperations
.newCodeOwnerConfig()
@@ -1419,9 +1421,9 @@
.collect(toList());
for (int i = 0; i < 10; i++) {
assertThat(
- queryCodeOwners(
- getCodeOwnersApi().query().setResolveAllUsers(true).withSeed(seed),
- "/foo/bar/baz.md"))
+ queryCodeOwners(
+ getCodeOwnersApi().query().setResolveAllUsers(true).withSeed(seed),
+ "/foo/bar/baz.md"))
.hasCodeOwnersThat()
.comparingElementsUsing(hasAccountId())
.containsExactlyElementsIn(expectedAccountIds)
@@ -1985,4 +1987,82 @@
.assertNoImportMode()
.assertNoUnresolvedErrorMessage();
}
+
+
+ @Test
+ public void testCodeOwnersScoringsDistance() 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(user.email())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ CodeOwnersInfo codeOwnersInfo =
+ queryCodeOwners("/foo/bar/baz.md");
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user2.id(), user.id(), admin.id())
+ .inOrder();
+
+ Integer adminOwnershipDistance = 3;
+ Integer userOwnershipDistance = 2;
+ Integer user2OwnershipDistance = 1;
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .comparingElementsUsing(hasScoring(CodeOwnerScore.DISTANCE))
+ .containsExactly(user2OwnershipDistance, userOwnershipDistance, adminOwnershipDistance)
+ .inOrder();
+ }
+
+
+ @Test
+ @GerritConfig(name = "accounts.visibility", value = "ALL")
+ public void testCodeOwnersScoringsIsExplicitlyMentioned() throws Exception {
+ // Add a code owner config that makes all users code owners.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail("*")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ CodeOwnersInfo codeOwnersInfo =
+ queryCodeOwners(getCodeOwnersApi().query().setResolveAllUsers(true), "/foo/bar/baz.md");
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(admin.id(), user.id())
+ .inOrder();
+
+ Integer explicitlyMentioned = 1;
+ Integer notExplicitlyMentioned = 0;
+ assertThat(codeOwnersInfo)
+ .hasCodeOwnersThat()
+ .comparingElementsUsing(hasScoring(CodeOwnerScore.IS_EXPLICITLY_MENTIONED))
+ .containsExactly(explicitlyMentioned, notExplicitlyMentioned)
+ .inOrder();
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJsonTest.java
index 2aa5b6f..498ff09 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerJsonTest.java
@@ -17,13 +17,16 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasAccountId;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasAccountName;
+import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerInfoSubject.hasScoring;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwner;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import java.util.EnumSet;
import org.junit.Before;
@@ -48,7 +51,8 @@
@Test
public void formatEmptyListOfCodeOwners() throws Exception {
- assertThat(codeOwnerJsonFactory.create(EnumSet.of(FillOptions.ID)).format(ImmutableList.of()))
+ assertThat(codeOwnerJsonFactory.create(EnumSet.of(FillOptions.ID))
+ .format(ImmutableList.of(), ImmutableMap.of()))
.isEmpty();
}
@@ -57,7 +61,7 @@
ImmutableList<CodeOwnerInfo> codeOwnerInfos =
codeOwnerJsonFactory
.create(EnumSet.of(FillOptions.ID))
- .format(ImmutableList.of(CodeOwner.create(admin.id()), CodeOwner.create(user.id())));
+ .format(ImmutableList.of(CodeOwner.create(admin.id()), CodeOwner.create(user.id())), ImmutableMap.of());
assertThat(codeOwnerInfos)
.comparingElementsUsing(hasAccountId())
.containsExactly(admin.id(), user.id())
@@ -70,7 +74,7 @@
ImmutableList<CodeOwnerInfo> codeOwnerInfos =
codeOwnerJsonFactory
.create(EnumSet.of(FillOptions.ID, FillOptions.NAME))
- .format(ImmutableList.of(CodeOwner.create(admin.id()), CodeOwner.create(user.id())));
+ .format(ImmutableList.of(CodeOwner.create(admin.id()), CodeOwner.create(user.id())), ImmutableMap.of());
assertThat(codeOwnerInfos)
.comparingElementsUsing(hasAccountId())
.containsExactly(admin.id(), user.id())
@@ -81,6 +85,34 @@
.inOrder();
}
+
+ @Test
+ public void formatCodeOwnersWithAccountIdAndScorings() throws Exception {
+ CodeOwner adminCodeOwner = CodeOwner.create(admin.id());
+ CodeOwner userCodeOwner = CodeOwner.create(user.id());
+ Integer adminOwnershipDistance = 1;
+ Integer userOwnershipDistance = 2;
+ ImmutableMap<CodeOwner, ImmutableMap<CodeOwnerScore, Integer>> scorings =
+ ImmutableMap.<CodeOwner, ImmutableMap<CodeOwnerScore, Integer>>builder()
+ .put(adminCodeOwner, ImmutableMap.of(CodeOwnerScore.DISTANCE, adminOwnershipDistance))
+ .put(userCodeOwner, ImmutableMap.of(CodeOwnerScore.DISTANCE, userOwnershipDistance))
+ .build();
+
+ ImmutableList<CodeOwnerInfo> codeOwnerInfos =
+ codeOwnerJsonFactory
+ .create(EnumSet.of(FillOptions.ID))
+ .format(ImmutableList.of(adminCodeOwner, userCodeOwner),
+ scorings);
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(admin.id(), user.id())
+ .inOrder();
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasScoring(CodeOwnerScore.DISTANCE))
+ .containsExactly(adminOwnershipDistance, userOwnershipDistance)
+ .inOrder();
+ }
+
@Test
public void cannotFormatNullCodeOwners() throws Exception {
NullPointerException npe =
@@ -89,7 +121,7 @@
() ->
codeOwnerJsonFactory
.create(EnumSet.of(FillOptions.ID))
- .format(/* codeOwners= */ null));
+ .format(/* codeOwners= */ null, ImmutableMap.of()));
assertThat(npe).hasMessageThat().isEqualTo("codeOwners");
}
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 9481c29..cd19fac 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -501,26 +501,46 @@
{
"account": {
"_account_id": 1000096
+ },
+ "scorings": {
+ "DISTANCE": 0,
+ "IS_EXPLICITLY_MENTIONED": 1
}
},
{
"account": {
"_account_id": 1001439
+ },
+ "scorings": {
+ "DISTANCE": 0,
+ "IS_EXPLICITLY_MENTIONED": 1
}
},
{
"account": {
"_account_id": 1007265
+ },
+ "scorings": {
+ "DISTANCE": 1,
+ "IS_EXPLICITLY_MENTIONED": 1
}
},
{
"account": {
"_account_id": 1009877
+ },
+ "scorings": {
+ "DISTANCE": 2,
+ "IS_EXPLICITLY_MENTIONED": 1
}
},
{
"account": {
"_account_id": 1002930
+ },
+ "scorings": {
+ "DISTANCE": 2,
+ "IS_EXPLICITLY_MENTIONED": 0
}
}
],
@@ -568,8 +588,8 @@
Other factors like OOO state, recent review activity or code authorship are not
considered.
-**NOTE:** The scores for the code owners are not exposed via the REST API but
-only influence the sort order.
+The scorings (without weights applied) are exposed via the REST API and can be found in the
+[CodeOwnerInfo](#code-owner-info) entity.
#### <a id="sortingExample">Sorting Example
@@ -993,9 +1013,10 @@
### <a id="code-owner-info"> CodeOwnerInfo
The `CodeOwnerInfo` entity contains information about a code owner.
-| Field Name | | Description |
-| ----------- | -------- | ----------- |
-| `account` | optional | The account of the code owner as an [AccountInfo](../../../Documentation/rest-api-accounts.html#account-info) entity. At the moment the `account` field is always set, but it's marked as optional as in the future we may also return groups as code owner and then the `account` field would be unset.
+| Field Name | | Description |
+|------------| -------- | ----------- |
+| `account` | optional | The account of the code owner as an [AccountInfo](../../../Documentation/rest-api-accounts.html#account-info) entity. At the moment the `account` field is always set, but it's marked as optional as in the future we may also return groups as code owner and then the `account` field would be unset.
+| `scorings` | optional | Score name to scoring value map, that contains the scorings that were taken into account when computing the score of the listed code owner. Note that the returned values are not weighted.
---
