diff options
author | Edwin Kempin <ekempin@google.com> | 2019-07-31 16:43:44 +0200 |
---|---|---|
committer | Edwin Kempin <ekempin@google.com> | 2019-07-31 16:44:48 +0200 |
commit | b615c84ad8d3250f851aa9c80938c08422b48975 (patch) | |
tree | aa16dfeeef643e711ac65a0364dc10fdf7c54108 | |
parent | 42b47b11c4b0dc79cb895cebeb241fcdf279b918 (diff) |
Consider change ETag computations from plugins when computing ETag for actions
Change I358e3d4fb added an extension point that allows plugins to
contribute to the ETag computation of changes. One case where plugins
need to contribute to the ETag computation is if they implement a
SubmitRule which is based on plugin data, because only then it is
guaranteed that callers always see current submittable information in
ChangeInfo.
In the case where a plugin implements a SubmitRule it is also important
to include the change ETag computations from plugins when the ETag for
revision actions is computed because the revision actions decide whether
the 'Submit' button should be shown. If change ETag computations from
plugins are not considered when the ETag for the revision actions is
computed it can happen that the submittable information in ChangeInfo
disagrees with the presence/absense of the 'Submit' button.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Id83533e7b67cc82737feb6615c8ec9277b676d1b
-rw-r--r-- | java/com/google/gerrit/server/change/ChangeResource.java | 15 | ||||
-rw-r--r-- | javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java | 30 |
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); |