Actually, I was off for 2 weeks. And yes, she's the first baby. Thanks God she's very calm....
Well, returning to the issue: Sorry, with hundreds of mails after those 2 weeks, I've actually seen your answer after I've attached the patch to https://issues.apache.org/jira/secure/ManageAttachments.jspa?id=12426288 Please, check the comment on the issue. The code is there needing a review, but I did implemented it several ifs, but it could be replaced by something like: // Collect which data sources will be searched List<String> ids = new ArrayList<String>(); ids.add(unit.getName()); for (WebModule webModule : app.getWebModules()) { ids.add(webModule.getId()); ids.add(webModule.getContextRoot()); } ids.add(app.getModuleId()); // Search for a matching data source for(String id : ids) { dataSources = findMatchingDataSources(id); if (dataSources != null) { jtaDataSourceId = dataSources[0]; nonJtaDataSourceId = dataSources[1]; break; } Also, I did added a PersistenceModule.getModuleId() case (with a TODO), so it will probably have to be removed. Anyway, the tests are covering all cases (except for PersistenceModule.getModuleId()), and I think the issue is resolved... Please, let me know if anything changes... -- Luis Fernando Planella Gonzalez Em Sexta-feira 20 Novembro 2009, às 19:57:55, David Blevins escreveu: > On Nov 19, 2009, at 3:46 AM, Luis Fernando Planella Gonzalez wrote: > > I'd love to keep helping on this issue... > > However I have an imminent issue: My daughter will be born on > > saturday, so > > I'll be off for a week or so. > > Afterwards, we can resume the subject. > > Wow! Congratulations! A week or so? Guessing this is not your first > kid, you sound like a pro! > > > Anyway, that algorithm should be used only when neither > > jtaDataSource nor > > nonJtaDataSource are used, right? At least for what I saw in code, > > it's > > possible to have only nonJtaDataSource (making a new jta datasource > > to be > > deployed). > > Yeah, I'm not sure. I'd have to take a look at our current > "fanciness" and see what makes the most sense. We'd want this sort of > "matching loop" to work for resolving any resource ID, so that might > change how we code it. > > > So, my guess would be to make the chunk of code I added to deploy(app, > > persistenceModule) a private method, let's say, > > locateDataSource(id), which > > would look for a jta data source with that name, then a non-jta, > > then an > > unespecified. > > > > So, could it be something like: > > * if explicit datasources are used, use them > > * if neither jta-data-source nor non-jta-data-source, try to guess: > > *** try locateDataSource(persitenceUnit.getName()) > > *** if not found, try > > locateDataSource(persistenceModule.getModuleId()) // Is > > it really necessary? As I saw, PersistenceModule.getModuleId() is > > always null > > *** if not found, for each WebModule try > > locateDataSource(webModule.getContextRoot()) > > Wasn't referring to the PersistenceModule.getModuleId(), but the > module in which the persistence unit was found. *Most* the time the > persistence.xml is inside an ejb jar or webapp. We don't retain that > information when we create the PersistenceModule now, but we could > maybe link it to the EjbModule or WebModule if it was. But we can add > that as a separate step. > > Definitely we would want this as some sort of loop of strings to check > rather than as a bunch of nested if statements. Then the same logical > routine could work for resolving any resource ref. For a plain > resource ref the list of strings might be something like this: > > 1. mappedName > 2. jndi name > 3. variable name > 4. module name > 5. app name > > Thinking out loud of course, the code might disagree with me :) Some > ideas sound really good and then turn out to be too much work given > the code. > > > -David >
