fixed JASSIST-162 and JASSIST-189

git-svn-id: http://anonsvn.jboss.org/repos/javassist/trunk@701 30ef5769-5b8d-40dd-aea6-55b5d6557bb3

Readme.html

@@ -281,6 +281,11 @@ see javassist.Dump.
 
 <h2>Changes</h2>
 
+<p>-version 3.18
+<ul>
+JIRA JASSIST-183, 189, 162. 
+</ul>
+
 <p>-version 3.17.1 on December 3, 2012
 <ul>
 	<li>JIRA JASSIST-177, 178, 182

build.xml

@@ -154,7 +154,7 @@ to ${build.classes.dir}.</echo>
   </target>
 
   <target name="jar" depends="compile14">
-    <jar jarfile="${target.jar}" manifest="${src.dir}/META-INF/MANIFEST.MF">
+    <jar jarfile="${target.jar}" update="true" manifest="${src.dir}/META-INF/MANIFEST.MF">
       <fileset dir="${build.classes.dir}">
       	  <include name="**/*.class"/>
       </fileset>

src/main/javassist/bytecode/ClassFile.java

@@ -690,12 +690,15 @@ public final class ClassFile {
             if (notBridgeMethod(minfo))
                 return true;
             else {
+            	// if the bridge method with the same signature
+            	// already exists, replace it.
                 it.remove();
                 return false;
             }
         }
         else
-           return notBridgeMethod(minfo) && notBridgeMethod(newMethod);
+        	return false;
+           // return notBridgeMethod(minfo) && notBridgeMethod(newMethod);
     }
 
     /* For a bridge method, see Sec. 15.12.4.5 of JLS 3rd Ed.

src/main/javassist/util/proxy/ProxyFactory.java

@@ -800,9 +800,9 @@ public class ProxyFactory {
     {
         checkClassAndSuperName();
 
+        hasGetHandler = false;      // getMethods() may set this to true.
         HashMap allMethods = getMethods(superClass, interfaces);
         signatureMethods = new ArrayList(allMethods.entrySet());
-        hasGetHandler = allMethods.get(HANDLER_GETTER_KEY) != null;
         Collections.sort(signatureMethods, sorter);
     }
 
@@ -981,17 +981,22 @@ public class ProxyFactory {
             Map.Entry e = (Map.Entry)it.next();
             String key = (String)e.getKey();
             Method meth = (Method)e.getValue();
-            int mod = meth.getModifiers();
-            if (testBit(signature, index)) {
-                override(className, meth, prefix, index,
-                         keyToDesc(key, meth), cf, cp, forwarders);
-            }
+            if (ClassFile.MAJOR_VERSION < ClassFile.JAVA_5 || !isBridge(meth))
+            	if (testBit(signature, index)) {
+            		override(className, meth, prefix, index,
+            				 keyToDesc(key, meth), cf, cp, forwarders);
+            	}
+
             index++;
         }
 
         return index;
     }
 
+    private static boolean isBridge(Method m) {
+    	return m.isBridge();
+    }
+
     private void override(String thisClassname, Method meth, String prefix,
                           int index, String desc, ClassFile cf, ConstPool cp, ArrayList forwarders)
         throws CannotCompileException
@@ -1083,7 +1088,9 @@ public class ProxyFactory {
             return name.substring(0, i);
     }
 
-    private static HashMap getMethods(Class superClass, Class[] interfaceTypes) {
+    /* getMethods() may set hasGetHandler to true.
+     */
+    private HashMap getMethods(Class superClass, Class[] interfaceTypes) {
         HashMap hash = new HashMap();
         HashSet set = new HashSet();
         for (int i = 0; i < interfaceTypes.length; i++)
@@ -1093,7 +1100,7 @@ public class ProxyFactory {
         return hash;
     }
 
-    private static void getMethods(HashMap hash, Class clazz, Set visitedClasses) {
+    private void getMethods(HashMap hash, Class clazz, Set visitedClasses) {
         // This both speeds up scanning by avoiding duplicate interfaces and is needed to
         // ensure that superinterfaces are always scanned before subinterfaces.
         if (!visitedClasses.add(clazz))
@@ -1107,12 +1114,19 @@ public class ProxyFactory {
         if (parent != null)
             getMethods(hash, parent, visitedClasses);
 
+        /* Java 5 or later allows covariant return types.
+         * It also allows contra-variant parameter types
+         * if a super class is a generics with concrete type arguments
+         * such as Foo<String>.  So the method-overriding rule is complex.
+         */
         Method[] methods = SecurityActions.getDeclaredMethods(clazz);
         for (int i = 0; i < methods.length; i++)
             if (!Modifier.isPrivate(methods[i].getModifiers())) {
                 Method m = methods[i];
-                // JIRA JASSIST-127 (covariant return types).
-                String key = m.getName() + ':' + RuntimeSupport.makeDescriptor(m.getParameterTypes(), null);
+                String key = m.getName() + ':' + RuntimeSupport.makeDescriptor(m);	// see keyToDesc().
+                if (key.startsWith(HANDLER_GETTER_KEY))
+                    hasGetHandler = true;
+
                 // JIRA JASSIST-85
                 // put the method to the cache, retrieve previous definition (if any) 
                 Method oldMethod = (Method)hash.put(key, methods[i]); 
@@ -1127,11 +1141,11 @@ public class ProxyFactory {
             }
     }
 
-    private static final String HANDLER_GETTER_KEY = HANDLER_GETTER + ":()";
+    private static final String HANDLER_GETTER_KEY
+    	= HANDLER_GETTER + ":()";
 
     private static String keyToDesc(String key, Method m) {
-        String params = key.substring(key.indexOf(':') + 1);
-        return RuntimeSupport.makeDescriptor(params, m.getReturnType());
+        return key.substring(key.indexOf(':') + 1);
     }
 
     private static MethodInfo makeConstructor(String thisClassName, Constructor cons,

src/test/javassist/JvstTest2.java

@@ -436,14 +436,14 @@ public class JvstTest2 extends JvstTestRoot {
 
     public void testAddMethod() throws Exception {
         CtClass cc = sloader.get("test2.AddMethod");
-	CtMethod m = CtNewMethod.make(
-                         "public void f() { return 1; }", cc);
+        CtMethod m = CtNewMethod.make(
+                         "public int f() { return 1; }", cc);
         try {
             cc.addMethod(m);
             fail();
         }
         catch (CannotCompileException e) {}
-	CtMethod m2 = CtNewMethod.make(
+        CtMethod m2 = CtNewMethod.make(
                          "public void f(int i, int j) { return 1; }", cc);
         cc.addMethod(m2);
         try {

src/test/test/javassist/proxy/JBPAPP9257Test.java

@@ -27,13 +27,14 @@ public class JBPAPP9257Test extends TestCase {
                 // method.
             }
         };
-        Foo foo = (Foo) c.newInstance();
+        Foo foo = (Foo)c.newInstance();
         try {
             ((ProxyObject)foo).setHandler(mi);
             fail("foo is a ProxyObject!");
         } catch (ClassCastException e) {}
         ((Proxy)foo).setHandler(mi);
         assertEquals("I'm doing something!", foo.doSomething());
+        assertEquals("This is a secret handler!", foo.getHandler());
     }
 
     public void testGetHandler2() throws Exception {
@@ -61,5 +62,6 @@ public class JBPAPP9257Test extends TestCase {
         } catch (ClassCastException e) {}
         ((Proxy)foo).setHandler(mi);
         assertEquals("do something!", foo.doSomething());
+        assertEquals("return a string!", foo.getHandler());
     }
 }

src/test/test1/Delegator.java

@@ -1,7 +1,7 @@
 package test1;
 
 class SuperDelegator {
-    public int f(int p) { return p + 1; }
+	public int f(int p) { return p + 1; }
     public static int g(int p) { return p + 1; }
 }
 

src/test/testproxy/ProxyTester.java

@@ -26,9 +26,9 @@ public class ProxyTester extends TestCase {
 
         public Object invoke(Object self, Method m, Method proceed,
                 Object[] args) throws Exception {
-            System.out.println("intercept: " + m + ", proceed: " + proceed
-                    + ", modifier: "
-                    + Modifier.toString(proceed.getModifiers()));
+            System.out.println("intercept: " + m + ", proceed: " + proceed);
+            System.out.println("     modifier: "
+            				   + Modifier.toString(proceed.getModifiers()));
             counter++;
             return proceed.invoke(self, args);
         }
@@ -396,6 +396,38 @@ public class ProxyTester extends TestCase {
         public Object writeReplace(int i) { return new Integer(i); }
     }
 
+    public static void testJIRA189() throws Exception {
+    	Class persistentClass = Target189.PublishedArticle.class;
+        ProxyFactory factory = new ProxyFactory();
+        factory.writeDirectory = ".";
+        factory.setUseCache(false);
+        factory.setSuperclass(persistentClass);
+        factory.setInterfaces(new Class[] { Target189.TestProxy.class });
+        Class cl = factory.createClass();
+        Target189.TestProxy proxy = (Target189.TestProxy)cl.newInstance();
+        Target189.TestMethodHandler methodHandler = new Target189.TestMethodHandler();
+        ((ProxyObject)proxy).setHandler(methodHandler);
+        ((Target189.Article)proxy).getIssue();
+        assertTrue(methodHandler.wasInvokedOnce());
+        methodHandler.reset();
+        Target189.PublishedArticle article = (Target189.PublishedArticle)proxy;
+        article.getIssue();
+        assertTrue(methodHandler.wasInvokedOnce());
+    }
+
+    public void testJIRA127() throws Exception {
+        ProxyFactory proxyFactory = new ProxyFactory();
+        proxyFactory.writeDirectory = ".";
+        proxyFactory.setInterfaces(new Class[]{ Target127.Sub.class });
+        Target127.Sub proxy = (Target127.Sub)proxyFactory.create(new Class[0], new Object[0], new MethodHandler() {
+            public Object invoke(Object self, Method thisMethod, Method proceed, Object[] args) throws Throwable {
+                return null;
+            }
+        });
+        ((Target127.Super)proxy).item();    // proxyFactory must generate a bridge method.
+        ((Target127.Sub)proxy).item();
+    }
+
     public static void main(String[] args) {
         // javassist.bytecode.ClassFile.MAJOR_VERSION = javassist.bytecode.ClassFile.JAVA_6;
         junit.textui.TestRunner.run(ProxyTester.class);

src/test/testproxy/Target127.java

@@ -0,0 +1,20 @@
+package testproxy;
+
+public class Target127 {
+    public interface Item { }
+    public interface CovariantItem extends Item { }
+
+    public interface Super {
+        Item item();
+    }
+
+    public interface Sub extends Super {
+        CovariantItem item();
+    }
+
+    public static class RealSub implements Sub {
+        public CovariantItem item() {
+            return null;
+        }
+    }
+}

src/test/testproxy/Target189.java

@@ -0,0 +1,74 @@
+package testproxy;
+
+import javassist.util.proxy.MethodHandler;
+import java.lang.reflect.Method;
+
+public class Target189 {
+	public interface TestProxy {
+	}
+
+	public static class TestMethodHandler implements MethodHandler {
+
+		int invoked = 0;
+
+		public Object invoke(Object self, Method thisMethod, Method proceed,
+				Object[] args) throws Throwable {
+			invoked++;
+			return proceed.invoke(self, args);
+		}
+
+		public boolean wasInvokedOnce() {
+			return invoked == 1;
+		}
+
+		public void reset() {
+			invoked = 0;
+		}
+	}
+
+	public static class Issue {
+
+		private Integer id;
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+	}
+
+	public static class PublishedIssue extends Issue {
+	}
+
+	public static abstract class Article {
+		private Integer id;
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public abstract Issue getIssue();
+	}
+
+	public static class PublishedArticle extends Article {
+
+		private PublishedIssue issue;
+
+		@Override
+		public PublishedIssue getIssue() {
+			return issue;
+		}
+
+		public void setIssue(PublishedIssue issue) {
+			this.issue = issue;
+		}
+
+	}
+
+}