Merge "Compute only as many owned paths as needed, not all"
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
index 4fdab65..c96af17 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
@@ -200,10 +200,14 @@
RevCommit baseCommit = revCommit.getParentCount() > 0 ? revCommit.getParent(0) : null;
logger.atFine().log("baseCommit = %s", baseCommit != null ? baseCommit.name() : "n/a");
+ // Detecting renames is expensive (since it requires Git to load and compare file contents of
+ // added and deleted files) and can significantly increase the latency for changes that touch
+ // large files. To avoid this latency we do not enable the rename detection on the
+ // DiffFormater. As a result of this renamed files will be returned as 2 ChangedFile's, one
+ // for the deletion of the old path and one for the addition of the new path.
try (DiffFormatter diffFormatter = new DiffFormatter(DisabledOutputStream.INSTANCE)) {
diffFormatter.setReader(revWalk.getObjectReader(), repoConfig);
diffFormatter.setDiffComparator(RawTextComparator.DEFAULT);
- diffFormatter.setDetectRenames(true);
List<DiffEntry> diffEntries = diffFormatter.scan(baseCommit, revCommit);
ImmutableSet<ChangedFile> changedFiles =
diffEntries.stream().map(ChangedFile::create).collect(toImmutableSet());
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java
index 805eccd..069122c 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/ChangedFileSubject.java
@@ -17,10 +17,13 @@
import static com.google.common.truth.Truth.assertAbout;
import static com.google.gerrit.truth.OptionalSubject.optionals;
+import com.google.common.collect.ImmutableList;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.Subject;
import com.google.gerrit.plugins.codeowners.common.ChangedFile;
+import com.google.gerrit.truth.ListSubject;
import com.google.gerrit.truth.OptionalSubject;
+import java.util.Collection;
/** {@link Subject} for doing assertions on {@link ChangedFile}s. */
public class ChangedFileSubject extends Subject {
@@ -34,6 +37,12 @@
return assertAbout(changedFiles()).that(changedFile);
}
+ /** Starts fluent chain to do assertions on a collection of {@link ChangedFile}s. */
+ public static ListSubject<ChangedFileSubject, ChangedFile> assertThatCollection(
+ Collection<ChangedFile> changedFiles) {
+ return ListSubject.assertThat(ImmutableList.copyOf(changedFiles), changedFiles());
+ }
+
/** Creates subject factory for mapping {@link ChangedFile}s to {@link ChangedFileSubject}s. */
public static Subject.Factory<ChangedFileSubject, ChangedFile> changedFiles() {
return ChangedFileSubject::new;
diff --git a/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java b/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java
index 03948f1..3a81908 100644
--- a/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java
+++ b/java/com/google/gerrit/plugins/codeowners/testing/FileCodeOwnerStatusSubject.java
@@ -20,11 +20,13 @@
import static com.google.gerrit.plugins.codeowners.testing.PathCodeOwnerStatusSubject.pathCodeOwnerStatuses;
import static com.google.gerrit.truth.OptionalSubject.optionals;
+import com.google.common.collect.ImmutableList;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.Subject;
import com.google.gerrit.plugins.codeowners.backend.FileCodeOwnerStatus;
import com.google.gerrit.truth.ListSubject;
import com.google.gerrit.truth.OptionalSubject;
+import java.util.Collection;
import java.util.stream.Stream;
/** {@link Subject} for doing assertions on {@link FileCodeOwnerStatus}es. */
@@ -39,6 +41,13 @@
fileCodeOwnerStatuses.collect(toImmutableList()), fileCodeOwnerStatuses());
}
+ /** Starts fluent chain to do assertions on a collection of {@link FileCodeOwnerStatus}es. */
+ public static ListSubject<FileCodeOwnerStatusSubject, FileCodeOwnerStatus> assertThatCollection(
+ Collection<FileCodeOwnerStatus> fileCodeOwnerStatuses) {
+ return ListSubject.assertThat(
+ ImmutableList.copyOf(fileCodeOwnerStatuses), fileCodeOwnerStatuses());
+ }
+
private static Factory<FileCodeOwnerStatusSubject, FileCodeOwnerStatus> fileCodeOwnerStatuses() {
return FileCodeOwnerStatusSubject::new;
}
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 64b24f8..531f118 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerStatusIT.java
@@ -78,6 +78,100 @@
}
@Test
+ public void getStatusForRenamedFile() throws Exception {
+ TestAccount user2 = accountCreator.user2();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/bar")
+ .addCodeOwnerEmail(user.email())
+ .create();
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/baz")
+ .addCodeOwnerEmail(user2.email())
+ .create();
+
+ String oldPath = "foo/bar/abc.txt";
+ String newPath = "foo/baz/abc.txt";
+ String changeId = createChangeWithFileRename(oldPath, newPath);
+
+ CodeOwnerStatusInfo codeOwnerStatus =
+ changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus();
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().hasSize(2);
+ FileCodeOwnerStatusInfoSubject fileCodeOwnerStatusInfoSubject1 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(0);
+ fileCodeOwnerStatusInfoSubject1.hasChangeTypeThat().isEqualTo(ChangeType.DELETED);
+ fileCodeOwnerStatusInfoSubject1.hasOldPathStatusThat().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusInfoSubject1
+ .hasOldPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ FileCodeOwnerStatusInfoSubject fileCodeOwnerStatusInfoSubject2 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(1);
+ fileCodeOwnerStatusInfoSubject2.hasChangeTypeThat().isEqualTo(ChangeType.ADDED);
+ fileCodeOwnerStatusInfoSubject2.hasNewPathStatusThat().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusInfoSubject2
+ .hasNewPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add a reviewer that is a code owner of the old path.
+ gApi.changes().id(changeId).addReviewer(user.email());
+
+ codeOwnerStatus = changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus();
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().hasSize(2);
+ fileCodeOwnerStatusInfoSubject1 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(0);
+ fileCodeOwnerStatusInfoSubject1.hasChangeTypeThat().isEqualTo(ChangeType.DELETED);
+ fileCodeOwnerStatusInfoSubject1.hasOldPathStatusThat().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusInfoSubject1
+ .hasOldPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ fileCodeOwnerStatusInfoSubject2 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(1);
+ fileCodeOwnerStatusInfoSubject2.hasChangeTypeThat().isEqualTo(ChangeType.ADDED);
+ fileCodeOwnerStatusInfoSubject2.hasNewPathStatusThat().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusInfoSubject2
+ .hasNewPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+ // Add a reviewer that is a code owner of the new path.
+ gApi.changes().id(changeId).addReviewer(user2.email());
+
+ codeOwnerStatus = changeCodeOwnersApiFactory.change(changeId).getCodeOwnerStatus();
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().hasSize(2);
+ fileCodeOwnerStatusInfoSubject1 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(0);
+ fileCodeOwnerStatusInfoSubject1.hasChangeTypeThat().isEqualTo(ChangeType.DELETED);
+ fileCodeOwnerStatusInfoSubject1.hasOldPathStatusThat().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusInfoSubject1
+ .hasOldPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ fileCodeOwnerStatusInfoSubject2 =
+ assertThat(codeOwnerStatus).hasFileCodeOwnerStatusesThat().element(1);
+ fileCodeOwnerStatusInfoSubject2.hasChangeTypeThat().isEqualTo(ChangeType.ADDED);
+ fileCodeOwnerStatusInfoSubject2.hasNewPathStatusThat().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusInfoSubject2
+ .hasNewPathStatusThat()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ }
+
+ @Test
public void getCodeOwnerStatusIfCodeOwnersFunctionalityIsDisabled() throws Exception {
disableCodeOwnersForProject(project);
String path = "foo/bar.baz";
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
index 3cf1aaf..34e0cb2 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
@@ -17,6 +17,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.plugins.codeowners.testing.ChangedFileSubject.assertThat;
+import static com.google.gerrit.plugins.codeowners.testing.ChangedFileSubject.assertThatCollection;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.junit.Assert.fail;
@@ -35,6 +36,7 @@
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.common.ChangedFile;
import com.google.gerrit.plugins.codeowners.common.MergeCommitStrategy;
+import com.google.gerrit.plugins.codeowners.testing.ChangedFileSubject;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
@@ -135,13 +137,21 @@
gApi.changes().id(changeId).current().files();
+ // A renamed file is reported as addition of new path + deletion of old path. This is because
+ // ChangedFiles uses a DiffFormatter without rename detection (because rename detection requires
+ // loading the file contents which is too expensive).
ImmutableSet<ChangedFile> changedFilesSet = changedFiles.compute(getRevisionResource(changeId));
- assertThat(changedFilesSet).hasSize(1);
- ChangedFile changedFile = Iterables.getOnlyElement(changedFilesSet);
- assertThat(changedFile).hasNewPath().value().isEqualTo(Paths.get(newPath));
- assertThat(changedFile).hasOldPath().value().isEqualTo(Paths.get(oldPath));
- assertThat(changedFile).isRename();
- assertThat(changedFile).isNoDeletion();
+ assertThat(changedFilesSet).hasSize(2);
+ ChangedFileSubject changedFile1 = assertThatCollection(changedFilesSet).element(0);
+ changedFile1.hasNewPath().value().isEqualTo(Paths.get(newPath));
+ changedFile1.hasOldPath().isEmpty();
+ changedFile1.isNoRename();
+ changedFile1.isNoDeletion();
+ ChangedFileSubject changedFile2 = assertThatCollection(changedFilesSet).element(1);
+ changedFile2.hasNewPath().isEmpty();
+ changedFile2.hasOldPath().value().isEqualTo(Paths.get(oldPath));
+ changedFile2.isNoRename();
+ changedFile2.isDeletion();
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
index f2c55e4..d23f647 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusSubject.assertThat;
+import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusSubject.assertThatCollection;
import static com.google.gerrit.plugins.codeowners.testing.FileCodeOwnerStatusSubject.assertThatStream;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -187,24 +188,29 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject1
.hasNewPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject2
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isDeletion();
}
@Test
@@ -321,24 +327,29 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.PENDING);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -358,24 +369,29 @@
requestScopeOperations.setApiUser(user2.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.PENDING);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.PENDING);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -473,24 +489,29 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.APPROVED);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -506,24 +527,29 @@
requestScopeOperations.setApiUser(user.id());
recommend(changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.APPROVED);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
.isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.APPROVED);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -672,18 +698,13 @@
String changeId = createChangeWithFileRename(oldPath, newPath);
amendChange(user, changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
- .hasNewPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
.hasOldPathStatus()
.value()
.hasStatusThat()
@@ -691,8 +712,18 @@
implicitApprovalsEnabled
? CodeOwnerStatus.APPROVED
: CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
+ .hasNewPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
@@ -719,12 +750,23 @@
String changeId = createChangeWithFileRename(oldPath, newPath);
amendChange(user, changeId);
- Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
- codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
- FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
- assertThatStream(fileCodeOwnerStatuses).onlyElement();
- fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
- fileCodeOwnerStatusSubject
+ ImmutableList<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+ codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId)).collect(toImmutableList());
+ assertThatCollection(fileCodeOwnerStatuses).hasSize(2);
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject1 =
+ assertThatCollection(fileCodeOwnerStatuses).element(0);
+ fileCodeOwnerStatusSubject1.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
+ fileCodeOwnerStatusSubject1
+ .hasOldPathStatus()
+ .value()
+ .hasStatusThat()
+ .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+ fileCodeOwnerStatusSubject1.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject1.hasChangedFile().isDeletion();
+ FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject2 =
+ assertThatCollection(fileCodeOwnerStatuses).element(1);
+ fileCodeOwnerStatusSubject2.hasNewPathStatus().value().hasPathThat().isEqualTo(newPath);
+ fileCodeOwnerStatusSubject2
.hasNewPathStatus()
.value()
.hasStatusThat()
@@ -732,14 +774,8 @@
implicitApprovalsEnabled
? CodeOwnerStatus.APPROVED
: CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasOldPathStatus().value().hasPathThat().isEqualTo(oldPath);
- fileCodeOwnerStatusSubject
- .hasOldPathStatus()
- .value()
- .hasStatusThat()
- .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
- fileCodeOwnerStatusSubject.hasChangedFile().isRename();
- fileCodeOwnerStatusSubject.hasChangedFile().isNoDeletion();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoRename();
+ fileCodeOwnerStatusSubject2.hasChangedFile().isNoDeletion();
}
@Test
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md
index dc5ac94..8207d33 100644
--- a/resources/Documentation/config.md
+++ b/resources/Documentation/config.md
@@ -147,7 +147,7 @@
(e.g. because they are bots and cannot react to review requests), they
can be added to the `Service Users` group (since members of this group
are not suggested as code owners).\
- Can be specified multiple time to set multiple global code owners.\
+ Can be specified multiple times to set multiple global code owners.\
Can be overridden per project by setting
[codeOwners.globalCodeOwner](#codeOwnersGlobalCodeOwner) in
`@PLUGIN@.config`.\
@@ -543,7 +543,7 @@
(e.g. because they are bots and cannot react to review requests), they
can be added to the `Service Users` group (since members of this group
are not suggested as code owners).\
- Can be specified multiple time to set multiple global code owners.\
+ Can be specified multiple times to set multiple global code owners.\
Overrides the global setting
[plugin.@PLUGIN@.globalCodeOwner](#pluginCodeOwnersGlobalCodeOwner) in
`gerrit.config` and the `codeOwners.globalCodeOwner` setting from parent
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index c0e608b..29d5fd8 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -615,17 +615,6 @@
"path": "docs/todo.txt",
"status": "PENDING"
}
- },
- {
- "change_type": "RENAMED",
- "old_path_status" {
- "path": "user-introduction.txt",
- "status": "INSUFFICIENT_REVIEWERS"
- },
- "new_path_status" {
- "path": "docs/user-intro.md",
- "status": "APPROVED"
- }
}
]
}
@@ -949,7 +938,7 @@
| Field Name | | Description |
| ------------- | -------- | ----------- |
-| `change_type` | optional | The type of the file modification. Can be `ADDED`, `MODIFIED`, `DELETED`, `RENAMED` or `COPIED`. Not set if `MODIFIED`.
+| `change_type` | optional | The type of the file modification. Can be `ADDED`, `MODIFIED`, `DELETED`, `RENAMED` or `COPIED`. Not set if `MODIFIED`. Renamed files might appear as separate addition and deletion or with type=RENAMED. Copied files might appear as addition or with type=COPIED.
| `old_path_status` | optional | The code owner status for the old path as [PathCodeOwnerStatusInfo](#path-code-owner-status-info) entity. Only set if `change_type` is `DELETED` or `RENAMED`.
| `new_path_status` | optional | The code owner status for the new path as [PathCodeOwnerStatusInfo](#path-code-owner-status-info) entity. Not set if `change_type` is `DELETED`.
