Re: [ANNOUNCE] Welcome Roberto Cortez as new Apache TomEE committer

2018-09-13 Thread Svetlin Zarev
Congratulations!

На вт, 11.09.2018 г. в 19:30 ч. Ivan Junckes Filho 
написа:

> Well deserved, congratulations Roberto!
>
> On Tue, Sep 11, 2018 at 1:25 PM Bruno Baptista  wrote:
>
> > Congratulations Roberto!
> >
> > Bruno Baptista
> > http://twitter.com/brunobat_
> >
> >
> > On 10/09/2018 08:20, Mark Struberg wrote:
> > > Good morning ladies and gents!
> > >
> > > It's a great pleasure to announce that Roberto Cortez has accepted our
> > invitation to become an Apache TomEE committer!
> > >
> > > Welcome Roberto!
> > >
> > > your Apache TomEE PMC
> >
> >
>


Re: [VOTE] Release Apache TomEE 7.0.5 (round 2)

2018-07-18 Thread Svetlin Zarev
Hi,

> If folks could reply to the original post with their +1 / 0 / -1 ...

+1 [VOTE]

Kind regards,
Svetlin

На вт, 10.07.2018 г. в 21:26 ч. Jonathan Gallimore <
jonathan.gallim...@gmail.com> написа:

> Hi Everyone,
>
> Here is the second roll of TomEE 7.0.5. Please can you take a look and
> vote? Everyone, committer or not, is encouraged to test and vote.
>
> Staging repo:
> https://repository.apache.org/content/repositories/orgapachetomee-1115
>
> Source zip:
>
> https://repository.apache.org/content/repositories/orgapachetomee-1115/org/apache/tomee/tomee-project/7.0.5/tomee-project-7.0.5-source-release.zip
>
> Dist area:
> https://dist.apache.org/repos/dist/dev/tomee/staging-1115/
>
> Legal:
> https://dist.apache.org/repos/dist/dev/tomee/staging-1115/legal.zip
>
> Keys:
> https://dist.apache.org/repos/dist/release/tomee/KEYS
>
> Libraries changed since TomEE 7.0.4:
>
> Tomcat => 8.5.32
> CXF => 3.1.15
> Johnzon => 1.0.1
> OWB => 1.7.5
> XBean => 4.9
> XmlSchema core => 2.2.3
> OpenJPA => 2.4.3
>
> Changes since the last roll:
>
> - Remove javax.xml.soap-api-1.3.5.jar library which was incorrectly
> included
> - Update to Tomcat 8.5.32
> - Change JNDI name used for datasource in CDI TCK test to use an equivalent
> name under the java: namespace
>
> Changelog:
> https://issues.apache.org/jira/browse/TOMEE-2175?jql=project
> %20%3D%20TOMEE%20AND%20(status%20%3D%20Resolved%20OR%20statu
> s%20%3D%20CLOSED)%20AND%20fixVersion%20%3D%207.0.5%20O
> RDER%20BY%20priority%20DESC%2C%20updated%20DESC
>
> (If anyone knows a better way to get that list, let me know ;-) )
>
> Please vote:
>  +1: Release
>  -1 Do not release because ...
>
> The vote will be open for 3 days or the consensus is binding (At least 3
> binding votes).
>
> Many thanks
>
> Jon
>


Re: [VOTE] Apache TomEE 7.0.5

2018-07-04 Thread Svetlin Zarev
Hi,

I've successfully built it as well (ubuntu + openjdk). I've triggered my
tests, but they take 36h+ to complete :) I'll update you one they finish.

Kind regards,
Svetlin

На ср, 4.07.2018 г. в 16:05 ч. Jonathan Gallimore <
jonathan.gallim...@gmail.com> написа:

> That test passes here:
>
> Jonathans-MBP:openejb-core jgallimore$ mvn
> -Dtest=BeanValidationCustomProviderTest -DfailIfNoTests=false test | tee
> build.log
> [INFO] Scanning for projects...
> [INFO]
> [INFO] ---< org.apache.tomee:openejb-core
> >
> [INFO] Building OpenEJB :: Container :: Core 7.0.5
> [INFO] [ jar
> ]-
> [INFO]
> [INFO] --- maven-remote-resources-plugin:1.5:process
> (process-resource-bundles) @ openejb-core ---
> [INFO]
> [INFO] --- maven-resources-plugin:3.0.2:resources (default-resources) @
> openejb-core ---
> [INFO] Using 'UTF-8' encoding to copy filtered resources.
> [INFO] Copying 43 resources
> [INFO] Copying 3 resources
> [INFO]
> [INFO] --- maven-dependency-plugin:3.0.1:copy (copy) @ openejb-core ---
> [INFO] Configured Artifact: org.apache.tomee:openejb-javaagent:7.0.5:jar
> [INFO] org.apache.tomee:openejb-javaagent:7.0.5:jar already exists in
> /Users/jgallimore/dev/tomee/container/openejb-core/target
> [INFO]
> [INFO] --- maven-compiler-plugin:3.6.2:compile (default-compile) @
> openejb-core ---
> [INFO] Nothing to compile - all classes are up to date
> [INFO]
> [INFO] --- dependency-report-plugin:1.0.2:report (default) @ openejb-core
> ---
> [INFO]
> [INFO] --- maven-bundle-plugin:3.3.0:manifest (bundle-manifest) @
> openejb-core ---
> [INFO]
> [INFO] --- maven-antrun-plugin:1.8:run (default) @ openejb-core ---
> [INFO] Executing tasks
>
> main:
> [INFO] Executed tasks
> [INFO]
> [INFO] --- maven-resources-plugin:3.0.2:testResources
> (default-testResources) @ openejb-core ---
> [INFO] Using 'UTF-8' encoding to copy filtered resources.
> [INFO] Copying 103 resources
> [INFO] Copying 3 resources
> [INFO]
> [INFO] --- maven-dependency-plugin:3.0.1:copy (AlternateDriverJarTest) @
> openejb-core ---
> [INFO] Configured Artifact: org.apache.derby:derby:10.10.1.1:jar
> [INFO] Configured Artifact: org.apache.derby:derby:10.9.1.0:jar
> [INFO] org.apache.derby:derby:10.10.1.1:jar already exists in
> /Users/jgallimore/dev/tomee/container/openejb-core/target/drivers
> [INFO] org.apache.derby:derby:10.9.1.0:jar already exists in
> /Users/jgallimore/dev/tomee/container/openejb-core/target/drivers
> [INFO]
> [INFO] --- maven-compiler-plugin:3.6.2:testCompile (default-testCompile) @
> openejb-core ---
> [INFO] Nothing to compile - all classes are up to date
> [INFO]
> [INFO] --- maven-surefire-plugin:2.17:test (default-test) @ openejb-core
> ---
> [INFO] Surefire report directory:
> /Users/jgallimore/dev/tomee/container/openejb-core/target/surefire-reports
>
> ---
>  T E S T S
> ---
>
> ---
>  T E S T S
> ---
> objc[11084]: Class JavaLaunchHelper is implemented in both
>
> /Library/Java/JavaVirtualMachines/jdk1.8.0_141.jdk/Contents/Home/jre/bin/java
> (0x10c0044c0) and
>
> /Library/Java/JavaVirtualMachines/jdk1.8.0_141.jdk/Contents/Home/jre/lib/libinstrument.dylib
> (0x10c0c24e0). One of the two will be used. Which one is undefined.
> 94  classpath-bootstrap  INFO   [main] openjpa.Runtime - OpenJPA
> dynamically loaded a validation provider.
> Running org.apache.openejb.bval.BeanValidationCustomProviderTest
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Created new singletonService
> org.apache.openejb.cdi.ThreadSingletonServiceImpl@12d4bf7e
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Succeeded in installing singleton service
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Cannot find the configuration file [conf/openejb.xml].  Will attempt
> to create one for the beans deployed.
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Configuring Service(id=Default Security Service,
> type=SecurityService, provider-id=Default Security Service)
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Configuring Service(id=Default Transaction Manager,
> type=TransactionManager, provider-id=Default Transaction Manager)
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Creating TransactionManager(id=Default Transaction Manager)
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Creating SecurityService(id=Default Security Service)
> Jul 04, 2018 2:02:57 PM org.apache.openejb.util.LogStreamAsync run
> INFO: Configuring enterprise application:
>
> /Users/jgallimore/dev/tomee/container/openejb-core/target/BeanValidationCustomProviderTest
> Jul 04, 

Re: Issue TOMEE-2063 not solved in TomEE 7.0.4?

2017-11-09 Thread Svetlin Zarev
Interceptor.Priority.PLATFORM_BEFORE has a value of 0. It's also a static
and final [1] which makes it a "compile time constant expression" [2]
So according to LJS 13.4.9-2 [3] the compiled annotation should not have
any reference to Interceptor.Priority.PLATFORM_BEFORE.
So 200 + 0 == 200, and that's exactly what you see in your decompiler
output.


[1]
https://docs.oracle.com/javaee/7/api/javax/interceptor/Interceptor.Priority.html#PLATFORM_BEFORE
[2] https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.28
[3] https://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html#jls-13.4.9

Kind regards,
Svetlin

2017-11-09 15:32 GMT+02:00 Frankie :

> Yes, I saw that in the git repo, but when I download TomEE 7.0.4 PluME, the
> fix seems not to be included, since my built-in check using reflection says
> so ...
>
> Decompiled RequiredInterceptor.class:
> *@Priority(200)*
> instead of:
> *@Priority(Interceptor.Priority.PLATFORM_BEFORE + 200)*
>
>
>
>
> --
> Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-
> f982480.html
>


Re: Issue TOMEE-2063 not solved in TomEE 7.0.4?

2017-11-09 Thread Svetlin Zarev
Hi,

The fix was pushed on 13 Jun [1] , so it is available in 7.0.4

[1]
https://github.com/apache/tomee/commit/29c7594baff0161f358f1e6585fe36730d14f43c


Kind regards,
Svetlin

2017-11-09 14:34 GMT+02:00 Frankie :

> Hello,
>
> it seems, the fix for the resolved Issue  TOMEE-2063
>    ist not included in
> TomEE 7.0.4? Did I misunderstand something?
>
> Thank you
>
> Best regards
> Frankie
>
>
>
> --
> Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-
> f982480.html
>


Re: Kill JVM on deployment failure

2017-11-08 Thread Svetlin Zarev
Hi,

For example this would not work if you have unsatisified resource-ref:



09-Nov-2017 09:38:59.118 SEVERE [localhost-startStop-1]
org.apache.tomee.catalina.TomcatWebAppBuilder.startInternal Unable to
deploy collapsed ear in war
StandardEngine[Catalina].StandardHost[localhost].StandardContext[]
 org.apache.openejb.OpenEJBException: Can't find resource for null. (No
provider available for resource-ref 'null' of type
'xxx.yyy.zzz.KeyStoreService' for '.Comp154478778'.)
at
org.apache.openejb.config.AutoConfig.processResourceRef(AutoConfig.java:1224)
at org.apache.openejb.config.AutoConfig.deploy(AutoConfig.java:873)
at org.apache.openejb.config.AutoConfig.deploy(AutoConfig.java:201)
at
org.apache.openejb.config.ConfigurationFactory$Chain.deploy(ConfigurationFactory.java:420)
at
org.apache.openejb.config.ConfigurationFactory.configureApplication(ConfigurationFactory.java:1037)
at
org.apache.tomee.catalina.TomcatWebAppBuilder.startInternal(TomcatWebAppBuilder.java:1277)
at
org.apache.tomee.catalina.TomcatWebAppBuilder.configureStart(TomcatWebAppBuilder.java:1125)
at
org.apache.tomee.catalina.GlobalListenerSupport.lifecycleEvent(GlobalListenerSupport.java:133

Or when a ServletContextListener fails:

09-Nov-2017 09:51:30.180 SEVERE [localhost-startStop-1]
org.apache.catalina.core.StandardContext.startInternal One or more
listeners failed to start. Full details will be found in the appropriate
container log file
09-Nov-2017 09:51:30.180 SEVERE [localhost-startStop-1]
org.apache.catalina.core.StandardContext.startInternal Context [] startup
failed due to previous errors

I didn't test it with ServletContainerInitializer, but I\m pretty sure it
will not work as well.


Have you considered using Tomcat's LifecycleListener to intercept
LifecycleEvent's ?


Best regards,
Svetlin

2017-11-09 4:50 GMT+02:00 Thiago Veronezi :

> Guys,
>
> https://github.com/apache/tomee/pull/111
>
> I've created this PR to kill the JVM on any deployment failure. I need to
> get an error code from TomEE so docker knows the server died due to some
> application error. This way, it will know it needs to start a new container
> and try it again until the application is successfully deployed.
>
> Do we have something like this already implemented? If yes, how to use it?
>
> If no, do you guys agree with this approach, or do you think theres a
> better way to get a system error?
>
> If you guys agree with the approach, do you have any tip on how to unit
> test this? It looks like we can do it with junit (
> https://stackoverflow.com/questions/309396/java-how-to-
> test-methods-that-call-system-exit),
> but I would like to know if you guys can see any clever way to test it
> without too much bytecode trickery.
>
> Example of output with this PR...
>
> 09-Nov-2017 02:38:39.532 INFO [main]
> org.apache.openejb.config.AutoConfig.createContainer Auto-creating a
> container for bean servlets.Comp1358857082: Container(type=MANAGED,
> id=Default Managed Container)
> 09-Nov-2017 02:38:39.591 INFO [main]
> org.apache.openejb.config.OutputGeneratedDescriptors.writeEjbJar Dumping
> Generated ejb-jar.xml to:
> /opt/tomee/temp/ejb-jar-4480173124784608261ejbs.xml
> 09-Nov-2017 02:38:39.842 INFO [main]
> org.apache.openejb.config.OutputGeneratedDescriptors.writeOpenejbJar
> Dumping Generated openejb-jar.xml to:
> /opt/tomee/temp/openejb-jar-1229834934073978679ejbs.xml
> 09-Nov-2017 02:38:39.888 SEVERE [main]
> org.apache.openejb.config.ReportValidationResults.logResults FAIL ...
> Hello: Missing
> class  com.lala.support.demo.ear.ejb2.hello.HelloHome
> 09-Nov-2017 02:38:39.889 SEVERE [main]
> org.apache.openejb.config.ReportValidationResults.logResults Invalid
> EjbModule(name=ejbs, path=/opt/tomee/apps/app/ejbs.jar)
> 09-Nov-2017 02:38:39.892 INFO [main]
> org.apache.openejb.config.ReportValidationResults.deploy Set the
> 'openejb.validation.output.level' system property to VERBOSE for increased
> validation details.
> 09-Nov-2017 02:38:39.893 WARNING [main]
> org.apache.openejb.config.ConfigurationFactory.configureApplication Jar
> not
> loaded. /opt/tomee/apps/app.ear.  Module failed validation.
> AppModule(name=app)
> 09-Nov-2017 02:38:39.893 WARNING [main]
> org.apache.openejb.config.ConfigurationFactory.getOpenEjbConfiguration
> System property tomee.kill.jvm.on.deployment.failure activated. We will
> kill the JVM due to deployment failure.
> laplata:demo-tomee-ear tveronezi$
>
> []s,
> Thiago.
>


Re: [VOTE] Apache TomEE 7.0.4 - Roll 2

2017-09-26 Thread Svetlin Zarev
+0

It would be great if the tomcat dependency is updated to 8.5.21 because of
the fixed regressions with the SSL configuration

Kind regadrs,
Svetlin

2017-09-27 0:24 GMT+03:00 Andy Gumbrecht :

> Hi Everyone,
>
> I'd kindly like to ask you all to take a look at this build and place your
> votes for a 7.0.4 release.
>
> The re-roll updates to CXF 3.1.13 and JAXB 2.3.0 (Provided).
>
> I added comments in the previous vote that relate to Java 9, which is
> experimental and requires the following (corrected) flag:
>
> --add-module java.xml.bind
>
>
> Staging repo:
> https://repository.apache.org/content/repositories/orgapachetomee-1107/
>
> Source zip:
> https://repository.apache.org/content/repositories/orgapache
> tomee-1107/org/apache/tomee/tomee-project/7.0.4/tomee-
> project-7.0.4-source-release.zip
>
> Dist area:
> https://dist.apache.org/repos/dist/dev/tomee/staging-1107/tomee-7.0.4/
>
> Legal:
> https://dist.apache.org/repos/dist/dev/tomee/staging-1107/to
> mee-7.0.4/legal.zip
>
> Keys:
> https://dist.apache.org/repos/dist/release/tomee/KEYS
>
> Changelog:
> https://issues.apache.org/jira/secure/ReleaseNote.jspa?proje
> ctId=12312320=12339959
>
> Green buildbot:
> https://ci.apache.org/builders/tomee-trunk-ubuntu-jvm8/builds/725
> https://ci.apache.org/builders/tomee-trunk-ubuntu/builds/839
>
> The RAT report indicates 0 Unknown Licenses.
>
> Please vote:
>  +1: Release
>  -1 Do not release because ... See below.
>
> If you vote -1 then please create a JIRA ticket here:
> https://issues.apache.org/jira/projects/TOMEE - Include as much
> information as possible and, if applicable, a unit test.
>
> The vote will be open for 3 days or the consensus is binding (At least 3
> binding votes).
>
> Everyone, committer or not, is encouraged to test and vote. Thank you very
> much for your time.
>
> Andy Gumbrecht.
>
>


Re: 7.0.4 release plans

2017-08-28 Thread Svetlin Zarev
Hi,

I'd suggest to wait until
https://bz.apache.org/bugzilla/show_bug.cgi?id=61450 is fixed or at least
to get more info if it will be fixed in a recent release.

Best regards,
Svetlin

2017-08-15 14:12 GMT+03:00 Jonathan Gallimore 
:

> Hi
>
> I'll give folks an opportunity to shout in case we're waiting on anything I
> don't know about. Assuming that there are no blockers to release
> highlighted, I'll kick off the release. I have a couple of fixes I want to
> look at for 1.7.x and then I'll kick off a 1.7.5 as well.
>
> Jon
>
> On Tue, Aug 15, 2017 at 11:52 AM, Frankie  wrote:
>
> > Is there any new information when this will happen?
> > I checked https://issues.apache.org/jira/projects/TOMEE/versions/
> 12339959
> > but didn't find any hints there. Is there a more up-to-date project
> status
> > page?
> >
> > Thank you
> >
> > Romain Manni-Bucau wrote
> > > Yes, will be released soon normally
> >
> >
> >
> >
> >
> > --
> > View this message in context: http://tomee-openejb.979440.
> > n4.nabble.com/7-0-4-release-plans-tp4682014p4682458.html
> > Sent from the TomEE Dev mailing list archive at Nabble.com.
> >
>


Outdated/Junk PR

2017-08-02 Thread Svetlin Zarev
Hi,

I constantly get notifications about [1], which is now more than 1.5 years
old. It looks like some mistake - i.e to merge the 1.7.x into master.,
Github is regularly sending notifications to all watchers about it So what
do you think about discarding & closing it ?

[1] https://github.com/apache/tomee/pull/29

Kind regards,
Svetlin


Re: [PR] TOMEE-2102: IvmContext bind/unbind creates duplicate contexts

2017-07-25 Thread Svetlin Zarev
Thanks!


2017-07-25 11:24 GMT+03:00 Jonathan Gallimore <jonathan.gallim...@gmail.com>
:

> Thanks for this Svetlin, I've merged it in.
>
> Cheers!
>
> Jon
>
> On Mon, Jul 24, 2017 at 10:53 PM, Jonathan Gallimore <
> jonathan.gallim...@gmail.com> wrote:
>
> > Hi Svetlin,
> >
> > Many thanks for the writeup - really helpful. I'm reviewing now.
> >
> > Jon
> >
> > On Fri, Jul 21, 2017 at 12:43 PM, Svetlin Zarev <
> > svetlin.angelov.za...@gmail.com> wrote:
> >
> >> Hi,
> >>
> >> Yesterday the topic was hijaced by unrelated issues, so I'd like to
> >> restart
> >> the discussion.
> >>
> >>
> >> The issue is that bind() may create alternative NameNodes for the very
> >> same
> >> JNDI name, which results in issues such as being able to bind to objects
> >> to
> >> the same JNDI name, or being unable to lookup an object although it is
> >> bound, or getting different objects uppon lookup() depending on the
> >> current
> >> IvmContext (i.e. NameNode) from which the lookup is being executed.
> >>
> >> The simplest example is:
> >>
> >> IvmContext root = IvmContext.createRootContext();
> >> root.bind("a/b/c, "first");
> >>
> >> IvmContext b = (IvmContext) root.lookup("a/b");
> >> b.bind("c", "second"); //must fail with NameAlreadyBound, yet it
> succeeds.
> >>
> >> So if you do the lookup() for "c" from root, you'll get "first", but if
> >> you
> >> do it relatively from "b", then you'll get "second".
> >>
> >> The issue is that in the first bind, bind() recursively creates the
> >> NameNodes in the subTree of the previous one (except for the toplevel
> >> context - in this case "a" which is bound to either "lessTree" ot
> >> "grtrTree"). But when doing bind relatively to "b", object is not bound
> in
> >> the subTree, but either in less/grtrTree, so as a result we end up
> having
> >> two separate NameNodes for "c".
> >>
> >> I have more cases covered in the unit tests in the first commit in my PR
> >> [1].
> >> I also prepared a short intro [2] into the IvmContext/NameNode logic,
> >> which
> >> I consider essential to understanding the issue and my PR.
> >>
> >> [1] https://github.com/apache/tomee/pull/94
> >> [2] https://gist.github.com/SvetlinZarev/81eb26ea37072634d76fe3721b9509
> eb
> >>
> >> Best regards,
> >> Svetlin
> >>
> >
> >
>


[PR] TOMEE-2102: IvmContext bind/unbind creates duplicate contexts

2017-07-21 Thread Svetlin Zarev
Hi,

Yesterday the topic was hijaced by unrelated issues, so I'd like to restart
the discussion.


The issue is that bind() may create alternative NameNodes for the very same
JNDI name, which results in issues such as being able to bind to objects to
the same JNDI name, or being unable to lookup an object although it is
bound, or getting different objects uppon lookup() depending on the current
IvmContext (i.e. NameNode) from which the lookup is being executed.

The simplest example is:

IvmContext root = IvmContext.createRootContext();
root.bind("a/b/c, "first");

IvmContext b = (IvmContext) root.lookup("a/b");
b.bind("c", "second"); //must fail with NameAlreadyBound, yet it succeeds.

So if you do the lookup() for "c" from root, you'll get "first", but if you
do it relatively from "b", then you'll get "second".

The issue is that in the first bind, bind() recursively creates the
NameNodes in the subTree of the previous one (except for the toplevel
context - in this case "a" which is bound to either "lessTree" ot
"grtrTree"). But when doing bind relatively to "b", object is not bound in
the subTree, but either in less/grtrTree, so as a result we end up having
two separate NameNodes for "c".

I have more cases covered in the unit tests in the first commit in my PR
[1].
I also prepared a short intro [2] into the IvmContext/NameNode logic, which
I consider essential to understanding the issue and my PR.

[1] https://github.com/apache/tomee/pull/94
[2] https://gist.github.com/SvetlinZarev/81eb26ea37072634d76fe3721b9509eb

Best regards,
Svetlin


Re: [jira] [Commented] (TOMEE-2102) IvmContext bind/unbind creates duplicate contexts

2017-07-20 Thread Svetlin Zarev
Actually the inner static class Writer does not have a default constructor.
This was introduced yesterday (commit 5830c209), so maybe it's not that
critical.

https://issues.apache.org/jira/browse/TOMEE-2099

2017-07-20 14:45 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> looks like the class is not packaged in an arquillian war so it is not
> instantiable. is that an option?
>
>
> 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-07-20 13:42 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > Hi,
> >
> > The IvmContext arquillian test passes against current master: [1]
> > The CXF tests fail because of no such method errors:
> > java.lang.NoSuchMethodException:
> > org.apache.openejb.server.cxf.rs.AppPropertiesPropagationTest$
> > Writer.()
> > Maybe CXF was updated ?
> >
> > [1] https://gist.github.com/SvetlinZarev/90d7deb0326e7b670440f0a9a442af
> 1d
> >
> > Kind regards,
> > Svetlin
> >
> > 2017-07-20 14:02 GMT+03:00 Jonathan Gallimore <
> > jonathan.gallim...@gmail.com>
> > :
> >
> > > Happy to try it again, but here's the tests that were failing for me on
> > > master:
> > >
> > > org.apache.openejb.arquillian.tests.naming.IvmContextTest.
> > > testListContextTree
> > > org.apache.openejb.arquillian.tests.naming.IvmContextTest.
> > > testContextListBindings
> > > org.apache.openejb.server.cxf.rs.CustomProviderTest.customProvider
> > > org.apache.openejb.server.cxf.rs.CustomProviderTest.
> > customSpecificProvider
> > > org.apache.openejb.server.cxf.rs.CustomProviderWithConfigTest.config
> > > org.apache.openejb.server.cxf.rs.DiscoverCustomProviderTest.
> > customProvider
> > >
> > > I haven't as yet dug into the root cause of the test failures. Will
> > likely
> > > be this evening before I can do that.
> > >
> > > Jon
> > >
> > > On Thu, Jul 20, 2017 at 9:28 AM, Jonathan Gallimore <
> > > jonathan.gallim...@gmail.com> wrote:
> > >
> > > > Thanks Svetlin - I'll review later today. I'm currently getting some
> > > > IvmContextTest test failures on master - I'll send over a list. Happy
> > to
> > > > help fix these.
> > > >
> > > > Jon
> > > >
> > > > On Thu, Jul 20, 2017 at 9:19 AM, ASF GitHub Bot (JIRA) <
> > j...@apache.org>
> > > > wrote:
> > > >
> > > >>
> > > >> [ https://issues.apache.org/jira/browse/TOMEE-2102?page=com.
> > > >> atlassian.jira.plugin.system.issuetabpanels:comment-tabpane
> > > >> l=16094340#comment-16094340 ]
> > > >>
> > > >> ASF GitHub Bot commented on TOMEE-2102:
> > > >> ---
> > > >>
> > > >> GitHub user SvetlinZarev opened a pull request:
> > > >>
> > > >> https://github.com/apache/tomee/pull/94
> > > >>
> > > >> TOMEE-2102: IvmContext bind/unbind creates duplicate contexts
> > > >>
> > > >>
> > > >>
> > > >> You can merge this pull request into a Git repository by running:
> > > >>
> > > >> $ git pull https://github.com/SvetlinZarev/tomee fixBindUndbind
> > > >>
> > > >> Alternatively you can review and apply these changes as the patch
> at:
> > > >>
> > > >> https://github.com/apache/tomee/pull/94.patch
> > > >>
> > > >> To close this pull request, make a commit to your master/trunk
> branch
> > > >> with (at least) the following in the commit message:
> > > >>
> > > >> This closes #94
> > > >>
> > > >> 
> > > >> commit 12bf481e9bdbaec232c655abb63e1fd496d98fdd
> > > >> Author: Svetlin Zarev <svetlin.za...@sap.com>
> > > >> Date:   2017-07-20T06:44:11Z
> > > >>
> > > >> Add tests that verify the behaviour of IvmContext bind/unbind
> > > >>
> > > >> 
> > > >>
> > > >>
> >

Re: [jira] [Commented] (TOMEE-2102) IvmContext bind/unbind creates duplicate contexts

2017-07-20 Thread Svetlin Zarev
Hi,

The IvmContext arquillian test passes against current master: [1]
The CXF tests fail because of no such method errors:
java.lang.NoSuchMethodException:
org.apache.openejb.server.cxf.rs.AppPropertiesPropagationTest$Writer.()
Maybe CXF was updated ?

[1] https://gist.github.com/SvetlinZarev/90d7deb0326e7b670440f0a9a442af1d

Kind regards,
Svetlin

2017-07-20 14:02 GMT+03:00 Jonathan Gallimore <jonathan.gallim...@gmail.com>
:

> Happy to try it again, but here's the tests that were failing for me on
> master:
>
> org.apache.openejb.arquillian.tests.naming.IvmContextTest.
> testListContextTree
> org.apache.openejb.arquillian.tests.naming.IvmContextTest.
> testContextListBindings
> org.apache.openejb.server.cxf.rs.CustomProviderTest.customProvider
> org.apache.openejb.server.cxf.rs.CustomProviderTest.customSpecificProvider
> org.apache.openejb.server.cxf.rs.CustomProviderWithConfigTest.config
> org.apache.openejb.server.cxf.rs.DiscoverCustomProviderTest.customProvider
>
> I haven't as yet dug into the root cause of the test failures. Will likely
> be this evening before I can do that.
>
> Jon
>
> On Thu, Jul 20, 2017 at 9:28 AM, Jonathan Gallimore <
> jonathan.gallim...@gmail.com> wrote:
>
> > Thanks Svetlin - I'll review later today. I'm currently getting some
> > IvmContextTest test failures on master - I'll send over a list. Happy to
> > help fix these.
> >
> > Jon
> >
> > On Thu, Jul 20, 2017 at 9:19 AM, ASF GitHub Bot (JIRA) <j...@apache.org>
> > wrote:
> >
> >>
> >> [ https://issues.apache.org/jira/browse/TOMEE-2102?page=com.
> >> atlassian.jira.plugin.system.issuetabpanels:comment-tabpane
> >> l=16094340#comment-16094340 ]
> >>
> >> ASF GitHub Bot commented on TOMEE-2102:
> >> ---
> >>
> >> GitHub user SvetlinZarev opened a pull request:
> >>
> >> https://github.com/apache/tomee/pull/94
> >>
> >> TOMEE-2102: IvmContext bind/unbind creates duplicate contexts
> >>
> >>
> >>
> >> You can merge this pull request into a Git repository by running:
> >>
> >> $ git pull https://github.com/SvetlinZarev/tomee fixBindUndbind
> >>
> >> Alternatively you can review and apply these changes as the patch at:
> >>
> >> https://github.com/apache/tomee/pull/94.patch
> >>
> >> To close this pull request, make a commit to your master/trunk branch
> >> with (at least) the following in the commit message:
> >>
> >> This closes #94
> >>
> >> 
> >> commit 12bf481e9bdbaec232c655abb63e1fd496d98fdd
> >> Author: Svetlin Zarev <svetlin.za...@sap.com>
> >> Date:   2017-07-20T06:44:11Z
> >>
> >> Add tests that verify the behaviour of IvmContext bind/unbind
> >>
> >> 
> >>
> >>
> >> > IvmContext bind/unbind creates duplicate contexts
> >> > -
> >> >
> >> > Key: TOMEE-2102
> >> > URL: https://issues.apache.org/jira/browse/TOMEE-2102
> >> > Project: TomEE
> >> >  Issue Type: Bug
> >> >Reporter: Svetlin Zarev
> >> >
> >> > Imagine you have the flowing context "a/b/object". The context tree
> can
> >> be created in two ways:
> >> > 1. Relative to the root or some
> >> > {code}
> >> > IvmContext root = IvmContext.createRootContext();
> >> > root.bind("a/b/object", new Object);
> >> > {code}
> >> > 2. Relative to some node:
> >> > {code}
> >> > IvmContext root = IvmContext.createRootContext();
> >> > root.bind("a", null);
> >> > IvmContext a = root.lookup("a");
> >> > a.bind("b", null);
> >> > IvmContext b = root.lookup("b")
> >> > a.bind("object", new Object())
> >> > {code}
> >> > So when one looks up "object" or "a" or "b" or object, one has to get
> >> the very same result regardless if the context tree was created by 1 or
> by
> >> 2. Yet this is not the case when it comes to the IvmContext. Maybe the
> most
> >> obvious (and shocking) issue is that IvmContext allows to bind 2
> different
> >> objects to the same name ! Example:
> >> > {code}
> >> >IvmContext root = IvmContext.createRootContext();
> >> > root.bind("a/b/object", new Object());
> >> > IvmContext b = (IvmContext) root.lookup("a/b");
> >> > //already bound from root -> must fail, yet it does not
> >> > b.bind("object", new Object());
> >> > {code}
> >> > I've provided various test cases for different combinations of
> >> bind/unbind/lookup that reproduce the issue.
> >>
> >>
> >>
> >> --
> >> This message was sent by Atlassian JIRA
> >> (v6.4.14#64029)
> >>
> >
> >
>


TOMEE-2102: IvmContext bind/unbind creates duplicate contexts

2017-07-20 Thread Svetlin Zarev
Hi,

While i was trying to simplify my IvmTests I stubmbled on another bug in
IvmContext - thi stime in bind()/unbind(). The issue is simple -> if
bind()-ing from a "relative" node (for instance lets have "a/b/c", and you
try to do b.bind(x, obj) instead of root.bind("a/b/x", obj)), bind() binds
the object in less/grtr tree, while if you bind with absolute path from the
root object, it binds in subTree-> letss/grtr. Hence it's possible to bind
two objects to the very same JNDI name !

I've created several junit tests that reproduce the issue. The fix is
simple -> we have to preprend the name of the current name node to the name
passed to bind()/unbind(). Actually that's exactly what lookup() is doing
at the moment.

I'll commit the proposed fix to my PR, as it currently contains only the
tests.


Also I want to challenge several of the existing tests as they are adopted
to the current behavior so they can just pass, for instance ->
IvmContextTest::rebind() has a typo so instead of rebinding it tries to
bind twice to the same jndi name and due to the bug  instead of failing as
required by the spec it passses.

@Jonathan - may you give more info on the issues you are having ? I'd be
happy to help with the IvmContexts.

Kind regards,
Svetlin


Re: [PR] TOMEE-2087 IvmContext.list() does not correctly list the context content

2017-07-19 Thread Svetlin Zarev
Thanks!

> Sveltin, if you’re up for it I can probably use this as an excuse to pull
some Arquillian devs over for some fun hacking :)

I'm not that knowledgeable in Arquillian, but I'll help with whatever I can
:)

> I also say +1.  I’d love to see the layers thinned out of the Arquillian
test, but this could be done in another PR

I removed the servlet, maybe I can remove the bean as well. It's not needed
as long as the test is executed against the IvmContext.

Kind regards,
Svetlin




2017-07-19 10:53 GMT+03:00 Jean-Louis Monteiro <jlmonte...@tomitribe.com>:

> Looks like we are all ok. I'll proceed and merge.
> Thanks
>
> Le 19 juil. 2017 00:56, "David Blevins" <david.blev...@gmail.com> a écrit
> :
>
> I also say +1.  I’d love to see the layers thinned out of the Arquillian
> test, but this could be done in another PR.
>
> Sveltin, if you’re up for it I can probably use this as an excuse to pull
> some Arquillian devs over for some fun hacking :)
>
>
> --
> David Blevins
> http://twitter.com/dblevins
> http://www.tomitribe.com
>
> > On Jul 14, 2017, at 5:14 AM, Romain Manni-Bucau <rmannibu...@gmail.com>
> wrote:
> >
> > looks good to apply to me
> >
> >
> > 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-07-14 12:55 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> >:
> >
> >> Hi,
> >>
> >> Do you have any comments ? Do you thing that something is missing or
> maybe
> >> that something additional is needed ?
> >>
> >> Kind regards,
> >> Svetlin
> >>
> >> 2017-07-11 15:07 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
> >>
> >>> 2017-07-11 12:38 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> >> com
> >>>> :
> >>>
> >>>> I've added several JUnit test cases in openejb-core that should verify
> >>>> IvmContext.list() behaviour, yet I'll feel safer if we keep the
> >>> arquillian
> >>>> test as well.
> >>>>
> >>>
> >>> +1, both are needed
> >>>
> >>>
> >>>>
> >>>> 2017-07-11 10:10 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com
> >:
> >>>>
> >>>>> side note: embedded (not tomcat based) testing is needed to ensure we
> >>>> don't
> >>>>> break but doesn't fully test the ivmcontext code because the
> >> federation
> >>>> is
> >>>>> different so guess starting with tomcat is not that bad.
> >>>>>
> >>>>>
> >>>>> 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-07-11 8:28 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> >>> com
> >>>>> :
> >>>>>
> >>>>>> Hi David,
> >>>>>>
> >>>>>> Thank you for sparing some time to look into my PR !
> >>>>>>
> >>>>>>> I’m not sure I can see how the test actually works
> >>>>>>
> >>>>>> The issue is that IvmContext.list() returns objects that are not
> >>> really
> >>>>>> bound into the listed context. For instance if you run the MCVE
> >>>> attached
> >>>>> to
> >>>>>> the jira ticket you'll see that it returns [1]. There you can see
> >>> that
> >>>>>> TestEJB is bound in "java:" (and even appears several times!) or
> >> that
> >>>>>> "java:global" is bound in "java:module". But if you try to look up
> >>>> those
> >>&g

Re: [PR] TOMEE-2087 IvmContext.list() does not correctly list the context content

2017-07-14 Thread Svetlin Zarev
Hi,

Do you have any comments ? Do you thing that something is missing or maybe
that something additional is needed ?

Kind regards,
Svetlin

2017-07-11 15:07 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> 2017-07-11 12:38 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > I've added several JUnit test cases in openejb-core that should verify
> > IvmContext.list() behaviour, yet I'll feel safer if we keep the
> arquillian
> > test as well.
> >
>
> +1, both are needed
>
>
> >
> > 2017-07-11 10:10 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
> >
> > > side note: embedded (not tomcat based) testing is needed to ensure we
> > don't
> > > break but doesn't fully test the ivmcontext code because the federation
> > is
> > > different so guess starting with tomcat is not that bad.
> > >
> > >
> > > 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-07-11 8:28 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> > >:
> > >
> > > > Hi David,
> > > >
> > > > Thank you for sparing some time to look into my PR !
> > > >
> > > > > I’m not sure I can see how the test actually works
> > > >
> > > > The issue is that IvmContext.list() returns objects that are not
> really
> > > > bound into the listed context. For instance if you run the MCVE
> > attached
> > > to
> > > > the jira ticket you'll see that it returns [1]. There you can see
> that
> > > > TestEJB is bound in "java:" (and even appears several times!) or that
> > > > "java:global" is bound in "java:module". But if you try to look up
> > those
> > > > entries, the lookup fails with a NameNotFoundException, because all
> > these
> > > > references are not really bound there. So the test recursively lists
> > all
> > > > contexts in the JNDI tree and tries to lookup up every name-class
> pair
> > > that
> > > > is returned. If the lookup fails, it means that list() has returned
> > > > something that is not really there. You can compare [1] and [2]
> 9after
> > > the
> > > > fix) to see the difference in list()'s behaviour
> > > >
> > > > > Is there a test for listBindings?  It’s mentioned as also broken,
> > but I
> > > > didn’t see a test for it.
> > > >
> > > > IvmContext.listBindings() and IvmContext.list() use the very same
> > > > IvmContext.MyNamingEnumeration abstract class and share the very same
> > > logic
> > > > to traverse the naming tree. I didn't write test for it because they
> > > share
> > > > the same code, but I can easily modify it to run aginst both methods.
> > > >
> > > > > What is the PrintWriter used for?  It seems it could be useful to
> > > assert
> > > > it prints what we expect.  Not sure if that is there and I am missing
> > it.
> > > >
> > > > I thought it would be helpful to be able to see what went wrong if
> the
> > > test
> > > > fails. The IvmContextTest class collects the output from the
> servlet's
> > > > output stream (the print writer) and if the test fails it prints it
> in
> > > the
> > > > console.
> > > >
> > > > > There is an IvmContextTest, could we put the test there?
> > > >
> > > > That was my initial idea, but AppComposer's naming tree is very
> > different
> > > > that tomee's. For instance it does not have the "app", "global", etc
> > top
> > > > level contexts and my fix has special code for such top-level
> > contexts. I
> > > > also was not bale to bind any env-entries to my EJB (I'm not really
> > > > familiar with how to write a proper appcomposer test, so I guess I
> > didn't
> > > > do something that I should have.). The env-entries are needed just to
> > > > create a couple of branches to the tree in order t

TOMEE-2098: TomcatWebAppBuilder does not correctly handle the case when startInternal(StandardContext) fails

2017-07-13 Thread Svetlin Zarev
Hi,

If the application configuration fails in
TomcatWebAppBuilder.startInternal(StandardContext), the web app builder
tries to "undeploy" the app, but this operation can never succeed because
the StandardContext is in state STARTING_PREP, while this operation
requires it to be in one of NEW, INITIALIZED, FAILED, BEFORE_DESTROY_EVENT,
STOPPED (see LifecycleBase:288). As a result tomcat logs an error: "An
invalid Lifecycle transition was attempted ([before_destroy]) for component
[StandardEngine[Catalina].StandardHost[localhost].StandardContext[/cditest]]
in state [STARTING_PREP]"

and undeploy() basically has no effect. If one tries to *redeploy* the app,
the operation will fail because tomcat would not have cleaned up after
itself.

The correct way to handle this case would be to mark the StandardContext as
unconfigured -> this will make StandardContext move into
LifecycleState.FAILED after it process the lifecycle listeners and do
proper clean up.

Here is PoC of my proposal (just for demo, it has to be cleaned up):
https://github.com/apache/tomee/compare/master...SvetlinZarev:ctxFailsToStart

What do you think ?

Best regards,
Svetlin


Re: @PostConstruct and @PreDestrory

2017-07-13 Thread Svetlin Zarev
Hi,

I'm not sure what will happen if the IvmContext is in read-only mode (i.e.
openejb.forceReadOnlyAppNamingContext=true). You may-need to make the
context writable before unbinding.

Svetlin


2017-07-13 13:46 GMT+03:00 Jonathan Gallimore 
:

> Hey folks
>
> I noticed an issue when creating a resource inside an application:
>
> package org.superbiz;
>
>
> import javax.annotation.PostConstruct;
> import javax.annotation.PreDestroy;
> import java.util.logging.Logger;
>
> public class Startup {
>
> private final Logger log = Logger.getLogger(Startup.class.getName());
>
> @PostConstruct
> public void start() {
> log.info("*** " + getClass().getName() + " started ***");
> }
>
> @PreDestroy
> public void stop() {
> log.info("*** " + getClass().getName() + " stopped ***");
> }
>
> }
>
> and using WEB-INF/resources.xml to create the resource:
>
> 
> 
> # any properties you need can go here
> 
> 
>
> It looks like my @PostConstruct is called ok, but my @PreDestroy is not. I
> believe this works ok with resources defined in tomee.xml, but I'm happy to
> check.
>
> I dug a bit deeper, and wrote a test. This appears to be an issue on both
> master and 1.7.x. I have created two PRs, and would appreciate any
> thoughts.
>
> https://github.com/apache/tomee/pull/91
> https://github.com/apache/tomee/pull/92
>
> Many thanks
>
> Jon
>


Re: Forgotten PRs

2017-07-10 Thread Svetlin Zarev
Thank you!

2017-07-10 14:40 GMT+03:00 Jean-Louis Monteiro <jlmonte...@tomitribe.com>:

> Done.
> Thank you
>
> --
> Jean-Louis Monteiro
> http://twitter.com/jlouismonteiro
> http://www.tomitribe.com
>
> On Mon, Jul 10, 2017 at 1:24 PM, Jean-Louis Monteiro <
> jlmonte...@tomitribe.com> wrote:
>
> > Hi,
> >
> > Thanks for pinging us here.
> > I can merge both for you.
> >
> > Jean-Louis
> >
> > --
> > Jean-Louis Monteiro
> > http://twitter.com/jlouismonteiro
> > http://www.tomitribe.com
> >
> > On Mon, Jul 10, 2017 at 1:15 PM, Svetlin Zarev <
> > svetlin.angelov.za...@gmail.com> wrote:
> >
> >> Hi,
> >>
> >> Would you consider merging [1] which fixes deployment issues in case of
> >> java 8 (it was approved by Romain last week) and [2] which adds a
> missing
> >> test scope to test dependency (approved last week by Romain and
> >> Jean-Louis).
> >>
> >>
> >> [1] https://github.com/apache/tomee/pull/84
> >> [2] https://github.com/apache/tomee/pull/82
> >>
> >> Thanks,
> >> Svetlin
> >>
> >
> >
>


Forgotten PRs

2017-07-10 Thread Svetlin Zarev
Hi,

Would you consider merging [1] which fixes deployment issues in case of
java 8 (it was approved by Romain last week) and [2] which adds a missing
test scope to test dependency (approved last week by Romain and
Jean-Louis).


[1] https://github.com/apache/tomee/pull/84
[2] https://github.com/apache/tomee/pull/82

Thanks,
Svetlin


Re: AutoConfig - IllegalArgumentException: Comparison method violates its general contract!

2017-07-04 Thread Svetlin Zarev
Created TomEE-2085 and https://github.com/apache/tomee/pull/84

What do you think ?

2017-07-04 10:41 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> +1, the indexOf was supposed to be done on getResourceIds(appResources,
> type, required) not the copy (which is sorted)
>
>
> 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-07-04 9:38 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com>:
>
> > Hi,
> >
> > I found a nasty bug in AutoConfig:2077. The comparator does not work
> > correctly with java 8.
> > In older java versions (older than 8), Collections.sort() always creates
> an
> > array from the list content, while starting with java 8 -> it delegates
> to
> > the sort() method implemented in the concrete list implementation.  The
> > implementation of ArrayList, works directly on the inner array, which
> > violates the assumption of the current comparator that the arraylist'
> > sbacking array is not modified in real time.
> >
> > I'll create a jira bug shortly and attach a reproducible test case.
> >
> > Kind regards,
> > Svetlin
> >
>


AutoConfig - IllegalArgumentException: Comparison method violates its general contract!

2017-07-04 Thread Svetlin Zarev
Hi,

I found a nasty bug in AutoConfig:2077. The comparator does not work
correctly with java 8.
In older java versions (older than 8), Collections.sort() always creates an
array from the list content, while starting with java 8 -> it delegates to
the sort() method implemented in the concrete list implementation.  The
implementation of ArrayList, works directly on the inner array, which
violates the assumption of the current comparator that the arraylist'
sbacking array is not modified in real time.

I'll create a jira bug shortly and attach a reproducible test case.

Kind regards,
Svetlin


Re: 7.0.4 release plans

2017-07-03 Thread Svetlin Zarev
I think there is one more dependency that needs to be checked -
OpenWebBeans is with a snapshot version, which makes the build not
reproducible.

2017-07-03 15:11 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> Hi Svetlin,
>
> as soon as this cglib issue is fixed/merged and we reviewed the dependency
> stack (no other "leak") we can release IMO. Build was fixed last week so we
> are all good.
>
>
> 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-07-03 14:09 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > Hi,
> >
> > Are there any plans for releasing 7.0.4 ? It has quite a lot of important
> > fixes and it would be great to have them GA. Also tomcat in 7.0.3 is
> quite
> > old (8.5.11) and there have been several security fixes in tomcat since
> > then [1][2].
> >
> >
> > [1] https://tomcat.apache.org/security-8.html#Fixed_in_
> > Apache_Tomcat_8.5.13
> > [2] https://tomcat.apache.org/security-8.html#Fixed_in_
> > Apache_Tomcat_8.5.15
> >
> > Kind regards,
> > Svetlin
> >
>


7.0.4 release plans

2017-07-03 Thread Svetlin Zarev
Hi,

Are there any plans for releasing 7.0.4 ? It has quite a lot of important
fixes and it would be great to have them GA. Also tomcat in 7.0.3 is quite
old (8.5.11) and there have been several security fixes in tomcat since
then [1][2].


[1] https://tomcat.apache.org/security-8.html#Fixed_in_Apache_Tomcat_8.5.13
[2] https://tomcat.apache.org/security-8.html#Fixed_in_Apache_Tomcat_8.5.15

Kind regards,
Svetlin


Re: Removing ANT from the packaged tomee distributions

2017-07-03 Thread Svetlin Zarev
Thanks!

Some additional info:
* cglib and its dependencies(ant, asm) were added to openejb-ejbd and hence
into tomee
* openejb-ejbd does not have imports to cglib classes
* without csglib a single test fail because of ClassNotFound in RMock

So as Romain suggested it seems to be a missed "test" scope.
Here is the PR: https://github.com/apache/tomee/pull/82

Bets regards,
Svetlin



2017-07-03 14:09 GMT+03:00 Jean-Louis Monteiro <jlmonte...@tomitribe.com>:

> If not needed I'm totally ok.
> Can you submit a PR?
> I'dbe happy to merge it for you
>
>
> Le 3 juil. 2017 11:45, "Svetlin Zarev" <svetlin.angelov.za...@gmail.com> a
> écrit :
>
> > Hi everyone!
> >
> > Recently CGLIB was added as a dependency to TomEE (commit id: 703e9770),
> > and in turn it brought Apache Ant as "compile" dependency. Yet it's not
> > used by cglib at runtime, so it shouldn't really be packaged in the final
> > assembly.
> >
> > What do you think about excluding it from the packaged TomEE
> distributions
> > ?
> >
> > Kind regards,
> > Svetlin
> >
>


Removing ANT from the packaged tomee distributions

2017-07-03 Thread Svetlin Zarev
Hi everyone!

Recently CGLIB was added as a dependency to TomEE (commit id: 703e9770),
and in turn it brought Apache Ant as "compile" dependency. Yet it's not
used by cglib at runtime, so it shouldn't really be packaged in the final
assembly.

What do you think about excluding it from the packaged TomEE distributions
?

Kind regards,
Svetlin


Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-19 Thread 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 <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/
> > >> >> > r

Re: Any EE8 spec API releases needed?

2017-06-17 Thread Svetlin Zarev
Hi Mark,

Have you considered fixing
https://issues.apache.org/jira/browse/GERONIMO-6569 ? It's about common
annotations spec compliance.

Kind regards,
SVetlin

2017-06-17 19:37 GMT+03:00 Mark Struberg :

> Hi folks!
>
> I'm about to release the common-annotation-1.3 and jcdi-2.0 APIs over in
> Geronimo.
> Does TomEE also need some API to be released?
> Would like to do things only once if possible ;)
>
> txs and LieGrue,
> strub


Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread 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 <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.
> 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 <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 -> 

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread 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 <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.za...@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 <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

Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread 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 <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.za...@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
> > > >
> > >
> >
>


Re: CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread 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 <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.za...@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
> >
>


CompManagedBean & @DatasourceDefinition - TOMEE-2053

2017-06-16 Thread 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


Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
It passed with " all-adapters" as well :)

2017-06-12 16:37 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> if you run it with the profile all-adapters from maven command line it will
> run with all tomee flavors (embedded, webprofile, plus, plume) so plume run
> will check with eclipselinks.
>
>
> 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-12 15:34 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > I've added Arquilian test in arquillian-tomee-webprofile-tests.
> > It passes on my machine :) but I'm pretty sure it runs only with
> > OpenJPA. How can I force it to run with eclipselink ?
> >
> > https://github.com/apache/tomee/pull/70
> >
> > 2017-06-12 12:04 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
> >
> > > 2017-06-12 11:02 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> > com
> > > >:
> > >
> > > > OK, I'll update my PR.
> > > >
> > > > I want to write a test as well. In which project
> > > > should I add it, so it's executed with both
> > > > OpenJPA & Eclipse link ?
> > > >
> > > >
> > > you have a few options here:
> > >
> > > 1. openejb-core
> > > 2. add an example in examples/ (we use them as itest sometimes)
> > > 3. arquillian/*tests
> > >
> > > The easiest is the example option probably but take the approach making
> > you
> > > comfortable. Only constraints are 1. don't break the whole build ;), 2.
> > > ensure it is covered :)
> > >
> > >
> > > >
> > > > 2017-06-12 11:43 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com
> >:
> > > >
> > > > > 2017-06-12 10:42 GMT+02:00 Svetlin Zarev
> > <svetlin.angelov.zarev@gmail.
> > > > com
> > > > > >:
> > > > >
> > > > > > I didn't think of that, but still I prefer to do it via the
> proper
> > > > > > interface,
> > > > > > because javax.transaction.Transaction does not expose the
> > interposed
> > > > > list.
> > > > > > So it would be transaction-manager implementation dependent and
> it
> > > > might
> > > > > > stop working if geronimo changes its impl.
> > > > > >
> > > > >
> > > > > Hmm, this is true...and if openejb removes SystemInstance too so
> > guess
> > > it
> > > > > is better to decrease the reflection. Also don't forget the test
> > would
> > > > > break if it changes
> > > > > and we would fix it ;).
> > > > >
> > > > > Side note: we are committer on this geronimo component too so see
> it
> > as
> > > > > part of tomee in term of risk.
> > > > >
> > > > >
> > > > > >
> > > > > > But I can modify my PR to use txn if you insist.
> > > > > >
> > > > > > 2017-06-12 11:36 GMT+03:00 Romain Manni-Bucau <
> > rmannibu...@gmail.com
> > > >:
> > > > > >
> > > > > > > 2017-06-12 10:29 GMT+02:00 Svetlin Zarev
> > > > <svetlin.angelov.zarev@gmail.
> > > > > > com
> > > > > > > >:
> > > > > > >
> > > > > > > > > why not relying on the
> > > > > > > > default? javax.transaction.Transaction.class.cast(txn).
> > > > > > > > registerSynchronization(listener
> > > > > > > >
> > > > > > > > Because it registers the transaction in the "regular"
> > > > > synchronizations
> > > > > > > > list. But
> > > > > > > > if the synchronization is registered via the
> > > > > > > > TransactionSynchronizationRegistry,
> > > > > > > > the synchronization will be registered in the "interposed"
> > > > > > > synchronization
> > > > > > > > list.
> > > > > > > >
> > > >

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
OK, I'll update my PR.

I want to write a test as well. In which project
should I add it, so it's executed with both
OpenJPA & Eclipse link ?


2017-06-12 11:43 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> 2017-06-12 10:42 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > I didn't think of that, but still I prefer to do it via the proper
> > interface,
> > because javax.transaction.Transaction does not expose the interposed
> list.
> > So it would be transaction-manager implementation dependent and it might
> > stop working if geronimo changes its impl.
> >
>
> Hmm, this is true...and if openejb removes SystemInstance too so guess it
> is better to decrease the reflection. Also don't forget the test would
> break if it changes
> and we would fix it ;).
>
> Side note: we are committer on this geronimo component too so see it as
> part of tomee in term of risk.
>
>
> >
> > But I can modify my PR to use txn if you insist.
> >
> > 2017-06-12 11:36 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
> >
> > > 2017-06-12 10:29 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> > com
> > > >:
> > >
> > > > > why not relying on the
> > > > default? javax.transaction.Transaction.class.cast(txn).
> > > > registerSynchronization(listener
> > > >
> > > > Because it registers the transaction in the "regular"
> synchronizations
> > > > list. But
> > > > if the synchronization is registered via the
> > > > TransactionSynchronizationRegistry,
> > > > the synchronization will be registered in the "interposed"
> > > synchronization
> > > > list.
> > > >
> > > > The synchronizations from the interposed list are always executed
> after
> > > the
> > > > synchronizations from the regular list.
> > > >
> > >
> > >
> > > Makes sense. Note you can just get registerInterposedSynchronization
> > from
> > > the txn directly, it would make less reflection and do exactly the
> same.
> > >
> > >
> > > >
> > > > OpenEJB's SessionSynchronizationCoordinator is registered into the
> > > > interposed list, so if we want for @BeforeCompletion to work
> > > > correctly with eclipselink , then either openejb's
> > > > SessionSynchronizationCoordinator
> > > > must be registered in the regular list, or eclipselink's
> > synchronization
> > > > must be registered in the interposed list.  I think that the second
> > > option
> > > > is less invasive.
> > > >
> > >
> > > Your patch (adapted with the previous reflection enhancement probably)
> > > looks good, just miss a test to ensure it works (surely in openejb-core
> > > with application composer)
> > >
> > >
> > > >
> > > >
> > > > 2017-06-12 11:22 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com
> >:
> > > >
> > > > > Hi Svetlin,
> > > > >
> > > > > why not relying on the
> > > > > default? javax.transaction.Transaction.class.cast(txn).
> > > > > registerSynchronization(listener)?
> > > > > This is what your code does except it goes through the registry to
> > find
> > > > the
> > > > > current transaction instead of using the one already passed and
> bound
> > > to
> > > > > the entity manager - should lead to the same if the application
> > doesnt
> > > > > abuse of transactions.
> > > > >
> > > > > The reflection is here cause openejb-core depends on
> > > > > openejb-jpa-integration so if you add openejb-core as a dependency
> it
> > > > > wouldn't build. This is done cause jpa-integration is added to
> > > > applications
> > > > > for the ones providing the jpa provider instead of using the
> > container
> > > > one.
> > > > >
> > > > > Side note: would be good to ensure any PR has some test(s) when
> > > possible
> > > > > otherwise it is easy to break it before next release without even
> > > > noticing
> > > > > it.
> > > > >
> > > > >
> > > > >
> > > > > Romain Manni-Bucau
> > > > > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
>

Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
I didn't think of that, but still I prefer to do it via the proper
interface,
because javax.transaction.Transaction does not expose the interposed list.
So it would be transaction-manager implementation dependent and it might
stop working if geronimo changes its impl.

But I can modify my PR to use txn if you insist.

2017-06-12 11:36 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> 2017-06-12 10:29 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > > why not relying on the
> > default? javax.transaction.Transaction.class.cast(txn).
> > registerSynchronization(listener
> >
> > Because it registers the transaction in the "regular" synchronizations
> > list. But
> > if the synchronization is registered via the
> > TransactionSynchronizationRegistry,
> > the synchronization will be registered in the "interposed"
> synchronization
> > list.
> >
> > The synchronizations from the interposed list are always executed after
> the
> > synchronizations from the regular list.
> >
>
>
> Makes sense. Note you can just get registerInterposedSynchronization from
> the txn directly, it would make less reflection and do exactly the same.
>
>
> >
> > OpenEJB's SessionSynchronizationCoordinator is registered into the
> > interposed list, so if we want for @BeforeCompletion to work
> > correctly with eclipselink , then either openejb's
> > SessionSynchronizationCoordinator
> > must be registered in the regular list, or eclipselink's synchronization
> > must be registered in the interposed list.  I think that the second
> option
> > is less invasive.
> >
>
> Your patch (adapted with the previous reflection enhancement probably)
> looks good, just miss a test to ensure it works (surely in openejb-core
> with application composer)
>
>
> >
> >
> > 2017-06-12 11:22 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
> >
> > > Hi Svetlin,
> > >
> > > why not relying on the
> > > default? javax.transaction.Transaction.class.cast(txn).
> > > registerSynchronization(listener)?
> > > This is what your code does except it goes through the registry to find
> > the
> > > current transaction instead of using the one already passed and bound
> to
> > > the entity manager - should lead to the same if the application doesnt
> > > abuse of transactions.
> > >
> > > The reflection is here cause openejb-core depends on
> > > openejb-jpa-integration so if you add openejb-core as a dependency it
> > > wouldn't build. This is done cause jpa-integration is added to
> > applications
> > > for the ones providing the jpa provider instead of using the container
> > one.
> > >
> > > Side note: would be good to ensure any PR has some test(s) when
> possible
> > > otherwise it is easy to break it before next release without even
> > noticing
> > > it.
> > >
> > >
> > >
> > > 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-12 10:13 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> > com
> > > >:
> > >
> > > > Hi Everyone, Romain,
> > > >
> > > > As JIRA is down, I'm writing to the mailing list.
> > > >
> > > > Recently I reported TOMEE-2057 -> modification to
> > > > entities done in the @BeforeCompletion callback were
> > > > not getting persisted. The root cause was that
> > > > eclipselink's synchronization was executed before
> > > > openejb's one. The same case works as expected
> > > > with OpenJPA.
> > > >
> > > > Hence I want to propose the following fix for the
> > > > eclipselink's integration with tomee: [1] OpenJPA
> > > > does the same, but it's implemented inside OpenJPA.
> > > >
> > > > [1] https://github.com/apache/tomee/pull/70
> > > >
> > > > PS: Is there really need for reflection ? Why don't we add
> > > > dependency to OpenEJB-Core and remove the reflection ?
> > > >
> > > > Best regards,
> > > > Svetlin
> > > >
> > >
> >
>


Re: TOMEE-2057 fix proposal

2017-06-12 Thread Svetlin Zarev
> why not relying on the
default? javax.transaction.Transaction.class.cast(txn).
registerSynchronization(listener

Because it registers the transaction in the "regular" synchronizations
list. But
if the synchronization is registered via the
TransactionSynchronizationRegistry,
the synchronization will be registered in the "interposed" synchronization
list.

The synchronizations from the interposed list are always executed after the
synchronizations from the regular list.

OpenEJB's SessionSynchronizationCoordinator is registered into the
interposed list, so if we want for @BeforeCompletion to work
correctly with eclipselink , then either openejb's
SessionSynchronizationCoordinator
must be registered in the regular list, or eclipselink's synchronization
must be registered in the interposed list.  I think that the second option
is less invasive.


2017-06-12 11:22 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> Hi Svetlin,
>
> why not relying on the
> default? javax.transaction.Transaction.class.cast(txn).
> registerSynchronization(listener)?
> This is what your code does except it goes through the registry to find the
> current transaction instead of using the one already passed and bound to
> the entity manager - should lead to the same if the application doesnt
> abuse of transactions.
>
> The reflection is here cause openejb-core depends on
> openejb-jpa-integration so if you add openejb-core as a dependency it
> wouldn't build. This is done cause jpa-integration is added to applications
> for the ones providing the jpa provider instead of using the container one.
>
> Side note: would be good to ensure any PR has some test(s) when possible
> otherwise it is easy to break it before next release without even noticing
> it.
>
>
>
> 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-12 10:13 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > Hi Everyone, Romain,
> >
> > As JIRA is down, I'm writing to the mailing list.
> >
> > Recently I reported TOMEE-2057 -> modification to
> > entities done in the @BeforeCompletion callback were
> > not getting persisted. The root cause was that
> > eclipselink's synchronization was executed before
> > openejb's one. The same case works as expected
> > with OpenJPA.
> >
> > Hence I want to propose the following fix for the
> > eclipselink's integration with tomee: [1] OpenJPA
> > does the same, but it's implemented inside OpenJPA.
> >
> > [1] https://github.com/apache/tomee/pull/70
> >
> > PS: Is there really need for reflection ? Why don't we add
> > dependency to OpenEJB-Core and remove the reflection ?
> >
> > Best regards,
> > Svetlin
> >
>


Re: Using non-default JsonProvider

2017-06-05 Thread Svetlin Zarev
Hi,

I've tested it and it works like a charm ! may you add johnzon to the
filter list ?

Thanks and best regards,
Svetlin

2017-06-05 12:39 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> Hi Svetlin,
>
> we can add jsonp in
> https://github.com/apache/tomee/blob/master/container/
> openejb-core/src/main/java/org/apache/openejb/util/classloader/
> URLClassLoaderFirst.java#L569
> list to ensure the behavior you desire work (but note we respect the spec
> in our current behavior). Can be workedaround with a custom classloader but
> fixing this is probably saner.
>
>
> 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-05 11:37 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > Hi,
> >
> > If the application brings its own JsonProvider, then
> > JsonProvider.provider() should return it instead of the default one.
> > Instead TomEE always returns the default one (Johnzon). The issue is that
> > JsonProvider.doLoadProvider() returns the first encountered one and does
> > not check if it's the  default, which results in TomEE never loading the
> > application's JsonProvider
> >
> >
> > Do you think if this can be worked around ? I can provide PR for the
> > geronimo-json spec api, but I'm not sure if the project is active.
> >
> > [1]
> > http://docs.oracle.com/javaee/7/api/javax/json/spi/
> > JsonProvider.html#provider--
> >
> > Kind regards,
> > Svetlin
> >
>


Using non-default JsonProvider

2017-06-05 Thread Svetlin Zarev
Hi,

If the application brings its own JsonProvider, then
JsonProvider.provider() should return it instead of the default one.
Instead TomEE always returns the default one (Johnzon). The issue is that
JsonProvider.doLoadProvider() returns the first encountered one and does
not check if it's the  default, which results in TomEE never loading the
application's JsonProvider


Do you think if this can be worked around ? I can provide PR for the
geronimo-json spec api, but I'm not sure if the project is active.

[1]
http://docs.oracle.com/javaee/7/api/javax/json/spi/JsonProvider.html#provider--

Kind regards,
Svetlin


Re: freeze OPENEJB jira project

2017-04-25 Thread Svetlin Zarev
Hi,

You can remove all groups and project roles from the "Create Issue",
"modify issue", add/edit comment etc permissions from the permission scheme
of the project.

Kind regards,
Svetlin

2017-04-25 12:47 GMT+03:00 Romain Manni-Bucau :

> Hi guys,
>
> anyone knows how to make OPENEJB project read only on jira?
>
> We updated the description to redirect people on TOMEE but seems it is not
> read ("oh surprise" ;)) so I'd like we make it read only now to avoid to
> have to maintain 2 projects which are actually the same now.
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github  rmannibucau> |
> LinkedIn  | JavaEE Factory
> 
>


Re: Customizing the dynamic deployer chain

2017-02-27 Thread Svetlin Zarev
The latest event that is fired before the AutoConfig deployer is the
BeforeDeploymentEvent, which is fired too early and does not contain the
AppModule object. A few ideas come to mind

1. Back to my original idea of extracting the deployer chain creation to a
protected method. This should be safe as it does not modify the behaviour
of TomEE at all. If someone modifies the chain, then it's his
responsibility to verify he has not broken something

2. The PreAutoConfigDeployer (i.e. the developer provided deployer) should
be moved right before the AutoConfig deployer.

3. Maybe a new deployer right before the AutoConfig one, that will fire a
new event - BeforeAutoConfigEvent(AppModule)

4. An event is fired after each deployer ->
AfterDeployerEvent(Deployer.class, AppModule)

What do you think ?

2017-02-27 15:43 GMT+02:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> try this: @Observes Object
>
> you will spy them all ;)
>
>
> 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-02-27 14:40 GMT+01:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > Hi,
> >
> > That event is fired too late :) When it's fired, the AutoConfig deployer
> > has already created the datasource. Is there an event fired just before
> the
> > AutoConfig deployer ?
> >
> > Kind regards,
> > Svetlin
> >
> > 2017-02-24 16:23 GMT+02:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> > >:
> >
> > > This event is quite powerful :) I'll definitely check it.
> > >
> > > Thanks,
> > > Svetlin
> > >
> > > 2017-02-24 16:05 GMT+02:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
> > >
> > >> You can observes an event to do that, no need to tune the deployer
> > chain:
> > >> BeforeAppInfoBuilderEvent
> > >>
> > >>
> > >> 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-02-24 15:02 GMT+01:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> > com
> > >> >:
> > >>
> > >> > I have a DynamicDeployer that creates the Resource objects for the
> > >> > datasources and the ConvertDataSourceDefinitions deployer does the
> > >> (almost)
> > >> > same thing, so I want to remove it from the chain
> > >> >
> > >> > 2017-02-24 15:56 GMT+02:00 Romain Manni-Bucau <
> rmannibu...@gmail.com
> > >:
> > >> >
> > >> > > Hi
> > >> > >
> > >> > > what's the actual goal? (= why do you need to extend
> > >> > ConfigurationFactory?)
> > >> > > Wonder if an event can't solve it if not already there.
> > >> > >
> > >> > >
> > >> > > 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-02-24 14:54 GMT+01:00 Svetlin Zarev
> > <svetlin.angelov.zarev@gmail.
> > >> > com
> > >> > > >:
> > >> > >
> > >> > > > Hello,
> > >> > > >
> > >> > > > I want to subclass the org.apache.openejb.config.Conf
> > >> igurationFactory
> > >> > > > class
> > >> > > > in order to customize the deployer chain. Unfortunately the
> chain
> > is
> > >> > > > constructed inside ConfigurationFactory(offline,
> > >> preAutoConfigDeployer)
> > >> > > > constructor, so when I use the ConfigurationFactory(offline,
> > >> > > deployerChain,
> > >> > > > configuration) constructor I have to copy-paste a huge portion
> of
> > >> the
> > >> > > code
> > >> > > > that initializes the deployer chain. Hence my suggestion: It
> would
> > >> be
> > >> > > great
> > >> > > > if the code that initializes the deployer chain is extracted in
> a
> > >> > > protected
> > >> > > > method that initializes the passed Chain -> protected void
> > >> > > > initDeployerChain(Chain chain). In that way subclasses can reuse
> > >> that
> > >> > > code
> > >> > > > while being able to painlessly add/remove dynamic deployers.
> > >> > > >
> > >> > > > Kind regards,
> > >> > > > Svetlin
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>


Re: Customizing the dynamic deployer chain

2017-02-27 Thread Svetlin Zarev
Hi,

That event is fired too late :) When it's fired, the AutoConfig deployer
has already created the datasource. Is there an event fired just before the
AutoConfig deployer ?

Kind regards,
Svetlin

2017-02-24 16:23 GMT+02:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com>:

> This event is quite powerful :) I'll definitely check it.
>
> Thanks,
> Svetlin
>
> 2017-02-24 16:05 GMT+02:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
>
>> You can observes an event to do that, no need to tune the deployer chain:
>> BeforeAppInfoBuilderEvent
>>
>>
>> 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-02-24 15:02 GMT+01:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
>> >:
>>
>> > I have a DynamicDeployer that creates the Resource objects for the
>> > datasources and the ConvertDataSourceDefinitions deployer does the
>> (almost)
>> > same thing, so I want to remove it from the chain
>> >
>> > 2017-02-24 15:56 GMT+02:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
>> >
>> > > Hi
>> > >
>> > > what's the actual goal? (= why do you need to extend
>> > ConfigurationFactory?)
>> > > Wonder if an event can't solve it if not already there.
>> > >
>> > >
>> > > 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-02-24 14:54 GMT+01:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
>> > com
>> > > >:
>> > >
>> > > > Hello,
>> > > >
>> > > > I want to subclass the org.apache.openejb.config.Conf
>> igurationFactory
>> > > > class
>> > > > in order to customize the deployer chain. Unfortunately the chain is
>> > > > constructed inside ConfigurationFactory(offline,
>> preAutoConfigDeployer)
>> > > > constructor, so when I use the ConfigurationFactory(offline,
>> > > deployerChain,
>> > > > configuration) constructor I have to copy-paste a huge portion of
>> the
>> > > code
>> > > > that initializes the deployer chain. Hence my suggestion: It would
>> be
>> > > great
>> > > > if the code that initializes the deployer chain is extracted in a
>> > > protected
>> > > > method that initializes the passed Chain -> protected void
>> > > > initDeployerChain(Chain chain). In that way subclasses can reuse
>> that
>> > > code
>> > > > while being able to painlessly add/remove dynamic deployers.
>> > > >
>> > > > Kind regards,
>> > > > Svetlin
>> > > >
>> > >
>> >
>>
>
>


Customizing the dynamic deployer chain

2017-02-24 Thread Svetlin Zarev
Hello,

I want to subclass the org.apache.openejb.config.ConfigurationFactory class
in order to customize the deployer chain. Unfortunately the chain is
constructed inside ConfigurationFactory(offline, preAutoConfigDeployer)
constructor, so when I use the ConfigurationFactory(offline, deployerChain,
configuration) constructor I have to copy-paste a huge portion of the code
that initializes the deployer chain. Hence my suggestion: It would be great
if the code that initializes the deployer chain is extracted in a protected
method that initializes the passed Chain -> protected void
initDeployerChain(Chain chain). In that way subclasses can reuse that code
while being able to painlessly add/remove dynamic deployers.

Kind regards,
Svetlin


Re: OpenEJB custom configuration factory & bval.in-container=true

2017-02-07 Thread Svetlin Zarev
> Side note (find it funny): you complain this property is skipped if you
use
> another constructor but same applies to the auto deploy one ;)

The auto deploy one (the no-arg constructor) sets this property :) While
the others - do not. Which results in a very unpleasant surprise when using
custom configuration factory. So may idea was to make the behavior
consistent among the different constructors. Also it's much easier to
override or check the value of the property if it's in system.properties,
rather than searching throughout the whole codebase. When I tried CDI
enabled app on my tomee with custom configuration factory, the deployment
started to fail because there were two BeanValidator classes and it was
pure luck that I accidentally discovered that hidden property.

Svetlin

2017-02-07 11:41 GMT+02:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> Hi
>
> 2017-02-07 10:31 GMT+01:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com
> >:
>
> > Hello,
> >
> > In *one* of the configuration factory constructors a system property is
> > set:
> >
> > public ConfigurationFactory() {
> > this(!shouldAutoDeploy());
> > System.setProperty("bval.in-container", "true");
> > }
> >
> > What do you think about moving this to system.properties ? The reason is
> > that:
> >
>
> well, not an option cause 80% of "tomee" usages are not in tomee and don't
> have system.properties - but doesn't mean we can't move it, just that the
> file will not work
>
>
> > * it's really hidden and not obvious why it's there
> >
>
> Was the most common piece of code executed early enough when added and also
> a place you can override and change if you desire to keep default behavior
> or have a custom one. We can surely make it part of SystemInstance but
> override will be harder.
>
>
> > * it's being set every time in case of vanilla tomee
> >
>
> Was one of the goal, not sure I see what the issue is
>
>
> > * it's not set when using custom configuration factory when you use one
> of
> > the other constructors (and this breaks CDI)
> >
>
> Not set: was part of the goal to let the custom factory change that easily
> but agree we can find a better place
> Breaks CDI: depends the CDI config actually but by default yes cause
> BValExtension will try to add a validator and validator factory but openejb
> does it as well
>
>
> > * IMO it's a good idea to have all global properties at one place,
> instead
> > of being scattered in the source
> >
>
> Yes and no, there are multiple kind of properties:
>
> - global one ("enforced") like this one: not against aggregating them but
> gain shouldn't be that big
> - speific ones: this needs to be done per code area and not aggregated all
> together (you will not put tomee properties in openejb code for instance)
> - "in case ones": we have some of that kind where it is there just cause it
> can break and it enables the user to make its app working even if default
> is not desired. These ones are not all very official and I don't find it
> that bad to hide them a bit cause we shouldn't encourage them
>
> So story short: we can aggregate several ones but we can't - I think - put
> them all in a single SystemProperties class.
>
> Side note (find it funny): you complain this property is skipped if you use
> another constructor but same applies to the auto deploy one ;)
>
> Finally note official properties are listed on
> http://tomee.apache.org/admin/configuration/server.html
>
> If we miss a bit we can add them but this bval one is an internal one. At
> some point we can even drop it from bval and just detect we run in tomee
> and set it this way instead of using a system property - maybe better?
>
>
> >
> > Kind regards,
> > Svetlin
> >
>


OpenEJB custom configuration factory & bval.in-container=true

2017-02-07 Thread Svetlin Zarev
Hello,

In *one* of the configuration factory constructors a system property is set:

public ConfigurationFactory() {
this(!shouldAutoDeploy());
System.setProperty("bval.in-container", "true");
}

What do you think about moving this to system.properties ? The reason is
that:
* it's really hidden and not obvious why it's there
* it's being set every time in case of vanilla tomee
* it's not set when using custom configuration factory when you use one of
the other constructors (and this breaks CDI)
* IMO it's a good idea to have all global properties at one place, instead
of being scattered in the source

Kind regards,
Svetlin


Re: getResourceAsStream() for a folder

2016-10-13 Thread Svetlin Zarev
Hi,

IMO this is a bug introduced with [1][2]. Although the root cause is in the
bad impl. of FileResource [3] that ignores the IOException and despite the
error, still returns a byte[] as if nothing bad happened.


[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=60146
[2]
https://github.com/apache/tomcat85/blob/5768426dd27422747f2b9b4da5a0927ee3328ffb/java/org/apache/catalina/webresources/CachedResource.java#L267
[3]
https://github.com/apache/tomcat85/blob/5768426dd27422747f2b9b4da5a0927ee3328ffb/java/org/apache/catalina/webresources/FileResource.java#L203

PS: Maybe this should be forwarded to the tomcat dev list ?

Kind regards,
Svetlin

2016-10-13 12:06 GMT+03:00 Romain Manni-Bucau :

> Hi guys,
>
> upgrading tomee to tomcat 8.5.6 I noticed request.getServletContext().
> getResourceAsStream(path) now returns an empty stream for / (or an existing
> folder probably). Not sure it is expected or unintended.
>
> Reading the spec it is not that clear to me since the folder is an existing
> resource but not really a webresource so not it should be considered as a
> resource and therefore having a stream (which doesn't mean much for a
> folder).
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Wordpress Blog
>  | Github  rmannibucau> |
> LinkedIn  | Tomitriber
>  | JavaEE Factory
> 
>


String equality checks in JAXB classes

2016-09-16 Thread Svetlin Zarev
Hi,

The static code analysis revealed that in all JAXB classes strings are
being compared using == and !=. Obviously TomEE works, so this is not such
a big issue, but I cannot explain to myself why it works :) I know that the
string literals are interned by default, but how do you guarantee that the
various XML parsers will return the interned instances and not different
instances ? Is the performance boost that big to make that hack worth it ?
Are you interested in converting all !=/== to using equals() ?

Kind regards,
Svetlin


Re: OpenEJB/TomEE static code analysis

2016-09-16 Thread Svetlin Zarev
I've cleaned up the PR to make it more readable. I'll be grateful for any
comments.

Svetlin

2016-09-14 16:15 GMT+03:00 Svetlin Zarev <svetlin.angelov.za...@gmail.com>:

> Hi,
>
> I think I've fixed[1]  all stream leaks to files or streams opened by
> classLoader.getResoureAsStream(). I intentionally ignored all streams
> opened bu UrlConnection, because the issue with the connection pooling has
> to be researched. I also reopened [2] because the patch is only for
> slurp(File) and not for slurp(URL) or slurp(InputStream) which should not
> close the stream.
>
> [1] https://github.com/apache/tomee/pull/44
> [2] https://github.com/apache/tomee/pull/40
>
> Svetlin
>
> 2016-09-14 10:58 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:
>
>> Tomee is java 7 for 7.x by spec do java 8 is not an option
>>
>> On the fixes: ensure to unit test each non trivial fix but you can put all
>> the "same" category ones in the same pr. Like "ensure streams are closed".
>> Side note on this particular one: since we pool connections sometimes
>> closing the stream would break it at runtime so unit testing is mandatory.
>>
>> Thanks to have a look to that
>>
>> Le 14 sept. 2016 09:48, "Mitia Alexandrov" <mitiaalexand...@gmail.com> a
>> écrit :
>>
>> > Hello,
>> > I've made several such runs from Idea but mainly for Java 8fy reasons.
>> > There are also several thousand places where the code may be rewritten
>> with
>> > java 8 constructions.
>> >
>> > Regards,
>> >
>> > Mitia
>> >
>> > 2016-09-14 10:35 GMT+03:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.c
>> om
>> > >:
>> >
>> > > Dear TomEE developers,
>> > >
>> > > I've been running static code analysis (fortify) against TomEE 7 and
>> as a
>> > > result I have a list of more than 8000 potential issues (I hope most
>> of
>> > > them are false positives). Unfortunately I'm not allowed to share the
>> > list
>> > > itself.
>> > >
>> > > Either way I'll have to go through that list and review every single
>> > > report, but it's impractical to open a bug report for every single
>> issue.
>> > >
>> > > So here are my questions:
>> > > * What would be the best way to handle the situation ?
>> > > * What's the minimum severity level that's worth reporting ?
>> > > * Should I open jira tickets for the minot/trivial/bad-practices
>> issues ?
>> > > * Should I provide PullRequests for the low priority issues or just
>> for
>> > the
>> > > higher priority ones?
>> > >
>> > >
>> > > Kind regards,
>> > > Svetlin
>> > >
>> >
>>
>
>


Re: OpenEJB/TomEE static code analysis

2016-09-14 Thread Svetlin Zarev
Hi,

I think I've fixed[1]  all stream leaks to files or streams opened by
classLoader.getResoureAsStream(). I intentionally ignored all streams
opened bu UrlConnection, because the issue with the connection pooling has
to be researched. I also reopened [2] because the patch is only for
slurp(File) and not for slurp(URL) or slurp(InputStream) which should not
close the stream.

[1] https://github.com/apache/tomee/pull/44
[2] https://github.com/apache/tomee/pull/40

Svetlin

2016-09-14 10:58 GMT+03:00 Romain Manni-Bucau <rmannibu...@gmail.com>:

> Tomee is java 7 for 7.x by spec do java 8 is not an option
>
> On the fixes: ensure to unit test each non trivial fix but you can put all
> the "same" category ones in the same pr. Like "ensure streams are closed".
> Side note on this particular one: since we pool connections sometimes
> closing the stream would break it at runtime so unit testing is mandatory.
>
> Thanks to have a look to that
>
> Le 14 sept. 2016 09:48, "Mitia Alexandrov" <mitiaalexand...@gmail.com> a
> écrit :
>
> > Hello,
> > I've made several such runs from Idea but mainly for Java 8fy reasons.
> > There are also several thousand places where the code may be rewritten
> with
> > java 8 constructions.
> >
> > Regards,
> >
> > Mitia
> >
> > 2016-09-14 10:35 GMT+03:00 Svetlin Zarev <svetlin.angelov.zarev@gmail.
> com
> > >:
> >
> > > Dear TomEE developers,
> > >
> > > I've been running static code analysis (fortify) against TomEE 7 and
> as a
> > > result I have a list of more than 8000 potential issues (I hope most of
> > > them are false positives). Unfortunately I'm not allowed to share the
> > list
> > > itself.
> > >
> > > Either way I'll have to go through that list and review every single
> > > report, but it's impractical to open a bug report for every single
> issue.
> > >
> > > So here are my questions:
> > > * What would be the best way to handle the situation ?
> > > * What's the minimum severity level that's worth reporting ?
> > > * Should I open jira tickets for the minot/trivial/bad-practices
> issues ?
> > > * Should I provide PullRequests for the low priority issues or just for
> > the
> > > higher priority ones?
> > >
> > >
> > > Kind regards,
> > > Svetlin
> > >
> >
>


OpenEJB/TomEE static code analysis

2016-09-14 Thread Svetlin Zarev
Dear TomEE developers,

I've been running static code analysis (fortify) against TomEE 7 and as a
result I have a list of more than 8000 potential issues (I hope most of
them are false positives). Unfortunately I'm not allowed to share the list
itself.

Either way I'll have to go through that list and review every single
report, but it's impractical to open a bug report for every single issue.

So here are my questions:
* What would be the best way to handle the situation ?
* What's the minimum severity level that's worth reporting ?
* Should I open jira tickets for the minot/trivial/bad-practices issues ?
* Should I provide PullRequests for the low priority issues or just for the
higher priority ones?


Kind regards,
Svetlin