You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by Luis Fernando Planella Gonzalez <lf...@gmail.com> on 2009/12/07 14:42:13 UTC
Re: Resolve datasource from the application name
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
>
Re: Resolve datasource from the application name
Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
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
> >>> >> >>
> >>> >> >
> >>> >> >
> >>> >>
> >>> >>
> >>> >
> >>> >
> >>>
> >>>
> >>
> >>
> >
> >
>
>
Re: Resolve datasource from the application name
Posted by Jean-Louis MONTEIRO <je...@atosorigin.com>.
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.
Re: Resolve datasource from the application name
Posted by Jean-Louis MONTEIRO <je...@atosorigin.com>.
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.
Re: Resolve datasource from the application name
Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
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
> >> >>
> >> >
> >> >
> >>
> >>
> >
> >
>
>
Re: Resolve datasource from the application name
Posted by Jean-Louis MONTEIRO <je...@atosorigin.com>.
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-tp25684131p26793833.html
Sent from the OpenEJB Dev mailing list archive at Nabble.com.
Re: Resolve datasource from the application name
Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
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
> >>
> >
> >
>
>
Re: Resolve datasource from the application name
Posted by Jean-Louis MONTEIRO <je...@atosorigin.com>.
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-tp25684131p26679345.html
Sent from the OpenEJB Dev mailing list archive at Nabble.com.
Re: Resolve datasource from the application name
Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
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
>
Re: Resolve datasource from the application name
Posted by Jean-Louis MONTEIRO <je...@atosorigin.com>.
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-tp25684131p26679167.html
Sent from the OpenEJB Dev mailing list archive at Nabble.com.
Re: Resolve datasource from the application name
Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
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