Merge changes I946a69e4,I45103f0e
* changes:
Add metric to record latency for resolving code owner references
Document resolve_path_code_owners metric
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java
index 3af457d..d070d81 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java
@@ -49,6 +49,8 @@
@VisibleForTesting
static final String KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS = "enableExperimentalRestEndpoints";
+ @VisibleForTesting static final int DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE = 10000;
+
private static final String KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE = "maxCodeOwnerConfigCacheSize";
private final CodeOwnersPluginConfigSnapshot.Factory codeOwnersPluginConfigSnapshotFactory;
@@ -132,7 +134,8 @@
int maxCodeOwnerConfigCacheSize =
pluginConfigFactory
.getFromGerritConfig(pluginName)
- .getInt(KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE, /* defaultValue= */ 0);
+ .getInt(
+ KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE, DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
return maxCodeOwnerConfigCacheSize > 0
? Optional.of(maxCodeOwnerConfigCacheSize)
: Optional.empty();
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/AbstractGetCodeOwnersForPathRestIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/AbstractGetCodeOwnersForPathRestIT.java
index cadf021..7aa024d 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/AbstractGetCodeOwnersForPathRestIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/AbstractGetCodeOwnersForPathRestIT.java
@@ -163,6 +163,15 @@
}
@Test
+ public void cannotGetCodeOwnersWithInvalidHexAccountOption() throws Exception {
+ String invalidHexOption = "INVALID";
+ RestResponse r = adminRestSession.get(getUrl(TEST_PATH, "O=" + invalidHexOption));
+ r.assertBadRequest();
+ assertThat(r.getEntityContent())
+ .isEqualTo(String.format("\"%s\" is not a valid value for \"-O\"", invalidHexOption));
+ }
+
+ @Test
@GerritConfig(name = "plugin.code-owners.backend", value = "non-existing-backend")
public void cannotGetCodeOwnersIfPluginConfigurationIsInvalid() throws Exception {
RestResponse r = adminRestSession.get(getUrl(TEST_PATH));
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java
index ff53824..d9c3e14 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java
@@ -91,8 +91,10 @@
}
@Test
- public void codeOwnerConfigCacheSizeIsUnlimitedByDefault() throws Exception {
- assertThat(codeOwnersPluginConfiguration.getMaxCodeOwnerConfigCacheSize()).isEmpty();
+ public void codeOwnerConfigCacheSizeIsLimitedByDefault() throws Exception {
+ assertThat(codeOwnersPluginConfiguration.getMaxCodeOwnerConfigCacheSize())
+ .value()
+ .isEqualTo(CodeOwnersPluginConfiguration.DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
}
@Test
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md
index 21a22bd..53c39d2 100644
--- a/resources/Documentation/config.md
+++ b/resources/Documentation/config.md
@@ -543,7 +543,7 @@
files are cached in memory for the time of the request.\
This configuration parameter allows to set a limit for the number of
code owner config files that are cached per request.\
- By default `0` (unlimited).
+ By default `10000`.
# <a id="projectConfiguration">Project configuration in @PLUGIN@.config</a>
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index c937cb2..20a6e5f 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -526,6 +526,24 @@
5\. [score=3] User E\
* `owned_by_all_users` in the response is `true`
+#### <a id="rootOwnersFaq">Why are root code owners not suggested first?
+
+Root code owners can normally approve all files in a repository. Due to this
+change owners often want to add them as reviewers to their changes, since they
+find it desirable to add as few code owners as possible. This is problematic
+since it means that the root code owners would receive all reviews which likely
+overloads them.
+
+To avoid that the root code owners become the bottleneck, the @PLUGIN@ plugin
+prefers local code owners and suggests them first (also see distant score
+[above](#scoringFactors)). This means that root code owners are ranked lower and
+often don't appear amongst the top suggestions.
+
+Local code owners are also preferred because it is more likely that they are
+experts of the modified code.
+
+The same applies for [default code owners](config-guide.html#codeOwners).
+
#### Request
```
@@ -643,7 +661,7 @@
The following code owners are filtered out additionally:
-* service users (members of the `Service Users` group)
+* [service users](#serviceUsers) (members of the `Service Users` group)
* the change owner (since the change owner cannot be added as reviewer)
In addition, by default the change number is used as seed if none was specified.
@@ -1028,6 +1046,39 @@
assigned on the `All-Project` project in the `Global Capabilities` access
section.
+---
+
+## <a id="serviceUsers">Service Users
+
+Some of the @PLUGIN@ REST endpoints have special handling of code owners that
+are service users:
+
+* The [Suggest Code Owners for path in change](#list-code-owners-for-path-in-change)
+ REST endpoint filters out code owners that are service users.
+
+To detect service users the @PLUGIN@ plugin relies on the `Service Users` group.
+This group should contain all service users, such as bots, and is maintained by
+the host admins.
+
+If you are a host admin, please make sure all bots that run against your host
+are part of the `Service Users` group.
+
+If you are a bot owner, please make sure your bot is part of the `Service Users`
+group on all hosts it runs on.
+
+To add users to the "Service Users" group, first ensure that the group exists on
+your host. If it doesn't, create it. The name must exactly be `Service Users`.
+
+To create a group, use the Gerrit UI: `BROWSE` -> `Groups` -> `CREATE NEW`.
+
+Then, add the bots as members in this group. Alternatively, add an existing
+group that only contains bots as a subgroup of the `Service Users` group.
+
+To add members or subgroups, use the Gerrit UI: `BROWSE` -> `Groups` ->
+search for `Service Users` -> `Members`.
+
+---
+
Back to [@PLUGIN@ documentation index](index.html)
Part of [Gerrit Code Review](../../../Documentation/index.html)
