Fixed toString(), toShortString(), and toLongString() methods for signature and join point (SPR-5437)

Author: ramnivas

org.springframework.aop/src/main/java/org/springframework/aop/aspectj/MethodInvocationProceedingJoinPoint.java

@@ -17,6 +17,7 @@
 package org.springframework.aop.aspectj;
 
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 
 import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.ProceedingJoinPoint;
@@ -44,6 +45,7 @@
  * @author Rod Johnson
  * @author Juergen Hoeller
  * @author Adrian Colyer
+ * @author Ramnivas Laddad
  * @since 2.0
  */
 public class MethodInvocationProceedingJoinPoint implements ProceedingJoinPoint, JoinPoint.StaticPart {
@@ -132,19 +134,17 @@ public String getKind() {
 	public JoinPoint.StaticPart getStaticPart() {
 		return this;
 	}
-	
 
 	public String toShortString() {
-		return "execution(" + this.methodInvocation.getMethod().getName() + ")";
+		return "execution(" + getSignature().toShortString() + ")";
 	}
 
 	public String toLongString() {
-		return getClass().getName() + ": execution: [" + this.methodInvocation + "]";
+		return "execution(" + getSignature().toLongString() + ")";
 	}
 
-	@Override
 	public String toString() {
-		return getClass().getName() + ": " + toShortString();
+		return "execution(" + getSignature().toString() + ")";
 	}
 
 
@@ -153,14 +153,6 @@ public String toString() {
 	 */
 	private class MethodSignatureImpl implements Signature, MethodSignature {
 
-		public String toShortString() {
-			return methodInvocation.getMethod().getName();
-		}
-
-		public String toLongString() {
-			return methodInvocation.getMethod().toString();
-		}
-
 		public String getName() {
 			return methodInvocation.getMethod().getName();
 		}
@@ -199,8 +191,75 @@ public String[] getParameterNames() {
 		public Class[] getExceptionTypes() {
 			return methodInvocation.getMethod().getExceptionTypes();
 		}
+
+		public String toShortString() {
+			return toString(false, false, false, false);
+		}
+
+		public String toLongString() {
+			return toString(true, true, true, true);
+		}
+
+		public String toString() {
+			return toString(false, true, false, true);
+		}
+
+		private String toString(boolean includeModifier,
+				boolean includeReturnTypeAndArgs,
+				boolean useLongReturnAndArgumentTypeName,
+				boolean useLongTypeName) {
+			StringBuilder sb = new StringBuilder();
+			if (includeModifier) {
+				sb.append(Modifier.toString(getModifiers()));
+				sb.append(" ");
+			}
+			if (includeReturnTypeAndArgs) {
+				appendType(sb, getReturnType(),
+						useLongReturnAndArgumentTypeName);
+				sb.append(" ");
+			}
+			appendType(sb, getDeclaringType(), useLongTypeName);
+			sb.append(".");
+			sb.append(getMethod().getName());
+			sb.append("(");
+
+			Class[] parametersTypes = getParameterTypes();
+			appendTypes(sb, parametersTypes, includeReturnTypeAndArgs,
+					useLongReturnAndArgumentTypeName);
+			sb.append(")");
+			return sb.toString();
+		}
+	}
+
+	private void appendTypes(StringBuilder sb, Class<?>[] types,
+			boolean includeArgs, boolean useLongReturnAndArgumentTypeName) {
+		if (includeArgs) {
+			for (int size = types.length, i = 0; i < size; i++) {
+				appendType(sb, types[i], useLongReturnAndArgumentTypeName);
+				if (i < size - 1) {
+					sb.append(",");
+				}
+			}
+		} else {
+			if (types.length != 0) {
+				sb.append("..");
+			}
+		}
 	}
 
+	private void appendType(StringBuilder sb, Class<?> type,
+			boolean useLongTypeName) {
+		if (type.isArray()) {
+			appendType(sb, type.getComponentType(), useLongTypeName);
+			sb.append("[]");
+		} else {
+			if (type.getPackage() != null
+					&& type.getPackage().equals("java.lang")) {
+				useLongTypeName = false;
+			}
+			sb.append(useLongTypeName ? type.getName() : type.getSimpleName());
+		}
+	}
 
 	/**
 	 * Lazily initialized SourceLocation.

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/MethodInvocationProceedingJoinPointTests.java

@@ -18,6 +18,7 @@
 
 import static org.junit.Assert.*;
 
+import java.io.IOException;
 import java.lang.reflect.Method;
 import java.util.Arrays;
 
@@ -26,6 +27,7 @@
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.reflect.MethodSignature;
 import org.aspectj.lang.reflect.SourceLocation;
+import org.aspectj.runtime.reflect.Factory;
 import org.junit.Test;
 
 import org.springframework.aop.MethodBeforeAdvice;
@@ -40,6 +42,7 @@
 /**
  * @author Rod Johnson
  * @author Chris Beams
+ * @author Ramnivas Laddad
  * @since 2.0
  */
 public final class MethodInvocationProceedingJoinPointTests {
@@ -186,4 +189,38 @@ public void before(Method method, Object[] args, Object target) throws Throwable
 		itb.getAge();
 	}
 
+	@Test
+	public void toShortAndLongStringFormedCorrectly() throws Exception {
+		final Object raw = new TestBean();
+		ProxyFactory pf = new ProxyFactory(raw);
+		pf.addAdvisor(ExposeInvocationInterceptor.ADVISOR);
+		pf.addAdvice(new MethodBeforeAdvice() {
+			public void before(Method method, Object[] args, Object target) throws Throwable {
+				// makeEncSJP, although meant for computing the enclosing join point,
+				// it serves our purpose here
+				JoinPoint.StaticPart aspectJVersionJp = Factory.makeEncSJP(method);
+				JoinPoint jp = AbstractAspectJAdvice.currentJoinPoint();
+				
+				assertEquals(aspectJVersionJp.getSignature().toLongString(), jp.getSignature().toLongString());
+				assertEquals(aspectJVersionJp.getSignature().toShortString(), jp.getSignature().toShortString());
+				assertEquals(aspectJVersionJp.getSignature().toString(), jp.getSignature().toString());
+
+				assertEquals(aspectJVersionJp.toLongString(), jp.toLongString());
+				assertEquals(aspectJVersionJp.toShortString(), jp.toShortString());
+				assertEquals(aspectJVersionJp.toString(), jp.toString());
+			}
+		});
+		ITestBean itb = (ITestBean) pf.getProxy();
+		itb.getAge();
+		itb.setName("foo");
+		itb.getDoctor();
+		itb.getStringArray();
+		itb.getSpouses();
+		itb.setSpouse(new TestBean());
+		try {
+			itb.unreliableFileOperation();
+		} catch (IOException ex) {
+			// we don't realy care...
+		}
+	}
 }