[ofbiz-framework] branch trunk updated: Improved: Headerize external script in multi-block html template (OFBIZ-11741)

[James Yong-2]

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 347ea63  Improved: Headerize external script in multi-block html template (OFBIZ-11741)
347ea63 is described below

commit 347ea63479d21db63653d0e52de60732d81654d7
Author: James Yong <[hidden email]>
AuthorDate: Sat Jun 6 10:23:41 2020 +0800

    Improved: Headerize external script in multi-block html template (OFBIZ-11741)
   
    Improve function for adding links to layoutSettings
---
 .../org/apache/ofbiz/widget/model/HtmlWidget.java  |  22 ++--
 .../ofbiz/widget/model/ModelScreenWidget.java      |   4 +-
 .../widget/model/MultiBlockHtmlTemplateUtil.java   | 139 ++++++++++++++++-----
 3 files changed, 126 insertions(+), 39 deletions(-)

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 b7a9cc9..1527be2 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
@@ -64,7 +64,8 @@ import freemarker.template.Version;
 public class HtmlWidget extends ModelScreenWidget {
     private static final String MODULE = HtmlWidget.class.getName();
 
-    private static final UtilCache<String, Template> specialTemplateCache = UtilCache.createUtilCache("widget.screen.template.ftl.general", 0, 0, false);
+    private static final UtilCache<String, Template> specialTemplateCache =
+            UtilCache.createUtilCache("widget.screen.template.ftl.general", 0, 0, false);
     protected static final Configuration specialConfig = FreeMarkerWorker.makeConfiguration(new ExtendedWrapper(FreeMarkerWorker.VERSION));
 
     // not sure if this is the best way to get FTL to use my fancy MapModel derivative, but should work at least...
@@ -140,7 +141,8 @@ public class HtmlWidget extends ModelScreenWidget {
     }
 
     @Override
-    public void renderWidgetString(Appendable writer, Map<String, Object> context, ScreenStringRenderer screenStringRenderer) throws GeneralException, IOException {
+    public void renderWidgetString(Appendable writer, Map<String, Object> context, ScreenStringRenderer screenStringRenderer)
+            throws GeneralException, IOException {
         for (ModelScreenWidget subWidget : subWidgets) {
             subWidget.renderWidgetString(writer, context, screenStringRenderer);
         }
@@ -181,7 +183,8 @@ public class HtmlWidget extends ModelScreenWidget {
         }
     }
 
-    public static void renderHtmlTemplateMultiBlock(Appendable writer, FlexibleStringExpander locationExdr, Map<String, Object> context) throws IOException {
+    public static void renderHtmlTemplateMultiBlock(Appendable writer, FlexibleStringExpander locationExdr,
+                                                    Map<String, Object> context) throws IOException {
         String location = locationExdr.expandString(context);
 
         StringWriter stringWriter = new StringWriter();
@@ -260,13 +263,13 @@ public class HtmlWidget extends ModelScreenWidget {
                     urls.add(origLoc);
                 } else {
                     try {
-                        urls = MultiBlockHtmlTemplateUtil.getHtmlImportsFromHtmlTemplate(origLoc);
+                        urls = MultiBlockHtmlTemplateUtil.getHtmlLinksFromHtmlTemplate(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);
+                MultiBlockHtmlTemplateUtil.addHtmlLinksToHtmlLinksForScreenCache(modelScreen.getSourceLocation(), modelScreen.getName(), urls);
             }
         }
 
@@ -306,7 +309,8 @@ public class HtmlWidget extends ModelScreenWidget {
             super(modelScreen, htmlTemplateDecoratorElement);
             this.locationExdr = FlexibleStringExpander.getInstance(htmlTemplateDecoratorElement.getAttribute("location"));
 
-            List<? extends Element> htmlTemplateDecoratorSectionElementList = UtilXml.childElementList(htmlTemplateDecoratorElement, "html-template-decorator-section");
+            List<? extends Element> htmlTemplateDecoratorSectionElementList = UtilXml.childElementList(
+                    htmlTemplateDecoratorElement, "html-template-decorator-section");
             for (Element htmlTemplateDecoratorSectionElement: htmlTemplateDecoratorSectionElementList) {
                 String name = htmlTemplateDecoratorSectionElement.getAttribute("name");
                 this.sectionMap.put(name, new HtmlTemplateDecoratorSection(modelScreen, htmlTemplateDecoratorSectionElement));
@@ -324,7 +328,8 @@ public class HtmlWidget extends ModelScreenWidget {
                 contextMs = UtilGenerics.cast(context);
             }
 
-            // create a standAloneStack, basically a "save point" for this SectionsRenderer, and make a new "screens" object just for it so it is isolated and doesn't follow the stack down
+            // create a standAloneStack, basically a "save point" for this SectionsRenderer,
+            // and make a new "screens" object just for it so it is isolated and doesn't follow the stack down
             MapStack<String> standAloneStack = contextMs.standAloneChildStack();
             standAloneStack.put("screens", new ScreenRenderer(writer, standAloneStack, screenStringRenderer));
             SectionsRenderer sections = new SectionsRenderer(this.sectionMap, standAloneStack, writer, screenStringRenderer);
@@ -361,7 +366,8 @@ public class HtmlWidget extends ModelScreenWidget {
         }
 
         @Override
-        public void renderWidgetString(Appendable writer, Map<String, Object> context, ScreenStringRenderer screenStringRenderer) throws GeneralException, IOException {
+        public void renderWidgetString(Appendable writer, Map<String, Object> context,
+                                       ScreenStringRenderer screenStringRenderer) throws GeneralException, IOException {
             // render sub-widgets
             renderSubWidgetsString(this.subWidgets, writer, context, screenStringRenderer);
         }
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 c34dfc5..e0914be 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
@@ -777,7 +777,7 @@ public abstract class ModelScreenWidget extends ModelWidget {
             this.nameExdr = FlexibleStringExpander.getInstance(includeScreenElement.getAttribute("name"));
             this.locationExdr = FlexibleStringExpander.getInstance(includeScreenElement.getAttribute("location"));
             this.shareScopeExdr = FlexibleStringExpander.getInstance(includeScreenElement.getAttribute("share-scope"));
-            MultiBlockHtmlTemplateUtil.collectChildScreenInfo(modelScreen, this.locationExdr.getOriginal(), this.nameExdr.getOriginal());
+            MultiBlockHtmlTemplateUtil.addChildScreen(modelScreen, this.locationExdr.getOriginal(), this.nameExdr.getOriginal());
         }
 
         @Override
@@ -873,7 +873,7 @@ public abstract class ModelScreenWidget extends ModelWidget {
                 sectionMap.put(decoratorSection.getName(), decoratorSection);
             }
             this.sectionMap = Collections.unmodifiableMap(sectionMap);
-            MultiBlockHtmlTemplateUtil.collectChildScreenInfo(modelScreen, this.locationExdr.getOriginal(), this.nameExdr.getOriginal());
+            MultiBlockHtmlTemplateUtil.addChildScreen(modelScreen, this.locationExdr.getOriginal(), this.nameExdr.getOriginal());
         }
 
         @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 e850c47..df005cd 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
@@ -19,7 +19,9 @@
 package org.apache.ofbiz.widget.model;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -29,7 +31,6 @@ 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;
@@ -63,9 +64,9 @@ public final class MultiBlockHtmlTemplateUtil {
     /**
      * 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.
+     * In this case, we need to expand the location expression before reading the html-template for any html imports.
      */
-    private static LinkedHashMap<String, Set<String>> htmlImportCache =
+    private static LinkedHashMap<String, Set<String>> htmlLinksForScreenCache =
             new LinkedHashMap<String, Set<String>>() {
                 private static final long serialVersionUID = 1L;
                 protected boolean removeEldestEntry(Map.Entry<String, Set<String>> eldest) {
@@ -76,7 +77,7 @@ public final class MultiBlockHtmlTemplateUtil {
      * 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 =
+    private static LinkedHashMap<String, Set<String>> childScreenCache =
             new LinkedHashMap<String, Set<String>>() {
                 private static final long serialVersionUID = 1L;
                 protected boolean removeEldestEntry(Map.Entry<String, Set<String>> eldest) {
@@ -87,17 +88,25 @@ public final class MultiBlockHtmlTemplateUtil {
     private MultiBlockHtmlTemplateUtil() { }
 
     /**
-     * Add child screen info to {@link MultiBlockHtmlTemplateUtil#dependentScreenCache}.
+     * Allow unit testing to check results
+     * @return
+     */
+    public static Map<String, Set<String>> getChildScreenCache() {
+        return Collections.unmodifiableMap(childScreenCache);
+    }
+
+    /**
+     * Add child screen info to {@link MultiBlockHtmlTemplateUtil#childScreenCache}.
      * @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) {
+    public static void addChildScreen(ModelScreen parentModelScreen, String location, String name) {
         String key = parentModelScreen.getSourceLocation() + "#" + parentModelScreen.getName();
-        Set<String> childList = dependentScreenCache.get(key);
+        Set<String> childList = childScreenCache.get(key);
         if (childList == null) {
             childList = new LinkedHashSet<>();
-            dependentScreenCache.put(key, childList);
+            childScreenCache.put(key, childList);
         }
         if (UtilValidate.isNotEmpty(location)) {
             childList.add(location + "#" + name);
@@ -107,22 +116,22 @@ public final class MultiBlockHtmlTemplateUtil {
     }
 
     /**
-     * Add Html Imports to {@link MultiBlockHtmlTemplateUtil#htmlImportCache}.
+     * Add Html Imports to {@link MultiBlockHtmlTemplateUtil#htmlLinksForScreenCache}.
      * @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.
      */
-    public static void addLinksToHtmlImportCache(String location, String name, Set<String> urls) {
+    public static void addHtmlLinksToHtmlLinksForScreenCache(String location, String name, Set<String> urls) {
         if (UtilValidate.isEmpty(urls)) {
             return;
         }
         String locHashName = location + "#" + name;
-        Set<String> existingUrls = htmlImportCache.get(locHashName);
-        if (existingUrls == null) {
-            existingUrls = new LinkedHashSet<>();
-            htmlImportCache.put(locHashName, existingUrls);
+        Set<String> htmlLinks = htmlLinksForScreenCache.get(locHashName);
+        if (htmlLinks == null) {
+            htmlLinks = new LinkedHashSet<>();
+            htmlLinksForScreenCache.put(locHashName, htmlLinks);
         }
-        existingUrls.addAll(urls);
+        htmlLinks.addAll(urls);
     }
 
     /**
@@ -130,7 +139,7 @@ public final class MultiBlockHtmlTemplateUtil {
      * @param fileLocation Location to html template. Expression is not allowed.
      * @return
      */
-    public static Set<String> getHtmlImportsFromHtmlTemplate(String fileLocation) throws IOException {
+    public static Set<String> getHtmlLinksFromHtmlTemplate(String fileLocation) throws IOException {
         Set<String> imports = new LinkedHashSet<>();
         String template = FileUtil.readString("UTF-8", FileUtil.getFile(fileLocation));
         Document doc = Jsoup.parseBodyFragment(template);
@@ -150,7 +159,8 @@ public final class MultiBlockHtmlTemplateUtil {
     }
 
     /**
-     * Store the 1st screen called by request
+     * Store the 1st screen called by request.
+     * Request attribute HTML_LINKS_FOR_HEAD will be set to a string containing the initial screen location + hash + name
      * @param context
      * @param location screen location. Expression is not allowed.
      * @param name screen name. Expression is not allowed.
@@ -164,7 +174,10 @@ public final class MultiBlockHtmlTemplateUtil {
     }
 
     /**
-     * Add html links to the header
+     * Add html links to the header.
+     * Request attribute HTML_LINKS_FOR_HEAD will be set to boolean when all html links are added to the 'layoutSettings'.
+     * Request attribute HTML_LINKS_FOR_HEAD will be set to Object[] when there are expressions for screen location or
+     * template location that need to be expanded, in order to read the content of screen or template.
      * @param context
      * @throws IOException
      */
@@ -187,27 +200,32 @@ public final class MultiBlockHtmlTemplateUtil {
         }
         Object objValue = request.getAttribute(HTML_LINKS_FOR_HEAD);
         if (objValue instanceof String) {
-            Set<String> retryHtmlLinks = new LinkedHashSet<>();
+            // store expressions for Template Location that is not expanded correctly, for retry.
+            Set<String> retryTemplateLocationExpressions = new LinkedHashSet<>();
+            // store expressions for Screen Location + Hash + Name that is not expanded correctly, for retry.
+            Set<String> retryScreenLocHashNameExpressions = new LinkedHashSet<>();
             String currentLocationHashName = (String) request.getAttribute(HTML_LINKS_FOR_HEAD);
+            // store the html links that will be added to 'layoutSettings'
             Set<String> htmlLinks = new LinkedHashSet<>();
             Set<String> locHashNameList = getRelatedScreenLocationHashName(currentLocationHashName, context);
             for (String locHashName:locHashNameList) {
-                String expandLocHashName = "";
                 if (locHashName.contains("${")) {
-                    expandLocHashName = FlexibleStringExpander.expandString(locHashName, context);
-                } else {
-                    expandLocHashName = locHashName;
+                    // as getRelatedScreenLocationHashName method has already tried to expand the expression;
+                    // just add to the retry variable for the next loop.
+                    retryScreenLocHashNameExpressions.add(locHashName);
+                    continue;
                 }
-                Set<String> urls = htmlImportCache.get(expandLocHashName);
+                // check for any html links associated with the screen
+                Set<String> urls = htmlLinksForScreenCache.get(locHashName);
                 if (UtilValidate.isNotEmpty(urls)) {
                     for (String url : urls) {
+                        // if an expression is found, treat it like template location location, instead of html link
                         if (url.contains("${")) {
                             String expandUrl = FlexibleStringExpander.expandString(url, context);
                             if (UtilValidate.isNotEmpty(expandUrl)) {
-                                htmlLinks.addAll(getHtmlImportsFromHtmlTemplate(expandUrl));
+                                htmlLinks.addAll(getHtmlLinksFromHtmlTemplate(expandUrl));
                             } else {
-                                retryHtmlLinks.add(url);
-                                Debug.log("Unable to expand " + url, MODULE);
+                                retryTemplateLocationExpressions.add(url);
                             }
                         } else {
                             if (!htmlLinks.contains(url)) {
@@ -225,8 +243,68 @@ public final class MultiBlockHtmlTemplateUtil {
                     }
                 }
             }
-            if (UtilValidate.isEmpty(retryHtmlLinks)) {
+            if (UtilValidate.isEmpty(retryScreenLocHashNameExpressions) && UtilValidate.isEmpty(retryTemplateLocationExpressions)) {
                 request.setAttribute(HTML_LINKS_FOR_HEAD, true);
+            } else {
+                Object[] retry = new Object[2];
+                retry[0] = retryScreenLocHashNameExpressions;
+                retry[1] = retryTemplateLocationExpressions;
+                request.setAttribute(HTML_LINKS_FOR_HEAD, retry);
+            }
+        } else if (objValue instanceof Object[]) {
+            Object[] retry = UtilGenerics.cast(request.getAttribute(HTML_LINKS_FOR_HEAD));
+            Set<String> retryScreenLocHashNameExpressions = UtilGenerics.cast(retry[0]);
+            Set<String> retryTemplateLocationExpressions = UtilGenerics.cast(retry[1]);
+            Set<String> htmlLinks = new HashSet<>();
+            if (UtilValidate.isNotEmpty(retryScreenLocHashNameExpressions)) {
+                for (Iterator<String> it = retryScreenLocHashNameExpressions.iterator(); it.hasNext();) {
+                    String locHashName = it.next();
+                    String expandLocHashName = FlexibleStringExpander.expandString(locHashName, context);
+                    if (!expandLocHashName.startsWith("#") && !expandLocHashName.endsWith("#")) {
+                        Set<String> urls = htmlLinksForScreenCache.get(expandLocHashName);
+                        if (UtilValidate.isNotEmpty(urls)) {
+                            for (String url : urls) {
+                                if (url.contains("${")) {
+                                    retryTemplateLocationExpressions.add(url);
+                                } else {
+                                    if (!htmlLinks.contains(url)) {
+                                        htmlLinks.add(url);
+                                    }
+                                }
+                            }
+                        }
+                        it.remove();
+                    }
+                }
+            }
+
+            if (UtilValidate.isNotEmpty(retryTemplateLocationExpressions)) {
+                for (Iterator<String> it = retryTemplateLocationExpressions.iterator(); it.hasNext();) {
+                    String url = it.next();
+                    // we know url contains "${", so we expand the url
+                    String expandUrl = FlexibleStringExpander.expandString(url, context);
+                    if (UtilValidate.isNotEmpty(expandUrl)) {
+                        htmlLinks.addAll(getHtmlLinksFromHtmlTemplate(expandUrl));
+                        it.remove();
+                    }
+
+                }
+            }
+            if (UtilValidate.isNotEmpty(htmlLinks)) {
+                // check url is not already in layoutSettings.javaScripts
+                for (String url : htmlLinks) {
+                    if (!layoutSettingsJsList.contains(url)) {
+                        layoutSettingsJsList.add(url);
+                    }
+                }
+            }
+            if (UtilValidate.isEmpty(retryScreenLocHashNameExpressions) && UtilValidate.isEmpty(retryTemplateLocationExpressions)) {
+                request.setAttribute(HTML_LINKS_FOR_HEAD, true);
+            } else {
+                Object[] retry2 = new Object[2];
+                retry2[0] = retryScreenLocHashNameExpressions;
+                retry2[1] = retryTemplateLocationExpressions;
+                request.setAttribute(HTML_LINKS_FOR_HEAD, retry2);
             }
         }
 
@@ -240,12 +318,15 @@ public final class MultiBlockHtmlTemplateUtil {
     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);
+        Set<String> locHashNameList = childScreenCache.get(locationHashName);
         if (locHashNameList != null) {
             for (String locHashName : locHashNameList) {
                 String exLocHashName = "";
                 if (locHashName.contains("${")) {
                     exLocHashName = FlexibleStringExpander.expandString(locHashName, context);
+                    if (exLocHashName.startsWith("#") || exLocHashName.endsWith("#")) {
+                        exLocHashName = locHashName;
+                    }
                 } else {
                     exLocHashName = locHashName;
                 }