You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@turbine.apache.org by Ex...@nokia.com on 2001/03/29 10:43:53 UTC

RE: cvs commit: jakarta-turbine/src/java/org/apache/turbine/servi ces/template...

Jason, really nice work, this is going to be great! I found only two defects
;-):

1) TR already contains a property specifying TurbineVelocityService
   as a TemplateEngineService but it doesn't yet implement the interface
   although the method already is there. I'm not sure whether 
   VelocityService interface should extend the TemplateEngineService 
   interface or TurbineVelocityService implement it separately.
2) TurbineTemplateService doesn't "realize" template paths with
   ServletService (see below), which makes the default search to fail
   (Velocity search works when 1) is fixed).

A general performance problem is that although found layouts are cached and
found quickly, the results of failed searches are not. This means that
screen specific layouts are searched for every time a new screen is loaded
or a cached one reloaded, even though in 99 % of cases the default layout is
the one to use (for some reason the mechanism seems to do the search three
times for every screen request, I try to check this). Maybe the screen cache
of TemplateService could be extended to cache a layout reference also.

A minor comment is that we should decide whether properties specifying
templates should have a '/' prefix in template names or not. Now it happens
quite often that generated URLs have double slashes or alternatively
templates like 'layoutsDefault.vm' can't be found. Code should be capable of
handling both cases correctly but string manipulation could be optimized for
the default. My suggestion for the default is to drop off the slash as it's
not a real part of template identication.

-- Ilkka


> -----Original Message-----
> From: ext jvanzyl@apache.org [mailto:jvanzyl@apache.org]
> Sent: 29. March 2001 1:23
> To: jakarta-turbine-cvs@apache.org
> Subject: cvs commit:
> jakarta-turbine/src/java/org/apache/turbine/services/template
> TurbineTemplateService.java TurbineTemplateService2.java
> 
> 
> jvanzyl     01/03/28 14:22:47
> 
>   Modified:    src/java/org/apache/turbine/services/template
>                         TurbineTemplateService.java
>   Removed:     src/java/org/apache/turbine/services/template
>                         TurbineTemplateService2.java
>   Log:
>   - updating the template service so that the searching for templates
>     can be handed off to the template engine. right now this only
>     works for velocity and all the other template services work
>     the way they previously did.
>   
>     for velocity multiple template paths are now supported as well
>     as loading from the classpath and jar files. for velocity i still
>     have not implemented the standard searching algorithm 
> over multiple
>     template paths but i figure this isn't a dire need as multiple
>     template paths were not supported before. i need to 
> change velocity
>     in order for this to work and i will probably wait until after
>     the 1.0 velocity release.
>   
>   Revision  Changes    Path
>   1.20      +343 -187  
> jakarta-turbine/src/java/org/apache/turbine/services/template/
> TurbineTemplateService.java
>   
>   Index: TurbineTemplateService.java
>   ===================================================================
>   RCS file: 
> /home/cvs/jakarta-turbine/src/java/org/apache/turbine/services
> /template/TurbineTemplateService.java,v
>   retrieving revision 1.19
>   retrieving revision 1.20
>   diff -u -r1.19 -r1.20
>   --- TurbineTemplateService.java	2001/03/15 20:09:09	1.19
>   +++ TurbineTemplateService.java	2001/03/28 22:22:46	1.20
>   @@ -54,26 +54,26 @@
>     * <http://www.apache.org/>.
>     */

******** deleted **********

>   -        // relative to the webapp root directory
>   -        String templatePaths = props
>   -            .getProperty("template.path", "/templates");
>   -
>   -        // If possible, transform paths to be webapp root relative.
>   -        templatePaths = TurbineServlet.getRealPath(templatePaths);

******** deleted ***********

>   +        templateRoot = config.getStringArray("template.path");

******** deleted ***********

---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org


Re: cvs commit: jakarta-turbine/src/java/org/apache/turbine/services/template...

Posted by Jason van Zyl <jv...@apache.org>.
Ilkka Priha wrote:
> 
> > -----Original Message-----
> > From: jvanzyl [mailto:jvanzyl]On Behalf Of Jason van Zyl
> > Sent: Thursday, March 29, 2001 16:44
> > To: turbine-dev@jakarta.apache.org
> > Subject: Re: cvs commit:
> > jakarta-turbine/src/java/org/apache/turbine/services/template...
> >
> 
> **** deleted ****
> 
> > > 2) TurbineTemplateService doesn't "realize" template paths with
> > >    ServletService (see below), which makes the default search to fail
> > >    (Velocity search works when 1) is fixed).
> >
> > Do you mean the TemplateService doesn't realize the paths of the
> > TemplateEngineService? I'm not sure where the ServletService is
> > coming from.
> 
> Sorry, I tried to be too clever with a foreign language. The template paths
> from the property "template.path" are not translated to the web-app context
> with ServletService.getRealPath() in the new version. So the search
> performed by the TemplateService itself, if an external template engine
> service hasn't been defined, will fail.

Oh, damn. Right, I will fix that immediately. My bad bad bad.

-- 
jvz.

Jason van Zyl
jvanzyl@apache.org

http://jakarta.apache.org/velocity
http://jakarta.apache.org/turbine
http://tambora.zenplex.org

---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org


RE: cvs commit: jakarta-turbine/src/java/org/apache/turbine/services/template...

Posted by Ilkka Priha <ip...@surfeu.fi>.
> -----Original Message-----
> From: jvanzyl [mailto:jvanzyl]On Behalf Of Jason van Zyl
> Sent: Thursday, March 29, 2001 16:44
> To: turbine-dev@jakarta.apache.org
> Subject: Re: cvs commit:
> jakarta-turbine/src/java/org/apache/turbine/services/template...
>

**** deleted ****

> > 2) TurbineTemplateService doesn't "realize" template paths with
> >    ServletService (see below), which makes the default search to fail
> >    (Velocity search works when 1) is fixed).
>
> Do you mean the TemplateService doesn't realize the paths of the
> TemplateEngineService? I'm not sure where the ServletService is
> coming from.

Sorry, I tried to be too clever with a foreign language. The template paths
from the property "template.path" are not translated to the web-app context
with ServletService.getRealPath() in the new version. So the search
performed by the TemplateService itself, if an external template engine
service hasn't been defined, will fail.

-- Ilkka


---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org


Re: cvs commit: jakarta-turbine/src/java/org/apache/turbine/services/template...

Posted by Jason van Zyl <jv...@apache.org>.
Ext-Ilkka.Priha@nokia.com wrote:
> 
> Jason, really nice work, this is going to be great! I found only two defects
> ;-):
> 
> 1) TR already contains a property specifying TurbineVelocityService
>    as a TemplateEngineService but it doesn't yet implement the interface
>    although the method already is there. I'm not sure whether
>    VelocityService interface should extend the TemplateEngineService
>    interface or TurbineVelocityService implement it separately.

Sorry about that I had the code an didn't commit it, for now the 
TurbineVelocityService implements the TemplateEngineService. That's
in now.

> 2) TurbineTemplateService doesn't "realize" template paths with
>    ServletService (see below), which makes the default search to fail
>    (Velocity search works when 1) is fixed).

Do you mean the TemplateService doesn't realize the paths of the
TemplateEngineService? I'm not sure where the ServletService is
coming from.
 
> A general performance problem is that although found layouts are cached and
> found quickly, the results of failed searches are not. This means that
> screen specific layouts are searched for every time a new screen is loaded
> or a cached one reloaded, even though in 99 % of cases the default layout is
> the one to use (for some reason the mechanism seems to do the search three
> times for every screen request, I try to check this). Maybe the screen cache
> of TemplateService could be extended to cache a layout reference also.

Yes, that should definitely be fixed. I noticed the multiple searching,
and I'm not sure if that's always been there because I didn't notice it
until after the changes I made so I may well have introduced that
undesirable
behaviour. I don't think it will be hard to weed out.
 
> A minor comment is that we should decide whether properties specifying
> templates should have a '/' prefix in template names or not. Now it happens
> quite often that generated URLs have double slashes or alternatively
> templates like 'layoutsDefault.vm' can't be found. Code should be capable of
> handling both cases correctly but string manipulation could be optimized for
> the default. My suggestion for the default is to drop off the slash as it's
> not a real part of template identication.

I'm +1 for that.
 
> -- Ilkka

-- 
jvz.

Jason van Zyl
jvanzyl@apache.org

http://jakarta.apache.org/velocity
http://jakarta.apache.org/turbine
http://tambora.zenplex.org

---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org