PathCodeOwnersResult: Include details about unresolved imports
This will allow us to provide these details to host admins when they
need to investigate why certain code owner config files do not work as
expected. Requires further changes to expose this information to the
host admins.
The warnings about the unresolved imports do no longer need to be logged
right when they are detected, as the information about them is now
included into PathCodeOwnersResult which is fully logged, including the
information about the unresolved imports.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ie11efb52d2af41c66dabe25f67bfb9b9ff34479a
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
index 078ba17..ae14883 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
@@ -20,6 +20,7 @@
import static java.util.stream.Collectors.toSet;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
@@ -35,6 +36,7 @@
import java.util.ArrayDeque;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Queue;
@@ -206,8 +208,8 @@
getMatchingPerFileCodeOwnerSets(codeOwnerConfig)
.forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
- boolean hasUnresolvedImports =
- !resolveImports(codeOwnerConfig, resolvedCodeOwnerConfigBuilder);
+ List<UnresolvedImport> unresolvedImports =
+ resolveImports(codeOwnerConfig, resolvedCodeOwnerConfigBuilder);
CodeOwnerConfig resolvedCodeOwnerConfig = resolvedCodeOwnerConfigBuilder.build();
@@ -230,7 +232,7 @@
}
this.pathCodeOwnersResult =
- PathCodeOwnersResult.create(path, resolvedCodeOwnerConfig, hasUnresolvedImports);
+ PathCodeOwnersResult.create(path, resolvedCodeOwnerConfig, unresolvedImports);
logger.atFine().log("path code owners result = %s", pathCodeOwnersResult);
return this.pathCodeOwnersResult;
}
@@ -241,12 +243,12 @@
*
* @param importingCodeOwnerConfig the code owner config for which imports should be resolved
* @param resolvedCodeOwnerConfigBuilder the builder for the resolved code owner config
- * @return whether all imports have been resolved successfully
+ * @return list of unresolved imports, empty list if all imports were successfully resolved
*/
- private boolean resolveImports(
+ private List<UnresolvedImport> resolveImports(
CodeOwnerConfig importingCodeOwnerConfig,
CodeOwnerConfig.Builder resolvedCodeOwnerConfigBuilder) {
- boolean hasUnresolvedImports = false;
+ ImmutableList.Builder<UnresolvedImport> unresolvedImports = ImmutableList.builder();
try (TraceTimer traceTimer =
TraceContext.newTimer(
"Resolve code owner config imports",
@@ -290,23 +292,24 @@
Optional<ProjectState> projectState =
projectCache.get(keyOfImportedCodeOwnerConfig.project());
if (!projectState.isPresent()) {
- hasUnresolvedImports = true;
- logger.atWarning().log(
- "cannot resolve code owner config %s that is imported by code owner config %s:"
- + " project %s not found",
- keyOfImportedCodeOwnerConfig,
- importingCodeOwnerConfig.key(),
- keyOfImportedCodeOwnerConfig.project().get());
+ unresolvedImports.add(
+ UnresolvedImport.create(
+ codeOwnerConfig.key(),
+ keyOfImportedCodeOwnerConfig,
+ codeOwnerConfigReference,
+ String.format(
+ "project %s not found", keyOfImportedCodeOwnerConfig.project().get())));
continue;
}
if (!projectState.get().statePermitsRead()) {
- hasUnresolvedImports = true;
- logger.atWarning().log(
- "cannot resolve code owner config %s that is imported by code owner config %s:"
- + " state of project %s doesn't permit read",
- keyOfImportedCodeOwnerConfig,
- importingCodeOwnerConfig.key(),
- keyOfImportedCodeOwnerConfig.project().get());
+ unresolvedImports.add(
+ UnresolvedImport.create(
+ codeOwnerConfig.key(),
+ keyOfImportedCodeOwnerConfig,
+ codeOwnerConfigReference,
+ String.format(
+ "state of project %s doesn't permit read",
+ keyOfImportedCodeOwnerConfig.project().get())));
continue;
}
@@ -322,13 +325,14 @@
: codeOwners.getFromCurrentRevision(keyOfImportedCodeOwnerConfig);
if (!mayBeImportedCodeOwnerConfig.isPresent()) {
- hasUnresolvedImports = true;
- logger.atWarning().log(
- "cannot resolve code owner config %s that is imported by code owner config %s"
- + " (revision = %s)",
- keyOfImportedCodeOwnerConfig,
- importingCodeOwnerConfig.key(),
- revision.map(ObjectId::name).orElse("current"));
+ unresolvedImports.add(
+ UnresolvedImport.create(
+ codeOwnerConfig.key(),
+ keyOfImportedCodeOwnerConfig,
+ codeOwnerConfigReference,
+ String.format(
+ "code owner config does not exist (revision = %s)",
+ revision.map(ObjectId::name).orElse("current"))));
continue;
}
@@ -390,7 +394,7 @@
}
}
}
- return !hasUnresolvedImports;
+ return unresolvedImports.build();
}
public static CodeOwnerConfig.Key createKeyForImportedCodeOwnerConfig(
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
index e347c30..934d730 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
@@ -18,9 +18,11 @@
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import java.nio.file.Path;
+import java.util.List;
/** The result of resolving path code owners via {@link PathCodeOwners}. */
@AutoValue
@@ -33,8 +35,13 @@
/** Gets the resolved code owner config. */
abstract CodeOwnerConfig codeOwnerConfig();
+ /** Gets a list of unresolved imports. */
+ public abstract ImmutableList<UnresolvedImport> unresolvedImports();
+
/** Whether there are unresolved imports. */
- public abstract boolean hasUnresolvedImports();
+ public boolean hasUnresolvedImports() {
+ return !unresolvedImports().isEmpty();
+ }
/**
* Gets the code owners from the code owner config that apply to the path.
@@ -68,13 +75,14 @@
return MoreObjects.toStringHelper(this)
.add("path", path())
.add("codeOwnerConfig", codeOwnerConfig())
- .add("hasUnresolvedImports", hasUnresolvedImports())
+ .add("unresolvedImports", unresolvedImports())
.toString();
}
/** Creates a {@link PathCodeOwnersResult} instance. */
public static PathCodeOwnersResult create(
- Path path, CodeOwnerConfig codeOwnerConfig, boolean hasUnresolvedImports) {
- return new AutoValue_PathCodeOwnersResult(path, codeOwnerConfig, hasUnresolvedImports);
+ Path path, CodeOwnerConfig codeOwnerConfig, List<UnresolvedImport> unresolvedImports) {
+ return new AutoValue_PathCodeOwnersResult(
+ path, codeOwnerConfig, ImmutableList.copyOf(unresolvedImports));
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImport.java b/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImport.java
new file mode 100644
index 0000000..f2491b7
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/backend/UnresolvedImport.java
@@ -0,0 +1,75 @@
+// 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 com.google.auto.value.AutoValue;
+import com.google.common.base.MoreObjects;
+import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
+
+/** Information about an unresolved import. */
+@AutoValue
+public abstract class UnresolvedImport {
+ /** Key of the importing code owner config. */
+ public abstract CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig();
+
+ /** Key of the imported code owner config. */
+ public abstract CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig();
+
+ /** The code owner config reference that was attempted to be resolved. */
+ public abstract CodeOwnerConfigReference codeOwnerConfigReference();
+
+ /** 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 String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("keyOfImportingCodeOwnerConfig", keyOfImportingCodeOwnerConfig())
+ .add("keyOfImportedCodeOwnerConfig", keyOfImportedCodeOwnerConfig())
+ .add("codeOwnerConfigReference", codeOwnerConfigReference())
+ .add("message", message())
+ .toString();
+ }
+
+ /** Creates a {@link UnresolvedImport} instance. */
+ static UnresolvedImport create(
+ CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig,
+ CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig,
+ CodeOwnerConfigReference codeOwnerConfigReference,
+ String message) {
+ return new AutoValue_UnresolvedImport(
+ keyOfImportingCodeOwnerConfig,
+ keyOfImportedCodeOwnerConfig,
+ codeOwnerConfigReference,
+ message);
+ }
+}
