Merge "Consider change ETag computations from plugins when computing ETag for actions"

2 files changed, 38 insertions, 7 deletions

diff --git a/java/com/google/gerrit/server/change/ChangeResource.java b/java/com/google/gerrit/server/change/ChangeResource.java
index bb0040de15..d8d82c628d 100644
--- a/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/java/com/google/gerrit/server/change/ChangeResource.java
@@ -197,6 +197,14 @@ public class ChangeResource implements RestResource, HasETag {
     for (ProjectState p : projectStateTree) {
       hashObjectId(h, p.getConfig().getRevision(), buf);
     }
+
+    changeETagComputation.runEach(
+        c -> {
+          String pluginETag = c.getETag(notes.getProjectName(), notes.getChangeId());
+          if (pluginETag != null) {
+            h.putString(pluginETag, UTF_8);
+          }
+        });
   }
 
   @Override
@@ -206,13 +214,6 @@ public class ChangeResource implements RestResource, HasETag {
       h.putString(starredChangesUtil.getObjectId(user.getAccountId(), getId()).name(), UTF_8);
     }
     prepareETag(h, user);
-    changeETagComputation.runEach(
-        c -> {
-          String pluginETag = c.getETag(notes.getProjectName(), notes.getChangeId());
-          if (pluginETag != null) {
-            h.putString(pluginETag, UTF_8);
-          }
-        });
     return h.hash().toString();
   }
 
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 5fb42da750..94ad94f7d4 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -36,6 +36,7 @@ import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.registration.RegistrationHandle;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.change.ChangeETagComputation;
 import com.google.gerrit.server.change.RevisionJson;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.testing.ConfigSuite;
@@ -58,6 +59,7 @@ public class ActionsIT extends AbstractDaemonTest {
   @Inject private DynamicSet<ActionVisitor> actionVisitors;
   @Inject private RequestScopeOperations requestScopeOperations;
   @Inject private RevisionJson.Factory revisionJsonFactory;
+  @Inject private DynamicSet<ChangeETagComputation> changeETagComputations;
 
   private RegistrationHandle visitorHandle;
 
@@ -206,6 +208,34 @@ public class ActionsIT extends AbstractDaemonTest {
   }
 
   @Test
+  public void pluginCanContributeToETagComputation() throws Exception {
+    String change = createChange().getChangeId();
+    String oldETag = getETag(change);
+
+    RegistrationHandle registrationHandle = changeETagComputations.add("gerrit", (p, id) -> "foo");
+    try {
+      assertThat(getETag(change)).isNotEqualTo(oldETag);
+    } finally {
+      registrationHandle.remove();
+    }
+
+    assertThat(getETag(change)).isEqualTo(oldETag);
+  }
+
+  @Test
+  public void returningNullFromETagComputationDoesNotBreakGerrit() throws Exception {
+    String change = createChange().getChangeId();
+    String oldETag = getETag(change);
+
+    RegistrationHandle registrationHandle = changeETagComputations.add("gerrit", (p, id) -> null);
+    try {
+      assertThat(getETag(change)).isEqualTo(oldETag);
+    } finally {
+      registrationHandle.remove();
+    }
+  }
+
+  @Test
   public void revisionActionsTwoChangesInTopic_conflicting() throws Exception {
     String changeId = createChangeWithTopic().getChangeId();
     approve(changeId);