You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by David Blevins <da...@visi.com> on 2009/10/09 22:07:32 UTC

Re: Resolve datasource from the application name

On Sep 30, 2009, at 9:05 AM, Luis Fernando Planella Gonzalez wrote:

> Hi.
> I've reported http://issues.apache.org/jira/browse/OPENEJB-1027 in  
> may, which
> is modifying a bit the heuristics to resolve a data source by  
> matching the app
> name (web app context in our case) if the data source has not been  
> explicitly
> specified. This was previously discussed with David Blevings, and he  
> suggested
> that way.
>
> Just to explain the use case: We're migrating cyclos (http://www.cyclos.org 
> ),
> an open source struts / hibernate application to gwt / ejb3. Many  
> people host
> several instances of the very same application in the same tomcat,  
> and to keep
> supporting that it would be necessary to unpack, change every  
> persistence.xml
> from each and repack each instance again. With this feature  
> implemented, we
> can just assume there will be a data source with the same name of the
> application context and we're done, and our users will be happy :)
>
> We can try patching ourselves, but we need some guidance on both  
> fundamental
> questions:
> * Where is this heuristic implemented?
> * How to get the current application name?

The logic is in:

http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/AutoConfig.java

There's a pretty good test case for the persistence unit side of that  
logic here:

http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/AutoConfigPersistenceUnitsTest.java

The javadoc was incorrect in some places, so I did a sweep through it  
and fixed it all for you.

I recommend starting with a copy of the AutoConfigPersistenceUnitsTest  
and expand it to always have two apps and spend a good amount of time  
covering all the scenarios  (what happens when there are more apps  
than data sources, what if there are no data sources that match,  
etc.)   The logic in AutoConfig is a bit of a beast.

Even if you aren't able to get the logic in, having a really good test  
case that covers more than just the "happy path" would be extremely  
valuable.  For functionality like this I spend at least 60% of the  
time on the test case, so even having that would be a major help  
towards getting this feature in.  The test case also kind of like a  
requirements doc, which is a cool thing when collaborating.


-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

Re: Resolve datasource from the application name

Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
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 David Blevins <da...@visi.com>.
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>.
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.

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).

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())

What do you think?

--
Luis Fernando Planella Gonzalez



Em Quinta-feira 19 Novembro 2009, às 01:20:19, David Blevins escreveu:
> On Nov 16, 2009, at 5:21 AM, Luis Fernando Planella Gonzalez wrote:
> > Well, assuming that my assumptions are right, I've patched AutoConfig
> > to use the first WebModule's root context name as a guess to the
> > DataSource name.
> > I've attached the patch, as well as complete files for AutoConfig and
> > AutoConfigPersistenceUnitsTest in
> > https://issues.apache.org/jira/browse/OPENEJB-1027
> > As this is my first code contribution to OpenEJB. So, please, correct
> > me if I did anything wrong...
> > Feedbacks are appreciated.
> 
> This is a great start, thanks!  I've gone ahead and committed it.
> 
> Would be great to get the other searches in there described in
> OPENEJB-1027 such as linking based on the persistence unit name, etc.
> I'm not sure the code is setup to make this easy now because the
> getResourceId() method is perhaps too aggressive -- it always finds an
> id no matter what -- but but at a high level it seems like something
> like this would be really cool.
> 
>      List<String> strings = new ArrayList<String>();
>      strings.add(jtaDataSourceName);
>      strings.add(persistenceUnitName);
>      strings.add(moduleName);
>      strings.add(applicationName);
> 
>      String resourceId;
> 
>      for (String string : strings) {
>          resourceId = getResourceId(string);
> 
>          if (resourceId != null) break;
>      }
> 
>      // use the resourceId ....
> 
> That's the high level thought at least.  Lot's of ways that could be
> rolled out with the existing code.  Maybe we just change
> getResourceId() so that the 'String resourceId' is actually a var-arg
> and move it to the end of the signature.  Or something.
> 
> Open to ideas.
> 
> And just as a side note, you're under no obligation to keep working on
> this, but you're doing great and we can always use the help! :)
> 
> -David
> 

Re: Resolve datasource from the application name

Posted by David Blevins <da...@visi.com>.
On Nov 16, 2009, at 5:21 AM, Luis Fernando Planella Gonzalez wrote:

> Well, assuming that my assumptions are right, I've patched AutoConfig
> to use the first WebModule's root context name as a guess to the
> DataSource name.
> I've attached the patch, as well as complete files for AutoConfig and
> AutoConfigPersistenceUnitsTest in
> https://issues.apache.org/jira/browse/OPENEJB-1027
> As this is my first code contribution to OpenEJB. So, please, correct
> me if I did anything wrong...
> Feedbacks are appreciated.

This is a great start, thanks!  I've gone ahead and committed it.

Would be great to get the other searches in there described in  
OPENEJB-1027 such as linking based on the persistence unit name, etc.   
I'm not sure the code is setup to make this easy now because the  
getResourceId() method is perhaps too aggressive -- it always finds an  
id no matter what -- but but at a high level it seems like something  
like this would be really cool.

     List<String> strings = new ArrayList<String>();
     strings.add(jtaDataSourceName);
     strings.add(persistenceUnitName);
     strings.add(moduleName);
     strings.add(applicationName);

     String resourceId;

     for (String string : strings) {
         resourceId = getResourceId(string);

         if (resourceId != null) break;
     }

     // use the resourceId ....

That's the high level thought at least.  Lot's of ways that could be  
rolled out with the existing code.  Maybe we just change  
getResourceId() so that the 'String resourceId' is actually a var-arg  
and move it to the end of the signature.  Or something.

Open to ideas.

And just as a side note, you're under no obligation to keep working on  
this, but you're doing great and we can always use the help! :)

-David


Re: Resolve datasource from the application name

Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
Well, assuming that my assumptions are right, I've patched AutoConfig
to use the first WebModule's root context name as a guess to the
DataSource name.
I've attached the patch, as well as complete files for AutoConfig and
AutoConfigPersistenceUnitsTest in
https://issues.apache.org/jira/browse/OPENEJB-1027
As this is my first code contribution to OpenEJB. So, please, correct
me if I did anything wrong...
Feedbacks are appreciated.
--
Luis Fernando Planella Gonzalez

Re: Resolve datasource from the application name

Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
Well, I'm starting to look at this issue today.

Just a small recap: I'm starting to write a patch to automatically
configure data sources in a persistence unit which match the deployed
web application context name. SO, if I have N instances of the same
app in the same server, all I need to do is setup N data sources, each
with the exact name of the web application context for each instance.

>From my previous post, you've answered the "Where is this heuristic
implemented?" question: in AutoConfig.
But I still need the answer for the second one "How to get the current
application name?".
My first idea would be to pick up the first WebModule in
AppModule.getWebModules() (if any) and invoke it's getContextRoot().
Is this ok? If that is the way to go, I would also have to change the
deploy(PersistenceModule) into deploy(AppModule, PersistenceModule),
so that the web applications may be read.

Am I in the right path?

For now, I'm also creating a new test in AutoConfigPersistenceUnitsTest:
    public void testFromWebApp() throws Exception {

        ResourceInfo jta = addDataSource("OrangeWeb",
OrangeDriver.class, "jdbc:orange-web:some:stuff", null);
        ResourceInfo nonJta = addDataSource("OrangeWebUnmanaged",
OrangeDriver.class, "jdbc:orange-web:some:stuff", null);

        PersistenceUnit persistenceUnit = new PersistenceUnit("orange");

        ClassLoader cl = this.getClass().getClassLoader();
        AppModule app = new AppModule(cl, "orange-app");
        app.getPersistenceModules().add(new PersistenceModule("root",
new Persistence(persistenceUnit)));
        WebApp webApp = new WebApp();
        webApp.setMetadataComplete(true);
        app.getWebModules().add(new WebModule(webApp, "orange-web",
cl, "war", "orange-web"));

        // Create app
        AppInfo appInfo = config.configureApplication(app);
        assembler.createApplication(appInfo);

        // Check results
        PersistenceUnitInfo orangeUnit = appInfo.persistenceUnits.get(0);

        assertEquals(jta.id, orangeUnit.jtaDataSource);
        assertEquals(nonJta.id, orangeUnit.nonJtaDataSource);
    }

--
Luis Fernando Planella Gonzalez
lfpg.dev@gmail.com

Re: Resolve datasource from the application name

Posted by Luis Fernando Planella Gonzalez <lf...@gmail.com>.
Thanks David.
I'll arrange my tasks to work on this.
I'll probably report back in a few weeks, as I have other tasks before digging 
into this one.
--
Luis Fernando Planella Gonzalez


Em Sexta-feira 09 Outubro 2009, às 17:07:32, David Blevins escreveu:
> On Sep 30, 2009, at 9:05 AM, Luis Fernando Planella Gonzalez wrote:
> > Hi.
> > I've reported http://issues.apache.org/jira/browse/OPENEJB-1027 in
> > may, which
> > is modifying a bit the heuristics to resolve a data source by
> > matching the app
> > name (web app context in our case) if the data source has not been
> > explicitly
> > specified. This was previously discussed with David Blevings, and he
> > suggested
> > that way.
> >
> > Just to explain the use case: We're migrating cyclos
> > (http://www.cyclos.org ),
> > an open source struts / hibernate application to gwt / ejb3. Many
> > people host
> > several instances of the very same application in the same tomcat,
> > and to keep
> > supporting that it would be necessary to unpack, change every
> > persistence.xml
> > from each and repack each instance again. With this feature
> > implemented, we
> > can just assume there will be a data source with the same name of the
> > application context and we're done, and our users will be happy :)
> >
> > We can try patching ourselves, but we need some guidance on both
> > fundamental
> > questions:
> > * Where is this heuristic implemented?
> > * How to get the current application name?
> 
> The logic is in:
> 
> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-co
> re/src/main/java/org/apache/openejb/config/AutoConfig.java
> 
> There's a pretty good test case for the persistence unit side of that
> logic here:
> 
> http://svn.apache.org/repos/asf/openejb/trunk/openejb3/container/openejb-co
> re/src/test/java/org/apache/openejb/config/AutoConfigPersistenceUnitsTest.j
> ava
> 
> The javadoc was incorrect in some places, so I did a sweep through it
> and fixed it all for you.
> 
> I recommend starting with a copy of the AutoConfigPersistenceUnitsTest
> and expand it to always have two apps and spend a good amount of time
> covering all the scenarios  (what happens when there are more apps
> than data sources, what if there are no data sources that match,
> etc.)   The logic in AutoConfig is a bit of a beast.
> 
> Even if you aren't able to get the logic in, having a really good test
> case that covers more than just the "happy path" would be extremely
> valuable.  For functionality like this I spend at least 60% of the
> time on the test case, so even having that would be a major help
> towards getting this feature in.  The test case also kind of like a
> requirements doc, which is a cool thing when collaborating.
> 
> 
> -David
>