Hi, Here it is: https://github.com/apache/tomee/pull/74
Kind regards, Svetlin 2017-06-16 23:44 GMT+03:00 Svetlin Zarev <[email protected]>: > 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 <[email protected]>: > >> 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 <[email protected]>: >> >> > 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 <[email protected] >> 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 <[email protected]>: >> >> >> >> > 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 >> <[email protected] >> >> 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 < >> [email protected] >> >> >: >> >> > > >> >> > > > 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 < >> >> [email protected] >> >> > >: >> >> > > > > >> >> > > > > > 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 >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> >> > >> >> >> > >> > >> > >
