2013/3/16 Michael-O <[email protected]>:
> Am 2013-03-16 10:46, schrieb Konstantin Kolinko:
>
>> 2013/3/16 Michael-O <[email protected]>:
>>>
>>> Hi,
>>>
>>> I'd like to make a string available as an env entry via JNDI. The string
>>> is
>>> static but must be created dynamically at startup time of the webapp.
>>>
>>> I have evaluated and implemented a listener which I have added to my
>>> context.xml. It listens for startup and shutdown events. It works pretty
>>> well. Rethinking this makes me ask whether a listener is the right thing
>>> to
>>> do.
>>>
>>> Would an object factory a better approach for this?
>>> Did I abuse the listener concept?
>>>
>>> Code is available for inspection.
>>>
>>
>> 1. Tomcat version =?
>
>
> I am on Tomcat 6.0.35.
>
>
>> 2. Looking at docs,
>>
>> http://tomcat.apache.org/tomcat-7.0-doc/config/context.html#Environment%20Entries
>> I do not see factory as an allowed attribute for environment entries.
>
>
> That was not my intent. I was referring to <Resource> instead of
> <Environment>.
>
>
>> 3. Do you have control over code that uses the value?
>
>
> No(t really), it's Logback. I retrieve the value from JNDI in the
> logback.xml.
>
>
>> 4. Listeners are OK. I wonder though whether JNDI is modifiable or
>> read-only at that point in time. If it works for you, then it is has
>> to be modifiable.
>
>
> It is modifiable. See my implementation please:
> http://tinyurl.com/a79jdgt
>
> And the usecase for:
> http://tinyurl.com/afu9ppl
>
> I have this in production for almost a year now but decided finally to make
> it publically available under ASLv2. That's the reason for my doubts.
>

Generally I do not see anything substantially wrong with the code.

Some comments on the code:

1. In logback-test.xml  you use ${CONTEXT_NAME}
I think it is a typo, as you are using "contextName" elsewhere.

2. In  LogbackContextNameListener.java
Dependency on commons-lang is a bad thing here, as the class has to go
into ${catalina.base}/lib  and it would be better to minimize
dependencies. You should not use libraries that might be included into
webapps, or you will be open to classloader issues, memory leaks etc.
The method you are using is trivial to be implemented inline.

3. I prefer to use  "constantString.equals(value)" instead of
"value.equals(constantString)" because of nulls. Not an issue here
though. Just style.

(in "le.getType().equals( Lifecycle.eventname)" calls).

4. You are not really changing JNDI. You are changing context
configuration that JNDI is populated from later.
(NamingContextListener registers itself as PropertyChangeListener on
NamingResources and propagates your changes to JNDI)

5. I would not clear the value on stop event. It causes unnecessary
work (propagating the value to JNDI) and shutdown is supposed to be
fast.

Double assignments during repeated starts should not be an issue, but
if they are then you can call
context.getNamingResources().removeEnvironment(name)  before adding.

6. A Factory will not have access to Context. A Listener is better here.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to