opendev/gerrit
Code Issues Proposed changes

Document and test that plugins can throw RuntimeExceptions from ETag computation

Browse Source 
If an error is encountered during the ETag computation in a plugin, 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.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I08a91bfae5d37fe881ccee491f73ef5b35b4c180
This commit is contained in:
Edwin Kempin
2019-08-01 09:54:37 +02:00
parent aee4c00e54
commit 064eaebd54
3 changed files with 43 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
java/com/google/gerrit/server/change/ChangeETagComputation.java
View File

@@ -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

19
javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -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();

19
javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
View File

@@ -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();