Allow plugins to contribute a value to the change ETag computation

Plugins can affect the result of the get change / get change details
REST endpoints by:

* providing plugin defined attributes to the 'plugins' field in
  ChangeInfo
* implementing a SubmitRule which affects the computation of
  'submittable' field in ChangeInfo

If the plugin defined part of ChangeInfo depends on plugin specific
data, callers that use change ETags to avoid unneeded recomputations of
ChangeInfos may see outdated plugin attributes and/or outdated
submittable information, because a ChangeInfo is only reloaded if the
change ETag changes.

By implementating the new ChangeETagComputation interface plugins can
now contribute to the change ETag computation and thus ensure that the
ETag changes when the plugin data was changed. This way it can be
ensured that callers do not see outdated ChangeInfos.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I358e3d4fb2145a164ec1387fc1b40778959ad41e

Documentation/dev-plugins.txt

@@ -964,6 +964,11 @@ Output:
 }
 ----
 
+Implementors of the `ChangeAttributeFactory` interface should check whether
+they need to contribute to the link:#change-etag-computation[change ETag
+computation] to prevent callers using ETags from potentially seeing outdated
+plugin attributes.
+
 [[simple-configuration]]
 == Simple Configuration in `gerrit.config`
 
@@ -2713,6 +2718,62 @@ to change in order to be compliant. These requirements should be kept once they
 are met, but marked as `OK`. If the requirements were not displayed, reviewers
 would need to use their precious time to manually check that they were met.
 
+Implementors of the `SubmitRule` interface should check whether they need to
+contribute to the link:#change-etag-computation[change ETag computation] to
+prevent callers using ETags from potentially seeing outdated submittability
+information.
+
+[[change-etag-computation]]
+== Change ETag Computation
+
+By implementing the `com.google.gerrit.server.change.ChangeETagComputation`
+interface plugins can contribute a value to the change ETag computation.
+
+Plugins can affect the result of the get change / get change details REST
+endpoints by:
+
+* providing link:#query_attributes[plugin defined attributes] in
+  link:rest-api-changes.html#change-info[ChangeInfo]
+* implementing a link:#pre-submit-evaluator[pre-submit evaluator] which affects
+  the computation of `submittable` field in
+  link:rest-api-changes.html#change-info[ChangeInfo]
+
+If the plugin defined part of link:rest-api-changes.html#change-info[
+ChangeInfo] depends on plugin specific data, callers that use change ETags to
+avoid unneeded recomputations of ChangeInfos may see outdated plugin attributes
+and/or outdated submittable information, because a ChangeInfo is only reloaded
+if the change ETag changes.
+
+By implementating the `com.google.gerrit.server.change.ChangeETagComputation`
+interface plugins can contribute to the ETag computation and thus ensure that
+the change ETag changes when the plugin data was changed. This way it can be
+ensured that callers do not see outdated ChangeInfos.
+
+IMPORTANT: Change ETags are computed very frequently and the computation must
+be cheap. Take good care to not perform any expensive computations when
+implementing this.
+
+[source, java]
+----
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.hash.Hasher;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.change.ChangeETagComputation;
+
+public class MyPluginChangeETagComputation implements ChangeETagComputation {
+  public String getETag(Project.NameKey projectName, Change.Id changeId) {
+    Hasher hasher = Hashing.murmur3_128().newHasher();
+
+    // Add hashes for all plugin-specific data that affects change infos.
+    hasher.putString(sha1OfPluginSpecificChangeRef, UTF_8);
+
+    return hasher.hash().toString();
+  }
+}
+----
+
 [[quota-enforcer]]
 == Quota Enforcer
 

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

@@ -0,0 +1,58 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+
+/**
+ * Allows plugins to contribute a value to the change ETag computation.
+ *
+ * <p>Plugins can affect the result of the get change / get change details REST endpoints by:
+ *
+ * <ul>
+ *   <li>providing plugin defined attributes to {@link
+ *       com.google.gerrit.extensions.common.ChangeInfo#plugins} (see {@link
+ *       ChangeAttributeFactory})
+ *   <li>implementing a {@link com.google.gerrit.server.rules.SubmitRule} which affects the
+ *       computation of {@link com.google.gerrit.extensions.common.ChangeInfo#submittable}
+ * </ul>
+ *
+ * <p>If the plugin defined part of {@link com.google.gerrit.extensions.common.ChangeInfo} depends
+ * on plugin specific data, callers that use the change ETags to avoid unneeded recomputations of
+ * ChangeInfos may see outdated plugin attributes and/or outdated submittable information, because a
+ * ChangeInfo is only reloaded if the change ETag changes.
+ *
+ * <p>By implementating this interface plugins can contribute to the change ETag computation and
+ * thus ensure that the ETag changes when the plugin data was changed. This way it is ensured that
+ * callers do not see outdated ChangeInfos.
+ *
+ * @see ChangeResource#getETag()
+ */
+@ExtensionPoint
+public interface ChangeETagComputation {
+  /**
+   * Computes an ETag of plugin-specific data for the given change.
+   *
+   * <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.
+   *
+   * @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
+   */
+  String getETag(Project.NameKey projectName, Change.Id changeId);
+}

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

@@ -38,6 +38,7 @@ import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountState;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.inject.Inject;
@@ -75,6 +76,7 @@ public class ChangeResource implements RestResource, HasETag {
   private final PermissionBackend permissionBackend;
   private final StarredChangesUtil starredChangesUtil;
   private final ProjectCache projectCache;
+  private final PluginSetContext<ChangeETagComputation> changeETagComputation;
   private final ChangeNotes notes;
   private final CurrentUser user;
 
@@ -86,6 +88,7 @@ public class ChangeResource implements RestResource, HasETag {
       PermissionBackend permissionBackend,
       StarredChangesUtil starredChangesUtil,
       ProjectCache projectCache,
+      PluginSetContext<ChangeETagComputation> changeETagComputation,
       @Assisted ChangeNotes notes,
       @Assisted CurrentUser user) {
     this.accountCache = accountCache;
@@ -94,6 +97,7 @@ public class ChangeResource implements RestResource, HasETag {
     this.permissionBackend = permissionBackend;
     this.starredChangesUtil = starredChangesUtil;
     this.projectCache = projectCache;
+    this.changeETagComputation = changeETagComputation;
     this.notes = notes;
     this.user = user;
   }
@@ -202,6 +206,13 @@ 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();
   }
 

java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -102,6 +102,7 @@ import com.google.gerrit.server.cache.CacheRemovalListener;
 import com.google.gerrit.server.change.AbandonOp;
 import com.google.gerrit.server.change.AccountPatchReviewStore;
 import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.gerrit.server.change.ChangeETagComputation;
 import com.google.gerrit.server.change.ChangeFinder;
 import com.google.gerrit.server.change.ChangeJson;
 import com.google.gerrit.server.change.ChangeKindCacheImpl;
@@ -388,6 +389,7 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicSet.setOf(binder(), PerformanceLogger.class);
     DynamicSet.setOf(binder(), RequestListener.class);
     DynamicSet.bind(binder(), RequestListener.class).to(TraceRequestListener.class);
+    DynamicSet.setOf(binder(), ChangeETagComputation.class);
 
     DynamicMap.mapOf(binder(), MailFilter.class);
     bind(MailFilter.class).annotatedWith(Exports.named("ListMailFilter")).to(ListMailFilter.class);

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

@@ -154,6 +154,7 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.StarredChangesUtil;
+import com.google.gerrit.server.change.ChangeETagComputation;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.git.ChangeMessageModifier;
 import com.google.gerrit.server.group.SystemGroupBackend;
@@ -217,6 +218,7 @@ public class ChangeIT extends AbstractDaemonTest {
   @Inject private IndexConfig indexConfig;
   @Inject private ProjectOperations projectOperations;
   @Inject private RequestScopeOperations requestScopeOperations;
+  @Inject private DynamicSet<ChangeETagComputation> changeETagComputations;
 
   @Inject
   @Named("diff")
@@ -2152,6 +2154,34 @@ public class ChangeIT extends AbstractDaemonTest {
     assertThat(rsrc.getETag()).isNotEqualTo(oldETag);
   }
 
+  @Test
+  public void pluginCanContributeToETagComputation() throws Exception {
+    PushOneCommit.Result r = createChange();
+    String oldETag = parseResource(r).getETag();
+
+    RegistrationHandle registrationHandle = changeETagComputations.add("gerrit", (p, id) -> "foo");
+    try {
+      assertThat(parseResource(r).getETag()).isNotEqualTo(oldETag);
+    } finally {
+      registrationHandle.remove();
+    }
+
+    assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
+  }
+
+  @Test
+  public void returningNullFromETagComputationDoesNotBreakGerrit() throws Exception {
+    PushOneCommit.Result r = createChange();
+    String oldETag = parseResource(r).getETag();
+
+    RegistrationHandle registrationHandle = changeETagComputations.add("gerrit", (p, id) -> null);
+    try {
+      assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
+    } finally {
+      registrationHandle.remove();
+    }
+  }
+
   @Test
   public void emailNotificationForFileLevelComment() throws Exception {
     String changeId = createChange().getChangeId();