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.
Passing in the context as an argument to the event handlers gives the handler developer flexibility to let a globally declared handler have one behavior most of the time, and a modified behavior some of the time. Escaping HTML is a good example of this. Most of the time in a web app you'd want the reference output to be escaped. But rarely (such as when inserting HTML itself) you want to turn it off. By giving the handler access to the context (regardless of how the event handler was registered) the developer can allow for this. Having said all this, this is a minor point. 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. 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. 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.apache. > > 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]
