Events: Make ListChangeOptions configurable

Gerrit's event system propagates changes and revisions using the same
data types that Gerrit exposes on it's REST API. For many reasons, this
is unfortunate. Performance is one of these reasons that this commit
addresses in a non-breaking way.

The event system uses ChangeJson to format the payload of the event.
Until now, this used pretty much all available change options making the
formatting very expensive. No core plugin needs this rich set of fields.
They work perfectly fine if the event just contains a minimal payload.

This commit makes the options used for formatting event payload
configurable to make sure this is a non-breaking change. If admins have
their own plugins set up that rely on specific options, they can specify
these in the Gerrit config and Gerrit will include them when sending
events. This is also useful for stream events.

There is a long-term plan to rework the event interface to remove the
usage of rich formatting altogether and just send IDs of the respective
entities instead. That plan is tangential to what this commit does.

Change-Id: I1c7a6fe0a15669b607cdfb8255b94e4f036b76bf

Documentation/config-gerrit.txt

@@ -3061,6 +3061,31 @@ Password used to connect to Elasticsearch.
 +
 Not set by default.
 
+[[event]]
+=== Section event
+
+[[event.payload.listChangeOptions]]events.payload.listChangeOptions::
++
+List of options that Gerrit applies when rendering the payload of an
+internal event. This is the same set of options that are documented
+link:rest-api-changes.html#query-options[here].
++
+Depending on the setup, these events might get serialized using stream
+events.
++
+This can be set to the set of minimal options that consumers of Gerrit's
+events need. A minimal set would be (`SKIP_MERGEABLE`,`SKIP_DIFFSTAT`).
++
+Every option that gets added here will have a performance impact. The
+general recommendation is therefore to set this to a minimal set of
+required options.
++
+Defaults to all available options minus `CHANGE_ACTIONS`,
+`CURRENT_ACTIONS` and `CHECK`. This is a rich default to make sure the
+config is backwards compatible with what the default was before the config
+was added.
+
+
 [[ldap]]
 === Section ldap
 

java/com/google/gerrit/extensions/events/ChangeEvent.java

@@ -20,6 +20,10 @@ import java.sql.Timestamp;
 
 /** Interface to be extended by Events with a Change. */
 public interface ChangeEvent extends GerritEvent {
+  /**
+   * Information about the change. Some fields might be null. {@see
+   * com.google.gerrit.server.extensions.events.EventUtil}.
+   */
   ChangeInfo getChange();
 
   AccountInfo getWho();

java/com/google/gerrit/extensions/events/RevisionEvent.java

@@ -18,5 +18,10 @@ import com.google.gerrit.extensions.common.RevisionInfo;
 
 /** Interface to be extended by Events with a Revision. */
 public interface RevisionEvent extends ChangeEvent {
+
+  /**
+   * Information about the revision. Some fields might be null. {@see
+   * com.google.gerrit.server.extensions.events.EventUtil}.
+   */
   RevisionInfo getRevision();
 }

java/com/google/gerrit/server/extensions/events/EventUtil.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.extensions.events;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
+import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.extensions.client.ListChangesOption;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ApprovalInfo;
@@ -29,6 +30,7 @@ import com.google.gerrit.server.GpgException;
 import com.google.gerrit.server.account.AccountState;
 import com.google.gerrit.server.change.ChangeJson;
 import com.google.gerrit.server.change.RevisionJson;
+import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.patch.PatchListNotAvailableException;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -39,42 +41,51 @@ import java.sql.Timestamp;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
+import org.eclipse.jgit.lib.Config;
 
+/**
+ * Formats change and revision info objects to serve as payload for Gerrit events.
+ *
+ * <p>Uses configurable options ({@code event.payload.listChangeOptions}) to decide which fields to
+ * populate.
+ */
 @Singleton
 public class EventUtil {
-  private static final ImmutableSet<ListChangesOption> CHANGE_OPTIONS;
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+  private static final ImmutableSet<ListChangesOption> DEFAULT_CHANGE_OPTIONS;
 
   static {
     EnumSet<ListChangesOption> opts = EnumSet.allOf(ListChangesOption.class);
-
     // Some options, like actions, are expensive to compute because they potentially have to walk
     // lots of history and inspect lots of other changes.
     opts.remove(ListChangesOption.CHANGE_ACTIONS);
     opts.remove(ListChangesOption.CURRENT_ACTIONS);
-
     // CHECK suppresses some exceptions on corrupt changes, which is not appropriate for passing
     // through the event system as we would rather let them propagate.
     opts.remove(ListChangesOption.CHECK);
-
-    CHANGE_OPTIONS = Sets.immutableEnumSet(opts);
+    DEFAULT_CHANGE_OPTIONS = Sets.immutableEnumSet(opts);
   }
 
   private final ChangeData.Factory changeDataFactory;
   private final ChangeJson.Factory changeJsonFactory;
   private final RevisionJson.Factory revisionJsonFactory;
+  private final ImmutableSet<ListChangesOption> changeOptions;
 
   @Inject
   EventUtil(
       ChangeJson.Factory changeJsonFactory,
       RevisionJson.Factory revisionJsonFactory,
-      ChangeData.Factory changeDataFactory) {
+      ChangeData.Factory changeDataFactory,
+      @GerritServerConfig Config gerritConfig) {
     this.changeDataFactory = changeDataFactory;
     this.changeJsonFactory = changeJsonFactory;
     this.revisionJsonFactory = revisionJsonFactory;
+    this.changeOptions = parseChangeListOptions(gerritConfig);
   }
 
   public ChangeInfo changeInfo(Change change) {
-    return changeJsonFactory.create(CHANGE_OPTIONS).format(change);
+    return changeJsonFactory.create(changeOptions).format(change);
   }
 
   public RevisionInfo revisionInfo(Project project, PatchSet ps)
@@ -85,7 +96,7 @@ public class EventUtil {
   public RevisionInfo revisionInfo(Project.NameKey project, PatchSet ps)
       throws PatchListNotAvailableException, GpgException, IOException, PermissionBackendException {
     ChangeData cd = changeDataFactory.create(project, ps.id().changeId());
-    return revisionJsonFactory.create(CHANGE_OPTIONS).getRevisionInfo(cd, ps);
+    return revisionJsonFactory.create(changeOptions).getRevisionInfo(cd, ps);
   }
 
   public AccountInfo accountInfo(AccountState accountState) {
@@ -110,4 +121,21 @@ public class EventUtil {
     }
     return result;
   }
+
+  private static ImmutableSet<ListChangesOption> parseChangeListOptions(Config gerritConfig) {
+    String[] config = gerritConfig.getStringList("event", "payload", "listChangeOptions");
+    if (config.length == 0) {
+      return DEFAULT_CHANGE_OPTIONS;
+    }
+
+    ImmutableSet.Builder<ListChangesOption> result = ImmutableSet.builder();
+    for (String c : config) {
+      try {
+        result.add(ListChangesOption.valueOf(c));
+      } catch (IllegalArgumentException e) {
+        logger.atWarning().withCause(e).log("could not parse list change option %s", c);
+      }
+    }
+    return result.build();
+  }
 }

javatests/com/google/gerrit/acceptance/server/event/EventPayloadIT.java

@@ -0,0 +1,65 @@
+// 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.acceptance.server.event;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.extensions.events.RevisionCreatedListener;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.inject.Inject;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+public class EventPayloadIT extends AbstractDaemonTest {
+  @Inject private DynamicSet<RevisionCreatedListener> revisionCreatedListeners;
+
+  private RegistrationHandle eventListenerRegistration;
+  private RevisionCreatedListener.Event lastEvent;
+
+  @Before
+  public void setUp() throws Exception {
+    eventListenerRegistration = revisionCreatedListeners.add("gerrit", event -> lastEvent = event);
+  }
+
+  @After
+  public void cleanup() {
+    eventListenerRegistration.remove();
+  }
+
+  @Test
+  public void defaultOptions() throws Exception {
+    createChange();
+
+    assertThat(lastEvent.getChange().submittable).isNotNull();
+    assertThat(lastEvent.getRevision().files).isNotEmpty();
+  }
+
+  @Test
+  @GerritConfig(name = "event.payload.listChangeOptions", value = "SKIP_MERGEABLE")
+  public void configuredOptions() throws Exception {
+    createChange();
+
+    assertThat(lastEvent.getChange().submittable).isNull();
+    assertThat(lastEvent.getChange().mergeable).isNull();
+    assertThat(lastEvent.getRevision().files).isNull();
+    assertThat(lastEvent.getChange().subject).isNotEmpty();
+  }
+}