Re: MicroProfile JWT 1.1

[Romain Manni-Bucau]

Hi Jon,

yes and no, idea is to be fast and for all producers it works except the
principal which is broken anyway in CDI 1.x so guess this was not fixed

in CDI 2 (tomee 8) we can impl it this way:
https://github.com/apache/geronimo-jwt-auth/blob/master/src/test/java/org/apache/geronimo/microprofile/impl/jwtauth/tck/TckSecurityService.java

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le mar. 30 oct. 2018 à 00:58, Jonathan Gallimore <
jonathan.gallim...@gmail.com> a écrit :

> Here's a question, probably for Mark or Romain. If I turn the proxy *off*
> in org.apache.webbeans.component.PrincipalBean, I'm finding that I get the
> wrong principal injected sometimes. Specifically, I get the whatever is on
> the proxyInstance field here:
>
> https://github.com/apache/openwebbeans/blob/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/ProviderBasedProducer.java#L51
>
> Should this line (line 66)
>
> https://github.com/apache/openwebbeans/blob/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/ProviderBasedProducer.java#L66
> ,
> not simply be:
>
> return provider.get();
>
> as opposed to
>
> proxyInstance = provider.get(); ?
>
> That way, the proxyInstance field would never get set if proxy mode is set
> to false. When proxy is true, this seems to work correctly (although I have
> other unrelated issues in TomEE).
>
> I can probably work around this some other way, but it seems to me like
> that behaviour isn't quite right.
>
> Trying to think of a way to test it - I can probably come up with
> something, but I'd appreciate some pointers. Happy to shift this to
> openwebbeans-dev, and submit a PR. Replying here initially as I ran into
> this while hacking on the JWT code.
>
> Jon
>
> On Wed, Oct 17, 2018 at 12:41 AM Roberto Cortez
> 
> wrote:
>
> > Please, go ahead. Let me know if need anything. Thanks!
> >
> > > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> > jonathan.gallim...@gmail.com> wrote:
> > >
> > > Any objection if I pick this up and have a go at the last tests, or is
> > > someone already working on this?
> > >
> > > On Thu, Sep 27, 2018 at 5:44 PM Romain Manni-Bucau <
> > rmannibu...@gmail.com>
> > > wrote:
> > >
> > >> Yep this feature. Then it must works since we support user principal
> if
> > the
> > >> jwt filter is corretly placed in the filter chain and we must inherit
> > from
> > >> the request principal.
> > >>
> > >> Le jeu. 27 sept. 2018 18:37, Roberto Cortez
>  > >
> > >> a
> > >> écrit :
> > >>
> > >>> I guess you are referring to this, to remove the proxy?
> > >>>
> > >>>
> > >>
> >
> https://github.com/apache/openwebbeans/commit/a21a949fb19247dcc39ee89292a1554b2cf1388e
> > >>> <
> > >>>
> > >>
> >
> https://github.com/apache/openwebbeans/commit/a21a949fb19247dcc39ee89292a1554b2cf1388e
> > 
> > >>>
> > >>> Yes, this one step.
> > >>>
> > >>> By default, we do inject the generic Principal of Tomcat. We probably
> > >> need
> > >>> to check first about the existence of a JWT Principal and then
> fallback
> > >> to
> > >>> the Tomcat one. I think I know how to do it, I was just trying to
> > broaden
> > >>> up the conversation about general integration with EE security.
> > >>>
> > >>> Cheers,
> > >>> Roberto
> > >>>
> >  On 26 Sep 2018, at 07:21, Romain Manni-Bucau  >
> > >>> wrote:
> > 
> >  OWB enable to do it - we did it in geronimo impl to pass tck of jwt
> > >> auth
> >  spec.
> > 
> >  Le mer. 26 sept. 2018 03:28, Roberto Cortez
> > >> 
> > >>> a
> >  écrit :
> > 
> > > Hi,
> > >
> > > I’ve done some work to push our MP JWT implementation from 1.0 to
> > 1.1.
> > >
> > > You can check it here:
> > > https://github.com/apache/tomee/pull/173 <
> > > https://github.com/apache/tomee/pull/173>
> > >
> > > There are still a couple of tests in the TCK that I have to fix
> and a
> > >>> few
> > > things that I would like to improve, but I think the majority of
> the
> > >>> work
> > > is done.
> > >
> > > Some time ago, there was a discussion in the list about how to
> > >> integrate
> > > MP JWT with EE security:
> > >
> > >
> > >>>
> > >>
> >
> http://tomee-openejb.979440.n4.nabble.com/Implementing-Microprofile-JWT-td4683212i40.html
> > > <
> > >
> > >>>
> > >>
> >
> http://tomee-openejb.979440.n4.nabble.com/Implementing-Microprofile-JWT-td4683212i40.html
> > >>
> > >
> > > I believe we need to revisit that conversation and figure out how
> to
> > >>> move
> > > forward.
> > >
> > > Right now for instance, we don’t support injecting a JWT Principal
> > >> since
> > > it clashes with the predefined by CDI. Most likely, we would need
> to
> 

Re: MicroProfile JWT 1.1

[Jonathan Gallimore]

Here's a question, probably for Mark or Romain. If I turn the proxy *off*
in org.apache.webbeans.component.PrincipalBean, I'm finding that I get the
wrong principal injected sometimes. Specifically, I get the whatever is on
the proxyInstance field here:
https://github.com/apache/openwebbeans/blob/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/ProviderBasedProducer.java#L51

Should this line (line 66)
https://github.com/apache/openwebbeans/blob/trunk/webbeans-impl/src/main/java/org/apache/webbeans/portable/ProviderBasedProducer.java#L66,
not simply be:

return provider.get();

as opposed to

proxyInstance = provider.get(); ?

That way, the proxyInstance field would never get set if proxy mode is set
to false. When proxy is true, this seems to work correctly (although I have
other unrelated issues in TomEE).

I can probably work around this some other way, but it seems to me like
that behaviour isn't quite right.

Trying to think of a way to test it - I can probably come up with
something, but I'd appreciate some pointers. Happy to shift this to
openwebbeans-dev, and submit a PR. Replying here initially as I ran into
this while hacking on the JWT code.

Jon

On Wed, Oct 17, 2018 at 12:41 AM Roberto Cortez 
wrote:

> Please, go ahead. Let me know if need anything. Thanks!
>
> > On 16 Oct 2018, at 21:53, Jonathan Gallimore <
> jonathan.gallim...@gmail.com> wrote:
> >
> > Any objection if I pick this up and have a go at the last tests, or is
> > someone already working on this?
> >
> > On Thu, Sep 27, 2018 at 5:44 PM Romain Manni-Bucau <
> rmannibu...@gmail.com>
> > wrote:
> >
> >> Yep this feature. Then it must works since we support user principal if
> the
> >> jwt filter is corretly placed in the filter chain and we must inherit
> from
> >> the request principal.
> >>
> >> Le jeu. 27 sept. 2018 18:37, Roberto Cortez  >
> >> a
> >> écrit :
> >>
> >>> I guess you are referring to this, to remove the proxy?
> >>>
> >>>
> >>
> https://github.com/apache/openwebbeans/commit/a21a949fb19247dcc39ee89292a1554b2cf1388e
> >>> <
> >>>
> >>
> https://github.com/apache/openwebbeans/commit/a21a949fb19247dcc39ee89292a1554b2cf1388e
> 
> >>>
> >>> Yes, this one step.
> >>>
> >>> By default, we do inject the generic Principal of Tomcat. We probably
> >> need
> >>> to check first about the existence of a JWT Principal and then fallback
> >> to
> >>> the Tomcat one. I think I know how to do it, I was just trying to
> broaden
> >>> up the conversation about general integration with EE security.
> >>>
> >>> Cheers,
> >>> Roberto
> >>>
>  On 26 Sep 2018, at 07:21, Romain Manni-Bucau 
> >>> wrote:
> 
>  OWB enable to do it - we did it in geronimo impl to pass tck of jwt
> >> auth
>  spec.
> 
>  Le mer. 26 sept. 2018 03:28, Roberto Cortez
> >> 
> >>> a
>  écrit :
> 
> > Hi,
> >
> > I’ve done some work to push our MP JWT implementation from 1.0 to
> 1.1.
> >
> > You can check it here:
> > https://github.com/apache/tomee/pull/173 <
> > https://github.com/apache/tomee/pull/173>
> >
> > There are still a couple of tests in the TCK that I have to fix and a
> >>> few
> > things that I would like to improve, but I think the majority of the
> >>> work
> > is done.
> >
> > Some time ago, there was a discussion in the list about how to
> >> integrate
> > MP JWT with EE security:
> >
> >
> >>>
> >>
> http://tomee-openejb.979440.n4.nabble.com/Implementing-Microprofile-JWT-td4683212i40.html
> > <
> >
> >>>
> >>
> http://tomee-openejb.979440.n4.nabble.com/Implementing-Microprofile-JWT-td4683212i40.html
> >>
> >
> > I believe we need to revisit that conversation and figure out how to
> >>> move
> > forward.
> >
> > Right now for instance, we don’t support injecting a JWT Principal
> >> since
> > it clashes with the predefined by CDI. Most likely, we would need to
> >>> plugin
> > the JWT Principal lookup in TomcatSecurityService. I’m not sure if we
> >>> want
> > to do it in that way, or if we want to think in something else.
> >
> > Cheers,
> > Roberto
> >>>
> >>>
> >>
>
>

Re: Shutdown issues with ManagedScheduledExecutorService

[Romain Manni-Bucau]

Did you try a semaphore waiting all tasks are done? It likely also needs a
boolean to prevent new submissions before starting waiting for completion.

Side note: @Schedule sounds not bad for your use case ;)

Le lun. 29 oct. 2018 20:55, ChrisOwens  a écrit :

> This is with TomEE plus 7.0.5
>
> I'm getting /Bean ... has been undeployed/ exception on shutdown, which is
> preventing orderly cleanup.  The issue appears to be that a POJO
> implementing Runnable is scheduled for periodic execution; that POJO was
> created with a reference to a @Stateless bean; and the @Stateless bean is
> undeployed before the POJO completes execution.
>
> "Supervisor" is a @Singleton. Supervisor injects, via @Resource, a
> ManagedScheduledExecutorService. I don't do any configuration on this
> service; I just accept what the container gives me.
>
> "Processor" is a @Stateless bean that does all sorts of work including
> entity management, transactions, etc.
>
> Supervisor obtains an instance of Processor via @Inject.
>
> "Runner" is a POJO that implements Runnable. One of its constructor method
> accepts an instance of Processor. (as an aside, The run method of Runner
> checks for Thread.currentThread().isInterrupted and returns immediately if
> true. It also catches InterruptedException (which might be thrown by some
> of
> the blocking methods it invokes), and calls
> Thread.currentThread().interrupt() and returns.)
>
> To get the work done on a regular basis, Supervisor creates an instance of
> Runner, passing Processor as an argument to its constructor, and then uses
> the ManagedScheduledExecutorService to run with fixed delay.  Supervisor
> keeps a list of all the Runner tasks it has created.
>
> Supervisor has a stop() method that performs task.cancel(true) on every
> Runner task it has created. I have tried, alternatively, annotating the
> stop
> method with @PreDestroy, or explicitly invoking it from a
> ServletContextListener.
>
> At shutdown, it looks like Processor is being undeployed before the tasks
> in
> Supervisor are canceled. It appears that the container doesn't know that
> Processor needs to be kept deployed while the Runner tasks are still
> active.
>
> This happens before the Supervisor's stop() method is invoked:
>
>
> I'm having a hard time reducing this to a test case I can provide.
>
>
>
>
>
> --
> Sent from:
> http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html
>

Shutdown issues with ManagedScheduledExecutorService

[ChrisOwens]

This is with TomEE plus 7.0.5

I'm getting /Bean ... has been undeployed/ exception on shutdown, which is
preventing orderly cleanup.  The issue appears to be that a POJO
implementing Runnable is scheduled for periodic execution; that POJO was
created with a reference to a @Stateless bean; and the @Stateless bean is
undeployed before the POJO completes execution.

"Supervisor" is a @Singleton. Supervisor injects, via @Resource, a
ManagedScheduledExecutorService. I don't do any configuration on this
service; I just accept what the container gives me. 

"Processor" is a @Stateless bean that does all sorts of work including
entity management, transactions, etc. 

Supervisor obtains an instance of Processor via @Inject. 

"Runner" is a POJO that implements Runnable. One of its constructor method
accepts an instance of Processor. (as an aside, The run method of Runner
checks for Thread.currentThread().isInterrupted and returns immediately if
true. It also catches InterruptedException (which might be thrown by some of
the blocking methods it invokes), and calls
Thread.currentThread().interrupt() and returns.)

To get the work done on a regular basis, Supervisor creates an instance of
Runner, passing Processor as an argument to its constructor, and then uses
the ManagedScheduledExecutorService to run with fixed delay.  Supervisor
keeps a list of all the Runner tasks it has created. 

Supervisor has a stop() method that performs task.cancel(true) on every
Runner task it has created. I have tried, alternatively, annotating the stop
method with @PreDestroy, or explicitly invoking it from a
ServletContextListener.

At shutdown, it looks like Processor is being undeployed before the tasks in
Supervisor are canceled. It appears that the container doesn't know that
Processor needs to be kept deployed while the Runner tasks are still active. 

This happens before the Supervisor's stop() method is invoked:


I'm having a hard time reducing this to a test case I can provide. 





--
Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html

Shutdown issues with ManagedScheduledExecutorService

[ChrisOwens]

This is with TomEE plus 7.0.5

I'm getting /Bean ... has been undeployed/ exception on shutdown, which is
preventing orderly cleanup.  The issue appears to be that a POJO
implementing Runnable is scheduled for periodic execution; that POJO was
created with a reference to a @Stateless bean; and the @Stateless bean is
undeployed before the POJO completes execution.

"Supervisor" is a @Singleton. Supervisor injects, via @Resource, a
ManagedScheduledExecutorService. I don't do any configuration on this
service; I just accept what the container gives me. 

"Processor" is a @Stateless bean that does all sorts of work including
entity management, transactions, etc. 

Supervisor obtains an instance of Processor via @Inject. 

"Runner" is a POJO that implements Runnable. One of its constructor method
accepts an instance of Processor. (as an aside, The run method of Runner
checks for Thread.currentThread().isInterrupted and returns immediately if
true. It also catches InterruptedException (which might be thrown by some of
the blocking methods it invokes), and calls
Thread.currentThread().interrupt() and returns.)

To get the work done on a regular basis, Supervisor creates an instance of
Runner, passing Processor as an argument to its constructor, and then uses
the ManagedScheduledExecutorService to run with fixed delay.  Supervisor
keeps a list of all the Runner tasks it has created. 

Supervisor has a stop() method that performs task.cancel(true) on every
Runner task it has created. I have tried, alternatively, annotating the stop
method with @PreDestroy, or explicitly invoking it from a
ServletContextListener.

At shutdown, it looks like Processor is being undeployed before the tasks in
Supervisor are canceled. It appears that the container doesn't know that
Processor needs to be kept deployed while the Runner tasks are still active. 

This happens before the Supervisor's stop() method is invoked:


I'm having a hard time reducing this to a test case I can provide. 





--
Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html

Shutdown issues with ManagedScheduledExecutorService

[ChrisOwens]

This is with TomEE plus 7.0.5

I'm getting /Bean ... has been undeployed/ exception on shutdown, which is
preventing orderly cleanup.  The issue appears to be that a POJO
implementing Runnable is scheduled for periodic execution; that POJO was
created with a reference to a @Stateless bean; and the @Stateless bean is
undeployed before the POJO completes execution.

"Supervisor" is a @Singleton. Supervisor injects, via @Resource, a
ManagedScheduledExecutorService. I don't do any configuration on this
service; I just accept what the container gives me. 

"Processor" is a @Stateless bean that does all sorts of work including
entity management, transactions, etc. 

Supervisor obtains an instance of Processor via @Inject. 

"Runner" is a POJO that implements Runnable. One of its constructor method
accepts an instance of Processor. (as an aside, The run method of Runner
checks for Thread.currentThread().isInterrupted and returns immediately if
true. It also catches InterruptedException (which might be thrown by some of
the blocking methods it invokes), and calls
Thread.currentThread().interrupt() and returns.)

To get the work done on a regular basis, Supervisor creates an instance of
Runner, passing Processor as an argument to its constructor, and then uses
the ManagedScheduledExecutorService to run with fixed delay.  Supervisor
keeps a list of all the Runner tasks it has created. 

Supervisor has a stop() method that performs task.cancel(true) on every
Runner task it has created. I have tried, alternatively, annotating the stop
method with @PreDestroy, or explicitly invoking it from a
ServletContextListener.

At shutdown, it looks like Processor is being undeployed before the tasks in
Supervisor are canceled. It appears that the container doesn't know that
Processor needs to be kept deployed while the Runner tasks are still active. 

This happens before the Supervisor's stop() method is invoked:


I'm having a hard time reducing this to a test case I can provide. 







--
Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html

Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11

[Jonathan Gallimore]

We should write a test case for both of these, and agree on them. I'd like
to be able to merge this in with confidence, and without the tests I don't
see how we can do that.

Jon

On Mon, Oct 29, 2018 at 3:35 PM Romain Manni-Bucau 
wrote:

> Hi Daniel,
>
> the point is that this code is reused in several places so we must be as
> clean as we can, here the cases I had in mind:
>
> 1. reused main in another main (so an app calls the main): here the pitfall
> is that the property openejb.classloader.first.disallow-system-load is not
> resetted and the TCLL neither so after the main you use a closed
> classloader (so will likely easily fail)
> 2. if a user relies in java 8 on the classloader being the system one then
> your new classloader level breaks it so we shouldnt create this new
> classloader when not needed and the old ClassPath impl is working IMO. We
> can enrich it with a warn log saying "it will fail in next version" (and we
> drop that code after 1 or 2 releases). Idea is to not break directly users
> who can not even know some of the libs they added in the classpath for
> logging purposes rely on that classloader setup - random example indeed ;).
>
> Romain Manni-Bucau
> @rmannibucau  |  Blog
>  | Old Blog
>  | Github <
> https://github.com/rmannibucau> |
> LinkedIn  | Book
> <
> https://www.packtpub.com/application-development/java-ee-8-high-performance
> >
>
>
> Le lun. 29 oct. 2018 à 16:14, Daniel Cunha  a
> écrit :
>
> > Hi,
> >
> > I updated the PR with tests!
> >
> > Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> > jonathan.gallim...@gmail.com> escreveu:
> >
> > > I saw your note on the PR regarding Romain's feedback.
> > >
> > > For visibility here, Romain is querying whether this patch will cause
> an
> > > issue with folks calling the CLI with a custom classpath using `java
> -cp
> > > ...` under Java 8. I'd suggest we add a test for it, which looks to be
> > what
> > > Daniel is doing.
> > >
> > > Thanks guys.
> > >
> > > Jon
> > >
> > > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha 
> > > wrote:
> > >
> > > > Hi Folks,
> > > >
> > > > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot of feedback
> > from
> > > > Romain (thank you Romain for all comments and help)
> > > >
> > > > I believe which we are in good shape to merge.
> > > > The changes cover from Java 8 till Java 11 which not provide any
> impact
> > > for
> > > > TomEE or application deployed on it.
> > > >
> > > > Please, look at the PR and let me know what you think.
> > > >
> > > > PR: https://github.com/apache/tomee/pull/176
> > > >
> > > > --
> > > > Daniel "soro" Cunha
> > > > https://twitter.com/dvlc_
> > > >
> > >
> >
> >
> > --
> > Daniel "soro" Cunha
> > https://twitter.com/dvlc_
> >
>

Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11

[Romain Manni-Bucau]

Hi Daniel,

the point is that this code is reused in several places so we must be as
clean as we can, here the cases I had in mind:

1. reused main in another main (so an app calls the main): here the pitfall
is that the property openejb.classloader.first.disallow-system-load is not
resetted and the TCLL neither so after the main you use a closed
classloader (so will likely easily fail)
2. if a user relies in java 8 on the classloader being the system one then
your new classloader level breaks it so we shouldnt create this new
classloader when not needed and the old ClassPath impl is working IMO. We
can enrich it with a warn log saying "it will fail in next version" (and we
drop that code after 1 or 2 releases). Idea is to not break directly users
who can not even know some of the libs they added in the classpath for
logging purposes rely on that classloader setup - random example indeed ;).

Romain Manni-Bucau
@rmannibucau  |  Blog
 | Old Blog
 | Github  |
LinkedIn  | Book



Le lun. 29 oct. 2018 à 16:14, Daniel Cunha  a écrit :

> Hi,
>
> I updated the PR with tests!
>
> Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
> jonathan.gallim...@gmail.com> escreveu:
>
> > I saw your note on the PR regarding Romain's feedback.
> >
> > For visibility here, Romain is querying whether this patch will cause an
> > issue with folks calling the CLI with a custom classpath using `java -cp
> > ...` under Java 8. I'd suggest we add a test for it, which looks to be
> what
> > Daniel is doing.
> >
> > Thanks guys.
> >
> > Jon
> >
> > On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha 
> > wrote:
> >
> > > Hi Folks,
> > >
> > > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot of feedback
> from
> > > Romain (thank you Romain for all comments and help)
> > >
> > > I believe which we are in good shape to merge.
> > > The changes cover from Java 8 till Java 11 which not provide any impact
> > for
> > > TomEE or application deployed on it.
> > >
> > > Please, look at the PR and let me know what you think.
> > >
> > > PR: https://github.com/apache/tomee/pull/176
> > >
> > > --
> > > Daniel "soro" Cunha
> > > https://twitter.com/dvlc_
> > >
> >
>
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>

Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11

[Daniel Cunha]

Hi,

I updated the PR with tests!

Em seg, 29 de out de 2018 às 08:27, Jonathan Gallimore <
jonathan.gallim...@gmail.com> escreveu:

> I saw your note on the PR regarding Romain's feedback.
>
> For visibility here, Romain is querying whether this patch will cause an
> issue with folks calling the CLI with a custom classpath using `java -cp
> ...` under Java 8. I'd suggest we add a test for it, which looks to be what
> Daniel is doing.
>
> Thanks guys.
>
> Jon
>
> On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha 
> wrote:
>
> > Hi Folks,
> >
> > I sent a PR on TomEE to fix the TOMEE-2253. I got a lot of feedback from
> > Romain (thank you Romain for all comments and help)
> >
> > I believe which we are in good shape to merge.
> > The changes cover from Java 8 till Java 11 which not provide any impact
> for
> > TomEE or application deployed on it.
> >
> > Please, look at the PR and let me know what you think.
> >
> > PR: https://github.com/apache/tomee/pull/176
> >
> > --
> > Daniel "soro" Cunha
> > https://twitter.com/dvlc_
> >
>


-- 
Daniel "soro" Cunha
https://twitter.com/dvlc_

2 JIRAs submitted when using Java 11 (TomEE 7.1.0 Plus and TomEE 8.0.0-M1 microprofile)

[COURTAULT Francois]

Hello,

Issue TOMEE-2263 - Copy a war in the webapps folder is not taken into account 
using Java 11
Issue TOMEE-2264 - Unable to deploy this war on TomEE Plus using Java 
11


Could you please look at them ?

Best Regards.



This message and any attachments are intended solely for the addressees and may 
contain confidential information. Any unauthorized use or disclosure, either 
whole or partial, is prohibited.
E-mails are susceptible to alteration. Our company shall not be liable for the 
message if altered, changed or falsified. If you are not the intended recipient 
of this message, please delete it and notify the sender.
Although all reasonable efforts have been made to keep this transmission free 
from viruses, the sender will not be liable for damages caused by a transmitted 
virus.

[GitHub] tomee pull request #176: TOMEE-2253 - tomee.sh -version not working properly...

[jgallimore]

Github user jgallimore commented on a diff in the pull request:

https://github.com/apache/tomee/pull/176#discussion_r228923707
  
--- Diff: tomee/apache-tomee/src/test/java/org/apache/tomee/TomEECliIT.java 
---
@@ -0,0 +1,66 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.tomee;
+
+import org.apache.openejb.loader.IO;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileFilter;
+import java.io.IOException;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TomEECliIT {
+
+@Test
+public void testTomEECli() throws IOException, InterruptedException {
+final File jar = new 
File(getClass().getClassLoader().getResource("test-1.0.jar").getFile());
+
+File work = new 
File("target/webprofile-work-dir/").getAbsoluteFile();
+if (!work.exists()) {
+work = new 
File("apache-tomee/target/webprofile-work-dir/").getAbsoluteFile();
+}
+
+final File[] files = work.listFiles(new FileFilter() {
+@Override
+public boolean accept(final File pathname) {
+return pathname.isDirectory() && 
pathname.getName().startsWith("apache-tomcat-");
+}
+});
+
+final File tomee = (null != files ? files[0] : null);
+if (tomee == null) {
+fail("Failed to find Tomcat directory required for this test - 
Ensure you have run at least the maven phase: mvn process-resources");
+}
+
+
+final ProcessBuilder builder = new ProcessBuilder()
+.command("java", "-cp", jar.getAbsolutePath() + ":" +
+tomee.getAbsolutePath() + 
"/lib/openejb-core-8.0.0-SNAPSHOT.jar:" +
+tomee.getAbsolutePath() + 
"/lib/commons-cli-1.2.jar",
+"org.apache.openejb.cli.Bootstrap", 
"classloadertest");
--- End diff --

Add another test with a directory as a classpath element.


---

[GitHub] tomee pull request #176: TOMEE-2253 - tomee.sh -version not working properly...

[jgallimore]

Github user jgallimore commented on a diff in the pull request:

https://github.com/apache/tomee/pull/176#discussion_r228923550
  
--- Diff: tomee/apache-tomee/src/test/java/org/apache/tomee/TomEECliIT.java 
---
@@ -0,0 +1,66 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.tomee;
+
+import org.apache.openejb.loader.IO;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileFilter;
+import java.io.IOException;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TomEECliIT {
+
+@Test
+public void testTomEECli() throws IOException, InterruptedException {
+final File jar = new 
File(getClass().getClassLoader().getResource("test-1.0.jar").getFile());
+
+File work = new 
File("target/webprofile-work-dir/").getAbsoluteFile();
+if (!work.exists()) {
+work = new 
File("apache-tomee/target/webprofile-work-dir/").getAbsoluteFile();
+}
+
+final File[] files = work.listFiles(new FileFilter() {
+@Override
+public boolean accept(final File pathname) {
+return pathname.isDirectory() && 
pathname.getName().startsWith("apache-tomcat-");
+}
+});
+
+final File tomee = (null != files ? files[0] : null);
+if (tomee == null) {
+fail("Failed to find Tomcat directory required for this test - 
Ensure you have run at least the maven phase: mvn process-resources");
+}
+
+
+final ProcessBuilder builder = new ProcessBuilder()
+.command("java", "-cp", jar.getAbsolutePath() + ":" +
+tomee.getAbsolutePath() + 
"/lib/openejb-core-8.0.0-SNAPSHOT.jar:" +
+tomee.getAbsolutePath() + 
"/lib/commons-cli-1.2.jar",
+"org.apache.openejb.cli.Bootstrap", 
"classloadertest");
+
+final Process start = builder.start();
+start.waitFor();
+
+final String result = IO.slurp(start.getInputStream());
--- End diff --

I suspect the logic that creates this output is in the JAR added in 
test/resources, making it a bit invisible. Can you assemble it with ShrinkWrap 
instead?


---

[GitHub] tomee pull request #176: TOMEE-2253 - tomee.sh -version not working properly...

[jgallimore]

Github user jgallimore commented on a diff in the pull request:

https://github.com/apache/tomee/pull/176#discussion_r228922983
  
--- Diff: tomee/apache-tomee/src/test/java/org/apache/tomee/TomEECliIT.java 
---
@@ -0,0 +1,66 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.tomee;
+
+import org.apache.openejb.loader.IO;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileFilter;
+import java.io.IOException;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TomEECliIT {
+
+@Test
+public void testTomEECli() throws IOException, InterruptedException {
+final File jar = new 
File(getClass().getClassLoader().getResource("test-1.0.jar").getFile());
+
+File work = new 
File("target/webprofile-work-dir/").getAbsoluteFile();
+if (!work.exists()) {
+work = new 
File("apache-tomee/target/webprofile-work-dir/").getAbsoluteFile();
+}
+
+final File[] files = work.listFiles(new FileFilter() {
+@Override
+public boolean accept(final File pathname) {
+return pathname.isDirectory() && 
pathname.getName().startsWith("apache-tomcat-");
+}
+});
+
+final File tomee = (null != files ? files[0] : null);
+if (tomee == null) {
+fail("Failed to find Tomcat directory required for this test - 
Ensure you have run at least the maven phase: mvn process-resources");
+}
+
+
+final ProcessBuilder builder = new ProcessBuilder()
+.command("java", "-cp", jar.getAbsolutePath() + ":" +
--- End diff --

Using : won't work on Windows.


---

[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

[jgallimore]

Github user jgallimore commented on the issue:

https://github.com/apache/tomee/pull/176
  
I think adding the test anyway is a good call.


---

[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

[danielsoro]

Github user danielsoro commented on the issue:

https://github.com/apache/tomee/pull/176
  
btw, it is what we already doing on tomee.sh:

`"$_RUNJAVA" $DEBUG $LOGGING_MANAGER -Dopenejb.base="$CATALINA_BASE" -cp 
"$CLASSPATH" org.apache.openejb.cli.Bootstrap "$@"`


https://github.com/apache/tomee/blob/master/tomee/apache-tomee/src/main/resources/tomee.sh#L133


---

Re: TOMEE-2253 - tomee.sh -version not working properly with Java 11

[Jonathan Gallimore]

I saw your note on the PR regarding Romain's feedback.

For visibility here, Romain is querying whether this patch will cause an
issue with folks calling the CLI with a custom classpath using `java -cp
...` under Java 8. I'd suggest we add a test for it, which looks to be what
Daniel is doing.

Thanks guys.

Jon

On Thu, Oct 25, 2018 at 12:35 PM Daniel Cunha  wrote:

> Hi Folks,
>
> I sent a PR on TomEE to fix the TOMEE-2253. I got a lot of feedback from
> Romain (thank you Romain for all comments and help)
>
> I believe which we are in good shape to merge.
> The changes cover from Java 8 till Java 11 which not provide any impact for
> TomEE or application deployed on it.
>
> Please, look at the PR and let me know what you think.
>
> PR: https://github.com/apache/tomee/pull/176
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>

[GitHub] tomee issue #176: TOMEE-2253 - tomee.sh -version not working properly with J...

[danielsoro]

Github user danielsoro commented on the issue:

https://github.com/apache/tomee/pull/176
  
Well.. it is calling inside a try-statement, I believe which the close will 
be called in the end of the program or when some exception occur: 
https://github.com/apache/tomee/pull/176/files#diff-78e3cde3f4b438e49a55568e9cee40d9R162

About java -cp.. I'm going to try provide a test for it.


---