CheckCodeOwnerConfigFiles: Allow to specify a path glob
This way the caller can control which code owner config files should be
validated.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I424a3c22801c3a2aa46a2d3bc582f953fd187ba1
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
index 6e2abfc..fd865b8 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
@@ -38,4 +38,12 @@
* validated.
*/
public List<String> branches;
+
+ /**
+ * Glob that limits the validation to code owner config files that have a path that matches this
+ * glob.
+ *
+ * <p>By default unset, which means that all code owner config files should be validated.
+ */
+ public String path;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
index 6f86d06..d4a839a 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
@@ -37,6 +37,7 @@
abstract class CheckCodeOwnerConfigFilesRequest {
private boolean validateDisabledBranches;
private ImmutableList<String> branches;
+ private String path;
/**
* Includes code owner config files in branches for which the code owners functionality is
@@ -81,6 +82,24 @@
}
/**
+ * Sets a glob that limits the validation to code owner config files that have a path that
+ * matches this glob.
+ */
+ public CheckCodeOwnerConfigFilesRequest setPath(String path) {
+ this.path = path;
+ return this;
+ }
+
+ /**
+ * Gets the glob that limits the validation to code owner config files that have a path that
+ * matches this glob.
+ */
+ @Nullable
+ public String getPath() {
+ return path;
+ }
+
+ /**
* 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 0a4fdad..6011572 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
@@ -85,6 +85,7 @@
CheckCodeOwnerConfigFilesInput input = new CheckCodeOwnerConfigFilesInput();
input.validateDisabledBranches = isValidateDisabledBranches();
input.branches = getBranches();
+ input.path = getPath();
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/backend/CodeOwnerConfigScanner.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
index 6585278..99ee653 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
@@ -19,6 +19,7 @@
import static java.util.Objects.requireNonNull;
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.exceptions.StorageException;
@@ -28,6 +29,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Optional;
@@ -82,16 +84,39 @@
* @param branchNameKey the project and branch for which the code owner config files should be
* visited
* @param codeOwnerConfigVisitor the callback that is invoked for each code owner config file
+ * @param invalidCodeOwnerConfigCallback callback that is invoked for invalid code owner config
+ * files
*/
public void visit(
BranchNameKey branchNameKey,
CodeOwnerConfigVisitor codeOwnerConfigVisitor,
InvalidCodeOwnerConfigCallback invalidCodeOwnerConfigCallback) {
+ visit(branchNameKey, codeOwnerConfigVisitor, invalidCodeOwnerConfigCallback, null);
+ }
+
+ /**
+ * Visits all code owner config files in the given project and branch.
+ *
+ * @param branchNameKey the project and branch for which the code owner config files should be
+ * visited
+ * @param codeOwnerConfigVisitor the callback that is invoked for each code owner config file
+ * @param invalidCodeOwnerConfigCallback callback that is invoked for invalid code owner config
+ * files
+ * @param pathGlob optional Java NIO glob that the paths of code owner config files must match
+ */
+ public void visit(
+ BranchNameKey branchNameKey,
+ CodeOwnerConfigVisitor codeOwnerConfigVisitor,
+ InvalidCodeOwnerConfigCallback invalidCodeOwnerConfigCallback,
+ @Nullable String pathGlob) {
requireNonNull(branchNameKey, "branchNameKey");
requireNonNull(codeOwnerConfigVisitor, "codeOwnerConfigVisitor");
requireNonNull(invalidCodeOwnerConfigCallback, "invalidCodeOwnerConfigCallback");
CodeOwnerBackend codeOwnerBackend = codeOwnersPluginConfiguration.getBackend(branchNameKey);
+ logger.atFine().log(
+ "scanning code owner files in branch %s of project %s (path glob = %s)",
+ branchNameKey.branch(), branchNameKey.project(), pathGlob);
try (Repository repository = repoManager.openRepository(branchNameKey.project());
RevWalk rw = new RevWalk(repository);
@@ -106,7 +131,8 @@
RevCommit revision = rw.parseCommit(ref.getObjectId());
treeWalk.addTree(revision.getTree());
treeWalk.setRecursive(true);
- treeWalk.setFilter(createCodeOwnerConfigFilter(codeOwnerBackend, branchNameKey.project()));
+ treeWalk.setFilter(
+ createCodeOwnerConfigFilter(codeOwnerBackend, branchNameKey.project(), pathGlob));
while (treeWalk.next()) {
Path filePath = Paths.get(treeWalk.getPathString());
@@ -151,9 +177,16 @@
}
}
- /** Creates a {@link TreeFilter} that matches code owner config files in the given project. */
+ /**
+ * Creates a {@link TreeFilter} that matches code owner config files in the given project.
+ *
+ * @param codeOwnerBackend the code owner backend that is being used
+ * @param project the name of the project in which code owner config files should be matched
+ * @param pathGlob optional Java NIO glob that the paths of code owner config files must match
+ * @return the created {@link TreeFilter}
+ */
private static TreeFilter createCodeOwnerConfigFilter(
- CodeOwnerBackend codeOwnerBackend, Project.NameKey project) {
+ CodeOwnerBackend codeOwnerBackend, Project.NameKey project, @Nullable String pathGlob) {
return new TreeFilter() {
@Override
public boolean shouldBeRecursive() {
@@ -166,6 +199,14 @@
walker.enterSubtree();
return false;
}
+ if (pathGlob != null
+ && !FileSystems.getDefault()
+ .getPathMatcher("glob:" + pathGlob)
+ .matches(JgitPath.of(walker.getPathString()).getAsAbsolutePath())) {
+ logger.atFine().log(
+ "%s filtered out because it doesn't match the path glob", walker.getPathString());
+ return false;
+ }
String fileName = Paths.get(walker.getPathString()).getFileName().toString();
return codeOwnerBackend.isCodeOwnerConfigFile(project, fileName);
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
index a00f476..09dc34d 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
@@ -108,8 +108,8 @@
logger.atFine().log(
"checking code owner config files for project %s"
- + " (validateDisabledBranches = %s, branches = %s)",
- projectResource.getNameKey(), input.validateDisabledBranches, input.branches);
+ + " (validateDisabledBranches = %s, branches = %s, path = %s)",
+ projectResource.getNameKey(), input.validateDisabledBranches, input.branches, input.path);
ImmutableSet<BranchNameKey> branches = branches(projectResource);
@@ -125,7 +125,8 @@
|| !codeOwnersPluginConfiguration.isDisabled(branchNameKey))
.forEach(
branchNameKey ->
- resultsByBranchBuilder.put(branchNameKey.branch(), checkBranch(branchNameKey)));
+ resultsByBranchBuilder.put(
+ branchNameKey.branch(), checkBranch(input.path, branchNameKey)));
return Response.ok(resultsByBranchBuilder.build());
}
@@ -137,7 +138,8 @@
.collect(toImmutableSet());
}
- private Map<String, List<ConsistencyProblemInfo>> checkBranch(BranchNameKey branchNameKey) {
+ private Map<String, List<ConsistencyProblemInfo>> checkBranch(
+ String pathGlob, BranchNameKey branchNameKey) {
ListMultimap<String, ConsistencyProblemInfo> problemsByPath = LinkedListMultimap.create();
CodeOwnerBackend codeOwnerBackend = codeOwnersPluginConfiguration.getBackend(branchNameKey);
codeOwnerConfigScanner.visit(
@@ -153,7 +155,8 @@
codeOwnerConfigFilePath.toString(),
new ConsistencyProblemInfo(
ConsistencyProblemInfo.Status.ERROR, configInvalidException.getMessage()));
- });
+ },
+ pathGlob);
return Multimaps.asMap(problemsByPath);
}
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 a2f5572..a0f3384 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
@@ -317,6 +317,135 @@
.containsExactly("refs/meta/config", ImmutableMap.of());
}
+ @Test
+ public void validateExactFile() throws Exception {
+ // imports are not supported for the proto backend
+ assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+
+ // Create some code owner config files with issues.
+ CodeOwnerConfig.Key keyOfInvalidConfig1 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "/not-a-code-owner-config"))
+ .create();
+
+ CodeOwnerConfig.Key keyOfInvalidConfig2 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail("unknown1@example.com")
+ .addCodeOwnerEmail("unknown2@example.com")
+ .create();
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("master"))
+ .setPath(getCodeOwnerConfigFilePath(keyOfInvalidConfig1))
+ .check())
+ .containsExactly(
+ "refs/heads/master",
+ ImmutableMap.of(
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig1),
+ ImmutableList.of(
+ error(
+ String.format(
+ "invalid global import in '%s': '/not-a-code-owner-config' is"
+ + " not a code owner config file",
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig1))))));
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("master"))
+ .setPath(getCodeOwnerConfigFilePath(keyOfInvalidConfig2))
+ .check())
+ .containsExactly(
+ "refs/heads/master",
+ ImmutableMap.of(
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig2),
+ ImmutableList.of(
+ error(
+ String.format(
+ "code owner email 'unknown1@example.com' in '%s' cannot be"
+ + " resolved for admin",
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig2))),
+ error(
+ String.format(
+ "code owner email 'unknown2@example.com' in '%s' cannot be"
+ + " resolved for admin",
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig2))))));
+ }
+
+ @Test
+ public void validateFilesMatchingGlob() throws Exception {
+ // imports are not supported for the proto backend
+ assume().that(backendConfig.getDefaultBackend()).isNotInstanceOf(ProtoBackend.class);
+
+ // Create some code owner config files with issues.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "/not-a-code-owner-config"))
+ .create();
+
+ CodeOwnerConfig.Key keyOfInvalidConfig2 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail("unknown1@example.com")
+ .create();
+
+ CodeOwnerConfig.Key keyOfInvalidConfig3 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail("unknown2@example.com")
+ .create();
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("master"))
+ .setPath("/foo/**")
+ .check())
+ .containsExactly(
+ "refs/heads/master",
+ ImmutableMap.of(
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig2),
+ ImmutableList.of(
+ error(
+ String.format(
+ "code owner email 'unknown1@example.com' in '%s' cannot be"
+ + " resolved for admin",
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig2)))),
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig3),
+ ImmutableList.of(
+ error(
+ String.format(
+ "code owner email 'unknown2@example.com' in '%s' cannot be"
+ + " resolved for admin",
+ getCodeOwnerConfigFilePath(keyOfInvalidConfig3))))));
+ }
+
private ConsistencyProblemInfo error(String message) {
return new ConsistencyProblemInfo(ConsistencyProblemInfo.Status.ERROR, message);
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 189ff23..84e36a1 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -383,6 +383,7 @@
| ---------------------------- | -------- | ----------- |
| `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`.
| `branches` | optional | List of branches for which code owner config files should be validated. The `refs/heads/` prefix may be omitted. By default unset, which means that code owner config files in all branches should be validated.
+| `path` | optional | Glob that limits the validation to code owner config files that have a path that matches this glob. By default unset, which means that all code owner config files should be validated.
---
