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 <rmannibu...@gmail.com>: > 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 <svetlin.angelov.za...@gmail.com>: > > > 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 <rmannibu...@gmail.com>: > > > > > >> 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 <rmannibu...@gmail.com > >: > > >> > > >> > 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.c > > >> 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 < > rmannibu...@gmail.com > > >: > > >> >> > > >> >> > 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.c > > >> >> 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 < > > >> rmannibu...@gmail.com > > >> >> >: > > >> >> > > > > >> >> > > > 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 < > > >> >> rmannibu...@gmail.com > > >> >> > >: > > >> >> > > > > > > >> >> > > > > > 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 > > >> >> > > > > > > > > >> >> > > > > > > > >> >> > > > > > > >> >> > > > > > >> >> > > > > >> >> > > > >> >> > > >> > > > >> > > > >> > > > > > > > > >
