GetCodeOwnerStatus: Allow to limit the number of returned statuses
Computing all code owner statuses can be expensive for large changes.
Often callers do not need all code owner statuses, hence it should be
possible to limit the result. E.g. the frontend uses this REST endpoint
to show the code owner statuses in the file list on the change screen,
but for large changes the frontend is not showing all files by default,
hence retrieving all code owner statuses in this case would be wasteful.
By default the change screen is showing at most 200 files and if the
user wants to see more/all files they need to expand the file list. The
frontend can set the limit for code owner statuses to 200 now so that
these 200 files can be covered (renames are reported as a single code
owner status, hence also 200 renames are covered).
The start parameter allows to skip file code owner statuses and can be
used to paginate over the code owner statuses.
When the user expands the file list on the change screen to all files
the frontend can use the start parameter to skip the 200 code owner
statuses that it already has.
Change-Id: I80acb3d1ea04d13db72700623c72dd8c4bf5f63a
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/api/ChangeCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/ChangeCodeOwners.java
index cd8aacb..12aebb4 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/ChangeCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/ChangeCodeOwners.java
@@ -16,6 +16,7 @@
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
+import java.util.Optional;
/**
* Java API for change code owners.
@@ -40,6 +41,39 @@
* <p>Allows to set parameters on the request before executing it by calling {@link #get()}.
*/
abstract class CodeOwnerStatusRequest {
+ private Integer start;
+ private Integer limit;
+
+ /**
+ * Sets a limit on the number of code owner statuses that should be returned.
+ *
+ * @param start number of code owner statuses to skip
+ */
+ public CodeOwnerStatusRequest withStart(int start) {
+ this.start = start;
+ return this;
+ }
+
+ /** Returns the number of code owner statuses to skip. */
+ public Optional<Integer> getStart() {
+ return Optional.ofNullable(start);
+ }
+
+ /**
+ * Sets a limit on the number of code owner statuses that should be returned.
+ *
+ * @param limit the limit
+ */
+ public CodeOwnerStatusRequest withLimit(int limit) {
+ this.limit = limit;
+ return this;
+ }
+
+ /** Returns the limit. */
+ public Optional<Integer> getLimit() {
+ return Optional.ofNullable(limit);
+ }
+
/**
* Executes this request and retrieves the code owner status.
*
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerStatusInfo.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerStatusInfo.java
index b10e2be..e9ae34d 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerStatusInfo.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerStatusInfo.java
@@ -34,4 +34,11 @@
/** List of the code owner statuses for the files in the change. */
public List<FileCodeOwnerStatusInfo> fileCodeOwnerStatuses;
+
+ /**
+ * Whether the request would deliver more results if not limited.
+ *
+ * <p>Not set if {@code false}.
+ */
+ public Boolean more;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/impl/ChangeCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/impl/ChangeCodeOwnersImpl.java
index 5a20f34..17c78cf 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/impl/ChangeCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/impl/ChangeCodeOwnersImpl.java
@@ -26,6 +26,7 @@
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.restapi.change.Revisions;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
/** Implementation of the {@link ChangeCodeOwners} API. */
@@ -37,17 +38,17 @@
private final Revisions revisions;
private final RevisionCodeOwnersImpl.Factory revisionCodeOwnersApi;
private final ChangeResource changeResource;
- private final GetCodeOwnerStatus getCodeOwnerStatus;
+ private final Provider<GetCodeOwnerStatus> getCodeOwnerStatusProvider;
@Inject
public ChangeCodeOwnersImpl(
Revisions revisions,
RevisionCodeOwnersImpl.Factory revisionCodeOwnersApi,
- GetCodeOwnerStatus getCodeOwnerStatus,
+ Provider<GetCodeOwnerStatus> getCodeOwnerStatusProvider,
@Assisted ChangeResource changeResource) {
this.revisions = revisions;
this.revisionCodeOwnersApi = revisionCodeOwnersApi;
- this.getCodeOwnerStatus = getCodeOwnerStatus;
+ this.getCodeOwnerStatusProvider = getCodeOwnerStatusProvider;
this.changeResource = changeResource;
}
@@ -57,6 +58,9 @@
@Override
public CodeOwnerStatusInfo get() throws RestApiException {
try {
+ GetCodeOwnerStatus getCodeOwnerStatus = getCodeOwnerStatusProvider.get();
+ getStart().ifPresent(getCodeOwnerStatus::setStart);
+ getLimit().ifPresent(getCodeOwnerStatus::setLimit);
return getCodeOwnerStatus.apply(changeResource).value();
} catch (Exception e) {
throw asRestApiException("Cannot get code owner status", e);
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index 236e62a..4317c77 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -211,18 +211,28 @@
* Gets the code owner statuses for all files/paths that were changed in the current revision of
* the given change as a set.
*
+ * @param start number of file statuses to skip
+ * @param limit the max number of file statuses that should be returned (0 = unlimited)
* @see #getFileStatuses(CodeOwnerConfigHierarchy, ChangeNotes)
*/
- public ImmutableSet<FileCodeOwnerStatus> getFileStatusesAsSet(ChangeNotes changeNotes)
+ public ImmutableSet<FileCodeOwnerStatus> getFileStatusesAsSet(
+ ChangeNotes changeNotes, int start, int limit)
throws ResourceConflictException, IOException, PatchListNotAvailableException,
DiffNotAvailableException {
requireNonNull(changeNotes, "changeNotes");
try (Timer0.Context ctx = codeOwnerMetrics.computeFileStatuses.start()) {
logger.atFine().log(
- "compute file statuses (project = %s, change = %d)",
- changeNotes.getProjectName(), changeNotes.getChangeId().get());
- return getFileStatuses(codeOwnerConfigHierarchyProvider.get(), changeNotes)
- .collect(toImmutableSet());
+ "compute file statuses (project = %s, change = %d, start = %d, limit = %d)",
+ changeNotes.getProjectName(), changeNotes.getChangeId().get(), start, limit);
+ Stream<FileCodeOwnerStatus> fileStatuses =
+ getFileStatuses(codeOwnerConfigHierarchyProvider.get(), changeNotes);
+ if (start > 0) {
+ fileStatuses = fileStatuses.skip(start);
+ }
+ if (limit > 0) {
+ fileStatuses = fileStatuses.limit(limit);
+ }
+ return fileStatuses.collect(toImmutableSet());
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java
index 34a1dfe..1952965 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerStatus.java
@@ -14,7 +14,10 @@
package com.google.gerrit.plugins.codeowners.restapi;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -26,8 +29,8 @@
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.Inject;
-import com.google.inject.Singleton;
import java.io.IOException;
+import org.kohsuke.args4j.Option;
/**
* REST endpoint that gets the code owner statuses for the files in a change.
@@ -49,10 +52,32 @@
* so that this approval can never happen)
* </ul>
*/
-@Singleton
public class GetCodeOwnerStatus implements RestReadView<ChangeResource> {
+ private static final int UNLIMITED = 0;
+
private final CodeOwnerApprovalCheck codeOwnerApprovalCheck;
+ private int start;
+ private int limit;
+
+ @Option(
+ name = "--limit",
+ aliases = {"-n"},
+ metaVar = "CNT",
+ usage = "maximum number of file code owner statuses to return (by default 0 aka unlimited)")
+ public void setLimit(int limit) {
+ this.limit = limit;
+ }
+
+ @Option(
+ name = "--start",
+ aliases = {"-S"},
+ metaVar = "CNT",
+ usage = "number of owned paths to skip")
+ public void setStart(int start) {
+ this.start = start;
+ }
+
@Inject
public GetCodeOwnerStatus(CodeOwnerApprovalCheck codeOwnerApprovalCheck) {
this.codeOwnerApprovalCheck = codeOwnerApprovalCheck;
@@ -62,10 +87,28 @@
public Response<CodeOwnerStatusInfo> apply(ChangeResource changeResource)
throws RestApiException, IOException, PermissionBackendException,
PatchListNotAvailableException, DiffNotAvailableException {
+ validateStartAndLimit();
+
ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(changeResource.getNotes());
- return Response.ok(
+ codeOwnerApprovalCheck.getFileStatusesAsSet(
+ changeResource.getNotes(), start, limit == UNLIMITED ? UNLIMITED : limit + 1);
+ CodeOwnerStatusInfo codeOwnerStatusInfo =
CodeOwnerStatusInfoJson.format(
- changeResource.getNotes().getCurrentPatchSet().id(), fileCodeOwnerStatuses));
+ changeResource.getNotes().getCurrentPatchSet().id(),
+ limit == UNLIMITED
+ ? fileCodeOwnerStatuses
+ : fileCodeOwnerStatuses.stream().limit(limit).collect(toImmutableSet()));
+ codeOwnerStatusInfo.more =
+ limit != UNLIMITED && fileCodeOwnerStatuses.size() > limit ? true : null;
+ return Response.ok(codeOwnerStatusInfo);
+ }
+
+ private void validateStartAndLimit() throws BadRequestException {
+ if (start < 0) {
+ throw new BadRequestException("start cannot be negative");
+ }
+ if (limit < 0) {
+ throw new BadRequestException("limit cannot be negative");
+ }
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerStatusInfoSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerStatusInfoSubject.java
index 6bd2e07..a0b3452 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerStatusInfoSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/CodeOwnerStatusInfoSubject.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusInfoSubject.fileCodeOwnerStatusInfos;
import static com.google.gerrit.truth.ListSubject.elements;
+import com.google.common.truth.BooleanSubject;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IntegerSubject;
import com.google.common.truth.Subject;
@@ -57,11 +58,15 @@
/** Returns a {@link ListSubject} for the file code owner statuses. */
public ListSubject<FileCodeOwnerStatusInfoSubject, FileCodeOwnerStatusInfo>
hasFileCodeOwnerStatusesThat() {
- return check("fileCodeOwnerStatuses")
+ return check("fileCodeOwnerStatuses()")
.about(elements())
.thatCustom(codeOwnerStatusInfo().fileCodeOwnerStatuses, fileCodeOwnerStatusInfos());
}
+ public BooleanSubject hasMoreThat() {
+ return check("more()").that(codeOwnerStatusInfo().more);
+ }
+
private CodeOwnerStatusInfo codeOwnerStatusInfo() {
isNotNull();
return codeOwnerStatusInfo;
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
index 04eb2fd..d8c51e6 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
@@ -14,19 +14,26 @@
package com.google.gerrit.plugins.codeowners.acceptance.api;
+import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerStatusInfoSubject.assertThat;
import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusInfoSubject.isFileCodeOwnerStatus;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerStatusInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnersExperimentFeaturesConstants;
import com.google.gerrit.plugins.codeowners.backend.FileCodeOwnerStatus;
import com.google.gerrit.plugins.codeowners.common.CodeOwnerStatus;
+import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.inject.Inject;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import org.junit.Test;
/**
@@ -67,6 +74,360 @@
.hasFileCodeOwnerStatusesThat()
.comparingElementsUsing(isFileCodeOwnerStatus())
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.PENDING));
+ assertThat(codeOwnerStatus).hasMoreThat().isNull();
+ }
+
+ @Test
+ public void getStatusWithStart() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ String path1 = "/foo/bar/baz.md";
+ String path2 = "/foo/baz/bar.md";
+ String path3 = "/bar/foo.md";
+ String path4 = "/bar/baz.md";
+
+ PushOneCommit.Result r =
+ createChange(
+ "Change Adding A File",
+ ImmutableMap.of(
+ JgitPath.of(path1).get(),
+ "file content",
+ JgitPath.of(path2).get(),
+ "file content",
+ JgitPath.of(path3).get(),
+ "file content",
+ JgitPath.of(path4).get(),
+ "file content"));
+ String changeId = r.getChangeId();
+
+ // Add a reviewer that is a code owner.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ // Add a Code-Review+1 (= code owner approval) from a user that is not a code owner.
+ requestScopeOperations.setApiUser(user2.id());
+ recommend(changeId);
+
+ CodeOwnerStatusInfo codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withStart(0).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path4, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path3, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.PENDING),
+ FileCodeOwnerStatus.addition(path2, CodeOwnerStatus.PENDING))
+ .inOrder();
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withStart(1).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path3, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.PENDING),
+ FileCodeOwnerStatus.addition(path2, CodeOwnerStatus.PENDING))
+ .inOrder();
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withStart(2).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.PENDING),
+ FileCodeOwnerStatus.addition(path2, CodeOwnerStatus.PENDING))
+ .inOrder();
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withStart(3).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(FileCodeOwnerStatus.addition(path2, CodeOwnerStatus.PENDING));
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withStart(4).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().isEmpty();
+ }
+
+ @Test
+ public void getStatusWithLimit() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ String path1 = "/foo/bar/baz.md";
+ String path2 = "/foo/baz/bar.md";
+ String path3 = "/bar/foo.md";
+ String path4 = "/bar/baz.md";
+
+ PushOneCommit.Result r =
+ createChange(
+ "Change Adding A File",
+ ImmutableMap.of(
+ JgitPath.of(path1).get(),
+ "file content",
+ JgitPath.of(path2).get(),
+ "file content",
+ JgitPath.of(path3).get(),
+ "file content",
+ JgitPath.of(path4).get(),
+ "file content"));
+ String changeId = r.getChangeId();
+
+ // Add a reviewer that is a code owner.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ // Add a Code-Review+1 (= code owner approval) from a user that is not a code owner.
+ requestScopeOperations.setApiUser(user2.id());
+ recommend(changeId);
+
+ CodeOwnerStatusInfo codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withLimit(1).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path4, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
+ assertThat(codeOwnerStatus).hasMoreThat().isTrue();
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withLimit(2).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path4, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path3, CodeOwnerStatus.INSUFFICIENT_REVIEWERS))
+ .inOrder();
+ assertThat(codeOwnerStatus).hasMoreThat().isTrue();
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withLimit(3).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path4, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path3, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.PENDING))
+ .inOrder();
+ assertThat(codeOwnerStatus).hasMoreThat().isTrue();
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withLimit(4).get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path4, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path3, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.PENDING),
+ FileCodeOwnerStatus.addition(path2, CodeOwnerStatus.PENDING))
+ .inOrder();
+ assertThat(codeOwnerStatus).hasMoreThat().isNull();
+ }
+
+ @Test
+ public void getStatusWithLimitForRename() throws Exception {
+ testGetStatusWithLimitForRenamedFile(/* useDiffCache= */ false);
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value = CodeOwnersExperimentFeaturesConstants.USE_DIFF_CACHE)
+ public void getStatusWithLimitForRename_useDiffCache() throws Exception {
+ testGetStatusWithLimitForRenamedFile(/* useDiffCache= */ true);
+ }
+
+ private void testGetStatusWithLimitForRenamedFile(boolean useDiffCache) throws Exception {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ Path oldPath = Paths.get("/foo/old.bar");
+ Path newPath = Paths.get("/bar/new.bar");
+ String changeId = createChangeWithFileRename(oldPath, newPath);
+
+ // Add a reviewer that is a code owner of the old path.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ CodeOwnerStatusInfo codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withLimit(1).get();
+ if (useDiffCache) {
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.rename(
+ oldPath,
+ CodeOwnerStatus.PENDING,
+ newPath,
+ CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
+ assertThat(codeOwnerStatus).hasMoreThat().isNull();
+ } else {
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(newPath, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
+ assertThat(codeOwnerStatus).hasMoreThat().isTrue();
+
+ codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withLimit(2).get();
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(newPath, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.deletion(oldPath, CodeOwnerStatus.PENDING));
+ assertThat(codeOwnerStatus).hasMoreThat().isNull();
+ }
+ }
+
+ @Test
+ public void getStatusWithStartAndLimit() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ String path1 = "/foo/bar/baz.md";
+ String path2 = "/foo/baz/bar.md";
+ String path3 = "/bar/foo.md";
+ String path4 = "/bar/baz.md";
+
+ PushOneCommit.Result r =
+ createChange(
+ "Change Adding A File",
+ ImmutableMap.of(
+ JgitPath.of(path1).get(),
+ "file content",
+ JgitPath.of(path2).get(),
+ "file content",
+ JgitPath.of(path3).get(),
+ "file content",
+ JgitPath.of(path4).get(),
+ "file content"));
+ String changeId = r.getChangeId();
+
+ // Add a reviewer that is a code owner.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ // Add a Code-Review+1 (= code owner approval) from a user that is not a code owner.
+ requestScopeOperations.setApiUser(user2.id());
+ recommend(changeId);
+
+ CodeOwnerStatusInfo codeOwnerStatus =
+ changeCodeOwnersApiFactory
+ .change(changeId)
+ .getCodeOwnerStatus()
+ .withStart(1)
+ .withLimit(2)
+ .get();
+ assertThat(codeOwnerStatus)
+ .hasPatchSetNumberThat()
+ .isEqualTo(r.getChange().currentPatchSet().id().get());
+ assertThat(codeOwnerStatus)
+ .hasFileCodeOwnerStatusesThat()
+ .comparingElementsUsing(isFileCodeOwnerStatus())
+ .containsExactly(
+ FileCodeOwnerStatus.addition(path3, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
+ FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.PENDING))
+ .inOrder();
+ assertThat(codeOwnerStatus).hasMoreThat().isTrue();
+ }
+
+ @Test
+ public void startCannotBeNegative() throws Exception {
+ String changeId = createChange().getChangeId();
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ changeCodeOwnersApiFactory
+ .change(changeId)
+ .getCodeOwnerStatus()
+ .withStart(-1)
+ .get());
+ assertThat(exception).hasMessageThat().isEqualTo("start cannot be negative");
+ }
+
+ @Test
+ public void limitCannotBeNegative() throws Exception {
+ String changeId = createChange().getChangeId();
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ changeCodeOwnersApiFactory
+ .change(changeId)
+ .getCodeOwnerStatus()
+ .withLimit(-1)
+ .get());
+ assertThat(exception).hasMessageThat().isEqualTo("limit cannot be negative");
+ }
+
+ @Test
+ public void getStatusWithoutLimit() throws Exception {
+ String changeId = createChange().getChangeId();
+ CodeOwnerStatusInfo codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus().withLimit(0).get();
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().isNotEmpty();
+ assertThat(codeOwnerStatus).hasMoreThat().isNull();
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
index a4e6437..8f4959d 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerStatusRestIT.java
@@ -88,6 +88,28 @@
filePath, project.get()));
}
+ @Test
+ public void cannotGetStatusWithInvalidStart() throws Exception {
+ String changeId = createChange().getChangeId();
+ RestResponse r =
+ adminRestSession.get(
+ String.format(
+ "/changes/%s/code_owners.status?start=invalid", IdString.fromDecoded(changeId)));
+ r.assertBadRequest();
+ assertThat(r.getEntityContent()).contains("\"invalid\" is not a valid value for \"--start\"");
+ }
+
+ @Test
+ public void cannotGetStatusWithInvalidLimit() throws Exception {
+ String changeId = createChange().getChangeId();
+ RestResponse r =
+ adminRestSession.get(
+ String.format(
+ "/changes/%s/code_owners.status?limit=invalid", IdString.fromDecoded(changeId)));
+ r.assertBadRequest();
+ assertThat(r.getEntityContent()).contains("\"invalid\" is not a valid value for \"--limit\"");
+ }
+
private CodeOwnerConfig.Key createCodeOwnerConfigKey(String folderPath) {
return CodeOwnerConfig.Key.create(project, "master", folderPath);
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
index 6c6b0f2..566c5cd 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
@@ -79,7 +79,9 @@
NullPointerException npe =
assertThrows(
NullPointerException.class,
- () -> codeOwnerApprovalCheck.getFileStatusesAsSet(/* changeNotes= */ null));
+ () ->
+ codeOwnerApprovalCheck.getFileStatusesAsSet(
+ /* changeNotes= */ null, /* start= */ 0, /* limit= */ 0));
assertThat(npe).hasMessageThat().isEqualTo("changeNotes");
}
@@ -98,8 +100,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -122,8 +123,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.modification(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -143,8 +143,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.deletion(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -178,8 +177,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
if (useDiffCache) {
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
@@ -213,8 +211,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.PENDING));
}
@@ -238,8 +235,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.modification(path, CodeOwnerStatus.PENDING));
}
@@ -260,8 +256,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.deletion(path, CodeOwnerStatus.PENDING));
}
@@ -295,8 +290,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
if (useDiffCache) {
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
@@ -342,8 +336,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
if (useDiffCache) {
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
@@ -372,8 +365,7 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -392,8 +384,7 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.modification(path, CodeOwnerStatus.APPROVED));
}
@@ -409,8 +400,7 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.deletion(path, CodeOwnerStatus.APPROVED));
}
@@ -440,8 +430,7 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
if (useDiffCache) {
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
@@ -483,8 +472,7 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
if (useDiffCache) {
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
@@ -523,8 +511,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
amendChange(user, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(
@@ -559,8 +546,7 @@
.getChangeId();
amendChange(user, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.modification(
@@ -591,8 +577,7 @@
String changeId = createChangeWithFileDeletion(path);
amendChange(user, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.deletion(
@@ -647,8 +632,7 @@
String changeId = createChangeWithFileRename(oldPath, newPath);
amendChange(user, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
if (useDiffCache) {
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
@@ -716,8 +700,7 @@
String changeId = createChangeWithFileRename(oldPath, newPath);
amendChange(user, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
if (useDiffCache) {
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
@@ -750,8 +733,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
amendChange(user, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -771,8 +753,7 @@
amendChange(user, changeId);
amendChange(user2, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -797,8 +778,7 @@
// Verify that the file is not approved yet (the change owner is a code owner, but
// implicit approvals are disabled).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -808,7 +788,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -840,8 +820,7 @@
String changeId =
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(
@@ -869,8 +848,7 @@
// Verify that the status of the file is INSUFFICIENT_REVIEWERS (since there is no implicit
// approval by default).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -879,7 +857,7 @@
gApi.changes().id(changeId).addReviewer(user.email());
// Check that the status of the file is PENDING now.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.PENDING));
}
@@ -898,8 +876,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -915,7 +892,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -943,8 +920,7 @@
createChange(bot, "Change Adding A File", JgitPath.of(path).get(), "file content")
.getChangeId();
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(
@@ -968,8 +944,7 @@
.getChangeId();
// Verify that the status of the file is INSUFFICIENT_REVIEWERS.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -979,7 +954,7 @@
// Check that the status of the file is PENDING now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.PENDING));
@@ -994,7 +969,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1010,8 +985,7 @@
// Verify that the file is not approved yet (the change owner is a global code owner, but
// implicit approvals are disabled).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1021,7 +995,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1048,8 +1022,7 @@
String changeId =
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(
@@ -1069,8 +1042,7 @@
// Verify that the status of the file is INSUFFICIENT_REVIEWERS (since there is no implicit
// approval by default).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1079,7 +1051,7 @@
gApi.changes().id(changeId).addReviewer(user.email());
// Check that the status of the file is PENDING now.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.PENDING));
}
@@ -1106,8 +1078,7 @@
// Add code owner from a lower level as reviewer.
gApi.changes().id(changeId).addReviewer(user2.email());
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
// The expected status is APPROVED since 'user' which is configured as code owner on the root
// level approved the change.
assertThatCollection(fileCodeOwnerStatuses)
@@ -1136,8 +1107,7 @@
.getChangeId();
// Without Owners-Override approval the expected status is INSUFFICIENT_REVIEWERS.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
@@ -1147,7 +1117,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
// With Owners-Override approval the expected status is APPROVED.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.APPROVED),
@@ -1179,8 +1149,7 @@
.getChangeId();
// Without override approval the expected status is INSUFFICIENT_REVIEWERS.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
@@ -1190,7 +1159,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
// With override approval the expected status is APPROVED.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.APPROVED),
@@ -1200,7 +1169,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 0));
// Without override approval the expected status is INSUFFICIENT_REVIEWERS.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.INSUFFICIENT_REVIEWERS),
@@ -1210,7 +1179,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Another-Override", 1));
// With override approval the expected status is APPROVED.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path1, CodeOwnerStatus.APPROVED),
@@ -1343,9 +1312,7 @@
gApi.projects().name(project.get()).deleteBranches(input);
ResourceConflictException exception =
- assertThrows(
- ResourceConflictException.class,
- () -> codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId)));
+ assertThrows(ResourceConflictException.class, () -> getFileCodeOwnerStatuses(changeId));
assertThat(exception).hasMessageThat().isEqualTo("destination branch not found");
}
@@ -1363,8 +1330,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1380,7 +1346,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
@@ -1400,7 +1366,7 @@
.isEqualTo(user.email());
// Check that the file is still approved.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1422,8 +1388,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1433,7 +1398,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
@@ -1452,7 +1417,7 @@
.isEqualTo(admin.email());
// Check that the file is still approved.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1471,8 +1436,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1488,7 +1452,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1523,8 +1487,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1535,7 +1498,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1553,8 +1516,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1570,7 +1532,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1595,8 +1557,7 @@
createChange(user, "Change Adding A File", JgitPath.of(path).get(), "file content")
.getChangeId();
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -1622,8 +1583,7 @@
.getChangeId();
// Verify that the status of the file is INSUFFICIENT_REVIEWERS.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1633,7 +1593,7 @@
// Check that the status of the file is PENDING now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.PENDING));
@@ -1648,7 +1608,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
@@ -1668,7 +1628,7 @@
// Check that the file is not approved.
ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeIdOfRevert));
+ getFileCodeOwnerStatuses(changeIdOfRevert);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.deletion(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1690,7 +1650,7 @@
// Check that the file is approved since it's a pure revert.
ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeIdOfRevert));
+ getFileCodeOwnerStatuses(changeIdOfRevert);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.deletion(path, CodeOwnerStatus.APPROVED));
}
@@ -1726,7 +1686,7 @@
// Check that the file is not approved.
ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(revertChange.changeId));
+ getFileCodeOwnerStatuses(revertChange.changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.modification(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -1746,8 +1706,7 @@
// Check that the file is approved since the uploader is exempted from requiring code owner
// approvals.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
@@ -1757,12 +1716,18 @@
// Check that the file is no longer approved since the uploader is not exempted from requiring
// code owner approvals.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
}
+ private ImmutableSet<FileCodeOwnerStatus> getFileCodeOwnerStatuses(String changeId)
+ throws Exception {
+ return codeOwnerApprovalCheck.getFileStatusesAsSet(
+ getChangeNotes(changeId), /* start= */ 0, /* limit= */ 0);
+ }
+
private ChangeNotes getChangeNotes(String changeId) throws Exception {
return changeNotesFactory.create(project, Change.id(gApi.changes().id(changeId).get()._number));
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithAllUsersAsFallbackCodeOwnersTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithAllUsersAsFallbackCodeOwnersTest.java
index dc87ef7..08052d8 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithAllUsersAsFallbackCodeOwnersTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithAllUsersAsFallbackCodeOwnersTest.java
@@ -76,8 +76,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -92,7 +91,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -107,7 +106,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -130,8 +129,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -157,8 +155,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -173,7 +170,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -188,7 +185,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -215,8 +212,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -235,8 +231,7 @@
// Verify that the file is not approved yet (the change owner is a code owner, but
// implicit approvals are disabled).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -250,7 +245,7 @@
gApi.changes().id(changeId).addReviewer(user.email());
// Verify that the status is pending now .
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -264,7 +259,7 @@
recommend(changeId);
// Verify that the status is approved now
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -283,8 +278,7 @@
// Verify that the file is approved (the change owner is a code owner and implicit approvals are
// enabled).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -310,8 +304,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -326,7 +319,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -341,7 +334,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -367,8 +360,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -395,8 +387,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -411,7 +402,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -426,7 +417,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -453,8 +444,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -486,8 +476,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -502,7 +491,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -517,7 +506,7 @@
// Verify that the file is not approved (fallback code owner doesn't apply since a code owner is
// defined).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -550,8 +539,7 @@
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -562,6 +550,12 @@
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
}
+ private ImmutableSet<FileCodeOwnerStatus> getFileCodeOwnerStatuses(String changeId)
+ throws Exception {
+ return codeOwnerApprovalCheck.getFileStatusesAsSet(
+ getChangeNotes(changeId), /* start= */ 0, /* limit= */ 0);
+ }
+
private ChangeNotes getChangeNotes(String changeId) throws Exception {
return changeNotesFactory.create(project, Change.id(gApi.changes().id(changeId).get()._number));
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithProjectOwnersAsFallbackCodeOwnersTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithProjectOwnersAsFallbackCodeOwnersTest.java
index d17d51b..68382f9 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithProjectOwnersAsFallbackCodeOwnersTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithProjectOwnersAsFallbackCodeOwnersTest.java
@@ -83,8 +83,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -105,7 +104,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -138,8 +137,7 @@
createChange(bot, "Change Adding A File", JgitPath.of(path).get(), "file content")
.getChangeId();
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -167,8 +165,7 @@
.getChangeId();
// Verify that the status of the file is INSUFFICIENT_REVIEWERS.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -183,7 +180,7 @@
// Check that the status of the file is PENDING now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -203,7 +200,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -224,8 +221,7 @@
// Verify that the file is not approved yet (the change owner is a global code owner, but
// implicit approvals are disabled).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -240,7 +236,7 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -272,8 +268,7 @@
String changeId =
createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -297,8 +292,7 @@
// Verify that the status of the file is INSUFFICIENT_REVIEWERS (since there is no implicit
// approval by default).
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -312,7 +306,7 @@
gApi.changes().id(changeId).addReviewer(user.email());
// Check that the status of the file is PENDING now.
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -341,8 +335,7 @@
requestScopeOperations.setApiUser(user3.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -373,8 +366,7 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -400,8 +392,7 @@
requestScopeOperations.setApiUser(admin.id());
recommend(changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -437,8 +428,7 @@
// Amend change with a user that is a project owner.
amendChange(admin, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -466,8 +456,7 @@
// Amend change with a user that is not a project owner.
amendChange(user, changeId);
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -501,8 +490,7 @@
.getChangeId();
// Without Owners-Override approval the expected status is INSUFFICIENT_REVIEWERS.
- for (FileCodeOwnerStatus fileCodeOwnerStatus :
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId))) {
+ for (FileCodeOwnerStatus fileCodeOwnerStatus : getFileCodeOwnerStatuses(changeId)) {
assertThat(fileCodeOwnerStatus)
.hasNewPathStatus()
.value()
@@ -514,8 +502,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
// With Owners-Override approval the expected status is APPROVED.
- for (FileCodeOwnerStatus fileCodeOwnerStatus :
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId))) {
+ for (FileCodeOwnerStatus fileCodeOwnerStatus : getFileCodeOwnerStatuses(changeId)) {
assertThat(fileCodeOwnerStatus)
.hasNewPathStatus()
.value()
@@ -547,8 +534,7 @@
.getChangeId();
// Without override approval the expected status is INSUFFICIENT_REVIEWERS.
- for (FileCodeOwnerStatus fileCodeOwnerStatus :
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId))) {
+ for (FileCodeOwnerStatus fileCodeOwnerStatus : getFileCodeOwnerStatuses(changeId)) {
assertThat(fileCodeOwnerStatus)
.hasNewPathStatus()
.value()
@@ -561,8 +547,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
// With override approval the expected status is APPROVED.
- for (FileCodeOwnerStatus fileCodeOwnerStatus :
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId))) {
+ for (FileCodeOwnerStatus fileCodeOwnerStatus : getFileCodeOwnerStatuses(changeId)) {
assertThat(fileCodeOwnerStatus)
.hasNewPathStatus()
.value()
@@ -574,8 +559,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 0));
// Without override approval the expected status is INSUFFICIENT_REVIEWERS.
- for (FileCodeOwnerStatus fileCodeOwnerStatus :
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId))) {
+ for (FileCodeOwnerStatus fileCodeOwnerStatus : getFileCodeOwnerStatuses(changeId)) {
assertThat(fileCodeOwnerStatus)
.hasNewPathStatus()
.value()
@@ -587,8 +571,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Another-Override", 1));
// With override approval the expected status is APPROVED.
- for (FileCodeOwnerStatus fileCodeOwnerStatus :
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId))) {
+ for (FileCodeOwnerStatus fileCodeOwnerStatus : getFileCodeOwnerStatuses(changeId)) {
assertThat(fileCodeOwnerStatus)
.hasNewPathStatus()
.value()
@@ -610,8 +593,7 @@
.getChangeId();
// Verify that the file is not approved yet.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -621,7 +603,7 @@
approve(changeId);
// Verify that the file is not approved yet
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(
FileCodeOwnerStatus.addition(path, CodeOwnerStatus.INSUFFICIENT_REVIEWERS));
@@ -637,11 +619,17 @@
// Check that the file is approved now.
requestScopeOperations.setApiUser(admin.id());
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
assertThatCollection(fileCodeOwnerStatuses)
.containsExactly(FileCodeOwnerStatus.addition(path, CodeOwnerStatus.APPROVED));
}
+ private ImmutableSet<FileCodeOwnerStatus> getFileCodeOwnerStatuses(String changeId)
+ throws Exception {
+ return codeOwnerApprovalCheck.getFileStatusesAsSet(
+ getChangeNotes(changeId), /* start= */ 0, /* limit= */ 0);
+ }
+
private ChangeNotes getChangeNotes(String changeId) throws Exception {
return changeNotesFactory.create(project, Change.id(gApi.changes().id(changeId).get()._number));
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithSelfApprovalsIgnoredTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithSelfApprovalsIgnoredTest.java
index a27bde9..6cdf2b1 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithSelfApprovalsIgnoredTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckWithSelfApprovalsIgnoredTest.java
@@ -83,8 +83,7 @@
.getChangeId();
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -99,7 +98,7 @@
recommend(changeId);
// Verify that the file is not approved (since self approvals are ignored).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -125,8 +124,7 @@
amendChange(admin, changeId);
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -142,7 +140,7 @@
// Verify that the file is approved now (since the change owner is not the uploader of the
// current patch set).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -172,8 +170,7 @@
amendChange(codeOwner, changeId);
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -188,7 +185,7 @@
// Verify that the file is not pending (the code owner is the uploader of the current patch set
// and self approvals are ignored).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -203,7 +200,7 @@
// Verify that the file is not approved (since the code owner is the uploader of the current
// patch set and self approvals are ignored).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -227,8 +224,7 @@
.getChangeId();
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -260,8 +256,7 @@
amendChange(codeOwner, changeId);
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -286,8 +281,7 @@
.getChangeId();
// Verify that the file is approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -319,8 +313,7 @@
amendChange(codeOwner, changeId);
// Verify that the file is approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -343,8 +336,7 @@
.getChangeId();
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -360,7 +352,7 @@
// Verify that the file is not approved (since self approvals on the override label are
// ignored).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -385,8 +377,7 @@
amendChange(admin, changeId);
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -402,7 +393,7 @@
// Verify that the file is approved now (since the change owner is not the uploader of the
// current patch set and hence the override counts).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -427,8 +418,7 @@
amendChange(admin, changeId);
// Verify that the file is not approved.
- ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ ImmutableSet<FileCodeOwnerStatus> fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
@@ -442,7 +432,7 @@
gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
// Verify that the file is not approved (since the override from the uploader is ignored).
- fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatusesAsSet(getChangeNotes(changeId));
+ fileCodeOwnerStatuses = getFileCodeOwnerStatuses(changeId);
fileCodeOwnerStatusSubject = assertThatCollection(fileCodeOwnerStatuses).onlyElement();
fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
fileCodeOwnerStatusSubject
@@ -452,6 +442,12 @@
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
}
+ private ImmutableSet<FileCodeOwnerStatus> getFileCodeOwnerStatuses(String changeId)
+ throws Exception {
+ return codeOwnerApprovalCheck.getFileStatusesAsSet(
+ getChangeNotes(changeId), /* start= */ 0, /* limit= */ 0);
+ }
+
private ChangeNotes getChangeNotes(String changeId) throws Exception {
return changeNotesFactory.create(project, Change.id(gApi.changes().id(changeId).get()._number));
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index df39f6f..dc1232b 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -603,6 +603,13 @@
The code owner statuses are always listed for the files in the current revision
of the change (latest patch set).
+The following request parameters can be specified:
+
+| Field Name | | Description |
+| ------------ | --------- | ----------- |
+| `start`\|`S` | optional | Number of file code owner statuses to skip. Allows to page over the file code owner statuses. By default 0.
+| `limit`\|`n` | optional | Limit defining how many file code owner statuses should be returned at most. By default 0 (= unlimited).
+
The code owner statuses are returned as a
[CodeOwnerStatusInfo](#code-owner-status-info) entity.
@@ -938,10 +945,11 @@
The `CodeOwnerStatusInfo` entity describes the code owner statuses for the files
in a change.
-| Field Name | Description |
-| ------------------ | ----------- |
-| `patch_set_number` | The number of the patch set for which the code owner statuses are returned.
-| `file_code_owner_statuses` | List of the code owner statuses for the files in the change as [FileCodeOwnerStatusInfo](#file-code-owner-status-info) entities, sorted by new path, then old path.
+| Field Name | | Description |
+| ------------------ | -------- | ----------- |
+| `patch_set_number` | | The number of the patch set for which the code owner statuses are returned.
+| `file_code_owner_statuses` | | List of the code owner statuses for the files in the change as [FileCodeOwnerStatusInfo](#file-code-owner-status-info) entities, sorted by new path, then old path.
+| `more` | optional | Whether the request would deliver more results if not limited. Not set if `false`.
### <a id="code-owners-status-info"> CodeOwnersStatusInfo
The `CodeOwnersStatusInfo` contains information about whether the code owners
