CodeOwnersInChangeCollection: Get changed paths without rename detection
CodeOwnersInChangeCollection only needs to check whether the path exists
as a new path or as an old path of a rename or deletion. Since old paths
of deletions and renames are handled the same way we do not need to
detect renames. If we get the changed files without rename detection,
renames are returned as 2 changed files, one with the new path for the
addition and one with the old path for the deletion.
Change-Id: I9a857e0bf17262a7be56c7a2affcc2cbe4f12694
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
index 6c77dd0..31491fb 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/ChangedFiles.java
@@ -114,7 +114,7 @@
/**
* Gets the changed files.
*
- * <p>Rename detection is enabled.
+ * <p>Rename detection is disabled.
*
* <p>Uses the configured merge commit strategy.
*
@@ -123,10 +123,11 @@
* @throws IOException thrown if the computation fails due to an I/O error
* @see #get(Project.NameKey, ObjectId, MergeCommitStrategy)
*/
- public ImmutableList<ChangedFile> get(RevisionResource revisionResource)
+ public ImmutableList<ChangedFile> getWithoutRenameDetection(RevisionResource revisionResource)
throws IOException, DiffNotAvailableException {
requireNonNull(revisionResource, "revisionResource");
- return get(revisionResource.getProject(), revisionResource.getPatchSet().commitId());
+ return getWithoutRenameDetection(
+ revisionResource.getProject(), revisionResource.getPatchSet().commitId());
}
/**
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInChangeCollection.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInChangeCollection.java
index 53473f2..ec91b94 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInChangeCollection.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnersInChangeCollection.java
@@ -105,19 +105,26 @@
private void checkThatFileExists(
RevisionResource revisionResource, PathResource pathResource, IdString id)
throws RestApiException, IOException, DiffNotAvailableException {
- if (!changedFiles.get(revisionResource).stream()
+ // We only need to check whether the path exists as a new path or as an old path of a rename or
+ // deletion. Since old paths of deletions and renames are handled the same way we do not need to
+ // detect renames. If we get the changed files without rename detection, renames are returned as
+ // 2 changed files, one with the new path for the addition and one with the old path for the
+ // deletion.
+ if (!changedFiles.getWithoutRenameDetection(revisionResource).stream()
.anyMatch(
changedFile ->
// Check whether the path matches any file in the change.
changedFile.hasNewPath(pathResource.getPath())
- // For renamed and deleted files we also accept requests for the old path.
- // Listing code owners for the old path of renamed/deleted files should be
+ // Since the rename detection is disabled renamed files are returned as addition
+ // + deletion.
+ // For deleted files (includes renamed files) we accept requests for the old
+ // path. Listing code owners for the old path of deleted/renamed files should be
// possible because these files require a code owner approval on the old path
// for submit and users need to know whom they need to add as reviewer for this.
// For copied files the old path is not modified and hence no code owner
// approval for the old path is required. This is why users do not need to get
// code owners for the old path in case of copy.
- || ((changedFile.isRename() || changedFile.isDeletion())
+ || (changedFile.isDeletion()
&& changedFile.hasOldPath(pathResource.getPath())))) {
// Throw the exception with the path we got as input.
throw new ResourceNotFoundException(id);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
index b225828..5df0afb 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/ChangedFilesTest.java
@@ -69,7 +69,8 @@
public void cannotGetForNullRevisionResource() throws Exception {
NullPointerException npe =
assertThrows(
- NullPointerException.class, () -> changedFiles.get(/* revisionResource= */ null));
+ NullPointerException.class,
+ () -> changedFiles.getWithoutRenameDetection(/* revisionResource= */ null));
assertThat(npe).hasMessageThat().isEqualTo("revisionResource");
}
