You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by rm...@apache.org on 2014/12/23 16:06:37 UTC

tomee git commit: better validation of @Produces EE fields...not yet fully happy

Repository: tomee
Updated Branches:
  refs/heads/develop 5984e88c8 -> 877fd5059


better validation of @Produces EE fields...not yet fully happy


Project: http://git-wip-us.apache.org/repos/asf/tomee/repo
Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/877fd505
Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/877fd505
Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/877fd505

Branch: refs/heads/develop
Commit: 877fd50597c7223d7209cd53bb47548d41faf0c5
Parents: 5984e88
Author: Romain Manni-Bucau <rm...@apache.org>
Authored: Tue Dec 23 16:00:52 2014 +0100
Committer: Romain Manni-Bucau <rm...@apache.org>
Committed: Tue Dec 23 16:00:52 2014 +0100

----------------------------------------------------------------------
 .../openejb/OpenEJBArchiveProcessor.java        | 20 ++++++++-
 .../openejb/assembler/classic/Assembler.java    | 47 ++++++++++++++++++++
 .../openejb/config/AnnotationDeployer.java      | 44 +++++++++++-------
 .../apache/openejb/config/AppInfoBuilder.java   |  2 +-
 tck/cdi-embedded/src/test/resources/failing.xml |  2 +-
 5 files changed, 96 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tomee/blob/877fd505/arquillian/arquillian-openejb-embedded-5/src/main/java/org/apache/openejb/arquillian/openejb/OpenEJBArchiveProcessor.java
----------------------------------------------------------------------
diff --git a/arquillian/arquillian-openejb-embedded-5/src/main/java/org/apache/openejb/arquillian/openejb/OpenEJBArchiveProcessor.java b/arquillian/arquillian-openejb-embedded-5/src/main/java/org/apache/openejb/arquillian/openejb/OpenEJBArchiveProcessor.java
index e15338c..d3cce71 100644
--- a/arquillian/arquillian-openejb-embedded-5/src/main/java/org/apache/openejb/arquillian/openejb/OpenEJBArchiveProcessor.java
+++ b/arquillian/arquillian-openejb-embedded-5/src/main/java/org/apache/openejb/arquillian/openejb/OpenEJBArchiveProcessor.java
@@ -32,9 +32,11 @@ import org.apache.openejb.jee.EjbJar;
 import org.apache.openejb.jee.ManagedBean;
 import org.apache.openejb.jee.TransactionType;
 import org.apache.openejb.jee.WebApp;
+import org.apache.openejb.jee.WebApp$JAXB;
 import org.apache.openejb.jee.oejb3.EjbDeployment;
 import org.apache.openejb.jee.oejb3.OpenejbJar;
 import org.apache.openejb.loader.IO;
+import org.apache.openejb.sxc.Sxc;
 import org.apache.openejb.util.classloader.URLClassLoaderFirst;
 import org.apache.xbean.finder.archive.ClassesArchive;
 import org.apache.xbean.finder.archive.CompositeArchive;
@@ -147,7 +149,23 @@ public class OpenEJBArchiveProcessor {
             appModule.setDelegateFirst(false);
             appModule.setStandloneWebModule();
 
-            final WebModule webModule = new WebModule(new WebApp(), contextRoot(archive.getName()), loader, "", appModule.getModuleId());
+            WebApp webApp;
+            final Node webXml = archive.get(WEB_INF + "web.xml");
+            if (webXml == null) {
+                webApp = new WebApp();
+            } else {
+                InputStream inputStream = null;
+                try {
+                    inputStream = webXml.getAsset().openStream();
+                    webApp = Sxc.unmarshalJavaee(new WebApp$JAXB(), inputStream);
+                } catch (final Exception e) {
+                    webApp = new WebApp();
+                } finally {
+                    IO.close(inputStream);
+                }
+            }
+
+            final WebModule webModule = new WebModule(webApp, contextRoot(archive.getName()), loader, "", appModule.getModuleId());
             webModule.setUrls(additionalPaths);
             appModule.getWebModules().add(webModule);
         } else if (isEar) { // mainly for CDI TCKs

http://git-wip-us.apache.org/repos/asf/tomee/blob/877fd505/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
index ac2b031..82533c7 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
@@ -80,6 +80,8 @@ import org.apache.openejb.core.ivm.IntraVmProxy;
 import org.apache.openejb.core.ivm.naming.ContextualJndiReference;
 import org.apache.openejb.core.ivm.naming.IvmContext;
 import org.apache.openejb.core.ivm.naming.IvmJndiFactory;
+import org.apache.openejb.core.ivm.naming.JndiUrlReference;
+import org.apache.openejb.core.ivm.naming.Reference;
 import org.apache.openejb.core.security.SecurityContextHandler;
 import org.apache.openejb.core.timer.EjbTimerServiceImpl;
 import org.apache.openejb.core.timer.MemoryTimerStore;
@@ -132,6 +134,7 @@ import org.apache.openejb.util.classloader.ClassLoaderAwareHandler;
 import org.apache.openejb.util.classloader.URLClassLoaderFirst;
 import org.apache.openejb.util.proxy.ProxyFactory;
 import org.apache.openejb.util.proxy.ProxyManager;
+import org.apache.webbeans.component.ResourceBean;
 import org.apache.webbeans.config.WebBeansContext;
 import org.apache.webbeans.logger.JULLoggerFactory;
 import org.apache.webbeans.spi.BeanArchiveService;
@@ -143,6 +146,7 @@ import org.apache.webbeans.spi.ResourceInjectionService;
 import org.apache.webbeans.spi.ScannerService;
 import org.apache.webbeans.spi.TransactionService;
 import org.apache.webbeans.spi.adaptor.ELAdaptor;
+import org.apache.webbeans.spi.api.ResourceReference;
 import org.apache.xbean.finder.ClassLoaders;
 import org.apache.xbean.finder.ResourceFinder;
 import org.apache.xbean.finder.UrlSet;
@@ -154,6 +158,7 @@ import javax.enterprise.context.Dependent;
 import javax.enterprise.context.spi.CreationalContext;
 import javax.enterprise.inject.spi.Bean;
 import javax.enterprise.inject.spi.BeanManager;
+import javax.enterprise.inject.spi.DefinitionException;
 import javax.enterprise.inject.spi.DeploymentException;
 import javax.management.InstanceNotFoundException;
 import javax.management.MBeanRegistrationException;
@@ -946,6 +951,48 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A
             // bind all global values on global context
             bindGlobals(appContext.getBindings());
 
+            // validate @Produces @Resource/@PersistenceX/@EJB once all is bound to JNDI and we have our comp bean
+            if (appContext.isStandaloneModule() && !appContext.getWebContexts().isEmpty()) {
+                final Map<String, Object> bindings = appContext.getWebContexts().iterator().next().getBindings();
+                for (final Bean<?> bean : appContext.getWebBeansContext().getBeanManagerImpl().getBeans()) {
+                    if (ResourceBean.class.isInstance(bean)) {
+                        final ResourceReference reference = ResourceBean.class.cast(bean).getReference();
+                        final String jndi = reference.getJndiName().replace("java:", "");
+                        Object lookup = bindings.get(jndi);
+                        if (Reference.class.isInstance(lookup)) {
+                            try {
+                                lookup = Reference.class.cast(lookup).getContent();
+                            } catch (final Exception e) { // surely too early, let's try some known locations
+                                if (JndiUrlReference.class.isInstance(lookup)) {
+                                    final String path = JndiUrlReference.class.cast(lookup).getJndiName();
+                                    if ("java:comp/BeanManager".equals(path) && reference.getResourceType() != BeanManager.class) {
+                                        throw new DefinitionException(
+                                                "Resource " + reference.getJndiName() + " in " + reference.getOwnerClass() + " can't be cast to a BeanManager");
+                                    } else if ("java:comp/TransactionSynchronizationRegistry".equals(path) && reference.getResourceType() != TransactionSynchronizationRegistry.class) {
+                                        throw new DefinitionException(
+                                                "Resource " + reference.getJndiName() + " in " + reference.getOwnerClass() + " can't be cast to a TransactionSynchronizationRegistry");
+                                    } else if ("java:comp/TransactionManager".equals(path) && reference.getResourceType() != TransactionManager.class) {
+                                        throw new DefinitionException(
+                                                "Resource " + reference.getJndiName() + " in " + reference.getOwnerClass() + " can't be cast to a TransactionManager");
+                                    } else if ("java:comp/ValidatorFactory".equals(path) && reference.getResourceType() != ValidatorFactory.class) {
+                                        throw new DefinitionException(
+                                                "Resource " + reference.getJndiName() + " in " + reference.getOwnerClass() + " can't be cast to a ValidatorFactory");
+                                    } else if ("java:comp/Validator".equals(path) && reference.getResourceType() != Validator.class) {
+                                        throw new DefinitionException(
+                                                "Resource " + reference.getJndiName() + " in " + reference.getOwnerClass() + " can't be cast to a Validator");
+                                    }
+                                }
+                                continue;
+                            }
+                        }
+                        if (lookup != null && !reference.getResourceType().isInstance(lookup)) {
+                            throw new DefinitionException(
+                                    "Resource " + reference.getJndiName() + " in " + reference.getOwnerClass() + " can't be cast, instance is " + lookup);
+                        }
+                    }
+                }
+            }
+
             // deploy MBeans
             for (final String mbean : appInfo.mbeans) {
                 deployMBean(appContext.getWebBeansContext(), classLoader, mbean, appInfo.jmx, appInfo.appId);

http://git-wip-us.apache.org/repos/asf/tomee/blob/877fd505/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java b/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java
index 7c479a1..1f16335 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/config/AnnotationDeployer.java
@@ -3798,16 +3798,7 @@ public class AnnotationDeployer implements DynamicDeployer {
 
                 final Member member = new FieldMember(field.get());
 
-                final JndiReference ref = buildResource(consumer, resource, member);
-
-                // mainly for TCK cause this validation doesn't make sense - using JNDI you can do a lot of things so validating types would be wrong
-                //
-                // real check would be to lookup the entry and validate it matches field type...but can only be done at runtime
-                // and TCK (org.jboss.cdi.tck.tests.implementation.simple.resource.broken.type.ResourceDefinitionWithDifferentTypeTest) don't instantiate it
-                if (ref != null && field.getAnnotation(Produces.class) != null
-                        && "java:comp/BeanManager".equals(ref.getLookupName()) && BeanManager.class != member.getType()) {
-                    throw new DefinitionException("Jndi entry for " + field + " doesn't match actual type (" + BeanManager.class.getName() + ")");
-                }
+                buildResource(consumer, resource, member);
             }
 
             for (final Annotated<Method> method : annotationFinder.findMetaAnnotatedMethods(Resource.class)) {
@@ -4186,7 +4177,7 @@ public class AnnotationDeployer implements DynamicDeployer {
          * @param resource
          * @param member
          */
-        private JndiReference buildResource(final JndiConsumer consumer, final Resource resource, final Member member) {
+        private void buildResource(final JndiConsumer consumer, final Resource resource, final Member member) {
 
             /**
              * Was @Resource used at a class level without specifying the 'name' or 'beanInterface' attributes?
@@ -4194,7 +4185,7 @@ public class AnnotationDeployer implements DynamicDeployer {
             if (member == null) {
                 if (resource.name().length() == 0) {
                     fail(consumer.getJndiConsumerName(), "resourceAnnotation.onClassWithNoName");
-                    return null;
+                    return;
                 }
             }
 
@@ -4216,10 +4207,10 @@ public class AnnotationDeployer implements DynamicDeployer {
                     final Class type = member.getType();
                     if (EntityManager.class.isAssignableFrom(type)) {
                         fail(consumer.getJndiConsumerName(), "resourceRef.onEntityManager", refName);
-                        return null;
+                        return;
                     } else if (EntityManagerFactory.class.isAssignableFrom(type)) {
                         fail(consumer.getJndiConsumerName(), "resourceRef.onEntityManagerFactory", refName);
-                        return null;
+                        return;
                     }
                 }
 
@@ -4344,7 +4335,6 @@ public class AnnotationDeployer implements DynamicDeployer {
                     reference.setLookupName(lookupName);
                 }
             }
-            return reference;
         }
 
         private static Method getLookupMethod(final Class cls) {
@@ -4434,6 +4424,8 @@ public class AnnotationDeployer implements DynamicDeployer {
             if (member != null) {
                 final Class type = member.getType();
                 if (EntityManager.class.isAssignableFrom(type)) {
+                    failIfCdiProducer(member, "EntityManagerFactory");
+
                     /**
                      * Was @PersistenceUnit mistakenly used when @PersistenceContext should have been used?
                      */
@@ -4442,6 +4434,9 @@ public class AnnotationDeployer implements DynamicDeployer {
                     final String name = persistenceUnitRef.getName();
                     validationContext.fail(jndiConsumerName, "persistenceUnitAnnotation.onEntityManager", name);
                 } else if (!EntityManagerFactory.class.isAssignableFrom(type)) {
+                    failIfCdiProducer(member, "EntityManagerFactory");
+
+
                     /**
                      * Was @PersistenceUnit mistakenly used for something that isn't an EntityManagerFactory?
                      */
@@ -4560,11 +4555,15 @@ public class AnnotationDeployer implements DynamicDeployer {
             if (member != null) {
                 final Class type = member.getType();
                 if (EntityManagerFactory.class.isAssignableFrom(type)) {
+                    failIfCdiProducer(member, "EntityManager");
+
                     /**
                      * Was @PersistenceContext mistakenly used when @PersistenceUnit should have been used?
                      */
                     fail(consumer.getJndiConsumerName(), "persistenceContextAnnotation.onEntityManagerFactory", persistenceContextRef.getName());
                 } else if (!EntityManager.class.isAssignableFrom(type)) {
+                    failIfCdiProducer(member, "EntityManager");
+
                     /**
                      * Was @PersistenceContext mistakenly used for something that isn't an EntityManager?
                      */
@@ -4761,7 +4760,14 @@ public class AnnotationDeployer implements DynamicDeployer {
 
             // service qname
             if (serviceRef.getServiceQname() == null && refType != null) {
-                serviceRef.setServiceQname(JaxWsUtils.getServiceQName(refType));
+                try {
+                    serviceRef.setServiceQname(JaxWsUtils.getServiceQName(refType));
+                } catch (final IllegalArgumentException iae) {
+                    if (FieldMember.class.isInstance(member) && FieldMember.class.cast(member).field.getAnnotation(Produces.class) != null) {
+                        throw new DefinitionException(FieldMember.class.cast(member).field + " is not a webservice client");
+                    }
+                    throw iae;
+                }
             }
             if (serviceRef.getServiceQname() == null) {
                 serviceRef.setServiceQname(JaxWsUtils.getServiceQName(serviceInterface));
@@ -5289,6 +5295,12 @@ public class AnnotationDeployer implements DynamicDeployer {
         }
     }
 
+    private static void failIfCdiProducer(final Member member, final String type) {
+        if (FieldMember.class.isInstance(member) && FieldMember.class.cast(member).field.getAnnotation(Produces.class) != null) {
+            throw new DefinitionException(FieldMember.class.cast(member).field + " is not a " + type);
+        }
+    }
+
     private static void addRestClassesToScannedClasses(final WebModule webModule, final Set<Class> classes, final ClassLoader classLoader) throws OpenEJBException {
         for (final String rawClassName : webModule.getRestClasses()) {
             final String className = realClassName(rawClassName);

http://git-wip-us.apache.org/repos/asf/tomee/blob/877fd505/container/openejb-core/src/main/java/org/apache/openejb/config/AppInfoBuilder.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/config/AppInfoBuilder.java b/container/openejb-core/src/main/java/org/apache/openejb/config/AppInfoBuilder.java
index 032cff3..68edcd9 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/config/AppInfoBuilder.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/config/AppInfoBuilder.java
@@ -124,7 +124,7 @@ class AppInfoBuilder {
         final AppInfo appInfo = new AppInfo();
         appInfo.appId = appModule.getModuleId();
         appInfo.path = appModule.getJarLocation();
-        appInfo.standaloneModule = appModule.isStandaloneModule();
+        appInfo.standaloneModule = appModule.isStandaloneModule() || appModule.isWebapp();
         appInfo.delegateFirst = appModule.isDelegateFirst();
         appInfo.watchedResources.addAll(appModule.getWatchedResources());
         appInfo.mbeans.addAll(appModule.getAdditionalLibMbeans());

http://git-wip-us.apache.org/repos/asf/tomee/blob/877fd505/tck/cdi-embedded/src/test/resources/failing.xml
----------------------------------------------------------------------
diff --git a/tck/cdi-embedded/src/test/resources/failing.xml b/tck/cdi-embedded/src/test/resources/failing.xml
index b3e7ce9..13e22fc 100644
--- a/tck/cdi-embedded/src/test/resources/failing.xml
+++ b/tck/cdi-embedded/src/test/resources/failing.xml
@@ -18,7 +18,7 @@
 <suite name="CDI TCK" verbose="0">
   <test name="CDI TCK">
     <classes>
-      <class name="org.jboss.cdi.tck.tests.implementation.simple.resource.broken.type.ResourceDefinitionWithDifferentTypeTest" />
+      <class name="org.jboss.cdi.tck.tests.implementation.simple.resource.broken.type.ws.ResourceDefinitionWithDifferentTypeTest" />
     </classes>
   </test>
 </suite>