A recent discussion was proposing to switch to slf4j as the façade. On Aug 20, 2015 3:12 PM, "Claude Brisson" <cla...@renegat.net> wrote:
> Thanks for the review. > > Yes, we should be using Velocity logging facade, but I just don't see for > now how we should do this while preserving tools serialization. > > For now I just mimicked a behavior present in some deprecated classes, > re-introducing the dependency of velocity-tools-view on commons-logging. > > I agree that we should try to do better, but if a logger cannot be > statically reached from a session tool, and if it cannot be passed at > initialization because of serialization, there's a problem... > > Let say for instance that we use transient weak references on > non-serializable Velocity loggers, fed up at initialization time. Then the > tool could detect it has lost its logger at deserialization, and ask a new > one, but to whom? It's a bad idea either to keep a static reference on the > servlet context. > > I'm open to suggestions... > > > Claude > > > On 20/08/2015 20:45, Sergiu Dumitriu wrote: > >> Isn't the logging framework supposed to be pluggable, meaning that all >> calls go through org.apache.velocity.runtime.log.Log which is then >> passed to the actual logging library in use, be it log4j, slf4j, jcl or >> commons-logging? >> >> On 08/20/2015 01:16 PM, cbris...@apache.org wrote: >> >>> Author: cbrisson >>> Date: Thu Aug 20 17:16:02 2015 >>> New Revision: 1696822 >>> >>> URL: http://svn.apache.org/r1696822 >>> Log: >>> J2EE containers session serialization implies static logging from >>> session scoped tools >>> >>> Modified: >>> velocity/tools/trunk/src/site/xdoc/changes.xml >>> >>> >>> velocity/tools/trunk/velocity-tools-struts/src/main/java/org/apache/velocity/tools/struts/TilesTool.java >>> >>> >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/AbstractSearchTool.java >>> >>> >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/BrowserTool.java >>> >>> >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ImportSupport.java >>> >>> >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/tools/ImportTool.java >>> >>> Modified: velocity/tools/trunk/src/site/xdoc/changes.xml >>> URL: >>> http://svn.apache.org/viewvc/velocity/tools/trunk/src/site/xdoc/changes.xml?rev=1696822&r1=1696821&r2=1696822&view=diff >>> >>> ============================================================================== >>> --- velocity/tools/trunk/src/site/xdoc/changes.xml (original) >>> +++ velocity/tools/trunk/src/site/xdoc/changes.xml Thu Aug 20 17:16:02 >>> 2015 >>> @@ -35,6 +35,7 @@ >>> <li>Reflect Velocity Engine dependency shading of commons-lang >>> and commons-collections (cb)</li> >>> <li>Removed deprecated class >>> org.apache.velocity.tools.generic.log.LogSystemCommonsLog (cb)</li> >>> <li>MathTool: fixed result type calculations and added bitwise >>> operations (cb)</li> >>> + <li>have session scoped tools use a static logger to fix J2EE >>> containers session serialization (cb)</li> >>> </ul> >>> </subsection> >>> >>> Modified: >>> velocity/tools/trunk/velocity-tools-struts/src/main/java/org/apache/velocity/tools/struts/TilesTool.java >>> URL: >>> http://svn.apache.org/viewvc/velocity/tools/trunk/velocity-tools-struts/src/main/java/org/apache/velocity/tools/struts/TilesTool.java?rev=1696822&r1=1696821&r2=1696822&view=diff >>> >>> ============================================================================== >>> --- >>> velocity/tools/trunk/velocity-tools-struts/src/main/java/org/apache/velocity/tools/struts/TilesTool.java >>> (original) >>> +++ >>> velocity/tools/trunk/velocity-tools-struts/src/main/java/org/apache/velocity/tools/struts/TilesTool.java >>> Thu Aug 20 17:16:02 2015 >>> @@ -101,7 +101,6 @@ public class TilesTool extends ImportSup >>> setRequest(ctx.getRequest()); >>> setResponse(ctx.getResponse()); >>> setServletContext(ctx.getServletContext()); >>> - setLog(ctx.getVelocityEngine().getLog()); >>> } >>> } >>> >>> Modified: >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/AbstractSearchTool.java >>> URL: >>> http://svn.apache.org/viewvc/velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/AbstractSearchTool.java?rev=1696822&r1=1696821&r2=1696822&view=diff >>> >>> ============================================================================== >>> --- >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/AbstractSearchTool.java >>> (original) >>> +++ >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/AbstractSearchTool.java >>> Thu Aug 20 17:16:02 2015 >>> @@ -22,7 +22,8 @@ package org.apache.velocity.tools.view; >>> import java.util.Collections; >>> import java.util.List; >>> import javax.servlet.http.HttpServletRequest; >>> -import org.apache.velocity.runtime.log.Log; >>> +import org.apache.commons.logging.Log; >>> +import org.apache.commons.logging.LogFactory; >>> import org.apache.velocity.tools.Scope; >>> import org.apache.velocity.tools.config.DefaultKey; >>> import org.apache.velocity.tools.config.InvalidScope; >>> @@ -129,19 +130,11 @@ public abstract class AbstractSearchTool >>> protected static final String STORED_RESULTS_KEY = >>> StoredResults.class.getName(); >>> - protected Log LOG; >>> + protected static final Log LOG = >>> LogFactory.getLog(AbstractSearchTool.class); >>> + >>> private String criteriaKey = DEFAULT_CRITERIA_KEY; >>> private Object criteria; >>> - public void setLog(Log log) >>> - { >>> - if (log == null) >>> - { >>> - throw new NullPointerException("log should not be set to >>> null"); >>> - } >>> - this.LOG = log; >>> - } >>> - >>> /** >>> * Sets the criteria *if* it is set in the request parameters. >>> */ >>> >>> Modified: >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/BrowserTool.java >>> URL: >>> http://svn.apache.org/viewvc/velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/BrowserTool.java?rev=1696822&r1=1696821&r2=1696822&view=diff >>> >>> ============================================================================== >>> --- >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/BrowserTool.java >>> (original) >>> +++ >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/BrowserTool.java >>> Thu Aug 20 17:16:02 2015 >>> @@ -24,7 +24,8 @@ import java.util.regex.Pattern; >>> import java.util.regex.PatternSyntaxException; >>> import java.util.*; >>> import javax.servlet.http.HttpServletRequest; >>> -import org.apache.velocity.runtime.log.Log; >>> +import org.apache.commons.logging.Log; >>> +import org.apache.commons.logging.LogFactory; >>> import org.apache.velocity.tools.Scope; >>> import org.apache.velocity.tools.ConversionUtils; >>> import org.apache.velocity.tools.generic.FormatConfig; >>> @@ -73,8 +74,8 @@ import org.apache.velocity.tools.config. >>> public class BrowserTool extends FormatConfig implements >>> java.io.Serializable >>> { >>> private static final long serialVersionUID = 1734529350532353339L; >>> - >>> - protected Log LOG; >>> + >>> + protected static final Log LOG = >>> LogFactory.getLog(BrowserTool.class); >>> /* User-Agent header variables */ >>> private String userAgent = null; >>> @@ -187,19 +188,6 @@ public class BrowserTool extends FormatC >>> } >>> /** >>> - * Set log. >>> - */ >>> - public void setLog(Log log) >>> - { >>> - if (log == null) >>> - { >>> - throw new NullPointerException("log should not be set to >>> null"); >>> - } >>> - this.LOG = log; >>> - } >>> - >>> - >>> - /** >>> * Sets the User-Agent string to be parsed for info. If null, the >>> string >>> * will be empty and everything will return false or null. >>> Otherwise, >>> * it will set the whole string to lower case before storing to >>> simplify >>> >>> Modified: >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ImportSupport.java >>> URL: >>> http://svn.apache.org/viewvc/velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ImportSupport.java?rev=1696822&r1=1696821&r2=1696822&view=diff >>> >>> ============================================================================== >>> --- >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ImportSupport.java >>> (original) >>> +++ >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/ImportSupport.java >>> Thu Aug 20 17:16:02 2015 >>> @@ -39,12 +39,17 @@ import javax.servlet.ServletOutputStream >>> import javax.servlet.http.HttpServletRequest; >>> import javax.servlet.http.HttpServletResponse; >>> import javax.servlet.http.HttpServletResponseWrapper; >>> -import org.apache.velocity.runtime.log.Log; >>> +import org.apache.commons.logging.Log; >>> +import org.apache.commons.logging.LogFactory; >>> + >>> /** >>> * <p>Provides methods to import arbitrary local or remote resources >>> as strings.</p> >>> * <p>Based on ImportSupport from the JSTL taglib by Shawn Bayern</p> >>> * >>> + * <p>If you wish to take profit of your J2EE container session >>> serialization, >>> + * inheriting tools should always be in the request scope.</p> >>> + * >>> * @author <a href="mailto:mari...@centrum.is";>Marino A. Jonsson</a> >>> * @since VelocityTools 2.0 >>> * @version $Revision$ $Date$ >>> @@ -57,7 +62,8 @@ public abstract class ImportSupport >>> /** Default character encoding for response. */ >>> protected static final String DEFAULT_ENCODING = "ISO-8859-1"; >>> - protected Log LOG; >>> + protected static final Log LOG = >>> LogFactory.getLog(ImportSupport.class); >>> + >>> protected ServletContext application; >>> protected HttpServletRequest request; >>> protected HttpServletResponse response; >>> @@ -65,15 +71,6 @@ public abstract class ImportSupport >>> // --------------------------------------- Setup Methods >>> ------------- >>> - public void setLog(Log log) >>> - { >>> - if (log == null) >>> - { >>> - throw new NullPointerException("log should not be set to >>> null"); >>> - } >>> - this.LOG = log; >>> - } >>> - >>> /** >>> * Sets the current {@link HttpServletRequest}. This is required >>> * for this tool to operate and will throw a NullPointerException >>> >>> Modified: >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/tools/ImportTool.java >>> URL: >>> http://svn.apache.org/viewvc/velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/tools/ImportTool.java?rev=1696822&r1=1696821&r2=1696822&view=diff >>> >>> ============================================================================== >>> --- >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/tools/ImportTool.java >>> (original) >>> +++ >>> velocity/tools/trunk/velocity-tools-view/src/main/java/org/apache/velocity/tools/view/tools/ImportTool.java >>> Thu Aug 20 17:16:02 2015 >>> @@ -36,7 +36,6 @@ public class ImportTool extends org.apac >>> setRequest(ctx.getRequest()); >>> setResponse(ctx.getResponse()); >>> setServletContext(ctx.getServletContext()); >>> - setLog(ctx.getVelocityEngine().getLog()); >>> } >>> } >>> } >>> >>> >>> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org > For additional commands, e-mail: dev-h...@velocity.apache.org > >
