Fix wrongly returned import when per file owner is transitively imported

If an OWNERS file A has a global import of an OWNERS file B that has a
per-file import, then the per-file import was returned twice, once as
expected for OWNERS file B and once wrongly for OWNERS file A.

When resolving imports for per-file code owner sets we must only look at
the per-file code owner sets from the OWNERS file that is being
resolving and not at per-file code owner sets which have already been
imported via global imports.

Change-Id: I0ec04d121f4b1f81ca75e8f52c5e15c3a75f32b1
Signed-off-by: Edwin Kempin <ekempin@google.com>
Reviewed-on: https://gerrit-review.googlesource.com/c/plugins/code-owners/+/434757
Tested-by: Zuul <zuul-63@gerritcodereview-ci.iam.gserviceaccount.com>
Reviewed-by: Kamil Musin <kamilm@google.com>

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 40eaa21..05af4e3 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
@@ -248,8 +248,9 @@
       getGlobalCodeOwnerSets(codeOwnerConfig)
           .forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
       boolean globalCodeOwnersIgnored = false;
-      for (CodeOwnerSet codeOwnerSet :
-          getMatchingPerFileCodeOwnerSets(codeOwnerConfig).collect(toImmutableSet())) {
+      ImmutableSet<CodeOwnerSet> matchingPerFileCodeOwnerSets =
+          getMatchingPerFileCodeOwnerSets(codeOwnerConfig).collect(toImmutableSet());
+      for (CodeOwnerSet codeOwnerSet : matchingPerFileCodeOwnerSets) {
         messages.add(
             String.format(
                 "per-file code owner set with path expressions %s matches",
@@ -329,7 +330,7 @@
 
       // Resolve per-file imports.
       ImmutableSet<CodeOwnerImport> perFileImports =
-          getPerFileImports(0, codeOwnerConfig, resolvedCodeOwnerConfigBuilder.codeOwnerSets());
+          getPerFileImports(0, codeOwnerConfig, matchingPerFileCodeOwnerSets);
       OptionalResultWithMessages<CodeOwnerConfigImports> perFileImportedCodeOwnerConfigs =
           resolveImports(codeOwnerConfig.key(), perFileImports, resolvedCodeOwnerConfigBuilder);
       messages.addAll(perFileImportedCodeOwnerConfigs.messages());

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 064d8f6..7f9492f 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
@@ -2836,6 +2836,83 @@
     assertThat(pathCodeOwnersResult.unresolvedImports()).isEmpty();
   }
 
+  @Test
+  public void transitiveImportsOfPerFileCodeOwners() throws Exception {
+    TestAccount mdCodeOwner =
+        accountCreator.create(
+            "mdCodeOwner", "mdCodeOwner@example.com", "Md Code Owner", /* displayName= */ null);
+
+    CodeOwnerConfigReference barCodeOwnerConfigReference =
+        CodeOwnerConfigReference.create(
+            CodeOwnerConfigImportMode.ALL, "/bar/" + getCodeOwnerConfigFileName());
+    CodeOwnerConfig.Key fooCodeOwnerConfigKey =
+        codeOwnerConfigOperations
+            .newCodeOwnerConfig()
+            .project(project)
+            .branch("master")
+            .folderPath("/foo/")
+            .fileName("OWNERS")
+            .addImport(barCodeOwnerConfigReference)
+            .create();
+
+    CodeOwnerConfigReference bazCodeOwnerConfigReference =
+        CodeOwnerConfigReference.create(
+            CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY,
+            "/baz/" + getCodeOwnerConfigFileName());
+    CodeOwnerConfig.Key barCodeOwnerConfigKey =
+        codeOwnerConfigOperations
+            .newCodeOwnerConfig()
+            .project(project)
+            .branch("master")
+            .fileName("OWNERS")
+            .folderPath("/bar/")
+            .addCodeOwnerSet(
+                CodeOwnerSet.builder()
+                    .addPathExpression(testPathExpressions.matchFileType("md"))
+                    .addImport(bazCodeOwnerConfigReference)
+                    .build())
+            .create();
+
+    CodeOwnerConfig.Key bazCodeOwnerConfigKey =
+        codeOwnerConfigOperations
+            .newCodeOwnerConfig()
+            .project(project)
+            .branch("master")
+            .folderPath("/baz/")
+            .fileName("OWNERS")
+            .addCodeOwnerEmail(mdCodeOwner.email())
+            .create();
+
+    CodeOwnerConfig fooCodeOwnerConfig =
+        codeOwnerConfigOperations.codeOwnerConfig(fooCodeOwnerConfigKey).get();
+    CodeOwnerConfig barCodeOwnerConfig =
+        codeOwnerConfigOperations.codeOwnerConfig(barCodeOwnerConfigKey).get();
+    CodeOwnerConfig bazCodeOwnerConfig =
+        codeOwnerConfigOperations.codeOwnerConfig(bazCodeOwnerConfigKey).get();
+
+    Optional<PathCodeOwners> pathCodeOwners =
+        pathCodeOwnersFactory.create(
+            transientCodeOwnerConfigCacheProvider.get(),
+            fooCodeOwnerConfigKey,
+            projectOperations.project(project).getHead("master"),
+            Path.of("/foo/bar/baz.md"));
+    assertThat(pathCodeOwners).isPresent();
+
+    // Expectation: we get the per-file code owner of the code owner config that is transitively
+    // imported.
+    PathCodeOwnersResult pathCodeOwnersResult = pathCodeOwners.get().resolveCodeOwnerConfig().get();
+    assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().get().getPathCodeOwners())
+        .comparingElementsUsing(hasEmail())
+        .containsExactly(mdCodeOwner.email());
+    assertThat(pathCodeOwnersResult.resolvedImports())
+        .containsExactly(
+            CodeOwnerConfigImport.createResolvedImport(
+                fooCodeOwnerConfig, barCodeOwnerConfig, barCodeOwnerConfigReference),
+            CodeOwnerConfigImport.createResolvedImport(
+                barCodeOwnerConfig, bazCodeOwnerConfig, bazCodeOwnerConfigReference));
+    assertThat(pathCodeOwnersResult.unresolvedImports()).isEmpty();
+  }
+
   private CodeOwnerConfig.Builder createCodeOwnerBuilder() {
     return CodeOwnerConfig.builder(
         CodeOwnerConfig.Key.create(BranchNameKey.create(project, "master"), Path.of("/")),