Merge branch 'stable-3.5' into stable-3.6
* stable-3.5:
Make sure that `owners` are JDK8 compatible in `stable-3.4`
Extend 'GetFilesOwners' with Owners submit requirements
Simplify `GetFilesOwners`
Turn 'codeReviewLabelValue' to 'Stream' from 'Optional'
Filter out the files owners based on the owners' code-review value
Fix 'Redundant specification of type arguments <PathOwners>' warn
Clean up GetFileOwners from unused variables
Split owners integration tests into a separate build targets
Follow whole chain of parents for OWNERS in submit requirements
Avoid using Repository.resolve() when accessing exact ref-names
Fix `OWNERS` file over-caching problem
Follow the whole chain of parent projects for OWNERS
Make code_review_user/1 a native plugin predicate
Reuse the code_review_user Prolog rule across gerrit_owners.pl
Change-Id: Ie499186cc5f7744e8f604798703a0788072ea34a
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java
index 0f0db10..17b8af9 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java
@@ -19,9 +19,11 @@
import static org.eclipse.jgit.lib.FileMode.TYPE_FILE;
import static org.eclipse.jgit.lib.FileMode.TYPE_MASK;
+import com.google.gerrit.entities.RefNames;
import java.io.IOException;
import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -34,13 +36,14 @@
public static Optional<byte[]> getBlobAsBytes(Repository repository, String revision, String path)
throws IOException {
- ObjectId objectId = repository.resolve(revision);
- if (objectId == null) {
+ Ref ref = repository.getRefDatabase().exactRef(RefNames.fullName(revision));
+ if (ref == null) {
return Optional.empty();
}
try (final TreeWalk w =
- TreeWalk.forPath(repository, path, parseCommit(repository, objectId).getTree())) {
+ TreeWalk.forPath(
+ repository, path, parseCommit(repository, ref.getLeaf().getObjectId()).getTree())) {
return Optional.ofNullable(w)
.filter(walk -> (walk.getRawMode(0) & TYPE_MASK) == TYPE_FILE)
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
index ab3a7f1..b904715 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
@@ -35,6 +35,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.DiffSummary;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
+import com.google.gerrit.server.project.ProjectState;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
@@ -204,6 +205,12 @@
return label;
}
+ public static List<Project.NameKey> getParents(ProjectState projectState) {
+ return projectState.parents().stream()
+ .map(ProjectState::getNameKey)
+ .collect(Collectors.toList());
+ }
+
/**
* Fetched the owners for the associated patch list.
*
@@ -213,11 +220,11 @@
OwnersMap ownersMap = new OwnersMap();
try {
// Using a `map` would have needed a try/catch inside the lamba, resulting in more code
- List<PathOwnersEntry> parentsPathOwnersEntries =
+ List<ReadOnlyPathOwnersEntry> parentsPathOwnersEntries =
getPathOwnersEntries(parentProjectsNames, RefNames.REFS_CONFIG, cache);
- PathOwnersEntry projectEntry =
- getPathOwnersEntry(project, repository, RefNames.REFS_CONFIG, cache);
- PathOwnersEntry rootEntry = getPathOwnersEntry(project, repository, branch, cache);
+ ReadOnlyPathOwnersEntry projectEntry =
+ getPathOwnersEntryOrEmpty(project, repository, RefNames.REFS_CONFIG, cache);
+ PathOwnersEntry rootEntry = getPathOwnersEntryOrNew(project, repository, branch, cache);
Map<String, PathOwnersEntry> entries = new HashMap<>();
PathOwnersEntry currentEntry = null;
@@ -267,41 +274,51 @@
}
}
- private List<PathOwnersEntry> getPathOwnersEntries(
+ private List<ReadOnlyPathOwnersEntry> getPathOwnersEntries(
List<Project.NameKey> projectNames, String branch, PathOwnersEntriesCache cache)
throws IOException, ExecutionException {
- ImmutableList.Builder<PathOwnersEntry> pathOwnersEntries = ImmutableList.builder();
+ ImmutableList.Builder<ReadOnlyPathOwnersEntry> pathOwnersEntries = ImmutableList.builder();
for (Project.NameKey projectName : projectNames) {
try (Repository repo = repositoryManager.openRepository(projectName)) {
pathOwnersEntries =
- pathOwnersEntries.add(getPathOwnersEntry(projectName.get(), repo, branch, cache));
+ pathOwnersEntries.add(
+ getPathOwnersEntryOrEmpty(projectName.get(), repo, branch, cache));
}
}
return pathOwnersEntries.build();
}
- private PathOwnersEntry getPathOwnersEntry(
+ private ReadOnlyPathOwnersEntry getPathOwnersEntryOrEmpty(
+ String project, Repository repo, String branch, PathOwnersEntriesCache cache)
+ throws ExecutionException {
+ return getPathOwnersEntry(project, repo, branch, cache)
+ .map(v -> (ReadOnlyPathOwnersEntry) v)
+ .orElse(PathOwnersEntry.EMPTY);
+ }
+
+ private PathOwnersEntry getPathOwnersEntryOrNew(
+ String project, Repository repo, String branch, PathOwnersEntriesCache cache)
+ throws ExecutionException {
+ return getPathOwnersEntry(project, repo, branch, cache).orElseGet(PathOwnersEntry::new);
+ }
+
+ private Optional<PathOwnersEntry> getPathOwnersEntry(
String project, Repository repo, String branch, PathOwnersEntriesCache cache)
throws ExecutionException {
String rootPath = "OWNERS";
- return cache.get(
- project,
- branch,
- rootPath,
- () ->
- getOwnersConfig(repo, rootPath, branch)
- .map(
- conf ->
- new PathOwnersEntry(
- rootPath,
- conf,
- accounts,
- Optional.empty(),
- Collections.emptySet(),
- Collections.emptySet(),
- Collections.emptySet(),
- Collections.emptySet()))
- .orElse(PathOwnersEntry.EMPTY));
+ return cache
+ .get(project, branch, rootPath, () -> getOwnersConfig(repo, rootPath, branch))
+ .map(
+ conf ->
+ new PathOwnersEntry(
+ rootPath,
+ conf,
+ accounts,
+ Optional.empty(),
+ Collections.emptySet(),
+ Collections.emptySet(),
+ Collections.emptySet(),
+ Collections.emptySet()));
}
private void processMatcherPerPath(
@@ -352,8 +369,8 @@
String project,
String path,
String branch,
- PathOwnersEntry projectEntry,
- List<PathOwnersEntry> parentsPathOwnersEntries,
+ ReadOnlyPathOwnersEntry projectEntry,
+ List<ReadOnlyPathOwnersEntry> parentsPathOwnersEntries,
PathOwnersEntry rootEntry,
Map<String, PathOwnersEntry> entries,
PathOwnersEntriesCache cache)
@@ -366,7 +383,7 @@
calculateCurrentEntry(rootEntry, projectEntry, currentEntry);
// Inherit from Parent Project if OWNER in Project enables inheritance
- for (PathOwnersEntry parentPathOwnersEntry : parentsPathOwnersEntries) {
+ for (ReadOnlyPathOwnersEntry parentPathOwnersEntry : parentsPathOwnersEntries) {
calculateCurrentEntry(projectEntry, parentPathOwnersEntry, currentEntry);
}
@@ -384,31 +401,31 @@
String ownersPath = partial + "OWNERS";
PathOwnersEntry pathFallbackEntry = currentEntry;
currentEntry =
- cache.get(
- project,
- branch,
- ownersPath,
- () ->
- getOwnersConfig(repository, ownersPath, branch)
- .map(
- c -> {
- Optional<LabelDefinition> label = pathFallbackEntry.getLabel();
- final Set<Id> owners = pathFallbackEntry.getOwners();
- final Set<Id> reviewers = pathFallbackEntry.getReviewers();
- Collection<Matcher> inheritedMatchers =
- pathFallbackEntry.getMatchers().values();
- Set<String> groupOwners = pathFallbackEntry.getGroupOwners();
- return new PathOwnersEntry(
- ownersPath,
- c,
- accounts,
- label,
- owners,
- reviewers,
- inheritedMatchers,
- groupOwners);
- })
- .orElse(pathFallbackEntry));
+ cache
+ .get(
+ project,
+ branch,
+ ownersPath,
+ () -> getOwnersConfig(repository, ownersPath, branch))
+ .map(
+ c -> {
+ Optional<LabelDefinition> label = pathFallbackEntry.getLabel();
+ final Set<Id> owners = pathFallbackEntry.getOwners();
+ final Set<Id> reviewers = pathFallbackEntry.getReviewers();
+ Collection<Matcher> inheritedMatchers =
+ pathFallbackEntry.getMatchers().values();
+ Set<String> groupOwners = pathFallbackEntry.getGroupOwners();
+ return new PathOwnersEntry(
+ ownersPath,
+ c,
+ accounts,
+ label,
+ owners,
+ reviewers,
+ inheritedMatchers,
+ groupOwners);
+ })
+ .orElse(pathFallbackEntry);
entries.put(partial, currentEntry);
}
}
@@ -416,7 +433,9 @@
}
private void calculateCurrentEntry(
- PathOwnersEntry rootEntry, PathOwnersEntry projectEntry, PathOwnersEntry currentEntry) {
+ ReadOnlyPathOwnersEntry rootEntry,
+ ReadOnlyPathOwnersEntry projectEntry,
+ PathOwnersEntry currentEntry) {
if (rootEntry.isInherited()) {
for (Matcher matcher : projectEntry.getMatchers().values()) {
if (!currentEntry.hasMatcher(matcher.getPath())) {
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java
index 69584ab..901dbc8 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCache.java
@@ -25,7 +25,9 @@
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
import java.util.Objects;
+import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@@ -36,7 +38,7 @@
return new CacheModule() {
@Override
protected void configure() {
- cache(CACHE_NAME, Key.class, PathOwnersEntry.class);
+ cache(CACHE_NAME, Key.class, new TypeLiteral<Optional<OwnersConfig>>() {});
bind(PathOwnersEntriesCache.class).to(PathOwnersEntriesCacheImpl.class);
DynamicSet.bind(binder(), GitBatchRefUpdateListener.class)
.to(OwnersRefUpdateListener.class);
@@ -45,7 +47,8 @@
};
}
- PathOwnersEntry get(String project, String branch, String path, Callable<PathOwnersEntry> loader)
+ Optional<OwnersConfig> get(
+ String project, String branch, String path, Callable<Optional<OwnersConfig>> loader)
throws ExecutionException;
void invalidate(String project, String branch);
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java
index 74f9585..3af7a58 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java
@@ -25,18 +25,19 @@
import com.google.inject.name.Named;
import java.time.Duration;
import java.util.Collection;
+import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@Singleton
class PathOwnersEntriesCacheImpl implements PathOwnersEntriesCache {
- private final Cache<Key, PathOwnersEntry> cache;
+ private final Cache<Key, Optional<OwnersConfig>> cache;
private final Multimap<String, Key> keysIndex;
private final LoadingCache<String, Object> keyLocks;
@Inject
- PathOwnersEntriesCacheImpl(@Named(CACHE_NAME) Cache<Key, PathOwnersEntry> cache) {
+ PathOwnersEntriesCacheImpl(@Named(CACHE_NAME) Cache<Key, Optional<OwnersConfig>> cache) {
this.cache = cache;
this.keysIndex = HashMultimap.create();
this.keyLocks =
@@ -46,14 +47,14 @@
}
@Override
- public PathOwnersEntry get(
- String project, String branch, String path, Callable<PathOwnersEntry> loader)
+ public Optional<OwnersConfig> get(
+ String project, String branch, String path, Callable<Optional<OwnersConfig>> loader)
throws ExecutionException {
Key key = new Key(project, branch, path);
return cache.get(
key,
() -> {
- PathOwnersEntry entry = loader.call();
+ Optional<OwnersConfig> entry = loader.call();
String indexKey = indexKey(project, branch);
synchronized (keyLocks.getUnchecked(indexKey)) {
keysIndex.put(indexKey, key);
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
index 1ede8a6..8e4c53c 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
@@ -31,20 +31,11 @@
*
* <p>Used internally by PathOwners to represent and compute the owners at a specific path.
*/
-class PathOwnersEntry {
- static final PathOwnersEntry EMPTY = new PathOwnersEntry();
-
- private final boolean inherited;
- private Optional<LabelDefinition> label;
- private Set<Account.Id> owners = Sets.newHashSet();
- private Set<Account.Id> reviewers = Sets.newHashSet();
- private String ownersPath;
- private Map<String, Matcher> matchers = Maps.newHashMap();
- private Set<String> groupOwners = Sets.newHashSet();
+class PathOwnersEntry extends ReadOnlyPathOwnersEntry {
+ static final ReadOnlyPathOwnersEntry EMPTY = new ReadOnlyPathOwnersEntry(true) {};
public PathOwnersEntry() {
- inherited = true;
- label = Optional.empty();
+ super(true);
}
public PathOwnersEntry(
@@ -56,6 +47,7 @@
Set<Account.Id> inheritedReviewers,
Collection<Matcher> inheritedMatchers,
Set<String> inheritedGroupOwners) {
+ super(config.isInherited());
this.ownersPath = path;
this.owners =
config.getOwners().stream()
@@ -82,20 +74,6 @@
} else {
this.label = config.getLabel();
}
- this.inherited = config.isInherited();
- }
-
- @Override
- public String toString() {
- return "PathOwnersEntry [ownersPath="
- + ownersPath
- + ", owners="
- + owners
- + ", matchers="
- + matchers
- + ", label="
- + label
- + "]";
}
public void addMatcher(Matcher matcher) {
@@ -103,6 +81,52 @@
this.matchers.put(matcher.getPath(), matcher.merge(currMatchers));
}
+ public void setOwners(Set<Account.Id> owners) {
+ this.owners = owners;
+ }
+
+ public void setReviewers(Set<Account.Id> reviewers) {
+ this.reviewers = reviewers;
+ }
+
+ public void setOwnersPath(String ownersPath) {
+ this.ownersPath = ownersPath;
+ }
+
+ public void setMatchers(Map<String, Matcher> matchers) {
+ this.matchers = matchers;
+ }
+
+ public void setLabel(Optional<LabelDefinition> label) {
+ this.label = label;
+ }
+
+ public void addMatchers(Collection<Matcher> values) {
+ for (Matcher matcher : values) {
+ addMatcher(matcher);
+ }
+ }
+
+ @Override
+ protected String className() {
+ return getClass().getSimpleName();
+ }
+}
+
+abstract class ReadOnlyPathOwnersEntry {
+ protected final boolean inherited;
+ protected Optional<LabelDefinition> label;
+ protected Set<Account.Id> owners = Sets.newHashSet();
+ protected Set<Account.Id> reviewers = Sets.newHashSet();
+ protected String ownersPath;
+ protected Map<String, Matcher> matchers = Maps.newHashMap();
+ protected Set<String> groupOwners = Sets.newHashSet();
+
+ protected ReadOnlyPathOwnersEntry(boolean inherited) {
+ this.inherited = inherited;
+ label = Optional.empty();
+ }
+
public Map<String, Matcher> getMatchers() {
return matchers;
}
@@ -115,30 +139,14 @@
return groupOwners;
}
- public void setOwners(Set<Account.Id> owners) {
- this.owners = owners;
- }
-
public Set<Account.Id> getReviewers() {
return reviewers;
}
- public void setReviewers(Set<Account.Id> reviewers) {
- this.reviewers = reviewers;
- }
-
public String getOwnersPath() {
return ownersPath;
}
- public void setOwnersPath(String ownersPath) {
- this.ownersPath = ownersPath;
- }
-
- public void setMatchers(Map<String, Matcher> matchers) {
- this.matchers = matchers;
- }
-
public boolean isInherited() {
return inherited;
}
@@ -147,16 +155,6 @@
return label;
}
- public void setLabel(Optional<LabelDefinition> label) {
- this.label = label;
- }
-
- public void addMatchers(Collection<Matcher> values) {
- for (Matcher matcher : values) {
- addMatcher(matcher);
- }
- }
-
public boolean hasMatcher(String path) {
return this.matchers.containsKey(path);
}
@@ -164,4 +162,22 @@
public static String stripOwnerDomain(String owner) {
return Splitter.on('@').split(owner).iterator().next();
}
+
+ @Override
+ public String toString() {
+ return className()
+ + " [ownersPath="
+ + ownersPath
+ + ", owners="
+ + owners
+ + ", matchers="
+ + matchers
+ + ", label="
+ + label
+ + "]";
+ }
+
+ protected String className() {
+ return "ReadOnlyPathOwnersEntry";
+ }
}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheMock.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheMock.java
index 1eac62c..752bf7f 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheMock.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheMock.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.owners.common;
+import java.util.Optional;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import org.junit.Ignore;
@@ -30,8 +31,8 @@
public void invalidateIndexKey(Key key) {}
@Override
- public PathOwnersEntry get(
- String project, String branch, String path, Callable<PathOwnersEntry> loader)
+ public Optional<OwnersConfig> get(
+ String project, String branch, String path, Callable<Optional<OwnersConfig>> loader)
throws ExecutionException {
try {
hit++;
diff --git a/owners/BUILD b/owners/BUILD
index 402bd34..f08ec84 100644
--- a/owners/BUILD
+++ b/owners/BUILD
@@ -12,8 +12,8 @@
name = "gerrit-owners-predicates",
srcs = PROLOG_PREDICATES,
deps = [
+ "//owners-common",
"@prolog-runtime//jar:neverlink",
- "//owners-common:owners-common",
] + PLUGIN_DEPS_NEVERLINK,
)
@@ -50,15 +50,37 @@
java_library(
name = "owners__plugin_test_deps",
testonly = 1,
+ srcs = glob(
+ ["src/test/java/**/*.java"],
+ exclude = [
+ "src/test/java/**/*Test.java",
+ "src/test/java/**/*IT.java",
+ ],
+ ),
visibility = ["//visibility:public"],
exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":owners",
],
+ deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":owners",
+ ],
)
junit_tests(
name = "owners_tests",
- srcs = glob(["src/test/java/**/*.java"]),
+ srcs = glob(["src/test/java/**/*Test.java"]),
tags = ["owners"],
- deps = ["owners__plugin_test_deps"],
+ deps = [
+ ":owners__plugin_test_deps",
+ ],
)
+
+[junit_tests(
+ name = f[:f.index(".")].replace("/", "_"),
+ srcs = [f],
+ tags = ["owners"],
+ visibility = ["//visibility:public"],
+ deps = [
+ ":owners__plugin_test_deps",
+ ],
+) for f in glob(["src/test/java/**/*IT.java"])]
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
index 02ed2f8..c4d27e4 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
@@ -28,8 +28,6 @@
import com.googlesource.gerrit.owners.common.PathOwners;
import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache;
import com.googlesource.gerrit.owners.common.PluginSettings;
-import java.util.Arrays;
-import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -53,7 +51,7 @@
}
log.info("Initializing OwnerStoredValues");
PATH_OWNERS =
- new StoredValue<PathOwners>() {
+ new StoredValue<>() {
@Override
protected PathOwners createValue(Prolog engine) {
Map<String, FileDiffOutput> patchList = StoredValues.DIFF_LIST.get(engine);
@@ -63,17 +61,13 @@
metrics.countConfigLoads.increment();
try (Timer0.Context ctx = metrics.loadConfig.start()) {
- List<Project.NameKey> maybeParentProjectNameKey =
- Optional.ofNullable(projectState.getProject().getParent())
- .map(Arrays::asList)
- .orElse(Collections.emptyList());
-
+ List<Project.NameKey> parentProjectsNameKeys = PathOwners.getParents(projectState);
String branch = StoredValues.getChange(engine).getDest().branch();
return new PathOwners(
accounts,
gitRepositoryManager,
repository,
- maybeParentProjectNameKey,
+ parentProjectsNameKeys,
settings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
patchList,
settings.expandGroups(),
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
index f26ae71..3935a4b 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
@@ -52,8 +52,6 @@
import com.googlesource.gerrit.owners.common.PathOwnersEntriesCache;
import com.googlesource.gerrit.owners.common.PluginSettings;
import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -201,11 +199,7 @@
try (Timer0.Context ctx = metrics.loadConfig.start()) {
String branch = cd.change().getDest().branch();
- List<Project.NameKey> parents =
- Optional.<Project.NameKey>ofNullable(projectState.getProject().getParent())
- .map(Arrays::asList)
- .orElse(Collections.emptyList());
-
+ List<Project.NameKey> parents = PathOwners.getParents(projectState);
Project.NameKey nameKey = projectState.getNameKey();
try (Repository repo = repoManager.openRepository(nameKey)) {
PathOwners pathOwners =
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
index d5d403b..5f5b1fb 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -15,11 +15,11 @@
package com.googlesource.gerrit.owners.restapi;
-import com.google.common.base.Predicates;
import com.google.common.collect.Maps;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -27,13 +27,13 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
@@ -45,65 +45,62 @@
import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
import com.googlesource.gerrit.owners.entities.GroupOwner;
import com.googlesource.gerrit.owners.entities.Owner;
-import java.util.*;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
import java.util.stream.Collectors;
+import java.util.stream.Stream;
import org.eclipse.jgit.lib.Repository;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class GetFilesOwners implements RestReadView<RevisionResource> {
- private static final Logger log = LoggerFactory.getLogger(GetFilesOwners.class);
-
- private final PatchListCache patchListCache;
private final Accounts accounts;
private final AccountCache accountCache;
private final ProjectCache projectCache;
private final GitRepositoryManager repositoryManager;
private final PluginSettings pluginSettings;
private final GerritApi gerritApi;
- private final ChangeData.Factory changeDataFactory;
private final PathOwnersEntriesCache cache;
+ static final String MISSING_CODE_REVIEW_LABEL =
+ "Cannot calculate file owners state when review label is not configured";
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
@Inject
GetFilesOwners(
- PatchListCache patchListCache,
Accounts accounts,
AccountCache accountCache,
ProjectCache projectCache,
GitRepositoryManager repositoryManager,
PluginSettings pluginSettings,
GerritApi gerritApi,
- ChangeData.Factory changeDataFactory,
PathOwnersEntriesCache cache) {
- this.patchListCache = patchListCache;
this.accounts = accounts;
this.accountCache = accountCache;
this.projectCache = projectCache;
this.repositoryManager = repositoryManager;
this.pluginSettings = pluginSettings;
this.gerritApi = gerritApi;
- this.changeDataFactory = changeDataFactory;
this.cache = cache;
}
@Override
public Response<FilesOwnersResponse> apply(RevisionResource revision)
throws AuthException, BadRequestException, ResourceConflictException, Exception {
- PatchSet ps = revision.getPatchSet();
Change change = revision.getChange();
- int id = revision.getChangeResource().getChange().getChangeId();
+ ChangeData changeData = revision.getChangeResource().getChangeData();
Project.NameKey project = change.getProject();
- List<Project.NameKey> maybeParentProjectNameKey =
- projectCache
- .get(project)
- .map(p -> Arrays.asList(p.getProject().getParent()))
- .filter(Predicates.notNull())
- .orElse(Collections.emptyList());
+ List<Project.NameKey> projectParents =
+ projectCache.get(project).map(PathOwners::getParents).orElse(Collections.emptyList());
try (Repository repository = repositoryManager.openRepository(project)) {
- Set<String> changePaths = new HashSet<>(changeDataFactory.create(change).currentFilePaths());
+ Set<String> changePaths = new HashSet<>(changeData.currentFilePaths());
String branch = change.getDest().branch();
PathOwners owners =
@@ -111,31 +108,101 @@
accounts,
repositoryManager,
repository,
- maybeParentProjectNameKey,
+ projectParents,
pluginSettings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
changePaths,
pluginSettings.expandGroups(),
project.get(),
cache);
+ Map<String, Set<GroupOwner>> fileExpandedOwners =
+ Maps.transformValues(
+ owners.getFileOwners(),
+ ids ->
+ ids.stream()
+ .map(this::getOwnerFromAccountId)
+ .flatMap(Optional::stream)
+ .collect(Collectors.toSet()));
+
Map<String, Set<GroupOwner>> fileToOwners =
pluginSettings.expandGroups()
- ? Maps.transformValues(
- owners.getFileOwners(),
- ids ->
- ids.stream()
- .map(this::getOwnerFromAccountId)
- .flatMap(Optional::stream)
- .collect(Collectors.toSet()))
+ ? fileExpandedOwners
: Maps.transformValues(
owners.getFileGroupOwners(),
groupNames ->
groupNames.stream().map(GroupOwner::new).collect(Collectors.toSet()));
- return Response.ok(new FilesOwnersResponse(getLabels(id), fileToOwners));
+ Map<Integer, Map<String, Integer>> ownersLabels = getLabels(change.getChangeId());
+
+ LabelAndScore label = getLabelDefinition(owners, changeData);
+
+ Map<String, Set<GroupOwner>> filesWithPendingOwners =
+ Maps.filterEntries(
+ fileToOwners,
+ (fileOwnerEntry) ->
+ !isApprovedByOwner(
+ fileExpandedOwners.get(fileOwnerEntry.getKey()), ownersLabels, label));
+
+ return Response.ok(new FilesOwnersResponse(ownersLabels, filesWithPendingOwners));
}
}
+ private LabelAndScore getLabelDefinition(PathOwners owners, ChangeData changeData)
+ throws ResourceNotFoundException {
+
+ try {
+ return Optional.of(pluginSettings.enableSubmitRequirement())
+ .filter(Boolean::booleanValue)
+ .flatMap(enabled -> getLabelFromOwners(owners, changeData))
+ .orElseGet(
+ () ->
+ new LabelAndScore(
+ LabelId.CODE_REVIEW, getMaxScoreForLabel(changeData, LabelId.CODE_REVIEW)));
+ } catch (LabelNotFoundException e) {
+ logger.atInfo().withCause(e).log("Invalid configuration");
+ throw new ResourceNotFoundException(MISSING_CODE_REVIEW_LABEL, e);
+ }
+ }
+
+ private Optional<LabelAndScore> getLabelFromOwners(PathOwners owners, ChangeData changeData)
+ throws LabelNotFoundException {
+ return owners
+ .getLabel()
+ .map(
+ label ->
+ new LabelAndScore(
+ label.getName(),
+ label
+ .getScore()
+ .orElseGet(() -> getMaxScoreForLabel(changeData, label.getName()))));
+ }
+
+ private short getMaxScoreForLabel(ChangeData changeData, String labelId)
+ throws LabelNotFoundException {
+ return changeData
+ .getLabelTypes()
+ .byLabel(labelId)
+ .map(label -> label.getMaxPositive())
+ .orElseThrow(() -> new LabelNotFoundException(changeData.change().getProject(), labelId));
+ }
+
+ private boolean isApprovedByOwner(
+ Set<GroupOwner> fileOwners,
+ Map<Integer, Map<String, Integer>> ownersLabels,
+ LabelAndScore label) {
+ return fileOwners.stream()
+ .filter(owner -> owner instanceof Owner)
+ .map(owner -> ((Owner) owner).getId())
+ .flatMap(ownerId -> codeReviewLabelValue(ownersLabels, ownerId, label.getLabelId()))
+ .anyMatch(value -> value >= label.getScore());
+ }
+
+ private Stream<Integer> codeReviewLabelValue(
+ Map<Integer, Map<String, Integer>> ownersLabels, int ownerId, String labelId) {
+ return Stream.ofNullable(ownersLabels.get(ownerId))
+ .flatMap(m -> Stream.ofNullable(m.get(labelId)));
+ }
+
/**
* This method returns ta Map representing the "owners_labels" object of the response. When
* serialized the Map, has to to return the following JSON: the following JSON:
@@ -184,4 +251,30 @@
.get(accountId)
.map(as -> new Owner(as.account().fullName(), as.account().id().get()));
}
+
+ static class LabelNotFoundException extends RuntimeException {
+ private static final long serialVersionUID = 1L;
+
+ LabelNotFoundException(Project.NameKey project, String labelId) {
+ super(String.format("Project %s has no %s label defined", project, labelId));
+ }
+ }
+
+ private static class LabelAndScore {
+ private final String labelId;
+ private final short score;
+
+ private LabelAndScore(String labelId, short score) {
+ this.labelId = labelId;
+ this.score = score;
+ }
+
+ private String getLabelId() {
+ return labelId;
+ }
+
+ private short getScore() {
+ return score;
+ }
+ }
}
diff --git a/owners/src/main/java/gerrit_owners/PRED_code_review_user_1.java b/owners/src/main/java/gerrit_owners/PRED_code_review_user_1.java
new file mode 100644
index 0000000..a2dd06b
--- /dev/null
+++ b/owners/src/main/java/gerrit_owners/PRED_code_review_user_1.java
@@ -0,0 +1,117 @@
+// Copyright (C) 2023 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 gerrit_owners;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.LabelValue;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.rules.StoredValues;
+import com.googlecode.prolog_cafe.exceptions.PrologException;
+import com.googlecode.prolog_cafe.lang.IntegerTerm;
+import com.googlecode.prolog_cafe.lang.JavaObjectTerm;
+import com.googlecode.prolog_cafe.lang.Operation;
+import com.googlecode.prolog_cafe.lang.Predicate;
+import com.googlecode.prolog_cafe.lang.Prolog;
+import com.googlecode.prolog_cafe.lang.Term;
+import java.util.Iterator;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+/** 'code_review_user'(-User) */
+public class PRED_code_review_user_1 extends Predicate.P1 {
+
+ private static final PRED_code_review_user_check CODE_REVIEW_USER_CHECK =
+ new PRED_code_review_user_check();
+ private static final PRED_code_review_user_empty CODE_REVIEW_USER_EMPTY =
+ new PRED_code_review_user_empty();
+ private static final PRED_code_review_user_next CODE_REVIEW_USER_NEXT =
+ new PRED_code_review_user_next();
+
+ public PRED_code_review_user_1(Term a1, Operation n) {
+ this.arg1 = a1;
+ this.cont = n;
+ }
+
+ @Override
+ public Operation exec(Prolog engine) throws PrologException {
+ engine.cont = cont;
+ engine.setB0();
+
+ ChangeData cd = StoredValues.CHANGE_DATA.get(engine);
+ Optional<LabelValue> codeReviewMaxValue =
+ cd.getLabelTypes().byLabel(LabelId.CODE_REVIEW).map(LabelType::getMax);
+
+ Iterator<Account.Id> approvalsAccounts =
+ cd.currentApprovals().stream()
+ .filter(a -> LabelId.CODE_REVIEW.equalsIgnoreCase(a.labelId().get()))
+ .filter(a -> codeReviewMaxValue.filter(val -> val.getValue() == a.value()).isPresent())
+ .map(a -> a.accountId())
+ .collect(Collectors.toList())
+ .iterator();
+
+ engine.r1 = arg1;
+ engine.r2 = new JavaObjectTerm(approvalsAccounts);
+
+ return engine.jtry2(CODE_REVIEW_USER_CHECK, CODE_REVIEW_USER_NEXT);
+ }
+
+ private static class PRED_code_review_user_check extends Operation {
+
+ @Override
+ public Operation exec(Prolog engine) throws PrologException {
+ Term a1 = engine.r1;
+ Term a2 = engine.r2;
+
+ @SuppressWarnings("unchecked")
+ Iterator<Account.Id> iter = (Iterator<Account.Id>) ((JavaObjectTerm) a2).object();
+ while (iter.hasNext()) {
+ Account.Id accountId = iter.next();
+ IntegerTerm accountIdTerm = new IntegerTerm(accountId.get());
+ if (!a1.unify(accountIdTerm, engine.trail)) {
+ continue;
+ }
+ return engine.cont;
+ }
+ return engine.fail();
+ }
+ }
+
+ private static class PRED_code_review_user_next extends Operation {
+
+ @Override
+ public Operation exec(Prolog engine) throws PrologException {
+ return engine.trust(CODE_REVIEW_USER_EMPTY);
+ }
+ }
+
+ private static class PRED_code_review_user_empty extends Operation {
+
+ @Override
+ public Operation exec(Prolog engine) throws PrologException {
+ Term a2 = engine.r2;
+
+ @SuppressWarnings("unchecked")
+ Iterator<Account.Id> iter = (Iterator<Account.Id>) ((JavaObjectTerm) a2).object();
+ if (!iter.hasNext()) {
+ return engine.fail();
+ }
+
+ return engine.jtry2(CODE_REVIEW_USER_CHECK, CODE_REVIEW_USER_NEXT);
+ }
+ }
+}
diff --git a/owners/src/main/prolog/gerrit_owners.pl b/owners/src/main/prolog/gerrit_owners.pl
index 1fe2a22..5fba7e7 100644
--- a/owners/src/main/prolog/gerrit_owners.pl
+++ b/owners/src/main/prolog/gerrit_owners.pl
@@ -38,8 +38,8 @@
add_owner_approval(_, In, Out) :- In = Out.
owner_approved(Path) :-
- owner(Path, User),
- gerrit:commit_label(label('Code-Review', 2), User),
+ owner(Path, user(US)),
+ code_review_user(US),
!.
owner_approved(Users, Path) :-
@@ -56,9 +56,6 @@
findall(US,code_review_user(US),Approvers),
matcher_needed(Approvers,F,FileAndUser).
-code_review_user(U) :-
- gerrit:commit_label(label('Code-Review', 2), user(U)).
-
% this loops over all the paths and if for any
% we have some labels generated then add a single
% Owner-Code-Review need to block submit button
diff --git a/owners/src/main/resources/Documentation/rest-api.md b/owners/src/main/resources/Documentation/rest-api.md
index 4fb4285..68fb463 100644
--- a/owners/src/main/resources/Documentation/rest-api.md
+++ b/owners/src/main/resources/Documentation/rest-api.md
@@ -1,7 +1,7 @@
# Rest API
-The @PLUGIN@ exposes a Rest API endpoint to list the owners associated to each file and, for each owner,
-its associated labels and votes:
+The @PLUGIN@ exposes a Rest API endpoint to list the owners associated to each file that
+needs a review, and, for each owner, its current labels and votes:
```bash
GET /changes/{change-id}/revisions/{revision-id}/owners~files-owners
@@ -29,4 +29,7 @@
}
}
-```
\ No newline at end of file
+```
+
+> __NOTE__: The API does not work in the case when custom label is in
+> rules.pl configuration as described in [the config.md docs](https://gerrit.googlesource.com/plugins/owners/+/refs/heads/stable-3.4/owners/src/main/resources/Documentation/config.md#example-3-owners-file-without-matchers-and-custom-owner_approves-label)
\ No newline at end of file
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
index 9a65c45..55bfa3c 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersApprovalHasOperandIT.java
@@ -21,6 +21,8 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.acceptance.config.GlobalPluginConfig;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
@@ -35,7 +37,9 @@
import org.junit.Before;
import org.junit.Test;
-public class OwnersApprovalHasOperandIT extends OwnersSubmitRequirementIT {
+@TestPlugin(name = "owners", sysModule = "com.googlesource.gerrit.owners.OwnersModule")
+@UseLocalDisk
+public class OwnersApprovalHasOperandIT extends OwnersSubmitRequirementITAbstract {
private static final String REQUIREMENT_NAME = "Owner-Approval";
@Before
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
index 002dc4b..0d5c2c5 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementIT.java
@@ -15,444 +15,9 @@
package com.googlesource.gerrit.owners;
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth8.assertThat;
-import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
-import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
-import static com.google.gerrit.server.project.testing.TestLabels.value;
-import static java.util.stream.Collectors.joining;
-
-import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
-import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
-import com.google.gerrit.acceptance.config.GlobalPluginConfig;
-import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
-import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
-import com.google.gerrit.entities.LabelFunction;
-import com.google.gerrit.entities.LabelId;
-import com.google.gerrit.entities.LabelType;
-import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.extensions.api.changes.ChangeApi;
-import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.common.LegacySubmitRequirementInfo;
-import com.google.gerrit.extensions.common.SubmitRecordInfo;
-import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.project.testing.TestLabels;
-import com.google.inject.Inject;
-import com.googlesource.gerrit.owners.common.LabelDefinition;
-import java.util.Collection;
-import java.util.stream.Stream;
-import org.junit.Test;
@TestPlugin(name = "owners", sysModule = "com.googlesource.gerrit.owners.OwnersModule")
@UseLocalDisk
-public class OwnersSubmitRequirementIT extends LightweightPluginDaemonTest {
- private static final LegacySubmitRequirementInfo NOT_READY =
- new LegacySubmitRequirementInfo("NOT_READY", "Owners", "owners");
- private static final LegacySubmitRequirementInfo READY =
- new LegacySubmitRequirementInfo("OK", "Owners", "owners");
-
- @Inject protected RequestScopeOperations requestScopeOperations;
- @Inject private ProjectOperations projectOperations;
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldRequireAtLeastOneApprovalForMatchingPathFromOwner() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- TestAccount user1 = accountCreator.user1();
- addOwnerFileWithMatchersToRoot(true, ".md", admin2, user1);
-
- PushOneCommit.Result r = createChange("Add a file", "README.md", "foo");
- ChangeApi changeApi = forChange(r);
- ChangeInfo changeNotReady = changeApi.get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
-
- changeApi.current().review(ReviewInput.approve());
- ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
- assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
- assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(ReviewInput.approve());
- ChangeInfo changeReady = forChange(r).get();
- assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldNotRequireApprovalForNotMatchingPath() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileWithMatchersToRoot(true, ".md", admin2);
-
- PushOneCommit.Result r = createChange("Add a file", "README.txt", "foo");
- ChangeApi changeApi = forChange(r);
- ChangeInfo changeNotReady = changeApi.get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).isEmpty();
-
- changeApi.current().review(ReviewInput.approve());
- ChangeInfo changeReady = changeApi.get();
- assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).isEmpty();
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldRequireApprovalFromRootOwner() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileToRoot(true, admin2);
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- ChangeInfo changeNotReady = changeApi.get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
-
- changeApi.current().review(ReviewInput.approve());
- ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
- assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
- assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(ReviewInput.approve());
- ChangeInfo changeReady = forChange(r).get();
- assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldBlockOwnersApprovalForMaxNegativeVote() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileToRoot(true, admin2);
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- ChangeInfo changeNotReady = changeApi.get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(ReviewInput.approve());
- ChangeInfo changeReady = forChange(r).get();
- assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
-
- changeApi.current().review(ReviewInput.reject());
- assertThat(forChange(r).get().submittable).isFalse();
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldRequireVerifiedApprovalEvenIfCodeOwnerApproved() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileToRoot(true, admin2);
-
- installVerifiedLabel();
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- assertThat(changeApi.get().submittable).isFalse();
- assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(ReviewInput.approve());
- assertThat(forChange(r).get().submittable).isFalse();
- assertThat(forChange(r).get().requirements).containsExactly(READY);
- verifyHasSubmitRecord(
- forChange(r).get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.NEED);
-
- changeApi.current().review(new ReviewInput().label(LabelId.VERIFIED, 1));
- assertThat(changeApi.get().submittable).isTrue();
- verifyHasSubmitRecord(
- changeApi.get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldRequireCodeOwnerApprovalEvenIfVerifiedWasApproved() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileToRoot(true, admin2);
-
- installVerifiedLabel();
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- assertThat(changeApi.get().submittable).isFalse();
- assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(new ReviewInput().label(LabelId.VERIFIED, 1));
- ChangeInfo changeNotReady = forChange(r).get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
- verifyHasSubmitRecord(
- changeNotReady.submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK);
-
- forChange(r).current().review(ReviewInput.approve());
- ChangeInfo changeReady = forChange(r).get();
- assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldRequireConfiguredLabelByCodeOwner() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- String labelId = "Foo";
- addOwnerFileToRoot(true, LabelDefinition.parse(labelId).get(), admin2);
-
- installLabel(labelId);
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- assertThat(changeApi.get().submittable).isFalse();
- assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
-
- changeApi.current().review(ReviewInput.approve());
- ChangeInfo changeStillNotReady = changeApi.get();
- assertThat(changeStillNotReady.submittable).isFalse();
- assertThat(changeStillNotReady.requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(new ReviewInput().label(labelId, 1));
- ChangeInfo changeReady = forChange(r).get();
- assertThat(changeReady.submittable).isTrue();
- assertThat(changeReady.requirements).containsExactly(READY);
- verifyHasSubmitRecord(changeReady.submitRecords, labelId, SubmitRecordInfo.Label.Status.OK);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldRequireConfiguredLabelByCodeOwnerEvenIfItIsNotConfiguredForProject()
- throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- String notExistinglabelId = "Foo";
- addOwnerFileToRoot(true, LabelDefinition.parse(notExistinglabelId).get(), admin2);
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- assertThat(changeApi.get().submittable).isFalse();
- assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
-
- changeApi.current().review(ReviewInput.approve());
- ChangeInfo changeStillNotReady = changeApi.get();
- assertThat(changeStillNotReady.submittable).isFalse();
- assertThat(changeStillNotReady.requirements).containsExactly(NOT_READY);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldRequireConfiguredLabelScoreByCodeOwner() throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- ChangeInfo changeNotReady = changeApi.get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
-
- changeApi.current().review(ReviewInput.approve());
- ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
- assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
- assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(ReviewInput.recommend());
- ChangeInfo changeReadyWithOwnerScore = forChange(r).get();
- assertThat(changeReadyWithOwnerScore.submittable).isTrue();
- assertThat(changeReadyWithOwnerScore.requirements).containsExactly(READY);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldConfiguredLabelScoreByCodeOwnerBeNotSufficientIfLabelRequiresMaxValue()
- throws Exception {
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- ChangeInfo changeNotReady = changeApi.get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(ReviewInput.recommend());
- ChangeInfo ownersVoteNotSufficient = changeApi.get();
- assertThat(ownersVoteNotSufficient.submittable).isFalse();
- assertThat(ownersVoteNotSufficient.requirements).containsExactly(READY);
- verifyHasSubmitRecord(
- ownersVoteNotSufficient.submitRecords,
- LabelId.CODE_REVIEW,
- SubmitRecordInfo.Label.Status.NEED);
-
- requestScopeOperations.setApiUser(admin.id());
- forChange(r).current().review(ReviewInput.approve());
- ChangeInfo changeReadyWithMaxScore = forChange(r).get();
- assertThat(changeReadyWithMaxScore.submittable).isTrue();
- assertThat(changeReadyWithMaxScore.requirements).containsExactly(READY);
- verifyHasSubmitRecord(
- changeReadyWithMaxScore.submitRecords,
- LabelId.CODE_REVIEW,
- SubmitRecordInfo.Label.Status.OK);
- }
-
- @Test
- @GlobalPluginConfig(
- pluginName = "owners",
- name = "owners.enableSubmitRequirement",
- value = "true")
- public void shouldConfiguredLabelScoreByCodeOwnersOverwriteSubmitRequirement() throws Exception {
- installLabel(TestLabels.codeReview().toBuilder().setFunction(LabelFunction.NO_OP).build());
-
- TestAccount admin2 = accountCreator.admin2();
- addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
-
- PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
- ChangeApi changeApi = forChange(r);
- ChangeInfo changeNotReady = changeApi.get();
- assertThat(changeNotReady.submittable).isFalse();
- assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
-
- requestScopeOperations.setApiUser(admin2.id());
- forChange(r).current().review(ReviewInput.recommend());
- ChangeInfo ownersVoteSufficient = forChange(r).get();
- assertThat(ownersVoteSufficient.submittable).isTrue();
- assertThat(ownersVoteSufficient.requirements).containsExactly(READY);
- }
-
- private void verifyHasSubmitRecord(
- Collection<SubmitRecordInfo> records, String label, SubmitRecordInfo.Label.Status status) {
- assertThat(
- records.stream()
- .flatMap(record -> record.labels.stream())
- .filter(l -> l.label.equals(label) && l.status == status)
- .findAny())
- .isPresent();
- }
-
- private void installVerifiedLabel() throws Exception {
- installLabel(LabelId.VERIFIED);
- }
-
- private void installLabel(String labelId) throws Exception {
- LabelType verified =
- labelBuilder(labelId, value(1, "Verified"), value(0, "No score"), value(-1, "Fails"))
- .setFunction(LabelFunction.MAX_WITH_BLOCK)
- .build();
-
- installLabel(verified);
-
- String heads = RefNames.REFS_HEADS + "*";
- projectOperations
- .project(project)
- .forUpdate()
- .add(allowLabel(verified.getName()).ref(heads).group(REGISTERED_USERS).range(-1, 1))
- .update();
- }
-
- private void installLabel(LabelType label) throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig().upsertLabelType(label);
- u.save();
- }
- }
-
- protected ChangeApi forChange(PushOneCommit.Result r) throws RestApiException {
- return gApi.changes().id(r.getChangeId());
- }
-
- private void addOwnerFileWithMatchersToRoot(
- boolean inherit, String extension, TestAccount... users) throws Exception {
- // Add OWNERS file to root:
- //
- // inherited: true
- // matchers:
- // - suffix: extension
- // owners:
- // - u1.email()
- // - ...
- // - uN.email()
- pushOwnersToMaster(
- String.format(
- "inherited: %s\nmatchers:\n" + "- suffix: %s\n owners:\n%s",
- inherit,
- extension,
- Stream.of(users)
- .map(user -> String.format(" - %s\n", user.email()))
- .collect(joining())));
- }
-
- private void addOwnerFileToRoot(boolean inherit, TestAccount u) throws Exception {
- // Add OWNERS file to root:
- //
- // inherited: true
- // owners:
- // - u.email()
- pushOwnersToMaster(String.format("inherited: %s\nowners:\n- %s\n", inherit, u.email()));
- }
-
- protected void addOwnerFileToRoot(boolean inherit, LabelDefinition label, TestAccount u)
- throws Exception {
- // Add OWNERS file to root:
- //
- // inherited: true
- // label: label,score # score is optional
- // owners:
- // - u.email()
- pushOwnersToMaster(
- String.format(
- "inherited: %s\nlabel: %s\nowners:\n- %s\n",
- inherit,
- String.format(
- "%s%s",
- label.getName(),
- label.getScore().map(value -> String.format(",%d", value)).orElse("")),
- u.email()));
- }
-
- private void pushOwnersToMaster(String owners) throws Exception {
- pushFactory
- .create(admin.newIdent(), testRepo, "Add OWNER file", "OWNERS", owners)
- .to(RefNames.fullName("master"))
- .assertOkStatus();
- }
-}
+public class OwnersSubmitRequirementIT extends OwnersSubmitRequirementITAbstract {}
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
new file mode 100644
index 0000000..d7cea60
--- /dev/null
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
@@ -0,0 +1,515 @@
+// Copyright (C) 2023 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.googlesource.gerrit.owners;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.server.project.testing.TestLabels.labelBuilder;
+import static com.google.gerrit.server.project.testing.TestLabels.value;
+import static java.util.stream.Collectors.joining;
+
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.config.GlobalPluginConfig;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.LabelFunction;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.LegacySubmitRequirementInfo;
+import com.google.gerrit.extensions.common.SubmitRecordInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.project.testing.TestLabels;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.owners.common.LabelDefinition;
+import java.util.Collection;
+import java.util.stream.Stream;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.junit.Test;
+
+abstract class OwnersSubmitRequirementITAbstract extends LightweightPluginDaemonTest {
+ private static final LegacySubmitRequirementInfo NOT_READY =
+ new LegacySubmitRequirementInfo("NOT_READY", "Owners", "owners");
+ private static final LegacySubmitRequirementInfo READY =
+ new LegacySubmitRequirementInfo("OK", "Owners", "owners");
+
+ @Inject protected RequestScopeOperations requestScopeOperations;
+ @Inject private ProjectOperations projectOperations;
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireAtLeastOneApprovalForMatchingPathFromOwner() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ TestAccount user1 = accountCreator.user1();
+ addOwnerFileWithMatchersToRoot(true, ".md", admin2, user1);
+
+ PushOneCommit.Result r = createChange("Add a file", "README.md", "foo");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+
+ changeApi.current().review(ReviewInput.approve());
+ ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
+ assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
+ assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.approve());
+ ChangeInfo changeReady = forChange(r).get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).containsExactly(READY);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldNotRequireApprovalForNotMatchingPath() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileWithMatchersToRoot(true, ".md", admin2);
+
+ PushOneCommit.Result r = createChange("Add a file", "README.txt", "foo");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).isEmpty();
+
+ changeApi.current().review(ReviewInput.approve());
+ ChangeInfo changeReady = changeApi.get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).isEmpty();
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireApprovalFromRootOwner() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, admin2);
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+
+ changeApi.current().review(ReviewInput.approve());
+ ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
+ assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
+ assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.approve());
+ ChangeInfo changeReady = forChange(r).get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).containsExactly(READY);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldBlockOwnersApprovalForMaxNegativeVote() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, admin2);
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.approve());
+ ChangeInfo changeReady = forChange(r).get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).containsExactly(READY);
+
+ changeApi.current().review(ReviewInput.reject());
+ assertThat(forChange(r).get().submittable).isFalse();
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireVerifiedApprovalEvenIfCodeOwnerApproved() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, admin2);
+
+ installVerifiedLabel();
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ assertThat(changeApi.get().submittable).isFalse();
+ assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.approve());
+ assertThat(forChange(r).get().submittable).isFalse();
+ assertThat(forChange(r).get().requirements).containsExactly(READY);
+ verifyHasSubmitRecord(
+ forChange(r).get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.NEED);
+
+ changeApi.current().review(new ReviewInput().label(LabelId.VERIFIED, 1));
+ assertThat(changeApi.get().submittable).isTrue();
+ verifyHasSubmitRecord(
+ changeApi.get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireCodeOwnerApprovalEvenIfVerifiedWasApproved() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, admin2);
+
+ installVerifiedLabel();
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ assertThat(changeApi.get().submittable).isFalse();
+ assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(new ReviewInput().label(LabelId.VERIFIED, 1));
+ ChangeInfo changeNotReady = forChange(r).get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+ verifyHasSubmitRecord(
+ changeNotReady.submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK);
+
+ forChange(r).current().review(ReviewInput.approve());
+ ChangeInfo changeReady = forChange(r).get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).containsExactly(READY);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireConfiguredLabelByCodeOwner() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ String labelId = "Foo";
+ addOwnerFileToRoot(true, LabelDefinition.parse(labelId).get(), admin2);
+
+ installLabel(labelId);
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ assertThat(changeApi.get().submittable).isFalse();
+ assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
+
+ changeApi.current().review(ReviewInput.approve());
+ ChangeInfo changeStillNotReady = changeApi.get();
+ assertThat(changeStillNotReady.submittable).isFalse();
+ assertThat(changeStillNotReady.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(new ReviewInput().label(labelId, 1));
+ ChangeInfo changeReady = forChange(r).get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).containsExactly(READY);
+ verifyHasSubmitRecord(changeReady.submitRecords, labelId, SubmitRecordInfo.Label.Status.OK);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireConfiguredLabelByCodeOwnerEvenIfItIsNotConfiguredForProject()
+ throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ String notExistinglabelId = "Foo";
+ addOwnerFileToRoot(true, LabelDefinition.parse(notExistinglabelId).get(), admin2);
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ assertThat(changeApi.get().submittable).isFalse();
+ assertThat(changeApi.get().requirements).containsExactly(NOT_READY);
+
+ changeApi.current().review(ReviewInput.approve());
+ ChangeInfo changeStillNotReady = changeApi.get();
+ assertThat(changeStillNotReady.submittable).isFalse();
+ assertThat(changeStillNotReady.requirements).containsExactly(NOT_READY);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireConfiguredLabelScoreByCodeOwner() throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+
+ changeApi.current().review(ReviewInput.approve());
+ ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
+ assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
+ assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.recommend());
+ ChangeInfo changeReadyWithOwnerScore = forChange(r).get();
+ assertThat(changeReadyWithOwnerScore.submittable).isTrue();
+ assertThat(changeReadyWithOwnerScore.requirements).containsExactly(READY);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldConfiguredLabelScoreByCodeOwnerBeNotSufficientIfLabelRequiresMaxValue()
+ throws Exception {
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.recommend());
+ ChangeInfo ownersVoteNotSufficient = changeApi.get();
+ assertThat(ownersVoteNotSufficient.submittable).isFalse();
+ assertThat(ownersVoteNotSufficient.requirements).containsExactly(READY);
+ verifyHasSubmitRecord(
+ ownersVoteNotSufficient.submitRecords,
+ LabelId.CODE_REVIEW,
+ SubmitRecordInfo.Label.Status.NEED);
+
+ requestScopeOperations.setApiUser(admin.id());
+ forChange(r).current().review(ReviewInput.approve());
+ ChangeInfo changeReadyWithMaxScore = forChange(r).get();
+ assertThat(changeReadyWithMaxScore.submittable).isTrue();
+ assertThat(changeReadyWithMaxScore.requirements).containsExactly(READY);
+ verifyHasSubmitRecord(
+ changeReadyWithMaxScore.submitRecords,
+ LabelId.CODE_REVIEW,
+ SubmitRecordInfo.Label.Status.OK);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldConfiguredLabelScoreByCodeOwnersOverwriteSubmitRequirement() throws Exception {
+ installLabel(TestLabels.codeReview().toBuilder().setFunction(LabelFunction.NO_OP).build());
+
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRoot(true, LabelDefinition.parse("Code-Review,1").get(), admin2);
+
+ PushOneCommit.Result r = createChange("Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.recommend());
+ ChangeInfo ownersVoteSufficient = forChange(r).get();
+ assertThat(ownersVoteSufficient.submittable).isTrue();
+ assertThat(ownersVoteSufficient.requirements).containsExactly(READY);
+ }
+
+ @Test
+ @GlobalPluginConfig(
+ pluginName = "owners",
+ name = "owners.enableSubmitRequirement",
+ value = "true")
+ public void shouldRequireApprovalFromGrandParentProjectOwner() throws Exception {
+ Project.NameKey parentProjectName =
+ createProjectOverAPI("parent", allProjects, true, SubmitType.FAST_FORWARD_ONLY);
+ Project.NameKey childProjectName =
+ createProjectOverAPI("child", parentProjectName, true, SubmitType.FAST_FORWARD_ONLY);
+ TestRepository<InMemoryRepository> childRepo = cloneProject(childProjectName);
+
+ TestAccount admin2 = accountCreator.admin2();
+ addOwnerFileToRefsMetaConfig(true, admin2, allProjects);
+
+ PushOneCommit.Result r =
+ createCommitAndPush(childRepo, "refs/for/master", "Add a file", "foo", "bar");
+ ChangeApi changeApi = forChange(r);
+ ChangeInfo changeNotReady = changeApi.get();
+ assertThat(changeNotReady.submittable).isFalse();
+ assertThat(changeNotReady.requirements).containsExactly(NOT_READY);
+
+ changeApi.current().review(ReviewInput.approve());
+ ChangeInfo changeNotReadyAfterSelfApproval = changeApi.get();
+ assertThat(changeNotReadyAfterSelfApproval.submittable).isFalse();
+ assertThat(changeNotReadyAfterSelfApproval.requirements).containsExactly(NOT_READY);
+
+ requestScopeOperations.setApiUser(admin2.id());
+ forChange(r).current().review(ReviewInput.approve());
+ ChangeInfo changeReady = forChange(r).get();
+ assertThat(changeReady.submittable).isTrue();
+ assertThat(changeReady.requirements).containsExactly(READY);
+ }
+
+ private void verifyHasSubmitRecord(
+ Collection<SubmitRecordInfo> records, String label, SubmitRecordInfo.Label.Status status) {
+ assertThat(
+ records.stream()
+ .flatMap(record -> record.labels.stream())
+ .filter(l -> l.label.equals(label) && l.status == status)
+ .findAny())
+ .isPresent();
+ }
+
+ private void installVerifiedLabel() throws Exception {
+ installLabel(LabelId.VERIFIED);
+ }
+
+ private void installLabel(String labelId) throws Exception {
+ LabelType verified =
+ labelBuilder(labelId, value(1, "Verified"), value(0, "No score"), value(-1, "Fails"))
+ .setFunction(LabelFunction.MAX_WITH_BLOCK)
+ .build();
+
+ installLabel(verified);
+
+ String heads = RefNames.REFS_HEADS + "*";
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(verified.getName()).ref(heads).group(REGISTERED_USERS).range(-1, 1))
+ .update();
+ }
+
+ private void installLabel(LabelType label) throws Exception {
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig().upsertLabelType(label);
+ u.save();
+ }
+ }
+
+ protected ChangeApi forChange(PushOneCommit.Result r) throws RestApiException {
+ return gApi.changes().id(r.getChangeId());
+ }
+
+ private void addOwnerFileWithMatchersToRoot(
+ boolean inherit, String extension, TestAccount... users) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // matchers:
+ // - suffix: extension
+ // owners:
+ // - u1.email()
+ // - ...
+ // - uN.email()
+ pushOwnersToMaster(
+ String.format(
+ "inherited: %s\nmatchers:\n" + "- suffix: %s\n owners:\n%s",
+ inherit,
+ extension,
+ Stream.of(users)
+ .map(user -> String.format(" - %s\n", user.email()))
+ .collect(joining())));
+ }
+
+ private void addOwnerFileToRoot(boolean inherit, TestAccount u) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // owners:
+ // - u.email()
+ pushOwnersToMaster(String.format("inherited: %s\nowners:\n- %s\n", inherit, u.email()));
+ }
+
+ private void addOwnerFileToRefsMetaConfig(
+ boolean inherit, TestAccount u, Project.NameKey projectName) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // owners:
+ // - u.email()
+ pushOwnersToRefsMetaConfig(
+ String.format("inherited: %s\nowners:\n- %s\n", inherit, u.email()), projectName);
+ }
+
+ protected void addOwnerFileToRoot(boolean inherit, LabelDefinition label, TestAccount u)
+ throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // label: label,score # score is optional
+ // owners:
+ // - u.email()
+ pushOwnersToMaster(
+ String.format(
+ "inherited: %s\nlabel: %s\nowners:\n- %s\n",
+ inherit,
+ String.format(
+ "%s%s",
+ label.getName(),
+ label.getScore().map(value -> String.format(",%d", value)).orElse("")),
+ u.email()));
+ }
+
+ private void pushOwnersToMaster(String owners) throws Exception {
+ pushFactory
+ .create(admin.newIdent(), testRepo, "Add OWNER file", "OWNERS", owners)
+ .to(RefNames.fullName("master"))
+ .assertOkStatus();
+ }
+
+ private void pushOwnersToRefsMetaConfig(String owners, Project.NameKey projectName)
+ throws Exception {
+ TestRepository<InMemoryRepository> project = cloneProject(projectName);
+ GitUtil.fetch(project, RefNames.REFS_CONFIG + ":" + RefNames.REFS_CONFIG);
+ project.reset(RefNames.REFS_CONFIG);
+ pushFactory
+ .create(admin.newIdent(), project, "Add OWNER file", "OWNERS", owners)
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+ }
+}
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
index ec3e816..76c4962 100644
--- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
@@ -15,196 +15,9 @@
package com.googlesource.gerrit.owners.restapi;
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.acceptance.GitUtil;
-import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.TestPlugin;
import com.google.gerrit.acceptance.UseLocalDisk;
-import com.google.gerrit.acceptance.config.GlobalPluginConfig;
-import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.extensions.restapi.Response;
-import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
-import com.googlesource.gerrit.owners.entities.GroupOwner;
-import com.googlesource.gerrit.owners.entities.Owner;
-import javax.servlet.http.HttpServletResponse;
-import org.apache.commons.compress.utils.Sets;
-import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
-import org.eclipse.jgit.junit.TestRepository;
-import org.junit.Test;
@TestPlugin(name = "owners", sysModule = "com.googlesource.gerrit.owners.OwnersModule")
@UseLocalDisk
-public class GetFilesOwnersIT extends LightweightPluginDaemonTest {
-
- private GetFilesOwners ownersApi;
- private Owner rootOwner;
- private Owner projectOwner;
-
- @Override
- public void setUpTestPlugin() throws Exception {
- super.setUpTestPlugin();
-
- rootOwner = new Owner(admin.fullName(), admin.id().get());
- projectOwner = new Owner(user.fullName(), user.id().get());
- ownersApi = plugin.getSysInjector().getInstance(GetFilesOwners.class);
- }
-
- @Test
- public void shouldReturnExactFileOwners() throws Exception {
- addOwnerFileToRoot(true);
- String changeId = createChange().getChangeId();
-
- Response<FilesOwnersResponse> resp =
- assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
-
- assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner));
- }
-
- @Test
- public void shouldReturnOwnersLabels() throws Exception {
- addOwnerFileToRoot(true);
- String changeId = createChange().getChangeId();
- approve(changeId);
-
- Response<FilesOwnersResponse> resp =
- assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
-
- assertThat(resp.value().ownersLabels)
- .containsExactly(admin.id().get(), ImmutableMap.builder().put("Code-Review", 2).build());
- }
-
- @Test
- @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
- public void shouldReturnResponseWithUnexpandedFileOwners() throws Exception {
- addOwnerFileToRoot(true);
- String changeId = createChange().getChangeId();
-
- Response<FilesOwnersResponse> resp =
- assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
-
- assertThat(resp.value().files)
- .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
- }
-
- @Test
- @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
- public void shouldReturnResponseWithUnexpandedFileMatchersOwners() throws Exception {
- addOwnerFileWithMatchersToRoot(true);
- String changeId = createChange().getChangeId();
-
- Response<FilesOwnersResponse> resp =
- assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
-
- assertThat(resp.value().files)
- .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
- }
-
- @Test
- @UseLocalDisk
- public void shouldReturnInheritedOwnersFromProjectsOwners() throws Exception {
- assertInheritFromProject(project);
- }
-
- @Test
- @UseLocalDisk
- public void shouldReturnInheritedOwnersFromParentProjectsOwners() throws Exception {
- assertInheritFromProject(allProjects);
- }
-
- @Test
- @UseLocalDisk
- public void shouldNotReturnInheritedOwnersFromProjectsOwners() throws Exception {
- assertNotInheritFromProject(project);
- }
-
- @Test
- @UseLocalDisk
- public void shouldNotReturnInheritedOwnersFromParentProjectsOwners() throws Exception {
- addOwnerFileToProjectConfig(project, false);
- assertNotInheritFromProject(allProjects);
- }
-
- private static <T> Response<T> assertResponseOk(Response<T> response) {
- assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
- return response;
- }
-
- private void assertNotInheritFromProject(Project.NameKey projectNameKey) throws Exception {
- addOwnerFileToRoot(false);
- addOwnerFileToProjectConfig(projectNameKey, true);
-
- String changeId = createChange().getChangeId();
- Response<FilesOwnersResponse> resp =
- assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
-
- assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner));
- }
-
- private void assertInheritFromProject(Project.NameKey projectNameKey) throws Exception {
- addOwnerFileToRoot(true);
- addOwnerFileToProjectConfig(projectNameKey, true);
-
- String changeId = createChange().getChangeId();
- Response<FilesOwnersResponse> resp =
- assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
-
- assertThat(resp.value().files)
- .containsExactly("a.txt", Sets.newHashSet(rootOwner, projectOwner));
- }
-
- private void addOwnerFileToProjectConfig(Project.NameKey projectNameKey, boolean inherit)
- throws Exception {
- TestRepository<InMemoryRepository> project = cloneProject(projectNameKey);
- GitUtil.fetch(project, RefNames.REFS_CONFIG + ":" + RefNames.REFS_CONFIG);
- project.reset(RefNames.REFS_CONFIG);
- pushFactory
- .create(
- admin.newIdent(),
- project,
- "Add OWNER file",
- "OWNERS",
- String.format(
- "inherited: %s\nmatchers:\n" + "- suffix: .txt\n owners:\n - %s\n",
- inherit, user.email()))
- .to(RefNames.REFS_CONFIG);
- }
-
- private void addOwnerFileToRoot(boolean inherit) throws Exception {
- // Add OWNERS file to root:
- //
- // inherited: true
- // owners:
- // - admin
- merge(
- createChange(
- testRepo,
- "master",
- "Add OWNER file",
- "OWNERS",
- String.format("inherited: %s\nowners:\n- %s\n", inherit, admin.email()),
- ""));
- }
-
- private void addOwnerFileWithMatchersToRoot(boolean inherit) throws Exception {
- // Add OWNERS file to root:
- //
- // inherited: true
- // matchers:
- // - suffix: .txt
- // owners:
- // - admin@mail.com
- merge(
- createChange(
- testRepo,
- "master",
- "Add OWNER file",
- "OWNERS",
- String.format(
- "inherited: %s\nmatchers:\n" + "- suffix: .txt\n owners:\n - %s\n",
- inherit, admin.email()),
- ""));
- }
-}
+public class GetFilesOwnersIT extends GetFilesOwnersITAbstract {}
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java
new file mode 100644
index 0000000..936f3d3
--- /dev/null
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java
@@ -0,0 +1,361 @@
+// Copyright (C) 2023 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.googlesource.gerrit.owners.restapi;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit.Result;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.config.GlobalPluginConfig;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.server.project.testing.TestLabels;
+import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
+import com.googlesource.gerrit.owners.entities.GroupOwner;
+import com.googlesource.gerrit.owners.entities.Owner;
+import com.googlesource.gerrit.owners.restapi.GetFilesOwners.LabelNotFoundException;
+import java.util.Arrays;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.compress.utils.Sets;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.transport.FetchResult;
+import org.eclipse.jgit.util.FS;
+import org.junit.Test;
+
+public abstract class GetFilesOwnersITAbstract extends LightweightPluginDaemonTest {
+
+ private static final String REFS_META_CONFIG = RefNames.REFS_META + "config";
+ protected GetFilesOwners ownersApi;
+ private Owner rootOwner;
+ private Owner projectOwner;
+ private NameKey parentProjectName;
+ private NameKey childProjectName;
+ private TestRepository<InMemoryRepository> childRepo;
+ private TestRepository<InMemoryRepository> parentRepo;
+ private TestRepository<InMemoryRepository> allProjectsRepo;
+
+ @Override
+ public void setUpTestPlugin() throws Exception {
+ super.setUpTestPlugin();
+
+ rootOwner = new Owner(admin.fullName(), admin.id().get());
+ projectOwner = new Owner(user.fullName(), user.id().get());
+ ownersApi = plugin.getSysInjector().getInstance(GetFilesOwners.class);
+
+ parentProjectName =
+ createProjectOverAPI("parent", allProjects, true, SubmitType.FAST_FORWARD_ONLY);
+ parentRepo = cloneProjectWithMetaRefs(parentProjectName);
+
+ childProjectName =
+ createProjectOverAPI("child", parentProjectName, true, SubmitType.FAST_FORWARD_ONLY);
+ childRepo = cloneProject(childProjectName);
+
+ allProjectsRepo = cloneProjectWithMetaRefs(allProjects);
+ }
+
+ @Test
+ public void shouldReturnExactFileOwners() throws Exception {
+ addOwnerFileToRoot(true);
+ assertChangeHasOwners(createChange().getChangeId());
+ }
+
+ @Test
+ public void shouldReturnExactFileOwnersWhenOwnersIsSetToAllProjects() throws Exception {
+ addOwnerFileWithMatchers(allProjectsRepo, REFS_META_CONFIG, true);
+ assertChangeHasOwners(createChange(childRepo).getChangeId());
+ }
+
+ @Test
+ public void shouldReturnExactFileOwnersWhenOwnersIsSetToParentProject() throws Exception {
+ addOwnerFileWithMatchers(parentRepo, REFS_META_CONFIG, true);
+ assertChangeHasOwners(createChange(childRepo).getChangeId());
+ }
+
+ @Test
+ public void shouldReturnOwnersLabelsWhenNotApprovedByOwners() throws Exception {
+ addOwnerFileToRoot(true);
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get())));
+
+ assertThat(resp.value().ownersLabels).isEmpty();
+ }
+
+ @Test
+ public void shouldReturnEmptyResponseWhenApprovedByOwners() throws Exception {
+ addOwnerFileToRoot(true);
+ String changeId = createChange().getChangeId();
+ approve(changeId);
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files).isEmpty();
+ }
+
+ @Test
+ @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
+ public void shouldReturnResponseWithUnexpandedFileOwners() throws Exception {
+ addOwnerFileToRoot(true);
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
+ }
+
+ @Test
+ @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
+ public void shouldReturnEmptyResponseWhenApprovedByOwnersWithUnexpandedFileOwners()
+ throws Exception {
+ addOwnerFileToRoot(true);
+ String changeId = createChange().getChangeId();
+ approve(changeId);
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files).isEmpty();
+ }
+
+ @Test
+ @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
+ public void shouldReturnResponseWithUnexpandedFileMatchersOwners() throws Exception {
+ addOwnerFileWithMatchersToRoot(true);
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldReturnInheritedOwnersFromProjectsOwners() throws Exception {
+ assertInheritFromProject(project);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldReturnInheritedOwnersFromParentProjectsOwners() throws Exception {
+ assertInheritFromProject(allProjects);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldReflectChangesInParentProject() throws Exception {
+ addOwnerFileToProjectConfig(allProjects, true, admin);
+
+ String changeId = createChange().getChangeId();
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner));
+
+ addOwnerFileToProjectConfig(allProjects, true, user);
+ resp = assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(projectOwner));
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldNotReturnInheritedOwnersFromProjectsOwners() throws Exception {
+ assertNotInheritFromProject(project);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldNotReturnInheritedOwnersFromParentProjectsOwners() throws Exception {
+ addOwnerFileToProjectConfig(project, false);
+ assertNotInheritFromProject(allProjects);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldThrowExceptionWhenCodeReviewLabelIsNotConfigured() throws Exception {
+ addOwnerFileToProjectConfig(project, false);
+ replaceCodeReviewWithLabel(
+ TestLabels.label(
+ "Foo", TestLabels.value(1, "Foo is fine"), TestLabels.value(-1, "Foo is not fine")));
+ String changeId = createChange().getChangeId();
+
+ ResourceNotFoundException thrown =
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(thrown).hasMessageThat().isEqualTo(GetFilesOwners.MISSING_CODE_REVIEW_LABEL);
+ assertThat(thrown).hasCauseThat().isInstanceOf(LabelNotFoundException.class);
+ }
+
+ protected void replaceCodeReviewWithLabel(LabelType label) throws Exception {
+ try (ProjectConfigUpdate u = updateProject(allProjects)) {
+ u.getConfig().getLabelSections().remove(LabelId.CODE_REVIEW);
+ u.getConfig().upsertLabelType(label);
+ u.save();
+ }
+ }
+
+ protected static <T> Response<T> assertResponseOk(Response<T> response) {
+ assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
+ return response;
+ }
+
+ private void assertNotInheritFromProject(Project.NameKey projectNameKey) throws Exception {
+ addOwnerFileToRoot(false);
+ addOwnerFileToProjectConfig(projectNameKey, true);
+
+ String changeId = createChange().getChangeId();
+ assertChangeHasOwners(changeId);
+ }
+
+ private void assertChangeHasOwners(String changeId)
+ throws AuthException, BadRequestException, ResourceConflictException, Exception {
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner));
+ }
+
+ private void assertInheritFromProject(Project.NameKey projectNameKey) throws Exception {
+ addOwnerFileToRoot(true);
+ addOwnerFileToProjectConfig(projectNameKey, true);
+
+ String changeId = createChange().getChangeId();
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(rootOwner, projectOwner));
+ }
+
+ private void addOwnerFileToProjectConfig(Project.NameKey projectNameKey, boolean inherit)
+ throws Exception {
+ addOwnerFileToProjectConfig(projectNameKey, inherit, user);
+ }
+
+ private void addOwnerFileToProjectConfig(
+ Project.NameKey projectNameKey, boolean inherit, TestAccount account) throws Exception {
+ TestRepository<InMemoryRepository> project = cloneProject(projectNameKey);
+ GitUtil.fetch(project, RefNames.REFS_CONFIG + ":" + RefNames.REFS_CONFIG);
+ project.reset(RefNames.REFS_CONFIG);
+ pushFactory
+ .create(
+ admin.newIdent(),
+ project,
+ "Add OWNER file",
+ "OWNERS",
+ String.format(
+ "inherited: %s\nmatchers:\n" + "- suffix: .txt\n owners:\n - %s\n",
+ inherit, account.email()))
+ .to(RefNames.REFS_CONFIG);
+ }
+
+ private void addOwnerFileToRoot(boolean inherit) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // owners:
+ // - admin
+ merge(
+ createChange(
+ testRepo,
+ "master",
+ "Add OWNER file",
+ "OWNERS",
+ String.format("inherited: %s\nowners:\n- %s\n", inherit, admin.email()),
+ ""));
+ }
+
+ private void addOwnerFileWithMatchersToRoot(boolean inherit) throws Exception {
+ addOwnerFileWithMatchers(testRepo, "master", inherit);
+ }
+
+ private void addOwnerFileWithMatchers(TestRepository<?> repo, String targetRef, boolean inherit)
+ throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // matchers:
+ // - suffix: .txt
+ // owners:
+ // - admin@mail.com
+ Result changeCreated =
+ createChange(
+ repo,
+ targetRef,
+ "Add OWNER file",
+ "OWNERS",
+ String.format(
+ "inherited: %s\nmatchers:\n" + "- suffix: .txt\n owners:\n - %s\n",
+ inherit, admin.email()),
+ "");
+ changeCreated.assertOkStatus();
+ merge(changeCreated);
+ }
+
+ public TestRepository<InMemoryRepository> cloneProjectWithMetaRefs(Project.NameKey project)
+ throws Exception {
+ String uri = registerRepoConnection(project, admin);
+ String initialRef = "refs/remotes/origin/config";
+ DfsRepositoryDescription desc = new DfsRepositoryDescription("clone of " + project.get());
+
+ InMemoryRepository.Builder b = new InMemoryRepository.Builder().setRepositoryDescription(desc);
+ if (uri.startsWith("ssh://")) {
+ // SshTransport depends on a real FS to read ~/.ssh/config, but InMemoryRepository by default
+ // uses a null FS.
+ // Avoid leaking user state into our tests.
+ b.setFS(FS.detect().setUserHome(null));
+ }
+ InMemoryRepository dest = b.build();
+ Config cfg = dest.getConfig();
+ cfg.setString("remote", "origin", "url", uri);
+ cfg.setStringList(
+ "remote",
+ "origin",
+ "fetch",
+ Arrays.asList(
+ "+refs/heads/*:refs/remotes/origin/*", "+refs/meta/config:refs/remotes/origin/config"));
+ TestRepository<InMemoryRepository> testRepo = GitUtil.newTestRepository(dest);
+ FetchResult result = testRepo.git().fetch().setRemote("origin").call();
+ if (result.getTrackingRefUpdate(initialRef) != null) {
+ testRepo.reset(initialRef);
+ }
+ return testRepo;
+ }
+}
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java
new file mode 100644
index 0000000..8172611
--- /dev/null
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java
@@ -0,0 +1,143 @@
+// Copyright (C) 2023 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.googlesource.gerrit.owners.restapi;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.server.project.testing.TestLabels;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.owners.common.LabelDefinition;
+import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
+import com.googlesource.gerrit.owners.entities.Owner;
+import java.nio.file.Files;
+import java.nio.file.StandardOpenOption;
+import java.util.Map;
+import org.apache.commons.compress.utils.Sets;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+@TestPlugin(name = "owners", sysModule = "com.googlesource.gerrit.owners.OwnersModule")
+@UseLocalDisk
+public class GetFilesOwnersSubmitRequirementsIT extends GetFilesOwnersITAbstract {
+ @Inject private ProjectOperations projectOperations;
+
+ @Override
+ public void setUpTestPlugin() throws Exception {
+ Config pluginCfg = pluginConfig.getGlobalPluginConfig("owners");
+ // enable submit requirements and store them to the file as there is no `ConfigSuite` mechanism
+ // for plugin config and there is no other way (but adding it to each test case) to enable it
+ // globally
+ pluginCfg.setBoolean("owners", null, "enableSubmitRequirement", true);
+ Files.writeString(
+ server.getSitePath().resolve("etc").resolve("owners.config"),
+ pluginCfg.toText(),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.APPEND);
+ super.setUpTestPlugin();
+ }
+
+ @Test
+ public void shouldRequireConfiguredCodeReviewScore() throws Exception {
+ // configure submit requirement to require CR+1 only
+ addOwnerFileToRoot(LabelDefinition.parse("Code-Review,1").get(), admin);
+
+ String changeId = createChange("Add a file", "foo", "bar").getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(resp.value().files)
+ .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get())));
+ assertThat(resp.value().ownersLabels).isEmpty();
+
+ // give CR+1 as requested
+ recommend(changeId);
+
+ resp = assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(resp.value().files).isEmpty();
+ assertThat(resp.value().ownersLabels)
+ .containsExactly(admin.id().get(), Map.of(LabelId.CODE_REVIEW, 1));
+ }
+
+ @Test
+ public void shouldRequireConfiguredLabelAndScore() throws Exception {
+ // configure submit requirement to require LabelFoo+1
+ String label = "LabelFoo";
+ addOwnerFileToRoot(LabelDefinition.parse(String.format("%s,1", label)).get(), admin);
+ replaceCodeReviewWithLabel(label);
+
+ String changeId = createChange("Add a file", "foo", "bar").getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(resp.value().files)
+ .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get())));
+ assertThat(resp.value().ownersLabels).isEmpty();
+
+ // give LabelFoo+1 as requested
+ gApi.changes().id(changeId).current().review(new ReviewInput().label(label, 1));
+
+ resp = assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+ assertThat(resp.value().files).isEmpty();
+ assertThat(resp.value().ownersLabels).containsEntry(admin.id().get(), Map.of(label, 1));
+ }
+
+ private void addOwnerFileToRoot(LabelDefinition label, TestAccount u) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // label: label,score # score is optional
+ // owners:
+ // - u.email()
+ String owners =
+ String.format(
+ "inherited: true\nlabel: %s\nowners:\n- %s\n",
+ String.format(
+ "%s%s",
+ label.getName(),
+ label.getScore().map(value -> String.format(",%d", value)).orElse("")),
+ u.email());
+ pushFactory
+ .create(admin.newIdent(), testRepo, "Add OWNER file", "OWNERS", owners)
+ .to(RefNames.fullName("master"))
+ .assertOkStatus();
+ }
+
+ private void replaceCodeReviewWithLabel(String labelId) throws Exception {
+ LabelType label =
+ TestLabels.label(labelId, TestLabels.value(1, "OK"), TestLabels.value(-1, "Not OK"));
+
+ replaceCodeReviewWithLabel(label);
+
+ // grant label to RegisteredUsers so that it is vote-able
+ String heads = RefNames.REFS_HEADS + "*";
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(label.getName()).ref(heads).group(REGISTERED_USERS).range(-1, 1))
+ .update();
+ }
+}
