Hi Roberto,
I'll take a look at the PR.
Cheers.
Bruno Baptista
https://twitter.com/brunobat_
On 23/01/19 11:30, Roberto Cortez wrote:
Hi folks,
Let me try to give a full overview on what I have been working on in the last
couple of days. Progress has been a bit slow unfortunately, due to the amount
of combinations and tests that I have to run every time I do a change. On the
other hand, I know understand way better how TomEE does the deployment :)
Anyway, I’m starting to question myself if I’m going in the right direction.
MP EAR Support:
MP CDI Extensions, or any CDI Extension is always loaded if found in the classpath
via the ServiceLoader. For WAR this works fine. On EAR, CDI Deployment is deferred
because it may be contained in the Webapp and not on the EJB jars, and EJB jars are
deployed first (TOMEE-189 and TOMEE-722). Until now, we didn’t rely on CDI to load
any of the server features, so this was fine. With MP, we added the ability to
include / exclude additional urls to be included in the CDI scanner
(https://github.com/apache/tomee/commit/021b9ca8d01a78f5b7ee3438f30fd8901ff60d5b
<https://github.com/apache/tomee/commit/021b9ca8d01a78f5b7ee3438f30fd8901ff60d5b>).
The issue here is that we now need to load only the build in CDI features,
while deferring the internal / possible CDI beans contained in the EAR file.
This might be a possible solution:
https://github.com/apache/tomee/commit/c6397e26e191f717b96934f2e279acfe320451b9
<https://github.com/apache/tomee/commit/c6397e26e191f717b96934f2e279acfe320451b9>.
Servlet / MP Rest endpoint clashing:
If an existent app is only using servlets and has a servlet mapping to the root
context or /*, MP endpoints will override the context root with a REST path and the
servlet will 404. This has nothing to do with MP itself, if you write an app with a
servlet mapping to /* and add a REST endpoint to /, the REST endpoint takes
precedence and you are unable to reach the servlet. My concern here is that someone
out there might run into this and we break their app with the new version of TomEE.
This should probably do the trick:
https://github.com/apache/tomee/commit/4ade980c56276a2ad4f2df921e12314e38e881cf
<https://github.com/apache/tomee/commit/4ade980c56276a2ad4f2df921e12314e38e881cf>
Tomcat TomEE Webapp (with Plus and Plume)
Deployment of the TomEE Webapp in Tomcat with MP was also not working. This was
because CDI scanning for the TomEE web app was disabled by default (since we didn’t
rely on any CDI services before). Also, the web.xml was marked as metadata complete,
which also skips any annotation deployment processing. I’m a little concern with the
change here due to the previous comment that it might affect TomEE embedded, but so
far it seemed fine:
https://github.com/apache/tomee/commit/e55760d0e230612de7f99b7c4940b1305456dbaf
<https://github.com/apache/tomee/commit/e55760d0e230612de7f99b7c4940b1305456dbaf>
ApplicationComposer on Arquillian Remote
Right now, I was not able to have this working. This is because
ApplicationComposer, when using CDI, you manually state in the annotation which
CDI beans are required. In this case, any additional Bean scanning is skipped.
Again, we probably need to adjust it to also include the container provided CDI
beans.
RestEndpoint / OpenAPI
I’ve run into a StringIndexOutOfBoundsException when OpenAPI is processing REST
annotations. I’m now looking into that. Not sure if it might be a bug in
OpenAPI implementation or something else.
Well, this is it for now. Sorry for the long email. All the work has been done
in this PR:
https://github.com/apache/tomee/pull/304/
<https://github.com/apache/tomee/pull/304/>
It would definitely need a few set of eyes to review it.
Thank you!
Cheers,
Roberto
On 23 Jan 2019, at 11:28, Roberto Cortez <radcor...@yahoo.com.INVALID> wrote:
We introduced a couple of new properties to allows additional jars to be added
in the DeploymentLoader, so they can be scanned.
This was done here:
https://github.com/apache/tomee/commit/021b9ca8d01a78f5b7ee3438f30fd8901ff60d5b
<https://github.com/apache/tomee/commit/021b9ca8d01a78f5b7ee3438f30fd8901ff60d5b>
What you probably need to do is have your custom classloader to also load the
mp specific libraries, so they can be also scanned.
Yes, I’ve run into multiple other issues. I’m going to send an email about it,
next. I think we can make it, we just need to validate if these changes make
sense and if they are right.
Thank you,
Roberto
On 23 Jan 2019, at 10:55, j4fm <james.m...@my-managed.net> wrote:
Okay, so digging into this loader, there is this line:
SystemInstance.get().setComponent(ParentClassLoaderFinder.class, fallback ->
MyClassLoader._getOrCreateInstance(parent));
Commenting it out seems to make it play nicely with MP but brakes the class
loading of the webapps when openejb's scanning annotations.
So am currently looking at other solutions to make both work. I think this
is something that can be solved but am suspecting there is a chance there is
a bug with openejb not using the correct class loaders for the webapps.
Trying some things.
Are you still finding issues with MP in Plus with the test cases failing?
Is there anything else blocking having MP in Plus for M2 release? I don't
want to become a blocker for that, will know today if this class loading
issue can be solved or a bug. It would be really great if M2 could have MP
in Plus but understand how tight it is.
--
Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html
