CodeOwnersOnPostReview: Do not log error if dest branch was deleted
It's possible that there are changes for branches that have been
deleted. In this case the owned files cannot be computed, but we should
not log an error in this case as it is an allowed change state that we
are handling correctly.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I69f0fee27f51844939fc7f61bd506b91e3cd4aaf
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java
index c26b821..2877c93 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java
@@ -22,7 +22,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval;
import com.google.gerrit.plugins.codeowners.common.CodeOwnerStatus;
@@ -261,7 +261,12 @@
.map(PathCodeOwnerStatus::path)
.sorted(comparing(Path::toString))
.collect(toImmutableList());
- } catch (IOException | ResourceConflictException | PatchListNotAvailableException e) {
+ } catch (RestApiException e) {
+ logger.atFine().withCause(e).log(
+ "Couldn't compute owned paths of change %s for account %s",
+ changeNotes.getChangeId(), accountId.get());
+ return ImmutableList.of();
+ } catch (IOException | PatchListNotAvailableException e) {
logger.atSevere().withCause(e).log(
"Failed to compute owned paths of change %s for account %s",
changeNotes.getChangeId(), accountId.get());
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
index 60bcf64..178b16b 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
@@ -24,7 +25,9 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.LabelDefinitionInput;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
@@ -898,4 +901,24 @@
Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Code-Review+1");
}
+
+ @Test
+ public void changeMessageNotExtendedIfDestinationBranchWasDeleted() throws Exception {
+ String branchName = "tempBranch";
+ createBranch(BranchNameKey.create(project, branchName));
+
+ String changeId = createChange("refs/for/" + branchName).getChangeId();
+
+ DeleteBranchesInput input = new DeleteBranchesInput();
+ input.branches = ImmutableList.of(branchName);
+ gApi.projects().name(project.get()).deleteBranches(input);
+
+ // Approve by a code-owner.
+ recommend(changeId);
+
+ // If the destination branch of the change no longer exits, the owned paths cannot be computed.
+ // Hence the change message cannot be extended in this case.
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Code-Review+1");
+ }
}
