This is an automated email from the ASF dual-hosted git repository.
jamesyong pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new f85040f Improved: Headerize external script in multi-block html
template (OFBIZ-11741)
f85040f is described below
commit f85040ff04a70ea17facfd312c329101c3fe817b
Author: James Yong <jamesy...@apache.org>
AuthorDate: Sun May 31 17:42:42 2020 +0800
Improved: Headerize external script in multi-block html template
(OFBIZ-11741)
Allow expression in template location.
---
build.gradle | 2 +-
framework/widget/dtd/widget-screen.xsd | 2 +-
.../widget/model/HtmlImportWidgetVisitor.java | 236 ---------------------
.../org/apache/ofbiz/widget/model/HtmlWidget.java | 19 ++
.../ofbiz/widget/model/ModelScreenWidget.java | 19 --
.../widget/model/MultiBlockHtmlTemplateUtil.java | 132 +++++++++---
6 files changed, 128 insertions(+), 282 deletions(-)
diff --git a/build.gradle b/build.gradle
index 7ad21a2..1e55676 100644
--- a/build.gradle
+++ b/build.gradle
@@ -286,7 +286,7 @@ checkstyle {
// the sum of errors found last time it was changed after using the
// ‘checkstyle’ tool present in the framework and in the official
// plugins.
- tasks.checkstyleMain.maxErrors = 26985
+ tasks.checkstyleMain.maxErrors = 26908
// Currently there are a lot of errors so we need to temporarily
// hide them to avoid polluting the terminal output.
showViolations = false
diff --git a/framework/widget/dtd/widget-screen.xsd
b/framework/widget/dtd/widget-screen.xsd
index 6e3fe97..3bf4f1b 100644
--- a/framework/widget/dtd/widget-screen.xsd
+++ b/framework/widget/dtd/widget-screen.xsd
@@ -524,7 +524,7 @@ under the License.
<xs:documentation>
Multi-block processing of template targeted for the html
body.
Inline script will be rendered as external script after
html body tag.
- External script tag with attribute data-import='head' will
be rendered within html head tag if the template location is static.
+ External script tag with attribute data-import='head' will
be rendered within html head tag.
</xs:documentation>
</xs:annotation>
</xs:attribute>
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlImportWidgetVisitor.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlImportWidgetVisitor.java
deleted file mode 100644
index 9e6add0..0000000
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlImportWidgetVisitor.java
+++ /dev/null
@@ -1,236 +0,0 @@
-/*******************************************************************************
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
-
*******************************************************************************/
-package org.apache.ofbiz.widget.model;
-
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
-import org.apache.ofbiz.base.util.FileUtil;
-import org.apache.ofbiz.base.util.UtilValidate;
-import org.jsoup.Jsoup;
-import org.jsoup.nodes.Document;
-import org.jsoup.select.Elements;
-
-public final class HtmlImportWidgetVisitor implements ModelWidgetVisitor {
-
- private Set<String> jsImports = new LinkedHashSet<String>();
-
- /**
- * Get the script source locations collected by HtmlImportWidgetVisitor
- * @return
- */
- public Set<String> getJsImports() {
- return jsImports;
- }
-
- @Override
- public void visit(HtmlWidget htmlWidget) throws Exception {
- List<ModelScreenWidget> widgetList = htmlWidget.getSubWidgets();
- for (ModelScreenWidget widget: widgetList) {
- // HtmlTemplate
- widget.accept(this);
- }
- }
-
- @Override
- public void visit(HtmlWidget.HtmlTemplate htmlTemplate) throws Exception {
- String fileLocation = htmlTemplate.locationExdr.getOriginal();
- boolean isStaticLocation = !fileLocation.contains("${");
- if (isStaticLocation && htmlTemplate.isMultiBlock()) {
- String template = FileUtil.readString("UTF-8",
FileUtil.getFile(fileLocation));
- Document doc = Jsoup.parseBodyFragment(template);
- Elements scriptElements = doc.select("script");
- if (scriptElements != null && scriptElements.size() > 0) {
- for (org.jsoup.nodes.Element script : scriptElements) {
- String src = script.attr("src");
- if (UtilValidate.isNotEmpty(src)) {
- String dataImport = script.attr("data-import");
- if ("head".equals(dataImport)) {
- jsImports.add(src);
- }
- }
- }
- }
- }
- }
-
- @Override
- public void visit(HtmlWidget.HtmlTemplateDecorator htmlTemplateDecorator)
throws Exception {
-
- }
-
- @Override
- public void visit(HtmlWidget.HtmlTemplateDecoratorSection
htmlTemplateDecoratorSection) throws Exception {
-
- }
-
- @Override
- public void visit(IterateSectionWidget iterateSectionWidget) throws
Exception {
-
- }
-
- @Override
- public void visit(ModelSingleForm modelForm) throws Exception {
-
- }
-
- @Override
- public void visit(ModelGrid modelGrid) throws Exception {
-
- }
-
- @Override
- public void visit(ModelMenu modelMenu) throws Exception {
-
- }
-
- @Override
- public void visit(ModelMenuItem modelMenuItem) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreen modelScreen) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.ColumnContainer columnContainer)
throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Container container) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Content content) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.DecoratorScreen decoratorScreen)
throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.DecoratorSection decoratorSection)
throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.DecoratorSectionInclude
decoratorSectionInclude) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Form form) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Grid grid) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.HorizontalSeparator
horizontalSeparator) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.ScreenImage image) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.IncludeScreen includeScreen) throws
Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Label label) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.ScreenLink link) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Menu menu) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.PlatformSpecific platformSpecific)
throws Exception {
- Map<String, ModelScreenWidget> widgetMap =
platformSpecific.getSubWidgets();
- for (Map.Entry<String, ModelScreenWidget> entry :
widgetMap.entrySet()) {
- if (entry.getKey().equals("html")) {
- // HtmlWidget
- entry.getValue().accept(this);
- }
- }
-
- }
-
- @Override
- public void visit(ModelScreenWidget.PortalPage portalPage) throws
Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Screenlet screenlet) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Section section) throws Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Tree tree) throws Exception {
-
- }
-
- @Override
- public void visit(ModelTree modelTree) throws Exception {
-
- }
-
- @Override
- public void visit(ModelTree.ModelNode modelNode) throws Exception {
-
- }
-
- @Override
- public void visit(ModelTree.ModelNode.ModelSubNode modelSubNode) throws
Exception {
-
- }
-
- @Override
- public void visit(ModelScreenWidget.Column column) throws Exception {
-
- }
-}
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
index 4817a37..3e4f3cd 100644
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/HtmlWidget.java
@@ -24,8 +24,10 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.GeneralException;
@@ -261,6 +263,23 @@ public class HtmlWidget extends ModelScreenWidget {
super(modelScreen, htmlTemplateElement);
this.locationExdr =
FlexibleStringExpander.getInstance(htmlTemplateElement.getAttribute("location"));
this.multiBlock =
!"false".equals(htmlTemplateElement.getAttribute("multi-block"));
+
+ if (this.isMultiBlock()) {
+ String origLoc = this.locationExdr.getOriginal();
+ Set<String> urls = null;
+ if (origLoc.contains("${")) {
+ urls = new LinkedHashSet<>();
+ urls.add(origLoc);
+ } else {
+ try {
+ urls =
MultiBlockHtmlTemplateUtil.getHtmlImportsFromHtmlTemplate(origLoc);
+ } catch (IOException e) {
+ String errMsg = "Error getting html imports from
template at location [" + origLoc + "]: " + e.toString();
+ Debug.logError(e, errMsg, MODULE);
+ }
+ }
+
MultiBlockHtmlTemplateUtil.addLinksToHtmlImportCache(modelScreen.getSourceLocation(),
modelScreen.getName(), urls);
+ }
}
public String getLocation(Map<String, Object> context) {
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelScreenWidget.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelScreenWidget.java
index a460679..c3701f3 100644
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelScreenWidget.java
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelScreenWidget.java
@@ -259,25 +259,6 @@ public abstract class ModelScreenWidget extends
ModelWidget {
this.failWidgets = Collections.emptyList();
}
this.isMainSection = isMainSection;
-
- String screenLocation = modelScreen.getSourceLocation();
- String screenName = modelScreen.getName();
-
- // check required html import from the widgets
- HtmlImportWidgetVisitor visitor = new HtmlImportWidgetVisitor();
- try {
- for (ModelScreenWidget widget : this.subWidgets) {
- widget.accept(visitor);
- }
- for (ModelScreenWidget widget : this.failWidgets) {
- widget.accept(visitor);
- }
-
MultiBlockHtmlTemplateUtil.addLinksToHtmlImportCache(screenLocation,
screenName, visitor.getJsImports());
- } catch (Exception e) {
- String errMsg = "Error adding links to Html Import Cache";
- Debug.logError(e, errMsg, MODULE);
- throw new RuntimeException(errMsg);
- }
}
@Override
diff --git
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MultiBlockHtmlTemplateUtil.java
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MultiBlockHtmlTemplateUtil.java
index 7404963..a7bc33e 100644
---
a/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MultiBlockHtmlTemplateUtil.java
+++
b/framework/widget/src/main/java/org/apache/ofbiz/widget/model/MultiBlockHtmlTemplateUtil.java
@@ -18,6 +18,7 @@
*******************************************************************************/
package org.apache.ofbiz.widget.model;
+import java.io.IOException;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
@@ -28,42 +29,69 @@ import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;
+import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.FileUtil;
import org.apache.ofbiz.base.util.UtilGenerics;
import org.apache.ofbiz.base.util.UtilValidate;
+import org.apache.ofbiz.base.util.string.FlexibleStringExpander;
+import org.jsoup.Jsoup;
+import org.jsoup.nodes.Document;
+import org.jsoup.select.Elements;
public final class MultiBlockHtmlTemplateUtil {
+ private static final String MODULE =
MultiBlockHtmlTemplateUtil.class.getName();
public static final String MULTI_BLOCK_WRITER = "multiBlockWriter";
private static final String HTML_LINKS_FOR_HEAD = "htmlLinksForHead";
private static final String SCRIPT_LINKS_FOR_FOOT = "ScriptLinksForFoot";
- private static int maxScriptCacheSizePerSession = 10;
- // store inline script from freemarker template by user session
+ private static int maxScriptCacheSizePerUserSession = 10;
+ private static int estimatedConcurrentUserSessions = 250;
+ private static int estimatedScreensWithMultiBlockHtmlTemplate = 200;
+ private static int estimatedScreensWithChildScreen = 200;
+ /**
+ * Store inline script extracted from freemarker template for a user
session.
+ * Number of inline scripts for a user session will be constraint by
{@link MultiBlockHtmlTemplateUtil#maxScriptCacheSizePerUserSession}
+ * {@link MultiBlockHtmlTemplateUtil#cleanupScriptCache(HttpSession)} will
be called to remove entry when session ends.
+ */
private static LinkedHashMap<String, Map<String, String>> scriptCache =
new LinkedHashMap<String, Map<String, String>>() {
private static final long serialVersionUID = 1L;
protected boolean removeEldestEntry(Map.Entry<String,
Map<String, String>> eldest) {
- return size() > 100; // TODO probably set to max number of
concurrent user
+ return size() > estimatedConcurrentUserSessions;
}
};
- // store the additional html import by screen location
+ /**
+ * For each screen containing html-template, store a set of html imports
headerized from html-template.
+ * The set may contain entry of an expression of the the html-template's
location.
+ * In this case, we need to expand the location expression, read the
html-template for any html imports.
+ */
private static LinkedHashMap<String, Set<String>> htmlImportCache =
new LinkedHashMap<String, Set<String>>() {
private static final long serialVersionUID = 1L;
protected boolean removeEldestEntry(Map.Entry<String,
Set<String>> eldest) {
- return size() > 300; // TODO probably set to max number of
screens
+ return size() > estimatedScreensWithMultiBlockHtmlTemplate;
}
};
- // store the child screen
+ /**
+ * Store set of dependent screens info, in form of screen location + hash
+ name, of a given parent screen.
+ * The set may contain entry of an expression of the dependent screen
location and will need to be expanded before use.
+ */
private static LinkedHashMap<String, Set<String>> dependentScreenCache =
new LinkedHashMap<String, Set<String>>() {
private static final long serialVersionUID = 1L;
protected boolean removeEldestEntry(Map.Entry<String,
Set<String>> eldest) {
- return size() > 100;
+ return size() > estimatedScreensWithChildScreen;
}
};
private MultiBlockHtmlTemplateUtil() { }
+ /**
+ * Add child screen info to {@link
MultiBlockHtmlTemplateUtil#dependentScreenCache}.
+ * @param parentModelScreen parent screen.
+ * @param location screen location. Expression is allowed.
+ * @param name screen name. Expression is allowed.
+ */
public static void collectChildScreenInfo(ModelScreen parentModelScreen,
String location, String name) {
String key = parentModelScreen.getSourceLocation() + "#" +
parentModelScreen.getName();
Set<String> childList = dependentScreenCache.get(key);
@@ -78,7 +106,14 @@ public final class MultiBlockHtmlTemplateUtil {
}
}
- public static void addLinksToHtmlImportCache(String location, String name,
Set<String> urls) throws Exception {
+ /**
+ * Add Html Imports to {@link MultiBlockHtmlTemplateUtil#htmlImportCache}.
+ * @param location screen location. Expression is not allowed.
+ * @param name screen name. Expression is not allowed.
+ * @param urls Set of html links associated with the screen. May contain
expression to html-template location.
+ * @throws Exception
+ */
+ public static void addLinksToHtmlImportCache(String location, String name,
Set<String> urls) {
if (UtilValidate.isEmpty(urls)) {
return;
}
@@ -91,6 +126,37 @@ public final class MultiBlockHtmlTemplateUtil {
existingUrls.addAll(urls);
}
+ /**
+ * Get html import scr location from html template
+ * @param fileLocation Location to html template. Expression is not
allowed.
+ * @return
+ */
+ public static Set<String> getHtmlImportsFromHtmlTemplate(String
fileLocation) throws IOException {
+ Set<String> imports = new LinkedHashSet<>();
+ String template = FileUtil.readString("UTF-8",
FileUtil.getFile(fileLocation));
+ Document doc = Jsoup.parseBodyFragment(template);
+ Elements scriptElements = doc.select("script");
+ if (scriptElements != null && scriptElements.size() > 0) {
+ for (org.jsoup.nodes.Element script : scriptElements) {
+ String src = script.attr("src");
+ if (UtilValidate.isNotEmpty(src)) {
+ String dataImport = script.attr("data-import");
+ if ("head".equals(dataImport)) {
+ imports.add(src);
+ }
+ }
+ }
+ }
+ return imports;
+ }
+
+ /**
+ * Add html links to the header
+ * @param context
+ * @param location screen location. Expression is not allowed.
+ * @param name screen name. Expression is not allowed.
+ * @throws Exception
+ */
public static void addLinksToLayoutSettings(final Map<String, Object>
context, String location, String name) throws Exception {
HttpServletRequest request = (HttpServletRequest)
context.get("request");
if (request.getAttribute(HTML_LINKS_FOR_HEAD) == null) {
@@ -107,18 +173,37 @@ public final class MultiBlockHtmlTemplateUtil {
if (UtilValidate.isEmpty(layoutSettingsJsList)) {
return;
}
+ // ensure initTheme.groovy has run.
+ Map<String, String> commonScreenLocations =
UtilGenerics.cast(context.get("commonScreenLocations"));
+ if (UtilValidate.isEmpty(commonScreenLocations)) {
+ return;
+ }
Object objValue = request.getAttribute(HTML_LINKS_FOR_HEAD);
if (objValue instanceof String) {
String currentLocationHashName = (String)
request.getAttribute(HTML_LINKS_FOR_HEAD);
Set<String> htmlLinks = new LinkedHashSet<>();
- Set<String> locHashNameList =
getRelatedScreenLocationHashName(currentLocationHashName);
+ Set<String> locHashNameList =
getRelatedScreenLocationHashName(currentLocationHashName, context);
for (String locHashName:locHashNameList) {
- Set<String> urls = htmlImportCache.get(locHashName);
+ String expandLocHashName = "";
+ if (locHashName.contains("${")) {
+ expandLocHashName =
FlexibleStringExpander.expandString(locHashName, context);
+ } else {
+ expandLocHashName = locHashName;
+ }
+ Set<String> urls = htmlImportCache.get(expandLocHashName);
if (UtilValidate.isNotEmpty(urls)) {
- // check url is not already in layoutSettings.javaScripts
for (String url : urls) {
- if (!htmlLinks.contains(url)) {
- htmlLinks.add(url);
+ if (url.contains("${")) {
+ String expandUrl =
FlexibleStringExpander.expandString(url, context);
+ if (UtilValidate.isNotEmpty(expandUrl)) {
+
htmlLinks.addAll(getHtmlImportsFromHtmlTemplate(expandUrl));
+ } else {
+ Debug.log("Unable to expand " + url, MODULE);
+ }
+ } else {
+ if (!htmlLinks.contains(url)) {
+ htmlLinks.add(url);
+ }
}
}
}
@@ -141,28 +226,25 @@ public final class MultiBlockHtmlTemplateUtil {
* @param locationHashName
* @return
*/
- private static Set<String> getRelatedScreenLocationHashName(String
locationHashName) {
+ private static Set<String> getRelatedScreenLocationHashName(String
locationHashName, final Map<String, Object> context) {
Set<String> resultList = new HashSet<>();
resultList.add(locationHashName);
Set<String> locHashNameList =
dependentScreenCache.get(locationHashName);
if (locHashNameList != null) {
for (String locHashName : locHashNameList) {
-
resultList.addAll(getRelatedScreenLocationHashName(locHashName));
+ String exLocHashName = "";
+ if (locHashName.contains("${")) {
+ exLocHashName =
FlexibleStringExpander.expandString(locHashName, context);
+ } else {
+ exLocHashName = locHashName;
+ }
+
resultList.addAll(getRelatedScreenLocationHashName(exLocHashName, context));
}
}
return resultList;
}
/**
- * add the script links that should be in the head tag
- * @param context
- * @param urls
- */
- private static void addJsLinkToLayoutSettings(final Map<String, Object>
context, final Set<String> urls) {
-
- }
-
- /**
* add script link for page footer.
* @param context
* @param filePath
@@ -203,7 +285,7 @@ public final class MultiBlockHtmlTemplateUtil {
scriptMap = new LinkedHashMap<String, String>() {
private static final long serialVersionUID = 1L;
protected boolean removeEldestEntry(Map.Entry<String, String>
eldest) {
- return size() > maxScriptCacheSizePerSession;
+ return size() > maxScriptCacheSizePerUserSession;
}
};
scriptCache.put(sessionId, scriptMap);
