Merge "Report logged in user role in logs"
diff --git a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
index 2b054b3..8ff1222 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
@@ -14,6 +14,8 @@
package com.google.gerrit.plugins.codeowners.api;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
import java.util.List;
@@ -27,7 +29,36 @@
CodeOwnerConfigFilesRequest codeOwnerConfigFiles() throws RestApiException;
abstract class CodeOwnerConfigFilesRequest {
+ private String email;
+
+ /**
+ * Limits the returned code owner config files to those that contain the given email.
+ *
+ * @param email the email that should appear in the returned code owner config files
+ */
+ public CodeOwnerConfigFilesRequest withEmail(String email) {
+ this.email = email;
+ return this;
+ }
+
+ /** Returns the email that should appear in the returned code owner config files/ */
+ @Nullable
+ public String getEmail() {
+ return email;
+ }
+
/** Executes the request and retrieves the paths of the requested code owner config file */
public abstract List<String> paths() throws RestApiException;
}
+
+ /**
+ * A default implementation which allows source compatibility when adding new methods to the
+ * interface.
+ */
+ class NotImplemented implements BranchCodeOwners {
+ @Override
+ public CodeOwnerConfigFilesRequest codeOwnerConfigFiles() throws RestApiException {
+ throw new NotImplementedException();
+ }
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
index 3383eb1..189b0d1 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
@@ -18,6 +18,7 @@
import com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerConfigFiles;
import com.google.gerrit.server.project.BranchResource;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import java.util.List;
@@ -27,13 +28,14 @@
BranchCodeOwnersImpl create(BranchResource branchResource);
}
- private final GetCodeOwnerConfigFiles getCodeOwnerConfigFiles;
+ private final Provider<GetCodeOwnerConfigFiles> getCodeOwnerConfigFilesProvider;
private final BranchResource branchResource;
@Inject
public BranchCodeOwnersImpl(
- GetCodeOwnerConfigFiles getCodeOwnerConfigFiles, @Assisted BranchResource branchResource) {
- this.getCodeOwnerConfigFiles = getCodeOwnerConfigFiles;
+ Provider<GetCodeOwnerConfigFiles> getCodeOwnerConfigFilesProvider,
+ @Assisted BranchResource branchResource) {
+ this.getCodeOwnerConfigFilesProvider = getCodeOwnerConfigFilesProvider;
this.branchResource = branchResource;
}
@@ -42,6 +44,8 @@
return new CodeOwnerConfigFilesRequest() {
@Override
public List<String> paths() throws RestApiException {
+ GetCodeOwnerConfigFiles getCodeOwnerConfigFiles = getCodeOwnerConfigFilesProvider.get();
+ getCodeOwnerConfigFiles.setEmail(getEmail());
return getCodeOwnerConfigFiles.apply(branchResource).value();
}
};
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..fd865b8
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/api/CheckCodeOwnerConfigFilesInput.java
@@ -0,0 +1,49 @@
+// 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;
+
+import java.util.List;
+
+/**
+ * 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;
+
+ /**
+ * Branches for which code owner config files should be validated.
+ *
+ * <p>The {@code refs/heads/} prefix may be omitted.
+ *
+ * <p>By default unset, which means that code owner config files in all branches should be
+ * 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 8138287..4d5159f 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwners.java
@@ -14,7 +14,13 @@
package com.google.gerrit.plugins.codeowners.api;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
+import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import java.util.List;
+import java.util.Map;
/**
* Project-level Java API of the code-owners plugin.
@@ -25,6 +31,104 @@
/** Returns the code owner project configuration. */
CodeOwnerProjectConfigInfo getConfig() throws RestApiException;
+ /** Create a request to check the code owner config files in the project. */
+ CheckCodeOwnerConfigFilesRequest checkCodeOwnerConfigFiles() throws RestApiException;
+
+ /** Request to check code owner config files. */
+ 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
+ * 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;
+ }
+
+ /** Sets the branches for which code owner config files should be validated. */
+ public CheckCodeOwnerConfigFilesRequest setBranches(List<String> branches) {
+ this.branches = ImmutableList.copyOf(branches);
+ return this;
+ }
+
+ /**
+ * Gets the branches for which code owner config files should be validated.
+ *
+ * <p>Returns {@code null} if no branches have been set.
+ */
+ @Nullable
+ public ImmutableList<String> getBranches() {
+ return branches;
+ }
+
+ /**
+ * 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.
+ */
+ public abstract Map<String, Map<String, List<ConsistencyProblemInfo>>> check()
+ throws RestApiException;
+ }
+
/** Returns the branch-level code owners API for the given branch. */
BranchCodeOwners branch(String branchName) throws RestApiException;
+
+ /**
+ * A default implementation which allows source compatibility when adding new methods to the
+ * interface.
+ */
+ class NotImplemented implements ProjectCodeOwners {
+ @Override
+ public CodeOwnerProjectConfigInfo getConfig() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public CheckCodeOwnerConfigFilesRequest checkCodeOwnerConfigFiles() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public BranchCodeOwners branch(String branchName) throws RestApiException {
+ throw new NotImplementedException();
+ }
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
index 0454dc0..6011572 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ProjectCodeOwnersImpl.java
@@ -16,14 +16,18 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
+import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles;
import com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerProjectConfig;
import com.google.gerrit.server.project.BranchResource;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.restapi.project.BranchesCollection;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
+import java.util.List;
+import java.util.Map;
/** Implementation of the {@link ProjectCodeOwners} API. */
public class ProjectCodeOwnersImpl implements ProjectCodeOwners {
@@ -34,6 +38,7 @@
private final BranchesCollection branchesCollection;
private final BranchCodeOwnersImpl.Factory branchCodeOwnersApi;
private final GetCodeOwnerProjectConfig getCodeOwnerProjectConfig;
+ private final CheckCodeOwnerConfigFiles checkCodeOwnerConfigFiles;
private final ProjectResource projectResource;
@Inject
@@ -41,10 +46,12 @@
BranchesCollection branchesCollection,
BranchCodeOwnersImpl.Factory branchCodeOwnersApi,
GetCodeOwnerProjectConfig getCodeOwnerProjectConfig,
+ CheckCodeOwnerConfigFiles checkCodeOwnerConfigFiles,
@Assisted ProjectResource projectResource) {
this.branchesCollection = branchesCollection;
this.branchCodeOwnersApi = branchCodeOwnersApi;
this.getCodeOwnerProjectConfig = getCodeOwnerProjectConfig;
+ this.checkCodeOwnerConfigFiles = checkCodeOwnerConfigFiles;
this.projectResource = projectResource;
}
@@ -67,4 +74,23 @@
throw asRestApiException("Cannot get code owner project config", e);
}
}
+
+ @Override
+ public CheckCodeOwnerConfigFilesRequest checkCodeOwnerConfigFiles() throws RestApiException {
+ return new CheckCodeOwnerConfigFilesRequest() {
+ @Override
+ public Map<String, Map<String, List<ConsistencyProblemInfo>>> check()
+ throws RestApiException {
+ try {
+ 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/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index f9c7027..5ae315b 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -344,8 +344,7 @@
Path absolutePath) {
logger.atFine().log("computing path status for %s (bootstrapping mode)", absolutePath);
- AtomicReference<CodeOwnerStatus> codeOwnerStatus =
- new AtomicReference<>(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ CodeOwnerStatus codeOwnerStatus = CodeOwnerStatus.INSUFFICIENT_REVIEWERS;
if (enableImplicitApprovalFromUploader && isProjectOwner(branch.project(), patchSetUploader)) {
// The uploader of the patch set is a project owner and thus a code owner. This means there
@@ -354,7 +353,7 @@
logger.atFine().log(
"the status for path %s is %s since the patch set uploader is a project owner",
absolutePath, CodeOwnerStatus.APPROVED.name());
- codeOwnerStatus.set(CodeOwnerStatus.APPROVED);
+ codeOwnerStatus = CodeOwnerStatus.APPROVED;
} else if (enableImplicitApprovalFromUploader
&& globalCodeOwnerAccountIds.contains(patchSetUploader)) {
// If the uploader of the patch set is a global code owner, there is an implicit code owner
@@ -362,31 +361,31 @@
logger.atFine().log(
"the status for path %s is %s since the patch set uploader is a global code owner",
absolutePath, CodeOwnerStatus.APPROVED.name());
- codeOwnerStatus.set(CodeOwnerStatus.APPROVED);
+ codeOwnerStatus = CodeOwnerStatus.APPROVED;
} else if (!Collections.disjoint(approverAccountIds, globalCodeOwnerAccountIds)) {
// At least one of the global code owners approved the change.
logger.atFine().log(
"the status for path %s is %s since at least one global code owner approved it",
absolutePath, CodeOwnerStatus.APPROVED.name());
- codeOwnerStatus.set(CodeOwnerStatus.APPROVED);
+ codeOwnerStatus = CodeOwnerStatus.APPROVED;
} else if (approverAccountIds.stream()
.anyMatch(approverAccountId -> isProjectOwner(branch.project(), approverAccountId))) {
// At least one of the approvers is a project owner and thus a code owner.
logger.atFine().log(
"the status for path %s is %s since at least one approvers is a project owner",
absolutePath, CodeOwnerStatus.APPROVED.name());
- codeOwnerStatus.set(CodeOwnerStatus.APPROVED);
+ codeOwnerStatus = CodeOwnerStatus.APPROVED;
} else if (reviewerAccountIds.stream()
.anyMatch(reviewerAccountId -> isProjectOwner(branch.project(), reviewerAccountId))) {
// At least one of the reviewers is a project owner and thus a code owner.
logger.atFine().log(
"the status for path %s is %s since at least one reviewer is a project owner",
absolutePath, CodeOwnerStatus.PENDING.name());
- codeOwnerStatus.set(CodeOwnerStatus.PENDING);
+ codeOwnerStatus = CodeOwnerStatus.PENDING;
}
PathCodeOwnerStatus pathCodeOwnerStatus =
- PathCodeOwnerStatus.create(absolutePath, codeOwnerStatus.get());
+ PathCodeOwnerStatus.create(absolutePath, codeOwnerStatus);
logger.atFine().log("pathCodeOwnerStatus = %s", pathCodeOwnerStatus);
return pathCodeOwnerStatus;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
index b4be755..99ee653 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScanner.java
@@ -15,8 +15,11 @@
package com.google.gerrit.plugins.codeowners.backend;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.plugins.codeowners.backend.CodeOwners.getInvalidConfigCause;
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;
@@ -26,10 +29,12 @@
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;
import java.util.concurrent.atomic.AtomicBoolean;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -40,6 +45,8 @@
/** Class to scan a branch for code owner config files. */
@Singleton
public class CodeOwnerConfigScanner {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final GitRepositoryManager repoManager;
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
@@ -66,7 +73,8 @@
codeOwnerConfig -> {
found.set(true);
return false;
- });
+ },
+ ignoreInvalidCodeOwnerConfigFiles());
return found.get();
}
@@ -76,12 +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) {
+ 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);
@@ -96,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());
@@ -107,8 +143,24 @@
String fileName = filePath.getFileName().toString();
CodeOwnerConfig.Key codeOwnerConfigKey =
CodeOwnerConfig.Key.create(branchNameKey, folderPath, fileName);
- Optional<CodeOwnerConfig> codeOwnerConfig =
- codeOwnerBackend.getCodeOwnerConfig(codeOwnerConfigKey, revision);
+ Optional<CodeOwnerConfig> codeOwnerConfig;
+
+ try {
+ codeOwnerConfig = codeOwnerBackend.getCodeOwnerConfig(codeOwnerConfigKey, revision);
+ } catch (StorageException storageException) {
+ Optional<ConfigInvalidException> configInvalidException =
+ getInvalidConfigCause(storageException);
+ if (!configInvalidException.isPresent()) {
+ // Propagate any failure that is not related to the contents of the code owner config.
+ throw storageException;
+ }
+
+ // The code owner config is invalid and cannot be parsed.
+ invalidCodeOwnerConfigCallback.onInvalidCodeOwnerConfig(
+ folderPath.resolve(fileName), configInvalidException.get());
+ continue;
+ }
+
checkState(codeOwnerConfig.isPresent(), "code owner config %s not found", codeOwnerConfig);
boolean visitFurtherCodeOwnerConfigFiles =
codeOwnerConfigVisitor.visit(codeOwnerConfig.get());
@@ -125,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() {
@@ -140,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);
}
@@ -150,4 +217,13 @@
}
};
}
+
+ /**
+ * Returns an {@link InvalidCodeOwnerConfigCallback} instance that ignores invalid code owner
+ * config files.
+ */
+ public static InvalidCodeOwnerConfigCallback ignoreInvalidCodeOwnerConfigFiles() {
+ return (codeOwnerConfigFilePath, configInvalidException) ->
+ logger.atFine().log("ignoring invalid code owner config file %s", codeOwnerConfigFilePath);
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java
index 0c38ef4..519bc8f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwners.java
@@ -16,10 +16,12 @@
import static java.util.Objects.requireNonNull;
+import com.google.common.base.Throwables;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
/**
@@ -69,4 +71,16 @@
codeOwnersPluginConfiguration.getBackend(codeOwnerConfigKey.branchNameKey());
return codeOwnerBackend.getCodeOwnerConfig(codeOwnerConfigKey, null);
}
+
+ /**
+ * Checks whether the given exception was caused by a non-parseable code owner config ({@link
+ * ConfigInvalidException}). If yes, the {@link ConfigInvalidException} is returned. If no, {@link
+ * Optional#empty()} is returned.
+ */
+ public static Optional<ConfigInvalidException> getInvalidConfigCause(Exception e) {
+ return Throwables.getCausalChain(e).stream()
+ .filter(t -> t instanceof ConfigInvalidException)
+ .map(t -> (ConfigInvalidException) t)
+ .findFirst();
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/InvalidCodeOwnerConfigCallback.java b/java/com/google/gerrit/plugins/codeowners/backend/InvalidCodeOwnerConfigCallback.java
new file mode 100644
index 0000000..fed838a
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/backend/InvalidCodeOwnerConfigCallback.java
@@ -0,0 +1,30 @@
+// 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.backend;
+
+import java.nio.file.Path;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+/** Callback interface to let callers handle invalid code owner config files. */
+public interface InvalidCodeOwnerConfigCallback {
+ /**
+ * Invoked when an invalid code owner config file is found.
+ *
+ * @param codeOwnerConfigFilePath the path of the invalid code owner config file
+ * @param configInvalidException the parsing exception
+ */
+ void onInvalidCodeOwnerConfig(
+ Path codeOwnerConfigFilePath, ConfigInvalidException configInvalidException);
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
index 6a0c27c..dd4fd6a 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/AbstractGetCodeOwnersForPath.java
@@ -21,6 +21,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.client.ListAccountsOption;
import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -32,6 +33,7 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScore;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerScoring;
+import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.ServiceUserClassifier;
@@ -59,6 +61,7 @@
@VisibleForTesting public static final int DEFAULT_LIMIT = 10;
private final PermissionBackend permissionBackend;
+ private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
private final CodeOwnerConfigHierarchy codeOwnerConfigHierarchy;
private final Provider<CodeOwnerResolver> codeOwnerResolver;
private final ServiceUserClassifier serviceUserClassifier;
@@ -94,11 +97,13 @@
protected AbstractGetCodeOwnersForPath(
PermissionBackend permissionBackend,
+ CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy,
Provider<CodeOwnerResolver> codeOwnerResolver,
ServiceUserClassifier serviceUserClassifier,
CodeOwnerJson.Factory codeOwnerJsonFactory) {
this.permissionBackend = permissionBackend;
+ this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.codeOwnerConfigHierarchy = codeOwnerConfigHierarchy;
this.codeOwnerResolver = codeOwnerResolver;
this.serviceUserClassifier = serviceUserClassifier;
@@ -112,9 +117,14 @@
parseHexOptions();
validateLimit();
- // The maximal possible distance. This is the distance that applies to code owners that are
- // defined in the root code owner configuration.
- int maxDistance = rsrc.getPath().getNameCount();
+ // The distance that applies to code owners that are defined in the root code owner
+ // configuration.
+ int rootDistance = rsrc.getPath().getNameCount();
+
+ // The maximal possible distance. This is the distance that applies to global code owners and is
+ // by 1 greater than the distance that applies to code owners that are defined in the root code
+ // owner configuration.
+ int maxDistance = rootDistance + 1;
CodeOwnerScoring.Builder distanceScoring = CodeOwnerScore.DISTANCE.createScoring(maxDistance);
@@ -127,7 +137,7 @@
ImmutableSet<CodeOwner> pathCodeOwners =
codeOwnerResolver.get().resolvePathCodeOwners(codeOwnerConfig, rsrc.getPath());
codeOwners.addAll(filterCodeOwners(rsrc, pathCodeOwners));
- int distance = maxDistance - codeOwnerConfig.key().folderPath().getNameCount();
+ int distance = rootDistance - codeOwnerConfig.key().folderPath().getNameCount();
pathCodeOwners.forEach(
localCodeOwner -> distanceScoring.putValueForCodeOwner(localCodeOwner, distance));
@@ -139,12 +149,29 @@
return codeOwners.size() < limit;
});
+ if (codeOwners.size() < limit) {
+ ImmutableSet<CodeOwner> globalCodeOwners = getGlobalCodeOwners(rsrc.getBranch().project());
+ globalCodeOwners.forEach(
+ codeOwner -> distanceScoring.putValueForCodeOwner(codeOwner, maxDistance));
+ codeOwners.addAll(filterCodeOwners(rsrc, globalCodeOwners));
+ }
+
return Response.ok(
codeOwnerJsonFactory
.create(getFillOptions())
.format(sortAndLimit(distanceScoring.build(), ImmutableSet.copyOf(codeOwners))));
}
+ private ImmutableSet<CodeOwner> getGlobalCodeOwners(Project.NameKey projectName) {
+ ImmutableSet<CodeOwner> globalCodeOwners =
+ codeOwnerResolver
+ .get()
+ .resolve(codeOwnersPluginConfiguration.getGlobalCodeOwners(projectName))
+ .collect(toImmutableSet());
+ logger.atFine().log("including global code owners = %s", globalCodeOwners);
+ return globalCodeOwners;
+ }
+
/**
* Filters out code owners that should not be suggested.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
new file mode 100644
index 0000000..09dc34d
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwnerConfigFiles.java
@@ -0,0 +1,236 @@
+// 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.restapi;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+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.entities.Project;
+import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+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.extensions.restapi.UnprocessableEntityException;
+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;
+import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.validation.CodeOwnerConfigValidator;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.git.validators.CommitValidationMessage;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.restapi.project.ListBranches;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * REST endpoint that checks/validates the code owner config files in a project.
+ *
+ * <p>This REST endpoint handles {@code POST
+ * /projects/<project-name>/branches/<branch-name>/code_owners.check_config} requests.
+ *
+ * <p>The implementation of this REST endpoint iterates over all code owner config files in the
+ * project. This means the expected performance of this REST endpoint is rather low and it should
+ * not be used in any critical path where performance matters.
+ */
+@Singleton
+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;
+ private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
+ private final CodeOwnerConfigScanner codeOwnerConfigScanner;
+ private final CodeOwnerConfigValidator codeOwnerConfigValidator;
+
+ @Inject
+ public CheckCodeOwnerConfigFiles(
+ CurrentUser currentUser,
+ PermissionBackend permissionBackend,
+ Provider<ListBranches> listBranches,
+ CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
+ CodeOwnerConfigScanner codeOwnerConfigScanner,
+ CodeOwnerConfigValidator codeOwnerConfigValidator) {
+ this.currentUser = currentUser;
+ this.permissionBackend = permissionBackend;
+ this.listBranches = listBranches;
+ this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
+ this.codeOwnerConfigScanner = codeOwnerConfigScanner;
+ this.codeOwnerConfigValidator = codeOwnerConfigValidator;
+ }
+
+ @Override
+ public Response<Map<String, Map<String, List<ConsistencyProblemInfo>>>> apply(
+ ProjectResource projectResource, CheckCodeOwnerConfigFilesInput input)
+ throws RestApiException, PermissionBackendException, IOException {
+ if (!currentUser.isIdentifiedUser()) {
+ throw new AuthException("Authentication required");
+ }
+
+ // This REST endpoint requires the caller to be a project owner.
+ permissionBackend
+ .currentUser()
+ .project(projectResource.getNameKey())
+ .check(ProjectPermission.WRITE_CONFIG);
+
+ logger.atFine().log(
+ "checking code owner config files for project %s"
+ + " (validateDisabledBranches = %s, branches = %s, path = %s)",
+ projectResource.getNameKey(), input.validateDisabledBranches, input.branches, input.path);
+
+ ImmutableSet<BranchNameKey> branches = branches(projectResource);
+
+ validateInput(projectResource.getNameKey(), branches, input);
+
+ ImmutableMap.Builder<String, Map<String, List<ConsistencyProblemInfo>>> resultsByBranchBuilder =
+ ImmutableMap.builder();
+ branches.stream()
+ .filter(branchNameKey -> shouldValidateBranch(input, branchNameKey))
+ .filter(
+ branchNameKey ->
+ validateDisabledBranches(input)
+ || !codeOwnersPluginConfiguration.isDisabled(branchNameKey))
+ .forEach(
+ branchNameKey ->
+ resultsByBranchBuilder.put(
+ branchNameKey.branch(), checkBranch(input.path, branchNameKey)));
+ return Response.ok(resultsByBranchBuilder.build());
+ }
+
+ private ImmutableSet<BranchNameKey> branches(ProjectResource projectResource)
+ throws RestApiException, IOException, PermissionBackendException {
+ return listBranches.get().apply(projectResource).value().stream()
+ .filter(branchInfo -> !"HEAD".equals(branchInfo.ref))
+ .map(branchInfo -> BranchNameKey.create(projectResource.getNameKey(), branchInfo.ref))
+ .collect(toImmutableSet());
+ }
+
+ private Map<String, List<ConsistencyProblemInfo>> checkBranch(
+ String pathGlob, BranchNameKey branchNameKey) {
+ ListMultimap<String, ConsistencyProblemInfo> problemsByPath = LinkedListMultimap.create();
+ CodeOwnerBackend codeOwnerBackend = codeOwnersPluginConfiguration.getBackend(branchNameKey);
+ codeOwnerConfigScanner.visit(
+ branchNameKey,
+ codeOwnerConfig -> {
+ problemsByPath.putAll(
+ codeOwnerBackend.getFilePath(codeOwnerConfig.key()).toString(),
+ checkCodeOwnerConfig(codeOwnerBackend, codeOwnerConfig));
+ return true;
+ },
+ (codeOwnerConfigFilePath, configInvalidException) -> {
+ problemsByPath.put(
+ codeOwnerConfigFilePath.toString(),
+ new ConsistencyProblemInfo(
+ ConsistencyProblemInfo.Status.ERROR, configInvalidException.getMessage()));
+ },
+ pathGlob);
+
+ return Multimaps.asMap(problemsByPath);
+ }
+
+ private ImmutableList<ConsistencyProblemInfo> checkCodeOwnerConfig(
+ CodeOwnerBackend codeOwnerBackend, CodeOwnerConfig codeOwnerConfig) {
+ return codeOwnerConfigValidator
+ .validateCodeOwnerConfig(currentUser.asIdentifiedUser(), codeOwnerBackend, codeOwnerConfig)
+ .map(this::createConsistencyProblemInfo)
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .collect(toImmutableList());
+ }
+
+ private Optional<ConsistencyProblemInfo> createConsistencyProblemInfo(
+ CommitValidationMessage commitValidationMessage) {
+ switch (commitValidationMessage.getType()) {
+ case ERROR:
+ return Optional.of(
+ new ConsistencyProblemInfo(
+ ConsistencyProblemInfo.Status.ERROR, commitValidationMessage.getMessage()));
+ case WARNING:
+ return Optional.of(
+ new ConsistencyProblemInfo(
+ ConsistencyProblemInfo.Status.WARNING, commitValidationMessage.getMessage()));
+ case HINT:
+ case OTHER:
+ return Optional.empty();
+ }
+
+ throw new IllegalStateException(
+ String.format(
+ "unknown message type %s for message %s",
+ commitValidationMessage.getType(), commitValidationMessage.getMessage()));
+ }
+
+ private void validateInput(
+ Project.NameKey projectName,
+ ImmutableSet<BranchNameKey> branches,
+ CheckCodeOwnerConfigFilesInput input)
+ throws RestApiException {
+ if (input.branches != null) {
+ for (String branchName : input.branches) {
+ BranchNameKey branchNameKey = BranchNameKey.create(projectName, branchName);
+ if (!branches.contains(branchNameKey)) {
+ throw new UnprocessableEntityException(String.format("branch %s not found", branchName));
+ }
+
+ if ((input.validateDisabledBranches == null || !input.validateDisabledBranches)
+ && codeOwnersPluginConfiguration.isDisabled(branchNameKey)) {
+ throw new BadRequestException(
+ String.format(
+ "code owners functionality for branch %s is disabled,"
+ + " set 'validate_disabled_braches' in the input to 'true' if code owner"
+ + " config files in this branch should be validated",
+ branchName));
+ }
+ }
+ }
+ }
+
+ private static boolean shouldValidateBranch(
+ CheckCodeOwnerConfigFilesInput input, BranchNameKey branchNameKey) {
+ boolean shouldValidateBranch =
+ input.branches == null
+ || input.branches.contains(branchNameKey.branch())
+ || input.branches.contains(branchNameKey.shortName());
+ if (!shouldValidateBranch) {
+ logger.atFine().log("skip validation for branch %s", branchNameKey.branch());
+ }
+ return shouldValidateBranch;
+ }
+
+ private static boolean validateDisabledBranches(CheckCodeOwnerConfigFilesInput input) {
+ return input.validateDisabledBranches != null && input.validateDisabledBranches;
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java
index 26f31fc..fe26037 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigFiles.java
@@ -15,18 +15,22 @@
package com.google.gerrit.plugins.codeowners.restapi;
import static com.google.common.collect.ImmutableList.toImmutableList;
+import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigScanner;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.project.BranchResource;
import com.google.inject.Inject;
-import com.google.inject.Singleton;
import java.nio.file.Path;
import java.util.List;
+import org.kohsuke.args4j.Option;
/**
* REST endpoint that lists the code owner config files in a branch.
@@ -38,11 +42,22 @@
* branch. This means the expected performance of this REST endpoint is rather low and it should not
* be used in any critical path where performance matters.
*/
-@Singleton
public class GetCodeOwnerConfigFiles implements RestReadView<BranchResource> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
private final CodeOwnerConfigScanner codeOwnerConfigScanner;
+ private String email;
+
+ @Option(
+ name = "--email",
+ metaVar = "EMAIL",
+ usage = "limits the returned code owner config files to those that contain this email")
+ public void setEmail(@Nullable String email) {
+ this.email = email;
+ }
+
@Inject
public GetCodeOwnerConfigFiles(
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
@@ -56,13 +71,47 @@
CodeOwnerBackend codeOwnerBackend =
codeOwnersPluginConfiguration.getBackend(resource.getBranchKey());
ImmutableList.Builder<Path> codeOwnerConfigs = ImmutableList.builder();
+
+ if (email != null) {
+ logger.atFine().log(
+ "limiting the returned code owner config files to those that contain the email %s",
+ email);
+ }
+
codeOwnerConfigScanner.visit(
resource.getBranchKey(),
codeOwnerConfig -> {
- codeOwnerConfigs.add(codeOwnerBackend.getFilePath(codeOwnerConfig.key()));
+ Path codeOwnerConfigPath = codeOwnerBackend.getFilePath(codeOwnerConfig.key());
+ if (email == null || containsEmail(codeOwnerConfig, codeOwnerConfigPath, email)) {
+ codeOwnerConfigs.add(codeOwnerConfigPath);
+ }
return true;
- });
+ },
+ CodeOwnerConfigScanner.ignoreInvalidCodeOwnerConfigFiles());
return Response.ok(
codeOwnerConfigs.build().stream().map(Path::toString).collect(toImmutableList()));
}
+
+ /**
+ * Checks whether the given code owner config contains the given email.
+ *
+ * @param codeOwnerConfig the code owner config for which it should be checked if it contains the
+ * email
+ * @param codeOwnerConfigPath the path of the code owner config
+ * @param email the email
+ * @return whether the given code owner config contains the given email
+ */
+ private boolean containsEmail(
+ CodeOwnerConfig codeOwnerConfig, Path codeOwnerConfigPath, String email) {
+ requireNonNull(email, "email");
+ boolean containsEmail =
+ codeOwnerConfig.codeOwnerSets().stream()
+ .flatMap(codeOwnerSet -> codeOwnerSet.codeOwners().stream())
+ .anyMatch(codeOwnerReference -> email.equals(codeOwnerReference.email()));
+ if (!containsEmail) {
+ logger.atFine().log(
+ "Filtering out %s since it doesn't contain the email", codeOwnerConfigPath);
+ }
+ return containsEmail;
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
index 6c5a3a6..baf260d 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInBranch.java
@@ -25,6 +25,7 @@
import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigHierarchy;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
+import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.account.ServiceUserClassifier;
import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -65,6 +66,7 @@
@Inject
GetCodeOwnersForPathInBranch(
PermissionBackend permissionBackend,
+ CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy,
Provider<CodeOwnerResolver> codeOwnerResolver,
ServiceUserClassifier serviceUserClassifier,
@@ -72,6 +74,7 @@
GitRepositoryManager repoManager) {
super(
permissionBackend,
+ codeOwnersPluginConfiguration,
codeOwnerConfigHierarchy,
codeOwnerResolver,
serviceUserClassifier,
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
index 4ad31aa..43a5d21 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnersForPathInChange.java
@@ -20,6 +20,7 @@
import com.google.gerrit.plugins.codeowners.api.CodeOwnerInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigHierarchy;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerResolver;
+import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.server.account.ServiceUserClassifier;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -40,12 +41,14 @@
@Inject
GetCodeOwnersForPathInChange(
PermissionBackend permissionBackend,
+ CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerConfigHierarchy codeOwnerConfigHierarchy,
Provider<CodeOwnerResolver> codeOwnerResolver,
ServiceUserClassifier serviceUserClassifier,
CodeOwnerJson.Factory codeOwnerJsonFactory) {
super(
permissionBackend,
+ codeOwnersPluginConfiguration,
codeOwnerConfigHierarchy,
codeOwnerResolver,
serviceUserClassifier,
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java b/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
index daf66ed..8599f39 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
@@ -43,5 +43,6 @@
get(CHANGE_KIND, "code_owners.status").to(GetCodeOwnerStatus.class);
get(PROJECT_KIND, "code_owners.project_config").to(GetCodeOwnerProjectConfig.class);
+ post(PROJECT_KIND, "code_owners.check_config").to(CheckCodeOwnerConfigFiles.class);
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index f78a016..54f8f09 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -17,10 +17,10 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.gerrit.plugins.codeowners.backend.CodeOwners.getInvalidConfigCause;
import static java.util.Objects.requireNonNull;
import com.google.auto.value.AutoValue;
-import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
@@ -523,18 +523,6 @@
}
/**
- * Checks whether the given exception was caused by a non-parseable code owner config ({@link
- * ConfigInvalidException}). If yes, the {@link ConfigInvalidException} is returned. If no, {@link
- * Optional#empty()} is returned.
- */
- private static Optional<ConfigInvalidException> getInvalidConfigCause(Exception e) {
- return Throwables.getCausalChain(e).stream()
- .filter(t -> t instanceof ConfigInvalidException)
- .map(t -> (ConfigInvalidException) t)
- .findFirst();
- }
-
- /**
* Validates the given code owner config and returns validation issues as stream.
*
* <p>Validation errors that exist in both code owner configs are returned as warning (because
@@ -575,7 +563,7 @@
* @return a stream of validation messages that describe issues with the code owner config, an
* empty stream if there are no issues
*/
- private Stream<CommitValidationMessage> validateCodeOwnerConfig(
+ public Stream<CommitValidationMessage> validateCodeOwnerConfig(
IdentifiedUser user, CodeOwnerBackend codeOwnerBackend, CodeOwnerConfig codeOwnerConfig) {
requireNonNull(codeOwnerConfig, "codeOwnerConfig");
return Streams.concat(
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
index d0951ac..360e2a7 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/AbstractGetCodeOwnersForPathIT.java
@@ -630,4 +630,95 @@
List<CodeOwnerInfo> codeOwnerInfos = queryCodeOwners("/foo/bar/baz.md");
assertThat(codeOwnerInfos).comparingElementsUsing(hasAccountId()).containsExactly(admin.id());
}
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "global.owner@example.com")
+ public void getGlobalCodeOwners() throws Exception {
+ TestAccount globalOwner =
+ accountCreator.create("global_owner", "global.owner@example.com", "Global Owner", null);
+ assertThat(queryCodeOwners("/foo/bar/baz.md"))
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(globalOwner.id());
+ }
+
+ @Test
+ @GerritConfig(
+ name = "plugin.code-owners.globalCodeOwner",
+ values = {"global.owner1@example.com", "global.owner2@example.com"})
+ public void getWithGlobalCodeOwnersAndLimit() throws Exception {
+ TestAccount globalOwner1 =
+ accountCreator.create(
+ "global_owner_1", "global.owner1@example.com", "Global Owner 1", null);
+ TestAccount globalOwner2 =
+ accountCreator.create(
+ "global_owner_2", "global.owner2@example.com", "Global Owner 2", null);
+
+ TestAccount user2 = accountCreator.user2();
+
+ // create some code owner configs
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail(user.email())
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ // get code owners with different limits
+ List<CodeOwnerInfo> codeOwnerInfos =
+ queryCodeOwners(getCodeOwnersApi().query().withLimit(1), "/foo/bar/baz.md");
+ assertThat(codeOwnerInfos).hasSize(1);
+ // the first 2 code owners have the same scoring, so their order is random and we don't know
+ // which of them we get when the limit is 1
+ assertThatList(codeOwnerInfos).element(0).hasAccountIdThat().isAnyOf(user.id(), user2.id());
+
+ codeOwnerInfos = queryCodeOwners(getCodeOwnersApi().query().withLimit(2), "/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(user.id(), user2.id());
+
+ codeOwnerInfos = getCodeOwnersApi().query().withLimit(3).get("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(admin.id(), user.id(), user2.id());
+
+ codeOwnerInfos = getCodeOwnersApi().query().withLimit(4).get("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos).hasSize(4);
+ // the order of the first 2 code owners is random
+ assertThatList(codeOwnerInfos).element(0).hasAccountIdThat().isAnyOf(user.id(), user2.id());
+ assertThatList(codeOwnerInfos).element(1).hasAccountIdThat().isAnyOf(user.id(), user2.id());
+ assertThatList(codeOwnerInfos).element(2).hasAccountIdThat().isEqualTo(admin.id());
+ // the order of the global code owners is random
+ assertThatList(codeOwnerInfos)
+ .element(3)
+ .hasAccountIdThat()
+ .isAnyOf(globalOwner1.id(), globalOwner2.id());
+
+ codeOwnerInfos = getCodeOwnersApi().query().withLimit(5).get("/foo/bar/baz.md");
+ assertThat(codeOwnerInfos)
+ .comparingElementsUsing(hasAccountId())
+ .containsExactly(admin.id(), user.id(), user2.id(), globalOwner1.id(), globalOwner2.id());
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "service.user@example.com")
+ public void globalCodeOwnersThatAreServiceUsersAreFilteredOut() throws Exception {
+ TestAccount serviceUser =
+ accountCreator.create("serviceUser", "service.user@example.com", "Service User", null);
+ groupOperations
+ .group(groupCache.get(AccountGroup.nameKey("Service Users")).get().getGroupUUID())
+ .forUpdate()
+ .addMember(serviceUser.id())
+ .update();
+ assertThat(queryCodeOwners("/foo/bar/baz.md")).isEmpty();
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
new file mode 100644
index 0000000..a0f3384
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerConfigFilesIT.java
@@ -0,0 +1,488 @@
+// 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.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;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+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.entities.Permission;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
+import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.plugins.codeowners.JgitPath;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigImportMode;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigReference;
+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.List;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Acceptance test for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles} REST endpoint.
+ *
+ * <p>Further tests for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles} REST endpoint that
+ * require using the REST API are implemented in {@link
+ * com.google.gerrit.plugins.codeowners.acceptance.restapi.CheckCodeOwnerConfigFilesRestIT}.
+ */
+public class CheckCodeOwnerConfigFilesIT extends AbstractCodeOwnersIT {
+ @Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ProjectOperations projectOperations;
+
+ private BackendConfig backendConfig;
+
+ @Before
+ public void setUpCodeOwnersPlugin() throws Exception {
+ backendConfig = plugin.getSysInjector().getInstance(BackendConfig.class);
+ }
+
+ @Test
+ public void requiresCallerToBeProjectOwner() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ AuthException authException =
+ assertThrows(AuthException.class, () -> checkCodeOwnerConfigFilesIn(project));
+ assertThat(authException).hasMessageThat().isEqualTo("write refs/meta/config not permitted");
+ }
+
+ @Test
+ public void noCodeOwnerConfigFile() throws Exception {
+ assertThat(checkCodeOwnerConfigFilesIn(project))
+ .containsExactly(
+ "refs/heads/master", ImmutableMap.of(),
+ "refs/meta/config", ImmutableMap.of());
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.disabledBranch", value = "refs/meta/config")
+ public void disabledBranchesAreSkipped() throws Exception {
+ assertThat(checkCodeOwnerConfigFilesIn(project))
+ .containsExactly("refs/heads/master", ImmutableMap.of());
+ }
+
+ @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
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/baz/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ assertThat(checkCodeOwnerConfigFilesIn(project))
+ .containsExactly(
+ "refs/heads/master", ImmutableMap.of(),
+ "refs/meta/config", ImmutableMap.of());
+ }
+
+ @Test
+ public void nonParseableCodeOwnerConfigFile() throws Exception {
+ String codeOwnerConfigPath = "/" + getCodeOwnerConfigFileName();
+ createInvalidCodeOwnerConfig(codeOwnerConfigPath);
+
+ assertThat(checkCodeOwnerConfigFilesIn(project))
+ .containsExactly(
+ "refs/heads/master",
+ ImmutableMap.of(
+ codeOwnerConfigPath,
+ ImmutableList.of(
+ error(
+ String.format(
+ "invalid code owner config file '%s':\n %s",
+ codeOwnerConfigPath,
+ getParsingErrorMessage(
+ ImmutableMap.of(
+ FindOwnersBackend.class,
+ "invalid line: INVALID",
+ ProtoBackend.class,
+ "1:8: Expected \"{\".")))))),
+ "refs/meta/config", ImmutableMap.of());
+ }
+
+ @Test
+ public void issuesInCodeOwnerConfigFile() 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();
+
+ // Also create a code owner config files without issues.
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/baz")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ assertThat(checkCodeOwnerConfigFilesIn(project))
+ .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)))),
+ 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))))),
+ "refs/meta/config", ImmutableMap.of());
+ }
+
+ @Test
+ public void validateSpecifiedBranches() throws Exception {
+ gApi.projects().name(project.get()).branch("stable-1.0").create(new BranchInput());
+ gApi.projects().name(project.get()).branch("stable-1.1").create(new BranchInput());
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("refs/heads/stable-1.0", "refs/heads/stable-1.1"))
+ .check())
+ .containsExactly(
+ "refs/heads/stable-1.0", ImmutableMap.of(),
+ "refs/heads/stable-1.1", ImmutableMap.of());
+ }
+
+ @Test
+ public void validateSpecifiedBranches_shortNames() throws Exception {
+ gApi.projects().name(project.get()).branch("stable-1.0").create(new BranchInput());
+ gApi.projects().name(project.get()).branch("stable-1.1").create(new BranchInput());
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("stable-1.0", "stable-1.1"))
+ .check())
+ .containsExactly(
+ "refs/heads/stable-1.0", ImmutableMap.of(),
+ "refs/heads/stable-1.1", ImmutableMap.of());
+ }
+
+ @Test
+ public void cannotValidateNonExistingBranch() throws Exception {
+ UnprocessableEntityException exception =
+ assertThrows(
+ UnprocessableEntityException.class,
+ () ->
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("refs/heads/non-existing"))
+ .check());
+ assertThat(exception).hasMessageThat().isEqualTo("branch refs/heads/non-existing not found");
+ }
+
+ @Test
+ public void cannotValidateNonVisibleBranch() throws Exception {
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(block(Permission.READ).ref("refs/*").group(REGISTERED_USERS))
+ .update();
+
+ UnprocessableEntityException exception =
+ assertThrows(
+ UnprocessableEntityException.class,
+ () ->
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("refs/heads/master"))
+ .check());
+ assertThat(exception).hasMessageThat().isEqualTo("branch refs/heads/master not found");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.disabledBranch", value = "refs/meta/config")
+ public void cannotValidateDisabledBranchWithoutEnablingValidationForDisabledBranches()
+ throws Exception {
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .setBranches(ImmutableList.of("refs/meta/config"))
+ .check());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ "code owners functionality for branch refs/meta/config is disabled,"
+ + " set 'validate_disabled_braches' in the input to 'true' if code owner config"
+ + " files in this branch should be validated");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.disabledBranch", value = "refs/meta/config")
+ public void validateSpecifiedBranchThatIsDisabled() throws Exception {
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .checkCodeOwnerConfigFiles()
+ .validateDisabledBranches()
+ .setBranches(ImmutableList.of("refs/meta/config"))
+ .check())
+ .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);
+ }
+
+ private Map<String, Map<String, List<ConsistencyProblemInfo>>> checkCodeOwnerConfigFilesIn(
+ Project.NameKey projectName) throws RestApiException {
+ return projectCodeOwnersApiFactory.project(projectName).checkCodeOwnerConfigFiles().check();
+ }
+
+ private String getCodeOwnerConfigFilePath(CodeOwnerConfig.Key codeOwnerConfigKey) {
+ return backendConfig.getDefaultBackend().getFilePath(codeOwnerConfigKey).toString();
+ }
+
+ private String getCodeOwnerConfigFileName() {
+ CodeOwnerBackend backend = backendConfig.getDefaultBackend();
+ if (backend instanceof FindOwnersBackend) {
+ return FindOwnersBackend.CODE_OWNER_CONFIG_FILE_NAME;
+ } else if (backend instanceof ProtoBackend) {
+ return ProtoBackend.CODE_OWNER_CONFIG_FILE_NAME;
+ }
+ throw new IllegalStateException("unknown code owner backend: " + backend.getClass().getName());
+ }
+
+ private void createInvalidCodeOwnerConfig(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");
+ }
+
+ private String getParsingErrorMessage(
+ ImmutableMap<Class<? extends CodeOwnerBackend>, String> messagesByBackend) {
+ CodeOwnerBackend codeOwnerBackend = backendConfig.getDefaultBackend();
+ assertThat(messagesByBackend).containsKey(codeOwnerBackend.getClass());
+ return messagesByBackend.get(codeOwnerBackend.getClass());
+ }
+}
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 1c0c145..88b8421 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerConfigFilesIT.java
@@ -16,12 +16,16 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.plugins.codeowners.JgitPath;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerSet;
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;
@@ -179,6 +183,129 @@
.isEmpty();
}
+ @Test
+ public void getCodeOwnerConfigFilesIfInvalidCodeOwnerConfigFilesExist() throws Exception {
+ createInvalidCodeOwnerConfig(getCodeOwnerConfigFileName());
+
+ CodeOwnerConfig.Key codeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .branch("master")
+ .codeOwnerConfigFiles()
+ .paths())
+ .containsExactly(getCodeOwnerConfigFilePath(codeOwnerConfigKey));
+ }
+
+ @Test
+ public void filterByEmail() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+ TestAccount user3 = accountCreator.create("user3", "user3@example.com", "User3", null);
+
+ CodeOwnerConfig.Key codeOwnerConfigKey1 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ CodeOwnerConfig.Key codeOwnerConfigKey2 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ CodeOwnerConfig.Key codeOwnerConfigKey3 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("foo")
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ CodeOwnerConfig.Key codeOwnerConfigKey4 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/baz/")
+ .addCodeOwnerEmail(admin.email())
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ CodeOwnerConfig.Key codeOwnerConfigKey5 =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/xyz/")
+ .addCodeOwnerEmail(admin.email())
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .branch("master")
+ .codeOwnerConfigFiles()
+ .withEmail(admin.email())
+ .paths())
+ .containsExactly(
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey2),
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey4),
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey5))
+ .inOrder();
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .branch("master")
+ .codeOwnerConfigFiles()
+ .withEmail(user.email())
+ .paths())
+ .containsExactly(
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey1),
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey3),
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey4))
+ .inOrder();
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .branch("master")
+ .codeOwnerConfigFiles()
+ .withEmail(user2.email())
+ .paths())
+ .containsExactly(getCodeOwnerConfigFilePath(codeOwnerConfigKey5));
+
+ assertThat(
+ projectCodeOwnersApiFactory
+ .project(project)
+ .branch("master")
+ .codeOwnerConfigFiles()
+ .withEmail(user3.email())
+ .paths())
+ .isEmpty();
+ }
+
private String getCodeOwnerConfigFilePath(CodeOwnerConfig.Key codeOwnerConfigKey) {
return backendConfig.getDefaultBackend().getFilePath(codeOwnerConfigKey).toString();
}
@@ -192,4 +319,14 @@
}
throw new IllegalStateException("unknown code owner backend: " + backend.getClass().getName());
}
+
+ private void createInvalidCodeOwnerConfig(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");
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CheckCodeOwnerConfigFilesRestIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CheckCodeOwnerConfigFilesRestIT.java
new file mode 100644
index 0000000..eaf3ef1
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CheckCodeOwnerConfigFilesRestIT.java
@@ -0,0 +1,64 @@
+// 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.acceptance.restapi;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
+import org.junit.Test;
+
+/**
+ * Acceptance test for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles} REST endpoint that
+ * require using REST.
+ *
+ * <p>Acceptance test for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.CheckCodeOwnerConfigFiles} REST endpoint that can
+ * use the Java API are implemented in {@link
+ * com.google.gerrit.plugins.codeowners.acceptance.api.CheckCodeOwnerConfigFilesIT}.
+ *
+ * <p>The tests in this class do not depend on the used code owner backend, hence we do not need to
+ * extend {@link AbstractCodeOwnersIT}.
+ */
+public class CheckCodeOwnerConfigFilesRestIT extends AbstractCodeOwnersTest {
+ @Test
+ @GerritConfig(name = "plugin.code-owners.backend", value = "non-existing-backend")
+ public void cannotCheckCodeOwnerConfigFilesIfPluginConfigurationIsInvalid() throws Exception {
+ RestResponse r =
+ adminRestSession.post(
+ String.format(
+ "/projects/%s/code_owners.check_config", IdString.fromDecoded(project.get())));
+ r.assertConflict();
+ assertThat(r.getEntityContent())
+ .contains(
+ "Invalid configuration of the code-owners plugin. Code owner backend"
+ + " 'non-existing-backend' that is configured in gerrit.config (parameter"
+ + " plugin.code-owners.backend) not found.");
+ }
+
+ @Test
+ public void cannotCheckCodeOwnerConfigFilesAnonymously() throws Exception {
+ RestResponse r =
+ anonymousRestSession.post(
+ String.format(
+ "/projects/%s/code_owners.check_config", IdString.fromDecoded(project.get())));
+ r.assertForbidden();
+ assertThat(r.getEntityContent()).contains("Authentication required");
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java
index ae1aee8..7d90176 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java
@@ -41,7 +41,9 @@
ImmutableList.of(RestCall.get("/changes/%s/code-owners~code_owners.status"));
private static final ImmutableList<RestCall> PROJECT_ENDPOINTS =
- ImmutableList.of(RestCall.get("/projects/%s/code-owners~code_owners.project_config"));
+ ImmutableList.of(
+ RestCall.get("/projects/%s/code-owners~code_owners.project_config"),
+ RestCall.post("/projects/%s/code_owners.check_config"));
private static final ImmutableList<RestCall> BRANCH_ENDPOINTS =
ImmutableList.of(
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScannerTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScannerTest.java
index feaa728..ea0eab7 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScannerTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigScannerTest.java
@@ -17,14 +17,19 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.plugins.codeowners.JgitPath;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.acceptance.testsuite.CodeOwnerConfigOperations;
+import com.google.gerrit.plugins.codeowners.config.StatusConfig;
+import java.nio.file.Paths;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -44,6 +49,7 @@
@Rule public final MockitoRule mockito = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);
@Mock private CodeOwnerConfigVisitor visitor;
+ @Mock private InvalidCodeOwnerConfigCallback invalidCodeOwnerConfigCallback;
private CodeOwnerConfigOperations codeOwnerConfigOperations;
private CodeOwnerConfigScanner codeOwnerConfigScanner;
@@ -58,7 +64,9 @@
@Test
public void cannotVisitCodeOwnerConfigsForNullBranch() throws Exception {
NullPointerException npe =
- assertThrows(NullPointerException.class, () -> codeOwnerConfigScanner.visit(null, visitor));
+ assertThrows(
+ NullPointerException.class,
+ () -> codeOwnerConfigScanner.visit(null, visitor, invalidCodeOwnerConfigCallback));
assertThat(npe).hasMessageThat().isEqualTo("branchNameKey");
}
@@ -67,17 +75,31 @@
BranchNameKey branchNameKey = BranchNameKey.create(project, "master");
NullPointerException npe =
assertThrows(
- NullPointerException.class, () -> codeOwnerConfigScanner.visit(branchNameKey, null));
+ NullPointerException.class,
+ () ->
+ codeOwnerConfigScanner.visit(branchNameKey, null, invalidCodeOwnerConfigCallback));
assertThat(npe).hasMessageThat().isEqualTo("codeOwnerConfigVisitor");
}
@Test
+ public void cannotVisitCodeOwnerConfigsWithNullCallback() throws Exception {
+ BranchNameKey branchNameKey = BranchNameKey.create(project, "master");
+ NullPointerException npe =
+ assertThrows(
+ NullPointerException.class,
+ () -> codeOwnerConfigScanner.visit(branchNameKey, visitor, null));
+ assertThat(npe).hasMessageThat().isEqualTo("invalidCodeOwnerConfigCallback");
+ }
+
+ @Test
public void cannotVisitCodeOwnerConfigsForNonExistingBranch() throws Exception {
BranchNameKey branchNameKey = BranchNameKey.create(project, "non-existing");
IllegalStateException exception =
assertThrows(
IllegalStateException.class,
- () -> codeOwnerConfigScanner.visit(branchNameKey, visitor));
+ () ->
+ codeOwnerConfigScanner.visit(
+ branchNameKey, visitor, invalidCodeOwnerConfigCallback));
assertThat(exception)
.hasMessageThat()
.isEqualTo(
@@ -89,6 +111,7 @@
public void visitorNotInvokedIfNoCodeOwnerConfigFilesExists() throws Exception {
visit();
verifyZeroInteractions(visitor);
+ verifyZeroInteractions(invalidCodeOwnerConfigCallback);
}
@Test
@@ -112,6 +135,50 @@
visit();
verifyZeroInteractions(visitor);
+ verifyZeroInteractions(invalidCodeOwnerConfigCallback);
+ }
+
+ @Test
+ public void visitorNotInvokedForInvalidCodeOwnerConfigFiles() throws Exception {
+ createInvalidCodeOwnerConfig("/OWNERS");
+
+ visit();
+ verifyZeroInteractions(visitor);
+
+ // Verify that we received the expected callbacks for the invalid code onwer config.
+ Mockito.verify(invalidCodeOwnerConfigCallback)
+ .onInvalidCodeOwnerConfig(eq(Paths.get("/OWNERS")), any(ConfigInvalidException.class));
+ verifyNoMoreInteractions(invalidCodeOwnerConfigCallback);
+ }
+
+ @Test
+ public void visitorInvokedForValidCodeOwnerConfigFilesEvenIfInvalidCodeOwnerConfigFileExist()
+ throws Exception {
+ createInvalidCodeOwnerConfig("/OWNERS");
+
+ // Create a valid code owner config file.
+ CodeOwnerConfig.Key codeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .fileName("OWNERS")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ when(visitor.visit(any(CodeOwnerConfig.class))).thenReturn(true);
+ visit();
+
+ // Verify that we received the expected callbacks.
+ Mockito.verify(visitor)
+ .visit(codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).get());
+ verifyNoMoreInteractions(visitor);
+
+ // Verify that we received the expected callbacks for the invalid code onwer config.
+ Mockito.verify(invalidCodeOwnerConfigCallback)
+ .onInvalidCodeOwnerConfig(eq(Paths.get("/OWNERS")), any(ConfigInvalidException.class));
+ verifyNoMoreInteractions(invalidCodeOwnerConfigCallback);
}
@Test
@@ -200,6 +267,8 @@
Mockito.verify(visitor)
.visit(codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).get());
verifyNoMoreInteractions(visitor);
+
+ verifyZeroInteractions(invalidCodeOwnerConfigCallback);
}
@Test
@@ -249,6 +318,8 @@
.verify(visitor)
.visit(codeOwnerConfigOperations.codeOwnerConfig(fooBarCodeOwnerConfigKey).get());
verifyNoMoreInteractions(visitor);
+
+ verifyZeroInteractions(invalidCodeOwnerConfigCallback);
}
@Test
@@ -297,6 +368,8 @@
.verify(visitor)
.visit(codeOwnerConfigOperations.codeOwnerConfig(fooCodeOwnerConfigKey).get());
verifyNoMoreInteractions(visitor);
+
+ verifyZeroInteractions(invalidCodeOwnerConfigCallback);
}
@Test
@@ -324,7 +397,37 @@
.isTrue();
}
+ @Test
+ public void containsACodeOwnerConfigFile_invalidCodeOwnerConfigFileExists() throws Exception {
+ createInvalidCodeOwnerConfig("/OWNERS");
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .fileName("OWNERS")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ assertThat(
+ codeOwnerConfigScanner.containsAnyCodeOwnerConfigFile(
+ BranchNameKey.create(project, "master")))
+ .isTrue();
+ }
+
private void visit() {
- codeOwnerConfigScanner.visit(BranchNameKey.create(project, "master"), visitor);
+ codeOwnerConfigScanner.visit(
+ BranchNameKey.create(project, "master"), visitor, invalidCodeOwnerConfigCallback);
+ }
+
+ private void createInvalidCodeOwnerConfig(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");
}
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index b3ea9b9..84e36a1 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -56,6 +56,73 @@
}
```
+### <a id="check-code-owner-config-files">Check Code Owner Config Files
+_'POST /projects/[\{project-name\}](../../../Documentation/rest-api-projects.html#project-name)/code_owners.check_config'_
+
+Checks/validates the code owner config files in a project.
+
+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), 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
+[ConsistencyProblemInfo](../../../Documentation/rest-api-config.html#consistency-problem-info)
+entities.
+
+Code owner config files that have no issues are omitted from the response.
+
+#### Request
+
+```
+ POST /projects/foo%2Fbar/code_owners.check_config HTTP/1.0
+```
+
+#### Response
+
+```
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "refs/heads/master": {
+ "/OWNERS": [
+ {
+ "status": "ERROR",
+ "message": "code owner email 'foo@example.com' in '/OWNERS' cannot be resolved for John Doe"
+ },
+ {
+ "status": "ERROR",
+ "message": "code owner email 'bar@example.com' in '/OWNERS' cannot be resolved for John Doe"
+ }
+ ],
+ "/foo/OWNERS": [
+ {
+ "status": "ERROR",
+ "message": "invalid global import in '/foo/OWNERS': '/not-a-code-owner-config' is not a code owner config file"
+ }
+ ]
+ },
+ "refs/heads/stable-1.0" {},
+ "refs/heads/stable-1.1" {
+ "/foo/OWNERS": [
+ {
+ "status": "ERROR",
+ "message": "invalid global import in '/foo/OWNERS': '/not-a-code-owner-config' is not a code owner config file"
+ }
+ ]
+ }
+ }
+```
+
## <a id="branch-endpoints">Branch Endpoints
### <a id="list-code-owner-config-files">List Code Owner Config Files
@@ -68,6 +135,13 @@
why this REST endpoint must not be used in any critical paths where performance
matters.
+The following request parameters can be specified:
+
+| Field Name | | Description |
+| ----------- | -------- | ----------- |
+| `email` | optional | Code owner email that must appear in the returned
+code owner config files.
+
#### Request
```
@@ -78,6 +152,8 @@
result also includes code owner config that use name prefixes
('\<prefix\>_OWNERS') or name extensions ('OWNERS_\<extension\>').
+Non-parseable code owner config files are omitted from the response.
+
#### Response
```
@@ -299,6 +375,18 @@
---
+### <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`.
+| `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.
+
+---
+
### <a id="code-owner-config-info"> CodeOwnerConfigInfo
The `CodeOwnerConfigInfo` entity contains information about a code owner config
for a path.
diff --git a/resources/Documentation/setup-guide.md b/resources/Documentation/setup-guide.md
index 62cd076..507f50c 100644
--- a/resources/Documentation/setup-guide.md
+++ b/resources/Documentation/setup-guide.md
@@ -349,6 +349,11 @@
submitting an invalid code owner config that may block the submission of all
changes (e.g. if it is not parseable).
+**NOTE** If the repository contains pre-existing code owner config files, it is
+recommended to validate them via the [Check code owners files REST
+endpoint](rest-api.html#check-code-owner-config-files) and fix the reported
+issues.
+
### <a id="faq">FAQ's
##### <a id="updateCodeOwnersConfig">How to update the code-owners.config file for a project
diff --git a/resources/Documentation/validation.md b/resources/Documentation/validation.md
index 4d3165d..9966f09 100644
--- a/resources/Documentation/validation.md
+++ b/resources/Documentation/validation.md
@@ -9,7 +9,9 @@
configuration of the `@PLUGIN@` plugin
To reduce the risk that these files become invalid, they are validated when
-they are modified and invalid modifications are rejected.
+they are modified and invalid modifications are rejected. In addition code owner
+config files in a repository can be validated on demand by the [Check code
+owners files REST endpoint](rest-api.html#check-code-owner-config-files).
**NOTE:** Most configuration issues are gracefully handled and do not break the
code owners functionality (e.g. non-resolveable code owners or non-resolveable
