Skip to content

Commit

Permalink
Fix: theme files can now be referenced as theme/theme-name
Browse files Browse the repository at this point in the history 
Theme files are now copied undet theme/[theme-name]
and can be refernced by theme/them-name/path/file.ff
even though they are located at VAADIN/static

Fixes #9405 and #9535
  • Loading branch information
@caalador
caalador committed Dec 9, 2020
1 parent cb5bc35 commit 40a9699
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 27 deletions.
19 changes: 14 additions & 5 deletions flow-server/src/main/java/com/vaadin/flow/server/StaticFileServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ public class StaticFileServer implements StaticFileHandler {
private final VaadinServletService servletService;
private DeploymentConfiguration deploymentConfiguration;

// Matcher to match string starting with '/theme/[theme-name]/'
private static final Pattern themeFolder = Pattern.compile("^\\/theme\\/[\\s\\S]*?\\/");

/**
* Constructs a file server.
*
Expand All @@ -80,7 +83,7 @@ public boolean isStaticResourceRequest(HttpServletRequest request) {
return false;
}

if (requestFilename.startsWith("/" + VAADIN_STATIC_FILES_PATH)
if (themeFolder.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
Expand Down Expand Up @@ -111,8 +114,13 @@ public boolean serveStaticResource(HttpServletRequest request,

URL resourceUrl = null;
if (isAllowedVAADINBuildOrStaticUrl(filenameWithPath)) {
resourceUrl = servletService.getClassLoader()
if(themeFolder.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);
Expand Down Expand Up @@ -196,8 +204,8 @@ private String fixIncorrectWebjarPath(String requestFilename) {
*/
private boolean isAllowedVAADINBuildOrStaticUrl(String filenameWithPath) {
// Check that we target VAADIN/build
return filenameWithPath.startsWith("/" + VAADIN_BUILD_FILES_PATH)
|| filenameWithPath.startsWith("/" + VAADIN_STATIC_FILES_PATH);
return filenameWithPath.startsWith("/" + VAADIN_BUILD_FILES_PATH) || filenameWithPath.startsWith("/" + VAADIN_STATIC_FILES_PATH)
|| themeFolder.matcher(filenameWithPath).find();
}

/**
Expand Down Expand Up @@ -288,7 +296,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)
|| themeFolder.matcher(request.getPathInfo()).find()) {
return request.getPathInfo();
}
return request.getServletPath() + request.getPathInfo();
Expand Down
2 changes: 1 addition & 1 deletion flow-server/src/main/resources/plugins/application-theme-plugin/application-theme-plugin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion flow-server/src/main/resources/plugins/application-theme-plugin/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 5 additions & 4 deletions flow-server/src/main/resources/plugins/application-theme-plugin/theme-copy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -66,7 +67,7 @@ function copyStaticAssets(themeProperties, projectStaticAssetsOutputFolder, logg
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 targetFolder = path.resolve(projectStaticAssetsOutputFolder, "theme", themeName, copyRules[copyRule]);

fs.mkdirSync(targetFolder, {
recursive: true
Expand Down
10 changes: 3 additions & 7 deletions flow-server/src/main/resources/webpack.generated.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,11 @@ 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]';
}
}
}],
},
Expand Down
2 changes: 1 addition & 1 deletion flow-tests/test-themes/src/main/java/com/vaadin/flow/uitest/ui/theme/ThemeView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,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);

Expand Down
19 changes: 11 additions & 8 deletions flow-tests/test-themes/src/test/java/com/vaadin/flow/uitest/ui/theme/ThemeIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ public void typeScriptCssImport_stylesAreApplied() {

@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 Not Found"));

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 Not Found"));
}
Expand All @@ -70,13 +70,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"));
}
Expand Down Expand Up @@ -128,22 +130,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)
Expand Down

0 comments on commit 40a9699

Please sign in to comment.