Refactor how transaction managers are maintained

I think the 1:m relation between transaction mode and database instances is OK.

A n:m relation would require to select transaction manager at db.begin() and I don't see advantages with that. By using 1 jdo-conf per database the current 1:m relation behaves as being a 1:1 relation.

Fine by me. Iow, let's enhance the code to allow for more than one JDO configuration file, and let's document the required assumptions.

I think this is also linked to the lifecycle problem; I'll copy some text I wrote in CASTOR-1135 here, where it seems more appropriate.

Fundamentally, I feel that they're actually the same problem.

Gut feeling: what we really need to do to avoid all this, and permanently eliminate the visibility issue of shared contexts (which, admittedly, isn't what everyone is interested in, but...)

1) Allow JDOManager API to use two methods: An internal, aggregated set of JDOConf objects which can be used situationally to hold the necessary configuration information for a given database instance. DBRegistry would probably become ConfigRegistry and would have to manage all of the config information, not just the stuff related to the database.

2) Extend programmatic API to allow users to specify the datasource itself to use; this will either require that a JDOContext object wrap the JDOConf + all instance-usage data, or that JDOConf not be auto-generated from schema (as it would need to be able to hold instances and the like). Context is probably the easier way. Allow that context to be returned.

3) Add a method to getDatabase() with a context + name, instead of name only.

4) Extend JDOManager to search for the name in getDatabase() by walking the list of known internal contexts.

Now, personally, I think I'd like to see the API go away for JDO instances from JNDI, and for it all to be handled through contexts - or for the JNDI loader/handler to be entirely separate from JDOManager-managed API, and have code in it that basically knows how to load a file, get the context object, and use that context object to get database instances. A standalone JDOObjectFactory, for use from JNDI.

Then, change all API that loads configs to return a JDOContext representing that config. Keep no internal state; remember nothing. JDOContext will contain all the state information related to that context's operation, and is as global as the config is today. When the context is destroyed, so is everything related to our knowledge of that config, database, classloader, etc. Whether or not this also eliminates caches keyed based on that config - well, who knows, but I think it would be nice.

JDOObjectFactory can use that API to get its context and create database objects through getDatabase(); when the factory is destroyed by the JNDI environment, its context goes with it.

I think - regardless of the final direction of the lifecycle problem - this patch is a step in the right direction, and I'd like to see it go in; however, I'm not sure moving lots of API to JDOManager/JDO is a good idea until we get some idea of how to solve the lifecycle problem; that may, in turn, eliminate a lot of API that we're currently wondering about the proper location of; conveniently, it might also give us better/more control over how it's instanced.

Basically: Handle configs through contexts; let people choose and manage their contexts themselves. Drop the idea of a 'global registry' of this information, and keep it all within the context of its use. The transaction manager, the databases themseves which apply to those, etc., are all specific to the context which would apply.

Greg, why not prepare a simple patch that shows some of the concepts explained above ? Forget about any implementation details, just interfaces and(or method stubs would do fine ...

First version of a patch with quite finished refactoring of TransactionManager handling. The patch passes all tests of CTF and should resolve the problems seen at current CVS.

Still some additional work needs to be done to fully solve the duality of _source and _configURI in JDOManager. Also some code cleanup and comments are missing. In addition I like to think about seperating some of the responsibilities from ConnectionFactory.

+1 for checkin, it's a good solution, imo.

Looking at this, I do have quite some concerns with regards to the 'falling back to local mode business'. If you intend to run in a J2EE container (or with Spring using global transaction demarcation), in my view you cannot trick the user that Castor will be participating in an XA transaction, whilst internally we fall back to local transaction mode. This basically violates the contract we are meant to be adhering to when supporting J2EE apps that use a (distributed) transaction monitor.

Having said that, given that it takes us longer to sort this out than initially perceived, how would you feel about reverting to CASTOR-1173 for the time being, to re-introduce stability to the current dev environment.

In TransactionManagerFactoryRegistry.load(), the comment seems to be in conflict with the fact that the load() method is still declared synchronized.

I wonder whether LocalTransactionManagerFactory should actually be changed to return a Null Object rather than null ...

Updated patch. Now throws exception if fails to get a transaction in J2EE mode. Also removed synchronization where not required and did some code cleanup in transactionmanager package.

I tend to commit this and move additional refactoring at JDOManager and ConectionFactory to a new issue and not to back out CASTOR-1173.

Updated patch introducing a LocalTransactionManager returned by LoaclTransactionManagerFactory to replace null checks for better readability of the code in JDOManager.getDatabase().

+1.