From 91ff0dbd01e7e3db853e3a881caceba8d8dd3387 Mon Sep 17 00:00:00 2001 From: Marco Collovati Date: Wed, 26 Feb 2025 16:47:43 +0100 Subject: [PATCH] fix: compute client-side feature flag values at runtime Activating a feature flag in the project does not work when a default bundle is used because the values of the flags sent to the client are hard-coded at bundle build time. Additionally, if a feature flag is active during bundle creation, it remains active on the client side even if the project does not activate it in the `vaadin-featureflags.properties` file. This change ensures that feature flags in the frontend bundle are always disabled initially and are activated on page load based on the current project settings. Fixes #20991 --- .../plugin/base/BuildFrontendUtilTest.java | 4 +- .../IndexHtmlRequestHandler.java | 27 ++++++++++++++ .../communication/WebComponentProvider.java | 4 +- .../frontend/TaskGenerateFeatureFlags.java | 37 +++++++++++++++---- .../TaskGenerateWebComponentBootstrap.java | 3 +- .../IndexHtmlRequestHandlerTest.java | 11 ++++++ .../WebComponentProviderTest.java | 18 ++++++++- .../TaskGenerateFeatureFlagsTest.java | 11 ++---- ...TaskGenerateWebComponentBootstrapTest.java | 10 +++++ 9 files changed, 103 insertions(+), 22 deletions(-) diff --git a/flow-plugins/flow-plugin-base/src/test/java/com/vaadin/flow/plugin/base/BuildFrontendUtilTest.java b/flow-plugins/flow-plugin-base/src/test/java/com/vaadin/flow/plugin/base/BuildFrontendUtilTest.java index abc50d218a0..b2ed6598621 100644 --- a/flow-plugins/flow-plugin-base/src/test/java/com/vaadin/flow/plugin/base/BuildFrontendUtilTest.java +++ b/flow-plugins/flow-plugin-base/src/test/java/com/vaadin/flow/plugin/base/BuildFrontendUtilTest.java @@ -532,9 +532,9 @@ public void runNodeUpdater_generateFeatureFlagsJsFile() throws Exception { .readString(generatedFeatureFlagsFile.toPath()) .replace("\r\n", "\n"); - Assert.assertTrue("Example feature flag is not set", + Assert.assertTrue("Example feature should not be set at build time", featureFlagsJs.contains( - "window.Vaadin.featureFlags.exampleFeatureFlag = true;\n")); + "window.Vaadin.featureFlags.exampleFeatureFlag = false;\n")); } private void fillAdapter() throws URISyntaxException { diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandler.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandler.java index a679ec45dab..f9329c884fd 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandler.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandler.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Locale; import java.util.Optional; +import java.util.stream.Collectors; import java.util.stream.Stream; import com.fasterxml.jackson.databind.JsonNode; @@ -40,6 +41,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.vaadin.experimental.Feature; +import com.vaadin.experimental.FeatureFlags; import com.vaadin.flow.component.UI; import com.vaadin.flow.function.DeploymentConfiguration; import com.vaadin.flow.internal.BootstrapHandlerHelper; @@ -108,6 +111,8 @@ public boolean synchronizedHandleRequest(VaadinSession session, htmlElement.attr("lang", locale.getLanguage()); } + initializeFeatureFlags(indexDocument, request); + ObjectNode initialJson = JacksonUtils.createObjectNode(); if (service.getBootstrapInitialPredicate() @@ -207,6 +212,28 @@ public boolean synchronizedHandleRequest(VaadinSession session, return true; } + private void initializeFeatureFlags(Document indexDocument, + VaadinRequest request) { + String script = featureFlagsInitializer(request); + Element scriptElement = indexDocument.head().prependElement("script"); + scriptElement.attr(SCRIPT_INITIAL, ""); + scriptElement.appendChild(new DataNode(script)); + } + + static String featureFlagsInitializer(VaadinRequest request) { + return FeatureFlags.get(request.getService().getContext()).getFeatures() + .stream().filter(Feature::isEnabled) + .map(feature -> String.format("activator(\"%s\");", + feature.getId())) + .collect(Collectors.joining("\n", + """ + window.Vaadin = window.Vaadin || {}; + window.Vaadin.featureFlagsUpdaters = window.Vaadin.featureFlagsUpdaters || []; + window.Vaadin.featureFlagsUpdaters.push((activator) => { + """, + "});")); + } + private static void addDevBundleTheme(Document document, VaadinContext context) { ApplicationConfiguration config = ApplicationConfiguration.get(context); diff --git a/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentProvider.java b/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentProvider.java index f313df7cc7f..7ad89e458db 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentProvider.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/communication/WebComponentProvider.java @@ -193,7 +193,9 @@ protected String generateNPMResponse(String tagName, VaadinRequest request, // get the running script boolean productionMode = request.getService() .getDeploymentConfiguration().isProductionMode(); - return getThisScript(tagName) + "var scriptUri = thisScript.src;" + + return IndexHtmlRequestHandler.featureFlagsInitializer(request) + + getThisScript(tagName) + "var scriptUri = thisScript.src;" + "var index = scriptUri.lastIndexOf('" + WEB_COMPONENT_PATH + "');" + "var context = scriptUri.substring(0, index+" + WEB_COMPONENT_PATH.length() + ");" diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlags.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlags.java index 4ffd937ada7..9ac7b5c66a3 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlags.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlags.java @@ -15,13 +15,14 @@ */ package com.vaadin.flow.server.frontend; -import com.vaadin.experimental.FeatureFlags; - import java.io.File; import java.util.ArrayList; import java.util.List; -import static com.vaadin.flow.server.frontend.FrontendUtils.*; +import com.vaadin.experimental.Feature; + +import static com.vaadin.flow.server.frontend.FrontendUtils.FEATURE_FLAGS_FILE_NAME; +import static com.vaadin.flow.server.frontend.FrontendUtils.GENERATED; /** * A task for generating the feature flags file @@ -47,11 +48,31 @@ protected String getFileContent() { lines.add( "window.Vaadin.featureFlags = window.Vaadin.featureFlags || {};"); - FeatureFlags featureFlags = options.getFeatureFlags(); - featureFlags.getFeatures().forEach(feature -> { - lines.add(String.format("window.Vaadin.featureFlags.%s = %s;", - feature.getId(), featureFlags.isEnabled(feature))); - }); + // Initialize the flag entries only once. For exported web-components, + // this script may be executed multiple times (one per embedded + // component) and we should prevent active flags get overridden. + List featureFlags = options.getFeatureFlags().getFeatures(); + if (!featureFlags.isEmpty()) { + lines.add( + "if (Object.keys(window.Vaadin.featureFlags).length === 0) {"); + featureFlags.forEach(feature -> { + lines.add( + String.format("window.Vaadin.featureFlags.%s = false;", + feature.getId())); + }); + lines.add("};"); + } + + // Multiple feature flags updater functions can be registered, in case + // of exported web-component. If the component comes from different web + // applications, the active flags might not be the same. + lines.add("if (window.Vaadin.featureFlagsUpdaters) { "); + lines.add( + "const activator = (id) => window.Vaadin.featureFlags[id] = true;"); + lines.add( + "window.Vaadin.featureFlagsUpdaters.forEach(updater => updater(activator));"); + lines.add("delete window.Vaadin.featureFlagsUpdaters;"); + lines.add("} "); // See https://github.com/vaadin/flow/issues/14184 lines.add("export {};"); diff --git a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java index 82c69c9b21d..8ecf443cbbc 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrap.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; +import static com.vaadin.flow.server.frontend.FrontendUtils.FEATURE_FLAGS_FILE_NAME; import static com.vaadin.flow.server.frontend.FrontendUtils.GENERATED; import static com.vaadin.flow.server.frontend.FrontendUtils.WEB_COMPONENT_BOOTSTRAP_FILE_NAME; @@ -51,7 +52,7 @@ public class TaskGenerateWebComponentBootstrap @Override protected String getFileContent() { List lines = new ArrayList<>(); - + lines.add(String.format("import './%s';%n", FEATURE_FLAGS_FILE_NAME)); lines.add("import 'Frontend/generated/flow/" + FrontendUtils.IMPORTS_WEB_COMPONENT_NAME + "';"); lines.add("import { init } from '" + FrontendUtils.JAR_RESOURCES_IMPORT diff --git a/flow-server/src/test/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandlerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandlerTest.java index b0fc2c64efb..5ecb8c65edd 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandlerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/communication/IndexHtmlRequestHandlerTest.java @@ -210,6 +210,17 @@ public void serveIndexHtml_requestWithSomePath_hasBaseHrefElement() indexHtml.contains(" {")); + } + @Test public void canHandleRequest_requestWithRootPath_handleRequest() { boolean canHandleRequest = indexHtmlRequestHandler diff --git a/flow-server/src/test/java/com/vaadin/flow/server/communication/WebComponentProviderTest.java b/flow-server/src/test/java/com/vaadin/flow/server/communication/WebComponentProviderTest.java index 1524d2ab363..237dd06ed2f 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/communication/WebComponentProviderTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/communication/WebComponentProviderTest.java @@ -17,11 +17,11 @@ package com.vaadin.flow.server.communication; import jakarta.servlet.ServletContext; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.HashSet; import java.util.Set; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -31,6 +31,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @@ -42,6 +43,7 @@ import com.vaadin.flow.component.page.Push; import com.vaadin.flow.component.webcomponent.WebComponent; import com.vaadin.flow.component.webcomponent.WebComponentConfiguration; +import com.vaadin.flow.di.Lookup; import com.vaadin.flow.function.DeploymentConfiguration; import com.vaadin.flow.internal.CurrentInstance; import com.vaadin.flow.server.DefaultDeploymentConfiguration; @@ -101,6 +103,14 @@ public void init() { .getArguments()[0]) .when(context) .setAttribute(any(WebComponentConfigurationRegistry.class)); + + final Lookup lookup = Mockito.mock(Lookup.class); + Mockito.when(context.getAttribute(Lookup.class)).thenReturn(lookup); + Mockito.doAnswer(i -> i.getArgument(1, Supplier.class).get()) + .when(context).getAttribute( + ArgumentMatchers.argThat(aClass -> "FeatureFlagsWrapper" + .equals(aClass.getSimpleName())), + any()); VaadinService.setCurrent(service); Mockito.when(service.getInstantiator()) .thenReturn(new MockInstantiator()); @@ -178,7 +188,7 @@ public void webComponentNotPresent_responseReturns404() throws IOException { public void webComponentGenerator_responseGetsResult() throws IOException { registry = setupConfigurations(MyComponentExporter.class); - ByteArrayOutputStream out = Mockito.mock(ByteArrayOutputStream.class); + ByteArrayOutputStream out = Mockito.spy(new ByteArrayOutputStream()); DefaultDeploymentConfiguration configuration = Mockito .mock(DefaultDeploymentConfiguration.class); @@ -191,6 +201,10 @@ public void webComponentGenerator_responseGetsResult() throws IOException { Assert.assertTrue("Provider should handle web-component request", provider.synchronizedHandleRequest(session, request, response)); + Assert.assertTrue("Response should have Feature Flags updater function", + out.toString().contains( + "window.Vaadin.featureFlagsUpdaters.push((activator) => {")); + Mockito.verify(response).getOutputStream(); Mockito.verify(out).write(Mockito.any(), Mockito.anyInt(), Mockito.anyInt()); diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlagsTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlagsTest.java index ceba253759e..9d627dc7201 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlagsTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateFeatureFlagsTest.java @@ -87,17 +87,12 @@ public void should_defineAllFeatureFlags() throws ExecutionFailedException { } @Test - public void should_defineCorrectEnabledValue() + public void should_callFeatureFlagsUpdaterFunction() throws ExecutionFailedException { - // Enable example feature - featureFlags.getFeatures().stream() - .filter(feature -> feature.equals(FeatureFlags.EXAMPLE)) - .forEach(feature -> feature.setEnabled(true)); - taskGenerateFeatureFlags.execute(); String content = taskGenerateFeatureFlags.getFileContent(); - - assertFeatureFlagGlobal(content, FeatureFlags.EXAMPLE, true); + Assert.assertTrue(content.contains( + "window.Vaadin.featureFlagsUpdaters.forEach(updater => updater(activator))")); } @Test diff --git a/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrapTest.java b/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrapTest.java index 55941ed720a..758147e477c 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrapTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/frontend/TaskGenerateWebComponentBootstrapTest.java @@ -27,6 +27,7 @@ import org.mockito.Mockito; import static com.vaadin.flow.server.frontend.FrontendUtils.DEFAULT_FRONTEND_DIR; +import static com.vaadin.flow.server.frontend.FrontendUtils.FEATURE_FLAGS_FILE_NAME; public class TaskGenerateWebComponentBootstrapTest { @Rule @@ -68,4 +69,13 @@ public void should_importAndInitializeFlowClient() "import { init } from '" + FrontendUtils.JAR_RESOURCES_IMPORT + "FlowClient.js';\n" + "init()")); } + + @Test + public void should_importFeatureFlagTS() throws ExecutionFailedException { + taskGenerateWebComponentBootstrap.execute(); + String content = taskGenerateWebComponentBootstrap.getFileContent(); + Assert.assertTrue(content.contains( + String.format("import './%s';", FEATURE_FLAGS_FILE_NAME))); + } + }