You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by mt...@apache.org on 2019/12/07 22:25:13 UTC

[ofbiz-framework] branch trunk updated (62c617e -> 4d4a1ba)

This is an automated email from the ASF dual-hosted git repository.

mthl pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.


    from 62c617e  Fixed: Add XML declaration in “web.xml” files (OFBIZ-6993)
     new 98bfafd  Improved: Resolve classpath conflict on `freemarkerTransforms.properties` (OFBIZ-11161)
     new 376b818  Improved: Do not add classpath info to the classloader classpath (OFBIZ-11161)
     new 4d4a1ba  Improved: Lint ‘FreeMarkerWorker’ class (OFBIZ-11161)

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../ofbiz/content}/freemarkerTransforms.properties |   0
 .../ofbiz/product}/freemarkerTransforms.properties |   0
 build.gradle                                       |   2 +-
 .../org/apache/ofbiz/base/container/Classpath.java |  91 ------------
 .../ofbiz/base/container/ComponentContainer.java   |  45 ------
 .../ofbiz/base/util/template/FreeMarkerWorker.java | 157 ++++++++++++---------
 .../base/container/ComponentContainerTest.java     |  11 --
 .../ofbiz/webapp}/freemarkerTransforms.properties  |   0
 .../org/apache/ofbiz/widget/model/HtmlWidget.java  |   2 +-
 .../ofbiz/widget}/freemarkerTransforms.properties  |   0
 10 files changed, 89 insertions(+), 219 deletions(-)
 rename applications/content/{config => src/main/resources/org/apache/ofbiz/content}/freemarkerTransforms.properties (100%)
 rename applications/product/{config => src/main/resources/org/apache/ofbiz/product}/freemarkerTransforms.properties (100%)
 delete mode 100644 framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java
 rename framework/webapp/{config => src/main/resources/org/apache/ofbiz/webapp}/freemarkerTransforms.properties (100%)
 rename framework/widget/{config => src/main/resources/org/apache/ofbiz/widget}/freemarkerTransforms.properties (100%)


[ofbiz-framework] 01/03: Improved: Resolve classpath conflict on `freemarkerTransforms.properties` (OFBIZ-11161)

Posted by mt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mthl pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 98bfafd1485574d849c24a22158f407578840d0e
Author: Samuel Trégouët <sa...@nereide.fr>
AuthorDate: Sat Dec 7 14:47:34 2019 +0100

    Improved: Resolve classpath conflict on `freemarkerTransforms.properties`
    (OFBIZ-11161)
    
    All files in {compoment}/config directories are placed in jar
    root (cf. sourceSets definition in build.gradle) but there are 4 different
    `freemarkerTransforms.properties` in ofbiz (content, product, webapp, widget).
    This works fine because we are adding config directories in classpath (through
    classpath tag in ofbiz-component.xml), but this is not really intuitive. So to
    make things clearer this patch move `freemarkerTransforms.properties` into
    `src/main/resources` with a namespace package and `FreeMarkerWorker.java` is
    modified accordingly.
---
 .../ofbiz/content}/freemarkerTransforms.properties |  0
 .../ofbiz/product}/freemarkerTransforms.properties |  0
 .../ofbiz/base/util/template/FreeMarkerWorker.java | 39 +++++++++++++---------
 .../ofbiz/webapp}/freemarkerTransforms.properties  |  0
 .../ofbiz/widget}/freemarkerTransforms.properties  |  0
 5 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/applications/content/config/freemarkerTransforms.properties b/applications/content/src/main/resources/org/apache/ofbiz/content/freemarkerTransforms.properties
similarity index 100%
rename from applications/content/config/freemarkerTransforms.properties
rename to applications/content/src/main/resources/org/apache/ofbiz/content/freemarkerTransforms.properties
diff --git a/applications/product/config/freemarkerTransforms.properties b/applications/product/src/main/resources/org/apache/ofbiz/product/freemarkerTransforms.properties
similarity index 100%
rename from applications/product/config/freemarkerTransforms.properties
rename to applications/product/src/main/resources/org/apache/ofbiz/product/freemarkerTransforms.properties
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
index 2b574f1..edc2c25 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
@@ -23,19 +23,21 @@ import java.io.IOException;
 import java.io.Writer;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
 import java.util.TimeZone;
+import java.util.stream.Stream;
 
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
 
 import org.apache.ofbiz.base.location.FlexibleLocation;
+import org.apache.ofbiz.base.component.ComponentConfig;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.StringUtil;
 import org.apache.ofbiz.base.util.UtilGenerics;
@@ -68,6 +70,8 @@ import freemarker.template.Version;
  * FreeMarkerWorker - Freemarker Template Engine Utilities.
  */
 public final class FreeMarkerWorker {
+    /** The template used to retrieved Freemarker transforms from multiple component classpaths. */
+    private static final String TRANSFORMS_PROPERTIES = "org/apache/ofbiz/%s/freemarkerTransforms.properties";
 
     public static final String module = FreeMarkerWorker.class.getName();
 
@@ -122,27 +126,30 @@ public final class FreeMarkerWorker {
         }
         // Transforms properties file set up as key=transform name, property=transform class name
         ClassLoader loader = Thread.currentThread().getContextClassLoader();
-        Enumeration<URL> resources;
-        try {
-            resources = loader.getResources("freemarkerTransforms.properties");
-        } catch (IOException e) {
-            Debug.logError(e, "Could not load list of freemarkerTransforms.properties", module);
-            throw UtilMisc.initCause(new InternalError(e.getMessage()), e);
-        }
-        while (resources.hasMoreElements()) {
-            URL propertyURL = resources.nextElement();
-            Debug.logInfo("loading properties: " + propertyURL, module);
-            Properties props = UtilProperties.getProperties(propertyURL);
-            if (UtilValidate.isEmpty(props)) {
-                Debug.logError("Unable to locate properties file " + propertyURL, module);
+        transformsURL(loader).forEach(url -> {
+            Properties props = UtilProperties.getProperties(url);
+            if (props == null) {
+                Debug.logError("Unable to load properties file " + url, module);
             } else {
+                Debug.logInfo("loading properties: " + url, module);
                 loadTransforms(loader, props, newConfig);
             }
-        }
-
+        });
         return newConfig;
     }
 
+    /**
+     * Provides the sequence of existing {@code freemarkerTransforms.properties} files.
+     *
+     * @return a stream of resource location.
+     */
+    private static Stream<URL> transformsURL(ClassLoader loader) {
+        return ComponentConfig.components()
+                .map(cc -> String.format(TRANSFORMS_PROPERTIES, cc.getComponentName()))
+                .map(loader::getResource)
+                .filter(Objects::nonNull);
+    }
+
     private static void loadTransforms(ClassLoader loader, Properties props, Configuration config) {
         for (Object object : props.keySet()) {
             String key = (String) object;
diff --git a/framework/webapp/config/freemarkerTransforms.properties b/framework/webapp/src/main/resources/org/apache/ofbiz/webapp/freemarkerTransforms.properties
similarity index 100%
rename from framework/webapp/config/freemarkerTransforms.properties
rename to framework/webapp/src/main/resources/org/apache/ofbiz/webapp/freemarkerTransforms.properties
diff --git a/framework/widget/config/freemarkerTransforms.properties b/framework/widget/src/main/resources/org/apache/ofbiz/widget/freemarkerTransforms.properties
similarity index 100%
rename from framework/widget/config/freemarkerTransforms.properties
rename to framework/widget/src/main/resources/org/apache/ofbiz/widget/freemarkerTransforms.properties


[ofbiz-framework] 03/03: Improved: Lint ‘FreeMarkerWorker’ class (OFBIZ-11161)

Posted by mt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mthl pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 4d4a1ba1cda48f02d0cf2ce0cb95e9abacaef968
Author: Mathieu Lirzin <ma...@nereide.fr>
AuthorDate: Sat Dec 7 18:38:36 2019 +0100

    Improved: Lint ‘FreeMarkerWorker’ class
    (OFBIZ-11161)
---
 build.gradle                                       |   2 +-
 .../ofbiz/base/util/template/FreeMarkerWorker.java | 122 +++++++++++----------
 .../org/apache/ofbiz/widget/model/HtmlWidget.java  |   2 +-
 3 files changed, 68 insertions(+), 58 deletions(-)

diff --git a/build.gradle b/build.gradle
index 9d1981c..3d4b81b 100644
--- a/build.gradle
+++ b/build.gradle
@@ -285,7 +285,7 @@ checkstyle {
     // the sum of errors that were present before introducing the
     // ‘checkstyle’ tool present in the framework and in the official
     // plugins.
-    tasks.checkstyleMain.maxErrors = 37769
+    tasks.checkstyleMain.maxErrors = 37729
     // 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/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
index edc2c25..558ec48 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/template/FreeMarkerWorker.java
@@ -1,4 +1,4 @@
-/*******************************************************************************
+/*
  * 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
@@ -15,7 +15,7 @@
  * KIND, either express or implied.  See the License for the
  * specific language governing permissions and limitations
  * under the License.
- *******************************************************************************/
+ */
 package org.apache.ofbiz.base.util.template;
 
 import java.io.File;
@@ -60,7 +60,6 @@ import freemarker.template.SimpleHash;
 import freemarker.template.SimpleScalar;
 import freemarker.template.Template;
 import freemarker.template.TemplateException;
-import freemarker.template.TemplateExceptionHandler;
 import freemarker.template.TemplateHashModel;
 import freemarker.template.TemplateModel;
 import freemarker.template.TemplateModelException;
@@ -73,23 +72,25 @@ public final class FreeMarkerWorker {
     /** The template used to retrieved Freemarker transforms from multiple component classpaths. */
     private static final String TRANSFORMS_PROPERTIES = "org/apache/ofbiz/%s/freemarkerTransforms.properties";
 
-    public static final String module = FreeMarkerWorker.class.getName();
+    private static final String MODULE = FreeMarkerWorker.class.getName();
 
-    public static final Version version = Configuration.VERSION_2_3_29;
+    public static final Version VERSION = Configuration.VERSION_2_3_29;
 
-    private FreeMarkerWorker () {}
+    private FreeMarkerWorker() { }
 
-    // use soft references for this so that things from Content records don't kill all of our memory, or maybe not for performance reasons... hmmm, leave to config file...
-    private static final UtilCache<String, Template> cachedTemplates = UtilCache.createUtilCache("template.ftl.general", 0, 0, false);
-    private static final BeansWrapper defaultOfbizWrapper = new BeansWrapperBuilder(version).build();
-    private static final Configuration defaultOfbizConfig = makeConfiguration(defaultOfbizWrapper);
+    // Use soft references for this so that things from Content records don't kill all of our memory,
+    // or maybe not for performance reasons... hmmm, leave to config file...
+    private static final UtilCache<String, Template> CACHED_TEMPLATES =
+            UtilCache.createUtilCache("template.ftl.general", 0, 0, false);
+    private static final BeansWrapper DEFAULT_OFBIZ_WRAPPER = new BeansWrapperBuilder(VERSION).build();
+    private static final Configuration DEFAULT_OFBIZ_CONFIG = makeConfiguration(DEFAULT_OFBIZ_WRAPPER);
 
     public static BeansWrapper getDefaultOfbizWrapper() {
-        return defaultOfbizWrapper;
+        return DEFAULT_OFBIZ_WRAPPER;
     }
 
     public static Configuration newConfiguration() {
-        return new Configuration(version);
+        return new Configuration(VERSION);
     }
 
     public static Configuration makeConfiguration(BeansWrapper wrapper) {
@@ -101,7 +102,7 @@ public final class FreeMarkerWorker {
         try {
             newConfig.setSharedVariable("EntityQuery", staticModels.get("org.apache.ofbiz.entity.util.EntityQuery"));
         } catch (TemplateModelException e) {
-            Debug.logError(e, module);
+            Debug.logError(e, MODULE);
         }
         newConfig.setLocalizedLookup(false);
         newConfig.setSharedVariable("StringUtil", new BeanModel(StringUtil.INSTANCE, wrapper));
@@ -122,16 +123,16 @@ public final class FreeMarkerWorker {
             newConfig.setSetting("datetime_format", "yyyy-MM-dd HH:mm:ss.SSS");
             newConfig.setSetting("number_format", "0.##########");
         } catch (TemplateException e) {
-            Debug.logError("Unable to set date/time and number formats in FreeMarker: " + e, module);
+            Debug.logError("Unable to set date/time and number formats in FreeMarker: " + e, MODULE);
         }
         // Transforms properties file set up as key=transform name, property=transform class name
         ClassLoader loader = Thread.currentThread().getContextClassLoader();
         transformsURL(loader).forEach(url -> {
             Properties props = UtilProperties.getProperties(url);
             if (props == null) {
-                Debug.logError("Unable to load properties file " + url, module);
+                Debug.logError("Unable to load properties file " + url, MODULE);
             } else {
-                Debug.logInfo("loading properties: " + url, module);
+                Debug.logInfo("loading properties: " + url, MODULE);
                 loadTransforms(loader, props, newConfig);
             }
         });
@@ -155,12 +156,12 @@ public final class FreeMarkerWorker {
             String key = (String) object;
             String className = props.getProperty(key);
             if (Debug.verboseOn()) {
-                Debug.logVerbose("Adding FTL Transform " + key + " with class " + className, module);
+                Debug.logVerbose("Adding FTL Transform " + key + " with class " + className, MODULE);
             }
             try {
                 config.setSharedVariable(key, loader.loadClass(className).getDeclaredConstructor().newInstance());
             } catch (Exception e) {
-                Debug.logError(e, "Could not pre-initialize dynamically loaded class: " + className + ": " + e, module);
+                Debug.logError(e, "Could not pre-initialize dynamically loaded class: " + className + ": " + e, MODULE);
             }
         }
     }
@@ -171,18 +172,22 @@ public final class FreeMarkerWorker {
      * @param context The context Map
      * @param outWriter The Writer to render to
      */
-    public static void renderTemplate(String templateLocation, Map<String, Object> context, Appendable outWriter) throws TemplateException, IOException {
+    public static void renderTemplate(String templateLocation, Map<String, Object> context, Appendable outWriter)
+            throws TemplateException, IOException {
         Template template = getTemplate(templateLocation);
         renderTemplate(template, context, outWriter);
     }
 
-    public static void renderTemplateFromString(String templateName, String templateString, Map<String, Object> context, Appendable outWriter, long lastModificationTime, boolean useCache) throws TemplateException, IOException {
+    public static void renderTemplateFromString(String templateName, String templateString,
+            Map<String, Object> context, Appendable outWriter, long lastModificationTime, boolean useCache)
+                    throws TemplateException, IOException {
         Template template = null;
         if (useCache) {
-            template = cachedTemplates.get(templateName);
+            template = CACHED_TEMPLATES.get(templateName);
         }
         if (template == null) {
-            StringTemplateLoader stringTemplateLoader = (StringTemplateLoader)((MultiTemplateLoader)defaultOfbizConfig.getTemplateLoader()).getTemplateLoader(1);
+            MultiTemplateLoader templateLoader = (MultiTemplateLoader) DEFAULT_OFBIZ_CONFIG.getTemplateLoader();
+            StringTemplateLoader stringTemplateLoader = (StringTemplateLoader) templateLoader.getTemplateLoader(1);
             Object templateSource = stringTemplateLoader.findTemplateSource(templateName);
             if (templateSource == null || stringTemplateLoader.getLastModified(templateSource) < lastModificationTime) {
                 stringTemplateLoader.putTemplate(templateName, templateString, lastModificationTime);
@@ -194,11 +199,11 @@ public final class FreeMarkerWorker {
     }
 
     public static void clearTemplateFromCache(String templateLocation) {
-        cachedTemplates.remove(templateLocation);
+        CACHED_TEMPLATES.remove(templateLocation);
         try {
-            defaultOfbizConfig.removeTemplateFromCache(templateLocation);
-        } catch(Exception e) {
-            Debug.logInfo("Template not found in Fremarker cache with name: " + templateLocation, module);
+            DEFAULT_OFBIZ_CONFIG.removeTemplateFromCache(templateLocation);
+        } catch (Exception e) {
+            Debug.logInfo("Template not found in Fremarker cache with name: " + templateLocation, MODULE);
         }
     }
 
@@ -208,7 +213,8 @@ public final class FreeMarkerWorker {
      * @param context The context Map
      * @param outWriter The Writer to render to
      */
-    public static Environment renderTemplate(Template template, Map<String, Object> context, Appendable outWriter) throws TemplateException, IOException {
+    public static Environment renderTemplate(Template template, Map<String, Object> context, Appendable outWriter)
+            throws TemplateException, IOException {
         // make sure there is no "null" string in there as FreeMarker will try to use it
         context.remove("null");
         // Since the template cache keeps a single instance of a Template that is shared among users,
@@ -252,7 +258,7 @@ public final class FreeMarkerWorker {
      * @return A <code>Configuration</code> instance.
      */
     public static Configuration getDefaultOfbizConfig() {
-        return defaultOfbizConfig;
+        return DEFAULT_OFBIZ_CONFIG;
     }
 
     /**
@@ -261,10 +267,11 @@ public final class FreeMarkerWorker {
      * @param templateLocation Location of the template - file path or URL
      */
     public static Template getTemplate(String templateLocation) throws IOException {
-        return getTemplate(templateLocation, cachedTemplates, defaultOfbizConfig);
+        return getTemplate(templateLocation, CACHED_TEMPLATES, DEFAULT_OFBIZ_CONFIG);
     }
 
-    public static Template getTemplate(String templateLocation, UtilCache<String, Template> cache, Configuration config) throws IOException {
+    public static Template getTemplate(String templateLocation, UtilCache<String, Template> cache, Configuration config)
+            throws IOException {
         Template template = cache.get(templateLocation);
         if (template == null) {
             template = config.getTemplate(templateLocation);
@@ -278,7 +285,8 @@ public final class FreeMarkerWorker {
         return getArg(args, key, templateContext);
     }
 
-    public static String getArg(Map<String, ? extends Object> args, String key, Map<String, ? extends Object> templateContext) {
+    public static String getArg(Map<String, ? extends Object> args, String key,
+            Map<String, ? extends Object> templateContext) {
         Object o = args.get(key);
         String returnVal = (String) unwrap(o);
         if (returnVal == null) {
@@ -287,7 +295,7 @@ public final class FreeMarkerWorker {
                     returnVal = (String) templateContext.get(key);
                 }
             } catch (ClassCastException e2) {
-                Debug.logInfo(e2.getMessage(), module);
+                Debug.logInfo(e2.getMessage(), MODULE);
             }
         }
         return returnVal;
@@ -313,7 +321,7 @@ public final class FreeMarkerWorker {
                 }
             }
         } catch (TemplateModelException e) {
-            Debug.logInfo(e.getMessage(), module);
+            Debug.logInfo(e.getMessage(), MODULE);
         }
         return UtilGenerics.<T>cast(obj);
     }
@@ -323,7 +331,7 @@ public final class FreeMarkerWorker {
         try {
             o = args.get(key);
         } catch (TemplateModelException e) {
-            Debug.logVerbose(e.getMessage(), module);
+            Debug.logVerbose(e.getMessage(), MODULE);
             return null;
         }
 
@@ -334,7 +342,7 @@ public final class FreeMarkerWorker {
             try {
                 ctxObj = args.get("context");
             } catch (TemplateModelException e) {
-                Debug.logInfo(e.getMessage(), module);
+                Debug.logInfo(e.getMessage(), MODULE);
                 return returnObj;
             }
             Map<String, ?> ctx = null;
@@ -369,7 +377,8 @@ public final class FreeMarkerWorker {
         try {
             varNames = UtilGenerics.cast(env.getKnownVariableNames());
         } catch (TemplateModelException e1) {
-            Debug.logError(e1, "Error getting FreeMarker variable names, will not put pass current context on to sub-content", module);
+            String msg = "Error getting FreeMarker variable names, will not put pass current context on to sub-content";
+            Debug.logError(e1, msg, MODULE);
         }
         if (varNames != null) {
             for (String varName: varNames) {
@@ -379,7 +388,8 @@ public final class FreeMarkerWorker {
         return templateRoot;
     }
 
-    public static void saveContextValues(Map<String, Object> context, String [] saveKeyNames, Map<String, Object> saveMap) {
+    public static void saveContextValues(Map<String, Object> context, String[] saveKeyNames,
+            Map<String, Object> saveMap) {
         for (String key: saveKeyNames) {
             Object o = context.get(key);
             if (o instanceof Map<?, ?>) {
@@ -391,7 +401,7 @@ public final class FreeMarkerWorker {
         }
     }
 
-    public static Map<String, Object> saveValues(Map<String, Object> context, String [] saveKeyNames) {
+    public static Map<String, Object> saveValues(Map<String, Object> context, String[] saveKeyNames) {
         Map<String, Object> saveMap = new HashMap<>();
         for (String key: saveKeyNames) {
             Object o = context.get(key);
@@ -456,9 +466,9 @@ public final class FreeMarkerWorker {
             throw new IllegalArgumentException("Error in getSiteParameters, context/ctx cannot be null");
         }
         ServletContext servletContext = request.getSession().getServletContext();
-        String rootDir = (String)ctx.get("rootDir");
-        String webSiteId = (String)ctx.get("webSiteId");
-        String https = (String)ctx.get("https");
+        String rootDir = (String) ctx.get("rootDir");
+        String webSiteId = (String) ctx.get("webSiteId");
+        String https = (String) ctx.get("https");
         if (UtilValidate.isEmpty(rootDir)) {
             rootDir = servletContext.getRealPath("/");
             ctx.put("rootDir", rootDir);
@@ -474,13 +484,13 @@ public final class FreeMarkerWorker {
     }
 
     public static TemplateModel autoWrap(Object obj, Environment env) {
-       TemplateModel templateModelObj = null;
-       try {
-           templateModelObj = getDefaultOfbizWrapper().wrap(obj);
-       } catch (TemplateModelException e) {
-           throw new RuntimeException(e.getMessage());
-       }
-       return templateModelObj;
+        TemplateModel templateModelObj = null;
+        try {
+            templateModelObj = getDefaultOfbizWrapper().wrap(obj);
+        } catch (TemplateModelException e) {
+            throw new RuntimeException(e.getMessage());
+        }
+        return templateModelObj;
     }
 
     /*
@@ -498,9 +508,9 @@ public final class FreeMarkerWorker {
             try {
                 locationUrl = FlexibleLocation.resolveLocation(name);
             } catch (Exception e) {
-                Debug.logWarning("Unable to locate the template: " + name, module);
+                Debug.logWarning("Unable to locate the template: " + name, MODULE);
             }
-            return locationUrl != null && new File(locationUrl.getFile()).exists()? locationUrl: null;
+            return locationUrl != null && new File(locationUrl.getFile()).exists() ? locationUrl : null;
         }
     }
 
@@ -510,7 +520,7 @@ public final class FreeMarkerWorker {
      * This is done by suppressing the exception and replacing it by a generic char for quiet alert.
      * Note that exception is still logged.
      * <p>
-     * This implements the {@link TemplateExceptionHandler} functional interface.
+     * This implements the {@link freemarker.template.TemplateExceptionHandler} functional interface.
      *
      * @param te  the exception that occurred
      * @param env  the runtime environment of the template
@@ -519,9 +529,9 @@ public final class FreeMarkerWorker {
     private static void handleTemplateException(TemplateException te, Environment env, Writer out) {
         try {
             out.write(UtilProperties.getPropertyValue("widget", "widget.freemarker.template.exception.message", "∎"));
-            Debug.logError(te, module);
+            Debug.logError(te, MODULE);
         } catch (IOException e) {
-            Debug.logError(e, module);
+            Debug.logError(e, MODULE);
         }
     }
 
@@ -532,7 +542,7 @@ public final class FreeMarkerWorker {
      * present in the stack trace are sanitized before printing them to the output writer.
      * Note that exception is still logged.
      * <p>
-     * This implements the {@link TemplateExceptionHandler} functional interface.
+     * This implements the {@link freemarker.template.TemplateExceptionHandler} functional interface.
      *
      * @param te  the exception that occurred
      * @param env  the runtime environment of the template
@@ -541,9 +551,9 @@ public final class FreeMarkerWorker {
     private static void handleTemplateExceptionVerbosily(TemplateException te, Environment env, Writer out) {
         try {
             out.write(te.getMessage());
-            Debug.logError(te, module);
+            Debug.logError(te, MODULE);
         } catch (IOException e) {
-            Debug.logError(e, module);
+            Debug.logError(e, MODULE);
         }
     }
 }
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 0065792..dda71e2 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
@@ -59,7 +59,7 @@ public class HtmlWidget extends ModelScreenWidget {
     public static final String module = HtmlWidget.class.getName();
 
     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));
+    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...
     public static class ExtendedWrapper extends BeansWrapper {


[ofbiz-framework] 02/03: Improved: Do not add classpath info to the classloader classpath (OFBIZ-11161)

Posted by mt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mthl pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 376b81846436e291263b3f45e4aeb1b9257c685d
Author: Mathieu Lirzin <ma...@nereide.fr>
AuthorDate: Fri Nov 8 19:02:45 2019 +0100

    Improved: Do not add classpath info to the classloader classpath (OFBIZ-11161)
    
    Those directories are already added in the classpath by the build
    system. The classpath info defined in “ofbiz-component.xml” files are
    now only used to retrieve label files.
---
 .../org/apache/ofbiz/base/container/Classpath.java | 91 ----------------------
 .../ofbiz/base/container/ComponentContainer.java   | 45 -----------
 .../base/container/ComponentContainerTest.java     | 11 ---
 3 files changed, 147 deletions(-)

diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java b/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java
deleted file mode 100644
index f49b8bb..0000000
--- a/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java
+++ /dev/null
@@ -1,91 +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.base.container;
-
-import java.io.File;
-import java.io.IOException;
-import java.net.URI;
-import java.nio.file.FileSystems;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.PathMatcher;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-
-import org.apache.ofbiz.base.component.ComponentConfig.ClasspathInfo;
-
-/**
- * A class path object.
- *
- * <p>This reifies the notion of a Java class path to be able to manipulate them programmatically.
- */
-final class Classpath {
-    /** {@code .jar} and {@code .zip} files matcher. */
-    private static final PathMatcher JAR_ZIP_FILES = FileSystems.getDefault().getPathMatcher("glob:*.{java,zip}");
-
-    /** A sequence of unique path elements. */
-    private final LinkedHashSet<Path> elements = new LinkedHashSet<>();
-
-    /**
-     * Adds a directory or a file to the class path.
-     *
-     * In the directory case, all files ending with ".jar" or ".zip" inside this directory
-     * are added to the class path.
-     *
-     * @param cpi  a valid class path information
-     * @throws NullPointerException when {@code cpi} is {@code null}.
-     */
-    void add(ClasspathInfo cpi) {
-        Path file = cpi.location();
-        switch (cpi.type()) {
-        case JAR:
-            elements.add(file);
-            break;
-        case DIR:
-            elements.add(file);
-            try (Stream<Path> innerFiles = Files.list(file)) {
-                innerFiles.filter(JAR_ZIP_FILES::matches).forEach(elements::add);
-            } catch (IOException e) {
-                String fmt = "Warning : Module classpath component '%s' is not valid and will be ignored...";
-                System.err.println(String.format(fmt, file));
-            }
-            break;
-        }
-    }
-
-    @Override
-    public String toString() {
-        return elements.stream()
-                .map(Path::toString)
-                .collect(Collectors.joining(File.pathSeparator));
-    }
-
-    /**
-     * Returns the list of class path component URIs.
-     *
-     * @return a list of class path component URIs
-     */
-    List<URI> toUris() {
-        return elements.stream()
-                .map(Path::toUri)
-                .collect(Collectors.toList());
-     }
-}
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java b/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
index 31f6e60..621d690 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
@@ -20,15 +20,11 @@ package org.apache.ofbiz.base.container;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
-import java.net.URI;
 import java.net.URL;
-import java.net.URLClassLoader;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.apache.ofbiz.base.component.ComponentConfig;
@@ -83,12 +79,6 @@ public class ComponentContainer implements Container {
         } catch (IOException | ComponentException e) {
             throw new ContainerException(e);
         }
-        String fmt = "Added class path for component : [%s]";
-        List<Classpath> componentsClassPath = ComponentConfig.components()
-                .peek(cmpnt -> Debug.logInfo(String.format(fmt, cmpnt.getComponentName()), module))
-                .map(cmpnt -> buildClasspathFromComponentConfig(cmpnt))
-                .collect(Collectors.toList());
-        loadClassPathForAllComponents(componentsClassPath);
         Debug.logInfo("All components loaded", module);
     }
 
@@ -98,28 +88,6 @@ public class ComponentContainer implements Container {
     }
 
     /**
-     * Iterate over all the components and load their classpath URLs into the classloader
-     * and set the classloader as the context classloader
-     *
-     * @param componentsClassPath a list of classpaths for all components
-     */
-    private static void loadClassPathForAllComponents(List<Classpath> componentsClassPath) {
-        List<URL> allComponentUrls = new ArrayList<>();
-        for (Classpath classPath : componentsClassPath) {
-            try {
-                for (URI uri : classPath.toUris()) {
-                    allComponentUrls.add(uri.toURL());
-                }
-            } catch (MalformedURLException e) {
-                Debug.logError(e, "Unable to load component classpath %s", module, classPath);
-            }
-        }
-        URL[] componentURLs = allComponentUrls.toArray(new URL[allComponentUrls.size()]);
-        URLClassLoader classLoader = new URLClassLoader(componentURLs, Thread.currentThread().getContextClassLoader());
-        Thread.currentThread().setContextClassLoader(classLoader);
-    }
-
-    /**
      * Loads any kind of component definition.
      *
      * @param dir  the location where the component should be loaded
@@ -223,19 +191,6 @@ public class ComponentContainer implements Container {
         return config;
     }
 
-    /**
-     * Constructs a {@code Classpath} object for a specific component definition.
-     *
-     * @param config  the component configuration
-     * @return the associated class path information
-     * @see ComponentConfig
-     */
-    private static Classpath buildClasspathFromComponentConfig(ComponentConfig config) {
-        Classpath res = new Classpath();
-        config.getClasspathInfos().forEach(res::add);
-        return res;
-    }
-
     @Override
     public void stop() {
     }
diff --git a/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java b/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java
index 480b162..617b21a 100644
--- a/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java
+++ b/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java
@@ -22,8 +22,6 @@ import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
-import java.net.URL;
-import java.net.URLClassLoader;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -69,15 +67,6 @@ public class ComponentContainerTest {
         List<String> loadedComponents = ComponentConfig.components()
                 .map(c -> c.getGlobalName())
                 .collect(Collectors.toList());
-        // we can cast ContextClassLoader since ComponentContainer.loadClassPathForAllComponents has called
-        // setContextClassLoader with an URLClassLoader instance
-        URL[] classpath = ((URLClassLoader) Thread.currentThread().getContextClassLoader()).getURLs();
-        List<URL> actualClasspath = Arrays.asList(classpath);
-        List<URL> expectedClasspath = Arrays.asList(
-                ofbizHome.resolve(ORDER_CONFIG).toUri().toURL(),
-                ofbizHome.resolve(ACCOUNTING_CONFIG).toUri().toURL());
-
-        assertEquals(expectedClasspath, actualClasspath);
         assertEquals(Arrays.asList("order", "accounting"), loadedComponents);
     }
 }