You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2006/04/27 04:00:24 UTC

svn commit: r397359 - in /tapestry/tapestry5/tapestry-core/trunk/src: main/java/org/apache/tapestry/internal/transform/ main/java/org/apache/tapestry/transform/ test/java/org/apache/tapestry/internal/transform/ test/java/org/apache/tapestry/internal/tr...

Author: hlship
Date: Wed Apr 26 19:00:23 2006
New Revision: 397359

URL: http://svn.apache.org/viewcvs?rev=397359&view=rev
Log:
Handle components that extend from other components, including properly extending methods declared in the super-class.

Added:
    tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/pages/BasicSubComponent.java
Modified:
    tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/ClassTransformerImpl.java
    tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/InternalClassTransformationImpl.java
    tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/transform/ClassTransformation.java
    tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/ComponentInstantiatorSourceImplTest.java
    tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/InternalClassTransformationImplTest.java

Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/ClassTransformerImpl.java
URL: http://svn.apache.org/viewcvs/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/ClassTransformerImpl.java?rev=397359&r1=397358&r2=397359&view=diff
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/ClassTransformerImpl.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/ClassTransformerImpl.java Wed Apr 26 19:00:23 2006
@@ -17,6 +17,7 @@
 import java.util.Map;
 
 import javassist.CtClass;
+import javassist.NotFoundException;
 
 import org.apache.tapestry.annotations.ComponentClass;
 import org.apache.tapestry.internal.annotations.Synchronized;
@@ -44,9 +45,28 @@
     {
         String classname = ctClass.getName();
 
+        String parentClassname;
+
+        try
+        {
+            parentClassname = ctClass.getSuperclass().getName();
+        }
+        catch (NotFoundException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+
+        // If the parent class is in a controlled package, it will already have been loaded and
+        // transformed (that is driven by the ComponentInstantiatorSource).
+
+        InternalClassTransformation parentTransformation = _nameToClassTransformation
+                .get(parentClassname);
+
         // TODO: Check that the name is not already in the map.
 
-        InternalClassTransformation transformation = new InternalClassTransformationImpl(ctClass);
+        InternalClassTransformation transformation = parentTransformation == null ? new InternalClassTransformationImpl(
+                ctClass)
+                : new InternalClassTransformationImpl(ctClass, parentTransformation);
 
         // Not all classes in the packages are components.
 

Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/InternalClassTransformationImpl.java
URL: http://svn.apache.org/viewcvs/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/InternalClassTransformationImpl.java?rev=397359&r1=397358&r2=397359&view=diff
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/InternalClassTransformationImpl.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/transform/InternalClassTransformationImpl.java Wed Apr 26 19:00:23 2006
@@ -68,7 +68,7 @@
     // private final Map<CtField, FieldEncapsulation> _encapsulations = newMap();
 
     /** Map from a field to the annotation objects for that field. */
-    private Map<String, Object[]> _fieldAnnotations = newMap();
+    private Map<String, List<Annotation>> _fieldAnnotations = newMap();
 
     /** Used to identify fields that have been "claimed" by other annotations. */
     private Map<String, Object> _claimedFields = newMap();
@@ -77,7 +77,7 @@
 
     // Cache of class annotations
 
-    private Object[] _classAnnotations;
+    private List<Annotation> _classAnnotations;
 
     /** Contains the assembled Javassist code for the class' default constructor. */
     private StringBuffer _constructor = new StringBuffer();
@@ -175,7 +175,7 @@
 
     public <T extends Annotation> T getFieldAnnotation(String fieldName, Class<T> annotationClass)
     {
-        Object[] annotations = findFieldAnnotations(fieldName);
+        List<Annotation> annotations = findFieldAnnotations(fieldName);
 
         return findAnnotationInList(annotationClass, annotations);
     }
@@ -191,7 +191,7 @@
      *            the available annotations
      * @return the matching annotation instance, or null if not found
      */
-    private <T> T findAnnotationInList(Class<T> annotationClass, Object[] annotations)
+    private <T> T findAnnotationInList(Class<T> annotationClass, List<Annotation> annotations)
     {
         for (Object annotation : annotations)
         {
@@ -204,52 +204,38 @@
 
     public <T extends Annotation> T getAnnotation(Class<T> annotationClass)
     {
-        Object[] annotations = findClassAnnotations();
-
-        return findAnnotationInList(annotationClass, annotations);
-    }
-
-    private Object[] findClassAnnotations()
-    {
-        if (_classAnnotations == null)
-        {
-            try
-            {
-                _classAnnotations = _ctClass.getAnnotations();
-            }
-            catch (ClassNotFoundException ex)
-            {
-                throw new RuntimeException(ex);
-            }
-        }
-
-        return _classAnnotations;
+        return findAnnotationInList(annotationClass, getClassAnnotations());
     }
 
-    private Object[] findFieldAnnotations(String fieldName)
+    private List<Annotation> findFieldAnnotations(String fieldName)
     {
-        Object[] annotations = _fieldAnnotations.get(fieldName);
+        List<Annotation> annotations = _fieldAnnotations.get(fieldName);
 
         if (annotations == null)
         {
             annotations = findAnnotationsForField(fieldName);
             _fieldAnnotations.put(fieldName, annotations);
         }
+
         return annotations;
     }
 
-    private Object[] findAnnotationsForField(String fieldName)
+    private List<Annotation> findAnnotationsForField(String fieldName)
     {
         CtField field = findDeclaredCtField(fieldName);
 
         return extractAnnotations(field);
     }
 
-    private Object[] extractAnnotations(CtField field)
+    private List<Annotation> extractAnnotations(CtField field)
     {
         try
         {
-            return field.getAnnotations();
+            List<Annotation> result = newList();
+
+            addAnnotationsToList(result, field.getAnnotations());
+
+            return result;
         }
         catch (ClassNotFoundException ex)
         {
@@ -257,6 +243,15 @@
         }
     }
 
+    private void addAnnotationsToList(List<Annotation> list, Object[] annotations)
+    {
+        for (Object o : annotations)
+        {
+            Annotation a = (Annotation) o;
+            list.add(a);
+        }
+    }
+
     private CtField findDeclaredCtField(String fieldName)
     {
         try
@@ -516,11 +511,51 @@
                 return method;
         }
 
+        CtMethod result = addOverrideOfSuperclassMethod(methodSignature);
+
+        if (result != null)
+            return result;
+
         throw new IllegalArgumentException(TransformMessages.noDeclaredMethod(
                 methodSignature,
                 _ctClass));
     }
 
+    private CtMethod addOverrideOfSuperclassMethod(MethodSignature methodSignature)
+    {
+        try
+        {
+            for (CtClass current = _ctClass; current != null; current = current.getSuperclass())
+            {
+                for (CtMethod method : current.getDeclaredMethods())
+                {
+                    if (match(method, methodSignature))
+                    {
+                        // TODO: If the moethod is not overridable (i.e. private, or final)?
+                        // Perhaps we should limit it to just public methods.
+
+                        CtMethod newMethod = CtNewMethod.delegator(method, _ctClass);
+                        _ctClass.addMethod(newMethod);
+
+                        return newMethod;
+                    }
+                }
+            }
+        }
+        catch (NotFoundException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+        catch (CannotCompileException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+
+        // Not found in a super-class.
+
+        return null;
+    }
+
     private boolean match(CtMethod method, MethodSignature sig)
     {
         if (!sig.getMethodName().equals(method.getName()))
@@ -566,7 +601,7 @@
         {
             String fieldName = field.getName();
 
-            Object[] annotations = findFieldAnnotations(fieldName);
+            List<Annotation> annotations = findFieldAnnotations(fieldName);
 
             // Check to see if any of the annotations are the right type, if
             // so, add the field name to the result.
@@ -735,7 +770,6 @@
         _claimedFields = null;
         _addedFieldNames = null;
         _fieldAnnotations = null;
-        _classAnnotations = null;
         _constructor = null;
     }
 
@@ -789,6 +823,35 @@
         failIfNotFrozen();
 
         return CollectionFactory.newList(_constructorArgs);
+    }
+
+    public List<Annotation> getClassAnnotations()
+    {
+        if (_classAnnotations == null)
+            assembleClassAnnotations();
+
+        return _classAnnotations;
+    }
+
+    private void assembleClassAnnotations()
+    {
+        _classAnnotations = newList();
+
+        try
+        {
+            for (CtClass current = _ctClass; current != null; current = current.getSuperclass())
+            {
+                addAnnotationsToList(_classAnnotations, current.getAnnotations());
+            }
+        }
+        catch (NotFoundException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+        catch (ClassNotFoundException ex)
+        {
+            throw new RuntimeException(ex);
+        }
     }
 
 }

Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/transform/ClassTransformation.java
URL: http://svn.apache.org/viewcvs/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/transform/ClassTransformation.java?rev=397359&r1=397358&r2=397359&view=diff
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/transform/ClassTransformation.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/transform/ClassTransformation.java Wed Apr 26 19:00:23 2006
@@ -60,8 +60,8 @@
     List<String> findFieldsWithAnnotation(Class<? extends Annotation> annotationClass);
 
     /**
-     * Finds an annotation for the class itself. TODO: Deal with the fact that Javassist does not
-     * handled Inherited annotations properly.
+     * Finds an annotation for the class itself, including any annotations inherited from parent
+     * classes.
      * 
      * @param <T>
      * @param annotationClass
@@ -167,6 +167,9 @@
      * Extends an existing method. The provided method body is inserted at the end of the existing
      * method (i.e. {@link javassist.CtBehavior#insertAfter(java.lang.String)}). To access or
      * change the return value, use the <code>$_</code> pseudo variable.
+     * <p>
+     * The method may be declared in the class, or may be inherited from a super-class. For
+     * inherited methods, a method is added that first invokes the super implementation.
      * 
      * @param signature
      *            the signature of the method to extend

Modified: tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/ComponentInstantiatorSourceImplTest.java
URL: http://svn.apache.org/viewcvs/tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/ComponentInstantiatorSourceImplTest.java?rev=397359&r1=397358&r2=397359&view=diff
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/ComponentInstantiatorSourceImplTest.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/ComponentInstantiatorSourceImplTest.java Wed Apr 26 19:00:23 2006
@@ -20,13 +20,16 @@
 import org.apache.tapestry.events.ComponentLifecycle;
 import org.apache.tapestry.internal.InternalComponentResources;
 import org.apache.tapestry.internal.transform.pages.BasicComponent;
+import org.apache.tapestry.internal.transform.pages.BasicSubComponent;
 import org.apache.tapestry.test.BaseTestCase;
 import org.testng.Assert;
+import org.testng.annotations.Configuration;
 import org.testng.annotations.Test;
 
 import static java.lang.Thread.currentThread;
 import static org.apache.hivemind.util.PropertyUtils.read;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNull;
 
 /**
  * Tests for {@link org.apache.tapestry.internal.transform.ComponentInstantiatorSourceImpl}.
@@ -37,6 +40,10 @@
 {
     private final ClassLoader _defaultClassLoader = currentThread().getContextClassLoader();
 
+    private ComponentInstantiatorSource _source;
+
+    private Registry _registry;
+
     @Test
     public void controlledPackagesTest() throws Exception
     {
@@ -68,25 +75,7 @@
     @Test
     public void loadComponentViaService() throws Exception
     {
-        InternalComponentResources resources = newInternalComponentResources();
-
-        replay();
-
-        Registry registry = RegistryBuilder.constructDefaultRegistry();
-
-        // Can't wait for the HiveMind code base to start using some generics for this kind of
-        // thing.
-
-        ComponentInstantiatorSource source = (ComponentInstantiatorSource) registry
-                .getService(ComponentInstantiatorSource.class);
-
-        source.addPackage("org.apache.tapestry.internal.transform.pages");
-
-        Instantiator inst = source.createInstantiator(BasicComponent.class.getName());
-
-        Object target = inst.newInstance(resources);
-
-        verify();
+        ComponentLifecycle target = createComponent(BasicComponent.class);
 
         // Should not be an instance, since it is loaded by a different class loader.
         Assert.assertFalse(BasicComponent.class.isInstance(target));
@@ -97,20 +86,73 @@
         PropertyUtils.write(target, "retainedValue", "some retained value");
         assertEquals(read(target, "retainedValue"), "some retained value");
 
-        ComponentLifecycle lifecycle = (ComponentLifecycle) target;
-        
         // Setting a property value before pageDidLoad will cause that value
         // to be the default when the page detaches.
-        
-        lifecycle.containingPageDidLoad();
+
+        target.containingPageDidLoad();
 
         PropertyUtils.write(target, "value", "some transient value");
         assertEquals(read(target, "value"), "some transient value");
 
-        lifecycle.containingPageDidDetach();
+        target.containingPageDidDetach();
 
         assertEquals(read(target, "value"), "some default value");
         assertEquals(read(target, "retainedValue"), "some retained value");
+    }
+
+    @Test
+    public void testSubComponentViaService() throws Exception
+    {
+        ComponentLifecycle target = createComponent(BasicSubComponent.class);
+
+        target.containingPageDidLoad();
+
+        PropertyUtils.write(target, "value", "base class");
+        assertEquals(read(target, "value"), "base class");
+
+        PropertyUtils.write(target, "intValue", 33);
+        assertEquals(read(target, "intValue"), 33);
+
+        target.containingPageDidDetach();
+
+        assertNull(read(target, "value"));
+        assertEquals(read(target, "intValue"), 0);
+    }
+
+    private ComponentLifecycle createComponent(Class componentClass)
+    {
+        InternalComponentResources resources = newInternalComponentResources();
+
+        replay();
+
+        // Can't wait for the HiveMind code base to start using some generics for this kind of
+        // thing.
+
+        Instantiator inst = _source.createInstantiator(componentClass.getName());
+
+        ComponentLifecycle target = inst.newInstance(resources);
+
+        verify();
+
+        return target;
+    }
+
+    @Configuration(beforeTestClass = true)
+    public void createRegistry()
+    {
+        _registry = RegistryBuilder.constructDefaultRegistry();
+        _source = (ComponentInstantiatorSource) _registry
+                .getService(ComponentInstantiatorSource.class);
+
+        _source.addPackage("org.apache.tapestry.internal.transform.pages");
+    }
+
+    @Configuration(afterTestClass = true)
+    public void shutdownRegistry()
+    {
+        _registry.shutdown();
 
+        _registry = null;
+        _source = null;
     }
 }

Modified: tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/InternalClassTransformationImplTest.java
URL: http://svn.apache.org/viewcvs/tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/InternalClassTransformationImplTest.java?rev=397359&r1=397358&r2=397359&view=diff
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/InternalClassTransformationImplTest.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/InternalClassTransformationImplTest.java Wed Apr 26 19:00:23 2006
@@ -228,12 +228,10 @@
     /**
      * More a test of how Javassist works. Javassist does not honor the Inherited annotation for
      * classes (this kind of makes sense, since it won't necessarily have the super-class in
-     * memory). Eventually we'll sort that out inside of InternalClassTransformationImpl. In the
-     * meantime, we just make sure Javassist works the way we think it does (in case a later
-     * Javassist changes this behavior).
+     * memory).
      */
     @Test
-    public void ensureSubclassesDoNotInheritAnnotations() throws Exception
+    public void ensureSubclassesInheritClassAnnotations() throws Exception
     {
         // The Java runtime does honor @Inherited
         assertNotNull(ChildClassInheritsAnnotation.class.getAnnotation(ComponentClass.class));
@@ -242,8 +240,9 @@
 
         ComponentClass cc = ct.getAnnotation(ComponentClass.class);
 
-        // But Javassist does not.
-        assertNull(cc);
+        // Javassist does not, but ClassTransformation patches around that.
+
+        assertNotNull(cc);
     }
 
     /**

Added: tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/pages/BasicSubComponent.java
URL: http://svn.apache.org/viewcvs/tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/pages/BasicSubComponent.java?rev=397359&view=auto
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/pages/BasicSubComponent.java (added)
+++ tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/transform/pages/BasicSubComponent.java Wed Apr 26 19:00:23 2006
@@ -0,0 +1,17 @@
+package org.apache.tapestry.internal.transform.pages;
+
+/** @author Howard M. Lewis Ship */
+public class BasicSubComponent extends BasicComponent
+{
+    private int _intValue;
+
+    public final int getIntValue()
+    {
+        return _intValue;
+    }
+
+    public final void setIntValue(int intValue)
+    {
+        _intValue = intValue;
+    }
+}



---------------------------------------------------------------------
To unsubscribe, e-mail: tapestry-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tapestry-dev-help@jakarta.apache.org