CheckCodeOwner: Inform in debug logs about imported code owner configs
So far the CheckCodeOwner REST endpoint only tells about unresolveable
imports, but not which code owner configs have been imported. Include
this info to make debugging easier.
This also fixes the name of the importing config in messages that inform
about unresolved imports (so far it always used the name of the start
code owner config, even if the unresolveable import appeared in an
imported code owner config).
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: If287594c0be6f66dc1e5496469234bd12bed66d6
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigReference.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigReference.java
index 2bc571c..7f9d4c4 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigReference.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfigReference.java
@@ -75,6 +75,19 @@
return filePath().getFileName().toString();
}
+ /** User-readable string representing this code owner config reference. */
+ public String format() {
+ StringBuilder formatted = new StringBuilder();
+ if (project().isPresent()) {
+ formatted.append(project().get()).append(":");
+ }
+ if (branch().isPresent()) {
+ formatted.append(branch().get()).append(":");
+ }
+ formatted.append(filePath());
+ return formatted.toString();
+ }
+
/**
* Creates a builder from this code owner config reference.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index 40864e3..4da2a52 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -62,6 +62,7 @@
private final AccountControl.Factory accountControlFactory;
private final PathCodeOwners.Factory pathCodeOwnersFactory;
private final CodeOwnerMetrics codeOwnerMetrics;
+ private final UnresolvedImportFormatter unresolvedImportFormatter;
// Enforce visibility by default.
private boolean enforceVisibility = true;
@@ -80,7 +81,8 @@
AccountCache accountCache,
AccountControl.Factory accountControlFactory,
PathCodeOwners.Factory pathCodeOwnersFactory,
- CodeOwnerMetrics codeOwnerMetrics) {
+ CodeOwnerMetrics codeOwnerMetrics,
+ UnresolvedImportFormatter unresolvedImportFormatter) {
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.permissionBackend = permissionBackend;
this.currentUser = currentUser;
@@ -89,6 +91,7 @@
this.accountControlFactory = accountControlFactory;
this.pathCodeOwnersFactory = pathCodeOwnersFactory;
this.codeOwnerMetrics = codeOwnerMetrics;
+ this.unresolvedImportFormatter = unresolvedImportFormatter;
}
/**
@@ -206,7 +209,7 @@
AtomicBoolean hasUnresolvedCodeOwners = new AtomicBoolean(false);
List<String> messages = new ArrayList<>(pathCodeOwnersMessages);
unresolvedImports.forEach(
- unresolvedImport -> messages.add(unresolvedImport.format(codeOwnersPluginConfiguration)));
+ unresolvedImport -> messages.add(unresolvedImportFormatter.format(unresolvedImport)));
ImmutableSet<CodeOwner> codeOwners =
codeOwnerReferences.stream()
.filter(
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
index 05843d1..3703b5e 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
@@ -19,6 +19,7 @@
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toSet;
+import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -220,7 +221,7 @@
resolvedCodeOwnerConfigBuilder.addCodeOwnerSet(codeOwnerSet);
});
- List<UnresolvedImport> unresolvedImports =
+ OptionalResultWithMessages<List<UnresolvedImport>> unresolvedImports =
resolveImports(codeOwnerConfig, resolvedCodeOwnerConfigBuilder);
CodeOwnerConfig resolvedCodeOwnerConfig = resolvedCodeOwnerConfigBuilder.build();
@@ -253,9 +254,10 @@
.build();
}
+ messages.addAll(unresolvedImports.messages());
this.pathCodeOwnersResult =
OptionalResultWithMessages.create(
- PathCodeOwnersResult.create(path, resolvedCodeOwnerConfig, unresolvedImports),
+ PathCodeOwnersResult.create(path, resolvedCodeOwnerConfig, unresolvedImports.get()),
messages);
logger.atFine().log("path code owners result = %s", pathCodeOwnersResult);
return this.pathCodeOwnersResult;
@@ -269,10 +271,11 @@
* @param resolvedCodeOwnerConfigBuilder the builder for the resolved code owner config
* @return list of unresolved imports, empty list if all imports were successfully resolved
*/
- private List<UnresolvedImport> resolveImports(
+ private OptionalResultWithMessages<List<UnresolvedImport>> resolveImports(
CodeOwnerConfig importingCodeOwnerConfig,
CodeOwnerConfig.Builder resolvedCodeOwnerConfigBuilder) {
ImmutableList.Builder<UnresolvedImport> unresolvedImports = ImmutableList.builder();
+ StringBuilder messageBuilder = new StringBuilder();
try (Timer0.Context ctx = codeOwnerMetrics.resolveCodeOwnerConfigImports.start()) {
logger.atFine().log("resolve imports of codeOwnerConfig %s", importingCodeOwnerConfig.key());
@@ -285,18 +288,28 @@
Map<BranchNameKey, ObjectId> revisionMap = new HashMap<>();
revisionMap.put(codeOwnerConfig.key().branchNameKey(), codeOwnerConfig.revision());
- Queue<CodeOwnerConfigReference> codeOwnerConfigsToImport = new ArrayDeque<>();
- codeOwnerConfigsToImport.addAll(importingCodeOwnerConfig.imports());
+ Queue<CodeOwnerConfigImport> codeOwnerConfigsToImport = new ArrayDeque<>();
+ codeOwnerConfigsToImport.addAll(getGlobalImports(0, importingCodeOwnerConfig));
codeOwnerConfigsToImport.addAll(
- resolvedCodeOwnerConfigBuilder.codeOwnerSets().stream()
- .flatMap(codeOwnerSet -> codeOwnerSet.imports().stream())
- .collect(toImmutableSet()));
+ getPerFileImports(
+ 0, codeOwnerConfig.key(), resolvedCodeOwnerConfigBuilder.codeOwnerSets()));
+ if (!codeOwnerConfigsToImport.isEmpty()) {
+ messageBuilder.append(
+ String.format(
+ "Code owner config %s imports:\n",
+ importingCodeOwnerConfig.key().format(codeOwners)));
+ }
while (!codeOwnerConfigsToImport.isEmpty()) {
- CodeOwnerConfigReference codeOwnerConfigReference = codeOwnerConfigsToImport.poll();
+ CodeOwnerConfigImport codeOwnerConfigImport = codeOwnerConfigsToImport.poll();
+ messageBuilder.append(codeOwnerConfigImport.format());
+
+ CodeOwnerConfigReference codeOwnerConfigReference =
+ codeOwnerConfigImport.referenceToImportedCodeOwnerConfig();
CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig =
createKeyForImportedCodeOwnerConfig(
importingCodeOwnerConfig.key(), codeOwnerConfigReference);
+
try (Timer0.Context ctx2 = codeOwnerMetrics.resolveCodeOwnerConfigImport.start()) {
logger.atFine().log(
"resolve import of code owner config %s", keyOfImportedCodeOwnerConfig);
@@ -306,22 +319,27 @@
if (!projectState.isPresent()) {
unresolvedImports.add(
UnresolvedImport.create(
- codeOwnerConfig.key(),
+ codeOwnerConfigImport.importingCodeOwnerConfig(),
keyOfImportedCodeOwnerConfig,
codeOwnerConfigReference,
String.format(
"project %s not found", keyOfImportedCodeOwnerConfig.project().get())));
+ messageBuilder.append(
+ codeOwnerConfigImport.formatSubItem("failed to resolve (project not found)\n"));
continue;
}
if (!projectState.get().statePermitsRead()) {
unresolvedImports.add(
UnresolvedImport.create(
- codeOwnerConfig.key(),
+ codeOwnerConfigImport.importingCodeOwnerConfig(),
keyOfImportedCodeOwnerConfig,
codeOwnerConfigReference,
String.format(
"state of project %s doesn't permit read",
keyOfImportedCodeOwnerConfig.project().get())));
+ messageBuilder.append(
+ codeOwnerConfigImport.formatSubItem(
+ "failed to resolve (project state doesn't allow read)\n"));
continue;
}
@@ -339,12 +357,15 @@
if (!mayBeImportedCodeOwnerConfig.isPresent()) {
unresolvedImports.add(
UnresolvedImport.create(
- codeOwnerConfig.key(),
+ codeOwnerConfigImport.importingCodeOwnerConfig(),
keyOfImportedCodeOwnerConfig,
codeOwnerConfigReference,
String.format(
"code owner config does not exist (revision = %s)",
revision.map(ObjectId::name).orElse("current"))));
+ messageBuilder.append(
+ codeOwnerConfigImport.formatSubItem(
+ "failed to resolve (code owner config not found)\n"));
continue;
}
@@ -371,18 +392,28 @@
getMatchingPerFileCodeOwnerSets(importedCodeOwnerConfig).collect(toImmutableSet());
if (importMode.importPerFileCodeOwnerSets()) {
logger.atFine().log("import per-file code owners");
- matchingPerFileCodeOwnerSets.forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
+ matchingPerFileCodeOwnerSets.forEach(
+ codeOwnerSet -> {
+ messageBuilder.append(
+ codeOwnerConfigImport.formatSubItem(
+ String.format(
+ "per-file code owner set with path expressions %s matches\n",
+ codeOwnerSet.pathExpressions())));
+ resolvedCodeOwnerConfigBuilder.addCodeOwnerSet(codeOwnerSet);
+ });
}
if (importMode.resolveImportsOfImport()
&& seenCodeOwnerConfigs.add(keyOfImportedCodeOwnerConfig)) {
logger.atFine().log("resolve imports of imported code owner config");
- Set<CodeOwnerConfigReference> transitiveImports = new HashSet<>();
- transitiveImports.addAll(importedCodeOwnerConfig.imports());
+ Set<CodeOwnerConfigImport> transitiveImports = new HashSet<>();
transitiveImports.addAll(
- matchingPerFileCodeOwnerSets.stream()
- .flatMap(codeOwnerSet -> codeOwnerSet.imports().stream())
- .collect(toImmutableSet()));
+ getGlobalImports(codeOwnerConfigImport.importLevel() + 1, importedCodeOwnerConfig));
+ transitiveImports.addAll(
+ getPerFileImports(
+ codeOwnerConfigImport.importLevel() + 1,
+ importedCodeOwnerConfig.key(),
+ matchingPerFileCodeOwnerSets));
if (importMode == CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY) {
// If only global code owners should be imported, transitive imports should also only
@@ -394,10 +425,14 @@
transitiveImports =
transitiveImports.stream()
.map(
- codeOwnerCfgRef ->
- CodeOwnerConfigReference.copyWithNewImportMode(
- codeOwnerCfgRef,
- CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY))
+ codeOwnerCfgImport ->
+ CodeOwnerConfigImport.create(
+ codeOwnerCfgImport.importLevel(),
+ codeOwnerCfgImport.importingCodeOwnerConfig(),
+ CodeOwnerConfigReference.copyWithNewImportMode(
+ codeOwnerCfgImport.referenceToImportedCodeOwnerConfig(),
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY),
+ codeOwnerCfgImport.codeOwnerSet()))
.collect(toSet());
}
@@ -407,7 +442,40 @@
}
}
}
- return unresolvedImports.build();
+ String message = messageBuilder.toString();
+ if (message.endsWith("\n")) {
+ message = message.substring(0, message.length() - 1);
+ }
+ return OptionalResultWithMessages.create(unresolvedImports.build(), ImmutableList.of(message));
+ }
+
+ private Set<CodeOwnerConfigImport> getGlobalImports(
+ int importLevel, CodeOwnerConfig codeOwnerConfig) {
+ return codeOwnerConfig.imports().stream()
+ .map(
+ codeOwnerConfigReference ->
+ CodeOwnerConfigImport.create(
+ importLevel, codeOwnerConfig.key(), codeOwnerConfigReference))
+ .collect(toImmutableSet());
+ }
+
+ private Set<CodeOwnerConfigImport> getPerFileImports(
+ int importLevel,
+ CodeOwnerConfig.Key importingCodeOwnerConfig,
+ Set<CodeOwnerSet> codeOwnerSets) {
+ Set<CodeOwnerConfigImport> codeOwnerConfigImports = new HashSet<>();
+ for (CodeOwnerSet codeOwnerSet : codeOwnerSets) {
+ codeOwnerSet.imports().stream()
+ .forEach(
+ codeOwnerConfigReference ->
+ codeOwnerConfigImports.add(
+ CodeOwnerConfigImport.create(
+ importLevel,
+ importingCodeOwnerConfig,
+ codeOwnerConfigReference,
+ codeOwnerSet)));
+ }
+ return codeOwnerConfigImports;
}
public static CodeOwnerConfig.Key createKeyForImportedCodeOwnerConfig(
@@ -482,4 +550,95 @@
return codeOwnerSet.pathExpressions().stream()
.anyMatch(pathExpression -> matcher.matches(pathExpression, relativePath));
}
+
+ @AutoValue
+ abstract static class CodeOwnerConfigImport {
+ /**
+ * The import level.
+ *
+ * <p>{@code 0} for direct import, {@code 1} if imported by a directly imported file, {@code 2},
+ * if imported by a file that was imported by an directly imported file, etc.
+ */
+ public abstract int importLevel();
+
+ /** The key of the code owner config that contains the import. */
+ public abstract CodeOwnerConfig.Key importingCodeOwnerConfig();
+
+ /** The reference to the imported code owner config */
+ public abstract CodeOwnerConfigReference referenceToImportedCodeOwnerConfig();
+
+ /** The code owner set that specified the import, empty if it is a global import. */
+ public abstract Optional<CodeOwnerSet> codeOwnerSet();
+
+ public String format() {
+ if (codeOwnerSet().isPresent()) {
+ return getPrefix()
+ + String.format(
+ "* %s (per-file import, import mode = %s, path expressions = %s)\n",
+ referenceToImportedCodeOwnerConfig().format(),
+ referenceToImportedCodeOwnerConfig().importMode(),
+ codeOwnerSet().get().pathExpressions());
+ }
+ return getPrefix()
+ + String.format(
+ "* %s (global import, import mode = %s)\n",
+ referenceToImportedCodeOwnerConfig().format(),
+ referenceToImportedCodeOwnerConfig().importMode());
+ }
+
+ public String formatSubItem(String message) {
+ return getPrefixForSubItem() + message;
+ }
+
+ private String getPrefix() {
+ return getPrefix(importLevel());
+ }
+
+ private String getPrefixForSubItem() {
+ return getPrefix(importLevel() + 1) + "* ";
+ }
+
+ private String getPrefix(int levels) {
+ // 2 spaces per level
+ //
+ // String.format("%<num>s", "") creates a string with <num> spaces:
+ // * '%' introduces a format sequence
+ // * <num> means that the resulting string should be <num> characters long
+ // * 's' is the character string format code, and ends the format sequence
+ // * the second parameter for String.format, is the string that should be
+ // prefixed with as many spaces as are needed to make the string <num>
+ // characters long
+ // * <num> must be > 0, hence we special case the handling of levels == 0
+ return levels > 0 ? String.format("%" + (levels * 2) + "s", "") : "";
+ }
+
+ public static CodeOwnerConfigImport create(
+ int importLevel,
+ CodeOwnerConfig.Key importingCodeOwnerConfig,
+ CodeOwnerConfigReference codeOwnerConfigReference) {
+ return create(
+ importLevel, importingCodeOwnerConfig, codeOwnerConfigReference, Optional.empty());
+ }
+
+ public static CodeOwnerConfigImport create(
+ int importLevel,
+ CodeOwnerConfig.Key importingCodeOwnerConfig,
+ CodeOwnerConfigReference codeOwnerConfigReference,
+ CodeOwnerSet codeOwnerSet) {
+ return create(
+ importLevel,
+ importingCodeOwnerConfig,
+ codeOwnerConfigReference,
+ Optional.of(codeOwnerSet));
+ }
+
+ public static CodeOwnerConfigImport create(
+ int importLevel,
+ CodeOwnerConfig.Key importingCodeOwnerConfig,
+ CodeOwnerConfigReference codeOwnerConfigReference,
+ Optional<CodeOwnerSet> codeOwnerSet) {
+ return new AutoValue_PathCodeOwners_CodeOwnerConfigImport(
+ importLevel, importingCodeOwnerConfig, codeOwnerConfigReference, codeOwnerSet);
+ }
+ }
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImport.java b/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImport.java
index 865c469..bea1e08 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImport.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImport.java
@@ -16,7 +16,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
/** Information about an unresolved import. */
@AutoValue
@@ -33,23 +32,6 @@
/** Message explaining why the code owner config reference couldn't be resolved. */
public abstract String message();
- /** Returns a user-readable string representation of this unresolved import. */
- public String format(CodeOwnersPluginConfiguration codeOwnersPluginConfiguration) {
- return String.format(
- "The import of %s:%s:%s in %s:%s:%s cannot be resolved: %s",
- keyOfImportedCodeOwnerConfig().project(),
- keyOfImportedCodeOwnerConfig().shortBranchName(),
- codeOwnersPluginConfiguration
- .getBackend(keyOfImportedCodeOwnerConfig().branchNameKey())
- .getFilePath(keyOfImportedCodeOwnerConfig()),
- keyOfImportingCodeOwnerConfig().project(),
- keyOfImportingCodeOwnerConfig().shortBranchName(),
- codeOwnersPluginConfiguration
- .getBackend(keyOfImportingCodeOwnerConfig().branchNameKey())
- .getFilePath(keyOfImportingCodeOwnerConfig()),
- message());
- }
-
@Override
public final String toString() {
return MoreObjects.toStringHelper(this)
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImportFormatter.java b/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImportFormatter.java
new file mode 100644
index 0000000..0029d9e
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImportFormatter.java
@@ -0,0 +1,68 @@
+// Copyright (C) 2021 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 com.google.gerrit.plugins.codeowners.backend.config.BackendConfig;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.inject.Inject;
+import java.nio.file.Path;
+
+/** Class to format an {@link UnresolvedImport} as a user-readable string. */
+public class UnresolvedImportFormatter {
+ private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
+ private final ProjectCache projectCache;
+ private final BackendConfig backendConfig;
+
+ @Inject
+ UnresolvedImportFormatter(
+ CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
+ ProjectCache projectCache,
+ BackendConfig backendConfig) {
+ this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
+ this.projectCache = projectCache;
+ this.backendConfig = backendConfig;
+ }
+
+ /** Returns a user-readable string representation of the given unresolved import. */
+ public String format(UnresolvedImport unresolvedImport) {
+ return String.format(
+ "The import of %s:%s:%s in %s:%s:%s cannot be resolved: %s",
+ unresolvedImport.keyOfImportedCodeOwnerConfig().project(),
+ unresolvedImport.keyOfImportedCodeOwnerConfig().shortBranchName(),
+ getFilePath(unresolvedImport.keyOfImportedCodeOwnerConfig()),
+ unresolvedImport.keyOfImportingCodeOwnerConfig().project(),
+ unresolvedImport.keyOfImportingCodeOwnerConfig().shortBranchName(),
+ getFilePath(unresolvedImport.keyOfImportingCodeOwnerConfig()),
+ unresolvedImport.message());
+ }
+
+ private Path getFilePath(CodeOwnerConfig.Key codeOwnerConfigKey) {
+ return getBackend(codeOwnerConfigKey).getFilePath(codeOwnerConfigKey);
+ }
+
+ /**
+ * Returns the code owner backend for the given code owner config key.
+ *
+ * <p>If the project of the code owner config key doesn't exist, the default code owner backend is
+ * returned.
+ */
+ private CodeOwnerBackend getBackend(CodeOwnerConfig.Key codeOwnerConfigKey) {
+ if (projectCache.get(codeOwnerConfigKey.project()).isPresent()) {
+ return codeOwnersPluginConfiguration.getBackend(codeOwnerConfigKey.branchNameKey());
+ }
+ return backendConfig.getDefaultBackend();
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
index de2e10f..f9a0655 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CheckCodeOwner.java
@@ -34,6 +34,7 @@
import com.google.gerrit.plugins.codeowners.backend.OptionalResultWithMessages;
import com.google.gerrit.plugins.codeowners.backend.PathCodeOwners;
import com.google.gerrit.plugins.codeowners.backend.PathCodeOwnersResult;
+import com.google.gerrit.plugins.codeowners.backend.UnresolvedImportFormatter;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.gerrit.server.IdentifiedUser;
@@ -68,6 +69,7 @@
private final Provider<CodeOwnerResolver> codeOwnerResolverProvider;
private final CodeOwners codeOwners;
private final AccountsCollection accountsCollection;
+ private final UnresolvedImportFormatter unresolvedImportFormatter;
private String email;
private String path;
@@ -83,7 +85,8 @@
PathCodeOwners.Factory pathCodeOwnersFactory,
Provider<CodeOwnerResolver> codeOwnerResolverProvider,
CodeOwners codeOwners,
- AccountsCollection accountsCollection) {
+ AccountsCollection accountsCollection,
+ UnresolvedImportFormatter unresolvedImportFormatter) {
this.checkCodeOwnerCapability = checkCodeOwnerCapability;
this.permissionBackend = permissionBackend;
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
@@ -92,6 +95,7 @@
this.codeOwnerResolverProvider = codeOwnerResolverProvider;
this.codeOwners = codeOwners;
this.accountsCollection = accountsCollection;
+ this.unresolvedImportFormatter = unresolvedImportFormatter;
}
@Option(name = "--email", usage = "email for which the code ownership should be checked")
@@ -142,7 +146,7 @@
.unresolvedImports()
.forEach(
unresolvedImport ->
- messages.add(unresolvedImport.format(codeOwnersPluginConfiguration)));
+ messages.add(unresolvedImportFormatter.format(unresolvedImport)));
Optional<CodeOwnerReference> codeOwnerReference =
pathCodeOwnersResult.get().getPathCodeOwners().stream()
.filter(cor -> cor.email().equals(email))
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
index 4964186..b3e26d8 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CheckCodeOwnerIT.java
@@ -27,6 +27,9 @@
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.projects.ConfigInput;
+import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -617,22 +620,129 @@
public void debugLogsContainUnresolvedImports() throws Exception {
skipTestIfImportsNotSupportedByCodeOwnersBackend();
- CodeOwnerConfigReference unresolvableCodeOwnerConfigReference =
+ CodeOwnerConfigReference unresolvableCodeOwnerConfigReferenceCodeOwnerConfigNotFound =
CodeOwnerConfigReference.create(
CodeOwnerConfigImportMode.ALL, "non-existing/" + getCodeOwnerConfigFileName());
+
+ CodeOwnerConfigReference unresolvableCodeOwnerConfigReferenceProjectNotFound =
+ CodeOwnerConfigReference.builder(
+ CodeOwnerConfigImportMode.ALL, getCodeOwnerConfigFileName())
+ .setProject(Project.nameKey("non-existing"))
+ .build();
+
+ Project.NameKey nonReadableProject =
+ projectOperations.newProject().name("non-readable").create();
+ ConfigInput configInput = new ConfigInput();
+ configInput.state = ProjectState.HIDDEN;
+ gApi.projects().name(nonReadableProject.get()).config(configInput);
+ CodeOwnerConfigReference unresolvableCodeOwnerConfigReferenceProjectNotReadable =
+ CodeOwnerConfigReference.builder(
+ CodeOwnerConfigImportMode.ALL, getCodeOwnerConfigFileName())
+ .setProject(nonReadableProject)
+ .build();
+
CodeOwnerConfig.Key codeOwnerConfigKey =
codeOwnerConfigOperations
.newCodeOwnerConfig()
.project(project)
.branch("master")
.folderPath(ROOT_PATH)
- .addImport(unresolvableCodeOwnerConfigReference)
+ .addImport(unresolvableCodeOwnerConfigReferenceCodeOwnerConfigNotFound)
+ .addImport(unresolvableCodeOwnerConfigReferenceProjectNotFound)
+ .addImport(unresolvableCodeOwnerConfigReferenceProjectNotReadable)
.create();
CodeOwnerCheckInfo checkCodeOwnerInfo = checkCodeOwner(ROOT_PATH, user.email());
assertThat(checkCodeOwnerInfo)
- .hasDebugLogsThat()
- .contains(
+ .hasDebugLogsThatContainAllOf(
+ String.format(
+ "Code owner config %s:%s:%s imports:\n"
+ + "* %s (global import, import mode = ALL)\n"
+ + " * failed to resolve (code owner config not found)\n"
+ + "* %s:%s (global import, import mode = ALL)\n"
+ + " * failed to resolve (project not found)\n"
+ + "* %s:%s (global import, import mode = ALL)\n"
+ + " * failed to resolve (project state doesn't allow read)",
+ project,
+ "master",
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey.folderPath().toString()),
+ unresolvableCodeOwnerConfigReferenceCodeOwnerConfigNotFound.filePath(),
+ unresolvableCodeOwnerConfigReferenceProjectNotFound.project().get(),
+ unresolvableCodeOwnerConfigReferenceProjectNotFound.filePath(),
+ unresolvableCodeOwnerConfigReferenceProjectNotReadable.project().get(),
+ unresolvableCodeOwnerConfigReferenceProjectNotReadable.filePath()),
+ String.format(
+ "The import of %s:%s:%s in %s:%s:%s cannot be resolved:"
+ + " code owner config does not exist (revision = %s)",
+ project,
+ "master",
+ JgitPath.of(unresolvableCodeOwnerConfigReferenceCodeOwnerConfigNotFound.filePath())
+ .getAsAbsolutePath(),
+ project,
+ "master",
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey.folderPath().toString()),
+ projectOperations.project(project).getHead("master").name()),
+ String.format(
+ "The import of %s:%s:%s in %s:%s:%s cannot be resolved: project %s not found",
+ unresolvableCodeOwnerConfigReferenceProjectNotFound.project().get(),
+ "master",
+ JgitPath.of(unresolvableCodeOwnerConfigReferenceProjectNotFound.filePath())
+ .getAsAbsolutePath(),
+ project,
+ "master",
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey.folderPath().toString()),
+ unresolvableCodeOwnerConfigReferenceProjectNotFound.project().get()),
+ String.format(
+ "The import of %s:%s:%s in %s:%s:%s cannot be resolved:"
+ + " state of project %s doesn't permit read",
+ unresolvableCodeOwnerConfigReferenceProjectNotReadable.project().get(),
+ "master",
+ JgitPath.of(unresolvableCodeOwnerConfigReferenceProjectNotReadable.filePath())
+ .getAsAbsolutePath(),
+ project,
+ "master",
+ getCodeOwnerConfigFilePath(codeOwnerConfigKey.folderPath().toString()),
+ unresolvableCodeOwnerConfigReferenceProjectNotReadable.project().get()));
+ }
+
+ @Test
+ public void debugLogsContainUnresolvedTransitiveImports() throws Exception {
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath(ROOT_PATH)
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "/foo/" + getCodeOwnerConfigFileName()))
+ .create();
+
+ CodeOwnerConfigReference unresolvableCodeOwnerConfigReference =
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "non-existing/" + getCodeOwnerConfigFileName());
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addImport(unresolvableCodeOwnerConfigReference)
+ .create();
+
+ CodeOwnerCheckInfo checkCodeOwnerInfo = checkCodeOwner(ROOT_PATH, user.email());
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatContainAllOf(
+ String.format(
+ "Code owner config %s:%s:%s imports:\n"
+ + "* %s (global import, import mode = ALL)\n"
+ + " * %s (global import, import mode = ALL)\n"
+ + " * failed to resolve (code owner config not found)",
+ project,
+ "master",
+ getCodeOwnerConfigFilePath("/"),
+ getCodeOwnerConfigFilePath("/foo/"),
+ unresolvableCodeOwnerConfigReference.filePath()),
String.format(
"The import of %s:%s:%s in %s:%s:%s cannot be resolved:"
+ " code owner config does not exist (revision = %s)",
@@ -641,7 +751,7 @@
JgitPath.of(unresolvableCodeOwnerConfigReference.filePath()).getAsAbsolutePath(),
project,
"master",
- getCodeOwnerConfigFilePath(codeOwnerConfigKey.folderPath().toString()),
+ getCodeOwnerConfigFilePath("/foo/"),
projectOperations.project(project).getHead("master").name()));
}
@@ -690,8 +800,7 @@
assertThat(checkCodeOwnerInfo)
.hasDebugLogsThatDoNotContainAnyOf(
String.format(
- "per-file code owner set with path expressions [%s] matches",
- testPathExpressions.matchFileType("txt")));
+ "path expressions [%s] matches", testPathExpressions.matchFileType("txt")));
}
@Test
@@ -761,6 +870,164 @@
String.format("resolved to account %s", fileCodeOwner.id()));
}
+ @Test
+ public void checkCodeOwnerFromImportedConfig() throws Exception {
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
+
+ TestAccount codeOwner =
+ accountCreator.create(
+ "codeOwner", "codeOwner@example.com", "Code Owner", /* displayName= */ null);
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "/bar/" + getCodeOwnerConfigFileName()))
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "/baz/" + getCodeOwnerConfigFileName()))
+ .create();
+
+ setAsCodeOwners("/baz/", codeOwner);
+
+ String path = "/foo/bar/baz.md";
+ CodeOwnerCheckInfo checkCodeOwnerInfo = checkCodeOwner(path, codeOwner.email());
+ assertThat(checkCodeOwnerInfo).isCodeOwner();
+ assertThat(checkCodeOwnerInfo).isResolvable();
+ assertThat(checkCodeOwnerInfo)
+ .hasCodeOwnerConfigFilePathsThat()
+ .containsExactly(getCodeOwnerConfigFilePath("/foo/"));
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatContainAllOf(
+ String.format(
+ "Code owner config %s:%s:/foo/%s imports:\n"
+ + "* /bar/%s (global import, import mode = ALL)\n"
+ + " * /baz/%s (global import, import mode = ALL)",
+ project,
+ "master",
+ getCodeOwnerConfigFileName(),
+ getCodeOwnerConfigFileName(),
+ getCodeOwnerConfigFileName()),
+ String.format(
+ "found email %s as code owner in %s",
+ codeOwner.email(), getCodeOwnerConfigFilePath("/foo/")),
+ String.format("resolved to account %s", codeOwner.id()));
+ }
+
+ @Test
+ public void checkCodeOwnerFromImportedPerFileConfig() throws Exception {
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
+
+ TestAccount mdCodeOwner =
+ accountCreator.create(
+ "mdCodeOwner", "mdCodeOwner@example.com", "Md Code Owner", /* displayName= */ null);
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "/bar/" + getCodeOwnerConfigFileName()))
+ .create();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression(testPathExpressions.matchFileType("md"))
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY,
+ "/baz/" + getCodeOwnerConfigFileName()))
+ .build())
+ .create();
+
+ setAsCodeOwners("/baz/", mdCodeOwner);
+
+ // 1. check for mdCodeOwner and path of an md file
+ String path = "/foo/bar/baz.md";
+ CodeOwnerCheckInfo checkCodeOwnerInfo = checkCodeOwner(path, mdCodeOwner.email());
+ assertThat(checkCodeOwnerInfo).isCodeOwner();
+ assertThat(checkCodeOwnerInfo).isResolvable();
+ assertThat(checkCodeOwnerInfo)
+ .hasCodeOwnerConfigFilePathsThat()
+ .containsExactly(getCodeOwnerConfigFilePath("/foo/"));
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatContainAllOf(
+ String.format(
+ "Code owner config %s:%s:/foo/%s imports:\n"
+ + "* /bar/%s (global import, import mode = ALL)\n"
+ + " * per-file code owner set with path expressions [%s] matches\n"
+ + " * /baz/%s (per-file import, import mode = GLOBAL_CODE_OWNER_SETS_ONLY,"
+ + " path expressions = [%s])",
+ project,
+ "master",
+ getCodeOwnerConfigFileName(),
+ getCodeOwnerConfigFileName(),
+ testPathExpressions.matchFileType("md"),
+ getCodeOwnerConfigFileName(),
+ testPathExpressions.matchFileType("md")),
+ String.format(
+ "found email %s as code owner in %s",
+ mdCodeOwner.email(), getCodeOwnerConfigFilePath("/foo/")),
+ String.format("resolved to account %s", mdCodeOwner.id()));
+
+ // 2. check for user and path of an md file
+ checkCodeOwnerInfo = checkCodeOwner(path, user.email());
+ assertThat(checkCodeOwnerInfo).isNotCodeOwner();
+ assertThat(checkCodeOwnerInfo).isResolvable();
+ assertThat(checkCodeOwnerInfo).hasCodeOwnerConfigFilePathsThat().isEmpty();
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatContainAllOf(
+ String.format(
+ "Code owner config %s:%s:/foo/%s imports:\n"
+ + "* /bar/%s (global import, import mode = ALL)\n"
+ + " * per-file code owner set with path expressions [%s] matches\n"
+ + " * /baz/%s (per-file import, import mode = GLOBAL_CODE_OWNER_SETS_ONLY,"
+ + " path expressions = [%s])",
+ project,
+ "master",
+ getCodeOwnerConfigFileName(),
+ getCodeOwnerConfigFileName(),
+ testPathExpressions.matchFileType("md"),
+ getCodeOwnerConfigFileName(),
+ testPathExpressions.matchFileType("md")),
+ String.format("resolved to account %s", user.id()));
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatDoNotContainAnyOf(String.format("email %s", user.email()));
+
+ // 3. check for mdCodeOwner and path of an txt file
+ path = "/foo/bar/baz.txt";
+ checkCodeOwnerInfo = checkCodeOwner(path, mdCodeOwner.email());
+ assertThat(checkCodeOwnerInfo).isNotCodeOwner();
+ assertThat(checkCodeOwnerInfo).isResolvable();
+ assertThat(checkCodeOwnerInfo).hasCodeOwnerConfigFilePathsThat().isEmpty();
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatContainAllOf(
+ String.format(
+ "Code owner config %s:%s:/foo/%s imports:\n"
+ + "* /bar/%s (global import, import mode = ALL)",
+ project, "master", getCodeOwnerConfigFileName(), getCodeOwnerConfigFileName()),
+ String.format("resolved to account %s", mdCodeOwner.id()));
+ assertThat(checkCodeOwnerInfo)
+ .hasDebugLogsThatDoNotContainAnyOf(String.format("email %s", mdCodeOwner.email()));
+ }
+
private CodeOwnerCheckInfo checkCodeOwner(String path, String email) throws RestApiException {
return checkCodeOwner(path, email, null);
}
