Title: RE: [PATCH] ResourceManager.java and VelocityServlet.java

Christoph Reck [mailto:[EMAIL PROTECTED]]
>
> Two patches below:
> 1. ResourceManager to return a null template if the file was not
>    found (this is what the comment some lines above states it
>    would do; I tested that this is the correct behaviour -
>    otherwise it would emit wrong error doing template.parse() ).

Seems right.

> 2. VelocityServlet to provide a hook for subclasses to be able to
>    setup (a layered) context.
>    I also believe that VelocityServlet should not be abstract any
>    more to allow something like the JSP examples which operate
>    directly on the request and response objects.

Hm.  Not sure I am overjoyed about the change to handleRequest(), but I can't really think much about it until tonight when I get home.

>
> These patches are fully backward compatible.

Famous last words ;)
 
> Next things I'm going to do:
> A. Fix the log messages to reflect the correct template name
>    for errors within #parsed files. Also fix the parse depth
>    bug.
>    I will use a stack for the template names in the
>    LocalContextAdapter.
> B. Fix FileResouceLoader and Logfile to allow files relative
>    to the servlet WEBAPP (when the properties entries do not
>    start with a /). If no servlet context has been setup,
>    then it will continue using the current pwd.

Jason and I talked about that a while ago, and it's not clear that the FileResourceLoader is the way to go there, as it's something that can and should be handled at application level, as using Velocity in a servlet in a servlet runner is an application thing.

In the interest of full disclosure :), I wanted to make a 'wrapper loader' that would use FileResourceLoader, but prefix as you suggest above (and to not do it conditionally :) ), but on further consideration, and having it pointed out that Turbine does this internally just fine, I dropped the idea.  The less app-specific support cruft in Velocity, the better, I think.

For example, that might be something to add to VelocityServlet, incorporated in the init(), which is how I assume turbine does it.

> C. Submit an ApplicationServlet and context tools that
>    allows creating an MVC servlet based on VTL. This is already
>    working greatly for me (I moved my whole application from
>    Trubine to Velocity! - since Turbine dev is yet to dynamic
>    and, AFAICS, Vel is reaching a state where it can be released).

Sounds good, but I am 100% -1 on adding 'too many' special purpose context tools to the vel core distribution for a whole bunch of reasons.  I think we should talk more about making a place for unsupported-and-dont-come-with-the-core-but-the-community-actively-supports toolsets like this.

I will put in the ResourceManager patch tonight if there are no objections, but want a chance to look at the servlet stuff with clear focus.

Good job.

geir

> Patch for ResourceManager.java
> --------------------------------------------------------------
> ------------
> ---
> Apache-Velocity-20010131/src/java/org/apache/velocity/runtime/
> resource/ResourceManager.java Wed Dec 20 00:57:20 2000
> +++
> Apache-Velocity-20010126/src/java/org/apache/velocity/runtime/
> resource/ResourceManager.java Fri Jan 26 16:44:06 2001
> @@ -273,6 +273,7 @@
>              catch (Exception e)
>              {
>                  Runtime.error(e);
> +                resource = null; // return null
>              }
>          }
>          return resource;
> --------------------------------------------------------------
> ------------
>
>
> Patch for VelocityServlet.java
> --------------------------------------------------------------
> ------------
> ---
> Apache-Velocity-20010131/src/java/org/apache/velocity/servlet/
> VelocityServlet.java  Wed Jan  3 06:28:51 2001
> +++
> Apache-Velocity-20010126/src/java/org/apache/velocity/servlet/
> VelocityServlet.java  Thu Jan 25 17:26:12 2001
> @@ -59,6 +59,7 @@
>  import java.io.Writer;
>  import java.io.BufferedWriter;
>  import java.io.OutputStreamWriter;
> +import java.io.OutputStream;

>  import java.util.Stack;

> @@ -80,7 +81,8 @@

>  /**
>   * Base class which simplifies the use of Velocity with Servlets.
> - * Extend this class, implement the
> <code>handleRequest()</code> method,
> + * Extend this class, optionally implement the
> <code>prepareContext()</code>
> + * method, implement the <code>handleRequest()</code> method,
>   * and add your data to the context.  Then call
>   * <code>getTemplate("myTemplate.wm")</code>.
>   *
> @@ -91,8 +93,8 @@
>   * "res" - The HttpServletResponse object
>   * </pre>
>   *
> - * If you put a contentType object into the context within
> either your
> - * serlvet or within your template, then that will be used
> to override
> + * If you put a contentType object into the context within the
> + * <code>prepareContext()</code>, then that will be used to override
>   * the default content type specified in the properties file.
>   *
>   * "contentType" - The value for the Content-Type: header
> @@ -102,7 +104,7 @@
>   * @author <a href="mailto:[EMAIL PROTECTED]">Geir
> Magnusson Jr.</a>
>   * $Id: VelocityServlet.java,v 1.20 2001/01/03 05:28:51 geirm Exp $
>   */
> -public abstract class VelocityServlet extends HttpServlet
> +public class VelocityServlet extends HttpServlet
>  {
>      /**
>       * The HTTP request object context key.
> @@ -138,7 +140,6 @@
>      /**
>       * Cache of writers
>       */
> -  
>      private static SimplePool writerPool = new SimplePool(40);
>    
>      /**
> @@ -208,6 +209,23 @@
>      }

>      /**
> +     * Prepare and process the request (this method may be
> +     * overridden by subclasses).
> +     */
> +    protected Context prepareContext( HttpServletRequest request,
> +                                      HttpServletResponse response )
> +    {
> +        // prepare the local context
> +        VelocityContext context = new VelocityContext();
> +
> +        // put the request/response objects into the context
> +        context.put(REQUEST, request);
> +        context.put(RESPONSE, response);
> +
> +        return context;
> +    }
> +
> +    /**
>       * Process the request.
>       */
>      private void doRequest(HttpServletRequest request,
> @@ -216,16 +234,11 @@
>      {
>          ServletOutputStream output = response.getOutputStream();
>          String contentType = null;
> -        VelocityWriter vw = null;
>         
>          try
>          {
> -            // create a new context
> -            VelocityContext context = new VelocityContext();
> -           
> -            // put the request/response objects into the context
> -            context.put (REQUEST, request);
> -            context.put (RESPONSE, response);
> +            // allow subclasses do perform different setups
> +            Context context = prepareContext(request, response);

>               // check for a content type in the context   
>              if (context.containsKey(CONTENT_TYPE))
> @@ -245,7 +258,41 @@
>              if ( template == null )
>                  throw new Exception ("Cannot find the template!" );
>             
> +            mergeTemplate( template, context, output );
> +        }
> +        catch (Exception e)
> +        {
> +            // display error messages
> +            error( output, e.getMessage() );
> +        }
> +        finally
> +        {
> +            try
> +            {
> +                output.close();
> +            }
> +            catch(Exception e)
> +            {
> +                // do nothing
> +            }
> +        }
> +    }
> +
> +    /**
> +     * Merge a template with the context values to the output.
> +     * This may be used in subclasses to ingest templates (e.g. to
> +     * populate the context).<p>
> +     * The closing of the OutputStream must be done by the caller.
> +     */
> +    protected void mergeTemplate( Template template,
> +                                  Context context,
> +                                  OutputStream output )
> +         throws ServletException, IOException, Exception
> +    {
> +        VelocityWriter vw = null;
>           
> +        try
> +        {
>              vw = (VelocityWriter) writerPool.get();
>           
>              if (vw == null)
> @@ -256,11 +303,6 @@
>            
>              template.merge( context, vw);
>          }
> -        catch (Exception e)
> -        {
> -            // display error messages
> -            error (output, e.getMessage());
> -        }
>          finally
>          {
>              try
> @@ -269,7 +311,6 @@
>                  {
>                      vw.flush();
>                      writerPool.put(vw);
> -                    output.close();
>                  }               
>              }
>              catch (Exception e)
> @@ -297,10 +338,16 @@
>       * calling the <code>getTemplate()</code> method to
> produce your return
>       * value.
>       *
> -     * @param ctx The context to add your data to.
> +     * @param context The context to add your data to.
>       * @return    The template to merge with your context.
>       */
> -    protected abstract Template handleRequest( Context ctx );
> +    protected Template handleRequest( Context context )
> throws Exception
> +    {
> +        String templateName =
> +            ((HttpServletRequest)
> context.get(REQUEST)).getPathInfo();
> +
> +        return getTemplate(templateName);
> +    }
>  
>      /**
>       * Send an error message to the client.
> @@ -318,4 +365,3 @@
>          out.print( html.toString() );
>      }
>  }
> -
> --------------------------------------------------------------
> ------------
>
> :) Christoph
>

Reply via email to