On Apr 15, 2004, at 9:32 AM, Claude Brisson wrote:
broken is broken! I'm sure there will be a way to accomodate though w/o breaking.
Maybe by just having 2 interfaces, the old one and the new one ?
That would be one way, yes...
CloD
----- Original Message ----- From: "Geir Magnusson Jr" <[EMAIL PROTECTED]> To: "Velocity Developers List" <[EMAIL PROTECTED]> Sent: Thursday, April 15, 2004 1:41 PM Subject: Re: Event Handler patch
On Apr 14, 2004, at 4:24 PM, Will Glass-Husain wrote:
Hi Geir,
Thanks for the detailed comments, particularly on global/local. I think such a distinction is a good one, but I suggest we should give the handler developer some flexibility. The old way (attaching to the context) still works (almost exactly the same). The new way (velocity.properties) allows for global handlers.
I don't quite get the meaning of 'but' here. All I did was name the ones attached via vel.prop as 'global' and the others as 'local', meaning global to all renderings vs local to a specific one.
[SNIP]
Having said all this, this is a minor point.
Yes.
If you or others feel that it's confusing to have the context as an argument in the EventHandler interfaces, (giving the ability to take local circumstances into account in a globally registered event handler) we can take it out.
It makes perfect sense w/ a global handler, and a local one too. The good thing about a local one is that the author can do it if they choose... we wouldn't need special support for the local ones (esp if the support broke things)
Next, regarding backwards compatibility... look forward to your
thoughts on
this once you review the patch. The break is very minor, just a
change in
the method signatures for the event handler interfaces. (For example,
the
handlers now require an initialize method). I'm not sure how to
structure
this in a backwards compatible way without a whole new interface, which
seems redundant.
broken is broken! I'm sure there will be a way to accomodate though w/o breaking.
Finally, answer your questions:Hm. These both then are examples of handlers w/ specific purposes?
The new interface "IncludeEventHandler" is a class of event handlers
(similar to ReferenceInsertionEventHandler and
MethodExceptionEventHandler)
that allows the developer to modify the behavior of #include and
#parse.
Essentially, it's what I proposed previously as InputEventHandler in
Bug #
20342). I've included two specific implementations IncludeNotFound,
that
provides for a (configurable) page to include when the requested
template
can not be found, and "IncludeRelativePath", that provides for relative
paths for includes. Similarly, I've created "EscapeXMLEntities"
which a
specific implementation of the ReferenceInsertionEventHandler to
escapes
HTML/XML entities. I think it'd be nice to include in Velocity a small
number of handlers for commonly requested features. (developers will
want
to create custom handlers for more specific situations, of course).
How do you configure them? [IncludeNotFound]Through velocity.properties. See the Javadocs.
This will probably make better sense once you have a chance to browse the patch.
Best regards, WILL
----- Original Message ----- From: "Geir Magnusson Jr" <[EMAIL PROTECTED]> To: "Velocity Developers List" <[EMAIL PROTECTED]> Sent: Wednesday, April 14, 2004 12:15 PM Subject: Re: Event Handler patch
On Apr 14, 2004, at 1:42 PM, Will Glass-Husain wrote:
Hi,
I've just posted in Bugzilla a fairly substantial patch to the event
handling facilities of Velocity. I'd love to get feedback from other
Velocity developers, particularly those using event handlers. This
patch is
intended to make the event handlers easier to use and also more
flexible.
It includes a new event handler interface "IncludeEventHandler", and
several
pre-built handlers that can be just plugged in.
BACKGROUND
I created this in response to a couple of frustrations
- Event handlers are currently somewhat cumbersome to configure, with
special code required to "attach" them to the context. This always
seemed a
little odd to me, since from the user perspective this has nothing to
do
with the primary purpose of the context. Now the event handlers can
be
configured in velocity.properties.
It certainly does - if your handler is specific to the request, then
the context is the right way to do it. That said, 'global' handlers
aren't a bad idea, but a global solution doesn't discount the need for
a local one.
- event handlers currently run in isolation, which limits their
utility.
They have no ability to call system services (e.g. to see if a
template
exists or to check a velocity property) and no ability to tell where
they
are on the page or what else is being processed. Now event handlers
have
access to RuntimeServices and to the current context.
That's probably not a bad idea. I'm trying to remember if there were
any 'security' issues surrounding this, or we just didn't do it 'cuz'.
- There's been the occasional call on the list for modifications to
the
behavior of #parse and #include. For example, using retrieving files
with
relative paths, or displaying a "page not found" message when the
template
is missing. The new IncludeEventHandler allows the developer to
modify the
behavior of these directives.
- Some developers also need HTML escaping of references (for example,
this
is turned on by default in JSTL). This patch provides a built in
ReferenceInsertionHandler to accomplish this.
Hm. These both then are examples of handlers w/ specific purposes?
SPECIFICS
The specific changes are listed below. If this is too much for a
single
patch, let me know and we can break them apart. I've created a
"readme.txt"
that gives more detailed instructions. Once the patch is accepted,
I'll
write the docs. Geir's made a few positive comments about this
approach, so
I'm optimistic about eventual committer action. Regardless, I really
look
forward to feedback.
(1) Event handlers can be registered in the velocity-properties, e.g.
eventhandler.referenceinsertion.class = org.apache.velocity.app.event.implement.EscapeXMLEntities
(2) Multiple event handlers can be registered for a given type and
are
applied in a "filter" pattern. The specifics vary per event handler
(documented in the JavaDocs). The following example would first
enforce a
relative path rule to included templates, and would then check to see
if the
template is actually available.
eventhandler.include.class =
org.apache.velocity.app.event.implement.IncludeRelativePath,org.apa ch
e.
veloc
ity.app.event.implement.IncludeNotFound
How do you configure them?
(3) Event handlers have a new lifecycle. Before they are called they
are
initialized with a link to RuntimeServices. This allows the handlers
to
retrieve Velocity properties and to have access to a number of
interesting
system methods. Each event handler is also given the current context
when
it is called. This allows the template designer (or custom
directives) to
change the behavior of the event. A nice example of this is the
EscapeXMLEntities reference insertion handler, which stops escaping
HTML
when an "ignoreEscape" flag is set in the context.
Now you've come full circle and are making this context dependent - I
as the developer have to trust/assume that someone configured the
engine externally to have a EscapeEXMLEntities handler and then use it
via the context?
This sounds like a nice handler, but I wonder if having a 'global' handler be context dependent makes sense.
(4) New implemented event handlers include: -- EscapeXMLEntities (escapes HTML/XML & < > ") -- IncludeNotFound -- IncludeRelativePath -- PrintExceptions
Using these built-in event handlers, users can do useful things (e.g.
escape
HTML) with no extra coding just by configuring velocity.properties.
I'm pretty sure that there are cases where a engine-global hander is
useful, but I'd also bet there's lots of cases (escaping entities or
relative path stuff) where current context matters - who is logged in,
what the material being rendered is, etc... What portion of a page is
being rendered...
(5) The patch is almost entirely backwards compatible. The old
"attach the
handler to the context" still works, although this results in a
per-request
rather than per-application lifecycle for the handler. One
difference...
the API for the event handlers has changed, and as such will require
minor
editing and recompilation of user code. (I think this is worth it).
All
tests (old and new) work.
Well, I'll dig in and look. I suspect I'll come back w/ a suggestion
that we don't break backwards compatibility (surprise, surprise) and
that maybe we expand the system to have 'global' and 'local', where
the
global are as you suggest - configurable in the velocity.properties
and
local as they are now, possibly w/ the addition of being able to
accept
runtime services.
I see that you could have two kinds of handlers - one, the current
'local' handlers which are context specific for anything where there's
a runtime decision or configuration involved, and the 'global'
external
type for cases where it doesn't matter - it should apply globally for
all renderings....
geir
Best regards,
WILL _______________________________________ Forio Business Simulations
Will Glass-Husain [EMAIL PROTECTED] www.forio.com
------------------------------------------------------------------- --
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
-- Geir Magnusson Jr 203-247-1713(m) [EMAIL PROTECTED]
-------------------------------------------------------------------- -
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
-- Geir Magnusson Jr 203-247-1713(m) [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
-- Geir Magnusson Jr 203-247-1713(m) [EMAIL PROTECTED]
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
