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. ---