Velocity Tools
  1. Velocity Tools
  2. VELTOOLS-68

VelocityViewServlet has hard coded ServletLogger

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.3
    • Component/s: VelocityView
    • Labels:
      None

      Description

      VelocityViewServlet configures the Velocity engine to use the ServletLogger (~line 323). Integration of the servlet into applications that use other logging systems (e.g. log4j / commons-logging) is very hard because of that.

      • VVS should honor the logging setttings in velocity.properties (e.g. for explicit jdk logging)
      • It should add the ServletLogger to the log engine search path and only use it if no other logger was found (log4j / avalon etc.)
      • The ServletLogger class implements the deprecated LogSystem. VelocityTools should provide a LogChute implementation of the SrevletLogger.
      1. velocity-tools.patch
        7 kB
        Henning Schmiedehausen

        Activity

        Hide
        Nathan Bubna added a comment -

        Integration w/commons-logging used to be trivial until Tomcat decided to use c-l as a full logging provider instead of a wrapper.

        Adding the ServletLogger to the search path sounds good, however, there is the question of where in the priority it should be. i don't necessarily think it should be the last option.

        i do have LogChute implementations for VelocityTools on the wiki (http://wiki.apache.org/jakarta-velocity/VelocityTools), but i've held off adding them to Veltools 1.3-dev because i want to be free to release 1.3 before Velocity 1.5 comes out. If we get Vel 1.5 out before i roll Veltools 1.3, then i'll add the LogChutes in.

        Show
        Nathan Bubna added a comment - Integration w/commons-logging used to be trivial until Tomcat decided to use c-l as a full logging provider instead of a wrapper. Adding the ServletLogger to the search path sounds good, however, there is the question of where in the priority it should be. i don't necessarily think it should be the last option. i do have LogChute implementations for VelocityTools on the wiki ( http://wiki.apache.org/jakarta-velocity/VelocityTools ), but i've held off adding them to Veltools 1.3-dev because i want to be free to release 1.3 before Velocity 1.5 comes out. If we get Vel 1.5 out before i roll Veltools 1.3, then i'll add the LogChutes in.
        Hide
        Henning Schmiedehausen added a comment -

        Ah yes, I forgot about backwards compatibility again. So your point with LogSystem is surely valid.

        About the priority: Well as the current is Avalon, Log4j, JDK, I'd probably say that making it configurable for the user and adding a default of ServletLogging (which should always be available in a servlet environment... ) might be fine.

        Show
        Henning Schmiedehausen added a comment - Ah yes, I forgot about backwards compatibility again. So your point with LogSystem is surely valid. About the priority: Well as the current is Avalon, Log4j, JDK, I'd probably say that making it configurable for the user and adding a default of ServletLogging (which should always be available in a servlet environment... ) might be fine.
        Hide
        Henning Schmiedehausen added a comment -

        How about the attached patch? It factors out the property setting by the servlet to work similar to the engine itself (load a properties file from the class path). This explicitly shows what gets set automagically by the servlet and gives useful hints to users that e.g. want to turn on caching for the webapp loader in the application velocity.properties...

        (webapp.resource.loader.cache = true)

        The patch also cleans up a number of minor nits, e.g. the usage of getServletContext().log() instead of log() directly...

        Show
        Henning Schmiedehausen added a comment - How about the attached patch? It factors out the property setting by the servlet to work similar to the engine itself (load a properties file from the class path). This explicitly shows what gets set automagically by the servlet and gives useful hints to users that e.g. want to turn on caching for the webapp loader in the application velocity.properties... (webapp.resource.loader.cache = true) The patch also cleans up a number of minor nits, e.g. the usage of getServletContext().log() instead of log() directly...
        Hide
        Nathan Bubna added a comment -

        changes made in rev 473430

        Show
        Nathan Bubna added a comment - changes made in rev 473430
        Hide
        Claude Brisson added a comment -

        I don't know exactly why, but this patch breaks velocity 1.4 compatibility: Velocity 1.4 will search for Avalon and fail with:

        java.lang.NoClassDefFoundError: org/apache/log/format/Formatter
        at org.apache.velocity.runtime.log.LogManager.createLogSystem(LogManager.java:162)
        at org.apache.velocity.runtime.RuntimeInstance.initializeLogger(RuntimeInstance.java:553)
        at org.apache.velocity.runtime.RuntimeInstance.init(RuntimeInstance.java:226)
        at org.apache.velocity.app.VelocityEngine.init(VelocityEngine.java:80)
        at org.apache.velocity.tools.view.servlet.VelocityViewServlet.initVelocity(Unknown Source)
        at org.apache.velocity.tools.view.servlet.VelocityViewServlet.init(Unknown Source)
        ...

        And it will throw this exception even when velocity.properties contains:

        1. VelocityViewServlet uses the WebappLoader.
          resource.loader = webapp
          webapp.resource.loader.class = org.apache.velocity.tools.view.servlet.WebappLoader
          webapp.resource.loader.path = /
          webapp.resource.loader.extension = vtl

        Everything works fine with velocity 1.5-dev.

        Show
        Claude Brisson added a comment - I don't know exactly why, but this patch breaks velocity 1.4 compatibility: Velocity 1.4 will search for Avalon and fail with: java.lang.NoClassDefFoundError: org/apache/log/format/Formatter at org.apache.velocity.runtime.log.LogManager.createLogSystem(LogManager.java:162) at org.apache.velocity.runtime.RuntimeInstance.initializeLogger(RuntimeInstance.java:553) at org.apache.velocity.runtime.RuntimeInstance.init(RuntimeInstance.java:226) at org.apache.velocity.app.VelocityEngine.init(VelocityEngine.java:80) at org.apache.velocity.tools.view.servlet.VelocityViewServlet.initVelocity(Unknown Source) at org.apache.velocity.tools.view.servlet.VelocityViewServlet.init(Unknown Source) ... And it will throw this exception even when velocity.properties contains: VelocityViewServlet uses the WebappLoader. resource.loader = webapp webapp.resource.loader.class = org.apache.velocity.tools.view.servlet.WebappLoader webapp.resource.loader.path = / webapp.resource.loader.extension = vtl Everything works fine with velocity 1.5-dev.
        Hide
        Claude Brisson added a comment -

        hummm... ignore the end of my previous comment... it should be:

        The exception will show up when velocity.properties does not contain

        runtime.log.logsystem.class = org.apache.velocity.tools.view.servlet.ServletLogger

        So it looks like the VVS fails setting up the ServletLogger if nothing else is present in the path under velocity 1.4.

        Show
        Claude Brisson added a comment - hummm... ignore the end of my previous comment... it should be: The exception will show up when velocity.properties does not contain runtime.log.logsystem.class = org.apache.velocity.tools.view.servlet.ServletLogger So it looks like the VVS fails setting up the ServletLogger if nothing else is present in the path under velocity 1.4.
        Hide
        Nathan Bubna added a comment -

        Sorry about that. I forgot to check in the updated build.xml for this.

        Show
        Nathan Bubna added a comment - Sorry about that. I forgot to check in the updated build.xml for this.

          People

          • Assignee:
            Unassigned
            Reporter:
            Henning Schmiedehausen
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development