Merge changes If8f371e7,I968977a0,I84560336
* changes:
User guide: Document how to submit changes with files that have no code owners
Document that imports and global owners are always read from the current revisions
GetCodeOwnerBranchConfig: Include a flag if no code owner config file exists yet
diff --git a/java/com/google/gerrit/plugins/codeowners/BatchModule.java b/java/com/google/gerrit/plugins/codeowners/BatchModule.java
new file mode 100644
index 0000000..3267d31
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/BatchModule.java
@@ -0,0 +1,33 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners;
+
+import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.plugins.codeowners.backend.BackendModule;
+import com.google.inject.AbstractModule;
+
+/**
+ * Binds a subset of the code-owners plugin functionality that should be available in batch jobs.
+ */
+@UsedAt(UsedAt.Project.GOOGLE)
+public class BatchModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ // We only need the CodeOwnerSubmitRule in batch jobs, but since the CodeOwnerSubmitRule depends
+ // on the CodeOwnerBackend, CodeOwnerBackend must be bound too. This means we can simply install
+ // the whole BackendModule.
+ install(new BackendModule());
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInRevisionInput.java b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInRevisionInput.java
index 45c4156..daad0df 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInRevisionInput.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInRevisionInput.java
@@ -14,6 +14,8 @@
package com.google.gerrit.plugins.codeowners.api;
+import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
+
/**
* The input for the {@link
* com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFilesInRevision} REST endpoint.
@@ -26,4 +28,19 @@
* <p>By default unset, which means that all code owner config files should be validated.
*/
public String path;
+
+ /**
+ * Level that controls which code owner config file issues are returned.
+ *
+ * <p>The following values are supported:
+ *
+ * <ul>
+ * <li>{@code FATAL}: only fatal issues are returned
+ * <li>{@code ERROR}: only fatal and error issues are returned
+ * <li>{@code WARNING}: all issues (warning, error and fatal) are returned
+ * </ul>
+ *
+ * <p>If unset, {@code WARNING} is used.
+ */
+ public ConsistencyProblemInfo.Status verbosity;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
index fd865b8..37f6757 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
@@ -14,6 +14,7 @@
package com.google.gerrit.plugins.codeowners.api;
+import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
import java.util.List;
/**
@@ -46,4 +47,19 @@
* <p>By default unset, which means that all code owner config files should be validated.
*/
public String path;
+
+ /**
+ * Level that controls which code owner config file issues are returned.
+ *
+ * <p>The following values are supported:
+ *
+ * <ul>
+ * <li>{@code FATAL}: only fatal issues are returned
+ * <li>{@code ERROR}: only fatal and error issues are returned
+ * <li>{@code WARNING}: all issues (warning, error and fatal) are returned
+ * </ul>
+ *
+ * <p>If unset, {@code WARNING} is used.
+ */
+ public ConsistencyProblemInfo.Status verbosity;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
index 4d5159f..ccce1ed 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
@@ -39,6 +39,7 @@
private boolean validateDisabledBranches;
private ImmutableList<String> branches;
private String path;
+ private ConsistencyProblemInfo.Status verbosity;
/**
* Includes code owner config files in branches for which the code owners functionality is
@@ -101,6 +102,30 @@
}
/**
+ * Sets the verbosity level that controls which kind of issues should be returned.
+ *
+ * <p>The following values are supported:
+ *
+ * <ul>
+ * <li>{@code FATAL}: only fatal issues are returned
+ * <li>{@code ERROR}: only fatal and error issues are returned
+ * <li>{@code WARNING}: all issues (warning, error and fatal) are returned
+ * </ul>
+ *
+ * <p>If unset, {@code WARNING} is used.
+ */
+ public CheckCodeOwnerConfigFilesRequest setVerbosity(
+ @Nullable ConsistencyProblemInfo.Status verbosity) {
+ this.verbosity = verbosity;
+ return this;
+ }
+
+ /** Gets the verbosity level that controls which kind of issues should be returned. */
+ public ConsistencyProblemInfo.Status getVerbosity() {
+ return verbosity;
+ }
+
+ /**
* Executes the request to check the code owner config files and retrieves the result of the
* validation.
*/
diff --git a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
index 6011572..54b37ec 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
@@ -86,6 +86,7 @@
input.validateDisabledBranches = isValidateDisabledBranches();
input.branches = getBranches();
input.path = getPath();
+ input.verbosity = getVerbosity();
return checkCodeOwnerConfigFiles.apply(projectResource, input).value();
} catch (Exception e) {
throw asRestApiException("Cannot check code owner config files", e);
diff --git a/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwners.java
index 1471280..df9ed04 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwners.java
@@ -37,6 +37,7 @@
/** Request to check code owner config files. */
abstract class CheckCodeOwnerConfigFilesRequest {
private String path;
+ private ConsistencyProblemInfo.Status verbosity;
/**
* Sets a glob that limits the validation to code owner config files that have a path that
@@ -57,6 +58,30 @@
}
/**
+ * Sets the verbosity level that controls which kind of issues should be returned.
+ *
+ * <p>The following values are supported:
+ *
+ * <ul>
+ * <li>{@code FATAL}: only fatal issues are returned
+ * <li>{@code ERROR}: only fatal and error issues are returned
+ * <li>{@code WARNING}: all issues (warning, error and fatal) are returned
+ * </ul>
+ *
+ * <p>If unset, {@code WARNING} is used.
+ */
+ public CheckCodeOwnerConfigFilesRequest setVerbosity(
+ @Nullable ConsistencyProblemInfo.Status verbosity) {
+ this.verbosity = verbosity;
+ return this;
+ }
+
+ /** Gets the verbosity level that controls which kind of issues should be returned. */
+ public ConsistencyProblemInfo.Status getVerbosity() {
+ return verbosity;
+ }
+
+ /**
* Executes the request to check the code owner config files and retrieves the result of the
* validation.
*/
diff --git a/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwnersImpl.java
index e05b281..817639f 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/RevisionCodeOwnersImpl.java
@@ -51,6 +51,7 @@
CheckCodeOwnerConfigFilesInRevisionInput input =
new CheckCodeOwnerConfigFilesInRevisionInput();
input.path = getPath();
+ input.verbosity = getVerbosity();
return checkCodeOwnerConfigFilesInRevision.apply(revisionResource, input).value();
} catch (Exception e) {
throw asRestApiException("Cannot check code owner config files", e);
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index 5f55f62..7e402c7 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -191,8 +191,7 @@
codeOwnerResolver
.get()
.enforceVisibility(false)
- .resolve(
- codeOwnersPluginConfiguration.getGlobalCodeOwners(changeNotes.getProjectName()));
+ .resolveGlobalCodeOwners(changeNotes.getProjectName());
logger.atFine().log("global code owners = %s", globalCodeOwners);
// If the branch doesn't contain any code owner config file yet, we apply special logic
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index 3cb377b..6d9d05e 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.CurrentUser;
@@ -158,6 +159,16 @@
}
/**
+ * Resolves the global code owners for the given project.
+ *
+ * @param projectName the name of the project for which the global code owners should be resolved
+ * @return the resolved global code owners of the given project
+ */
+ public CodeOwnerResolverResult resolveGlobalCodeOwners(Project.NameKey projectName) {
+ return resolve(codeOwnersPluginConfiguration.getGlobalCodeOwners(projectName));
+ }
+
+ /**
* Resolves the given {@link CodeOwnerReference}s to {@link CodeOwner}s.
*
* @param codeOwnerReferences the code owner references that should be resolved
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
index 07788ca..6829041 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
@@ -24,6 +24,7 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Multimaps;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
@@ -126,7 +127,8 @@
.forEach(
branchNameKey ->
resultsByBranchBuilder.put(
- branchNameKey.branch(), checkBranch(input.path, branchNameKey)));
+ branchNameKey.branch(),
+ checkBranch(input.path, branchNameKey, input.verbosity)));
return Response.ok(resultsByBranchBuilder.build());
}
@@ -139,7 +141,9 @@
}
private Map<String, List<ConsistencyProblemInfo>> checkBranch(
- String pathGlob, BranchNameKey branchNameKey) {
+ String pathGlob,
+ BranchNameKey branchNameKey,
+ @Nullable ConsistencyProblemInfo.Status verbosity) {
ListMultimap<String, ConsistencyProblemInfo> problemsByPath = LinkedListMultimap.create();
CodeOwnerBackend codeOwnerBackend = codeOwnersPluginConfiguration.getBackend(branchNameKey);
codeOwnerConfigScanner.visit(
@@ -147,7 +151,7 @@
codeOwnerConfig -> {
problemsByPath.putAll(
codeOwnerBackend.getFilePath(codeOwnerConfig.key()).toString(),
- checkCodeOwnerConfig(codeOwnerBackend, codeOwnerConfig));
+ checkCodeOwnerConfig(codeOwnerBackend, codeOwnerConfig, verbosity));
return true;
},
(codeOwnerConfigFilePath, configInvalidException) -> {
@@ -162,28 +166,42 @@
}
private ImmutableList<ConsistencyProblemInfo> checkCodeOwnerConfig(
- CodeOwnerBackend codeOwnerBackend, CodeOwnerConfig codeOwnerConfig) {
+ CodeOwnerBackend codeOwnerBackend,
+ CodeOwnerConfig codeOwnerConfig,
+ @Nullable ConsistencyProblemInfo.Status verbosity) {
return codeOwnerConfigValidator
.validateCodeOwnerConfig(
currentUser.get().asIdentifiedUser(), codeOwnerBackend, codeOwnerConfig)
- .map(CheckCodeOwnerConfigFiles::createConsistencyProblemInfo)
+ .map(
+ commitValidationMessage ->
+ createConsistencyProblemInfo(commitValidationMessage, verbosity))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(toImmutableList());
}
public static Optional<ConsistencyProblemInfo> createConsistencyProblemInfo(
- CommitValidationMessage commitValidationMessage) {
+ CommitValidationMessage commitValidationMessage,
+ @Nullable ConsistencyProblemInfo.Status verbosity) {
switch (commitValidationMessage.getType()) {
case FATAL:
return Optional.of(
new ConsistencyProblemInfo(
ConsistencyProblemInfo.Status.FATAL, commitValidationMessage.getMessage()));
case ERROR:
+ if (ConsistencyProblemInfo.Status.FATAL.equals(verbosity)) {
+ // errors should not be reported
+ return Optional.empty();
+ }
return Optional.of(
new ConsistencyProblemInfo(
ConsistencyProblemInfo.Status.ERROR, commitValidationMessage.getMessage()));
case WARNING:
+ if (ConsistencyProblemInfo.Status.FATAL.equals(verbosity)
+ || ConsistencyProblemInfo.Status.ERROR.equals(verbosity)) {
+ // warnings should not be reported
+ return Optional.empty();
+ }
return Optional.of(
new ConsistencyProblemInfo(
ConsistencyProblemInfo.Status.WARNING, commitValidationMessage.getMessage()));
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFilesInRevision.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFilesInRevision.java
index 737e389..b3e2494 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFilesInRevision.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFilesInRevision.java
@@ -122,7 +122,10 @@
changedFile,
rw,
commit)
- .map(CheckCodeOwnerConfigFiles::createConsistencyProblemInfo)
+ .map(
+ commitValidationMessage ->
+ CheckCodeOwnerConfigFiles.createConsistencyProblemInfo(
+ commitValidationMessage, input.verbosity))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(toImmutableList()))));
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 2c7b15d..16432bd 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
@@ -25,6 +25,7 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
@@ -43,8 +44,8 @@
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.config.BackendConfig;
-import com.google.gerrit.plugins.codeowners.config.StatusConfig;
import com.google.inject.Inject;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.Before;
@@ -151,7 +152,7 @@
@Test
public void nonParseableCodeOwnerConfigFile() throws Exception {
String codeOwnerConfigPath = "/" + getCodeOwnerConfigFileName();
- createInvalidCodeOwnerConfig(codeOwnerConfigPath);
+ createNonParseableCodeOwnerConfig(codeOwnerConfigPath);
assertThat(checkCodeOwnerConfigFilesIn(project))
.containsExactly(
@@ -475,6 +476,87 @@
pathOfInvalidConfig3)))));
}
+ @Test
+ public void allIssuesAreReturnedIfNoLevelIsSpecified() throws Exception {
+ testIssuesAreFilteredByVerbosity(
+ /** verbosity */
+ null);
+ }
+
+ @Test
+ public void allIssuesAreReturnedIfLevelIsSetToWarning() throws Exception {
+ testIssuesAreFilteredByVerbosity(ConsistencyProblemInfo.Status.WARNING);
+ }
+
+ @Test
+ public void onlyFatalAndErrorIssuesAreReturnedIfLevelIsSetToError() throws Exception {
+ testIssuesAreFilteredByVerbosity(ConsistencyProblemInfo.Status.ERROR);
+ }
+
+ @Test
+ public void onlyFatalIssuesAreReturnedIfLevelIsSetToFatal() throws Exception {
+ testIssuesAreFilteredByVerbosity(ConsistencyProblemInfo.Status.FATAL);
+ }
+
+ private void testIssuesAreFilteredByVerbosity(@Nullable ConsistencyProblemInfo.Status verbosity)
+ throws Exception {
+ // create a non-parseable code owner config, that will be reported as fatal
+ String pathOfNonParseableCodeOwnerConfig = "/" + getCodeOwnerConfigFileName();
+ createNonParseableCodeOwnerConfig(pathOfNonParseableCodeOwnerConfig);
+
+ // create an invalid code owner config, that will be reported as error
+ CodeOwnerConfig.Key keyOfInvalidConfig =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail("unknown@example.com")
+ .create();
+ String pathOfInvalidConfig =
+ codeOwnerConfigOperations.codeOwnerConfig(keyOfInvalidConfig).getFilePath();
+
+ // there is currently nothing that triggers a warning
+
+ Map<String, List<ConsistencyProblemInfo>> expectedMasterIssues = new HashMap<>();
+ // the fatal issue is always expected
+ expectedMasterIssues.put(
+ pathOfNonParseableCodeOwnerConfig,
+ ImmutableList.of(
+ fatal(
+ String.format(
+ "invalid code owner config file '%s':\n %s",
+ pathOfNonParseableCodeOwnerConfig,
+ getParsingErrorMessage(
+ ImmutableMap.of(
+ FindOwnersBackend.class,
+ "invalid line: INVALID",
+ ProtoBackend.class,
+ "1:8: Expected \"{\"."))))));
+ if (verbosity == null
+ || ConsistencyProblemInfo.Status.ERROR.equals(verbosity)
+ || ConsistencyProblemInfo.Status.WARNING.equals(verbosity)) {
+ expectedMasterIssues.put(
+ pathOfInvalidConfig,
+ ImmutableList.of(
+ error(
+ String.format(
+ "code owner email 'unknown@example.com' in '%s' cannot be"
+ + " resolved for admin",
+ pathOfInvalidConfig))));
+ }
+
+ Map<String, Map<String, List<ConsistencyProblemInfo>>> result =
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setVerbosity(verbosity)
+ .check();
+ assertThat(result)
+ .containsExactly(
+ "refs/heads/master", expectedMasterIssues, "refs/meta/config", ImmutableMap.of());
+ }
+
private ConsistencyProblemInfo fatal(String message) {
return new ConsistencyProblemInfo(ConsistencyProblemInfo.Status.FATAL, message);
}
@@ -498,14 +580,14 @@
throw new IllegalStateException("unknown code owner backend: " + backend.getClass().getName());
}
- private void createInvalidCodeOwnerConfig(String path) throws Exception {
+ private void createNonParseableCodeOwnerConfig(String path) throws Exception {
disableCodeOwnersForProject(project);
String changeId =
createChange("Add invalid code owners file", JgitPath.of(path).get(), "INVALID")
.getChangeId();
approve(changeId);
gApi.changes().id(changeId).current().submit();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
}
private String getParsingErrorMessage(
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java
index 57b9506..98a071c 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesInRevisionIT.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.JgitPath;
@@ -32,7 +33,7 @@
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoCodeOwnerConfigParser;
import com.google.gerrit.plugins.codeowners.config.BackendConfig;
-import com.google.gerrit.plugins.codeowners.config.StatusConfig;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
@@ -120,7 +121,7 @@
String changeId =
createChange("Add code owners", JgitPath.of(codeOwnerConfigPath).get(), "INVALID")
.getChangeId();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
assertThat(checkCodeOwnerConfigFilesIn(changeId))
.containsExactly(
@@ -168,7 +169,7 @@
unknownEmail1, admin.email(), unknownEmail2))
.build()))
.getChangeId();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
Map<String, List<ConsistencyProblemInfo>> problemsByPath =
checkCodeOwnerConfigFilesIn(changeId);
@@ -209,7 +210,7 @@
CodeOwnerSet.createWithoutPathExpressions(admin.email(), user.email()))
.build()))
.getChangeId();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
// The validation request is done by 'admin' which can see 'admin' and 'user', however the
// validation is performed from the perspective of the uploader which is 'user2' and 'user2'
@@ -251,7 +252,7 @@
.getChangeId();
approve(changeId);
gApi.changes().id(changeId).current().submit();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
// Create a change that adds another code owner config file without issues.
CodeOwnerConfig.Key codeOwnerConfigKey2 = createCodeOwnerConfigKey("/foo/");
@@ -277,7 +278,7 @@
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath();
disableCodeOwnersForProject(project);
String changeId = createChangeWithFileDeletion(codeOwnerConfigPath);
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
assertThat(checkCodeOwnerConfigFilesIn(changeId)).isEmpty();
}
@@ -312,7 +313,7 @@
unknownEmail2, admin.email()))
.build())))
.getChangeId();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
Map<String, List<ConsistencyProblemInfo>> problemsByPath =
changeCodeOwnersApiFactory
@@ -373,7 +374,7 @@
unknownEmail3, admin.email()))
.build())))
.getChangeId();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
Map<String, List<ConsistencyProblemInfo>> problemsByPath =
changeCodeOwnersApiFactory
@@ -398,6 +399,97 @@
unknownEmail3, codeOwnerConfigPath3))));
}
+ @Test
+ public void allIssuesAreReturnedIfNoLevelIsSpecified() throws Exception {
+ testIssuesAreFilteredByVerbosity(
+ /** verbosity */
+ null);
+ }
+
+ @Test
+ public void allIssuesAreReturnedIfLevelIsSetToWarning() throws Exception {
+ testIssuesAreFilteredByVerbosity(ConsistencyProblemInfo.Status.WARNING);
+ }
+
+ @Test
+ public void onlyFatalAndErrorIssuesAreReturnedIfLevelIsSetToError() throws Exception {
+ testIssuesAreFilteredByVerbosity(ConsistencyProblemInfo.Status.ERROR);
+ }
+
+ @Test
+ public void onlyFatalIssuesAreReturnedIfLevelIsSetToFatal() throws Exception {
+ testIssuesAreFilteredByVerbosity(ConsistencyProblemInfo.Status.FATAL);
+ }
+
+ private void testIssuesAreFilteredByVerbosity(@Nullable ConsistencyProblemInfo.Status verbosity)
+ throws Exception {
+ CodeOwnerConfig.Key keyOfNonParseableCodeOwnerConfig = createCodeOwnerConfigKey("/");
+ String pathOfNonParseableCodeOwnerConfig =
+ codeOwnerConfigOperations.codeOwnerConfig(keyOfNonParseableCodeOwnerConfig).getFilePath();
+
+ CodeOwnerConfig.Key keyOfInvalidCodeOwnerConfig = createCodeOwnerConfigKey("/foo/");
+ String pathOfInvalidCodeOwnerConfig =
+ codeOwnerConfigOperations.codeOwnerConfig(keyOfInvalidCodeOwnerConfig).getFilePath();
+ String unknownEmail = "unknown@example.com";
+
+ // create a change with a) a non-parseable code owner config that will be reported as fatal and
+ // b) an invalid code owner config with an unknown email that will be reported as error
+ // (there is currently nothing that triggers a warning)
+ disableCodeOwnersForProject(project);
+ String changeId =
+ createChange(
+ "Add code owners",
+ ImmutableMap.of(
+ JgitPath.of(pathOfNonParseableCodeOwnerConfig).get(),
+ "INVALID",
+ JgitPath.of(pathOfInvalidCodeOwnerConfig).get(),
+ format(
+ CodeOwnerConfig.builder(keyOfInvalidCodeOwnerConfig, TEST_REVISION)
+ .addCodeOwnerSet(
+ CodeOwnerSet.createWithoutPathExpressions(unknownEmail))
+ .build())))
+ .getChangeId();
+ enableCodeOwnersForProject(project);
+
+ Map<String, List<ConsistencyProblemInfo>> expectedIssues = new HashMap<>();
+ // the fatal issue is always expected
+ expectedIssues.put(
+ pathOfNonParseableCodeOwnerConfig,
+ ImmutableList.of(
+ fatal(
+ String.format(
+ "invalid code owner config file '%s':\n %s",
+ pathOfNonParseableCodeOwnerConfig,
+ getParsingErrorMessage(
+ ImmutableMap.of(
+ FindOwnersBackend.class,
+ "invalid line: INVALID",
+ ProtoBackend.class,
+ "1:8: Expected \"{\"."))))));
+ if (verbosity == null
+ || ConsistencyProblemInfo.Status.ERROR.equals(verbosity)
+ || ConsistencyProblemInfo.Status.WARNING.equals(verbosity)) {
+ expectedIssues.put(
+ pathOfInvalidCodeOwnerConfig,
+ ImmutableList.of(
+ error(
+ String.format(
+ "code owner email '%s' in '%s' cannot be" + " resolved for admin",
+ unknownEmail, pathOfInvalidCodeOwnerConfig))));
+ } else {
+ expectedIssues.put(pathOfInvalidCodeOwnerConfig, ImmutableList.of());
+ }
+
+ Map<String, List<ConsistencyProblemInfo>> result =
+ changeCodeOwnersApiFactory
+ .change(changeId)
+ .current()
+ .checkCodeOwnerConfigFiles()
+ .setVerbosity(verbosity)
+ .check();
+ assertThat(result).isEqualTo(expectedIssues);
+ }
+
private ConsistencyProblemInfo fatal(String message) {
return new ConsistencyProblemInfo(ConsistencyProblemInfo.Status.FATAL, message);
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
index 0489c38..804fcbf 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
@@ -25,7 +25,6 @@
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.config.BackendConfig;
-import com.google.gerrit.plugins.codeowners.config.StatusConfig;
import org.junit.Before;
import org.junit.Test;
@@ -325,6 +324,6 @@
.getChangeId();
approve(changeId);
gApi.changes().id(changeId).current().submit();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
}
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/batch/BUILD b/javatests/com/google/gerrit/plugins/codeowners/acceptance/batch/BUILD
new file mode 100644
index 0000000..cef4da8
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/batch/BUILD
@@ -0,0 +1,17 @@
+load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
+
+package(
+ default_testonly = True,
+ default_visibility = ["//plugins/code-owners:visibility"],
+)
+
+acceptance_tests(
+ srcs = glob(
+ ["*IT.java"],
+ ),
+ group = "acceptance_batch",
+ deps = [
+ "//plugins/code-owners:code-owners__plugin",
+ "//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/testing",
+ ],
+)
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/batch/CodeOwnerSubmitRuleBatchIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/batch/CodeOwnerSubmitRuleBatchIT.java
new file mode 100644
index 0000000..9365151
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/batch/CodeOwnerSubmitRuleBatchIT.java
@@ -0,0 +1,74 @@
+package com.google.gerrit.plugins.codeowners.acceptance.batch;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.plugins.codeowners.testing.SubmitRequirementInfoSubject.assertThatCollection;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.plugins.codeowners.testing.SubmitRequirementInfoSubject;
+import com.google.inject.Inject;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.junit.Test;
+
+/**
+ * Test that verifies that the {@link com.google.gerrit.plugins.codeowners.BatchModule} has bound
+ * all classes that are needed to run {@code
+ * com.google.gerrit.plugins.codeowners.backend.CodeOwnerSubmitRule}.
+ */
+@TestPlugin(name = "code-owners", sysModule = "com.google.gerrit.plugins.codeowners.BatchModule")
+public class CodeOwnerSubmitRuleBatchIT extends LightweightPluginDaemonTest {
+ @Inject private ProjectOperations projectOperations;
+ @Inject private RequestScopeOperations requestScopeOperations;
+
+ @Test
+ public void invokeCodeOwnerSubmitRule() throws Exception {
+ // Upload a change as a non-code owner.
+ TestRepository<InMemoryRepository> testRepo = cloneProject(project, user);
+ PushOneCommit push =
+ pushFactory.create(user.newIdent(), testRepo, "Test Change", "foo/bar.baz", "file content");
+ String changeId = push.to("refs/for/master").getChangeId();
+
+ // Approve by a non-code-owner.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel("Code-Review").ref("refs/heads/*").group(REGISTERED_USERS).range(-2, +2))
+ .update();
+ requestScopeOperations.setApiUser(user.id());
+ approve(changeId);
+
+ // Verify that the change is not submittable.
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get(ListChangesOption.SUBMITTABLE);
+ assertThat(changeInfo.submittable).isFalse();
+
+ // Check the submit requirement.
+ SubmitRequirementInfoSubject submitRequirementInfoSubject =
+ assertThatCollection(changeInfo.requirements).onlyElement();
+ submitRequirementInfoSubject.hasStatusThat().isEqualTo("NOT_READY");
+ submitRequirementInfoSubject.hasFallbackTextThat().isEqualTo("Code Owners");
+ submitRequirementInfoSubject.hasTypeThat().isEqualTo("code-owners");
+
+ // Approve by a project owner who is code owner since there is no code owner config file yet,
+ // and hence we are in bootstrapping mode.
+ requestScopeOperations.setApiUser(admin.id());
+ approve(changeId);
+
+ // Verify that the change is submittable now.
+ changeInfo = gApi.changes().id(changeId).get(ListChangesOption.SUBMITTABLE);
+ assertThat(changeInfo.submittable).isTrue();
+
+ // Check the submit requirement.
+ submitRequirementInfoSubject = assertThatCollection(changeInfo.requirements).onlyElement();
+ submitRequirementInfoSubject.hasStatusThat().isEqualTo("OK");
+ submitRequirementInfoSubject.hasFallbackTextThat().isEqualTo("Code Owners");
+ submitRequirementInfoSubject.hasTypeThat().isEqualTo("code-owners");
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
index 8788ddb..bcbb0cf 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
@@ -24,7 +24,6 @@
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.acceptance.testsuite.CodeOwnerConfigOperations;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
-import com.google.gerrit.plugins.codeowners.config.StatusConfig;
import org.junit.Before;
import org.junit.Test;
@@ -74,7 +73,7 @@
createChange("Add code owners", JgitPath.of(filePath).get(), "INVALID").getChangeId();
approve(changeId);
gApi.changes().id(changeId).current().submit();
- setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED, "false");
+ enableCodeOwnersForProject(project);
String changeId2 = createChange().getChangeId();
RestResponse r =
diff --git a/ui/owner-requirement.js b/ui/owner-requirement.js
index ce8b37a..04ac8f5 100644
--- a/ui/owner-requirement.js
+++ b/ui/owner-requirement.js
@@ -167,12 +167,12 @@
.reduce((prev, cur) => {
const oldPathStatus = cur.old_path_status;
const newPathStatus = cur.new_path_status;
- if (this._isMissing(newPathStatus.status)) {
+ if (newPathStatus && this._isMissing(newPathStatus.status)) {
prev.missing ++;
- } else if (this._isPending(newPathStatus.status)) {
+ } else if (newPathStatus && this._isPending(newPathStatus.status)) {
prev.pending ++;
} else if (oldPathStatus) {
- // check oldPath if newPath approved
+ // check oldPath if newPath approved or the file is deleted
if (this._isMissing(oldPathStatus.status)) {
prev.missing ++;
} else if (this._isPending(oldPathStatus.status)) {
