I've been looking at the classes Renderer and DirectRenderer and I have
a two general concerns.
The first is that Renderer seems to define many methods which are not
used. The role of renderer is to help Valves render templates, however
it defines a number of unused methods which are outside of that
responsibility. For clarity I think it would be good to eliminate these
methods.
Second, I feel it is cumbersome to be required to include the rundata on
each method call. Since a renderer is linked to a request it can easily
maintain a reference to its rundata. This allows simpler calls in
templates.
I have included a patch with this method that contains the changes I am
suggesting. It passes the test suite using both the classic and direct
pipelines, keeping in mind that the patch it contains changes to the
default layout in the test.
Thanks,
James Taylor
PS: This is my first post to the list, so howdy!
? cactus_server.log
? target
Index: src/java/org/apache/turbine/pipeline/DefaultTargetValve.java
===================================================================
RCS file:
/home/cvspublic/jakarta-turbine-3/src/java/org/apache/turbine/pipeline/DefaultTargetValve.java,v
retrieving revision 1.6
diff -u -r1.6 DefaultTargetValve.java
--- src/java/org/apache/turbine/pipeline/DefaultTargetValve.java 18 Jan 2002
15:53:42 -0000 1.6
+++ src/java/org/apache/turbine/pipeline/DefaultTargetValve.java 19 Jan 2002
+02:21:26 -0000
@@ -170,15 +170,15 @@
log.debug( "Rendering target " + target );
}
- Renderer r = new Renderer();
+ Renderer r = new Renderer( data );
- // FIXME: Can we remove some hardcoding here?
+ // FIXME: Can we remove hardcoding here?
- context.put("renderer", r);
+ context.put( "renderer", r );
// Render the target
- String out = r.render(data, target);
+ String out = r.render( target );
// Write the composed string to the response
Index: src/java/org/apache/turbine/pipeline/DirectRenderer.java
===================================================================
RCS file:
/home/cvspublic/jakarta-turbine-3/src/java/org/apache/turbine/pipeline/DirectRenderer.java,v
retrieving revision 1.4
diff -u -r1.4 DirectRenderer.java
--- src/java/org/apache/turbine/pipeline/DirectRenderer.java 18 Jan 2002 23:21:38
-0000 1.4
+++ src/java/org/apache/turbine/pipeline/DirectRenderer.java 19 Jan 2002 02:21:26
+-0000
@@ -63,7 +63,8 @@
import org.apache.log4j.Category;
/**
- * This class is used in the context to render templates.
+ * Helper class for rendering templates. Unlike its parent, it renders directly
+ * to the Writer associated with its RunData.
*
* @author <a href="mailto:[EMAIL PROTECTED]";>Jason van Zyl</a>
* @author <a href="mailto:[EMAIL PROTECTED]";>Jon S. Stevens</a>
@@ -79,47 +80,24 @@
private static final String EMPTY = "";
- private Writer writer;
-
- public DirectRenderer(Writer writer)
+ public DirectRenderer( RunData data )
{
- this.writer = writer;
+ super( data );
}
/**
- * @see org.apache.turbine.pipeline.Renderer#render(TemplateContext,String)
- * @return zero length String
- */
- public String render(TemplateContext context, String target)
- throws TurbineException,Exception
- {
- try
- {
- log.debug("Rendering target " + target);
- Module.handleRequest(context, target, writer);
- }
- catch (Exception e)
- {
- log.error("Rendering failed", e);
- Module.handleRequest(context, "screens/Error.vm", writer);
- }
-
- return EMPTY;
- }
-
- /**
- * @see org.apache.turbine.pipeline.Renderer#render(RunData,String)
+ * @see org.apache.turbine.pipeline.Renderer#render( String )
* @return zero length String
*/
- public String render(RunData data, String target)
+ public String render( String template )
throws TurbineException, Exception
{
- log.debug("Rendering target " + target);
+ log.debug( "Rendering template " + template );
- TemplateContext context = Module.getTemplateContext(data);
-
- Module.handleRequest(context, target, writer);
+ TemplateContext context = Module.getTemplateContext( data );
+ Module.handleRequest( context, template, data.getOut() );
+
return EMPTY;
}
}
Index: src/java/org/apache/turbine/pipeline/DirectTargetValve.java
===================================================================
RCS file:
/home/cvspublic/jakarta-turbine-3/src/java/org/apache/turbine/pipeline/DirectTargetValve.java,v
retrieving revision 1.3
diff -u -r1.3 DirectTargetValve.java
--- src/java/org/apache/turbine/pipeline/DirectTargetValve.java 18 Jan 2002 15:53:42
-0000 1.3
+++ src/java/org/apache/turbine/pipeline/DirectTargetValve.java 19 Jan 2002 02:21:26
+-0000
@@ -93,15 +93,15 @@
log.debug( "Rendering target " + target );
}
- Renderer r = new DirectRenderer( data.getOut() );
+ Renderer r = new DirectRenderer( data );
- // FIXME: Can we remove some hardcoding here?
+ // FIXME: Can we remove hardcoding here?
- context.put("renderer", r);
+ context.put( "renderer", r );
// Render the target directly to the response
- r.render( data, target );
+ r.render( target );
}
}
Index: src/java/org/apache/turbine/pipeline/Renderer.java
===================================================================
RCS file:
/home/cvspublic/jakarta-turbine-3/src/java/org/apache/turbine/pipeline/Renderer.java,v
retrieving revision 1.6
diff -u -r1.6 Renderer.java
--- src/java/org/apache/turbine/pipeline/Renderer.java 18 Jan 2002 03:33:58 -0000
1.6
+++ src/java/org/apache/turbine/pipeline/Renderer.java 19 Jan 2002 02:21:27 -0000
@@ -63,85 +63,83 @@
import org.apache.log4j.Category;
/**
- * This class is used in the context to render templates.
+ * Helper class for rendering templates. This is used by valves to render
+ * templates. It can also be placed on the context to allow templates to render
+ * other templates.
+ *
+ * Renderers are not thread safe, they must be constructed for and used by a
+ * single RunData. In the future this class is a candidate for being made thread
+ * safe or poolable.
*
* @author <a href="mailto:[EMAIL PROTECTED]";>Jason van Zyl</a>
* @author <a href="mailto:[EMAIL PROTECTED]";>Jon S. Stevens</a>
* @author <a href="mailto:[EMAIL PROTECTED]";>Mike Haberman</a>
+ * @author <a href="mailto:[EMAIL PROTECTED]";>James Taylor</a>
* @version $Id: Renderer.java,v 1.6 2002/01/18 03:33:58 jvanzyl Exp $
*/
public class Renderer
{
private static final Category log =
Category.getInstance( Renderer.class );
-
- public String render(TemplateContext context, String target)
- throws TurbineException, Exception
- {
- try
- {
- log.debug("Rendering target " + target);
- return Module.handleRequest(context, target);
- }
- catch (Exception e)
- {
- log.error("Rendering failed", e);
- return Module.handleRequest(context, "screens/Error.vm");
- }
- }
/**
- * use the resolver to find the template
+ * RunData of the request this Renderer is for.
+ */
+ protected RunData data = null;
+
+ /**
+ * Construct a renderer for the given RunData.
*/
- public String render(String moduleType, RunData data, String template)
- throws TurbineException,Exception
+ public Renderer( RunData data )
{
- String target = Turbine.getResolver().getTemplate(moduleType, template);
- return render(data, target);
+ this.data = data;
}
/**
- * This method is called in the ClassicPipeline because it doesn't
- * catch exceptions when attempting to render the template. That way,
- * exceptions can be caught way up in the Turbine class and the
- * error page can be dealt with properly without a lot of extra code
- * here.
+ * Render the given template. TemplateContext to use will be extracted from
+ * the RunData with which this Renderer was constructer.
+ *
+ * @param template name/path of template in the format expected by the
+ * appropriate template engine service
+ * @return Result of template merge
*/
- public String render(RunData data, String target)
+ public String render( String template )
throws TurbineException, Exception
{
- log.debug("Rendering target " + target);
- TemplateContext context = Module.getTemplateContext(data);
- return Module.handleRequest(context, target);
+ log.debug( "Rendering template " + template );
+
+ TemplateContext context = Module.getTemplateContext( data );
+
+ return Module.handleRequest( context, template );
}
- public String render(String moduleType, TemplateContext context,
- String template)
+ /**
+ * Use the resolver to find the template, then render.
+ */
+ public String render( String type, String target )
throws TurbineException, Exception
{
- String target = Turbine.getResolver().getTemplate(moduleType, template);
- return render(context, target);
+ String template = Turbine.getResolver().getTemplate( type, target );
+
+ return render( template );
}
/**
- * Attemps to locate the template and if it can't find it, then
- * it will attempt to render the defaultTemplate
+ * Attemps to locate the template for target and render, if it can't find
+ * it, then it will attempt to render the template for defaultTarget
*/
- public String render(String moduleType, RunData data,
- String template, String defaultTemplate)
- throws TurbineException,Exception
+ public String render( String type, String target, String defaultTarget )
+ throws TurbineException, Exception
{
- String target = Turbine.getResolver().getTemplate(moduleType, template);
+ String template = Turbine.getResolver().getTemplate( type, target );
- String output = null;
- if (Module.templateExists(target))
+ if ( Module.templateExists( template ) )
{
- output = render(data, target);
+ return render( template );
}
else
{
- output = render(moduleType, data, defaultTemplate);
+ return render( type, defaultTarget );
}
- return output;
}
}
Index: src/rttest/testapp/templates/app/layouts/Default.vm
===================================================================
RCS file:
/home/cvspublic/jakarta-turbine-3/src/rttest/testapp/templates/app/layouts/Default.vm,v
retrieving revision 1.2
diff -u -r1.2 Default.vm
--- src/rttest/testapp/templates/app/layouts/Default.vm 18 Jan 2002 15:53:42 -0000
1.2
+++ src/rttest/testapp/templates/app/layouts/Default.vm 19 Jan 2002 02:21:27 -0000
@@ -10,17 +10,17 @@
<table id=layout width="100%">
<tr>
<td id=topNav>
- $renderer.render("navigations", $data, "/DefaultTop.vm")
+ $renderer.render("navigations", "/DefaultTop.vm")
</td>
</tr>
<tr>
<td align="left" valign="top" id=screen>
- $renderer.render("screens", $data, $template)
+ $renderer.render("screens", $template)
</td>
</tr>
<tr>
<td id=bottomNav>
- $renderer.render("navigations", $data, "/DefaultBottom.vm")
+ $renderer.render("navigations", "/DefaultBottom.vm")
</td>
</tr>
</table>
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
