Fixing some interceptor functionality - structural validation, deploy…

…ment error reporting.

impl/src/main/java/org/jboss/weld/BeanManagerImpl.java

@@ -1055,6 +1055,17 @@ private void checkResolveDecoratorsArguments(Set<Type> types, Collection<Annotat
     */
    public List<Interceptor<?>> resolveInterceptors(InterceptionType type, Annotation... interceptorBindings)
    {
+      if (interceptorBindings.length == 0)
+         throw new IllegalArgumentException("Interceptor bindings list cannot be empty");
+      Set<Class<?>> uniqueInterceptorBindings = new HashSet<Class<?>>();
+      for (Annotation interceptorBinding: interceptorBindings)
+      {
+         if (uniqueInterceptorBindings.contains(interceptorBinding.annotationType()))
+            throw new IllegalArgumentException("Duplicate interceptor binding type: " + interceptorBinding.annotationType());
+         if (!isInterceptorBindingType(interceptorBinding.annotationType()))
+            throw new IllegalArgumentException("Trying to resolve interceptors with non-binding type: " + interceptorBinding.annotationType());
+         uniqueInterceptorBindings.add(interceptorBinding.annotationType());
+      }
       return new ArrayList<Interceptor<?>>(interceptorResolver.resolve(ResolvableFactory.of(type,interceptorBindings)));
    }
 
@@ -1280,7 +1291,7 @@ public Set<Annotation> getInterceptorBindingTypeDefinition(Class<? extends Annot
    {
       if (getServices().get(MetaAnnotationStore.class).getInterceptorBindingModel(bindingType).isValid())
       {
-         return getServices().get(MetaAnnotationStore.class).getInterceptorBindingModel(bindingType).getInheritedInterceptionBindingTypes();
+         return getServices().get(MetaAnnotationStore.class).getInterceptorBindingModel(bindingType).getMetaAnnotations();
       }
       else
       {

impl/src/main/java/org/jboss/weld/bean/AbstractClassBean.java

@@ -42,6 +42,7 @@
 import org.jboss.interceptor.model.InterceptionModelBuilder;
 import org.jboss.weld.BeanManagerImpl;
 import org.jboss.weld.DefinitionException;
+import org.jboss.weld.DeploymentException;
 import org.jboss.weld.bean.proxy.DecoratorProxyMethodHandler;
 import org.jboss.weld.bootstrap.BeanDeployerEnvironment;
 import org.jboss.weld.context.SerializableContextualInstance;
@@ -405,31 +406,40 @@ protected static Set<Annotation> flattenInterceptorBindings(BeanManagerImpl mana
       return foundInterceptionBindingTypes;
    }
 
+
+
    protected void initInterceptors()
    {
       if (manager.getBoundInterceptorsRegistry().getInterceptionModel(getType()) == null)
       {
          InterceptionModelBuilder<Class<?>, SerializableContextual<Interceptor<?>, ?>> builder = InterceptionModelBuilder.newBuilderFor(getType(), (Class) SerializableContextual.class);
-
          Set<Annotation> classBindingAnnotations = flattenInterceptorBindings(manager, getAnnotatedItem().getAnnotations());
          for (Class<? extends Annotation> annotation : getStereotypes())
          {
             classBindingAnnotations.addAll(flattenInterceptorBindings(manager, manager.getStereotypeDefinition(annotation)));
          }
-
-         Annotation[] classBindingAnnotationsArray = classBindingAnnotations.toArray(new Annotation[0]);
-         builder.interceptPostConstruct().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.POST_CONSTRUCT, classBindingAnnotationsArray)));
-         builder.interceptPreDestroy().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.PRE_DESTROY, classBindingAnnotationsArray)));
-         builder.interceptPrePassivate().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.PRE_PASSIVATE, classBindingAnnotationsArray)));
-         builder.interceptPostActivate().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.POST_ACTIVATE, classBindingAnnotationsArray)));
-
+         if (classBindingAnnotations.size() > 0)
+         {
+            if (Beans.findInterceptorBindingConflicts(manager, classBindingAnnotations))
+               throw new DeploymentException("Conflicting interceptor bindings found on " + getType());
+            Annotation[] classBindingAnnotationsArray = classBindingAnnotations.toArray(new Annotation[0]);
+            builder.interceptPostConstruct().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.POST_CONSTRUCT, classBindingAnnotationsArray)));
+            builder.interceptPreDestroy().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.PRE_DESTROY, classBindingAnnotationsArray)));
+            builder.interceptPrePassivate().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.PRE_PASSIVATE, classBindingAnnotationsArray)));
+            builder.interceptPostActivate().with(toSerializableContextualArray(manager.resolveInterceptors(InterceptionType.POST_ACTIVATE, classBindingAnnotationsArray)));
+         }
          List<WeldMethod<?, ?>> businessMethods = Beans.getInterceptableBusinessMethods(getAnnotatedItem());
          for (WeldMethod<?, ?> method : businessMethods)
          {
-            List<Annotation> methodBindingAnnotations = new ArrayList<Annotation>(classBindingAnnotations);
+            Set<Annotation> methodBindingAnnotations = new HashSet<Annotation>(classBindingAnnotations);
             methodBindingAnnotations.addAll(flattenInterceptorBindings(manager, method.getAnnotations()));
-            List<Interceptor<?>> methodBoundInterceptors = manager.resolveInterceptors(InterceptionType.AROUND_INVOKE, methodBindingAnnotations.toArray(new Annotation[]{}));
-            builder.interceptAroundInvoke(((AnnotatedMethod) method).getJavaMember()).with(toSerializableContextualArray(methodBoundInterceptors));
+            if (methodBindingAnnotations.size() > 0)
+            {
+               if (Beans.findInterceptorBindingConflicts(manager, classBindingAnnotations))
+                  throw new DeploymentException("Conflicting interceptor bindings found on " + getType() + "." + method.getName() + "()");
+               List<Interceptor<?>> methodBoundInterceptors = manager.resolveInterceptors(InterceptionType.AROUND_INVOKE, methodBindingAnnotations.toArray(new Annotation[]{}));
+               builder.interceptAroundInvoke(((AnnotatedMethod) method).getJavaMember()).with(toSerializableContextualArray(methodBoundInterceptors));
+            }
          }
          manager.getBoundInterceptorsRegistry().registerInterceptionModel(getType(), builder.build());
       }

impl/src/main/java/org/jboss/weld/bean/InterceptorImpl.java

@@ -19,6 +19,7 @@
 
 import java.lang.annotation.Annotation;
 import java.util.Set;
+import java.util.HashSet;
 
 import javax.enterprise.inject.spi.InterceptionType;
 import javax.enterprise.inject.spi.Interceptor;
@@ -28,6 +29,8 @@
 import org.jboss.interceptor.proxy.DirectClassInterceptionHandler;
 import org.jboss.interceptor.registry.InterceptorClassMetadataRegistry;
 import org.jboss.weld.BeanManagerImpl;
+import org.jboss.weld.DeploymentException;
+import org.jboss.weld.util.Beans;
 import org.jboss.weld.introspector.WeldClass;
 
 /**
@@ -43,7 +46,20 @@ protected InterceptorImpl(WeldClass<T> type, BeanManagerImpl manager)
    {
       super(type, new StringBuilder().append(Interceptor.class.getSimpleName()).append(BEAN_ID_SEPARATOR).append(type.getName()).toString(), manager);
       this.interceptorClassMetadata = InterceptorClassMetadataRegistry.getRegistry().getInterceptorClassMetadata(type.getJavaClass());
-      this.interceptorBindingTypes = flattenInterceptorBindings(manager, getAnnotatedItem().getAnnotations());
+      this.interceptorBindingTypes = new HashSet<Annotation>();
+      interceptorBindingTypes.addAll(flattenInterceptorBindings(manager, getAnnotatedItem().getAnnotations()));
+      for (Class<? extends Annotation> annotation : getStereotypes())
+      {
+         interceptorBindingTypes.addAll(flattenInterceptorBindings(manager, manager.getStereotypeDefinition(annotation)));
+      }
+      if (this.interceptorBindingTypes.size() == 0)
+      {
+         throw new DeploymentException("An interceptor must have at least one binding, but " + type.getName() + " has none");
+      }
+      if (Beans.findInterceptorBindingConflicts(manager, interceptorBindingTypes))
+      {
+         throw new DeploymentException("Conflicting interceptor bindings found on " + getType());
+      }
    }
 
    public static <T> InterceptorImpl<T> of(WeldClass<T> type, BeanManagerImpl manager)

impl/src/main/java/org/jboss/weld/bean/interceptor/ClassInterceptionHandlerFactory.java

@@ -17,13 +17,16 @@
 
 package org.jboss.weld.bean.interceptor;
 
+import java.lang.reflect.Constructor;
+
 import javax.enterprise.context.spi.CreationalContext;
 
 import org.jboss.interceptor.proxy.InterceptionHandlerFactory;
 import org.jboss.interceptor.proxy.InterceptionHandler;
 import org.jboss.interceptor.proxy.DirectClassInterceptionHandler;
 import org.jboss.weld.BeanManagerImpl;
 import org.jboss.weld.DeploymentException;
+import org.jboss.weld.util.Reflections;
 
 /**
  * @author Marius Bogoevici
@@ -44,7 +47,9 @@ public InterceptionHandler createFor(Class clazz)
       try
       {
          // this is not a managed instance - assume no-argument constructor exists
-         Object interceptorInstance = clazz.newInstance();
+         Constructor constructor = clazz.getDeclaredConstructor();
+         Reflections.ensureAccessible(constructor);
+         Object interceptorInstance = constructor.newInstance();
          // inject
          manager.createInjectionTarget(manager.createAnnotatedType(clazz)).inject(interceptorInstance, creationalContext);
          return new DirectClassInterceptionHandler(interceptorInstance, clazz);

impl/src/main/java/org/jboss/weld/bootstrap/AbstractBeanDeployer.java

@@ -23,6 +23,7 @@
 import javax.enterprise.inject.Disposes;
 import javax.enterprise.inject.Produces;
 import javax.inject.Inject;
+import javax.interceptor.Interceptor;
 
 import org.jboss.weld.BeanManagerImpl;
 import org.jboss.weld.bean.AbstractClassBean;

impl/src/main/java/org/jboss/weld/bootstrap/BeanDeployer.java

@@ -25,6 +25,7 @@
 
 import org.jboss.weld.BeanManagerImpl;
 import org.jboss.weld.Container;
+import org.jboss.weld.DeploymentException;
 import org.jboss.weld.bootstrap.events.ProcessAnnotatedTypeImpl;
 import org.jboss.weld.ejb.EjbDescriptors;
 import org.jboss.weld.ejb.InternalEjbDescriptor;
@@ -98,10 +99,12 @@ public BeanDeployer createBeans()
          boolean managedBeanOrDecorator = !getEnvironment().getEjbDescriptors().contains(clazz.getJavaClass()) && isTypeManagedBeanOrDecorator(clazz);
          if (managedBeanOrDecorator && clazz.isAnnotationPresent(Decorator.class))
          {
+            validateDecorator(clazz);
             createDecorator(clazz);
          }
          else if (managedBeanOrDecorator && clazz.isAnnotationPresent(Interceptor.class))
          {
+            validateInterceptor(clazz);
             createInterceptor(clazz);
          }
          else if (managedBeanOrDecorator && !clazz.isAbstract())
@@ -116,4 +119,20 @@ else if (managedBeanOrDecorator && !clazz.isAbstract())
       return this;
    }
 
+   private void validateInterceptor(WeldClass<?> clazz)
+   {
+      if (clazz.isAnnotationPresent(Decorator.class))
+      {
+         throw new DeploymentException("Class " + clazz.getName() + " has both @Interceptor and @Decorator annotations");
+      }
+   }
+
+   private void validateDecorator(WeldClass<?> clazz)
+   {
+      if (clazz.isAnnotationPresent(Interceptor.class))
+      {
+         throw new DeploymentException("Class " + clazz.getName() + " has both @Interceptor and @Decorator annotations");
+      }
+   }
+
 }

impl/src/main/java/org/jboss/weld/metadata/cache/InterceptorBindingModel.java

@@ -74,12 +74,17 @@ public Set<Annotation> getInheritedInterceptionBindingTypes()
    }
 
    public boolean isEqual(Annotation instance, Annotation other)
+   {
+       return isEqual(instance, other, false);
+   }
+
+   public boolean isEqual(Annotation instance, Annotation other, boolean includeNonBindingTypes)
    {
       if (instance.annotationType().equals(getRawType()) && other.annotationType().equals(getRawType()))
       {
          for (WeldMethod<?, ?> annotatedMethod : getAnnotatedAnnotation().getMembers())
          {
-            if (!nonBindingTypes.contains(annotatedMethod))
+            if (includeNonBindingTypes || !nonBindingTypes.contains(annotatedMethod))
             {
                try
                {

impl/src/main/java/org/jboss/weld/util/Beans.java

@@ -232,8 +232,6 @@ public static List<WeldMethod<?,?>> getInterceptableBusinessMethods(WeldClass<?>
       {
          int modifiers = ((WeldMember) annotatedMethod).getJavaMember().getModifiers();
          boolean businessMethod = !annotatedMethod.isStatic()
-               && (Modifier.isPublic(modifiers)
-                  || Modifier.isProtected(modifiers))
                && !annotatedMethod.isAnnotationPresent(Inject.class);
 
          if (businessMethod)
@@ -479,6 +477,27 @@ public static boolean containsAllInterceptionBindings(Set<Annotation> expectedBi
       }
       return true;
    }
+
+   public static boolean findInterceptorBindingConflicts(BeanManagerImpl manager, Set<Annotation> bindings)
+   {
+      Map<Class<? extends Annotation>, Annotation> foundAnnotations = new HashMap<Class<? extends Annotation>, Annotation>();
+      for (Annotation binding: bindings)
+      {
+         if (foundAnnotations.containsKey(binding.annotationType()))
+         {
+            InterceptorBindingModel<?> bindingType = manager.getServices().get(MetaAnnotationStore.class).getInterceptorBindingModel(binding.annotationType());
+            if (!bindingType.isEqual(binding, foundAnnotations.get(binding.annotationType()), false))
+            {
+                return true;
+            }
+         }
+         else
+         {
+            foundAnnotations.put(binding.annotationType(), binding);
+         }
+      }
+      return false;      
+   }
 
 
    /**
@@ -719,5 +738,5 @@ public static <T> boolean isDecorator(WeldClass<T> annotatedItem)
    {
       return annotatedItem.isAnnotationPresent(Decorator.class);
    }
-   
+
 }

impl/src/main/java/org/jboss/weld/xml/BeansXmlParser.java

@@ -28,6 +28,7 @@
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
+import javax.interceptor.Interceptor;
 
 import org.jboss.weld.DeploymentException;
 import org.jboss.weld.resources.spi.ResourceLoader;
@@ -203,7 +204,7 @@ else if (decoratorsElements.size() == 1)
       else if (interceptorsElements.size() == 1)
       {
          enabledInterceptorClasses = new ArrayList<Class<?>>();
-         enabledInterceptorClasses.addAll(processElement(resourceLoader, interceptorsElements.get(0)));
+         enabledInterceptorClasses.addAll(processInterceptorElement(resourceLoader, interceptorsElements.get(0)));
       }
 
    }
@@ -268,5 +269,39 @@ private static List<Class<?>> processElement(ResourceLoader resourceLoader, XmlE
       }
       return list;
    }
+
+   private static List<Class<?>> processInterceptorElement(ResourceLoader resourceLoader, XmlElement element)
+   {
+      List<Class<?>> list = new ArrayList<Class<?>>();
+      for (Node child : new NodeListIterable(element.getElement().getChildNodes()))
+      {
+         String className = processNode(child);
+         if (className != null)
+         {
+            try
+            {
+               Class<?> clazz = resourceLoader.classForName(className);
+               if (!clazz.isAnnotationPresent(Interceptor.class))
+               {
+                   throw new DeploymentException("Class " + clazz.getName() + " is enabled as an interceptor," +
+                        " but it does not have the appropriate annotation");
+               }
+               else if (list.contains(clazz))
+               {
+                   throw new DeploymentException("Class " + clazz.getName() + " is listed twice as an enabled interceptor");
+               }
+               else
+               {
+                  list.add(clazz);
+               }
+            }
+            catch (ResourceLoadingException e)
+            {
+               throw new DeploymentException("Cannot load class " + className + " defined in " + element.getFile().toString());
+            }
+         }
+      }
+      return list;
+   }
 
 }