It looks like that with slf4j, we could keep loggers injection even in
session tools, so no need to go static:
"As of SLF4J version 1.5.3, logger instances survive serialization.
Thus, serialization of the host class no longer requires any special
action, even when loggers are declared as instance variables. In
previous versions, logger instances needed to be declared as transient
in the host class."
( http://www.slf4j.org/faq.html#declared_static )
So it looks like the perfect solution, and will simplify many things.
Claude
On 20/08/2015 21:32, Sergiu Dumitriu wrote:
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
