Do not run the code owners submit rule for closed changes
Running the code owners submit rule is rather expensive, since we need
to compute the code owner statuses for all files which have been touched
in the change, which requires loading the relevant OWNERS files for each
of the files. Submit rules are also invoked for closed changes (e.g.
each time a change is reindexed), but the code owner status on closed
changes is not relevant, hence we should not compute it.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I6060d8788cb7ba9136ad395ac8bffe402d8d5b5c
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
index 21f411c..2dc2d37 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRule.java
@@ -73,6 +73,10 @@
try {
requireNonNull(changeData, "changeData");
+ if (changeData.change().isClosed()) {
+ return Optional.empty();
+ }
+
try (Timer0.Context ctx = codeOwnerMetrics.runCodeOwnerSubmitRule.start()) {
logger.atFine().log(
"run code owner submit rule (project = %s, change = %d)",
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java
index b495bd4..1772809 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerSubmitRuleTest.java
@@ -20,6 +20,7 @@
import static org.mockito.Mockito.when;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.acceptance.testsuite.CodeOwnerConfigOperations;
import com.google.gerrit.plugins.codeowners.testing.SubmitRecordSubject;
@@ -50,6 +51,34 @@
}
@Test
+ public void emptyIfChangeIdClosed() throws Exception {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(user.email())
+ .create();
+
+ String path = "foo/bar.baz";
+ Change change = createChange("Change Adding A File", path, "file content").getChange().change();
+ String changeId = change.getKey().get();
+
+ // Add a Code-Review+1 from a code owner (by default this counts as code owner approval).
+ requestScopeOperations.setApiUser(user.id());
+ recommend(changeId);
+
+ // Approve and submit.
+ requestScopeOperations.setApiUser(admin.id());
+ approve(changeId);
+ gApi.changes().id(changeId).current().submit();
+
+ // Run the code owners submit rule on the closed change.
+ ChangeData changeData = changeDataFactory.create(project, change.getId());
+ assertThat(codeOwnerSubmitRule.evaluate(changeData)).isEmpty();
+ }
+
+ @Test
public void notReady() throws Exception {
ChangeData changeData = createChange().getChange();
SubmitRecordSubject submitRecordSubject =
