Support global imports of code owner configs
Code owner configs already contain references to other code owner
configs that should be imported, but so far they had no effect (see
commit message of change I46005fbab for context). With this change these
imports are now resolved when code owners for a path are computed.
Non-resolvable imports are silently ignored and cyclic dependencies are
detected and handled.
Imports that are loaded from the same project/branch as the importing
code owner config are imported from the same revision from which the
importing code owner config was loaded. Imports that are loaded from
other projects/branches are imported from the current revision. If
several imports are loaded from the same project/branch we guarantee
that they are all loaded from the same revision, even if the current
revision is changed by a concurrent request while the resolution is
being performed.
Imports from other projects are always loaded from the same branch from
which the importing code owner config was loaded. Later we may allow to
specify the branch explicitly in the code owner config reference.
Change-Id: I4ebfbb55bc36fc2464f41d5a201b8e1bba5ae58d
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfig.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfig.java
index 647f1e0..0abd856 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfig.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerConfig.java
@@ -363,7 +363,7 @@
}
@AutoValue.Builder
- abstract static class Builder {
+ public abstract static class Builder {
/**
* Sets the project and branch for this owner config key.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
index 86dca27..3a6cfa9 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/PathCodeOwners.java
@@ -20,11 +20,20 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.Map;
import java.util.Optional;
+import java.util.Queue;
+import java.util.Set;
import java.util.stream.Stream;
import org.eclipse.jgit.lib.ObjectId;
@@ -34,6 +43,8 @@
* <p>Code owners from inherited code owner configs are not considered.
*/
class PathCodeOwners {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
@Singleton
public static class Factory {
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
@@ -47,7 +58,8 @@
public PathCodeOwners create(CodeOwnerConfig codeOwnerConfig, Path absolutePath) {
requireNonNull(codeOwnerConfig, "codeOwnerConfig");
- return new PathCodeOwners(codeOwnerConfig, absolutePath, getMatcher(codeOwnerConfig.key()));
+ return new PathCodeOwners(
+ codeOwners, codeOwnerConfig, absolutePath, getMatcher(codeOwnerConfig.key()));
}
public Optional<PathCodeOwners> create(
@@ -57,7 +69,7 @@
.map(
codeOwnerConfig ->
new PathCodeOwners(
- codeOwnerConfig, absolutePath, getMatcher(codeOwnerConfigKey)));
+ codeOwners, codeOwnerConfig, absolutePath, getMatcher(codeOwnerConfigKey)));
}
/**
@@ -86,12 +98,19 @@
}
}
+ private final CodeOwners codeOwners;
private final CodeOwnerConfig codeOwnerConfig;
private final Path path;
private final PathExpressionMatcher pathExpressionMatcher;
+ private CodeOwnerConfig resolvedCodeOwnerConfig;
+
private PathCodeOwners(
- CodeOwnerConfig codeOwnerConfig, Path path, PathExpressionMatcher pathExpressionMatcher) {
+ CodeOwners codeOwners,
+ CodeOwnerConfig codeOwnerConfig,
+ Path path,
+ PathExpressionMatcher pathExpressionMatcher) {
+ this.codeOwners = requireNonNull(codeOwners, "codeOwners");
this.codeOwnerConfig = requireNonNull(codeOwnerConfig, "codeOwnerConfig");
this.path = requireNonNull(path, "path");
this.pathExpressionMatcher = requireNonNull(pathExpressionMatcher, "pathExpressionMatcher");
@@ -112,21 +131,7 @@
* @return the code owners of the path
*/
public ImmutableSet<CodeOwnerReference> get() {
- Stream.Builder<CodeOwnerSet> matchingCodeOwnerSets = Stream.builder();
-
- // Add all code owner sets that have matching path expressions.
- getMatchingPerFileCodeOwnerSets().forEach(matchingCodeOwnerSets::add);
-
- // Add all code owner sets without path expressions if global code owners are not ignored.
- if (!ignoreGlobalCodeOwners()) {
- codeOwnerConfig.codeOwnerSets().stream()
- .filter(codeOwnerSet -> codeOwnerSet.pathExpressions().isEmpty())
- .forEach(matchingCodeOwnerSets::add);
- }
-
- // Resolve the matching code owner sets to code owner references.
- return matchingCodeOwnerSets
- .build()
+ return resolveCodeOwnerConfig().codeOwnerSets().stream()
.flatMap(codeOwnerSet -> codeOwnerSet.codeOwners().stream())
.collect(toImmutableSet());
}
@@ -137,23 +142,192 @@
* @return whether parent code owners should be ignored for the path
*/
public boolean ignoreParentCodeOwners() {
- if (codeOwnerConfig.ignoreParentCodeOwners()) {
- return true;
+ return resolveCodeOwnerConfig().ignoreParentCodeOwners();
+ }
+
+ /**
+ * Resolves the {@link #codeOwnerConfig}.
+ *
+ * <p>Resolving means that:
+ *
+ * <ul>
+ * <li>non-matching per-file code owner sets are dropped (since code owner sets that do not
+ * match the {@link #path} are not relevant)
+ * <li>imported code owner configs are loaded and replaced with the parts of them which should
+ * be imported (depends on the {@link CodeOwnerConfigImportMode}) and that are relevant for
+ * the {@link #path}
+ * <li>global code owner sets are dropped if any matching per-file code owner set has the
+ * ignoreGlobalAndParentCodeOwners flag set to {@code true} (since in this case global code
+ * owners should be ignored and then the global code owner sets are not relevant)
+ * <li>the ignoreParentCodeOwners flag is set to {@code true} if any matching per-file code
+ * owner set has the ignoreGlobalAndParentCodeOwners flag set to true (since in this case
+ * code owners from parent configurations should be ignored)
+ * </ul>
+ *
+ * <p>When resolving imports, cycles are detected and code owner configs that have been seen
+ * already are not evaluated again.
+ *
+ * <p>Non-resolvable imports are silently ignored.
+ *
+ * <p>Imports that are loaded from the same project/branch as {@link #codeOwnerConfig} are
+ * imported from the same revision from which {@link #codeOwnerConfig} was loaded. Imports that
+ * are loaded from other projects/branches are imported from the current revision. If several
+ * imports are loaded from the same project/branch we guarantee that they are all loaded from the
+ * same revision, even if the current revision is changed by a concurrent request while the
+ * resolution is being performed.
+ *
+ * <p>Imports from other projects are always loaded from the same branch from which the importing
+ * code owner config was loaded.
+ *
+ * @return the resolved code owner config
+ */
+ private CodeOwnerConfig resolveCodeOwnerConfig() {
+ if (this.resolvedCodeOwnerConfig != null) {
+ return this.resolvedCodeOwnerConfig;
}
- return ignoreGlobalCodeOwners();
+ CodeOwnerConfig.Builder resolvedCodeOwnerConfigBuilder =
+ CodeOwnerConfig.builder(codeOwnerConfig.key(), codeOwnerConfig.revision());
+
+ // Add all data from the importing code owner config.
+ resolvedCodeOwnerConfigBuilder.setIgnoreParentCodeOwners(
+ codeOwnerConfig.ignoreParentCodeOwners());
+ getGlobalCodeOwnerSets(codeOwnerConfig)
+ .forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
+ getMatchingPerFileCodeOwnerSets(codeOwnerConfig)
+ .forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
+
+ resolveImports(codeOwnerConfig, resolvedCodeOwnerConfigBuilder);
+
+ CodeOwnerConfig resolvedCodeOwnerConfig = resolvedCodeOwnerConfigBuilder.build();
+
+ // Remove global code owner sets if any per-file code owner set has the
+ // ignoreGlobalAndParentCodeOwners flag set to true.
+ // In this case also set ignoreParentCodeOwners to true, so that we do not need to inspect the
+ // ignoreGlobalAndParentCodeOwners flags again.
+ if (getMatchingPerFileCodeOwnerSets(resolvedCodeOwnerConfig)
+ .anyMatch(codeOwnerSet -> codeOwnerSet.ignoreGlobalAndParentCodeOwners())) {
+ resolvedCodeOwnerConfig =
+ resolvedCodeOwnerConfig
+ .toBuilder()
+ .setIgnoreParentCodeOwners()
+ .setCodeOwnerSets(
+ resolvedCodeOwnerConfig.codeOwnerSets().stream()
+ .filter(codeOwnerSet -> !codeOwnerSet.pathExpressions().isEmpty())
+ .collect(toImmutableSet()))
+ .build();
+ }
+
+ this.resolvedCodeOwnerConfig = resolvedCodeOwnerConfig;
+ return this.resolvedCodeOwnerConfig;
}
- private boolean ignoreGlobalCodeOwners() {
- return getMatchingPerFileCodeOwnerSets()
- .anyMatch(CodeOwnerSet::ignoreGlobalAndParentCodeOwners);
+ private void resolveImports(
+ CodeOwnerConfig importingCodeOwnerConfig,
+ CodeOwnerConfig.Builder resolvedCodeOwnerConfigBuilder) {
+ // To detect cyclic dependencies we keep track of all seen code owner configs.
+ Set<CodeOwnerConfig.Key> seenCodeOwnerConfigs = new HashSet<>();
+ seenCodeOwnerConfigs.add(codeOwnerConfig.key());
+
+ // To ensure that code owner configs from the same project/branch are imported from the same
+ // revision we keep track of the revisions.
+ Map<BranchNameKey, ObjectId> revisionMap = new HashMap<>();
+ revisionMap.put(codeOwnerConfig.key().branchNameKey(), codeOwnerConfig.revision());
+
+ Queue<CodeOwnerConfigReference> codeOwnerConfigsToImport = new LinkedList<>();
+ codeOwnerConfigsToImport.addAll(importingCodeOwnerConfig.imports());
+
+ while (!codeOwnerConfigsToImport.isEmpty()) {
+ CodeOwnerConfigReference codeOwnerConfigReference = codeOwnerConfigsToImport.poll();
+ CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig =
+ createKeyForImportedCodeOwnerConfig(
+ importingCodeOwnerConfig.key(), codeOwnerConfigReference);
+
+ Optional<ObjectId> revision =
+ Optional.ofNullable(revisionMap.get(keyOfImportedCodeOwnerConfig.branchNameKey()));
+
+ Optional<CodeOwnerConfig> mayBeImportedCodeOwnerConfig =
+ revision.isPresent()
+ ? codeOwners.get(keyOfImportedCodeOwnerConfig, revision.get())
+ : codeOwners.getFromCurrentRevision(keyOfImportedCodeOwnerConfig);
+
+ if (!mayBeImportedCodeOwnerConfig.isPresent()) {
+ 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"));
+ continue;
+ }
+
+ CodeOwnerConfig importedCodeOwnerConfig = mayBeImportedCodeOwnerConfig.get();
+ CodeOwnerConfigImportMode importMode = codeOwnerConfigReference.importMode();
+
+ if (!revisionMap.containsKey(keyOfImportedCodeOwnerConfig.branchNameKey())) {
+ revisionMap.put(
+ keyOfImportedCodeOwnerConfig.branchNameKey(), importedCodeOwnerConfig.revision());
+ }
+
+ if (importMode.importIgnoreParentCodeOwners()
+ && importedCodeOwnerConfig.ignoreParentCodeOwners()) {
+ resolvedCodeOwnerConfigBuilder.setIgnoreParentCodeOwners();
+ }
+
+ if (importMode.importGlobalCodeOwnerSets()) {
+ getGlobalCodeOwnerSets(importedCodeOwnerConfig)
+ .forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
+ }
+
+ if (importMode.importPerFileCodeOwnerSets()) {
+ getMatchingPerFileCodeOwnerSets(importedCodeOwnerConfig)
+ .forEach(resolvedCodeOwnerConfigBuilder::addCodeOwnerSet);
+ }
+
+ if (importMode.resolveImportsOfImport()
+ && seenCodeOwnerConfigs.add(keyOfImportedCodeOwnerConfig)) {
+ codeOwnerConfigsToImport.addAll(importedCodeOwnerConfig.imports());
+ }
+ }
}
- private Stream<CodeOwnerSet> getMatchingPerFileCodeOwnerSets() {
- Path relativePath = codeOwnerConfig.relativize(path);
+ private CodeOwnerConfig.Key createKeyForImportedCodeOwnerConfig(
+ CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig,
+ CodeOwnerConfigReference codeOwnerConfigReference) {
+ // if the code owner config reference doesn't have a project, the imported code owner config
+ // file is contained in the same project as the importing code owner config
+ Project.NameKey project =
+ codeOwnerConfigReference.project().orElse(keyOfImportingCodeOwnerConfig.project());
+
+ // code owner configs are always imported from the same branch in which the importing code
+ // owner config is stored
+ String branch = keyOfImportingCodeOwnerConfig.branchNameKey().branch();
+
+ // if the path of the imported code owner config is relative, it should be resolved against
+ // the folder path of the importing code owner config
+ Path folderPath =
+ keyOfImportingCodeOwnerConfig
+ .folderPath()
+ .resolve(codeOwnerConfigReference.path())
+ .normalize();
+
+ return CodeOwnerConfig.Key.create(
+ BranchNameKey.create(project, branch), folderPath, codeOwnerConfigReference.fileName());
+ }
+
+ private Stream<CodeOwnerSet> getGlobalCodeOwnerSets(CodeOwnerConfig codeOwnerConfig) {
+ return codeOwnerConfig.codeOwnerSets().stream()
+ .filter(codeOwnerSet -> codeOwnerSet.pathExpressions().isEmpty());
+ }
+
+ private Stream<CodeOwnerSet> getMatchingPerFileCodeOwnerSets(CodeOwnerConfig codeOwnerConfig) {
return codeOwnerConfig.codeOwnerSets().stream()
.filter(codeOwnerSet -> !codeOwnerSet.pathExpressions().isEmpty())
- .filter(codeOwnerSet -> matches(codeOwnerSet, relativePath, pathExpressionMatcher));
+ .filter(codeOwnerSet -> matches(codeOwnerSet, getRelativePath(), pathExpressionMatcher));
+ }
+
+ private Path getRelativePath() {
+ return codeOwnerConfig.relativize(path);
}
/**
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
index bf89495..c9674f4 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/PathCodeOwnersTest.java
@@ -24,10 +24,13 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.PrivateInternals_DynamicMapImpl;
import com.google.gerrit.extensions.registration.RegistrationHandle;
@@ -41,6 +44,7 @@
import java.nio.file.Paths;
import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
@@ -429,6 +433,706 @@
}
@Test
+ public void nonResolveableImportIsIgnored() throws Exception {
+ // create importing config with non-resolveable import
+ CodeOwnerConfig.Key importingCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.ALL, "/non-existing/OWNERS"))
+ .addCodeOwnerSet(CodeOwnerSet.builder().addCodeOwnerEmail(admin.email()).build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ importingCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global code owner from the importing code owner config, the
+ // non-resolveable import is silenty ignored
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email());
+ }
+
+ @Test
+ public void importGlobalCodeOwners_importModeAll() throws Exception {
+ testImportGlobalCodeOwners(CodeOwnerConfigImportMode.ALL);
+ }
+
+ @Test
+ public void importGlobalCodeOwners_importModeGlobalCodeOwnerSetsOnly() throws Exception {
+ testImportGlobalCodeOwners(CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY);
+ }
+
+ private void testImportGlobalCodeOwners(CodeOwnerConfigImportMode importMode) throws Exception {
+ // create importing config with global code owner and import
+ CodeOwnerConfig.Key importingCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(CodeOwnerConfigReference.create(importMode, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with global code owner
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ importingCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global code owners from the importing and the imported code owner
+ // config
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email());
+ }
+
+ @Test
+ public void importPerFileCodeOwners_importModeAll() throws Exception {
+ // create importing config with matching per-file code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("*.md")
+ .addCodeOwnerEmail(admin.email())
+ .build())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with matching per-file code owner
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("*.md")
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the matching per-file code owners from the importing and the imported
+ // code owner config
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email());
+ }
+
+ @Test
+ public void nonMatchingPerFileCodeOwnersAreNotImported_importModeAll() throws Exception {
+ // create importing config with matching per-file code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("*.md")
+ .addCodeOwnerEmail(admin.email())
+ .build())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with non-matching per-file code owner
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("*.txt")
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we only get the matching per-file code owners from the importing code owner
+ // config, the per-file code owners from the imported code owner config are not relevant since
+ // they do not match
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email());
+ }
+
+ @Test
+ public void perFileCodeOwnersAreNotImported_importModeGlobalCodeOwnerSetsOnly() throws Exception {
+ // create importing config with matching per-file code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("*.md")
+ .addCodeOwnerEmail(admin.email())
+ .build())
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with matching per-file code owner
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .addPathExpression("*.md")
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we only get the matching per-file code owners from the importing code owner
+ // config, the matching per-file code owners from the imported code owner config are not
+ // relevant with import mode GLOBAL_CODE_OWNER_SETS_ONLY
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email());
+ }
+
+ @Test
+ public void
+ importIgnoreGlobalAndParentCodeOwnersFlagFromMatchingPerFileCodeOwnerSet_importModeAll()
+ throws Exception {
+ // create importing config with global code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with matching per-file code owner that has the
+ // ignoreGlobalAndParentCodeOwners flag set to true
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .setIgnoreGlobalAndParentCodeOwners()
+ .addPathExpression("*.md")
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we only get the matching per-file code owners from the imported code owner
+ // config, the global code owners from the importing code owner config are not relevant since
+ // the matching per-file code owner set in the imported code owner config has the
+ // ignoreGlobalAndParentCodeOwners flag set to true which causes global code owners to be
+ // ignored, in addition this flag causes parent code owners to be ignored
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(user.email());
+ assertThat(pathCodeOwners.get().ignoreParentCodeOwners()).isTrue();
+ }
+
+ @Test
+ public void
+ ignoreGlobalAndParentCodeOwnersFlagIsNotImportedFromNonMatchingPerFileCodeOwnerSet_importModeAll()
+ throws Exception {
+ // create importing config with global code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with non-matching per-file code owner that has the
+ // ignoreGlobalAndParentCodeOwners flag set to true
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .setIgnoreGlobalAndParentCodeOwners()
+ .addPathExpression("*.txt")
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we only get the global code owners from the importing code owner config, the
+ // per-file code owners from the imported code owner config and its
+ // ignoreGlobalAndParentCodeOwners flag are not relevant since the per-file code owner set does
+ // not match
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email());
+ assertThat(pathCodeOwners.get().ignoreParentCodeOwners()).isFalse();
+ }
+
+ @Test
+ public void ignoreGlobalAndParentCodeOwnersFlagIsNotImported_importModeGlobalCodeOwnerSetsOnly()
+ throws Exception {
+ // create importing config with global code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with matching per-file code owner that has the
+ // ignoreGlobalAndParentCodeOwners flag set to true
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerSet(
+ CodeOwnerSet.builder()
+ .setIgnoreGlobalAndParentCodeOwners()
+ .addPathExpression("*.md")
+ .addCodeOwnerEmail(user.email())
+ .build())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we only get the global code owners from the importing code owner config, the
+ // matching per-file code owners from the imported code owner config and its
+ // ignoreGlobalAndParentCodeOwners flag are not relevant with import mode
+ // GLOBAL_CODE_OWNER_SETS_ONLY
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email());
+ assertThat(pathCodeOwners.get().ignoreParentCodeOwners()).isFalse();
+ }
+
+ @Test
+ public void importIgnoreParentCodeOwnersFlag_importModeAll() throws Exception {
+ // create importing config
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with the ignoreParentCodeOnwers flag set to true
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .ignoreParentCodeOwners()
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: ignoreParentCodeOwners is true because the ignoreParentCodeOwners flag in the
+ // imported code owner config is set to true
+ assertThat(pathCodeOwners.get().ignoreParentCodeOwners()).isTrue();
+ }
+
+ @Test
+ public void ignoreParentCodeOwnersFlagNotImported_importModeGlobalCodeOwnerSetsOnly()
+ throws Exception {
+ // create importing config
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with the ignoreParentCodeOnwers flag set to true
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .ignoreParentCodeOwners()
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: ignoreParentCodeOwners is false because the ignoreParentCodeOwners flag in the
+ // imported code owner config is not relevant with import mode GLOBAL_CODE_OWNER_SETS_ONLY
+ assertThat(pathCodeOwners.get().ignoreParentCodeOwners()).isFalse();
+ }
+
+ @Test
+ public void importsOfImportedCodeOwenerConfigAreResolved_importModeAll() throws Exception {
+ testImportsOfImportedCodeOwenerConfigAreResolved(CodeOwnerConfigImportMode.ALL);
+ }
+
+ @Test
+ public void importsOfImportedCodeOwenerConfigAreResolved_importModeGlobalCodeOwnerSetsOnly()
+ throws Exception {
+ testImportsOfImportedCodeOwenerConfigAreResolved(
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY);
+ }
+
+ private void testImportsOfImportedCodeOwenerConfigAreResolved(
+ CodeOwnerConfigImportMode importMode) throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create importing config with global code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(CodeOwnerConfigReference.create(importMode, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with global code owner and import
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerEmail(user.email())
+ .addImport(CodeOwnerConfigReference.create(importMode, "/baz/OWNERS"))
+ .create();
+
+ // create config with global code owner that is imported by the imported config
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/baz/")
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global owners from the importing code owner config, the imported code
+ // owner config and the code owner config that is imported by the imported code owner config
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email(), user2.email());
+ }
+
+ @Test
+ public void cyclicImports() throws Exception {
+ // create importing config with global code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with global code owner and that imports the importing config
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerEmail(user.email())
+ .addImport(CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/OWNERS"))
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global owners from the importing and the imported code owner config
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email());
+ }
+
+ @Test
+ public void importsAreResolvedFromSameRevision() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ // create importing config with global code owner and import
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS"))
+ .create();
+
+ // create imported config with global code owner
+ CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ // remember the revision
+ RevCommit oldRevision = projectOperations.project(project).getHead("master");
+
+ // update imported config and add one additional global code owner
+ codeOwnerConfigOperations
+ .codeOwnerConfig(keyOfImportedCodeOwnerConfig)
+ .forUpdate()
+ .codeOwnerSetsModification(CodeOwnerSetModification.addToOnlySet(user2.email()))
+ .update();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(rootCodeOwnerConfigKey, oldRevision, Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global owners from the importing and the imported code owner config
+ // as they were defined at oldRevision
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email());
+ }
+
+ @Test
+ public void importWithRelativePath() throws Exception {
+ // create importing config with global code owner and import with relative path
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.create(CodeOwnerConfigImportMode.ALL, "../baz/OWNERS"))
+ .create();
+
+ // create imported config with global code owner
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/baz/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo/bar/baz.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global owners from the importing and the imported code owner config
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email());
+ }
+
+ @Test
+ public void importFromOtherProject() throws Exception {
+ Project.NameKey otherProject = projectOperations.newProject().create();
+
+ // create importing config with global code owner and import with relative path
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.builder(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS")
+ .setProject(otherProject)
+ .build())
+ .create();
+
+ // create imported config with global code owner
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(otherProject)
+ .branch("master")
+ .folderPath("/bar/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead("master"),
+ Paths.get("/foo/bar/baz.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global owners from the importing and the imported code owner config
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email());
+ }
+
+ @Test
+ public void importFromOtherProjectIsResolvedFromSameBranch() throws Exception {
+ Project.NameKey otherProject = projectOperations.newProject().create();
+
+ // Create other branches in project.
+ String branchName = "foo";
+ BranchInput branchInput = new BranchInput();
+ branchInput.ref = branchName;
+ branchInput.revision = projectOperations.project(project).getHead("master").name();
+ gApi.projects().name(project.get()).branch(branchInput.ref).create(branchInput);
+
+ // Create other branches in other project.
+ branchInput.revision = projectOperations.project(otherProject).getHead("master").name();
+ gApi.projects().name(otherProject.get()).branch(branchInput.ref).create(branchInput);
+
+ // create importing config with global code owner and import with relative path
+ CodeOwnerConfig.Key rootCodeOwnerConfigKey =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch(branchName)
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .addImport(
+ CodeOwnerConfigReference.builder(CodeOwnerConfigImportMode.ALL, "/bar/OWNERS")
+ .setProject(otherProject)
+ .build())
+ .create();
+
+ // create imported config with global code owner
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(otherProject)
+ .branch(branchName)
+ .folderPath("/bar/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ Optional<PathCodeOwners> pathCodeOwners =
+ pathCodeOwnersFactory.create(
+ rootCodeOwnerConfigKey,
+ projectOperations.project(project).getHead(branchName),
+ Paths.get("/foo.md"));
+ assertThat(pathCodeOwners).isPresent();
+
+ // Expectation: we get the global owners from the importing and the imported code owner config
+ assertThat(pathCodeOwners.get().get())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(admin.email(), user.email());
+ }
+
+ @Test
public void cannotMatchAgainstNullCodeOwnerSet() throws Exception {
NullPointerException npe =
assertThrows(
