Add predicate for sticky owner approvals
Introduce a new approval query operand, `already-approved-by_owners`,
that allows copying owner approvals across patch sets only when it is
safe to do so.
`AlreadyApprovedByOwnerOperand` registers a `UserInOperandFactory` for
the new operand, and `AlreadyApprovedByOwnerPredicate` implements the
matching logic. When evaluating a copy condition for an approval, the
predicate:
- Computes the set of files modified between the source and target
patch sets using `DiffOperations`.
- Checks whether any of the newly modified files are owned by the
current approver. If so, the approval is not copied and the owner
must re-approve.
- If none of the newly modified files are owned by the approver, copies
the approval only when the approver has owned at least one file in the
previous patchset.
To support this, `GetFilesOwners` is extended with an `isAnyFileOwnedBy`
helper that computes ownership for a given set of paths, and the
construction of `PathOwners` is factored into a private `getPathOwners`
method reused by both the REST endpoint and the new predicate.
Finally, `OwnersModule` is updated to install the new operand module so
that the `alreadyApprovedBy` operand is available to approval query
expressions.
Bug: Issue 458084508
Change-Id: I1fb57d70756e1a04cae19475afe80b2a329a4d11
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/AlreadyApprovedByOperand.java b/owners/src/main/java/com/googlesource/gerrit/owners/AlreadyApprovedByOperand.java
new file mode 100644
index 0000000..2f90db1
--- /dev/null
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/AlreadyApprovedByOperand.java
@@ -0,0 +1,57 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.owners;
+
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.patch.DiffOperations;
+import com.google.gerrit.server.query.approval.ApprovalContext;
+import com.google.gerrit.server.query.approval.ApprovalQueryBuilder.UserInOperandFactory;
+import com.google.gerrit.server.query.approval.UserInPredicate;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.owners.restapi.GetFilesOwners;
+
+@Singleton
+public class AlreadyApprovedByOperand implements UserInOperandFactory {
+ public static final String OPERAND = "already-approved-by";
+ private final GetFilesOwners getFilesOwners;
+ private final DiffOperations diffOperations;
+
+ public static class Module extends AbstractModule {
+
+ @Override
+ protected void configure() {
+ bind(UserInOperandFactory.class)
+ .annotatedWith(Exports.named(OPERAND))
+ .to(AlreadyApprovedByOperand.class);
+ }
+ }
+
+ public static String FULL_OPERAND_WITH_PLUGIN_NAME = String.format("%s_%s", OPERAND, "owners");
+
+ @Inject
+ AlreadyApprovedByOperand(GetFilesOwners getFilesOwners, DiffOperations diffOperations) {
+ this.getFilesOwners = getFilesOwners;
+ this.diffOperations = diffOperations;
+ }
+
+ @Override
+ public Predicate<ApprovalContext> create(UserInPredicate.Field field) throws QueryParseException {
+ return new AlreadyApprovedByPredicate(getFilesOwners, diffOperations, field);
+ }
+}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/AlreadyApprovedByPredicate.java b/owners/src/main/java/com/googlesource/gerrit/owners/AlreadyApprovedByPredicate.java
new file mode 100644
index 0000000..f7ee5b0
--- /dev/null
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/AlreadyApprovedByPredicate.java
@@ -0,0 +1,159 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.owners;
+
+import static com.google.common.base.Preconditions.checkState;
+import static com.googlesource.gerrit.owners.AlreadyApprovedByOperand.FULL_OPERAND_WITH_PLUGIN_NAME;
+import static com.googlesource.gerrit.owners.AlreadyApprovedByOperand.OPERAND;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.query.Matchable;
+import com.google.gerrit.index.query.OperatorPredicate;
+import com.google.gerrit.server.git.InMemoryInserter;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.gerrit.server.patch.DiffOperations;
+import com.google.gerrit.server.patch.gitdiff.ModifiedFile;
+import com.google.gerrit.server.query.approval.ApprovalContext;
+import com.google.gerrit.server.query.approval.UserInPredicate;
+import com.googlesource.gerrit.owners.common.InvalidOwnersFileException;
+import com.googlesource.gerrit.owners.restapi.GetFilesOwners;
+import java.io.IOException;
+import java.util.Map;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+public class AlreadyApprovedByPredicate extends OperatorPredicate<ApprovalContext>
+ implements Matchable<ApprovalContext> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final GetFilesOwners getFilesOwners;
+ private final DiffOperations diffOperations;
+ private final UserInPredicate.Field predicateField;
+
+ private static final boolean DISABLE_RENAME_DETECTION = false;
+
+ public AlreadyApprovedByPredicate(
+ GetFilesOwners getFilesOwners,
+ DiffOperations diffOperations,
+ UserInPredicate.Field predicateField) {
+ super("approverin", OPERAND);
+ this.getFilesOwners = getFilesOwners;
+ this.diffOperations = diffOperations;
+ this.predicateField = predicateField;
+ }
+
+ @Override
+ public boolean match(ApprovalContext ctx) {
+ try {
+ Account.Id currentApprover = ctx.approverId();
+ Project.NameKey project = ctx.changeData().project();
+ PatchSet targetPatchSet = ctx.targetPatchSet();
+ PatchSet sourcePatchSet = ctx.changeNotes().getPatchSets().get(ctx.sourcePatchSetId());
+
+ checkState(
+ predicateField == UserInPredicate.Field.APPROVER,
+ "%s copy-condition can only be applied to `approverin:` predicates. Label %s for user %s"
+ + " will NOT be copied.",
+ FULL_OPERAND_WITH_PLUGIN_NAME,
+ ctx.labelType().getName(),
+ currentApprover);
+
+ checkState(
+ sourcePatchSet != null,
+ "Could not get source patch-set for %s for project %s",
+ ctx.sourcePatchSetId(),
+ project);
+
+ Map<String, ModifiedFile> priorVsCurrent =
+ diffOperations.loadModifiedFilesIfNecessary(
+ project,
+ sourcePatchSet.commitId(),
+ targetPatchSet.commitId(),
+ ctx.repoView().getRevWalk(),
+ ctx.repoView().getConfig(),
+ DISABLE_RENAME_DETECTION);
+
+ boolean newPatchSetHasFilesOwnedByMe =
+ getFilesOwners.isAnyFileOwnedBy(
+ currentApprover,
+ priorVsCurrent.keySet(),
+ project,
+ ctx.changeData().branchOrThrow().branch());
+
+ if (newPatchSetHasFilesOwnedByMe) {
+ logger.atFinest().log(
+ "Approver '%s' owns files that were changed in this new patch set, must re-approve",
+ currentApprover);
+ return false;
+ }
+
+ // The new patchSet has not modified anything I own.
+ // I will copy my label, but only if I used to own something in the change.
+ try (ObjectInserter ins =
+ new InMemoryInserter(ctx.repoView().getRevWalk().getObjectReader())) {
+
+ Map<String, ModifiedFile> baseVsPrior =
+ diffOperations.loadModifiedFilesAgainstParentIfNecessary(
+ project,
+ sourcePatchSet.commitId(),
+ getParentNum(targetPatchSet.commitId(), ctx.repoView().getRevWalk()),
+ ctx.repoView(),
+ ins,
+ DISABLE_RENAME_DETECTION);
+ boolean oldPatchSetHasFilesOwnedByMe =
+ getFilesOwners.isAnyFileOwnedBy(
+ currentApprover,
+ baseVsPrior.keySet(),
+ project,
+ ctx.changeData().branchOrThrow().branch());
+
+ logger.atFinest().log(
+ "Has approver '%s' ever owned anything in this change? %s",
+ currentApprover,
+ oldPatchSetHasFilesOwnedByMe
+ ? "yes, will copy approval"
+ : "No, will not copy approval");
+
+ return oldPatchSetHasFilesOwnedByMe;
+ }
+ } catch (DiffNotAvailableException | IOException | InvalidOwnersFileException e) {
+ throw new StorageException(
+ String.format(
+ "Failed to compute %s, label will not be copied.", FULL_OPERAND_WITH_PLUGIN_NAME),
+ e);
+ }
+ }
+
+ private int getParentNum(ObjectId objectId, RevWalk revWalk) {
+ try {
+ RevCommit commit = revWalk.parseCommit(objectId);
+ // merge commit with 2 or more parents: must use parentNum = 1 to compare against the first
+ // parent. Using parentNum = 0 would compare against the auto-merge.
+ return commit.getParentCount() > 1 ? 1 : 0;
+ } catch (IOException ex) {
+ throw new StorageException(ex);
+ }
+ }
+
+ @Override
+ public int getCost() {
+ return 10;
+ }
+}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
index 2d5a2b0..3e38324 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
@@ -41,6 +41,7 @@
.asEagerSingleton();
install(new OwnersRestApiModule());
install(new OwnersApprovalHasOperand.OwnerApprovalHasOperandModule());
+ install(new AlreadyApprovedByOperand.Module());
if (pluginSettings.enableSubmitRequirement()) {
install(new OwnersSubmitRequirement.OwnersSubmitRequirementModule());
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
index 1327fcb..d13e709 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -46,6 +46,7 @@
import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
import com.googlesource.gerrit.owners.entities.GroupOwner;
import com.googlesource.gerrit.owners.entities.Owner;
+import java.io.IOException;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
@@ -90,6 +91,16 @@
this.cache = cache;
}
+ public boolean isAnyFileOwnedBy(
+ Account.Id owner, Set<String> changePaths, Project.NameKey project, String branch)
+ throws IOException, InvalidOwnersFileException {
+ PathOwners owners = getPathOwners(project, branch, changePaths);
+ Map<String, Set<Account.Id>> filesWithOwner = owners.getFileOwners();
+
+ return changePaths.stream()
+ .anyMatch(filePath -> filesWithOwner.getOrDefault(filePath, Set.of()).contains(owner));
+ }
+
@Override
public Response<FilesOwnersResponse> apply(RevisionResource revision)
throws AuthException, BadRequestException, ResourceConflictException, Exception {
@@ -97,25 +108,11 @@
ChangeData changeData = revision.getChangeResource().getChangeData();
Project.NameKey project = change.getProject();
- List<Project.NameKey> projectParents =
- projectCache.get(project).map(PathOwners::getParents).orElse(Collections.emptyList());
-
- try (Repository repository = repositoryManager.openRepository(project)) {
+ try {
Set<String> changePaths = new HashSet<>(changeData.currentFilePaths());
String branch = change.getDest().branch();
- PathOwners owners =
- new PathOwners(
- accounts,
- repositoryManager,
- repository,
- projectParents,
- pluginSettings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
- changePaths,
- pluginSettings.expandGroups(),
- project.get(),
- cache,
- pluginSettings.globalLabel());
+ PathOwners owners = getPathOwners(project, branch, changePaths);
Map<String, Set<GroupOwner>> fileExpandedOwners =
Maps.transformValues(
@@ -160,6 +157,25 @@
}
}
+ private PathOwners getPathOwners(Project.NameKey project, String branch, Set<String> changePaths)
+ throws InvalidOwnersFileException, IOException {
+ List<Project.NameKey> projectParents =
+ projectCache.get(project).map(PathOwners::getParents).orElse(Collections.emptyList());
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ return new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ projectParents,
+ pluginSettings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
+ changePaths,
+ pluginSettings.expandGroups(),
+ project.get(),
+ cache,
+ pluginSettings.globalLabel());
+ }
+ }
+
private LabelAndScore getLabelDefinition(PathOwners owners, ChangeData changeData)
throws ResourceNotFoundException {
diff --git a/owners/src/main/resources/Documentation/copy-conditions.md b/owners/src/main/resources/Documentation/copy-conditions.md
new file mode 100644
index 0000000..de970f1
--- /dev/null
+++ b/owners/src/main/resources/Documentation/copy-conditions.md
@@ -0,0 +1,80 @@
+@PLUGIN@ CopyCondition
+======================
+
+The @PLUGIN@ plugin provides an
+additional [copy condition](https://gerrit-review.googlesource.com/Documentation/config-labels.html#label_copyCondition)
+operand that can be used in label definitions to control when owner approvals should be preserved
+across patch sets.
+
+This document describes the behavior of the operand, its purpose, and how to
+configure it.
+
+## approverin:already-approved-by_owners
+
+This operand enables approval copying based on file ownership and patch-set differences.
+It allows owner approvals to be copied over if the owned files in the latest patch set haven't been
+modified.
+
+By doing so, it reduces unnecessary re-review, allowing owners to retain their previous approval
+when their files are unchanged.
+
+When evaluating a label's `copyCondition`, the operand `approverin:already-approved-by_owners`
+returns true only when:
+
+1. The approval was previously given by a user who owns at least one file in the change; and
+2. The newly uploaded patch set does not modify any files owned by that user.
+
+If a patch set updates files that the approver owns, the approval is cleared and the owner is
+expected to review the new changes.
+
+**NOTE**: this operand is only supported for `approverin:` predicates. Using it with an `uploaderin`
+predicate will result in the label **not** being copied (and an error emitted in the logs).
+
+## Example Scenarios
+
+Given an `OWNERS` file that splits ownerships between `frontend` users and `backend` users based on
+file matchers:
+
+```text
+inherited: true
+matchers:
+- suffix: .js
+ owners:
+ - user-frontend
+- suffix: .java
+ owners:
+ - user-backend
+```
+
+### Approval copied
+
+* A `user-frontend` owner gives `Code-Review +2` on `Patch Set 1`.
+* `Patch Set 2` updates only backend files.
+* The `user-frontend` owner does not own any of the modified files in the new Patch Set.
+
+**Result**: The `Code-Review +2` is copied forward.
+
+### Approval not copied
+
+* A `user-backend` owner gives `Code-Review +2` on `Patch Set 3`.
+* `Patch Set 4` modifies a backend-owned file.
+
+**Result**: The approval is cleared because the owner must review the new changes.
+
+## Label configuration
+
+The copy condition is a `label` property and as such needs to be set in the label configuration.
+The following is an example of the `approverin:already-approved-by_owners` configured for the
+`Code-Review` label.
+
+```text
+[label "Code-Review"]
+ function = NoBlock
+ defaultValue = 0
+ value = -2 This shall not be submitted
+ value = -1 I would prefer this is not submitted as is
+ value = 0 No score
+ value = +1 Looks good to me, but someone else must approve
+ value = +2 Looks good to me, approved
+ copyCondition = approverin:already-approved-by_owners
+```
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/AlreadyApprovedByCopyConditionIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/AlreadyApprovedByCopyConditionIT.java
new file mode 100644
index 0000000..0e2ea6e
--- /dev/null
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/AlreadyApprovedByCopyConditionIT.java
@@ -0,0 +1,280 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.owners;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_COMMIT;
+import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
+import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.googlesource.gerrit.owners.AlreadyApprovedByOperand.FULL_OPERAND_WITH_PLUGIN_NAME;
+import static java.util.stream.Collectors.joining;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.common.ApprovalInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.server.project.testing.TestLabels;
+import com.google.inject.Inject;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
+import org.junit.Before;
+import org.junit.Test;
+
+@TestPlugin(name = "owners", sysModule = "com.googlesource.gerrit.owners.OwnersModule")
+@UseLocalDisk
+public class AlreadyApprovedByCopyConditionIT extends LightweightPluginDaemonTest {
+ @Inject protected RequestScopeOperations requestScopeOperations;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private ChangeOperations changeOperations;
+
+ private TestAccount FRONTEND_FILES_OWNER;
+ private TestAccount BACKEND_FILES_OWNER;
+
+ private static final String FRONTEND_OWNED_FILE = "foo.js";
+ private static final String BACKEND_OWNED_FILE = "foo.java";
+ private static final String FILE_WITH_NO_OWNERS = "foo.txt";
+
+ @Before
+ public void setup() throws Exception {
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .add(
+ allowLabel(TestLabels.codeReview().getName())
+ .ref(RefNames.REFS_HEADS + "*")
+ .group(REGISTERED_USERS)
+ .range(-2, 2))
+ .update();
+
+ FRONTEND_FILES_OWNER = accountCreator.create("user-frontend");
+ BACKEND_FILES_OWNER = accountCreator.create("user-backend");
+
+ addOwnerFileWithMatchersToRoot(
+ Map.of(
+ ".js", List.of(FRONTEND_FILES_OWNER),
+ ".java", List.of(BACKEND_FILES_OWNER)));
+
+ updateLabel(b -> b.setCopyCondition("approverin:" + FULL_OPERAND_WITH_PLUGIN_NAME));
+ }
+
+ @Test
+ public void shouldCopyOwnerApprovalOnlyWhenOwnedFilesAreUnchanged() throws Exception {
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .file(FRONTEND_OWNED_FILE)
+ .content("some frontend change")
+ .create();
+
+ vote(FRONTEND_FILES_OWNER, changeId.toString(), 2);
+ vote(BACKEND_FILES_OWNER, changeId.toString(), 2);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file(BACKEND_OWNED_FILE)
+ .content("some java content")
+ .create();
+
+ ChangeInfo c = detailedChange(changeId.toString());
+ assertVotes(c, FRONTEND_FILES_OWNER, 2);
+ assertVotes(c, BACKEND_FILES_OWNER, 0);
+ }
+
+ @Test
+ public void shouldNotCopyApprovalForOwnerWhenNoOwnedFileExists() throws Exception {
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .file(FILE_WITH_NO_OWNERS)
+ .content("file with no owners")
+ .create();
+
+ vote(FRONTEND_FILES_OWNER, changeId.toString(), 2);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file(FILE_WITH_NO_OWNERS)
+ .content("updated text")
+ .create();
+
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ assertVotes(c, FRONTEND_FILES_OWNER, 0);
+ }
+
+ @Test
+ public void shouldNotCopyApprovalWhenOwnedFilesAreIntroducedInLaterPatchSet() throws Exception {
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .file(FILE_WITH_NO_OWNERS)
+ .content("file with no owners")
+ .create();
+
+ vote(BACKEND_FILES_OWNER, changeId.toString(), 2);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file(BACKEND_OWNED_FILE)
+ .content("some java content")
+ .create();
+
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ assertVotes(c, BACKEND_FILES_OWNER, 0);
+ }
+
+ @Test
+ public void shouldNotCopyApprovalWhenOwnedFileIsDeleted() throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().file(BACKEND_OWNED_FILE).content("java content").create();
+
+ vote(BACKEND_FILES_OWNER, changeId.toString(), 2);
+
+ changeOperations.change(changeId).newPatchset().file(BACKEND_OWNED_FILE).delete().create();
+
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ assertVotes(c, BACKEND_FILES_OWNER, 0);
+ }
+
+ @Test
+ public void shouldNotCopyApprovalWhenOwnedFileIsRenamedToOwnedFile() throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().file(BACKEND_OWNED_FILE).content("java content").create();
+
+ vote(BACKEND_FILES_OWNER, changeId.toString(), 2);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file(BACKEND_OWNED_FILE)
+ .renameTo("renamed-" + BACKEND_OWNED_FILE)
+ .create();
+
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ assertVotes(c, BACKEND_FILES_OWNER, 0);
+ }
+
+ @Test
+ public void shouldNotCopyApprovalWhenOwnedFileIsRenamedToNonOwnedFile() throws Exception {
+ Change.Id changeId =
+ changeOperations.newChange().file(BACKEND_OWNED_FILE).content("java content").create();
+
+ vote(BACKEND_FILES_OWNER, changeId.toString(), 2);
+
+ changeOperations
+ .change(changeId)
+ .newPatchset()
+ .file(BACKEND_OWNED_FILE)
+ .renameTo(FILE_WITH_NO_OWNERS)
+ .create();
+
+ ChangeInfo c = detailedChange(changeId.toString());
+
+ assertVotes(c, BACKEND_FILES_OWNER, 0);
+ }
+
+ private ChangeInfo detailedChange(String changeId) throws Exception {
+ return gApi.changes().id(changeId).get(DETAILED_LABELS, CURRENT_REVISION, CURRENT_COMMIT);
+ }
+
+ private void assertVotes(ChangeInfo c, TestAccount user, int expectedVote) {
+ Integer vote = 0;
+ if (c.labels.get(LabelId.CODE_REVIEW) != null
+ && c.labels.get(LabelId.CODE_REVIEW).all != null) {
+ for (ApprovalInfo approval : c.labels.get(LabelId.CODE_REVIEW).all) {
+ if (approval._accountId == user.id().get()) {
+ vote = approval.value;
+ break;
+ }
+ }
+ }
+
+ assertThat(vote).isEqualTo(expectedVote);
+ }
+
+ private void vote(TestAccount user, String changeId, int vote) throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes()
+ .id(changeId)
+ .current()
+ .review(new ReviewInput().label(LabelId.CODE_REVIEW, vote));
+ }
+
+ private void updateLabel(Consumer<LabelType.Builder> update) throws Exception {
+ try (ProjectConfigUpdate u = updateProject(allProjects)) {
+ u.getConfig().updateLabelType(LabelId.CODE_REVIEW, update);
+ u.save();
+ }
+ }
+
+ private void addOwnerFileWithMatchersToRoot(Map<String, List<TestAccount>> ownersBySuffix)
+ throws Exception {
+ // Example generated OWNERS:
+ //
+ // inherited: true
+ // matchers:
+ // - suffix: .js
+ // owners:
+ // - fe1@example.com
+ // - fe2@example.com
+ // - suffix: .java
+ // owners:
+ // - be1@example.com
+ // - be2@example.com
+ //
+ String matchersYaml =
+ ownersBySuffix.entrySet().stream()
+ .map(
+ entry -> {
+ String suffix = entry.getKey();
+ List<TestAccount> users = entry.getValue();
+ String ownersYaml =
+ users.stream()
+ .map(user -> String.format(" - %s\n", user.username()))
+ .collect(joining());
+ return String.format("- suffix: %s\n owners:\n%s", suffix, ownersYaml);
+ })
+ .collect(joining());
+
+ pushOwnersToMaster(String.format("inherited: %s\nmatchers:\n%s", true, matchersYaml));
+ }
+
+ private void pushOwnersToMaster(String owners) throws Exception {
+ pushFactory
+ .create(admin.newIdent(), testRepo, "Add OWNER file", "OWNERS", owners)
+ .to(RefNames.fullName("master"))
+ .assertOkStatus();
+ }
+}
