FileTemplateLoader doesnt close reader ?

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

FileTemplateLoader doesnt close reader ?

Albert Kam
I took a peek at FileTemplateLoader.java and notices that the closeTemplateSource() is empty method.
Is it okay not to close the Reader returned from getReader() ?

I am asking this because i am trying to mimic the best practice when creating a custom TemplateLoader.

In my case, when following the instruction to auto-escape in here : 

- I found out that closing the reader returns an empty page after the process. Commenting the IOUtils.closeQuietly() solves the empty page problem.
- So i thought maybe i should close the reader (maybe through a ThreadLocal storing the reader) in the closeTemplateSource()
- So i peeked the FileTemplateLoader for an example, and thus this post to clarify.

Please share your thoughts.

Thank you,
Albert Kam

--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)

------------------------------------------------------------------------------
Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester  
Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the  
endpoint security space. For insight on selecting the right partner to
tackle endpoint security challenges, access the full report.
http://p.sf.net/sfu/symantec-dev2dev
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user
Reply | Threaded
Open this post in threaded view
|

Re: FileTemplateLoader doesnt close reader ?

Daniel Dekany
Tuesday, March 12, 2013, 8:20:18 AM, Albert Kam wrote:

> I took a peek at FileTemplateLoader.java and notices that the
> closeTemplateSource() is empty method.
> Is it okay not to close the Reader returned from getReader() ?

The Reader is closed by whoever has called getReader() by calling
java.io.Reader.close(), not with a TemplateLoader method. The
closeTemplateSource() just closes the "template source", which in the
case of FileTemplateLoader is a File, and hence there's nothing to
close in it.

> I am asking this because i am trying to mimic the best practice
> when creating a custom TemplateLoader.
>
> In my case, when following the instruction to auto-escape in here :
> http://watchitlater.com/blog/2011/10/default-html-escape-using-freemarker/ :
>
> - I found out that closing the reader returns an empty page after
> the process. Commenting the IOUtils.closeQuietly() solves the empty page problem.

That sounds impossible. By the time IOUtils.closeQuietly() is called,
the StringReader and the String it reads is already done. Maybe
IOUtils.closeQuietly() throws an exception that's improperly handled,
gibing an empty page?

> - So i thought maybe i should close the reader (maybe through a
> ThreadLocal storing the reader) in the closeTemplateSource()

You should not close the Reader that you return to the caller. But in
this case you have a Reader that you don't return to the caller or to
anybody, so of course you have to close it yourself, but that you
should do right where it's done on that blog.

> - So i peeked the FileTemplateLoader for an example, and thus this post to clarify.
>
> Please share your thoughts.

That implementation is not very good, because:

- It can't handle the case when the template already has an #ftl
  header

- That it pre-loads the template, instead of creating a wrapping
  Reader that appends and prepends what's needed right in the
  character stream. Although it's not a big deal in practice, it's not
  very nice.

You may look into the source code of FMPP, and there
Engine.wrapReader. (I was substantially younger when I wrote that, but
let's hope it's OK... :) )

> Thank you,
> Albert Kam

--
Best regards,
 Daniel Dekany


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user
Reply | Threaded
Open this post in threaded view
|

Re: FileTemplateLoader doesnt close reader ?

Albert Kam

The Reader is closed by whoever has called getReader() by calling
java.io.Reader.close(), not with a TemplateLoader method. The
closeTemplateSource() just closes the "template source", which in the
case of FileTemplateLoader is a File, and hence there's nothing to
close in it.

That sounds impossible. By the time IOUtils.closeQuietly() is called,
the StringReader and the String it reads is already done. Maybe
IOUtils.closeQuietly() throws an exception that's improperly handled,
gibing an empty page?

I tried closing the reader and catching and logging the exception instead of just using IOTools.closeQuietly(),
and notices that there's no exception happening, and still gave me the empty page.
Commenting the close reader section gave me back the output.


You should not close the Reader that you return to the caller. But in
this case you have a Reader that you don't return to the caller or to
anybody, so of course you have to close it yourself, but that you
should do right where it's done on that blog.

Aha, maybe that's the reason it gave me the empty output.
"Closes the stream and releases any system resources associated with it. 
Once the stream has been closed, further read(), ready(), mark(), or reset() invocations will throw an IOException. Closing a previously closed stream has no effect."
Maybe the caller of the closed-reader got an IOException somewhere and hence, the empty output ?
 

That implementation is not very good, because:

- It can't handle the case when the template already has an #ftl
  header

- That it pre-loads the template, instead of creating a wrapping
  Reader that appends and prepends what's needed right in the
  character stream. Although it's not a big deal in practice, it's not
  very nice.

You may look into the source code of FMPP, and there
Engine.wrapReader. (I was substantially younger when I wrote that, but
let's hope it's OK... :) )

I actually have switched back to the normal FileTemplateLoader without wrapping the escape directive for several reasons :

- I actually make use of functions for URLs generation.
  And it's ugly to see the noescape tags wrapping every link calls, some thing like :
  <#noescape>${url.home("Return to main page")}</#noescape>
  I prefer using ?html as needed

- Because i am using JBoss Tools for the freemarker editor (which does help me in parsing error detection, highlighting, etc), it errors the line of every noescape because it's not inside an escape directive, which gives me the tension i dont need.

 And thank you for the wrapReader sharing, i will add it to my library in case i find a use of it in the future.

Regards from Jakarta,
Albert Kam

--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user
Reply | Threaded
Open this post in threaded view
|

Re: FileTemplateLoader doesnt close reader ?

Daniel Dekany
Wednesday, March 13, 2013, 3:42:36 AM, Albert Kam wrote:

[snip]
> I tried closing the reader and catching and logging the exception
> instead of just using IOTools.closeQuietly(),
> and notices that there's no exception happening, and still gave me the empty page.
> Commenting the close reader section gave me back the output.

Are you returning a StringReader before calling
IOTools.closeQuietly()on *another* reader? Because then this
phenomenon it doesn't make any sense.

[snip]
> Aha, maybe that's the reason it gave me the empty output.
> According to
> http://docs.oracle.com/javase/7/docs/api/java/io/StringReader.html#close()
> "Closes the stream and releases any system resources associated with it.
> Once the stream has been closed, further read(), ready(), mark(),
> or reset() invocations will throw an IOException. Closing a
> previously closed stream has no effect."
> Maybe the caller of the closed-reader got an IOException somewhere and hence, the empty output ?

You aren't supposed to close the StringReader inside the
TemplateLoader. You should close the *other* reader, the one that you
don't return. Also, if FreeMarker gives empty output when an
IOException occurs, that's a bug. But I hope it doesn't do that, it's
rather the framework that calls it that suppresses that exception
instead of giving a HTTP 500.

[snip]
> I actually have switched back to the normal FileTemplateLoader
> without wrapping the escape directive for several reasons :
>
> - I actually make use of functions for URLs generation.
>   And it's ugly to see the noescape tags wrapping every link calls, some thing like :
>   <#noescape>${url.home("Return to main page")}</#noescape>
>   I prefer using ?html as needed

It's a strange coincidence that you say this just after 2 weeks or so
after I have proposed #p. It addresses exactly this annoyance, but I
don't remember anybody but myself ever brought this up, till now.
Anyway, it will be in 2.3.20 most probably.

> - Because i am using JBoss Tools for the freemarker editor (which
> does help me in parsing error detection, highlighting, etc), it
> errors the line of every noescape because it's not inside an escape
> directive, which gives me the tension i dont need.

This will also be solved by #p, since then you won't need #noescpe
anymore. Indeed, I might as well deprecate #noescape then.

>  And thank you for the wrapReader sharing, i will add it to my
> library in case i find a use of it in the future.
>
> Regards from Jakarta,
> Albert Kam

--
Best regards,
 Daniel Dekany


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user
Reply | Threaded
Open this post in threaded view
|

Re: FileTemplateLoader doesnt close reader ?

Albert Kam
> You aren't supposed to close the StringReader inside the
TemplateLoader.
Yeah, i returned a closed stringReader. What an omg-mistake !
Now i have changed it to close the 'wrapped' reader instead.
Thanks so much for the guides.



On Thu, Mar 14, 2013 at 3:06 AM, Daniel Dekany <[hidden email]> wrote:
Wednesday, March 13, 2013, 3:42:36 AM, Albert Kam wrote:

[snip]
> I tried closing the reader and catching and logging the exception
> instead of just using IOTools.closeQuietly(),
> and notices that there's no exception happening, and still gave me the empty page.
> Commenting the close reader section gave me back the output.

Are you returning a StringReader before calling
IOTools.closeQuietly()on *another* reader? Because then this
phenomenon it doesn't make any sense.

[snip]
> Aha, maybe that's the reason it gave me the empty output.
> According to
> http://docs.oracle.com/javase/7/docs/api/java/io/StringReader.html#close()
> "Closes the stream and releases any system resources associated with it.
> Once the stream has been closed, further read(), ready(), mark(),
> or reset() invocations will throw an IOException. Closing a
> previously closed stream has no effect."
> Maybe the caller of the closed-reader got an IOException somewhere and hence, the empty output ?

You aren't supposed to close the StringReader inside the
TemplateLoader. You should close the *other* reader, the one that you
don't return. Also, if FreeMarker gives empty output when an
IOException occurs, that's a bug. But I hope it doesn't do that, it's
rather the framework that calls it that suppresses that exception
instead of giving a HTTP 500.

[snip]
> I actually have switched back to the normal FileTemplateLoader
> without wrapping the escape directive for several reasons :
>
> - I actually make use of functions for URLs generation.
>   And it's ugly to see the noescape tags wrapping every link calls, some thing like :
>   <#noescape>${url.home("Return to main page")}</#noescape>
>   I prefer using ?html as needed

It's a strange coincidence that you say this just after 2 weeks or so
after I have proposed #p. It addresses exactly this annoyance, but I
don't remember anybody but myself ever brought this up, till now.
Anyway, it will be in 2.3.20 most probably.

> - Because i am using JBoss Tools for the freemarker editor (which
> does help me in parsing error detection, highlighting, etc), it
> errors the line of every noescape because it's not inside an escape
> directive, which gives me the tension i dont need.

This will also be solved by #p, since then you won't need #noescpe
anymore. Indeed, I might as well deprecate #noescape then.

>  And thank you for the wrapReader sharing, i will add it to my
> library in case i find a use of it in the future.
>
> Regards from Jakarta,
> Albert Kam

--
Best regards,
 Daniel Dekany


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user



--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user
Reply | Threaded
Open this post in threaded view
|

Re: FileTemplateLoader doesnt close reader ?

Albert Kam
Sorry almost forgot.

I think the #p solution would be a good addition to the arsenal of freemarker, 
if paired with the automatic escaping feature.

Will be excited for 2.3.20 updates, but for now, 
i'm content with manual escaping the expressions on user-generated-content.

(OOT) Wishful thinking :
From my short but intense experience of Freemarker, i can say that it feels natural and powerful to use.
I just love building reusable directives (even the recursive ones !)
I have decided to use it over jsp and jsf + apache tiles, both of which i have used before in previous projects.
I just wish there's a freemarker eclipse plugin that's much more functional in helping maintaining freemarker templates. 

The JBoss Tool's freemarker plugin is good, but i stil miss these features :
- ability to list the ftl-s that make use of a specific function / macro
- ability to refactor function / macro's name, parameter name
- detect function / macro usages error after changing the function / macro definition
- can operate based on the configuration passed, like showing the imported libraries in the template editor, or optionally show the settings
- can link current active template to another, and then it see what variables are available (those assigned from the container template)
- link a model to a concrete class to help support with autocompletion on methods, data type validations, etc
- etc







On Thu, Mar 14, 2013 at 9:36 AM, Albert Kam <[hidden email]> wrote:
> You aren't supposed to close the StringReader inside the
TemplateLoader.
Yeah, i returned a closed stringReader. What an omg-mistake !
Now i have changed it to close the 'wrapped' reader instead.
Thanks so much for the guides.



On Thu, Mar 14, 2013 at 3:06 AM, Daniel Dekany <[hidden email]> wrote:
Wednesday, March 13, 2013, 3:42:36 AM, Albert Kam wrote:

[snip]
> I tried closing the reader and catching and logging the exception
> instead of just using IOTools.closeQuietly(),
> and notices that there's no exception happening, and still gave me the empty page.
> Commenting the close reader section gave me back the output.

Are you returning a StringReader before calling
IOTools.closeQuietly()on *another* reader? Because then this
phenomenon it doesn't make any sense.

[snip]
> Aha, maybe that's the reason it gave me the empty output.
> According to
> http://docs.oracle.com/javase/7/docs/api/java/io/StringReader.html#close()
> "Closes the stream and releases any system resources associated with it.
> Once the stream has been closed, further read(), ready(), mark(),
> or reset() invocations will throw an IOException. Closing a
> previously closed stream has no effect."
> Maybe the caller of the closed-reader got an IOException somewhere and hence, the empty output ?

You aren't supposed to close the StringReader inside the
TemplateLoader. You should close the *other* reader, the one that you
don't return. Also, if FreeMarker gives empty output when an
IOException occurs, that's a bug. But I hope it doesn't do that, it's
rather the framework that calls it that suppresses that exception
instead of giving a HTTP 500.

[snip]
> I actually have switched back to the normal FileTemplateLoader
> without wrapping the escape directive for several reasons :
>
> - I actually make use of functions for URLs generation.
>   And it's ugly to see the noescape tags wrapping every link calls, some thing like :
>   <#noescape>${url.home("Return to main page")}</#noescape>
>   I prefer using ?html as needed

It's a strange coincidence that you say this just after 2 weeks or so
after I have proposed #p. It addresses exactly this annoyance, but I
don't remember anybody but myself ever brought this up, till now.
Anyway, it will be in 2.3.20 most probably.

> - Because i am using JBoss Tools for the freemarker editor (which
> does help me in parsing error detection, highlighting, etc), it
> errors the line of every noescape because it's not inside an escape
> directive, which gives me the tension i dont need.

This will also be solved by #p, since then you won't need #noescpe
anymore. Indeed, I might as well deprecate #noescape then.

>  And thank you for the wrapReader sharing, i will add it to my
> library in case i find a use of it in the future.
>
> Regards from Jakarta,
> Albert Kam

--
Best regards,
 Daniel Dekany


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user



--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)



--
Do not pursue the past. Do not lose yourself in the future.
The past no longer is. The future has not yet come.
Looking deeply at life as it is in the very here and now,
the practitioner dwells in stability and freedom.
(Thich Nhat Hanh)

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user
Reply | Threaded
Open this post in threaded view
|

Re: FileTemplateLoader doesnt close reader ?

Daniel Dekany
Thursday, March 14, 2013, 3:52:53 AM, Albert Kam wrote:

> Sorry almost forgot.
>
> I think the #p solution would be a good addition to the arsenal of freemarker,
> if paired with the automatic escaping feature.
>
> Will be excited for 2.3.20 updates, but for now,
> i'm content with manual escaping the expressions on user-generated-content.
>
> (OOT) Wishful thinking :
> From my short but intense experience of Freemarker, i can say that
> it feels natural and powerful to use.
> I just love building reusable directives (even the recursive ones !)
> I have decided to use it over jsp and jsf + apache tiles, both of
> which i have used before in previous projects.
> I just wish there's a freemarker eclipse plugin that's much more
> functional in helping maintaining freemarker templates.

Yeah, especially nowadays, good IDE support is critical for success. I
have no idea how and when will I have time for that, however... The
plans for the summer effort are bold enough as they are, and they also
must be done, as those are the things everything else can build on.
Plus I have no experience with Eclipse plugin development. So for this
to happen, most probably someone else has to do it. I remember Angelo
Zerr has started to develop a DLTK-based plugin back in 2010, and he
needed some changes in FreeMarkerf for that to work. But then that
effort has died because it was developed against FreeMarker 3, which
was pretty much cancelled. Man, that was shame. Well, if I will see
that this time it will make it, I will contact him, although it was
like 3 years ago, so that's no much hope.

> The JBoss Tool's freemarker plugin is good, but i stil miss these features :
> - ability to list the ftl-s that make use of a specific function / macro
> - ability to refactor function / macro's name, parameter name
> - detect function / macro usages error after changing the function / macro definition

These are tricky, as FreeMarker is not a static language. Of course
you can write a tool that helps in refactoring, but it will never be
100% reliable. Same with detecting errors. (Actually, I'm thinking of
language feature that can make these more reliable, by allowing one to
mark a template "strict", which means enforcing certain good practices
and banning making macros conditionally and so on. But I'm not yet
sure how that will work out.)

> - can operate based on the configuration passed, like showing the
> imported libraries in the template editor, or optionally show the settings
> - can link current active template to another, and then it see what
> variables are available (those assigned from the container template)

It's also a RFE to be able to query what data-model variables a
template accesses. It's again impossible to do reliably because of the
dynamic nature of the language, but of course if it's only to give you
hints while editing, it could work reasonably well.

> - link a model to a concrete class to help support with
> autocompletion on methods, data type validations, etc

And here the obstacle is that ObjectWrapper-s are pluggable and
configurable. But assuming a certain wrapper, it could be done to a
degree. But then, somewhere you had to declare what data-model
variables are expected with what type. I think in bigger projects you
want to document this anyway, and then if there was a standard way of
making documentation comments in templates, that would be a good place
to do this.

Funny that there's infinitely many things to do even in a poor little
MVC template engine. Then pair that with near 0 free time and having a
life... :)

--
Best regards,
 Daniel Dekany


> - etc
>
>
>
>
>
>
>
> On Thu, Mar 14, 2013 at 9:36 AM, Albert Kam <[hidden email]> wrote:
>> You aren't supposed to close the StringReader inside the
> TemplateLoader.
> Yeah, i returned a closed stringReader. What an omg-mistake !
> Now i have changed it to close the 'wrapped' reader instead.
> Thanks so much for the guides.
>
>
>
> On Thu, Mar 14, 2013 at 3:06 AM, Daniel Dekany <[hidden email]> wrote:
> Wednesday, March 13, 2013, 3:42:36 AM, Albert Kam wrote:
>
> [snip]
>> I tried closing the reader and catching and logging the exception
>> instead of just using IOTools.closeQuietly(),
>> and notices that there's no exception happening, and still gave me the empty page.
>> Commenting the close reader section gave me back the output.
>
> Are you returning a StringReader before calling
> IOTools.closeQuietly()on *another* reader? Because then this
> phenomenon it doesn't make any sense.
>
> [snip]
>> Aha, maybe that's the reason it gave me the empty output.
>> According to
>> http://docs.oracle.com/javase/7/docs/api/java/io/StringReader.html#close()
>> "Closes the stream and releases any system resources associated with it.
>> Once the stream has been closed, further read(), ready(), mark(),
>> or reset() invocations will throw an IOException. Closing a
>> previously closed stream has no effect."
>> Maybe the caller of the closed-reader got an IOException somewhere and hence, the empty output ?
>
> You aren't supposed to close the StringReader inside the
> TemplateLoader. You should close the *other* reader, the one that you
> don't return. Also, if FreeMarker gives empty output when an
> IOException occurs, that's a bug. But I hope it doesn't do that, it's
> rather the framework that calls it that suppresses that exception
> instead of giving a HTTP 500.
>
> [snip]
>> I actually have switched back to the normal FileTemplateLoader
>> without wrapping the escape directive for several reasons :
>>
>> - I actually make use of functions for URLs generation.
>>   And it's ugly to see the noescape tags wrapping every link calls, some thing like :
>>   <#noescape>${url.home("Return to main page")}</#noescape>
>>   I prefer using ?html as needed
>
> It's a strange coincidence that you say this just after 2 weeks or so
> after I have proposed #p. It addresses exactly this annoyance, but I
> don't remember anybody but myself ever brought this up, till now.
> Anyway, it will be in 2.3.20 most probably.
>
>> - Because i am using JBoss Tools for the freemarker editor (which
>> does help me in parsing error detection, highlighting, etc), it
>> errors the line of every noescape because it's not inside an escape
>> directive, which gives me the tension i dont need.
>
> This will also be solved by #p, since then you won't need #noescpe
> anymore. Indeed, I might as well deprecate #noescape then.
>
>>  And thank you for the wrapReader sharing, i will add it to my
>> library in case i find a use of it in the future.
>>
>> Regards from Jakarta,
>> Albert Kam
>
> --
> Best regards,
>  Daniel Dekany


------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
FreeMarker-user mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/freemarker-user