From be98d8c640de4715adc54973e848f7a2835ed302 Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Mon, 7 Dec 2020 14:59:55 +0100 Subject: [PATCH] Make UI experiments configurable from gerrit.config This allows users who upgrade Gerrit to make use of experimental features or temporarily revert to previous behavior in case the new behavior breaks them. The specific use case we have in mind for this is to turn off patch-set-level comments in 3.3 which breaks some CI workflows. Change-Id: Ib1f79cf4c799b0b52e2597801ccff4cbcbfa44c6 --- Documentation/config-gerrit.txt | 20 +++++++++++++++++++ .../gerrit/httpd/raw/IndexHtmlUtil.java | 7 ++++++- .../google/gerrit/httpd/raw/IndexServlet.java | 15 ++++++++++++-- .../google/gerrit/httpd/raw/StaticModule.java | 2 +- .../gerrit/httpd/raw/IndexServletTest.java | 10 ++++++++-- 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 3461774226..cf1ccc024c 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -3362,6 +3362,26 @@ Defaults to all available options minus `CHANGE_ACTIONS`, config is backwards compatible with what the default was before the config was added. +[[experiments]] +=== Section experiments + +This section covers experimental new features. Gerrit's frontend uses experiments +to research new behavior. Once the research is done, the experimental feature +either stays and the experimentation flag gets removed, or the feature as a whole +gets removed + +[[experiments.enabled]]experiments.enabled:: ++ +List of experiments that are currently enabled. The release notes contain currently +available experiments. ++ +We will not remove experiments in stable patch releases. They are likely to be +removed in the next stable version. + +---- +[experiments] + enabled = ExperimentKey +---- [[ldap]] === Section ldap diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java index 77d02c16d0..d7ce88f1b6 100644 --- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java +++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java @@ -37,9 +37,11 @@ import java.net.URISyntaxException; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.function.Function; +import org.eclipse.jgit.lib.Config; /** Helper for generating parts of {@code index.html}. */ @UsedAt(Project.GOOGLE) @@ -53,6 +55,7 @@ public class IndexHtmlUtil { */ public static ImmutableMap templateData( GerritApi gerritApi, + Config gerritServerConfig, String canonicalURL, String cdnPath, String faviconPath, @@ -66,7 +69,9 @@ public class IndexHtmlUtil { canonicalURL, cdnPath, faviconPath, urlParameterMap, urlInScriptTagOrdainer)) .putAll(dynamicTemplateData(gerritApi, requestedURL)); - Set enabledExperiments = experimentData(urlParameterMap); + Set enabledExperiments = new HashSet<>(experimentData(urlParameterMap)); + Arrays.stream(gerritServerConfig.getStringList("experiments", null, "enabled")) + .forEach(enabledExperiments::add); if (!enabledExperiments.isEmpty()) { data.put("enabledExperiments", serializeObject(GSON, enabledExperiments).toString()); } diff --git a/java/com/google/gerrit/httpd/raw/IndexServlet.java b/java/com/google/gerrit/httpd/raw/IndexServlet.java index 97d22701de..b2bdf7cfcd 100644 --- a/java/com/google/gerrit/httpd/raw/IndexServlet.java +++ b/java/com/google/gerrit/httpd/raw/IndexServlet.java @@ -34,6 +34,7 @@ import java.util.function.Function; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.lib.Config; public class IndexServlet extends HttpServlet { private static final long serialVersionUID = 1L; @@ -42,6 +43,7 @@ public class IndexServlet extends HttpServlet { @Nullable private final String cdnPath; @Nullable private final String faviconPath; private final GerritApi gerritApi; + private final Config gerritServerConfig; private final SoySauce soySauce; private final Function urlOrdainer; @@ -49,11 +51,13 @@ public class IndexServlet extends HttpServlet { @Nullable String canonicalUrl, @Nullable String cdnPath, @Nullable String faviconPath, - GerritApi gerritApi) { + GerritApi gerritApi, + Config gerritServerConfig) { this.canonicalUrl = canonicalUrl; this.cdnPath = cdnPath; this.faviconPath = faviconPath; this.gerritApi = gerritApi; + this.gerritServerConfig = gerritServerConfig; this.soySauce = SoyFileSet.builder() .add(Resources.getResource("com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy")) @@ -74,7 +78,14 @@ public class IndexServlet extends HttpServlet { // TODO(hiesel): Remove URL ordainer as parameter once Soy is consistent ImmutableMap templateData = IndexHtmlUtil.templateData( - gerritApi, canonicalUrl, cdnPath, faviconPath, parameterMap, urlOrdainer, requestUrl); + gerritApi, + gerritServerConfig, + canonicalUrl, + cdnPath, + faviconPath, + parameterMap, + urlOrdainer, + requestUrl); renderer = soySauce.renderTemplate("com.google.gerrit.httpd.raw.Index").setData(templateData); } catch (URISyntaxException | RestApiException e) { throw new IOException(e); diff --git a/java/com/google/gerrit/httpd/raw/StaticModule.java b/java/com/google/gerrit/httpd/raw/StaticModule.java index 4b2c8a9733..66e107bbc8 100644 --- a/java/com/google/gerrit/httpd/raw/StaticModule.java +++ b/java/com/google/gerrit/httpd/raw/StaticModule.java @@ -225,7 +225,7 @@ public class StaticModule extends ServletModule { String cdnPath = options.useDevCdn() ? options.devCdn() : cfg.getString("gerrit", null, "cdnPath"); String faviconPath = cfg.getString("gerrit", null, "faviconPath"); - return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi); + return new IndexServlet(canonicalUrl, cdnPath, faviconPath, gerritApi, cfg); } @Provides diff --git a/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java b/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java index 77ab58b1ae..dc976ad909 100644 --- a/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java +++ b/javatests/com/google/gerrit/httpd/raw/IndexServletTest.java @@ -53,8 +53,11 @@ public class IndexServletTest { String testCanonicalUrl = "foo-url"; String testCdnPath = "bar-cdn"; String testFaviconURL = "zaz-url"; + + org.eclipse.jgit.lib.Config serverConfig = new org.eclipse.jgit.lib.Config(); + serverConfig.setStringList("experiments", null, "enabled", ImmutableList.of("NewFeature")); IndexServlet servlet = - new IndexServlet(testCanonicalUrl, testCdnPath, testFaviconURL, gerritApi); + new IndexServlet(testCanonicalUrl, testCdnPath, testFaviconURL, gerritApi, serverConfig); FakeHttpServletResponse response = new FakeHttpServletResponse(); @@ -76,6 +79,9 @@ public class IndexServletTest { + "'\\x7b\\x22\\/config\\/server\\/version\\x22: \\x22123\\x22, " + "\\x22\\/config\\/server\\/info\\x22: \\x7b\\x22default_theme\\x22:" + "\\x22my-default-theme\\x22\\x7d, \\x22\\/config\\/server\\/top-menus\\x22: " - + "\\x5b\\x5d\\x7d');"); + + "\\x5b\\x5d\\x7d');"); + assertThat(output) + .contains( + "window.ENABLED_EXPERIMENTS = JSON.parse('\\x5b\\x22NewFeature\\x22\\x5d');"); } }