Allow to validate code owner config files in disabled branches
This is useful to validate the code owner config files in branches for
which the code owners functionality should be enabled, to ensure that
they do not contain issues before turning on the code owners
functionality.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iac36eb013a5139e217599b213d0ae2595ae6989f
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
new file mode 100644
index 0000000..f06edf4
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
@@ -0,0 +1,29 @@
+// 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.api;
+
+/**
+ * The input for the {@link com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles}
+ * REST endpoint.
+ */
+public class CheckCodeOwnerConfigFilesInput {
+ /**
+ * Whether code owner config files in branches for which the code owners functionality is disabled
+ * should be validated too.
+ *
+ * <p>By default unset, {@code false}.
+ */
+ public Boolean validateDisabledBranches;
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
index 4d3785b..b2823ea 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
@@ -33,6 +33,34 @@
/** Request to check code owner config files. */
abstract class CheckCodeOwnerConfigFilesRequest {
+ private boolean validateDisabledBranches;
+
+ /**
+ * Includes code owner config files in branches for which the code owners functionality is
+ * disabled into the validation.
+ */
+ public CheckCodeOwnerConfigFilesRequest validateDisabledBranches() {
+ return validateDisabledBranches(true);
+ }
+
+ /**
+ * Sets whether code owner config files in branches for which the code owners functionality is
+ * disabled should be validated.
+ */
+ public CheckCodeOwnerConfigFilesRequest validateDisabledBranches(
+ boolean validateDisabledBranches) {
+ this.validateDisabledBranches = validateDisabledBranches;
+ return this;
+ }
+
+ /**
+ * Whether code owner config files in branches for which the code owners functionality is
+ * disabled should be validated.
+ */
+ public boolean isValidateDisabledBranches() {
+ return validateDisabledBranches;
+ }
+
/**
* 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 813f6a8..750d98d 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
@@ -17,7 +17,6 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
-import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles;
@@ -83,7 +82,9 @@
public Map<String, Map<String, List<ConsistencyProblemInfo>>> check()
throws RestApiException {
try {
- return checkCodeOwnerConfigFiles.apply(projectResource, new Input()).value();
+ CheckCodeOwnerConfigFilesInput input = new CheckCodeOwnerConfigFilesInput();
+ input.validateDisabledBranches = isValidateDisabledBranches();
+ 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/restapi/CheckCodeOwnerConfigFiles.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
index 7dc9d62..cb55949 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
@@ -21,13 +21,14 @@
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Multimaps;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
-import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.plugins.codeowners.api.CheckCodeOwnerConfigFilesInput;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigScanner;
@@ -60,7 +61,10 @@
* not be used in any critical path where performance matters.
*/
@Singleton
-public class CheckCodeOwnerConfigFiles implements RestModifyView<ProjectResource, Input> {
+public class CheckCodeOwnerConfigFiles
+ implements RestModifyView<ProjectResource, CheckCodeOwnerConfigFilesInput> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final CurrentUser currentUser;
private final PermissionBackend permissionBackend;
private final Provider<ListBranches> listBranches;
@@ -86,7 +90,7 @@
@Override
public Response<Map<String, Map<String, List<ConsistencyProblemInfo>>>> apply(
- ProjectResource projectResource, Input input)
+ ProjectResource projectResource, CheckCodeOwnerConfigFilesInput input)
throws RestApiException, PermissionBackendException, IOException {
if (!currentUser.isIdentifiedUser()) {
throw new AuthException("Authentication required");
@@ -98,10 +102,17 @@
.project(projectResource.getNameKey())
.check(ProjectPermission.WRITE_CONFIG);
+ logger.atFine().log(
+ "checking code owner config files for project %s (validateDisabledBranches = %s)",
+ projectResource.getNameKey(), input.validateDisabledBranches);
+
ImmutableMap.Builder<String, Map<String, List<ConsistencyProblemInfo>>> resultsByBranchBuilder =
ImmutableMap.builder();
branches(projectResource)
- .filter(branchNameKey -> !codeOwnersPluginConfiguration.isDisabled(branchNameKey))
+ .filter(
+ branchNameKey ->
+ validateDisabledBranches(input)
+ || !codeOwnersPluginConfiguration.isDisabled(branchNameKey))
.forEach(
branchNameKey ->
resultsByBranchBuilder.put(branchNameKey.branch(), checkBranch(branchNameKey)));
@@ -167,4 +178,8 @@
"unknown message type %s for message %s",
commitValidationMessage.getType(), commitValidationMessage.getMessage()));
}
+
+ private static boolean validateDisabledBranches(CheckCodeOwnerConfigFilesInput input) {
+ return input.validateDisabledBranches != null && input.validateDisabledBranches;
+ }
}
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 47d78fa..f742ffe 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
@@ -85,6 +85,20 @@
}
@Test
+ @GerritConfig(name = "plugin.code-owners.disabledBranch", value = "refs/meta/config")
+ public void validateDisabledBranches() throws Exception {
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .validateDisabledBranches()
+ .check())
+ .containsExactly(
+ "refs/heads/master", ImmutableMap.of(),
+ "refs/meta/config", ImmutableMap.of());
+ }
+
+ @Test
public void noIssuesInCodeOwnerConfigFile() throws Exception {
// Create some code owner config files.
codeOwnerConfigOperations
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 242c362..5c23d5b 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -63,8 +63,13 @@
Requires that the caller is an owner of the project.
+Input options can be set in the request body as a
+[CheckCodeOwnerConfigFilesInput](#check-code-owner-config-files-input) entity.
+
No validation is done for branches for which the code owner functionality is
-[disabled](config.html#codeOwnersDisabledBranch).
+[disabled](config.html#codeOwnersDisabledBranch), unless
+`validate_disabled_branches` is set to `true` in the
+[input](#check-code-owner-config-files-input).
As a response a map is returned that maps a branch name to a map that maps an
owner configuration file path to a list of
@@ -370,6 +375,16 @@
---
+### <a id="check-code-owner-config-files-input"> CheckCodeOwnerConfigFilesInput
+The `CheckCodeOwnerConfigFilesInput` allows to set options for the [Check Code
+Owner Config Files REST endpoint](#check-code-owner-config-files).
+
+| Field Name | | Description |
+| ---------------------------- | -------- | ----------- |
+| `validate_disabled_branches` | optional | Whether code owner config files in branches for which the code owners functionality is disabled should be validated too. By default unset, `false`.
+
+---
+
### <a id="code-owner-config-info"> CodeOwnerConfigInfo
The `CodeOwnerConfigInfo` entity contains information about a code owner config
for a path.
