You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by an...@apache.org on 2014/09/23 14:51:47 UTC
svn commit: r1626990 -
/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Author: andygumbrecht
Date: Tue Sep 23 12:51:46 2014
New Revision: 1626990
URL: http://svn.apache.org/r1626990
Log:
TOMEE-1359 - Returns a better resolution if found multiple times. Works for known failing tests in tribestream
This is for review - Test case in TomEE to follow.
Modified:
tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Modified: tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
URL: http://svn.apache.org/viewvc/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java?rev=1626990&r1=1626989&r2=1626990&view=diff
==============================================================================
--- tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java (original)
+++ tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java Tue Sep 23 12:51:46 2014
@@ -28,6 +28,10 @@ import org.jboss.arquillian.test.spi.Tes
import org.jboss.arquillian.test.spi.TestEnricher;
import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -46,13 +50,90 @@ public class TomEEInjectionEnricher impl
private AppContext getAppContext(final String className) {
final ContainerSystem containerSystem = SystemInstance.get().getComponent(ContainerSystem.class);
- for (final AppContext app : containerSystem.getAppContexts()) {
+ final List<AppContext> appContexts = containerSystem.getAppContexts();
+
+ final int size = appContexts.size();
+ if (size == 1) {
+ return appContexts.get(0);
+ }
+
+ final List<AppContext> found = new ArrayList<AppContext>(size);
+
+ for (final AppContext app : appContexts) {
final BeanContext context = containerSystem.getBeanContext(app.getId() + "_" + className);
if (context != null) {
- return app;
+ found.add(app);
}
}
+ if (found.size() > 0) {
+
+ Collections.sort(found, new Comparator<AppContext>() {
+
+ /**
+ * If multiple apps are found that contain the test class then a best guess effort needs to be made
+ * to find the context that best matches the test class application.
+ *
+ * @param ac1 AppContext
+ * @param ac2 AppContext
+ * @return int
+ */
+ @Override
+ public int compare(final AppContext ac1, final AppContext ac2) {
+ int c = 0;
+
+ if (isBeanManagerInUse(ac1) && !isBeanManagerInUse(ac2)) {
+ c--;
+ } else if (!isBeanManagerInUse(ac1) && isBeanManagerInUse(ac2)) {
+ c++;
+ }
+
+ if (ac1.isCdiEnabled() && !ac2.isCdiEnabled()) {
+ c--;
+ } else if (!ac1.isCdiEnabled() && ac2.isCdiEnabled()) {
+ c++;
+ }
+
+ int size1 = ac1.getBeanContexts().size();
+ int size2 = ac2.getBeanContexts().size();
+ if (size1 > size2) {
+ c--;
+ } else if (size2 > size1) {
+ c++;
+ }
+
+ size1 = ac1.getBindings().size();
+ size2 = ac2.getBindings().size();
+ if (size1 > size2) {
+ c--;
+ } else if (size2 > size1) {
+ c++;
+ }
+
+ size1 = ac1.getWebContexts().size();
+ size2 = ac2.getWebContexts().size();
+ if (size1 > size2) {
+ c--;
+ } else if (size2 > size1) {
+ c++;
+ }
+
+ return c;
+ }
+
+ private boolean isBeanManagerInUse(final AppContext ac) {
+ try {
+ return ac.getWebBeansContext().getBeanManagerImpl().isInUse();
+ } catch (final Exception e) {
+ return false;
+ }
+ }
+ });
+
+ //Return the most likely candidate
+ return found.get(0);
+ }
+
Logger.getLogger(TomEEInjectionEnricher.class.getName()).log(Level.WARNING, "Failed to find AppContext for: " + className);
return null;
Re: Fwd: svn commit: r1626990 - /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Posted by Andy Gumbrecht <ag...@tomitribe.com>.
Just wrote the same in the test case I just checked in ;-) - Looks odd?
Basically you can see what the issue is now - So if you think there is a
rock solid solution to it then please go for it by all means.
When I check these things in they are 'always' for review, that's why we
are a team and a community.
Andy
On 23/09/2014 19:59, Romain Manni-Bucau wrote:
> Ok think I get it. In this case there are few questions: why does the
> test enrich all apps? Shouldn't you declare some of them as not
> deployable?
>
> Regarding the fix I'm still -1 on it - we used more or less the same
> to get WebBeansContext and that's pure bullshit and it breaks pretty
> easily - but +1 to check that's the class we want with the
> classloader.
>
> wdyt? Can you validate it is enough for your case?
>
>
>
--
Andy Gumbrecht
https://twitter.com/AndyGeeDe
http://www.tomitribe.com
Re: Fwd: svn commit: r1626990 - /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Ok so for this test classloader check is the one I think. Avoid to use
luck (I'd prefer an exception we found.size() == 2 but it would break
your test).
For memories all tests using size (of ejbs, cdi beans etc...) to
select an instance are just broken and breakable easily. We used it in
OWB singleton just to not rewrite all our openejb-core tests but it is
known as broken.
I'll try to fix it tomorrow, since there is a test now and the fix is
quite trivial it should be fast.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-09-23 20:29 GMT+02:00 Andy Gumbrecht <ag...@tomitribe.com>:
> If there is something wrong with the test itself, and I really can't see
> that there is right now, then we need to at least warn in the logs and offer
> a solution/recommendation.
> It was after all something that has only become visible through Java8 - I
> wouldn't make them not deployable just for the sake of the test, it's what
> highlights the issue.
>
> Andy.
>
> On 23/09/2014 19:59, Romain Manni-Bucau wrote:
>>
>> test enrich all apps? Shouldn't you declare some of them as not
>> deployable?
>>
>
> --
> Andy Gumbrecht
> https://twitter.com/AndyGeeDe
> http://www.tomitribe.com
>
Re: Fwd: svn commit: r1626990 - /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Posted by Andy Gumbrecht <ag...@tomitribe.com>.
If there is something wrong with the test itself, and I really can't see
that there is right now, then we need to at least warn in the logs and
offer a solution/recommendation.
It was after all something that has only become visible through Java8 -
I wouldn't make them not deployable just for the sake of the test, it's
what highlights the issue.
Andy.
On 23/09/2014 19:59, Romain Manni-Bucau wrote:
> test enrich all apps? Shouldn't you declare some of them as not
> deployable?
>
--
Andy Gumbrecht
https://twitter.com/AndyGeeDe
http://www.tomitribe.com
Re: Fwd: svn commit: r1626990 - /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Ok think I get it. In this case there are few questions: why does the
test enrich all apps? Shouldn't you declare some of them as not
deployable?
Regarding the fix I'm still -1 on it - we used more or less the same
to get WebBeansContext and that's pure bullshit and it breaks pretty
easily - but +1 to check that's the class we want with the
classloader.
wdyt? Can you validate it is enough for your case?
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
2014-09-23 19:38 GMT+02:00 Andy Gumbrecht <ag...@tomitribe.com>:
> The tests work fine in Java 8 and fail in Java 7, so it's not even close to
> say work only on the test? The issue may be related to the test, but it is
> not the test itself.
>
> The sort algorithm does not rely on CDI, that is just 'one' of 'five'
> rankings used to determine the order.
>
> The test declares several app deployments. The ContainerSystem deployments
> list the following BeanContexts, which looks right - but as you say may be
> wrong (and should be the focus if it is):
>
> yellow_com.fqn.ApplicationApiTest
> blue_com.fqn.ApplicationApiTest
> orange_com.fqn.ApplicationApiTest
> green_com.fqn.ApplicationApiTest
>
> We are looking for 'orange' because that is the actual test app. So if we
> provide 'appname_ + className' as the single criteria to locate this
> AppContext, and only an iterator to find it then we are just using luck.
>
> On Java 8 we just get lucky that 'orange_com.fqn.ApplicationApiTest' is
> first in the list, on Java 7 it's 'green_com.fqn.ApplicationApiTest', so the
> tests clearly fail.
>
> If there is only one AppContext then it is 'highly' likely to be the test
> AppContext for the provided class, else something has gone seriously wrong?
> - You could still check the BeanContexts if you really feel that is worth
> it.
>
> We can revert it if you provide a better solution, which I am sure you will,
> then please do. But not until then, as the fix currently solves the issue.
>
> Andy.
>
>
> On 23/09/2014 15:02, Romain Manni-Bucau wrote:
>>
>> Hmm
>>
>> seems wrong:
>>
>> 1) if a single app you return it -> that's not what we want most of
>> the time and will lead to the issue you reported
>> 2) sort algorithm relies on CDI which is not at all what we want since
>> we support not cdi enrichment as well
>>
>> I'd revert it and work on the test first - well I'll do if you don't ;).
>>
>> Idea is to be able to identify the bean AppContext. ClassLoader can
>> help (we have an utility for it).
>>
>> If classloader is not enough for it then you need to enrich with all
>> appcontexts but I guess it can be a config issue then.
>>
>>
>> Romain Manni-Bucau
>> Twitter: @rmannibucau
>> Blog: http://rmannibucau.wordpress.com/
>> LinkedIn: http://fr.linkedin.com/in/rmannibucau
>> Github: https://github.com/rmannibucau
>>
>>
>>
>> ---------- Forwarded message ----------
>> From: <an...@apache.org>
>> Date: 2014-09-23 14:51 GMT+02:00
>> Subject: svn commit: r1626990 -
>>
>> /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
>> To: commits@tomee.apache.org
>>
>>
>> Author: andygumbrecht
>> Date: Tue Sep 23 12:51:46 2014
>> New Revision: 1626990
>>
>> URL: http://svn.apache.org/r1626990
>> Log:
>> TOMEE-1359 - Returns a better resolution if found multiple times.
>> Works for known failing tests in tribestream
>> This is for review - Test case in TomEE to follow.
>>
>> Modified:
>>
>> tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
>>
>> Modified:
>> tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
>> URL:
>> http://svn.apache.org/viewvc/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java?rev=1626990&r1=1626989&r2=1626990&view=diff
>>
>> ==============================================================================
>> ---
>> tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
>> (original)
>> +++
>> tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
>> Tue Sep 23 12:51:46 2014
>> @@ -28,6 +28,10 @@ import org.jboss.arquillian.test.spi.Tes
>> import org.jboss.arquillian.test.spi.TestEnricher;
>>
>> import java.lang.reflect.Method;
>> +import java.util.ArrayList;
>> +import java.util.Collections;
>> +import java.util.Comparator;
>> +import java.util.List;
>> import java.util.logging.Level;
>> import java.util.logging.Logger;
>>
>> @@ -46,13 +50,90 @@ public class TomEEInjectionEnricher impl
>>
>> private AppContext getAppContext(final String className) {
>> final ContainerSystem containerSystem =
>> SystemInstance.get().getComponent(ContainerSystem.class);
>> - for (final AppContext app : containerSystem.getAppContexts()) {
>> + final List<AppContext> appContexts =
>> containerSystem.getAppContexts();
>> +
>> + final int size = appContexts.size();
>> + if (size == 1) {
>> + return appContexts.get(0);
>> + }
>> +
>> + final List<AppContext> found = new ArrayList<AppContext>(size);
>> +
>> + for (final AppContext app : appContexts) {
>> final BeanContext context =
>> containerSystem.getBeanContext(app.getId() + "_" + className);
>> if (context != null) {
>> - return app;
>> + found.add(app);
>> }
>> }
>>
>> + if (found.size() > 0) {
>> +
>> + Collections.sort(found, new Comparator<AppContext>() {
>> +
>> + /**
>> + * If multiple apps are found that contain the test
>> class then a best guess effort needs to be made
>> + * to find the context that best matches the test
>> class application.
>> + *
>> + * @param ac1 AppContext
>> + * @param ac2 AppContext
>> + * @return int
>> + */
>> + @Override
>> + public int compare(final AppContext ac1, final
>> AppContext ac2) {
>> + int c = 0;
>> +
>> + if (isBeanManagerInUse(ac1) &&
>> !isBeanManagerInUse(ac2)) {
>> + c--;
>> + } else if (!isBeanManagerInUse(ac1) &&
>> isBeanManagerInUse(ac2)) {
>> + c++;
>> + }
>> +
>> + if (ac1.isCdiEnabled() && !ac2.isCdiEnabled()) {
>> + c--;
>> + } else if (!ac1.isCdiEnabled() && ac2.isCdiEnabled())
>> {
>> + c++;
>> + }
>> +
>> + int size1 = ac1.getBeanContexts().size();
>> + int size2 = ac2.getBeanContexts().size();
>> + if (size1 > size2) {
>> + c--;
>> + } else if (size2 > size1) {
>> + c++;
>> + }
>> +
>> + size1 = ac1.getBindings().size();
>> + size2 = ac2.getBindings().size();
>> + if (size1 > size2) {
>> + c--;
>> + } else if (size2 > size1) {
>> + c++;
>> + }
>> +
>> + size1 = ac1.getWebContexts().size();
>> + size2 = ac2.getWebContexts().size();
>> + if (size1 > size2) {
>> + c--;
>> + } else if (size2 > size1) {
>> + c++;
>> + }
>> +
>> + return c;
>> + }
>> +
>> + private boolean isBeanManagerInUse(final AppContext ac) {
>> + try {
>> + return
>> ac.getWebBeansContext().getBeanManagerImpl().isInUse();
>> + } catch (final Exception e) {
>> + return false;
>> + }
>> + }
>> + });
>> +
>> + //Return the most likely candidate
>> + return found.get(0);
>> + }
>> +
>>
>> Logger.getLogger(TomEEInjectionEnricher.class.getName()).log(Level.WARNING,
>> "Failed to find AppContext for: " + className);
>>
>> return null;
>>
>>
>
>
> --
> Andy Gumbrecht
> https://twitter.com/AndyGeeDe
> http://www.tomitribe.com
>
Re: Fwd: svn commit: r1626990 - /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Posted by Andy Gumbrecht <ag...@tomitribe.com>.
The tests work fine in Java 8 and fail in Java 7, so it's not even close
to say work only on the test? The issue may be related to the test, but
it is not the test itself.
The sort algorithm does not rely on CDI, that is just 'one' of 'five'
rankings used to determine the order.
The test declares several app deployments. The ContainerSystem
deployments list the following BeanContexts, which looks right - but as
you say may be wrong (and should be the focus if it is):
yellow_com.fqn.ApplicationApiTest
blue_com.fqn.ApplicationApiTest
orange_com.fqn.ApplicationApiTest
green_com.fqn.ApplicationApiTest
We are looking for 'orange' because that is the actual test app. So if
we provide 'appname_ + className' as the single criteria to locate this
AppContext, and only an iterator to find it then we are just using luck.
On Java 8 we just get lucky that 'orange_com.fqn.ApplicationApiTest' is
first in the list, on Java 7 it's 'green_com.fqn.ApplicationApiTest', so
the tests clearly fail.
If there is only one AppContext then it is 'highly' likely to be the
test AppContext for the provided class, else something has gone
seriously wrong? - You could still check the BeanContexts if you really
feel that is worth it.
We can revert it if you provide a better solution, which I am sure you
will, then please do. But not until then, as the fix currently solves
the issue.
Andy.
On 23/09/2014 15:02, Romain Manni-Bucau wrote:
> Hmm
>
> seems wrong:
>
> 1) if a single app you return it -> that's not what we want most of
> the time and will lead to the issue you reported
> 2) sort algorithm relies on CDI which is not at all what we want since
> we support not cdi enrichment as well
>
> I'd revert it and work on the test first - well I'll do if you don't ;).
>
> Idea is to be able to identify the bean AppContext. ClassLoader can
> help (we have an utility for it).
>
> If classloader is not enough for it then you need to enrich with all
> appcontexts but I guess it can be a config issue then.
>
>
> Romain Manni-Bucau
> Twitter: @rmannibucau
> Blog: http://rmannibucau.wordpress.com/
> LinkedIn: http://fr.linkedin.com/in/rmannibucau
> Github: https://github.com/rmannibucau
>
>
>
> ---------- Forwarded message ----------
> From: <an...@apache.org>
> Date: 2014-09-23 14:51 GMT+02:00
> Subject: svn commit: r1626990 -
> /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
> To: commits@tomee.apache.org
>
>
> Author: andygumbrecht
> Date: Tue Sep 23 12:51:46 2014
> New Revision: 1626990
>
> URL: http://svn.apache.org/r1626990
> Log:
> TOMEE-1359 - Returns a better resolution if found multiple times.
> Works for known failing tests in tribestream
> This is for review - Test case in TomEE to follow.
>
> Modified:
> tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
>
> Modified: tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
> URL: http://svn.apache.org/viewvc/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java?rev=1626990&r1=1626989&r2=1626990&view=diff
> ==============================================================================
> --- tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
> (original)
> +++ tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
> Tue Sep 23 12:51:46 2014
> @@ -28,6 +28,10 @@ import org.jboss.arquillian.test.spi.Tes
> import org.jboss.arquillian.test.spi.TestEnricher;
>
> import java.lang.reflect.Method;
> +import java.util.ArrayList;
> +import java.util.Collections;
> +import java.util.Comparator;
> +import java.util.List;
> import java.util.logging.Level;
> import java.util.logging.Logger;
>
> @@ -46,13 +50,90 @@ public class TomEEInjectionEnricher impl
>
> private AppContext getAppContext(final String className) {
> final ContainerSystem containerSystem =
> SystemInstance.get().getComponent(ContainerSystem.class);
> - for (final AppContext app : containerSystem.getAppContexts()) {
> + final List<AppContext> appContexts = containerSystem.getAppContexts();
> +
> + final int size = appContexts.size();
> + if (size == 1) {
> + return appContexts.get(0);
> + }
> +
> + final List<AppContext> found = new ArrayList<AppContext>(size);
> +
> + for (final AppContext app : appContexts) {
> final BeanContext context =
> containerSystem.getBeanContext(app.getId() + "_" + className);
> if (context != null) {
> - return app;
> + found.add(app);
> }
> }
>
> + if (found.size() > 0) {
> +
> + Collections.sort(found, new Comparator<AppContext>() {
> +
> + /**
> + * If multiple apps are found that contain the test
> class then a best guess effort needs to be made
> + * to find the context that best matches the test
> class application.
> + *
> + * @param ac1 AppContext
> + * @param ac2 AppContext
> + * @return int
> + */
> + @Override
> + public int compare(final AppContext ac1, final
> AppContext ac2) {
> + int c = 0;
> +
> + if (isBeanManagerInUse(ac1) && !isBeanManagerInUse(ac2)) {
> + c--;
> + } else if (!isBeanManagerInUse(ac1) &&
> isBeanManagerInUse(ac2)) {
> + c++;
> + }
> +
> + if (ac1.isCdiEnabled() && !ac2.isCdiEnabled()) {
> + c--;
> + } else if (!ac1.isCdiEnabled() && ac2.isCdiEnabled()) {
> + c++;
> + }
> +
> + int size1 = ac1.getBeanContexts().size();
> + int size2 = ac2.getBeanContexts().size();
> + if (size1 > size2) {
> + c--;
> + } else if (size2 > size1) {
> + c++;
> + }
> +
> + size1 = ac1.getBindings().size();
> + size2 = ac2.getBindings().size();
> + if (size1 > size2) {
> + c--;
> + } else if (size2 > size1) {
> + c++;
> + }
> +
> + size1 = ac1.getWebContexts().size();
> + size2 = ac2.getWebContexts().size();
> + if (size1 > size2) {
> + c--;
> + } else if (size2 > size1) {
> + c++;
> + }
> +
> + return c;
> + }
> +
> + private boolean isBeanManagerInUse(final AppContext ac) {
> + try {
> + return
> ac.getWebBeansContext().getBeanManagerImpl().isInUse();
> + } catch (final Exception e) {
> + return false;
> + }
> + }
> + });
> +
> + //Return the most likely candidate
> + return found.get(0);
> + }
> +
> Logger.getLogger(TomEEInjectionEnricher.class.getName()).log(Level.WARNING,
> "Failed to find AppContext for: " + className);
>
> return null;
>
>
--
Andy Gumbrecht
https://twitter.com/AndyGeeDe
http://www.tomitribe.com
Fwd: svn commit: r1626990 - /tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hmm
seems wrong:
1) if a single app you return it -> that's not what we want most of
the time and will lead to the issue you reported
2) sort algorithm relies on CDI which is not at all what we want since
we support not cdi enrichment as well
I'd revert it and work on the test first - well I'll do if you don't ;).
Idea is to be able to identify the bean AppContext. ClassLoader can
help (we have an utility for it).
If classloader is not enough for it then you need to enrich with all
appcontexts but I guess it can be a config issue then.
Romain Manni-Bucau
Twitter: @rmannibucau
Blog: http://rmannibucau.wordpress.com/
LinkedIn: http://fr.linkedin.com/in/rmannibucau
Github: https://github.com/rmannibucau
---------- Forwarded message ----------
From: <an...@apache.org>
Date: 2014-09-23 14:51 GMT+02:00
Subject: svn commit: r1626990 -
/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
To: commits@tomee.apache.org
Author: andygumbrecht
Date: Tue Sep 23 12:51:46 2014
New Revision: 1626990
URL: http://svn.apache.org/r1626990
Log:
TOMEE-1359 - Returns a better resolution if found multiple times.
Works for known failing tests in tribestream
This is for review - Test case in TomEE to follow.
Modified:
tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Modified: tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
URL: http://svn.apache.org/viewvc/tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java?rev=1626990&r1=1626989&r2=1626990&view=diff
==============================================================================
--- tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
(original)
+++ tomee/tomee/branches/tomee-1.7.x/arquillian/arquillian-tomee-common/src/main/java/org/apache/openejb/arquillian/common/TomEEInjectionEnricher.java
Tue Sep 23 12:51:46 2014
@@ -28,6 +28,10 @@ import org.jboss.arquillian.test.spi.Tes
import org.jboss.arquillian.test.spi.TestEnricher;
import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -46,13 +50,90 @@ public class TomEEInjectionEnricher impl
private AppContext getAppContext(final String className) {
final ContainerSystem containerSystem =
SystemInstance.get().getComponent(ContainerSystem.class);
- for (final AppContext app : containerSystem.getAppContexts()) {
+ final List<AppContext> appContexts = containerSystem.getAppContexts();
+
+ final int size = appContexts.size();
+ if (size == 1) {
+ return appContexts.get(0);
+ }
+
+ final List<AppContext> found = new ArrayList<AppContext>(size);
+
+ for (final AppContext app : appContexts) {
final BeanContext context =
containerSystem.getBeanContext(app.getId() + "_" + className);
if (context != null) {
- return app;
+ found.add(app);
}
}
+ if (found.size() > 0) {
+
+ Collections.sort(found, new Comparator<AppContext>() {
+
+ /**
+ * If multiple apps are found that contain the test
class then a best guess effort needs to be made
+ * to find the context that best matches the test
class application.
+ *
+ * @param ac1 AppContext
+ * @param ac2 AppContext
+ * @return int
+ */
+ @Override
+ public int compare(final AppContext ac1, final
AppContext ac2) {
+ int c = 0;
+
+ if (isBeanManagerInUse(ac1) && !isBeanManagerInUse(ac2)) {
+ c--;
+ } else if (!isBeanManagerInUse(ac1) &&
isBeanManagerInUse(ac2)) {
+ c++;
+ }
+
+ if (ac1.isCdiEnabled() && !ac2.isCdiEnabled()) {
+ c--;
+ } else if (!ac1.isCdiEnabled() && ac2.isCdiEnabled()) {
+ c++;
+ }
+
+ int size1 = ac1.getBeanContexts().size();
+ int size2 = ac2.getBeanContexts().size();
+ if (size1 > size2) {
+ c--;
+ } else if (size2 > size1) {
+ c++;
+ }
+
+ size1 = ac1.getBindings().size();
+ size2 = ac2.getBindings().size();
+ if (size1 > size2) {
+ c--;
+ } else if (size2 > size1) {
+ c++;
+ }
+
+ size1 = ac1.getWebContexts().size();
+ size2 = ac2.getWebContexts().size();
+ if (size1 > size2) {
+ c--;
+ } else if (size2 > size1) {
+ c++;
+ }
+
+ return c;
+ }
+
+ private boolean isBeanManagerInUse(final AppContext ac) {
+ try {
+ return
ac.getWebBeansContext().getBeanManagerImpl().isInUse();
+ } catch (final Exception e) {
+ return false;
+ }
+ }
+ });
+
+ //Return the most likely candidate
+ return found.get(0);
+ }
+
Logger.getLogger(TomEEInjectionEnricher.class.getName()).log(Level.WARNING,
"Failed to find AppContext for: " + className);
return null;