Merge "Document and test that plugins can throw RuntimeExceptions from ETag computation"

java/com/google/gerrit/server/change/ChangeETagComputation.java

@@ -50,6 +50,11 @@ public interface ChangeETagComputation {
    * <p><strong>Note:</strong> Change ETags are computed very frequently and the computation must be
    * cheap. Take good care to not perform any expensive computations when implementing this.
    *
+   * <p>If an error is encountered during the ETag computation the plugin can indicate this by
+   * throwing any RuntimeException. In this case no value will be included in the change ETag
+   * computation. This means if the error is transient, the ETag will differ when the computation
+   * succeeds on a follow-up run.
+   *
    * @param projectName the name of the project that contains the change
    * @param changeId ID of the change for which the ETag should be computed
    * @return the ETag

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -87,6 +87,7 @@ import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.data.LabelFunction;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.annotations.Exports;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerResult;
@@ -2182,6 +2183,24 @@ public class ChangeIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void throwingExceptionFromETagComputationDoesNotBreakGerrit() throws Exception {
+    PushOneCommit.Result r = createChange();
+    String oldETag = parseResource(r).getETag();
+
+    RegistrationHandle registrationHandle =
+        changeETagComputations.add(
+            "gerrit",
+            (p, id) -> {
+              throw new StorageException("exception during test");
+            });
+    try {
+      assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
+    } finally {
+      registrationHandle.remove();
+    }
+  }
+
   @Test
   public void emailNotificationForFileLevelComment() throws Exception {
     String changeId = createChange().getChangeId();

javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java

@@ -25,6 +25,7 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.api.changes.ActionVisitor;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.client.ListChangesOption;
@@ -235,6 +236,24 @@ public class ActionsIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void throwingExceptionFromETagComputationDoesNotBreakGerrit() throws Exception {
+    String change = createChange().getChangeId();
+    String oldETag = getETag(change);
+
+    RegistrationHandle registrationHandle =
+        changeETagComputations.add(
+            "gerrit",
+            (p, id) -> {
+              throw new StorageException("exception during test");
+            });
+    try {
+      assertThat(getETag(change)).isEqualTo(oldETag);
+    } finally {
+      registrationHandle.remove();
+    }
+  }
+
   @Test
   public void revisionActionsTwoChangesInTopic_conflicting() throws Exception {
     String changeId = createChangeWithTopic().getChangeId();