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
This commit is contained in:
		| @@ -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 | ||||
|   | ||||
| @@ -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<String, Object> 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<String> enabledExperiments = experimentData(urlParameterMap); | ||||
|     Set<String> 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()); | ||||
|     } | ||||
|   | ||||
| @@ -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<String, SanitizedContent> 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<String, Object> 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); | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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');</script>"); | ||||
|                 + "\\x5b\\x5d\\x7d');"); | ||||
|     assertThat(output) | ||||
|         .contains( | ||||
|             "window.ENABLED_EXPERIMENTS = JSON.parse('\\x5b\\x22NewFeature\\x22\\x5d');</script>"); | ||||
|   } | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Patrick Hiesel
					Patrick Hiesel