fix: theme files can now be referenced as theme/theme-name (#9590)

Theme files are now copied under theme/[theme-name]
and can be referenced by theme/them-name/path/file.ff
even though they are located at VAADIN/static

Fixes: #9405 and #9535
# Conflicts:
#	flow-server/src/main/java/com/vaadin/flow/server/DevModeHandler.java
#	flow-server/src/main/java/com/vaadin/flow/server/StaticFileServer.java

flow-server/src/main/java/com/vaadin/flow/server/DevModeHandler.java

@@ -55,10 +55,12 @@
 import com.vaadin.flow.server.frontend.FrontendTools;
 import com.vaadin.flow.server.frontend.FrontendUtils;
 
+import static com.vaadin.flow.server.Constants.VAADIN_MAPPING;
 import static com.vaadin.flow.server.InitParameters.SERVLET_PARAMETER_DEVMODE_WEBPACK_ERROR_PATTERN;
 import static com.vaadin.flow.server.InitParameters.SERVLET_PARAMETER_DEVMODE_WEBPACK_OPTIONS;
 import static com.vaadin.flow.server.InitParameters.SERVLET_PARAMETER_DEVMODE_WEBPACK_SUCCESS_PATTERN;
 import static com.vaadin.flow.server.InitParameters.SERVLET_PARAMETER_DEVMODE_WEBPACK_TIMEOUT;
+import static com.vaadin.flow.server.StaticFileServer.APP_THEME_PATTERN;
 import static com.vaadin.flow.server.frontend.FrontendUtils.GREEN;
 import static com.vaadin.flow.server.frontend.FrontendUtils.RED;
 import static com.vaadin.flow.server.frontend.FrontendUtils.YELLOW;
@@ -280,9 +282,10 @@ private static DevModeHandler createInstance(int runningPort,
      * @return true if the request should be forwarded to webpack
      */
     public boolean isDevModeRequest(HttpServletRequest request) {
-        final String pathInfo = request.getPathInfo();
-        return pathInfo != null && pathInfo.matches(".+\\.js") && !pathInfo
-                .startsWith("/" + StreamRequestHandler.DYN_RES_PREFIX);
+        String pathInfo = request.getPathInfo();
+        return pathInfo != null && (pathInfo.startsWith("/" + VAADIN_MAPPING)
+            || APP_THEME_PATTERN.matcher(pathInfo).find()) && !pathInfo
+            .startsWith("/" + StreamRequestHandler.DYN_RES_PREFIX);
     }
 
     /**
@@ -320,6 +323,11 @@ public boolean serveDevModeRequest(HttpServletRequest request,
             return true;
         }
 
+        // Redirect theme source request
+        if(APP_THEME_PATTERN.matcher(requestFilename).find()) {
+            requestFilename = "/VAADIN/static" + requestFilename;
+        }
+
         HttpURLConnection connection = prepareConnection(requestFilename,
                 request.getMethod());
 

flow-server/src/main/java/com/vaadin/flow/server/StaticFileServer.java

@@ -56,6 +56,10 @@ public class StaticFileServer implements StaticFileHandler {
     private final VaadinServletService servletService;
     private DeploymentConfiguration deploymentConfiguration;
 
+    // Matcher to match string starting with '/theme/[theme-name]/'
+    protected static final Pattern APP_THEME_PATTERN = Pattern
+        .compile("^\\/theme\\/[\\s\\S]+?\\/");
+
     /**
      * Constructs a file server.
      *
@@ -80,8 +84,9 @@ public boolean isStaticResourceRequest(HttpServletRequest request) {
             return false;
         }
 
-        if (requestFilename.startsWith("/" + VAADIN_STATIC_FILES_PATH)
-                || requestFilename.startsWith("/" + VAADIN_BUILD_FILES_PATH)) {
+        if (APP_THEME_PATTERN.matcher(requestFilename).find() || requestFilename
+            .startsWith("/" + VAADIN_STATIC_FILES_PATH) || requestFilename
+            .startsWith("/" + VAADIN_BUILD_FILES_PATH)) {
             // The path is reserved for internal resources only
             // We rather serve 404 than let it fall through
             return true;
@@ -111,8 +116,13 @@ public boolean serveStaticResource(HttpServletRequest request,
 
         URL resourceUrl = null;
         if (isAllowedVAADINBuildOrStaticUrl(filenameWithPath)) {
-            resourceUrl = servletService.getClassLoader()
+            if(APP_THEME_PATTERN.matcher(filenameWithPath).find()) {
+                resourceUrl = servletService.getClassLoader()
+                    .getResource("META-INF/VAADIN/static" + filenameWithPath);
+            } else {
+                resourceUrl = servletService.getClassLoader()
                     .getResource("META-INF" + filenameWithPath);
+            }
         }
         if (resourceUrl == null) {
             resourceUrl = servletService.getStaticResource(filenameWithPath);
@@ -203,10 +213,10 @@ private boolean isAllowedVAADINBuildOrStaticUrl(String filenameWithPath) {
                     filenameWithPath);
             return false;
         }
-
-        // Check that we target VAADIN/build
+        // Check that we target VAADIN/build | VAADIN/static | theme/theme-name
         return filenameWithPath.startsWith("/" + VAADIN_BUILD_FILES_PATH)
-            || filenameWithPath.startsWith("/" + VAADIN_STATIC_FILES_PATH);
+            || filenameWithPath.startsWith("/" + VAADIN_STATIC_FILES_PATH)
+            || APP_THEME_PATTERN.matcher(filenameWithPath).find();
     }
 
     /**
@@ -297,7 +307,8 @@ String getRequestFilename(HttpServletRequest request) {
         // /VAADIN/folder/file.js
         if (request.getPathInfo() == null) {
             return request.getServletPath();
-        } else if (request.getPathInfo().startsWith("/" + VAADIN_MAPPING)) {
+        } else if (request.getPathInfo().startsWith("/" + VAADIN_MAPPING)
+            || APP_THEME_PATTERN.matcher(request.getPathInfo()).find()) {
             return request.getPathInfo();
         }
         return request.getServletPath() + request.getPathInfo();

flow-server/src/main/resources/plugins/application-theme-plugin/application-theme-plugin.js

@@ -112,7 +112,7 @@ function handleThemes(themeName, themesFolder, projectStaticAssetsOutputFolder)
 
     const themeProperties = getThemeProperties(themeFolder);
 
-    copyStaticAssets(themeProperties, projectStaticAssetsOutputFolder, logger);
+    copyStaticAssets(themeName, themeProperties, projectStaticAssetsOutputFolder, logger);
 
     const themeFile = generateThemeFile(themeFolder, themeName, themeProperties);
 

flow-server/src/main/resources/plugins/application-theme-plugin/package.json

@@ -6,7 +6,7 @@
   ],
   "repository": "vaadin/flow",
   "name": "@vaadin/application-theme-plugin",
-  "version": "0.2.2",
+  "version": "0.2.3",
   "main": "application-theme-plugin.js",
   "author": "Vaadin Ltd",
   "license": "Apache-2.0",

flow-server/src/main/resources/plugins/application-theme-plugin/theme-copy.js

@@ -45,11 +45,12 @@ const glob = require('glob');
  *
  * Note! there can be multiple copy-rules with target folders for one npm package asset.
  *
- * @param {json} themeProperties
- * @param {string} projectStaticAssetsOutputFolder
+ * @param {string} themeName name of the theme we are copying assets for
+ * @param {json} themeProperties theme properties json with data on assets
+ * @param {string} projectStaticAssetsOutputFolder project output folder where we copy assets to under theme/[themeName]
  * @param {logger} theme plugin logger
  */
-function copyStaticAssets(themeProperties, projectStaticAssetsOutputFolder, logger) {
+function copyStaticAssets(themeName, themeProperties, projectStaticAssetsOutputFolder, logger) {
 
   const assets = themeProperties['assets'];
   if (!assets) {
@@ -65,15 +66,19 @@ function copyStaticAssets(themeProperties, projectStaticAssetsOutputFolder, logg
     const copyRules = assets[module];
     Object.keys(copyRules).forEach((copyRule) => {
       const nodeSources = path.resolve('node_modules/', module, copyRule);
-      const files = glob.sync(nodeSources, { nodir: true });
-      const targetFolder = path.resolve(projectStaticAssetsOutputFolder, copyRules[copyRule]);
+      const files = glob.sync(nodeSources, {nodir: true});
+      const targetFolder = path.resolve(projectStaticAssetsOutputFolder, "theme", themeName, copyRules[copyRule]);
 
       fs.mkdirSync(targetFolder, {
         recursive: true
       });
       files.forEach((file) => {
-        logger.trace("Copying: ", file, '=>', targetFolder);
-        fs.copyFileSync(file, path.resolve(targetFolder, path.basename(file)));
+        const copyTarget = path.resolve(targetFolder, path.basename(file));
+        // Only copy if target file doesn't exist or if file to copy is newer
+        if (!fs.existsSync(copyTarget) || fs.statSync(copyTarget).mtime < fs.statSync(file).mtime) {
+          logger.trace("Copying: ", file, '=>', targetFolder);
+          fs.copyFileSync(file, copyTarget);
+        }
       });
     });
   });

flow-server/src/main/resources/webpack.generated.js

@@ -190,12 +190,8 @@ module.exports = {
           options: {
             outputPath: 'static/',
             name(resourcePath, resourceQuery) {
-              const urlResource = resourcePath.substring(frontendFolder.length);
-              if(urlResource.match(themePartRegex)){
-                return /^(\\|\/)theme\1[\s\S]*?\1(.*)/.exec(urlResource)[2].replace(/\\/, "/");
-              }
-              if(urlResource.match(/(\\|\/)node_modules\1/)) {
-                return /(\\|\/)node_modules\1(?!.*node_modules)([\S]*)/.exec(urlResource)[2].replace(/\\/g, "/");
+              if (resourcePath.match(/(\\|\/)node_modules\1/)) {
+                return /(\\|\/)node_modules\1(?!.*node_modules)([\S]+)/.exec(resourcePath)[2].replace(/\\/g, "/");
               }
               return '[path][name].[ext]';
             }

flow-server/src/test/java/com/vaadin/flow/server/DevModeHandlerTest.java

@@ -356,7 +356,7 @@ public void webpack_forDifferentRequests_shouldHaveCorrectResponse()
     @Test
     public void vaadinServlet_forDifferentRequests_shouldHaveCorrectResponse()
             throws Exception {
-        HttpServletRequest request = prepareRequest("/foo.js");
+        HttpServletRequest request = prepareRequest("/VAADIN/foo.js");
         HttpServletResponse response = prepareResponse();
         int port = prepareHttpServer(0, HTTP_OK, "");
 

flow-tests/test-embedding/test-embedding-application-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java

@@ -45,20 +45,20 @@ public void applicationTheme_GlobalCss_isUsedOnlyInEmbeddeComponent() {
         final TestBenchElement themedComponent = $("themed-component").first();
 
         Assert.assertEquals(
-            "url(\"" + getRootURL() + "/VAADIN/static/img/bg.jpg\")",
+            "url(\"" + getRootURL() + "/VAADIN/static/theme/embedded-theme/img/bg.jpg\")",
             themedComponent.getCssValue("background-image"));
 
         Assert.assertEquals("Ostrich",
             themedComponent.getCssValue("font-family"));
 
         final WebElement body = findElement(By.tagName("body"));
         Assert.assertNotEquals(
-            "url(\"" + getRootURL() + "/path/VAADIN/static/img/bg.jpg\")",
+            "url(\"" + getRootURL() + "/path/VAADIN/static/theme/embedded-theme/img/bg.jpg\")",
             body.getCssValue("background-image"));
 
         Assert.assertNotEquals("Ostrich", body.getCssValue("font-family"));
 
-        getDriver().get(getRootURL() + "/VAADIN/static/img/bg.jpg");
+        getDriver().get(getRootURL() + "/VAADIN/static/theme/embedded-theme/img/bg.jpg");
         Assert.assertFalse("app-theme background file should be served",
             driver.getPageSource().contains("Could not navigate"));
     }

flow-tests/test-themes/src/main/java/com/vaadin/flow/uitest/ui/theme/ThemeView.java

@@ -56,7 +56,7 @@ public ThemeView() {
         faText.setId(FONTAWESOME_ID);
 
         Image snowFlake = new Image(
-            "VAADIN/static/fortawesome/icons/snowflake.svg", "snowflake");
+            "theme/app-theme/fortawesome/icons/snowflake.svg", "snowflake");
         snowFlake.setHeight("1em");
         snowFlake.setId(SNOWFLAKE_ID);
 

flow-tests/test-themes/src/test/java/com/vaadin/flow/uitest/ui/theme/ThemeIT.java

@@ -38,11 +38,11 @@ public class ThemeIT extends ChromeBrowserTest {
 
     @Test
     public void secondTheme_staticFilesNotCopied() {
-        getDriver().get(getRootURL() + "/path/VAADIN/static/img/bg.jpg");
+        getDriver().get(getRootURL() + "/path/theme/app-theme/img/bg.jpg");
         Assert.assertFalse("app-theme static files should be copied",
             driver.getPageSource().contains("HTTP ERROR 404"));
 
-        getDriver().get(getRootURL() + "/path/VAADIN/static/no-copy.txt");
+        getDriver().get(getRootURL() + "/path/theme/app-theme/no-copy.txt");
         Assert.assertTrue("no-copy theme should not be handled",
             driver.getPageSource().contains("HTTP ERROR 404"));
     }
@@ -54,13 +54,15 @@ public void applicationTheme_GlobalCss_isUsed() {
         checkLogsForErrors();
 
         final WebElement body = findElement(By.tagName("body"));
+        // Note theme/app-theme gets VAADIN/static from the file-loader
         Assert.assertEquals(
-            "url(\"" + getRootURL() + "/path/VAADIN/static/img/bg.jpg\")",
+            "url(\"" + getRootURL() + "/path/VAADIN/static/theme/app-theme/img/bg.jpg\")",
             body.getCssValue("background-image"));
 
         Assert.assertEquals("Ostrich", body.getCssValue("font-family"));
 
-        getDriver().get(getRootURL() + "/path/VAADIN/static/img/bg.jpg");
+        // Note theme/app-theme gets VAADIN/static from the file-loader
+        getDriver().get(getRootURL() + "/path/VAADIN/static/theme/app-theme/img/bg.jpg");
         Assert.assertFalse("app-theme background file should be served",
             driver.getPageSource().contains("Could not navigate"));
     }
@@ -112,22 +114,23 @@ public void subCssWithRelativePath_urlPathIsNotRelative() {
         open();
         checkLogsForErrors();
 
+        // Note theme/app-theme gets VAADIN/static from the file-loader
         Assert.assertEquals("Imported css file URLs should have been handled.",
             "url(\"" + getRootURL()
-                + "/path/VAADIN/static/icons/archive.png\")",
+                + "/path/VAADIN/static/theme/app-theme/icons/archive.png\")",
             $(SpanElement.class).id(SUB_COMPONENT_ID)
                 .getCssValue("background-image"));
     }
 
     @Test
-    public void staticModuleAsset_servedFromStatic() {
+    public void staticModuleAsset_servedFromAppTheme() {
         open();
         checkLogsForErrors();
 
         Assert.assertEquals(
-            "Node assets should have been copied to 'VAADIN/static'",
+            "Node assets should have been copied to 'theme/app-theme'",
             getRootURL()
-                + "/path/VAADIN/static/fortawesome/icons/snowflake.svg",
+                + "/path/theme/app-theme/fortawesome/icons/snowflake.svg",
             $(ImageElement.class).id(SNOWFLAKE_ID).getAttribute("src"));
 
         open(getRootURL() + "/path/" + $(ImageElement.class).id(SNOWFLAKE_ID)