CompManagedBean & @DatasourceDefinition - TOMEE-2053

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

CompManagedBean & @DatasourceDefinition - TOMEE-2053

Svetlin Zarev
Hi Everyone,

What's the purpose of the org.apache.openejb.config.CompManagedBean ? I'm
asking in the context of TOMEE-2053.

I have a @DataSourceDefinition with some attributes which should be
overrriden by ejb-jar.xml. Everithing works great, with the sole exception
of CompManagedBean. It seems that it "aggregates" the annotations from the
other beans, but as it's artificially added to the ejb-jar by openejb, it
does not have an entry in the ejb-jar.xml. Hence when the
AnnotationDeployer processes the DataSourceDefinition annotation, if never
finds an exisitin datasource definition in the ejb-jar.xml for it. This in
turn makes the annotation deployer to add a datasource with wrong
configuration to the AppModule's ejb-jar. So far so good, but later, the
ConvertDataSourceDefinitions deployer collects all datasources from all
JndiConsumers, so it collects the invalid definition as well and adds it to
the AppModule's resources. And this breaks the application startup.

Kind regards,
Svetlin
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Romain Manni-Bucau
Hi Svetlin

this is a way to aggregate the webapp java:comp/env namespace without
handling it too specifically in the code base - at least it comes from that
idea.

We can add it in EjbJar and just skip it at deploy time (we do something
similar already, don't recally exactly where but it is typed enough to know
it is the comp bean).

Does it give you enough input to work on it or do you want some particular
code reference?


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://blog-rmannibucau.rhcloud.com> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2017-06-16 15:10 GMT+02:00 Svetlin Zarev <[hidden email]>:

> Hi Everyone,
>
> What's the purpose of the org.apache.openejb.config.CompManagedBean ? I'm
> asking in the context of TOMEE-2053.
>
> I have a @DataSourceDefinition with some attributes which should be
> overrriden by ejb-jar.xml. Everithing works great, with the sole exception
> of CompManagedBean. It seems that it "aggregates" the annotations from the
> other beans, but as it's artificially added to the ejb-jar by openejb, it
> does not have an entry in the ejb-jar.xml. Hence when the
> AnnotationDeployer processes the DataSourceDefinition annotation, if never
> finds an exisitin datasource definition in the ejb-jar.xml for it. This in
> turn makes the annotation deployer to add a datasource with wrong
> configuration to the AppModule's ejb-jar. So far so good, but later, the
> ConvertDataSourceDefinitions deployer collects all datasources from all
> JndiConsumers, so it collects the invalid definition as well and adds it to
> the AppModule's resources. And this breaks the application startup.
>
> Kind regards,
> Svetlin
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Svetlin Zarev
Hi,

I was thinking about something like
https://github.com/SvetlinZarev/tomee/commit/067fd220e909e89a8c17d90122b0e2158468ece4

What do you think ? Is there a more appropriate place to check for it ?
If it's OK, I can make PR with the fix  + tests.

Thanks,
Svetlin


2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <[hidden email]>:

> Hi Svetlin
>
> this is a way to aggregate the webapp java:comp/env namespace without
> handling it too specifically in the code base - at least it comes from that
> idea.
>
> We can add it in EjbJar and just skip it at deploy time (we do something
> similar already, don't recally exactly where but it is typed enough to know
> it is the comp bean).
>
> Does it give you enough input to work on it or do you want some particular
> code reference?
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
>
> 2017-06-16 15:10 GMT+02:00 Svetlin Zarev <[hidden email]
> >:
>
> > Hi Everyone,
> >
> > What's the purpose of the org.apache.openejb.config.CompManagedBean ?
> I'm
> > asking in the context of TOMEE-2053.
> >
> > I have a @DataSourceDefinition with some attributes which should be
> > overrriden by ejb-jar.xml. Everithing works great, with the sole
> exception
> > of CompManagedBean. It seems that it "aggregates" the annotations from
> the
> > other beans, but as it's artificially added to the ejb-jar by openejb, it
> > does not have an entry in the ejb-jar.xml. Hence when the
> > AnnotationDeployer processes the DataSourceDefinition annotation, if
> never
> > finds an exisitin datasource definition in the ejb-jar.xml for it. This
> in
> > turn makes the annotation deployer to add a datasource with wrong
> > configuration to the AppModule's ejb-jar. So far so good, but later, the
> > ConvertDataSourceDefinitions deployer collects all datasources from all
> > JndiConsumers, so it collects the invalid definition as well and adds it
> to
> > the AppModule's resources. And this breaks the application startup.
> >
> > Kind regards,
> > Svetlin
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Romain Manni-Bucau
Hmm, if you inject such a datasource in a servlet does it work? Don't think
so - tests are the thing to start with when editing this code, saying that
by experience if you get me ;).

So concretely testing the type like that is good but it shouldn't break web
component injections.


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://blog-rmannibucau.rhcloud.com> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2017-06-16 15:27 GMT+02:00 Svetlin Zarev <[hidden email]>:

> Hi,
>
> I was thinking about something like
> https://github.com/SvetlinZarev/tomee/commit/
> 067fd220e909e89a8c17d90122b0e2158468ece4
>
> What do you think ? Is there a more appropriate place to check for it ?
> If it's OK, I can make PR with the fix  + tests.
>
> Thanks,
> Svetlin
>
>
> 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
>
> > Hi Svetlin
> >
> > this is a way to aggregate the webapp java:comp/env namespace without
> > handling it too specifically in the code base - at least it comes from
> that
> > idea.
> >
> > We can add it in EjbJar and just skip it at deploy time (we do something
> > similar already, don't recally exactly where but it is typed enough to
> know
> > it is the comp bean).
> >
> > Does it give you enough input to work on it or do you want some
> particular
> > code reference?
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >
> > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> > >:
> >
> > > Hi Everyone,
> > >
> > > What's the purpose of the org.apache.openejb.config.CompManagedBean ?
> > I'm
> > > asking in the context of TOMEE-2053.
> > >
> > > I have a @DataSourceDefinition with some attributes which should be
> > > overrriden by ejb-jar.xml. Everithing works great, with the sole
> > exception
> > > of CompManagedBean. It seems that it "aggregates" the annotations from
> > the
> > > other beans, but as it's artificially added to the ejb-jar by openejb,
> it
> > > does not have an entry in the ejb-jar.xml. Hence when the
> > > AnnotationDeployer processes the DataSourceDefinition annotation, if
> > never
> > > finds an exisitin datasource definition in the ejb-jar.xml for it. This
> > in
> > > turn makes the annotation deployer to add a datasource with wrong
> > > configuration to the AppModule's ejb-jar. So far so good, but later,
> the
> > > ConvertDataSourceDefinitions deployer collects all datasources from all
> > > JndiConsumers, so it collects the invalid definition as well and adds
> it
> > to
> > > the AppModule's resources. And this breaks the application startup.
> > >
> > > Kind regards,
> > > Svetlin
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Svetlin Zarev
I'm back with tests:) :
https://github.com/apache/tomee/compare/master...SvetlinZarev:ds_def?expand=1

Unless I misunderstood you, it seems that DS injection in servlet works
after the fix and does not before it (well, it injects it, but with wrong
config).
The reason it works is that the ConvertDataSourceDefinitions deployer only
processes the DataSource definitions by converting them to Resource objects
which are not associated with specific component, and since the
CompManagedBean does not have its own, unique definitions, if we exclude it
from processing in that specific deployer we won't lose anything, because
data sources with the same IDs ( and with correct config !) will be pulled
from the JndiConsumers that actually provide them.

So to summarize, the flow before the proposed fix is:
1. Collect all JndiConsumers
2. MyBean -> provide DataSource with correct config
3. CompManagedBean -> overwrite MyBean's datasource, with one with bad
config
4. We end up with one resource with bad config

And after the fix it becomes:
1. Collect all JndiConsumers
2. MyBean -> provide DataSource with correct config
3. We end up with one resource with good config

I hope I don't miss anything :) May you give more details about your idea ?

Kind regards,
Svetlin


2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <[hidden email]>:

> Hmm, if you inject such a datasource in a servlet does it work? Don't think
> so - tests are the thing to start with when editing this code, saying that
> by experience if you get me ;).
>
> So concretely testing the type like that is good but it shouldn't break web
> component injections.
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
>
> 2017-06-16 15:27 GMT+02:00 Svetlin Zarev <[hidden email]
> >:
>
> > Hi,
> >
> > I was thinking about something like
> > https://github.com/SvetlinZarev/tomee/commit/
> > 067fd220e909e89a8c17d90122b0e2158468ece4
> >
> > What do you think ? Is there a more appropriate place to check for it ?
> > If it's OK, I can make PR with the fix  + tests.
> >
> > Thanks,
> > Svetlin
> >
> >
> > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
> >
> > > Hi Svetlin
> > >
> > > this is a way to aggregate the webapp java:comp/env namespace without
> > > handling it too specifically in the code base - at least it comes from
> > that
> > > idea.
> > >
> > > We can add it in EjbJar and just skip it at deploy time (we do
> something
> > > similar already, don't recally exactly where but it is typed enough to
> > know
> > > it is the comp bean).
> > >
> > > Does it give you enough input to work on it or do you want some
> > particular
> > > code reference?
> > >
> > >
> > > Romain Manni-Bucau
> > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > rmannibucau> |
> > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > >
> > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> > com
> > > >:
> > >
> > > > Hi Everyone,
> > > >
> > > > What's the purpose of the org.apache.openejb.config.CompManagedBean
> ?
> > > I'm
> > > > asking in the context of TOMEE-2053.
> > > >
> > > > I have a @DataSourceDefinition with some attributes which should be
> > > > overrriden by ejb-jar.xml. Everithing works great, with the sole
> > > exception
> > > > of CompManagedBean. It seems that it "aggregates" the annotations
> from
> > > the
> > > > other beans, but as it's artificially added to the ejb-jar by
> openejb,
> > it
> > > > does not have an entry in the ejb-jar.xml. Hence when the
> > > > AnnotationDeployer processes the DataSourceDefinition annotation, if
> > > never
> > > > finds an exisitin datasource definition in the ejb-jar.xml for it.
> This
> > > in
> > > > turn makes the annotation deployer to add a datasource with wrong
> > > > configuration to the AppModule's ejb-jar. So far so good, but later,
> > the
> > > > ConvertDataSourceDefinitions deployer collects all datasources from
> all
> > > > JndiConsumers, so it collects the invalid definition as well and adds
> > it
> > > to
> > > > the AppModule's resources. And this breaks the application startup.
> > > >
> > > > Kind regards,
> > > > Svetlin
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Romain Manni-Bucau
This looks good, while the merge deployer still makes available the
resource to comp it works :)

Think you need to fix details on your PR like headers etc but looks ok to
merge once the build pass.

Side note: by curiosity, did you check jms resource as well? it should be
close ot datasources.


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://blog-rmannibucau.rhcloud.com> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2017-06-16 19:56 GMT+02:00 Svetlin Zarev <[hidden email]>:

> I'm back with tests:) :
> https://github.com/apache/tomee/compare/master...
> SvetlinZarev:ds_def?expand=1
>
> Unless I misunderstood you, it seems that DS injection in servlet works
> after the fix and does not before it (well, it injects it, but with wrong
> config).
> The reason it works is that the ConvertDataSourceDefinitions deployer only
> processes the DataSource definitions by converting them to Resource objects
> which are not associated with specific component, and since the
> CompManagedBean does not have its own, unique definitions, if we exclude it
> from processing in that specific deployer we won't lose anything, because
> data sources with the same IDs ( and with correct config !) will be pulled
> from the JndiConsumers that actually provide them.
>
> So to summarize, the flow before the proposed fix is:
> 1. Collect all JndiConsumers
> 2. MyBean -> provide DataSource with correct config
> 3. CompManagedBean -> overwrite MyBean's datasource, with one with bad
> config
> 4. We end up with one resource with bad config
>
> And after the fix it becomes:
> 1. Collect all JndiConsumers
> 2. MyBean -> provide DataSource with correct config
> 3. We end up with one resource with good config
>
> I hope I don't miss anything :) May you give more details about your idea ?
>
> Kind regards,
> Svetlin
>
>
> 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
>
> > Hmm, if you inject such a datasource in a servlet does it work? Don't
> think
> > so - tests are the thing to start with when editing this code, saying
> that
> > by experience if you get me ;).
> >
> > So concretely testing the type like that is good but it shouldn't break
> web
> > component injections.
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >
> > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> > >:
> >
> > > Hi,
> > >
> > > I was thinking about something like
> > > https://github.com/SvetlinZarev/tomee/commit/
> > > 067fd220e909e89a8c17d90122b0e2158468ece4
> > >
> > > What do you think ? Is there a more appropriate place to check for it ?
> > > If it's OK, I can make PR with the fix  + tests.
> > >
> > > Thanks,
> > > Svetlin
> > >
> > >
> > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
> > >
> > > > Hi Svetlin
> > > >
> > > > this is a way to aggregate the webapp java:comp/env namespace without
> > > > handling it too specifically in the code base - at least it comes
> from
> > > that
> > > > idea.
> > > >
> > > > We can add it in EjbJar and just skip it at deploy time (we do
> > something
> > > > similar already, don't recally exactly where but it is typed enough
> to
> > > know
> > > > it is the comp bean).
> > > >
> > > > Does it give you enough input to work on it or do you want some
> > > particular
> > > > code reference?
> > > >
> > > >
> > > > Romain Manni-Bucau
> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > > rmannibucau> |
> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > >
> > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
> <svetlin.angelov.zarev@gmail.
> > > com
> > > > >:
> > > >
> > > > > Hi Everyone,
> > > > >
> > > > > What's the purpose of the org.apache.openejb.config.
> CompManagedBean
> > ?
> > > > I'm
> > > > > asking in the context of TOMEE-2053.
> > > > >
> > > > > I have a @DataSourceDefinition with some attributes which should be
> > > > > overrriden by ejb-jar.xml. Everithing works great, with the sole
> > > > exception
> > > > > of CompManagedBean. It seems that it "aggregates" the annotations
> > from
> > > > the
> > > > > other beans, but as it's artificially added to the ejb-jar by
> > openejb,
> > > it
> > > > > does not have an entry in the ejb-jar.xml. Hence when the
> > > > > AnnotationDeployer processes the DataSourceDefinition annotation,
> if
> > > > never
> > > > > finds an exisitin datasource definition in the ejb-jar.xml for it.
> > This
> > > > in
> > > > > turn makes the annotation deployer to add a datasource with wrong
> > > > > configuration to the AppModule's ejb-jar. So far so good, but
> later,
> > > the
> > > > > ConvertDataSourceDefinitions deployer collects all datasources from
> > all
> > > > > JndiConsumers, so it collects the invalid definition as well and
> adds
> > > it
> > > > to
> > > > > the AppModule's resources. And this breaks the application startup.
> > > > >
> > > > > Kind regards,
> > > > > Svetlin
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Svetlin Zarev
I've fixed the license headers. Here is the PR:
https://github.com/apache/tomee/pull/73

I didn't check JMS, only WepProfile specs.

2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <[hidden email]>:

> This looks good, while the merge deployer still makes available the
> resource to comp it works :)
>
> Think you need to fix details on your PR like headers etc but looks ok to
> merge once the build pass.
>
> Side note: by curiosity, did you check jms resource as well? it should be
> close ot datasources.
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
>
> 2017-06-16 19:56 GMT+02:00 Svetlin Zarev <[hidden email]
> >:
>
> > I'm back with tests:) :
> > https://github.com/apache/tomee/compare/master...
> > SvetlinZarev:ds_def?expand=1
> >
> > Unless I misunderstood you, it seems that DS injection in servlet works
> > after the fix and does not before it (well, it injects it, but with wrong
> > config).
> > The reason it works is that the ConvertDataSourceDefinitions deployer
> only
> > processes the DataSource definitions by converting them to Resource
> objects
> > which are not associated with specific component, and since the
> > CompManagedBean does not have its own, unique definitions, if we exclude
> it
> > from processing in that specific deployer we won't lose anything, because
> > data sources with the same IDs ( and with correct config !) will be
> pulled
> > from the JndiConsumers that actually provide them.
> >
> > So to summarize, the flow before the proposed fix is:
> > 1. Collect all JndiConsumers
> > 2. MyBean -> provide DataSource with correct config
> > 3. CompManagedBean -> overwrite MyBean's datasource, with one with bad
> > config
> > 4. We end up with one resource with bad config
> >
> > And after the fix it becomes:
> > 1. Collect all JndiConsumers
> > 2. MyBean -> provide DataSource with correct config
> > 3. We end up with one resource with good config
> >
> > I hope I don't miss anything :) May you give more details about your
> idea ?
> >
> > Kind regards,
> > Svetlin
> >
> >
> > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
> >
> > > Hmm, if you inject such a datasource in a servlet does it work? Don't
> > think
> > > so - tests are the thing to start with when editing this code, saying
> > that
> > > by experience if you get me ;).
> > >
> > > So concretely testing the type like that is good but it shouldn't break
> > web
> > > component injections.
> > >
> > >
> > > Romain Manni-Bucau
> > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > rmannibucau> |
> > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > >
> > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> > com
> > > >:
> > >
> > > > Hi,
> > > >
> > > > I was thinking about something like
> > > > https://github.com/SvetlinZarev/tomee/commit/
> > > > 067fd220e909e89a8c17d90122b0e2158468ece4
> > > >
> > > > What do you think ? Is there a more appropriate place to check for
> it ?
> > > > If it's OK, I can make PR with the fix  + tests.
> > > >
> > > > Thanks,
> > > > Svetlin
> > > >
> > > >
> > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <[hidden email]
> >:
> > > >
> > > > > Hi Svetlin
> > > > >
> > > > > this is a way to aggregate the webapp java:comp/env namespace
> without
> > > > > handling it too specifically in the code base - at least it comes
> > from
> > > > that
> > > > > idea.
> > > > >
> > > > > We can add it in EjbJar and just skip it at deploy time (we do
> > > something
> > > > > similar already, don't recally exactly where but it is typed enough
> > to
> > > > know
> > > > > it is the comp bean).
> > > > >
> > > > > Does it give you enough input to work on it or do you want some
> > > > particular
> > > > > code reference?
> > > > >
> > > > >
> > > > > Romain Manni-Bucau
> > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > > > rmannibucau> |
> > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> Factory
> > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > > >
> > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
> > <svetlin.angelov.zarev@gmail.
> > > > com
> > > > > >:
> > > > >
> > > > > > Hi Everyone,
> > > > > >
> > > > > > What's the purpose of the org.apache.openejb.config.
> > CompManagedBean
> > > ?
> > > > > I'm
> > > > > > asking in the context of TOMEE-2053.
> > > > > >
> > > > > > I have a @DataSourceDefinition with some attributes which should
> be
> > > > > > overrriden by ejb-jar.xml. Everithing works great, with the sole
> > > > > exception
> > > > > > of CompManagedBean. It seems that it "aggregates" the annotations
> > > from
> > > > > the
> > > > > > other beans, but as it's artificially added to the ejb-jar by
> > > openejb,
> > > > it
> > > > > > does not have an entry in the ejb-jar.xml. Hence when the
> > > > > > AnnotationDeployer processes the DataSourceDefinition annotation,
> > if
> > > > > never
> > > > > > finds an exisitin datasource definition in the ejb-jar.xml for
> it.
> > > This
> > > > > in
> > > > > > turn makes the annotation deployer to add a datasource with wrong
> > > > > > configuration to the AppModule's ejb-jar. So far so good, but
> > later,
> > > > the
> > > > > > ConvertDataSourceDefinitions deployer collects all datasources
> from
> > > all
> > > > > > JndiConsumers, so it collects the invalid definition as well and
> > adds
> > > > it
> > > > > to
> > > > > > the AppModule's resources. And this breaks the application
> startup.
> > > > > >
> > > > > > Kind regards,
> > > > > > Svetlin
> > > > > >
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Romain Manni-Bucau
applied, thanks!


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://blog-rmannibucau.rhcloud.com> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2017-06-16 20:33 GMT+02:00 Svetlin Zarev <[hidden email]>:

> I've fixed the license headers. Here is the PR:
> https://github.com/apache/tomee/pull/73
>
> I didn't check JMS, only WepProfile specs.
>
> 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
>
> > This looks good, while the merge deployer still makes available the
> > resource to comp it works :)
> >
> > Think you need to fix details on your PR like headers etc but looks ok to
> > merge once the build pass.
> >
> > Side note: by curiosity, did you check jms resource as well? it should be
> > close ot datasources.
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >
> > 2017-06-16 19:56 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> > >:
> >
> > > I'm back with tests:) :
> > > https://github.com/apache/tomee/compare/master...
> > > SvetlinZarev:ds_def?expand=1
> > >
> > > Unless I misunderstood you, it seems that DS injection in servlet works
> > > after the fix and does not before it (well, it injects it, but with
> wrong
> > > config).
> > > The reason it works is that the ConvertDataSourceDefinitions deployer
> > only
> > > processes the DataSource definitions by converting them to Resource
> > objects
> > > which are not associated with specific component, and since the
> > > CompManagedBean does not have its own, unique definitions, if we
> exclude
> > it
> > > from processing in that specific deployer we won't lose anything,
> because
> > > data sources with the same IDs ( and with correct config !) will be
> > pulled
> > > from the JndiConsumers that actually provide them.
> > >
> > > So to summarize, the flow before the proposed fix is:
> > > 1. Collect all JndiConsumers
> > > 2. MyBean -> provide DataSource with correct config
> > > 3. CompManagedBean -> overwrite MyBean's datasource, with one with bad
> > > config
> > > 4. We end up with one resource with bad config
> > >
> > > And after the fix it becomes:
> > > 1. Collect all JndiConsumers
> > > 2. MyBean -> provide DataSource with correct config
> > > 3. We end up with one resource with good config
> > >
> > > I hope I don't miss anything :) May you give more details about your
> > idea ?
> > >
> > > Kind regards,
> > > Svetlin
> > >
> > >
> > > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
> > >
> > > > Hmm, if you inject such a datasource in a servlet does it work? Don't
> > > think
> > > > so - tests are the thing to start with when editing this code, saying
> > > that
> > > > by experience if you get me ;).
> > > >
> > > > So concretely testing the type like that is good but it shouldn't
> break
> > > web
> > > > component injections.
> > > >
> > > >
> > > > Romain Manni-Bucau
> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > > rmannibucau> |
> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > >
> > > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev
> <svetlin.angelov.zarev@gmail.
> > > com
> > > > >:
> > > >
> > > > > Hi,
> > > > >
> > > > > I was thinking about something like
> > > > > https://github.com/SvetlinZarev/tomee/commit/
> > > > > 067fd220e909e89a8c17d90122b0e2158468ece4
> > > > >
> > > > > What do you think ? Is there a more appropriate place to check for
> > it ?
> > > > > If it's OK, I can make PR with the fix  + tests.
> > > > >
> > > > > Thanks,
> > > > > Svetlin
> > > > >
> > > > >
> > > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <
> [hidden email]
> > >:
> > > > >
> > > > > > Hi Svetlin
> > > > > >
> > > > > > this is a way to aggregate the webapp java:comp/env namespace
> > without
> > > > > > handling it too specifically in the code base - at least it comes
> > > from
> > > > > that
> > > > > > idea.
> > > > > >
> > > > > > We can add it in EjbJar and just skip it at deploy time (we do
> > > > something
> > > > > > similar already, don't recally exactly where but it is typed
> enough
> > > to
> > > > > know
> > > > > > it is the comp bean).
> > > > > >
> > > > > > Does it give you enough input to work on it or do you want some
> > > > > particular
> > > > > > code reference?
> > > > > >
> > > > > >
> > > > > > Romain Manni-Bucau
> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > > > > > rmannibucau> |
> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> > Factory
> > > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > > > >
> > > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
> > > <svetlin.angelov.zarev@gmail.
> > > > > com
> > > > > > >:
> > > > > >
> > > > > > > Hi Everyone,
> > > > > > >
> > > > > > > What's the purpose of the org.apache.openejb.config.
> > > CompManagedBean
> > > > ?
> > > > > > I'm
> > > > > > > asking in the context of TOMEE-2053.
> > > > > > >
> > > > > > > I have a @DataSourceDefinition with some attributes which
> should
> > be
> > > > > > > overrriden by ejb-jar.xml. Everithing works great, with the
> sole
> > > > > > exception
> > > > > > > of CompManagedBean. It seems that it "aggregates" the
> annotations
> > > > from
> > > > > > the
> > > > > > > other beans, but as it's artificially added to the ejb-jar by
> > > > openejb,
> > > > > it
> > > > > > > does not have an entry in the ejb-jar.xml. Hence when the
> > > > > > > AnnotationDeployer processes the DataSourceDefinition
> annotation,
> > > if
> > > > > > never
> > > > > > > finds an exisitin datasource definition in the ejb-jar.xml for
> > it.
> > > > This
> > > > > > in
> > > > > > > turn makes the annotation deployer to add a datasource with
> wrong
> > > > > > > configuration to the AppModule's ejb-jar. So far so good, but
> > > later,
> > > > > the
> > > > > > > ConvertDataSourceDefinitions deployer collects all datasources
> > from
> > > > all
> > > > > > > JndiConsumers, so it collects the invalid definition as well
> and
> > > adds
> > > > > it
> > > > > > to
> > > > > > > the AppModule's resources. And this breaks the application
> > startup.
> > > > > > >
> > > > > > > Kind regards,
> > > > > > > Svetlin
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Romain Manni-Bucau
Trunk seems to have an issue with this in the test
XADataSourceDefinitionTest

Want to have a look?


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://blog-rmannibucau.rhcloud.com> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2017-06-16 20:46 GMT+02:00 Romain Manni-Bucau <[hidden email]>:

> applied, thanks!
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github
> <https://github.com/rmannibucau> | LinkedIn
> <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
>
> 2017-06-16 20:33 GMT+02:00 Svetlin Zarev <[hidden email]>
> :
>
>> I've fixed the license headers. Here is the PR:
>> https://github.com/apache/tomee/pull/73
>>
>> I didn't check JMS, only WepProfile specs.
>>
>> 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
>>
>> > This looks good, while the merge deployer still makes available the
>> > resource to comp it works :)
>> >
>> > Think you need to fix details on your PR like headers etc but looks ok
>> to
>> > merge once the build pass.
>> >
>> > Side note: by curiosity, did you check jms resource as well? it should
>> be
>> > close ot datasources.
>> >
>> >
>> > Romain Manni-Bucau
>> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
>> > rmannibucau> |
>> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
>> > <https://javaeefactory-rmannibucau.rhcloud.com>
>> >
>> > 2017-06-16 19:56 GMT+02:00 Svetlin Zarev <[hidden email]
>> om
>> > >:
>> >
>> > > I'm back with tests:) :
>> > > https://github.com/apache/tomee/compare/master...
>> > > SvetlinZarev:ds_def?expand=1
>> > >
>> > > Unless I misunderstood you, it seems that DS injection in servlet
>> works
>> > > after the fix and does not before it (well, it injects it, but with
>> wrong
>> > > config).
>> > > The reason it works is that the ConvertDataSourceDefinitions deployer
>> > only
>> > > processes the DataSource definitions by converting them to Resource
>> > objects
>> > > which are not associated with specific component, and since the
>> > > CompManagedBean does not have its own, unique definitions, if we
>> exclude
>> > it
>> > > from processing in that specific deployer we won't lose anything,
>> because
>> > > data sources with the same IDs ( and with correct config !) will be
>> > pulled
>> > > from the JndiConsumers that actually provide them.
>> > >
>> > > So to summarize, the flow before the proposed fix is:
>> > > 1. Collect all JndiConsumers
>> > > 2. MyBean -> provide DataSource with correct config
>> > > 3. CompManagedBean -> overwrite MyBean's datasource, with one with bad
>> > > config
>> > > 4. We end up with one resource with bad config
>> > >
>> > > And after the fix it becomes:
>> > > 1. Collect all JndiConsumers
>> > > 2. MyBean -> provide DataSource with correct config
>> > > 3. We end up with one resource with good config
>> > >
>> > > I hope I don't miss anything :) May you give more details about your
>> > idea ?
>> > >
>> > > Kind regards,
>> > > Svetlin
>> > >
>> > >
>> > > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <[hidden email]
>> >:
>> > >
>> > > > Hmm, if you inject such a datasource in a servlet does it work?
>> Don't
>> > > think
>> > > > so - tests are the thing to start with when editing this code,
>> saying
>> > > that
>> > > > by experience if you get me ;).
>> > > >
>> > > > So concretely testing the type like that is good but it shouldn't
>> break
>> > > web
>> > > > component injections.
>> > > >
>> > > >
>> > > > Romain Manni-Bucau
>> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
>> > > > rmannibucau> |
>> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
>> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
>> > > >
>> > > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev
>> <svetlin.angelov.zarev@gmail.
>> > > com
>> > > > >:
>> > > >
>> > > > > Hi,
>> > > > >
>> > > > > I was thinking about something like
>> > > > > https://github.com/SvetlinZarev/tomee/commit/
>> > > > > 067fd220e909e89a8c17d90122b0e2158468ece4
>> > > > >
>> > > > > What do you think ? Is there a more appropriate place to check for
>> > it ?
>> > > > > If it's OK, I can make PR with the fix  + tests.
>> > > > >
>> > > > > Thanks,
>> > > > > Svetlin
>> > > > >
>> > > > >
>> > > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <
>> [hidden email]
>> > >:
>> > > > >
>> > > > > > Hi Svetlin
>> > > > > >
>> > > > > > this is a way to aggregate the webapp java:comp/env namespace
>> > without
>> > > > > > handling it too specifically in the code base - at least it
>> comes
>> > > from
>> > > > > that
>> > > > > > idea.
>> > > > > >
>> > > > > > We can add it in EjbJar and just skip it at deploy time (we do
>> > > > something
>> > > > > > similar already, don't recally exactly where but it is typed
>> enough
>> > > to
>> > > > > know
>> > > > > > it is the comp bean).
>> > > > > >
>> > > > > > Does it give you enough input to work on it or do you want some
>> > > > > particular
>> > > > > > code reference?
>> > > > > >
>> > > > > >
>> > > > > > Romain Manni-Bucau
>> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> > > > > > <http://rmannibucau.wordpress.com> | Github <
>> https://github.com/
>> > > > > > rmannibucau> |
>> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
>> > Factory
>> > > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
>> > > > > >
>> > > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
>> > > <svetlin.angelov.zarev@gmail.
>> > > > > com
>> > > > > > >:
>> > > > > >
>> > > > > > > Hi Everyone,
>> > > > > > >
>> > > > > > > What's the purpose of the org.apache.openejb.config.
>> > > CompManagedBean
>> > > > ?
>> > > > > > I'm
>> > > > > > > asking in the context of TOMEE-2053.
>> > > > > > >
>> > > > > > > I have a @DataSourceDefinition with some attributes which
>> should
>> > be
>> > > > > > > overrriden by ejb-jar.xml. Everithing works great, with the
>> sole
>> > > > > > exception
>> > > > > > > of CompManagedBean. It seems that it "aggregates" the
>> annotations
>> > > > from
>> > > > > > the
>> > > > > > > other beans, but as it's artificially added to the ejb-jar by
>> > > > openejb,
>> > > > > it
>> > > > > > > does not have an entry in the ejb-jar.xml. Hence when the
>> > > > > > > AnnotationDeployer processes the DataSourceDefinition
>> annotation,
>> > > if
>> > > > > > never
>> > > > > > > finds an exisitin datasource definition in the ejb-jar.xml for
>> > it.
>> > > > This
>> > > > > > in
>> > > > > > > turn makes the annotation deployer to add a datasource with
>> wrong
>> > > > > > > configuration to the AppModule's ejb-jar. So far so good, but
>> > > later,
>> > > > > the
>> > > > > > > ConvertDataSourceDefinitions deployer collects all datasources
>> > from
>> > > > all
>> > > > > > > JndiConsumers, so it collects the invalid definition as well
>> and
>> > > adds
>> > > > > it
>> > > > > > to
>> > > > > > > the AppModule's resources. And this breaks the application
>> > startup.
>> > > > > > >
>> > > > > > > Kind regards,
>> > > > > > > Svetlin
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Svetlin Zarev
The test fails, because the @DataSourceDefinition is used on a POJO - i.e.
it's not a JndiConsumer, hence the ConvertDataSourceDefinitions deployer
cannot know that such definition exists. I'll have to check the spec if it
mentions which classes can be annotated with @DataSourceDefinition, but
either way it would be easy to fix. I'll take a look tomorrow :)

Kind regards,
Svetlin

2017-06-16 22:37 GMT+03:00 Romain Manni-Bucau <[hidden email]>:

> Trunk seems to have an issue with this in the test
> XADataSourceDefinitionTest
>
> Want to have a look?
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
>
> 2017-06-16 20:46 GMT+02:00 Romain Manni-Bucau <[hidden email]>:
>
> > applied, thanks!
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github
> > <https://github.com/rmannibucau> | LinkedIn
> > <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >
> > 2017-06-16 20:33 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com>
> > :
> >
> >> I've fixed the license headers. Here is the PR:
> >> https://github.com/apache/tomee/pull/73
> >>
> >> I didn't check JMS, only WepProfile specs.
> >>
> >> 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
> >>
> >> > This looks good, while the merge deployer still makes available the
> >> > resource to comp it works :)
> >> >
> >> > Think you need to fix details on your PR like headers etc but looks ok
> >> to
> >> > merge once the build pass.
> >> >
> >> > Side note: by curiosity, did you check jms resource as well? it should
> >> be
> >> > close ot datasources.
> >> >
> >> >
> >> > Romain Manni-Bucau
> >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> >> > rmannibucau> |
> >> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> >> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >> >
> >> > 2017-06-16 19:56 GMT+02:00 Svetlin Zarev
> <[hidden email]
> >> om
> >> > >:
> >> >
> >> > > I'm back with tests:) :
> >> > > https://github.com/apache/tomee/compare/master...
> >> > > SvetlinZarev:ds_def?expand=1
> >> > >
> >> > > Unless I misunderstood you, it seems that DS injection in servlet
> >> works
> >> > > after the fix and does not before it (well, it injects it, but with
> >> wrong
> >> > > config).
> >> > > The reason it works is that the ConvertDataSourceDefinitions
> deployer
> >> > only
> >> > > processes the DataSource definitions by converting them to Resource
> >> > objects
> >> > > which are not associated with specific component, and since the
> >> > > CompManagedBean does not have its own, unique definitions, if we
> >> exclude
> >> > it
> >> > > from processing in that specific deployer we won't lose anything,
> >> because
> >> > > data sources with the same IDs ( and with correct config !) will be
> >> > pulled
> >> > > from the JndiConsumers that actually provide them.
> >> > >
> >> > > So to summarize, the flow before the proposed fix is:
> >> > > 1. Collect all JndiConsumers
> >> > > 2. MyBean -> provide DataSource with correct config
> >> > > 3. CompManagedBean -> overwrite MyBean's datasource, with one with
> bad
> >> > > config
> >> > > 4. We end up with one resource with bad config
> >> > >
> >> > > And after the fix it becomes:
> >> > > 1. Collect all JndiConsumers
> >> > > 2. MyBean -> provide DataSource with correct config
> >> > > 3. We end up with one resource with good config
> >> > >
> >> > > I hope I don't miss anything :) May you give more details about your
> >> > idea ?
> >> > >
> >> > > Kind regards,
> >> > > Svetlin
> >> > >
> >> > >
> >> > > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <
> [hidden email]
> >> >:
> >> > >
> >> > > > Hmm, if you inject such a datasource in a servlet does it work?
> >> Don't
> >> > > think
> >> > > > so - tests are the thing to start with when editing this code,
> >> saying
> >> > > that
> >> > > > by experience if you get me ;).
> >> > > >
> >> > > > So concretely testing the type like that is good but it shouldn't
> >> break
> >> > > web
> >> > > > component injections.
> >> > > >
> >> > > >
> >> > > > Romain Manni-Bucau
> >> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> >> > > > rmannibucau> |
> >> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> Factory
> >> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> >> > > >
> >> > > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev
> >> <svetlin.angelov.zarev@gmail.
> >> > > com
> >> > > > >:
> >> > > >
> >> > > > > Hi,
> >> > > > >
> >> > > > > I was thinking about something like
> >> > > > > https://github.com/SvetlinZarev/tomee/commit/
> >> > > > > 067fd220e909e89a8c17d90122b0e2158468ece4
> >> > > > >
> >> > > > > What do you think ? Is there a more appropriate place to check
> for
> >> > it ?
> >> > > > > If it's OK, I can make PR with the fix  + tests.
> >> > > > >
> >> > > > > Thanks,
> >> > > > > Svetlin
> >> > > > >
> >> > > > >
> >> > > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <
> >> [hidden email]
> >> > >:
> >> > > > >
> >> > > > > > Hi Svetlin
> >> > > > > >
> >> > > > > > this is a way to aggregate the webapp java:comp/env namespace
> >> > without
> >> > > > > > handling it too specifically in the code base - at least it
> >> comes
> >> > > from
> >> > > > > that
> >> > > > > > idea.
> >> > > > > >
> >> > > > > > We can add it in EjbJar and just skip it at deploy time (we do
> >> > > > something
> >> > > > > > similar already, don't recally exactly where but it is typed
> >> enough
> >> > > to
> >> > > > > know
> >> > > > > > it is the comp bean).
> >> > > > > >
> >> > > > > > Does it give you enough input to work on it or do you want
> some
> >> > > > > particular
> >> > > > > > code reference?
> >> > > > > >
> >> > > > > >
> >> > > > > > Romain Manni-Bucau
> >> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> > > > > > <http://rmannibucau.wordpress.com> | Github <
> >> https://github.com/
> >> > > > > > rmannibucau> |
> >> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> >> > Factory
> >> > > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> >> > > > > >
> >> > > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
> >> > > <svetlin.angelov.zarev@gmail.
> >> > > > > com
> >> > > > > > >:
> >> > > > > >
> >> > > > > > > Hi Everyone,
> >> > > > > > >
> >> > > > > > > What's the purpose of the org.apache.openejb.config.
> >> > > CompManagedBean
> >> > > > ?
> >> > > > > > I'm
> >> > > > > > > asking in the context of TOMEE-2053.
> >> > > > > > >
> >> > > > > > > I have a @DataSourceDefinition with some attributes which
> >> should
> >> > be
> >> > > > > > > overrriden by ejb-jar.xml. Everithing works great, with the
> >> sole
> >> > > > > > exception
> >> > > > > > > of CompManagedBean. It seems that it "aggregates" the
> >> annotations
> >> > > > from
> >> > > > > > the
> >> > > > > > > other beans, but as it's artificially added to the ejb-jar
> by
> >> > > > openejb,
> >> > > > > it
> >> > > > > > > does not have an entry in the ejb-jar.xml. Hence when the
> >> > > > > > > AnnotationDeployer processes the DataSourceDefinition
> >> annotation,
> >> > > if
> >> > > > > > never
> >> > > > > > > finds an exisitin datasource definition in the ejb-jar.xml
> for
> >> > it.
> >> > > > This
> >> > > > > > in
> >> > > > > > > turn makes the annotation deployer to add a datasource with
> >> wrong
> >> > > > > > > configuration to the AppModule's ejb-jar. So far so good,
> but
> >> > > later,
> >> > > > > the
> >> > > > > > > ConvertDataSourceDefinitions deployer collects all
> datasources
> >> > from
> >> > > > all
> >> > > > > > > JndiConsumers, so it collects the invalid definition as well
> >> and
> >> > > adds
> >> > > > > it
> >> > > > > > to
> >> > > > > > > the AppModule's resources. And this breaks the application
> >> > startup.
> >> > > > > > >
> >> > > > > > > Kind regards,
> >> > > > > > > Svetlin
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Svetlin Zarev
Hi,

Here it is: https://github.com/apache/tomee/pull/74

Kind regards,
Svetlin


2017-06-16 23:44 GMT+03:00 Svetlin Zarev <[hidden email]>:

> The test fails, because the @DataSourceDefinition is used on a POJO - i.e.
> it's not a JndiConsumer, hence the ConvertDataSourceDefinitions deployer
> cannot know that such definition exists. I'll have to check the spec if it
> mentions which classes can be annotated with @DataSourceDefinition, but
> either way it would be easy to fix. I'll take a look tomorrow :)
>
> Kind regards,
> Svetlin
>
> 2017-06-16 22:37 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
>
>> Trunk seems to have an issue with this in the test
>> XADataSourceDefinitionTest
>>
>> Want to have a look?
>>
>>
>> Romain Manni-Bucau
>> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> <http://rmannibucau.wordpress.com> | Github <
>> https://github.com/rmannibucau> |
>> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
>> <https://javaeefactory-rmannibucau.rhcloud.com>
>>
>> 2017-06-16 20:46 GMT+02:00 Romain Manni-Bucau <[hidden email]>:
>>
>> > applied, thanks!
>> >
>> >
>> > Romain Manni-Bucau
>> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> > <http://rmannibucau.wordpress.com> | Github
>> > <https://github.com/rmannibucau> | LinkedIn
>> > <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
>> > <https://javaeefactory-rmannibucau.rhcloud.com>
>> >
>> > 2017-06-16 20:33 GMT+02:00 Svetlin Zarev <[hidden email]
>> om>
>> > :
>> >
>> >> I've fixed the license headers. Here is the PR:
>> >> https://github.com/apache/tomee/pull/73
>> >>
>> >> I didn't check JMS, only WepProfile specs.
>> >>
>> >> 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
>> >>
>> >> > This looks good, while the merge deployer still makes available the
>> >> > resource to comp it works :)
>> >> >
>> >> > Think you need to fix details on your PR like headers etc but looks
>> ok
>> >> to
>> >> > merge once the build pass.
>> >> >
>> >> > Side note: by curiosity, did you check jms resource as well? it
>> should
>> >> be
>> >> > close ot datasources.
>> >> >
>> >> >
>> >> > Romain Manni-Bucau
>> >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> >> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
>> >> > rmannibucau> |
>> >> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
>> >> > <https://javaeefactory-rmannibucau.rhcloud.com>
>> >> >
>> >> > 2017-06-16 19:56 GMT+02:00 Svetlin Zarev
>> <[hidden email]
>> >> om
>> >> > >:
>> >> >
>> >> > > I'm back with tests:) :
>> >> > > https://github.com/apache/tomee/compare/master...
>> >> > > SvetlinZarev:ds_def?expand=1
>> >> > >
>> >> > > Unless I misunderstood you, it seems that DS injection in servlet
>> >> works
>> >> > > after the fix and does not before it (well, it injects it, but with
>> >> wrong
>> >> > > config).
>> >> > > The reason it works is that the ConvertDataSourceDefinitions
>> deployer
>> >> > only
>> >> > > processes the DataSource definitions by converting them to Resource
>> >> > objects
>> >> > > which are not associated with specific component, and since the
>> >> > > CompManagedBean does not have its own, unique definitions, if we
>> >> exclude
>> >> > it
>> >> > > from processing in that specific deployer we won't lose anything,
>> >> because
>> >> > > data sources with the same IDs ( and with correct config !) will be
>> >> > pulled
>> >> > > from the JndiConsumers that actually provide them.
>> >> > >
>> >> > > So to summarize, the flow before the proposed fix is:
>> >> > > 1. Collect all JndiConsumers
>> >> > > 2. MyBean -> provide DataSource with correct config
>> >> > > 3. CompManagedBean -> overwrite MyBean's datasource, with one with
>> bad
>> >> > > config
>> >> > > 4. We end up with one resource with bad config
>> >> > >
>> >> > > And after the fix it becomes:
>> >> > > 1. Collect all JndiConsumers
>> >> > > 2. MyBean -> provide DataSource with correct config
>> >> > > 3. We end up with one resource with good config
>> >> > >
>> >> > > I hope I don't miss anything :) May you give more details about
>> your
>> >> > idea ?
>> >> > >
>> >> > > Kind regards,
>> >> > > Svetlin
>> >> > >
>> >> > >
>> >> > > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <
>> [hidden email]
>> >> >:
>> >> > >
>> >> > > > Hmm, if you inject such a datasource in a servlet does it work?
>> >> Don't
>> >> > > think
>> >> > > > so - tests are the thing to start with when editing this code,
>> >> saying
>> >> > > that
>> >> > > > by experience if you get me ;).
>> >> > > >
>> >> > > > So concretely testing the type like that is good but it shouldn't
>> >> break
>> >> > > web
>> >> > > > component injections.
>> >> > > >
>> >> > > >
>> >> > > > Romain Manni-Bucau
>> >> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> >> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> >> > > > <http://rmannibucau.wordpress.com> | Github <https://github.com/
>> >> > > > rmannibucau> |
>> >> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
>> Factory
>> >> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
>> >> > > >
>> >> > > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev
>> >> <svetlin.angelov.zarev@gmail.
>> >> > > com
>> >> > > > >:
>> >> > > >
>> >> > > > > Hi,
>> >> > > > >
>> >> > > > > I was thinking about something like
>> >> > > > > https://github.com/SvetlinZarev/tomee/commit/
>> >> > > > > 067fd220e909e89a8c17d90122b0e2158468ece4
>> >> > > > >
>> >> > > > > What do you think ? Is there a more appropriate place to check
>> for
>> >> > it ?
>> >> > > > > If it's OK, I can make PR with the fix  + tests.
>> >> > > > >
>> >> > > > > Thanks,
>> >> > > > > Svetlin
>> >> > > > >
>> >> > > > >
>> >> > > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <
>> >> [hidden email]
>> >> > >:
>> >> > > > >
>> >> > > > > > Hi Svetlin
>> >> > > > > >
>> >> > > > > > this is a way to aggregate the webapp java:comp/env namespace
>> >> > without
>> >> > > > > > handling it too specifically in the code base - at least it
>> >> comes
>> >> > > from
>> >> > > > > that
>> >> > > > > > idea.
>> >> > > > > >
>> >> > > > > > We can add it in EjbJar and just skip it at deploy time (we
>> do
>> >> > > > something
>> >> > > > > > similar already, don't recally exactly where but it is typed
>> >> enough
>> >> > > to
>> >> > > > > know
>> >> > > > > > it is the comp bean).
>> >> > > > > >
>> >> > > > > > Does it give you enough input to work on it or do you want
>> some
>> >> > > > > particular
>> >> > > > > > code reference?
>> >> > > > > >
>> >> > > > > >
>> >> > > > > > Romain Manni-Bucau
>> >> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>> >> > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
>> >> > > > > > <http://rmannibucau.wordpress.com> | Github <
>> >> https://github.com/
>> >> > > > > > rmannibucau> |
>> >> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
>> >> > Factory
>> >> > > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
>> >> > > > > >
>> >> > > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
>> >> > > <svetlin.angelov.zarev@gmail.
>> >> > > > > com
>> >> > > > > > >:
>> >> > > > > >
>> >> > > > > > > Hi Everyone,
>> >> > > > > > >
>> >> > > > > > > What's the purpose of the org.apache.openejb.config.
>> >> > > CompManagedBean
>> >> > > > ?
>> >> > > > > > I'm
>> >> > > > > > > asking in the context of TOMEE-2053.
>> >> > > > > > >
>> >> > > > > > > I have a @DataSourceDefinition with some attributes which
>> >> should
>> >> > be
>> >> > > > > > > overrriden by ejb-jar.xml. Everithing works great, with the
>> >> sole
>> >> > > > > > exception
>> >> > > > > > > of CompManagedBean. It seems that it "aggregates" the
>> >> annotations
>> >> > > > from
>> >> > > > > > the
>> >> > > > > > > other beans, but as it's artificially added to the ejb-jar
>> by
>> >> > > > openejb,
>> >> > > > > it
>> >> > > > > > > does not have an entry in the ejb-jar.xml. Hence when the
>> >> > > > > > > AnnotationDeployer processes the DataSourceDefinition
>> >> annotation,
>> >> > > if
>> >> > > > > > never
>> >> > > > > > > finds an exisitin datasource definition in the ejb-jar.xml
>> for
>> >> > it.
>> >> > > > This
>> >> > > > > > in
>> >> > > > > > > turn makes the annotation deployer to add a datasource with
>> >> wrong
>> >> > > > > > > configuration to the AppModule's ejb-jar. So far so good,
>> but
>> >> > > later,
>> >> > > > > the
>> >> > > > > > > ConvertDataSourceDefinitions deployer collects all
>> datasources
>> >> > from
>> >> > > > all
>> >> > > > > > > JndiConsumers, so it collects the invalid definition as
>> well
>> >> and
>> >> > > adds
>> >> > > > > it
>> >> > > > > > to
>> >> > > > > > > the AppModule's resources. And this breaks the application
>> >> > startup.
>> >> > > > > > >
>> >> > > > > > > Kind regards,
>> >> > > > > > > Svetlin
>> >> > > > > > >
>> >> > > > > >
>> >> > > > >
>> >> > > >
>> >> > >
>> >> >
>> >>
>> >
>> >
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Romain Manni-Bucau
You rock Svetlin, applied, thanks a lot!


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://blog-rmannibucau.rhcloud.com> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2017-06-17 7:56 GMT+02:00 Svetlin Zarev <[hidden email]>:

> Hi,
>
> Here it is: https://github.com/apache/tomee/pull/74
>
> Kind regards,
> Svetlin
>
>
> 2017-06-16 23:44 GMT+03:00 Svetlin Zarev <[hidden email]
> >:
>
> > The test fails, because the @DataSourceDefinition is used on a POJO -
> i.e.
> > it's not a JndiConsumer, hence the ConvertDataSourceDefinitions deployer
> > cannot know that such definition exists. I'll have to check the spec if
> it
> > mentions which classes can be annotated with @DataSourceDefinition, but
> > either way it would be easy to fix. I'll take a look tomorrow :)
> >
> > Kind regards,
> > Svetlin
> >
> > 2017-06-16 22:37 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
> >
> >> Trunk seems to have an issue with this in the test
> >> XADataSourceDefinitionTest
> >>
> >> Want to have a look?
> >>
> >>
> >> Romain Manni-Bucau
> >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> <http://rmannibucau.wordpress.com> | Github <
> >> https://github.com/rmannibucau> |
> >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> >> <https://javaeefactory-rmannibucau.rhcloud.com>
> >>
> >> 2017-06-16 20:46 GMT+02:00 Romain Manni-Bucau <[hidden email]>:
> >>
> >> > applied, thanks!
> >> >
> >> >
> >> > Romain Manni-Bucau
> >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> > <http://rmannibucau.wordpress.com> | Github
> >> > <https://github.com/rmannibucau> | LinkedIn
> >> > <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> >> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >> >
> >> > 2017-06-16 20:33 GMT+02:00 Svetlin Zarev
> <[hidden email]
> >> om>
> >> > :
> >> >
> >> >> I've fixed the license headers. Here is the PR:
> >> >> https://github.com/apache/tomee/pull/73
> >> >>
> >> >> I didn't check JMS, only WepProfile specs.
> >> >>
> >> >> 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <[hidden email]
> >:
> >> >>
> >> >> > This looks good, while the merge deployer still makes available the
> >> >> > resource to comp it works :)
> >> >> >
> >> >> > Think you need to fix details on your PR like headers etc but looks
> >> ok
> >> >> to
> >> >> > merge once the build pass.
> >> >> >
> >> >> > Side note: by curiosity, did you check jms resource as well? it
> >> should
> >> >> be
> >> >> > close ot datasources.
> >> >> >
> >> >> >
> >> >> > Romain Manni-Bucau
> >> >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> >> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> >> >> > rmannibucau> |
> >> >> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> Factory
> >> >> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >> >> >
> >> >> > 2017-06-16 19:56 GMT+02:00 Svetlin Zarev
> >> <[hidden email]
> >> >> om
> >> >> > >:
> >> >> >
> >> >> > > I'm back with tests:) :
> >> >> > > https://github.com/apache/tomee/compare/master...
> >> >> > > SvetlinZarev:ds_def?expand=1
> >> >> > >
> >> >> > > Unless I misunderstood you, it seems that DS injection in servlet
> >> >> works
> >> >> > > after the fix and does not before it (well, it injects it, but
> with
> >> >> wrong
> >> >> > > config).
> >> >> > > The reason it works is that the ConvertDataSourceDefinitions
> >> deployer
> >> >> > only
> >> >> > > processes the DataSource definitions by converting them to
> Resource
> >> >> > objects
> >> >> > > which are not associated with specific component, and since the
> >> >> > > CompManagedBean does not have its own, unique definitions, if we
> >> >> exclude
> >> >> > it
> >> >> > > from processing in that specific deployer we won't lose anything,
> >> >> because
> >> >> > > data sources with the same IDs ( and with correct config !) will
> be
> >> >> > pulled
> >> >> > > from the JndiConsumers that actually provide them.
> >> >> > >
> >> >> > > So to summarize, the flow before the proposed fix is:
> >> >> > > 1. Collect all JndiConsumers
> >> >> > > 2. MyBean -> provide DataSource with correct config
> >> >> > > 3. CompManagedBean -> overwrite MyBean's datasource, with one
> with
> >> bad
> >> >> > > config
> >> >> > > 4. We end up with one resource with bad config
> >> >> > >
> >> >> > > And after the fix it becomes:
> >> >> > > 1. Collect all JndiConsumers
> >> >> > > 2. MyBean -> provide DataSource with correct config
> >> >> > > 3. We end up with one resource with good config
> >> >> > >
> >> >> > > I hope I don't miss anything :) May you give more details about
> >> your
> >> >> > idea ?
> >> >> > >
> >> >> > > Kind regards,
> >> >> > > Svetlin
> >> >> > >
> >> >> > >
> >> >> > > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <
> >> [hidden email]
> >> >> >:
> >> >> > >
> >> >> > > > Hmm, if you inject such a datasource in a servlet does it work?
> >> >> Don't
> >> >> > > think
> >> >> > > > so - tests are the thing to start with when editing this code,
> >> >> saying
> >> >> > > that
> >> >> > > > by experience if you get me ;).
> >> >> > > >
> >> >> > > > So concretely testing the type like that is good but it
> shouldn't
> >> >> break
> >> >> > > web
> >> >> > > > component injections.
> >> >> > > >
> >> >> > > >
> >> >> > > > Romain Manni-Bucau
> >> >> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> >> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> >> > > > <http://rmannibucau.wordpress.com> | Github <
> https://github.com/
> >> >> > > > rmannibucau> |
> >> >> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> >> Factory
> >> >> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> >> >> > > >
> >> >> > > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev
> >> >> <svetlin.angelov.zarev@gmail.
> >> >> > > com
> >> >> > > > >:
> >> >> > > >
> >> >> > > > > Hi,
> >> >> > > > >
> >> >> > > > > I was thinking about something like
> >> >> > > > > https://github.com/SvetlinZarev/tomee/commit/
> >> >> > > > > 067fd220e909e89a8c17d90122b0e2158468ece4
> >> >> > > > >
> >> >> > > > > What do you think ? Is there a more appropriate place to
> check
> >> for
> >> >> > it ?
> >> >> > > > > If it's OK, I can make PR with the fix  + tests.
> >> >> > > > >
> >> >> > > > > Thanks,
> >> >> > > > > Svetlin
> >> >> > > > >
> >> >> > > > >
> >> >> > > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <
> >> >> [hidden email]
> >> >> > >:
> >> >> > > > >
> >> >> > > > > > Hi Svetlin
> >> >> > > > > >
> >> >> > > > > > this is a way to aggregate the webapp java:comp/env
> namespace
> >> >> > without
> >> >> > > > > > handling it too specifically in the code base - at least it
> >> >> comes
> >> >> > > from
> >> >> > > > > that
> >> >> > > > > > idea.
> >> >> > > > > >
> >> >> > > > > > We can add it in EjbJar and just skip it at deploy time (we
> >> do
> >> >> > > > something
> >> >> > > > > > similar already, don't recally exactly where but it is
> typed
> >> >> enough
> >> >> > > to
> >> >> > > > > know
> >> >> > > > > > it is the comp bean).
> >> >> > > > > >
> >> >> > > > > > Does it give you enough input to work on it or do you want
> >> some
> >> >> > > > > particular
> >> >> > > > > > code reference?
> >> >> > > > > >
> >> >> > > > > >
> >> >> > > > > > Romain Manni-Bucau
> >> >> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> >> >> > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> >> >> > > > > > <http://rmannibucau.wordpress.com> | Github <
> >> >> https://github.com/
> >> >> > > > > > rmannibucau> |
> >> >> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> |
> JavaEE
> >> >> > Factory
> >> >> > > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> >> >> > > > > >
> >> >> > > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
> >> >> > > <svetlin.angelov.zarev@gmail.
> >> >> > > > > com
> >> >> > > > > > >:
> >> >> > > > > >
> >> >> > > > > > > Hi Everyone,
> >> >> > > > > > >
> >> >> > > > > > > What's the purpose of the org.apache.openejb.config.
> >> >> > > CompManagedBean
> >> >> > > > ?
> >> >> > > > > > I'm
> >> >> > > > > > > asking in the context of TOMEE-2053.
> >> >> > > > > > >
> >> >> > > > > > > I have a @DataSourceDefinition with some attributes which
> >> >> should
> >> >> > be
> >> >> > > > > > > overrriden by ejb-jar.xml. Everithing works great, with
> the
> >> >> sole
> >> >> > > > > > exception
> >> >> > > > > > > of CompManagedBean. It seems that it "aggregates" the
> >> >> annotations
> >> >> > > > from
> >> >> > > > > > the
> >> >> > > > > > > other beans, but as it's artificially added to the
> ejb-jar
> >> by
> >> >> > > > openejb,
> >> >> > > > > it
> >> >> > > > > > > does not have an entry in the ejb-jar.xml. Hence when the
> >> >> > > > > > > AnnotationDeployer processes the DataSourceDefinition
> >> >> annotation,
> >> >> > > if
> >> >> > > > > > never
> >> >> > > > > > > finds an exisitin datasource definition in the
> ejb-jar.xml
> >> for
> >> >> > it.
> >> >> > > > This
> >> >> > > > > > in
> >> >> > > > > > > turn makes the annotation deployer to add a datasource
> with
> >> >> wrong
> >> >> > > > > > > configuration to the AppModule's ejb-jar. So far so good,
> >> but
> >> >> > > later,
> >> >> > > > > the
> >> >> > > > > > > ConvertDataSourceDefinitions deployer collects all
> >> datasources
> >> >> > from
> >> >> > > > all
> >> >> > > > > > > JndiConsumers, so it collects the invalid definition as
> >> well
> >> >> and
> >> >> > > adds
> >> >> > > > > it
> >> >> > > > > > to
> >> >> > > > > > > the AppModule's resources. And this breaks the
> application
> >> >> > startup.
> >> >> > > > > > >
> >> >> > > > > > > Kind regards,
> >> >> > > > > > > Svetlin
> >> >> > > > > > >
> >> >> > > > > >
> >> >> > > > >
> >> >> > > >
> >> >> > >
> >> >> >
> >> >>
> >> >
> >> >
> >>
> >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Svetlin Zarev
Hi,

The DataSourcePojo.java is missing from master, and the arquilian tests
fail to compile. May you add it:
https://github.com/apache/tomee/pull/74/commits/26ee7018df455c3a5b46c6a0b87adb38feeab34f#diff-c89f954c712eb9ab263f8acf8d75586f
?

Kind regadrs,
Svetlin

2017-06-17 11:13 GMT+03:00 Romain Manni-Bucau <[hidden email]>:

> You rock Svetlin, applied, thanks a lot!
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> <http://rmannibucau.wordpress.com> | Github <https://github.com/
> rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
>
> 2017-06-17 7:56 GMT+02:00 Svetlin Zarev <[hidden email]>:
>
> > Hi,
> >
> > Here it is: https://github.com/apache/tomee/pull/74
> >
> > Kind regards,
> > Svetlin
> >
> >
> > 2017-06-16 23:44 GMT+03:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> > >:
> >
> > > The test fails, because the @DataSourceDefinition is used on a POJO -
> > i.e.
> > > it's not a JndiConsumer, hence the ConvertDataSourceDefinitions
> deployer
> > > cannot know that such definition exists. I'll have to check the spec if
> > it
> > > mentions which classes can be annotated with @DataSourceDefinition, but
> > > either way it would be easy to fix. I'll take a look tomorrow :)
> > >
> > > Kind regards,
> > > Svetlin
> > >
> > > 2017-06-16 22:37 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
> > >
> > >> Trunk seems to have an issue with this in the test
> > >> XADataSourceDefinitionTest
> > >>
> > >> Want to have a look?
> > >>
> > >>
> > >> Romain Manni-Bucau
> > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > >> <http://rmannibucau.wordpress.com> | Github <
> > >> https://github.com/rmannibucau> |
> > >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > >> <https://javaeefactory-rmannibucau.rhcloud.com>
> > >>
> > >> 2017-06-16 20:46 GMT+02:00 Romain Manni-Bucau <[hidden email]
> >:
> > >>
> > >> > applied, thanks!
> > >> >
> > >> >
> > >> > Romain Manni-Bucau
> > >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > >> > <http://rmannibucau.wordpress.com> | Github
> > >> > <https://github.com/rmannibucau> | LinkedIn
> > >> > <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > >> > <https://javaeefactory-rmannibucau.rhcloud.com>
> > >> >
> > >> > 2017-06-16 20:33 GMT+02:00 Svetlin Zarev
> > <[hidden email]
> > >> om>
> > >> > :
> > >> >
> > >> >> I've fixed the license headers. Here is the PR:
> > >> >> https://github.com/apache/tomee/pull/73
> > >> >>
> > >> >> I didn't check JMS, only WepProfile specs.
> > >> >>
> > >> >> 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <
> [hidden email]
> > >:
> > >> >>
> > >> >> > This looks good, while the merge deployer still makes available
> the
> > >> >> > resource to comp it works :)
> > >> >> >
> > >> >> > Think you need to fix details on your PR like headers etc but
> looks
> > >> ok
> > >> >> to
> > >> >> > merge once the build pass.
> > >> >> >
> > >> >> > Side note: by curiosity, did you check jms resource as well? it
> > >> should
> > >> >> be
> > >> >> > close ot datasources.
> > >> >> >
> > >> >> >
> > >> >> > Romain Manni-Bucau
> > >> >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > >> >> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > >> >> > rmannibucau> |
> > >> >> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> > Factory
> > >> >> > <https://javaeefactory-rmannibucau.rhcloud.com>
> > >> >> >
> > >> >> > 2017-06-16 19:56 GMT+02:00 Svetlin Zarev
> > >> <[hidden email]
> > >> >> om
> > >> >> > >:
> > >> >> >
> > >> >> > > I'm back with tests:) :
> > >> >> > > https://github.com/apache/tomee/compare/master...
> > >> >> > > SvetlinZarev:ds_def?expand=1
> > >> >> > >
> > >> >> > > Unless I misunderstood you, it seems that DS injection in
> servlet
> > >> >> works
> > >> >> > > after the fix and does not before it (well, it injects it, but
> > with
> > >> >> wrong
> > >> >> > > config).
> > >> >> > > The reason it works is that the ConvertDataSourceDefinitions
> > >> deployer
> > >> >> > only
> > >> >> > > processes the DataSource definitions by converting them to
> > Resource
> > >> >> > objects
> > >> >> > > which are not associated with specific component, and since the
> > >> >> > > CompManagedBean does not have its own, unique definitions, if
> we
> > >> >> exclude
> > >> >> > it
> > >> >> > > from processing in that specific deployer we won't lose
> anything,
> > >> >> because
> > >> >> > > data sources with the same IDs ( and with correct config !)
> will
> > be
> > >> >> > pulled
> > >> >> > > from the JndiConsumers that actually provide them.
> > >> >> > >
> > >> >> > > So to summarize, the flow before the proposed fix is:
> > >> >> > > 1. Collect all JndiConsumers
> > >> >> > > 2. MyBean -> provide DataSource with correct config
> > >> >> > > 3. CompManagedBean -> overwrite MyBean's datasource, with one
> > with
> > >> bad
> > >> >> > > config
> > >> >> > > 4. We end up with one resource with bad config
> > >> >> > >
> > >> >> > > And after the fix it becomes:
> > >> >> > > 1. Collect all JndiConsumers
> > >> >> > > 2. MyBean -> provide DataSource with correct config
> > >> >> > > 3. We end up with one resource with good config
> > >> >> > >
> > >> >> > > I hope I don't miss anything :) May you give more details about
> > >> your
> > >> >> > idea ?
> > >> >> > >
> > >> >> > > Kind regards,
> > >> >> > > Svetlin
> > >> >> > >
> > >> >> > >
> > >> >> > > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <
> > >> [hidden email]
> > >> >> >:
> > >> >> > >
> > >> >> > > > Hmm, if you inject such a datasource in a servlet does it
> work?
> > >> >> Don't
> > >> >> > > think
> > >> >> > > > so - tests are the thing to start with when editing this
> code,
> > >> >> saying
> > >> >> > > that
> > >> >> > > > by experience if you get me ;).
> > >> >> > > >
> > >> >> > > > So concretely testing the type like that is good but it
> > shouldn't
> > >> >> break
> > >> >> > > web
> > >> >> > > > component injections.
> > >> >> > > >
> > >> >> > > >
> > >> >> > > > Romain Manni-Bucau
> > >> >> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> >> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > >> >> > > > <http://rmannibucau.wordpress.com> | Github <
> > https://github.com/
> > >> >> > > > rmannibucau> |
> > >> >> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> > >> Factory
> > >> >> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > >> >> > > >
> > >> >> > > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev
> > >> >> <svetlin.angelov.zarev@gmail.
> > >> >> > > com
> > >> >> > > > >:
> > >> >> > > >
> > >> >> > > > > Hi,
> > >> >> > > > >
> > >> >> > > > > I was thinking about something like
> > >> >> > > > > https://github.com/SvetlinZarev/tomee/commit/
> > >> >> > > > > 067fd220e909e89a8c17d90122b0e2158468ece4
> > >> >> > > > >
> > >> >> > > > > What do you think ? Is there a more appropriate place to
> > check
> > >> for
> > >> >> > it ?
> > >> >> > > > > If it's OK, I can make PR with the fix  + tests.
> > >> >> > > > >
> > >> >> > > > > Thanks,
> > >> >> > > > > Svetlin
> > >> >> > > > >
> > >> >> > > > >
> > >> >> > > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <
> > >> >> [hidden email]
> > >> >> > >:
> > >> >> > > > >
> > >> >> > > > > > Hi Svetlin
> > >> >> > > > > >
> > >> >> > > > > > this is a way to aggregate the webapp java:comp/env
> > namespace
> > >> >> > without
> > >> >> > > > > > handling it too specifically in the code base - at least
> it
> > >> >> comes
> > >> >> > > from
> > >> >> > > > > that
> > >> >> > > > > > idea.
> > >> >> > > > > >
> > >> >> > > > > > We can add it in EjbJar and just skip it at deploy time
> (we
> > >> do
> > >> >> > > > something
> > >> >> > > > > > similar already, don't recally exactly where but it is
> > typed
> > >> >> enough
> > >> >> > > to
> > >> >> > > > > know
> > >> >> > > > > > it is the comp bean).
> > >> >> > > > > >
> > >> >> > > > > > Does it give you enough input to work on it or do you
> want
> > >> some
> > >> >> > > > > particular
> > >> >> > > > > > code reference?
> > >> >> > > > > >
> > >> >> > > > > >
> > >> >> > > > > > Romain Manni-Bucau
> > >> >> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > >> >> > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > >> >> > > > > > <http://rmannibucau.wordpress.com> | Github <
> > >> >> https://github.com/
> > >> >> > > > > > rmannibucau> |
> > >> >> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> |
> > JavaEE
> > >> >> > Factory
> > >> >> > > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > >> >> > > > > >
> > >> >> > > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
> > >> >> > > <svetlin.angelov.zarev@gmail.
> > >> >> > > > > com
> > >> >> > > > > > >:
> > >> >> > > > > >
> > >> >> > > > > > > Hi Everyone,
> > >> >> > > > > > >
> > >> >> > > > > > > What's the purpose of the org.apache.openejb.config.
> > >> >> > > CompManagedBean
> > >> >> > > > ?
> > >> >> > > > > > I'm
> > >> >> > > > > > > asking in the context of TOMEE-2053.
> > >> >> > > > > > >
> > >> >> > > > > > > I have a @DataSourceDefinition with some attributes
> which
> > >> >> should
> > >> >> > be
> > >> >> > > > > > > overrriden by ejb-jar.xml. Everithing works great, with
> > the
> > >> >> sole
> > >> >> > > > > > exception
> > >> >> > > > > > > of CompManagedBean. It seems that it "aggregates" the
> > >> >> annotations
> > >> >> > > > from
> > >> >> > > > > > the
> > >> >> > > > > > > other beans, but as it's artificially added to the
> > ejb-jar
> > >> by
> > >> >> > > > openejb,
> > >> >> > > > > it
> > >> >> > > > > > > does not have an entry in the ejb-jar.xml. Hence when
> the
> > >> >> > > > > > > AnnotationDeployer processes the DataSourceDefinition
> > >> >> annotation,
> > >> >> > > if
> > >> >> > > > > > never
> > >> >> > > > > > > finds an exisitin datasource definition in the
> > ejb-jar.xml
> > >> for
> > >> >> > it.
> > >> >> > > > This
> > >> >> > > > > > in
> > >> >> > > > > > > turn makes the annotation deployer to add a datasource
> > with
> > >> >> wrong
> > >> >> > > > > > > configuration to the AppModule's ejb-jar. So far so
> good,
> > >> but
> > >> >> > > later,
> > >> >> > > > > the
> > >> >> > > > > > > ConvertDataSourceDefinitions deployer collects all
> > >> datasources
> > >> >> > from
> > >> >> > > > all
> > >> >> > > > > > > JndiConsumers, so it collects the invalid definition as
> > >> well
> > >> >> and
> > >> >> > > adds
> > >> >> > > > > it
> > >> >> > > > > > to
> > >> >> > > > > > > the AppModule's resources. And this breaks the
> > application
> > >> >> > startup.
> > >> >> > > > > > >
> > >> >> > > > > > > Kind regards,
> > >> >> > > > > > > Svetlin
> > >> >> > > > > > >
> > >> >> > > > > >
> > >> >> > > > >
> > >> >> > > >
> > >> >> > >
> > >> >> >
> > >> >>
> > >> >
> > >> >
> > >>
> > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

Romain Manni-Bucau
big fingers :(

thanks for the catch, should be fixed. Build in progress at
https://ci.apache.org/builders/tomee-trunk-ubuntu/builds/777


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://blog-rmannibucau.rhcloud.com> | Old Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2017-06-19 9:20 GMT+02:00 Svetlin Zarev <[hidden email]>:

> Hi,
>
> The DataSourcePojo.java is missing from master, and the arquilian tests
> fail to compile. May you add it:
> https://github.com/apache/tomee/pull/74/commits/
> 26ee7018df455c3a5b46c6a0b87adb38feeab34f#diff-
> c89f954c712eb9ab263f8acf8d75586f
> ?
>
> Kind regadrs,
> Svetlin
>
> 2017-06-17 11:13 GMT+03:00 Romain Manni-Bucau <[hidden email]>:
>
> > You rock Svetlin, applied, thanks a lot!
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > <http://rmannibucau.wordpress.com> | Github <https://github.com/
> > rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >
> > 2017-06-17 7:56 GMT+02:00 Svetlin Zarev <[hidden email]
> >:
> >
> > > Hi,
> > >
> > > Here it is: https://github.com/apache/tomee/pull/74
> > >
> > > Kind regards,
> > > Svetlin
> > >
> > >
> > > 2017-06-16 23:44 GMT+03:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> > com
> > > >:
> > >
> > > > The test fails, because the @DataSourceDefinition is used on a POJO -
> > > i.e.
> > > > it's not a JndiConsumer, hence the ConvertDataSourceDefinitions
> > deployer
> > > > cannot know that such definition exists. I'll have to check the spec
> if
> > > it
> > > > mentions which classes can be annotated with @DataSourceDefinition,
> but
> > > > either way it would be easy to fix. I'll take a look tomorrow :)
> > > >
> > > > Kind regards,
> > > > Svetlin
> > > >
> > > > 2017-06-16 22:37 GMT+03:00 Romain Manni-Bucau <[hidden email]
> >:
> > > >
> > > >> Trunk seems to have an issue with this in the test
> > > >> XADataSourceDefinitionTest
> > > >>
> > > >> Want to have a look?
> > > >>
> > > >>
> > > >> Romain Manni-Bucau
> > > >> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > >> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > >> <http://rmannibucau.wordpress.com> | Github <
> > > >> https://github.com/rmannibucau> |
> > > >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > > >> <https://javaeefactory-rmannibucau.rhcloud.com>
> > > >>
> > > >> 2017-06-16 20:46 GMT+02:00 Romain Manni-Bucau <
> [hidden email]
> > >:
> > > >>
> > > >> > applied, thanks!
> > > >> >
> > > >> >
> > > >> > Romain Manni-Bucau
> > > >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > >> > <http://rmannibucau.wordpress.com> | Github
> > > >> > <https://github.com/rmannibucau> | LinkedIn
> > > >> > <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > > >> > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > >> >
> > > >> > 2017-06-16 20:33 GMT+02:00 Svetlin Zarev
> > > <[hidden email]
> > > >> om>
> > > >> > :
> > > >> >
> > > >> >> I've fixed the license headers. Here is the PR:
> > > >> >> https://github.com/apache/tomee/pull/73
> > > >> >>
> > > >> >> I didn't check JMS, only WepProfile specs.
> > > >> >>
> > > >> >> 2017-06-16 21:01 GMT+03:00 Romain Manni-Bucau <
> > [hidden email]
> > > >:
> > > >> >>
> > > >> >> > This looks good, while the merge deployer still makes available
> > the
> > > >> >> > resource to comp it works :)
> > > >> >> >
> > > >> >> > Think you need to fix details on your PR like headers etc but
> > looks
> > > >> ok
> > > >> >> to
> > > >> >> > merge once the build pass.
> > > >> >> >
> > > >> >> > Side note: by curiosity, did you check jms resource as well? it
> > > >> should
> > > >> >> be
> > > >> >> > close ot datasources.
> > > >> >> >
> > > >> >> >
> > > >> >> > Romain Manni-Bucau
> > > >> >> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > >> >> > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > >> >> > <http://rmannibucau.wordpress.com> | Github <
> https://github.com/
> > > >> >> > rmannibucau> |
> > > >> >> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE
> > > Factory
> > > >> >> > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > >> >> >
> > > >> >> > 2017-06-16 19:56 GMT+02:00 Svetlin Zarev
> > > >> <[hidden email]
> > > >> >> om
> > > >> >> > >:
> > > >> >> >
> > > >> >> > > I'm back with tests:) :
> > > >> >> > > https://github.com/apache/tomee/compare/master...
> > > >> >> > > SvetlinZarev:ds_def?expand=1
> > > >> >> > >
> > > >> >> > > Unless I misunderstood you, it seems that DS injection in
> > servlet
> > > >> >> works
> > > >> >> > > after the fix and does not before it (well, it injects it,
> but
> > > with
> > > >> >> wrong
> > > >> >> > > config).
> > > >> >> > > The reason it works is that the ConvertDataSourceDefinitions
> > > >> deployer
> > > >> >> > only
> > > >> >> > > processes the DataSource definitions by converting them to
> > > Resource
> > > >> >> > objects
> > > >> >> > > which are not associated with specific component, and since
> the
> > > >> >> > > CompManagedBean does not have its own, unique definitions, if
> > we
> > > >> >> exclude
> > > >> >> > it
> > > >> >> > > from processing in that specific deployer we won't lose
> > anything,
> > > >> >> because
> > > >> >> > > data sources with the same IDs ( and with correct config !)
> > will
> > > be
> > > >> >> > pulled
> > > >> >> > > from the JndiConsumers that actually provide them.
> > > >> >> > >
> > > >> >> > > So to summarize, the flow before the proposed fix is:
> > > >> >> > > 1. Collect all JndiConsumers
> > > >> >> > > 2. MyBean -> provide DataSource with correct config
> > > >> >> > > 3. CompManagedBean -> overwrite MyBean's datasource, with one
> > > with
> > > >> bad
> > > >> >> > > config
> > > >> >> > > 4. We end up with one resource with bad config
> > > >> >> > >
> > > >> >> > > And after the fix it becomes:
> > > >> >> > > 1. Collect all JndiConsumers
> > > >> >> > > 2. MyBean -> provide DataSource with correct config
> > > >> >> > > 3. We end up with one resource with good config
> > > >> >> > >
> > > >> >> > > I hope I don't miss anything :) May you give more details
> about
> > > >> your
> > > >> >> > idea ?
> > > >> >> > >
> > > >> >> > > Kind regards,
> > > >> >> > > Svetlin
> > > >> >> > >
> > > >> >> > >
> > > >> >> > > 2017-06-16 16:32 GMT+03:00 Romain Manni-Bucau <
> > > >> [hidden email]
> > > >> >> >:
> > > >> >> > >
> > > >> >> > > > Hmm, if you inject such a datasource in a servlet does it
> > work?
> > > >> >> Don't
> > > >> >> > > think
> > > >> >> > > > so - tests are the thing to start with when editing this
> > code,
> > > >> >> saying
> > > >> >> > > that
> > > >> >> > > > by experience if you get me ;).
> > > >> >> > > >
> > > >> >> > > > So concretely testing the type like that is good but it
> > > shouldn't
> > > >> >> break
> > > >> >> > > web
> > > >> >> > > > component injections.
> > > >> >> > > >
> > > >> >> > > >
> > > >> >> > > > Romain Manni-Bucau
> > > >> >> > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > >> >> > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > >> >> > > > <http://rmannibucau.wordpress.com> | Github <
> > > https://github.com/
> > > >> >> > > > rmannibucau> |
> > > >> >> > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> |
> JavaEE
> > > >> Factory
> > > >> >> > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > >> >> > > >
> > > >> >> > > > 2017-06-16 15:27 GMT+02:00 Svetlin Zarev
> > > >> >> <svetlin.angelov.zarev@gmail.
> > > >> >> > > com
> > > >> >> > > > >:
> > > >> >> > > >
> > > >> >> > > > > Hi,
> > > >> >> > > > >
> > > >> >> > > > > I was thinking about something like
> > > >> >> > > > > https://github.com/SvetlinZarev/tomee/commit/
> > > >> >> > > > > 067fd220e909e89a8c17d90122b0e2158468ece4
> > > >> >> > > > >
> > > >> >> > > > > What do you think ? Is there a more appropriate place to
> > > check
> > > >> for
> > > >> >> > it ?
> > > >> >> > > > > If it's OK, I can make PR with the fix  + tests.
> > > >> >> > > > >
> > > >> >> > > > > Thanks,
> > > >> >> > > > > Svetlin
> > > >> >> > > > >
> > > >> >> > > > >
> > > >> >> > > > > 2017-06-16 16:15 GMT+03:00 Romain Manni-Bucau <
> > > >> >> [hidden email]
> > > >> >> > >:
> > > >> >> > > > >
> > > >> >> > > > > > Hi Svetlin
> > > >> >> > > > > >
> > > >> >> > > > > > this is a way to aggregate the webapp java:comp/env
> > > namespace
> > > >> >> > without
> > > >> >> > > > > > handling it too specifically in the code base - at
> least
> > it
> > > >> >> comes
> > > >> >> > > from
> > > >> >> > > > > that
> > > >> >> > > > > > idea.
> > > >> >> > > > > >
> > > >> >> > > > > > We can add it in EjbJar and just skip it at deploy time
> > (we
> > > >> do
> > > >> >> > > > something
> > > >> >> > > > > > similar already, don't recally exactly where but it is
> > > typed
> > > >> >> enough
> > > >> >> > > to
> > > >> >> > > > > know
> > > >> >> > > > > > it is the comp bean).
> > > >> >> > > > > >
> > > >> >> > > > > > Does it give you enough input to work on it or do you
> > want
> > > >> some
> > > >> >> > > > > particular
> > > >> >> > > > > > code reference?
> > > >> >> > > > > >
> > > >> >> > > > > >
> > > >> >> > > > > > Romain Manni-Bucau
> > > >> >> > > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > > >> >> > > > > > <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > > >> >> > > > > > <http://rmannibucau.wordpress.com> | Github <
> > > >> >> https://github.com/
> > > >> >> > > > > > rmannibucau> |
> > > >> >> > > > > > LinkedIn <https://www.linkedin.com/in/rmannibucau> |
> > > JavaEE
> > > >> >> > Factory
> > > >> >> > > > > > <https://javaeefactory-rmannibucau.rhcloud.com>
> > > >> >> > > > > >
> > > >> >> > > > > > 2017-06-16 15:10 GMT+02:00 Svetlin Zarev
> > > >> >> > > <svetlin.angelov.zarev@gmail.
> > > >> >> > > > > com
> > > >> >> > > > > > >:
> > > >> >> > > > > >
> > > >> >> > > > > > > Hi Everyone,
> > > >> >> > > > > > >
> > > >> >> > > > > > > What's the purpose of the org.apache.openejb.config.
> > > >> >> > > CompManagedBean
> > > >> >> > > > ?
> > > >> >> > > > > > I'm
> > > >> >> > > > > > > asking in the context of TOMEE-2053.
> > > >> >> > > > > > >
> > > >> >> > > > > > > I have a @DataSourceDefinition with some attributes
> > which
> > > >> >> should
> > > >> >> > be
> > > >> >> > > > > > > overrriden by ejb-jar.xml. Everithing works great,
> with
> > > the
> > > >> >> sole
> > > >> >> > > > > > exception
> > > >> >> > > > > > > of CompManagedBean. It seems that it "aggregates" the
> > > >> >> annotations
> > > >> >> > > > from
> > > >> >> > > > > > the
> > > >> >> > > > > > > other beans, but as it's artificially added to the
> > > ejb-jar
> > > >> by
> > > >> >> > > > openejb,
> > > >> >> > > > > it
> > > >> >> > > > > > > does not have an entry in the ejb-jar.xml. Hence when
> > the
> > > >> >> > > > > > > AnnotationDeployer processes the DataSourceDefinition
> > > >> >> annotation,
> > > >> >> > > if
> > > >> >> > > > > > never
> > > >> >> > > > > > > finds an exisitin datasource definition in the
> > > ejb-jar.xml
> > > >> for
> > > >> >> > it.
> > > >> >> > > > This
> > > >> >> > > > > > in
> > > >> >> > > > > > > turn makes the annotation deployer to add a
> datasource
> > > with
> > > >> >> wrong
> > > >> >> > > > > > > configuration to the AppModule's ejb-jar. So far so
> > good,
> > > >> but
> > > >> >> > > later,
> > > >> >> > > > > the
> > > >> >> > > > > > > ConvertDataSourceDefinitions deployer collects all
> > > >> datasources
> > > >> >> > from
> > > >> >> > > > all
> > > >> >> > > > > > > JndiConsumers, so it collects the invalid definition
> as
> > > >> well
> > > >> >> and
> > > >> >> > > adds
> > > >> >> > > > > it
> > > >> >> > > > > > to
> > > >> >> > > > > > > the AppModule's resources. And this breaks the
> > > application
> > > >> >> > startup.
> > > >> >> > > > > > >
> > > >> >> > > > > > > Kind regards,
> > > >> >> > > > > > > Svetlin
> > > >> >> > > > > > >
> > > >> >> > > > > >
> > > >> >> > > > >
> > > >> >> > > >
> > > >> >> > >
> > > >> >> >
> > > >> >>
> > > >> >
> > > >> >
> > > >>
> > > >
> > > >
> > >
> >
>