Sorry, but I'm not sure I understand what you're proposing. Can you make
a short summary?

On 08/30/2015 12:07 PM, Claude Brisson wrote:
> Some remarks after a little investigation.
> 
> 1) Using the ServletContext logger doesn't seem to be a widespread
> practice among slf4j folks. There is a slf4j-servletcontext
> implementation on github, but it is not referenced on the official slf4j
> site. Moreover, I tested it, and it does not support deserialization (I
> had some doubts about it, because deserialization may happen before
> ServletContext initialization), so it cannot be used as is. Plus, it
> needs to be declared as context's listener in the deployment file.
> 
> It's possible to solve those two problems (alas not from our own codebase):
>  - The logger object can be a proxy to a transcient implementation that
> waits for the servlet context to be initialized.
>  - Since J2EE API v3.0, there is no need to declare the listener in the
> deployment descriptor, as it can be done using Java annotations, which
> would fit slf4j 'jar'-only binding strategy far more than the current
> state of the project.
> 
> But not using the ServletContext logger is not necessarily a bad idea.
> Of course, "it's here", so why not use it, but:
>  - In practice, container logger and webapp logger serve rather
> different purposes, as container logs won't be active but at
> initialization or destruction.
>  - They're not formatted the same way.
>  - It's not so cumbersome to watch several log files at once (I mean, on
> a real O.S. with a proper command line, of course) when you need to see
> both at the same time.
>  - It's much easier to configure a per-webapp logging (and consequently
> to ease webapp deployment).
>  - Gathering logs is always doable at the container level, which can
> also use slf4j (or one of its many bridges), bypassing ServletContext
> (which is just a proxy).
> 
> Anyhow, even it's a little drawback for some people, it can not weight
> as much in the balance as being freed of all logging
> 
> 2) We can -and should- keep our logger injection mechanism, since it not
> only helps separating different instances logs, but also permits
> unifying log line prefixes. No more need to define the logger in
> velocity.properties, by default it will be a static logger named
> "Velocity". The only constraint when trying to separate instances logs
> is to explicitly give different names to each logger (since the
> underlying LoggerFactory mechanism remains static). Not a big deal.
> 
> Without any other objection, I'll start this drastic simplification very
> soon. We don't even need to keep the o.a.v.runtime.log package. And four
> sub-modules can disappear...
> 
>   Claude
> 
> 
> On 21/08/2015 17:25, Nathan Bubna wrote:
>> I like simple. :)
>>
>> On Fri, Aug 21, 2015 at 2:18 AM, Claude Brisson<cla...@renegat.net> 
>> wrote:
>>
>>> 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
>>>
>>>
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
> For additional commands, e-mail: dev-h...@velocity.apache.org
> 


-- 
Sergiu Dumitriu
http://purl.org/net/sergiu/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org

Reply via email to