Merge changes I5ef96785,If5df1e55,If30928e1,I667dc8d8,I6896af18, ...
* changes:
Document how to avoid and investigate issues with code owner config files
Fix example in path expressions table
Fix rendering issues in the plugin documentation
Remove unneeded imports
Add helper method to skip tests
Register the BatchModule
Treat uncaught InvalidPathExceptions as '409 Conflict'
diff --git a/BUILD b/BUILD
index 506abaa..7c7f9e0 100644
--- a/BUILD
+++ b/BUILD
@@ -20,6 +20,7 @@
"Gerrit-PluginName: code-owners",
"Gerrit-Module: com.google.gerrit.plugins.codeowners.module.Module",
"Gerrit-HttpModule: com.google.gerrit.plugins.codeowners.module.HttpModule",
+ "Gerrit-BatchModule: com.google.gerrit.plugins.codeowners.module.BatchModule",
],
resource_jars = [":code-owners-fe-static"],
resource_strip_prefix = "plugins/code-owners/resources",
diff --git a/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersIT.java b/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersIT.java
index 0b50f6e..26578fc 100644
--- a/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersIT.java
+++ b/java/com/google/gerrit/plugins/codeowners/acceptance/AbstractCodeOwnersIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.plugins.codeowners.acceptance;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.google.common.truth.TruthJUnit.assume;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.plugins.codeowners.acceptance.testsuite.CodeOwnerConfigOperations;
@@ -24,6 +25,7 @@
import com.google.gerrit.plugins.codeowners.api.impl.ProjectCodeOwnersFactory;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
import com.google.gerrit.plugins.codeowners.backend.config.BackendConfig;
+import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.testing.ConfigSuite;
import java.util.Arrays;
import org.eclipse.jgit.lib.Config;
@@ -76,6 +78,8 @@
protected ChangeCodeOwnersFactory changeCodeOwnersApiFactory;
protected ProjectCodeOwnersFactory projectCodeOwnersApiFactory;
+ private BackendConfig backendConfig;
+
@Before
public void baseSetup() throws Exception {
codeOwnerConfigOperations =
@@ -85,5 +89,15 @@
changeCodeOwnersApiFactory = plugin.getSysInjector().getInstance(ChangeCodeOwnersFactory.class);
projectCodeOwnersApiFactory =
plugin.getSysInjector().getInstance(ProjectCodeOwnersFactory.class);
+ backendConfig = plugin.getSysInjector().getInstance(BackendConfig.class);
+ }
+
+ protected void skipTestIfImportsNotSupportedByCodeOwnersBackend() {
+ // the proto backend doesn't support imports
+ assumeThatCodeOwnersBackendIsNotProtoBackend();
+ }
+
+ protected void assumeThatCodeOwnersBackendIsNotProtoBackend() {
+ assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/BranchCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/BranchCodeOwnersImpl.java
index 81a418c..231d591 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/BranchCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/BranchCodeOwnersImpl.java
@@ -18,8 +18,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.api.BranchCodeOwners;
-import com.google.gerrit.plugins.codeowners.api.BranchCodeOwners.CodeOwnerCheckRequest;
-import com.google.gerrit.plugins.codeowners.api.BranchCodeOwners.CodeOwnerConfigFilesRequest;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerBranchConfigInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerCheckInfo;
import com.google.gerrit.plugins.codeowners.api.RenameEmailInput;
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
index f9e3fdf..fab0648 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInBranchImpl.java
@@ -20,7 +20,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwners;
-import com.google.gerrit.plugins.codeowners.api.CodeOwners.QueryRequest;
import com.google.gerrit.plugins.codeowners.restapi.CodeOwnersInBranchCollection;
import com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnersForPathInBranch;
import com.google.gerrit.server.project.BranchResource;
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
index d1281fd..f91c08f 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/CodeOwnersInChangeImpl.java
@@ -21,7 +21,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwners;
-import com.google.gerrit.plugins.codeowners.api.CodeOwners.QueryRequest;
import com.google.gerrit.plugins.codeowners.restapi.CodeOwnersInChangeCollection;
import com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnersForPathInChange;
import com.google.gerrit.server.change.RevisionResource;
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/ProjectCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/ProjectCodeOwnersImpl.java
index 7d578e5..2085640 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/ProjectCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/ProjectCodeOwnersImpl.java
@@ -23,7 +23,6 @@
import com.google.gerrit.plugins.codeowners.api.CheckCodeOwnerConfigFilesInput;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerProjectConfigInfo;
import com.google.gerrit.plugins.codeowners.api.ProjectCodeOwners;
-import com.google.gerrit.plugins.codeowners.api.ProjectCodeOwners.CheckCodeOwnerConfigFilesRequest;
import com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles;
import com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerProjectConfig;
import com.google.gerrit.server.project.BranchResource;
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/RevisionCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/RevisionCodeOwnersImpl.java
index a4ea2bd..11ab392 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/RevisionCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/RevisionCodeOwnersImpl.java
@@ -20,7 +20,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.api.CheckCodeOwnerConfigFilesInRevisionInput;
import com.google.gerrit.plugins.codeowners.api.RevisionCodeOwners;
-import com.google.gerrit.plugins.codeowners.api.RevisionCodeOwners.CheckCodeOwnerConfigFilesRequest;
import com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFilesInRevision;
import com.google.gerrit.server.change.RevisionResource;
import com.google.inject.Inject;
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
index 53d7cd2..8a2e85e 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHook.java
@@ -19,6 +19,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.plugins.codeowners.backend.config.InvalidPluginConfigurationException;
import com.google.gerrit.server.ExceptionHook;
+import java.nio.file.InvalidPathException;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -39,7 +40,8 @@
@Override
public boolean skipRetryWithTrace(String actionType, String actionName, Throwable throwable) {
return isInvalidPluginConfigurationException(throwable)
- || isInvalidCodeOwnerConfigException(throwable);
+ || isInvalidCodeOwnerConfigException(throwable)
+ || isInvalidPathException(throwable);
}
@Override
@@ -56,13 +58,19 @@
return ImmutableList.of(configInvalidException.get().getMessage());
}
+ Optional<InvalidPathException> invalidPathException = getInvalidPathException(throwable);
+ if (invalidPathException.isPresent()) {
+ return ImmutableList.of(invalidPathException.get().getMessage());
+ }
+
return ImmutableList.of();
}
@Override
public Optional<Status> getStatus(Throwable throwable) {
if (isInvalidPluginConfigurationException(throwable)
- || isInvalidCodeOwnerConfigException(throwable)) {
+ || isInvalidCodeOwnerConfigException(throwable)
+ || isInvalidPathException(throwable)) {
return Optional.of(Status.create(409, "Conflict"));
}
return Optional.empty();
@@ -74,9 +82,22 @@
private static Optional<InvalidPluginConfigurationException> getInvalidPluginConfigurationCause(
Throwable throwable) {
+ return getInvalidPluginConfigurationCause(InvalidPluginConfigurationException.class, throwable);
+ }
+
+ private static boolean isInvalidPathException(Throwable throwable) {
+ return getInvalidPathException(throwable).isPresent();
+ }
+
+ private static Optional<InvalidPathException> getInvalidPathException(Throwable throwable) {
+ return getInvalidPluginConfigurationCause(InvalidPathException.class, throwable);
+ }
+
+ private static <T extends Throwable> Optional<T> getInvalidPluginConfigurationCause(
+ Class<T> exceptionClass, Throwable throwable) {
return Throwables.getCausalChain(throwable).stream()
- .filter(t -> t instanceof InvalidPluginConfigurationException)
- .map(t -> (InvalidPluginConfigurationException) t)
+ .filter(exceptionClass::isInstance)
+ .map(exceptionClass::cast)
.findFirst();
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
index 758d0ef..a2e37ef 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.codeowners.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -196,8 +195,7 @@
private void testIssuesInCodeOwnerConfigFile(ConsistencyProblemInfo.Status expectedStatus)
throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// Create some code owner config files with issues.
CodeOwnerConfig.Key keyOfInvalidConfig1 =
@@ -393,8 +391,7 @@
@Test
public void validateExactFile() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// Create some code owner config files with issues.
CodeOwnerConfig.Key keyOfInvalidConfig1 =
@@ -466,8 +463,7 @@
@Test
public void validateFilesMatchingGlob() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// Create some code owner config files with issues.
codeOwnerConfigOperations
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
index 282594b..8166970 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.codeowners.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerCheckInfoSubject.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -612,8 +611,7 @@
@Test
public void debugLogsContainUnresolvedImports() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfigReference unresolvableCodeOwnerConfigReference =
CodeOwnerConfigReference.create(
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
index e65efbd..12b3360 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.codeowners.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
@@ -152,8 +151,7 @@
@Test
public void canUploadConfigWithoutIssues_withImport() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
@@ -198,8 +196,7 @@
@Test
public void canUploadConfigWithoutIssues_withImportFromOtherProject() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
@@ -243,8 +240,7 @@
@Test
public void canUploadConfigWithoutIssues_withImportFromOtherProjectAndBranch() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
@@ -292,8 +288,7 @@
public void
canUploadConfigWithImportOfConfigThatIsAddedInSameCommit_importModeGlobalCodeOwnersOnly()
throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig = createCodeOwnerConfigKey("/foo/");
@@ -332,8 +327,7 @@
@Test
public void canUploadConfigWithImportOfConfigThatIsAddedInSameCommit_importModeAll()
throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey = createCodeOwnerConfigKey("/");
CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig = createCodeOwnerConfigKey("/foo/");
@@ -1040,8 +1034,7 @@
private void testUploadConfigWithImportFromNonExistingProject(
CodeOwnerConfigImportType importType) throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// create a code owner config that imports a code owner config from a non-existing project
CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
@@ -1087,8 +1080,7 @@
private void testUploadConfigWithImportFromNonVisibleProject(CodeOwnerConfigImportType importType)
throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// create a non-visible project with a code owner config file that we try to import
Project.NameKey nonVisibleProject =
@@ -1150,8 +1142,7 @@
private void testUploadConfigWithImportFromHiddenProject(CodeOwnerConfigImportType importType)
throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// create a hidden project with a code owner config file
Project.NameKey hiddenProject =
@@ -1210,8 +1201,7 @@
private void testUploadConfigWithImportFromNonExistingBranch(CodeOwnerConfigImportType importType)
throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// create a code owner config that imports a code owner config from a non-existing branch
CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
@@ -1258,8 +1248,7 @@
private void testUploadConfigWithImportFromNonVisibleBranch(CodeOwnerConfigImportType importType)
throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
@@ -1323,8 +1312,7 @@
private void testUploadConfigWithImportOfNonCodeOwnerConfigFile(
CodeOwnerConfigImportType importType) throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// create a code owner config that imports a non code owner config file
CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
@@ -1368,8 +1356,7 @@
private void testUploadConfigWithImportOfNonExistingCodeOwnerConfig(
CodeOwnerConfigImportType importType) throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
// create a code owner config that imports a non-existing code owner config
CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
@@ -1421,8 +1408,7 @@
private void testUploadConfigWithImportOfNonParseableCodeOwnerConfig(
CodeOwnerConfigImportType importType) throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig =
CodeOwnerConfig.Key.create(project, "master", "/foo/");
@@ -1629,8 +1615,7 @@
@Test
@GerritConfig(name = "plugin.code-owners.rejectNonResolvableImports", value = "false")
public void canUploadAndSubmitConfigWithUnresolvableImports() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
@@ -1685,8 +1670,7 @@
@GerritConfig(name = "plugin.code-owners.enableValidationOnCommitReceived", value = "false")
@GerritConfig(name = "plugin.code-owners.enableValidationOnSubmit", value = "false")
public void rejectConfigOptionsAreIgnoredIfValidationIsDisabled() throws Exception {
- // imports are not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/RenameEmailIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/RenameEmailIT.java
index 40d87a6..463d623 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/RenameEmailIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/RenameEmailIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.codeowners.acceptance.api;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerConfigSubject.assertThat;
@@ -41,8 +40,6 @@
import com.google.gerrit.plugins.codeowners.api.RenameEmailResultInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigFileUpdateScanner;
-import com.google.gerrit.plugins.codeowners.backend.config.BackendConfig;
-import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.restapi.RenameEmail;
import com.google.inject.Inject;
import java.util.Optional;
@@ -62,12 +59,10 @@
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
- private BackendConfig backendConfig;
private CodeOwnerConfigFileUpdateScanner codeOwnerConfigFileUpdateScanner;
@Before
public void setup() throws Exception {
- backendConfig = plugin.getSysInjector().getInstance(BackendConfig.class);
codeOwnerConfigFileUpdateScanner =
plugin.getSysInjector().getInstance(CodeOwnerConfigFileUpdateScanner.class);
}
@@ -179,8 +174,7 @@
@Test
public void renameEmail_noUpdateIfEmailIsNotContainedInCodeOwnerConfigs() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
codeOwnerConfigOperations
.newCodeOwnerConfig()
@@ -210,8 +204,7 @@
@Test
public void renameOwnEmailWithDirectPushPermission() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey1 =
codeOwnerConfigOperations
@@ -263,8 +256,7 @@
@Test
public void renameOtherEmailWithDirectPushPermission() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey1 =
codeOwnerConfigOperations
@@ -323,8 +315,7 @@
@Test
public void renameOwnEmailAsProjectOwner() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey1 =
codeOwnerConfigOperations
@@ -368,8 +359,7 @@
@Test
public void renameOtherEmailAsProjectOwner() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey1 =
codeOwnerConfigOperations
@@ -413,8 +403,7 @@
@Test
public void renameEmail_callingUserBecomesCommitAuthor() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
codeOwnerConfigOperations
.newCodeOwnerConfig()
@@ -438,8 +427,7 @@
@Test
public void renameEmailWithDefaultCommitMessage() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
codeOwnerConfigOperations
.newCodeOwnerConfig()
@@ -463,8 +451,7 @@
@Test
public void renameEmailWithSpecifiedCommitMessage() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
codeOwnerConfigOperations
.newCodeOwnerConfig()
@@ -489,8 +476,7 @@
@Test
public void renameEmail_specifiedCommitMessageIsTrimmed() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
codeOwnerConfigOperations
.newCodeOwnerConfig()
@@ -516,8 +502,7 @@
@Test
public void renameEmail_lineCommentsArePreserved() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey =
codeOwnerConfigOperations
@@ -580,8 +565,7 @@
@Test
public void renameEmail_inlineCommentsArePreserved() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey =
codeOwnerConfigOperations
@@ -642,8 +626,7 @@
@Test
public void renameEmail_emailInCommentIsReplaced() throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
CodeOwnerConfig.Key codeOwnerConfigKey =
codeOwnerConfigOperations
@@ -687,8 +670,7 @@
@Test
public void renameEmail_emailThatContainsEmailToBeReplacesAsSubstringStaysIntact()
throws Exception {
- // renaming email is not supported for the proto backend
- assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+ skipTestIfRenameEmailNotSupportedByCodeOwnersBackend();
TestAccount otherUser1 =
accountCreator.create(
@@ -745,4 +727,9 @@
.branch(branchName)
.renameEmailInCodeOwnerConfigFiles(input);
}
+
+ private void skipTestIfRenameEmailNotSupportedByCodeOwnersBackend() {
+ // the proto backend doesn't support renaming emails
+ assumeThatCodeOwnersBackendIsNotProtoBackend();
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
index e05f8cd..c727080 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnersExceptionHookTest.java
@@ -21,6 +21,7 @@
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.backend.config.InvalidPluginConfigurationException;
import com.google.gerrit.server.ExceptionHook.Status;
+import java.nio.file.InvalidPathException;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Before;
@@ -44,6 +45,9 @@
assertThat(skipRetryWithTrace(newConfigInvalidException())).isTrue();
assertThat(skipRetryWithTrace(newExceptionWithCause(newConfigInvalidException()))).isTrue();
+ assertThat(skipRetryWithTrace(newInvalidPathException())).isTrue();
+ assertThat(skipRetryWithTrace(newExceptionWithCause(newInvalidPathException()))).isTrue();
+
assertThat(skipRetryWithTrace(new Exception())).isFalse();
assertThat(skipRetryWithTrace(newExceptionWithCause(new Exception()))).isFalse();
}
@@ -63,6 +67,12 @@
assertThat(getUserMessages(newExceptionWithCause(configInvalidException)))
.containsExactly(configInvalidException.getMessage());
+ InvalidPathException invalidPathException = newInvalidPathException();
+ assertThat(getUserMessages(invalidPathException))
+ .containsExactly(invalidPathException.getMessage());
+ assertThat(getUserMessages(newExceptionWithCause(invalidPathException)))
+ .containsExactly(invalidPathException.getMessage());
+
assertThat(getUserMessages(new Exception())).isEmpty();
assertThat(getUserMessages(newExceptionWithCause(new Exception()))).isEmpty();
}
@@ -82,6 +92,11 @@
.value()
.isEqualTo(conflictStatus);
+ assertThat(getStatus(newInvalidPathException())).value().isEqualTo(conflictStatus);
+ assertThat(getStatus(newExceptionWithCause(newInvalidPathException())))
+ .value()
+ .isEqualTo(conflictStatus);
+
assertThat(getStatus(new Exception())).isEmpty();
assertThat(getStatus(newExceptionWithCause(new Exception()))).isEmpty();
}
@@ -109,4 +124,8 @@
private ConfigInvalidException newConfigInvalidException() {
return new ConfigInvalidException("message");
}
+
+ private InvalidPathException newInvalidPathException() {
+ return new InvalidPathException("input", "reason");
+ }
}
diff --git a/resources/Documentation/backend-find-owners.md b/resources/Documentation/backend-find-owners.md
index aa7a8b7..a1af71d 100644
--- a/resources/Documentation/backend-find-owners.md
+++ b/resources/Documentation/backend-find-owners.md
@@ -14,7 +14,7 @@
contains an `OWNERS` file that disables the inheritance of code owners from the
parent directories via the [set noparent](#setNoparent) keyword).
-<a id="defaultCodeOwnerConfiguration">
+### <a id="defaultCodeOwnerConfiguration">
Default code owners that apply to all branches can be defined in an `OWNERS`
file in the root directory of the `refs/meta/config` branch. This `OWNERS` file
is the parent of the root `OWNERS` files in all branches. This means if a root
@@ -245,7 +245,7 @@
john.doe@example.com
per-file docs.config,*.md=richard.roe@example.com
```
-\
+
##### <a id="doNotUsePathExpressionsForSubdirectories">
**NOTE:** It is discouraged to use path expressions that explicitly name
subdirectories such as `my-subdir/*` as they will break when the subdirectory
diff --git a/resources/Documentation/build.md b/resources/Documentation/build.md
index a1469f4..6c2db04 100644
--- a/resources/Documentation/build.md
+++ b/resources/Documentation/build.md
@@ -10,13 +10,13 @@
```
bazel build plugins/@PLUGIN@
```
-
+\
The output is created in
```
bazel-bin/plugins/@PLUGIN@/@PLUGIN@.jar
```
-
+\
To execute the tests run:
```
diff --git a/resources/Documentation/path-expressions.md b/resources/Documentation/path-expressions.md
index bedbbfe..322d757 100644
--- a/resources/Documentation/path-expressions.md
+++ b/resources/Documentation/path-expressions.md
@@ -37,9 +37,9 @@
| To Match | Glob | Simple Path Expression |
| -------- | ---- | ---------------------- |
| Concrete file in current folder | `BUILD` | `BUILD` |
-| File type in current folder | `*.md` | `*.md` |
+| File type in current folder | not possible | `*.md` |
| Concrete file in the current folder and in all subfolders | `{**/,}BUILD` | needs 2 expressions: `BUILD` + `.../BUILD` |
-| File type in the current folder and in all subfolder | `**.md` | `....md` |
+| File type in the current folder and in all subfolders | `**.md` or `*.md` | `....md` |
| All files in a subfolder | `my-folder/**` | `my-folder/...` |
| All “foo-<1-digit-number>.txt” files in all subfolders | `{**/,}foo-[0-9].txt` | not possible |
| All “foo-<n-digit-number>.txt” files in all subfolders | not possible | not possible |
diff --git a/resources/Documentation/setup-guide.md b/resources/Documentation/setup-guide.md
index 7449164..9a0e85f 100644
--- a/resources/Documentation/setup-guide.md
+++ b/resources/Documentation/setup-guide.md
@@ -26,6 +26,8 @@
* [How to update the code-owners.config file for a project](#updateCodeOwnersConfig)
* [How to check if the code owners functionality is enabled for a project or branch](#checkIfEnabled)
+* [How to avoid issues with code owner config files](#avoidIssuesWithCodeOwnerConfigs)
+* [How to investigate issues with code owner config files](#investigateIssuesWithCodeOwnerConfigs)
Recommendations about further configuration parameters can be found in the
[config guide](config-guide.html).
@@ -406,6 +408,53 @@
`https://<host>/projects/<project-name>/code_owners.project_config`\
(remember to URL-encode the project-name and branch-name)
+##### <a id="avoidIssuesWithCodeOwnerConfigs">How to avoid issues with code owner config files
+
+To avoid issues with code owner config files it's highly recommended to keep the
+[validation](validation.html) of code owner config files that is performed on
+receive commits and submit enabled, as it prevents that issues are newly
+introduced to code owner config files. Whether this validation is enabled and
+whether code owner config files with new issues are rejected is controlled by
+the following configuration parameters:
+
+* [plugin.@PLUGIN@.enableValidationOnCommitReceived](config.html#pluginCodeOwnersEnableValidationOnCommitReceived)
+* [plugin.@PLUGIN@.enableValidationOnSubmit](config.html#pluginCodeOwnersEnableValidationOnSubmit)
+* [plugin.@PLUGIN@.rejectNonResolvableCodeOwners](config.html#pluginCodeOwnersRejectNonResolvableCodeOwners)
+* [plugin.@PLUGIN@.rejectNonResolvableImports](config.html#pluginCodeOwnersRejectNonResolvableImports)
+
+Since code owner config files can also get
+[issues](validation.html#howCodeOwnerConfigsCanGetIssuesAfterSubmit) after they
+have been submitted, host administrators and project owners are also recommended
+to regularly check the existing code owner config files for issues by calling
+the [Check Code Owner Config File REST
+endpoint](rest-api.html#check-code-owner-config-files) (e.g. from a cronjob) and
+then fix the reported issues.
+
+##### <a id="investigateIssuesWithCodeOwnerConfigs">How to investigate issues with code owner config files
+
+If code owners config files are not working as expected, this is either caused
+by:
+
+* issues in the code owner config files
+* a bug in the @PLUGIN@ plugin
+
+Since code owner config files are part of the source code, any issues with them
+should be investigated and fixed by the project owners and host administrators.
+To do this they can:
+
+* Check the code owner config files for issues by calling the [Check Code Owner
+ Config File REST endpoint](rest-api.html#check-code-owner-config-files)
+* Check the code ownership of a user for a certain path by calling the [Check
+ Code Owner REST endpoint](rest-api.html#check-code-owner) (requires the caller
+ to be host administrator or have the [Check Code Owner
+ capability](#checkCodeOwner)).
+
+Bugs with the @PLUGIN@ plugin should be filed as issues for the Gerrit team, but
+only after issues with the code owner config files have been excluded.
+
+Also see [above](#avoidIssuesWithCodeOwnerConfigs) how to avoid issues with code
+owner config files in the first place.
+
---
Back to [@PLUGIN@ documentation index](index.html)
diff --git a/resources/Documentation/validation.md b/resources/Documentation/validation.md
index 47c3ef5..756edd5 100644
--- a/resources/Documentation/validation.md
+++ b/resources/Documentation/validation.md
@@ -53,6 +53,7 @@
blocking all uploads, to reduce the risk of breaking the plugin configuration
`code-owner.config` files are validated too)
+## <a id="howCodeOwnerConfigsCanGetIssuesAfterSubmit">
In addition it is possible that [code owner config
files](user-guide.hmtl#codeOwnerConfigFiles) get issues after they have been
submitted:
