Merge "PathCodeOwners: Skip resolving global imports that will be ignored"
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
index 384de36..11b9b6b 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.codeowners.backend;
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 java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toSet;
@@ -236,6 +235,7 @@
codeOwnerConfig.ignoreParentCodeOwners());
getGlobalCodeOwnerSets(codeOwnerConfig)
.forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
+ boolean globalCodeOwnersIgnored = false;
for (CodeOwnerSet codeOwnerSet :
getMatchingPerFileCodeOwnerSets(codeOwnerConfig).collect(toImmutableSet())) {
messages.add(
@@ -243,13 +243,35 @@
"per-file code owner set with path expressions %s matches",
codeOwnerSet.pathExpressions()));
resolvedCodeOwnerConfigBuilder.addCodeOwnerSet(codeOwnerSet);
+ if (codeOwnerSet.ignoreGlobalAndParentCodeOwners()) {
+ globalCodeOwnersIgnored = true;
+ }
}
- // Resolve all global imports.
+ // Resolve global imports.
+ ImmutableList.Builder<UnresolvedImport> unresolvedImports = ImmutableList.builder();
ImmutableSet<CodeOwnerConfigImport> globalImports = getGlobalImports(0, codeOwnerConfig);
- OptionalResultWithMessages<List<UnresolvedImport>> unresolvedGlobalImports =
- resolveImports(codeOwnerConfig.key(), globalImports, resolvedCodeOwnerConfigBuilder);
+ OptionalResultWithMessages<List<UnresolvedImport>> unresolvedGlobalImports;
+ if (!globalCodeOwnersIgnored) {
+ unresolvedGlobalImports =
+ resolveImports(codeOwnerConfig.key(), globalImports, resolvedCodeOwnerConfigBuilder);
+ } else {
+ // skip global import with mode GLOBAL_CODE_OWNER_SETS_ONLY,
+ // since we already know that global code owners will be ignored, we do not need to resolve
+ // these imports
+ unresolvedGlobalImports =
+ resolveImports(
+ codeOwnerConfig.key(),
+ globalImports.stream()
+ .filter(
+ codeOwnerConfigImport ->
+ codeOwnerConfigImport.referenceToImportedCodeOwnerConfig().importMode()
+ != CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY)
+ .collect(toImmutableSet()),
+ resolvedCodeOwnerConfigBuilder);
+ }
messages.addAll(unresolvedGlobalImports.messages());
+ unresolvedImports.addAll(unresolvedGlobalImports.get());
// Remove all global code owner sets if any per-file code owner set has the
// ignoreGlobalAndParentCodeOwners flag set to true (as in this case they are ignored and
@@ -298,16 +320,12 @@
OptionalResultWithMessages<List<UnresolvedImport>> unresolvedPerFileImports =
resolveImports(codeOwnerConfig.key(), perFileImports, resolvedCodeOwnerConfigBuilder);
messages.addAll(unresolvedPerFileImports.messages());
+ unresolvedImports.addAll(unresolvedPerFileImports.get());
this.pathCodeOwnersResult =
OptionalResultWithMessages.create(
PathCodeOwnersResult.create(
- path,
- resolvedCodeOwnerConfigBuilder.build(),
- Stream.concat(
- unresolvedGlobalImports.get().stream(),
- unresolvedPerFileImports.get().stream())
- .collect(toImmutableList())),
+ path, resolvedCodeOwnerConfigBuilder.build(), unresolvedImports.build()),
messages);
logger.atFine().log("path code owners result = %s", pathCodeOwnersResult);
return this.pathCodeOwnersResult;
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
index d4ec968..dce1d52 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
@@ -2006,6 +2006,66 @@
.isFalse();
}
+ @Test
+ public void
+ perFileRuleThatIsImportedByAGlobalImportIsRespectedIfALocalPerFileRuleIgnoresGlobalCodeOwners()
+ throws Exception {
+ TestAccount user2 = accountCreator.user2();
+ TestAccount user3 =
+ accountCreator.create("user3", "user3@example.com", "User3", /* displayName= */ null);
+
+ // create importing config that has a global import with mode ALL and a per-file rule for md
+ // files that ignores global and parent code owners
+ CodeOwnerConfig.Key importingCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression(testPathExpressions.matchFileType("md"))
+ .setIgnoreGlobalAndParentCodeOwners()
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ // create imported config that has a matching per-file rule
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerEmail(user2.email())
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression(testPathExpressions.matchFileType("md"))
+ .addCodeOwnerEmail(user3.email())
+ .build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ transientCodeOwnerConfigCacheProvider.get(),
+ importingCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the code owner from the matching per-file rule in the importing code
+ // owner config and the code owner from the matching per-file rule in the imported code owner
+ // config, the global code owners are ignored since there is a matching per-file rule that
+ // ignores parent and global code owners
+ assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().get().getPathCodeOwners())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(user.email(), user3.email());
+ assertThat(pathCodeOwners.get().resolveCodeOwnerConfig().get().hasUnresolvedImports())
+ .isFalse();
+ }
+
private CodeOwnerConfig.Builder createCodeOwnerBuilder() {
return CodeOwnerConfig.builder(
CodeOwnerConfig.Key.create(BranchNameKey.create(project, "master"), Paths.get("/")),
