Nice! It was a pleasure to help. The other issue I'm in now is https://issues.apache.org/jira/browse/OPENEJB-1120, and have already posted in dev about it: http://old.nabble.com/Patching-TomcatSecurityService-to-return-the-guest-role-when-nobody-is-logged-in-td26815302.html There's also a patch attached, however I haven't modified any tests, as I couldn't find any... Luckily, the patch is very simple.
When any of you have time to review it, it would be nice. Luis Fernando Planella Gonzalez Em Quinta-feira 17 Dezembro 2009, às 13:29:14, Jean-Louis MONTEIRO escreveu: > > 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 > >>> >> >> > >>> >> > > >>> >> > > >>> >> > >>> >> > >>> > > >>> > > >>> > >>> > >> > >> > > > > > >
