WELD-741, fix the fix :-)

* Split out tests
* Consider all methods, detecting if the are
  overridden

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

@@ -415,6 +415,7 @@ public boolean isClientCanCallRemoveMethods()
     */
    protected void checkObserverMethods()
    {
+      // TODO Need to check super classes too
       for (WeldMethod<?, ?> method : this.annotatedItem.getDeclaredWeldMethodsWithAnnotatedParameters(Observes.class))
       {
          if (!method.isStatic())

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

@@ -26,7 +26,6 @@
 import java.lang.reflect.Member;
 import java.util.Set;
 
-import javax.enterprise.event.Observes;
 import javax.enterprise.inject.Disposes;
 import javax.enterprise.inject.Produces;
 import javax.enterprise.inject.spi.Extension;
@@ -63,6 +62,7 @@
 import org.jboss.weld.introspector.WeldMethod;
 import org.jboss.weld.manager.BeanManagerImpl;
 import org.jboss.weld.persistence.PersistenceApiAbstraction;
+import org.jboss.weld.util.Beans;
 import org.jboss.weld.util.reflection.Reflections;
 import org.jboss.weld.ws.WSApiAbstraction;
 import org.slf4j.cal10n.LocLogger;
@@ -217,12 +217,12 @@ protected <X> void createProducerFields(AbstractClassBean<X> declaringBean, Weld
       }
    }
 
-   protected <X> void createObserverMethods(RIBean<X> declaringBean, WeldClass<X> annotatedClass)
+   protected <X> void createObserverMethods(RIBean<X> declaringBean, WeldClass<? super X> annotatedClass)
    {
-      for (WeldMethod<?, ? super X> method : annotatedClass.getWeldMethodsWithAnnotatedParameters(Observes.class))
-      {
+	   for (WeldMethod<?, ? super X> method : Beans.getObserverMethods(annotatedClass))
+	   {
          createObserverMethod(declaringBean, method);
-      }
+	   }
    }
 
    protected <T, X> void createObserverMethod(RIBean<X> declaringBean, WeldMethod<T, ? super X> method)

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

@@ -22,7 +22,6 @@
 import java.util.Set;
 
 import javax.enterprise.context.spi.Context;
-import javax.enterprise.event.Observes;
 import javax.enterprise.inject.spi.Extension;
 
 import org.jboss.weld.Container;
@@ -37,6 +36,7 @@
 import org.jboss.weld.introspector.WeldMethod;
 import org.jboss.weld.manager.BeanManagerImpl;
 import org.jboss.weld.resources.ClassTransformer;
+import org.jboss.weld.util.Beans;
 import org.jboss.weld.util.DeploymentStructures;
 
 /**
@@ -99,9 +99,9 @@ public void addExtension(Metadata<Extension> extension)
       this.extensions.add(extension);
    }
 
-   protected <X> void createObserverMethods(RIBean<X> declaringBean, BeanManagerImpl beanManager, WeldClass<X> annotatedClass, Set<ObserverMethodImpl<?, ?>> observerMethods)
+   protected <X> void createObserverMethods(RIBean<X> declaringBean, BeanManagerImpl beanManager, WeldClass<? super X> annotatedClass, Set<ObserverMethodImpl<?, ?>> observerMethods)
    {
-      for (WeldMethod<?, ? super X> method : annotatedClass.getDeclaredWeldMethodsWithAnnotatedParameters(Observes.class))
+      for (WeldMethod<?, ? super X> method : Beans.getObserverMethods(annotatedClass))
       {
          createObserverMethod(declaringBean, beanManager, method, observerMethods);
       }

impl/src/main/java/org/jboss/weld/introspector/ForwardingWeldClass.java

@@ -19,7 +19,6 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.Collection;
-import java.util.Set;
 
 public abstract class ForwardingWeldClass<T> extends ForwardingWeldAnnotated<T, Class<T>> implements WeldClass<T>
 {
@@ -47,7 +46,7 @@ public WeldConstructor<T> getNoArgsWeldConstructor()
       return delegate().getNoArgsWeldConstructor();
    }
 
-   public Collection<WeldMethod<?, ?>> getWeldMethods()
+   public Collection<WeldMethod<?, ? super T>> getWeldMethods()
    {
       return delegate().getWeldMethods();
    }
@@ -83,7 +82,7 @@ public WeldConstructor<T> getNoArgsWeldConstructor()
       return delegate().getWeldMethod(signature);
    }
 
-   public WeldClass<?> getWeldSuperclass()
+   public WeldClass<? super T> getWeldSuperclass()
    {
       return delegate().getWeldSuperclass();
    }

impl/src/main/java/org/jboss/weld/introspector/WeldClass.java

@@ -19,7 +19,6 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.Collection;
-import java.util.Set;
 
 import javax.enterprise.inject.spi.AnnotatedType;
 
@@ -44,14 +43,14 @@ public interface WeldClass<T> extends WeldAnnotated<T, Class<T>>, AnnotatedType<
     * 
     * @return A set of abstracted fields
     */
-   public Collection<WeldMethod<?, ?>> getWeldMethods();
+   public Collection<WeldMethod<?, ? super T>> getWeldMethods();
 
    /**
     * Gets all fields on the type
     * 
     * @return A set of abstracted fields
     */
-   public Collection<WeldMethod<?, ?>> getDeclaredWeldMethods();
+   public Collection<WeldMethod<?, ? super T>> getDeclaredWeldMethods();
 
    /**
     * Get a field by name
@@ -165,22 +164,13 @@ public interface WeldClass<T> extends WeldAnnotated<T, Class<T>>, AnnotatedType<
     *         empty set if there are no matches
     */
    public Collection<WeldMethod<?, ? super T>> getDeclaredWeldMethodsWithAnnotatedParameters(Class<? extends Annotation> annotationType);
-
-   /**
-    * Gets all with parameters annotated with annotationType
-    * 
-    * @param annotationType The annotation to match
-    * @return A set of abstracted methods with the given annotation. Returns an
-    *         empty set if there are no matches
-    */
-   public Collection<WeldMethod<?, ? super T>> getWeldMethodsWithAnnotatedParameters(Class<? extends Annotation> annotationType);
 
    /**
     * Gets the superclass.
     * 
     * @return The abstracted superclass, null if there is no superclass
     */
-   public WeldClass<?> getWeldSuperclass();
+   public WeldClass<? super T> getWeldSuperclass();
 
    public boolean isParameterizedType();
 

impl/src/main/java/org/jboss/weld/introspector/jlr/WeldClassImpl.java

@@ -83,7 +83,7 @@ private static <T> void mapConstructorAnnotations(ArrayListMultimap<Class<? exte
    }
 
    // Class attributes
-   private final WeldClass<?> superclass;
+   private final WeldClass<? super T> superclass;
 
    // The set of abstracted fields
    private final Set<WeldField<?, ?>> fields;
@@ -99,14 +99,14 @@ private static <T> void mapConstructorAnnotations(ArrayListMultimap<Class<? exte
    private final ArrayListMultimap<Class<? extends Annotation>, WeldField<?, ?>> declaredMetaAnnotatedFields;
 
    // The set of abstracted methods
-   private final Set<WeldMethod<?, ?>> methods;
+   private final Set<WeldMethod<?, ? super T>> methods;
    private final Map<MethodSignature, WeldMethod<?, ?>> declaredMethodsBySignature;
    private final Map<MethodSignature, WeldMethod<?, ?>> methodsBySignature;
    // The map from annotation type to abstracted method with annotation
    private final ArrayListMultimap<Class<? extends Annotation>, WeldMethod<?, ?>> annotatedMethods;
 
    // The set of abstracted methods
-   private final ArraySet<WeldMethod<?, ?>> declaredMethods;
+   private final ArraySet<WeldMethod<?, ? super T>> declaredMethods;
    // The map from annotation type to abstracted method with annotation
    private final ArrayListMultimap<Class<? extends Annotation>, WeldMethod<?, ? super T>> declaredAnnotatedMethods;
    // The map from annotation type to method with a parameter with annotation
@@ -277,8 +277,8 @@ protected WeldClassImpl(Class<T> rawType, Type type, AnnotatedType<T> annotatedT
       this.declaredMethodsByAnnotatedParameters = ArrayListMultimap.<Class<? extends Annotation>, WeldMethod<?, ? super T>> create();
       this.declaredMethodsBySignature = new HashMap<MethodSignature, WeldMethod<?, ?>>();
 
-      Set<WeldMethod<?, ?>> methodsTemp = null;
-      ArrayList<WeldMethod<?, ?>> declaredMethodsTemp = new ArrayList<WeldMethod<?, ?>>();
+      Set<WeldMethod<?, ? super T>> methodsTemp = null;
+      ArrayList<WeldMethod<?, ? super T>> declaredMethodsTemp = new ArrayList<WeldMethod<?, ? super T>>();
       if (annotatedType == null)
       {
          this.annotatedMethods = null;
@@ -302,19 +302,20 @@ protected WeldClassImpl(Class<T> rawType, Type type, AnnotatedType<T> annotatedT
                   }
                }
             }
-            methodsTemp = new ArraySet<WeldMethod<?, ?>>(declaredMethodsTemp).trimToSize();
+            methodsTemp = new ArraySet<WeldMethod<?, ? super T>>(declaredMethodsTemp).trimToSize();
             if ((superclass != null) && (superclass.getJavaClass() != Object.class))
             {
-               methodsTemp = Sets.union(methodsTemp, (Set<WeldMethod<?, ?>>) superclass.getDeclaredWeldMethods());
+               Set<WeldMethod<?, ? super T>> superClassMethods = Reflections.cast(superclass.getDeclaredWeldMethods());
+               methodsTemp = Sets.union(methodsTemp, superClassMethods);
             }
          }
-         this.declaredMethods = new ArraySet<WeldMethod<?, ?>>(declaredMethodsTemp);
+         this.declaredMethods = new ArraySet<WeldMethod<?, ? super T>>(declaredMethodsTemp);
       }
       else
       {
          this.annotatedMethods = ArrayListMultimap.<Class<? extends Annotation>, WeldMethod<?, ?>> create();
          this.methodsBySignature = new HashMap<MethodSignature, WeldMethod<?, ?>>();
-         methodsTemp = new HashSet<WeldMethod<?, ?>>();
+         methodsTemp = new HashSet<WeldMethod<?, ? super T>>();
          for (AnnotatedMethod<? super T> method : annotatedType.getMethods())
          {
             WeldMethod<?, ? super T> weldMethod = WeldMethodImpl.of(method, this, classTransformer);
@@ -344,8 +345,8 @@ protected WeldClassImpl(Class<T> rawType, Type type, AnnotatedType<T> annotatedT
                }
             }
          }
-         this.declaredMethods = new ArraySet<WeldMethod<?, ?>>(declaredMethodsTemp);
-         methodsTemp = new ArraySet<WeldMethod<?, ?>>(methodsTemp).trimToSize();
+         this.declaredMethods = new ArraySet<WeldMethod<?, ? super T>>(declaredMethodsTemp);
+         methodsTemp = new ArraySet<WeldMethod<?, ? super T>>(methodsTemp).trimToSize();
          this.annotatedMethods.trimToSize();
       }
       this.methods = methodsTemp;
@@ -547,20 +548,6 @@ public WeldConstructor<T> getNoArgsWeldConstructor()
       return Collections.unmodifiableCollection(declaredMethodsByAnnotatedParameters.get(annotationType));
    }
 
-   public Collection<WeldMethod<?, ? super T>> getWeldMethodsWithAnnotatedParameters(Class<? extends Annotation> annotationType)
-   {
-      // TODO should be cached
-      ArrayList<WeldMethod<?, ? super T>> methods = new ArrayList<WeldMethod<?, ? super T>>();
-      for (WeldMethod<?, ?> method : getWeldMethods())
-      {
-         if (!method.getWeldParameters(annotationType).isEmpty())
-         {
-            methods.add((WeldMethod<?, ? super T>) method);
-         }
-      }
-      return Collections.unmodifiableCollection(methods);
-   }
-
    public WeldMethod<?, ?> getWeldMethod(Method methodDescriptor)
    {
       // TODO Should be cached
@@ -574,7 +561,7 @@ public WeldConstructor<T> getNoArgsWeldConstructor()
       return null;
    }
 
-   public Collection<WeldMethod<?, ?>> getWeldMethods()
+   public Collection<WeldMethod<?, ? super T>> getWeldMethods()
    {
       return Collections.unmodifiableSet(methods);
    }
@@ -592,7 +579,7 @@ public WeldConstructor<T> getNoArgsWeldConstructor()
       return null;
    }
 
-   public Collection<WeldMethod<?, ?>> getDeclaredWeldMethods()
+   public Collection<WeldMethod<?, ? super T>> getDeclaredWeldMethods()
    {
       return Collections.unmodifiableSet(declaredMethods);
    }
@@ -688,7 +675,7 @@ public String getName()
     * 
     * @return The superclass abstraction
     */
-   public WeldClass<?> getWeldSuperclass()
+   public WeldClass<? super T> getWeldSuperclass()
    {
       return superclass;
    }

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

@@ -259,6 +259,35 @@ else if (declaredMethods.size() == 1)
       }
       return methods;
    }
+
+   public static <T> List<WeldMethod<?, ? super T>> getObserverMethods(WeldClass<T> type)
+   {
+      List<WeldMethod<?, ? super T>> observerMethods = new ArrayList<WeldMethod<?,? super T>>();
+      // Keep track of all seen methods so we can ignore overridden methods
+      Multimap<MethodSignature, Package> seenMethods = Multimaps.newSetMultimap(new HashMap<MethodSignature, Collection<Package>>(), new Supplier<Set<Package>>()
+      {
+
+         public Set<Package> get()
+         {
+            return new HashSet<Package>();
+         }
+
+      });
+      WeldClass<? super T> t = type;
+      while (t != null && !t.getJavaClass().equals(Object.class))
+      {
+         for (WeldMethod<?, ? super T> method : t.getDeclaredWeldMethods())
+         {
+            if (!isOverridden(method, seenMethods) && !method.getWeldParameters(Observes.class).isEmpty())
+            {
+               observerMethods.add(method);
+            }
+            seenMethods.put(method.getSignature(), method.getPackage());
+         }
+         t = t.getWeldSuperclass();
+      }
+      return observerMethods;
+   }
 
    public static <T> List<WeldMethod<?, ? super T>> getPreDestroyMethods(WeldClass<T> type)
    {

tests-arquillian/src/test/java/org/jboss/weld/tests/event/observer/superclass/SuperclassObservers1Test.java

@@ -0,0 +1,60 @@
+/*
+ * JBoss, Home of Professional Open Source
+ * Copyright 2010, Red Hat, Inc., and individual contributors
+ * by the @authors tag. See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * Licensed 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.jboss.weld.tests.event.observer.superclass;
+
+import static org.junit.Assert.assertNotNull;
+
+import javax.enterprise.event.Event;
+import javax.inject.Inject;
+
+import junit.framework.Assert;
+
+import org.jboss.arquillian.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.shrinkwrap.api.Archive;
+import org.jboss.shrinkwrap.api.BeanArchive;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(Arquillian.class)
+public class SuperclassObservers1Test
+{
+   @Deployment
+   public static Archive<?> deploy()
+   {
+      return ShrinkWrap.create(BeanArchive.class).addPackage(SuperclassObservers1Test.class.getPackage());
+   }
+
+   @Inject
+   Event<TestEvent> event;
+
+   @Inject
+   @ReEnabled
+   private ReEnabledTestObserver reenabled;
+
+   @Test
+   public void testObserverMethodOnOverridesWithAnnotAreInvoked()
+   {
+      reenabled.reset();
+
+      Assert.assertNull(reenabled.getTestEvent());
+      event.fire(new TestEvent());
+      assertNotNull(reenabled.getTestEvent());
+   }
+
+}