Re: Register static JNDI env entry dynamically
Am 2013-03-16 11:43, schrieb Konstantin Kolinko: 2013/3/16 Michael-O <1983-01...@gmx.net>: Am 2013-03-16 10:46, schrieb Konstantin Kolinko: 2013/3/16 Michael-O <1983-01...@gmx.net>: 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 instead of . 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. First of all, thank you for your valuable inspection. It is highly appreciated. I will comment inline. 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. No, it's not. ${CONTEXT_NAME} is a context scoped, by default predefined variable with the context name. Please see here: http://logback.qos.ch/manual/configuration.html#variableSubstitution 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. I am fully aware of that and always keep that in mind. In that case, there is no need to worry for the extra dependency. I am shading all necessary classes in an internal package in the JAR. They won't collide with deployed webapps. See http://tinyurl.com/cx7tzne 3. I prefer to use "constantString.equals(value)" instead of "value.equals(constantString)" because of nulls. Not an issue here though. Just style. I have seen this approach already in several spots in some Apache projects but haven't yet grasped the advantages because I always check input for validity. 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) Wow, that's new. But this "context.getNamingResources().addEnvironment(ce);" does add objects to the context? So, if I understand correctly, startup phase has a staging set of objects from which the final, read-only JNDI context will be build? 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. Thanks, I will remove the if loop. But reading your comment, I should add a checker on startup which verifies whether that env entry is already set? Something like: if (le.getType().equals(Lifecycle.START_EVENT)) { check for presence; if yes remove; add value; } 6. A Factory will not have access to Context. A Listener is better here. Yes, but it would spit out the same string which will be cached and returned to the app. The outcome would be the same, at least from the app's point of view. Michael - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Register static JNDI env entry dynamically
2013/3/16 Michael-O <1983-01...@gmx.net>: > Am 2013-03-16 10:46, schrieb Konstantin Kolinko: > >> 2013/3/16 Michael-O <1983-01...@gmx.net>: >>> >>> 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 instead of > . > > >> 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: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Register static JNDI env entry dynamically
Am 2013-03-16 10:46, schrieb Konstantin Kolinko: 2013/3/16 Michael-O <1983-01...@gmx.net>: 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 instead of . 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. Michael - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Register static JNDI env entry dynamically
2013/3/16 Michael-O <1983-01...@gmx.net>: > 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 =? 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. 3. Do you have control over code that uses the value? 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. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Register static JNDI env entry dynamically
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. Thanks, Michael - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org