Re[2]: ContextAware event handlers

[Alexey Panchenko <al...@olmisoft.com>]

Will Glass-Husain wrote:

> It is kind of a hack.  I was trying to avoid the hassle of interface 
> versioning.

> Remember that Event Handlers can be configured in two different modes.  One 
> is a "Global" event handler that is defined in the velocity.properties file. 
> In such cases you have a singleton and the setContext is useless (if not 
> actively harmful).  This is documented in the ContextAware javadoc.

yes, it is documented
=============
Important Note: Only local event handlers attached to the context
(...) should implement ContextAware
=============

So what's the point in checking if global event handler implements
this interface ? The docs say "it should not", so lets remove the
check.

> The other case is attached to a specific context.  Contexts are not 
> threadsafe, and are typically created anew for each page generation.  (a 
> typical use is per-request).  In such a case the context and event handler 
> are both initialized, the event handler is attached to the context, and the 
> page is generated.  As long as this happens anew for each page generation 
> there is no issue.

> I'm open to other suggestions on this.  Maybe we need a few paragraphs about 
> this in the docs.  The threadsafe issue comes up from time to time.  (the 
> typical answer is "yes Velocity is threadsafe but Contexts may not be).

All the functions inside EventHandlerUtil have two loops - one on
application handlers and another one on context ones.

I would like the hack to be removed, i.e.

1) remove ContextAware checks for application (Global) handlers.

2) document that ContextAware is _executed_ only for context (local)
handlers (probably just rephrase the current javadocs).

I can make a patch.

> ----- Original Message ----- 
> From: "Alexey Panchenko" <al...@olmisoft.com>
> To: <ve...@jakarta.apache.org>
> Sent: Monday, November 21, 2005 9:47 PM
> Subject: ContextAware event handlers

>> Hello,
>>
>> I was looking through the EventHandlerUtil class and found that some
>> code looks a bit strange to me.
>>
>> For example lets look at methodException() method.
>>
>> =================================
>> EventCartridge ev1 = rsvc.getApplicationEventCartridge();
>>
>> // skipped
>>
>> if (ev1 != null)
>>  for (Iterator i = ev1.getMethodExceptionEventHandlers(); i.hasNext();)
>>  {
>>    MethodExceptionEventHandler eh = (MethodExceptionEventHandler) 
>> i.next();
>>    if (eh instanceof ContextAware)
>>      ((ContextAware) eh).setContext(context);
>>    return eh.methodException(claz, method, e);
>>  }
>> =================================
>>
>> I think that ContextAware check for _application_ event handlers is
>> useless in the multithreaded applications.
>>
>> There is no synchronization used, so handler can get "context" from
>> one thread and "claz,method,e" from the another thread.
>>
>> So, at least, for application event handlers the ContextAware lines
>> should be removed.
>>
>> If somebody wants context in application event handlers - the correct
>> way to go is to introduce another interface (for example
>> MethodExceptionEventHandler2) with new method, for example
>> methodException(context, claz, method, e);
>>
>> What do you think ?
>>
>> -- 
>> Best regards,
>> Alexey                          mailto:alex+news@olmisoft.com
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>> 


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




-- 
Best regards,
 Alexey                            mailto:alex+news@olmisoft.com


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