Committed revision 891762 Thanks Luis.
Jean-Louis MONTEIRO wrote: > > 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-tp25684131p26829746.html Sent from the OpenEJB Dev mailing list archive at Nabble.com.
