Ok, I've attached another patch in https://issues.apache.org/jira/browse/OPENEJB-1027 called dataSourceUpdated.patch. Thanks.
Luis Fernando Planella Gonzalez Em Terça-feira 15 Dezembro 2009, às 10:10:14, Jean-Louis MONTEIRO escreveu: > > Hi, > > actually i've been working on another JIRA so i didn't get any time to work > on that one. > To be honest, i don't like nested if statements. So if you could change is > according to the previous post, it would be great. > > IMO, the web module id should be the first. > > Definitely, you are more than welcome to propose enhancements and patches. > > Jean-Louis > > > > Luis F. Planella Gonzalez wrote: > > > > Hi. > > Any updates on this subject? > > I saw nothing was committed to date. If you want, I can create another > > patch with the updated idea of using a List<String> instead of nested > > ifs... > > Also, another doubt I have is: for WebModules, how is the moduleId > > obtained? I have only ran the test code on the JUnit test, and don't know > > how the tomcat integration configures it... This raises the issue: on the > > code I've mentioned in a previous e-mail, the web module id is searched > > before the context root. Should this be that way or the context root > > should be first? > > Thanks. > > > > Luis Fernando Planella Gonzalez > > > > > > > > Em Segunda-feira 07 Dezembro 2009, às 13:59:05, Jean-Louis MONTEIRO > > escreveu: > >> > >> Great! > >> Sorry ... > >> > >> Jean-Louis > >> > >> > >> Luis F. Planella Gonzalez wrote: > >> > > >> > The patch is there. It's called dataSourceFromModules.patch > >> > > >> (https://issues.apache.org/jira/secure/attachment/12427172/dataSourceFromModules.patch). > >> > I've attached the complete files just to make easier for someone to > >> > quickly see > >> > the entire files... > >> > -- > >> > Luis Fernando Planella Gonzalez > >> > > >> > > >> > Em Segunda-feira 07 Dezembro 2009, às 13:47:52, Jean-Louis MONTEIRO > >> > escreveu: > >> >> Hi, > >> >> > >> >> first of all, congrats for your baby ;-) > >> >> > >> >> Can you please attach a patch file instead of the source file? > >> >> Then, I gonna be more than happy to have a look and commit it for you. > >> >> > >> >> Jean-Louis > >> >> > >> >> Luis F. Planella Gonzalez wrote: > >> >> > Sorry for reposting, but if what I just proposed is to be > >> implemented, > >> >> > maybe > >> >> > the findMatchingDataSources(String) method could be inlined, as I > >> used > >> >> a > >> >> > String[] as return to be able to return 2 values. > >> >> > > >> >> > I actually liked this way better than what I've patched. The code is > >> >> > simpler > >> >> > and easier to add new ids in the check. > >> >> > > >> >> > -- > >> >> > Luis Fernando Planella Gonzalez > >> >> > > >> >> > Em Segunda-feira 07 Dezembro 2009, às 11:42:13, você escreveu: > >> >> >> 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 > >> >> > >> > > >> > > >> > >> > > > > > >
