Hi Luis, it looks really good. Gonna commit it.
Sorry for the delai. And thanks! Jean-Louis Luis F. Planella Gonzalez wrote: > > 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 >> >> >> >> >> > >> >> > >> >> >> >> >> > >> > >> >> > > -- View this message in context: http://old.nabble.com/Resolve-datasource-from-the-application-name-tp25684131p26810134.html Sent from the OpenEJB Dev mailing list archive at Nabble.com.
