PathCodeOwnersResult: Add flag that is being set if there are unresolved imports

Unresolved imports are silently ignored. However the fact that there are
unresolved imports is relevant for fallback code owners.

Fallback code owners should only apply if there are no code owners
defined for a path, but not if there are no code owners because an
import couldn't be resolved. In the latter case the intention was to
define code owners, and hence the fallback code owners should not apply.
Making use of the new flag in PathCodeOwnersResult for fallback code
owners will be implemented in a follow-up change.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iad879cbddb07daba5c3741e3b380b3b7951cb711

java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java

diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
index 9911150..078ba17 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
@@ -206,7 +206,8 @@
       getMatchingPerFileCodeOwnerSets(codeOwnerConfig)
           .forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
 
-      resolveImports(codeOwnerConfig, resolvedCodeOwnerConfigBuilder);
+      boolean hasUnresolvedImports =
+          !resolveImports(codeOwnerConfig, resolvedCodeOwnerConfigBuilder);
 
       CodeOwnerConfig resolvedCodeOwnerConfig = resolvedCodeOwnerConfigBuilder.build();
 
@@ -228,15 +229,24 @@
                 .build();
       }
 
-      this.pathCodeOwnersResult = PathCodeOwnersResult.create(path, resolvedCodeOwnerConfig);
+      this.pathCodeOwnersResult =
+          PathCodeOwnersResult.create(path, resolvedCodeOwnerConfig, hasUnresolvedImports);
       logger.atFine().log("path code owners result = %s", pathCodeOwnersResult);
       return this.pathCodeOwnersResult;
     }
   }
 
-  private void resolveImports(
+  /**
+   * Resolve the imports of the given code owner config.
+   *
+   * @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
+   */
+  private boolean resolveImports(
       CodeOwnerConfig importingCodeOwnerConfig,
       CodeOwnerConfig.Builder resolvedCodeOwnerConfigBuilder) {
+    boolean hasUnresolvedImports = false;
     try (TraceTimer traceTimer =
         TraceContext.newTimer(
             "Resolve code owner config imports",
@@ -280,6 +290,7 @@
           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",
@@ -289,6 +300,7 @@
             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",
@@ -310,6 +322,7 @@
                   : 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)",
@@ -377,6 +390,7 @@
         }
       }
     }
+    return !hasUnresolvedImports;
   }
 
   public static CodeOwnerConfig.Key createKeyForImportedCodeOwnerConfig(

java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java

diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
index fd15503..a463aef 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersResult.java
@@ -33,6 +33,9 @@
   /** Gets the resolved code owner config. */
   abstract CodeOwnerConfig codeOwnerConfig();
 
+  /** Whether there are unresolved imports. */
+  public abstract boolean hasUnresolvedImports();
+
   /**
    * Gets the code owners from the code owner config that apply to the path.
    *
@@ -65,11 +68,13 @@
     return MoreObjects.toStringHelper(this)
         .add("path", path())
         .add("codeOwnerConfig", codeOwnerConfig())
+        .add("hasUnresolvedImports", hasUnresolvedImports())
         .toString();
   }
 
   /** Creates a {@link CodeOwnerResolverResult} instance. */
-  public static PathCodeOwnersResult create(Path path, CodeOwnerConfig codeOwnerConfig) {
-    return new AutoValue_PathCodeOwnersResult(path, codeOwnerConfig);
+  public static PathCodeOwnersResult create(
+      Path path, CodeOwnerConfig codeOwnerConfig, boolean hasUnresolvedImports) {
+    return new AutoValue_PathCodeOwnersResult(path, codeOwnerConfig, hasUnresolvedImports);
   }
 }

javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java

diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
index 02a1ca8..0677ca4 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
@@ -206,6 +206,7 @@
     PathCodeOwners pathCodeOwners =
         pathCodeOwnersFactory.create(emptyCodeOwnerConfig, Paths.get("/foo/bar/baz.md"));
     assertThat(pathCodeOwners.resolveCodeOwnerConfig().getPathCodeOwners()).isEmpty();
+    assertThat(pathCodeOwners.resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -462,6 +463,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isTrue();
   }
 
   @Test
@@ -507,6 +509,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -552,6 +555,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -598,6 +602,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -645,6 +650,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -694,6 +700,7 @@
         .comparingElementsUsing(hasEmail())
         .containsExactly(user.email());
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().ignoreParentCodeOwners()).isTrue();
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -742,6 +749,7 @@
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().ignoreParentCodeOwners()).isFalse();
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -790,6 +798,7 @@
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().ignoreParentCodeOwners()).isFalse();
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -922,6 +931,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email(), user2.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -980,6 +990,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1150,6 +1161,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1179,6 +1191,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isTrue();
   }
 
   @Test
@@ -1223,6 +1236,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isTrue();
   }
 
   @Test
@@ -1253,6 +1267,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isTrue();
   }
 
   @Test
@@ -1293,6 +1308,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1340,6 +1356,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1383,6 +1400,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1428,6 +1446,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1462,6 +1481,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isTrue();
   }
 
   @Test
@@ -1517,6 +1537,7 @@
 
     // Expectation: the ignoreParentCodeOwners flag from the imported code owner config is ignored
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().ignoreParentCodeOwners()).isFalse();
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1573,6 +1594,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email(), user2.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test
@@ -1632,6 +1654,7 @@
     assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().getPathCodeOwners())
         .comparingElementsUsing(hasEmail())
         .containsExactly(admin.email(), user.email());
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().hasUnresolvedImports()).isFalse();
   }
 
   @Test