|Sean Schofield||Sep 30, 2004 12:23 pm||.java|
|Craig McClanahan||Oct 2, 2004 12:25 pm|
|Sean Schofield||Oct 5, 2004 7:38 am||.patch|
|Craig McClanahan||Oct 5, 2004 10:38 am|
|Sean Schofield||Oct 5, 2004 1:26 pm|
|Craig McClanahan||Oct 5, 2004 1:42 pm|
|Sean Schofield||Oct 5, 2004 2:29 pm|
|Sean Schofield||Oct 5, 2004 3:52 pm||.java|
|Subject:||Re: [chain] New interface: CatalogFactory|
|From:||Craig McClanahan (crai...@gmail.com)|
|Date:||Oct 5, 2004 10:38:55 am|
On Tue, 05 Oct 2004 10:38:54 -0400, Sean Schofield <sean...@schof.com> wrote:
Thanks for commiting the file. I agree that this might be the best way to generate discussion. I've answered some of your points below.
I've committed your proposed CatalogFactory.java interface to [chain], with minor tweaks to the Javadoc comments to conform to the usual style in [chain].
Sorry about the nonconforming javadoc. I did my best to conform to the style.
No problem. I'm sort of anal about putting well formed HTML in my Javadoc comments, including <p> ... </p> around the entire description section :-).
The getInstance() method seems out of place here ... in most factory pattern implementations I've seen, this would actually be a public static method on an implementation of CatalogFactory. What kind of use cases do you see for it here?
I agree. I had started wondering about this myself. I definitely plan on having it in my implementation as you point out. I've attached a patch that removes this. The patch also fixes a problem with excessive line spacing. I'm not sure if it was on my end or your end but the source file had an "extra" space for every line. The patch addresses that as well.
I should have checked on the line endings before committing ... was it built on a Windows system with CR-LF? I checked it in on Unix.
I'll apply the patch tonight, when I can reach the repository without my friendly corporate firewall in the way.
Also, your original motivation was to disconnect catalog lookups from something like the ServletContext attribute that ChainListener (containing a Catalog) sets up. How are you proposing to do this disconnect -- I presume it's something in the actual implementation of CatalogFactory? If so, will your proposed solution work when commons-chain.jar is included either in the WEB-INF/lib directory, and when it's installed in the servet container (thus making static variables global across webapps)?
Hmmm you seem to be pointing out something that I was not aware of. If I understand you corectly, you are saying that classes contained in a jar in the server's classpath (ex. server/lib) vs. the application classpath, then that class will be the one used by every application on the server. Is that right?
If so, then a static class loaded by the server container is what will be referenced by all applications using that class.
That's exactly right.
I had not thought of that. I'm not sure that this is a a deal breaker though. People wishing to deploy the chain.jar at the server level will just need to be aware that they should probably not use the "default" catalog. If each application adds their catalog with a unique name, there should not be a problem right?
That works, but requires a pretty fair degree of cooperation between application developers that I'm not sure you can count on ... as an application developer, I have no way to know what other apps I will be coexisting with.
Other libraries have dealt with this sort of thing in a number of different ways. Here are some ideas for us to consider:
* Leverage the fact that each web application has a unique class loader that can then be used as a map key to disambiguate the default catalog. The container guarantees to set up something called the "thread context class loader" before calling any code inside the webapp, so your getInstance() method could retrieve it like this:
ClassLoader cl = Thread.currentThread().getContextClassLoader();
and use it as the key to a Map that contains the actual catalogs for that webapp. The biggest problem with this approach is cleaning up when the application is undeployed or reloaded ... you'll want something like a ServletContextListener whose contextDestroyed() method can call a cleanup to release all the catalogs for that webapp.
NOTE: Commons Logging uses techniques like this.
* Make your catalogs into JNDI resources so that you can access them with symbolic names, just like you can access container-provided JDBC connection pools. The details of this will be container specific; for Tomcat, see:
especially the section "Adding Custom Resource Factories".
* Punt, and require that the library be loaded individually in each webapp instead of shared. This is obviously the least desireable solution, but it's the easiest.