Polishing

Author: sbrannen

spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactory.java

@@ -56,8 +56,8 @@
 
 /**
  * Factory that can create Spring AOP Advisors given AspectJ classes from
- * classes honoring the AspectJ 5 annotation syntax, using reflection to
- * invoke the corresponding advice methods.
+ * classes honoring AspectJ's annotation syntax, using reflection to invoke the
+ * corresponding advice methods.
  *
  * @author Rod Johnson
  * @author Adrian Colyer

spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJAwareAdvisorAutoProxyCreator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -50,27 +50,27 @@ public class AspectJAwareAdvisorAutoProxyCreator extends AbstractAdvisorAutoProx
 
 
 	/**
-	 * Sort the rest by AspectJ precedence. If two pieces of advice have
-	 * come from the same aspect they will have the same order.
-	 * Advice from the same aspect is then further ordered according to the
+	 * Sort the supplied {@link Advisor} instances according to AspectJ precedence.
+	 * <p>If two pieces of advice come from the same aspect, they will have the same
+	 * order. Advice from the same aspect is then further ordered according to the
 	 * following rules:
 	 * <ul>
-	 * <li>if either of the pair is after advice, then the advice declared
-	 * last gets highest precedence (runs last)</li>
-	 * <li>otherwise the advice declared first gets highest precedence (runs first)</li>
+	 * <li>If either of the pair is <em>after</em> advice, then the advice declared
+	 * last gets highest precedence (i.e., runs last).</li>
+	 * <li>Otherwise the advice declared first gets highest precedence (i.e., runs
+	 * first).</li>
 	 * </ul>
 	 * <p><b>Important:</b> Advisors are sorted in precedence order, from highest
 	 * precedence to lowest. "On the way in" to a join point, the highest precedence
-	 * advisor should run first. "On the way out" of a join point, the highest precedence
-	 * advisor should run last.
+	 * advisor should run first. "On the way out" of a join point, the highest
+	 * precedence advisor should run last.
 	 */
 	@Override
-	@SuppressWarnings("unchecked")
 	protected List<Advisor> sortAdvisors(List<Advisor> advisors) {
 		List<PartiallyComparableAdvisorHolder> partiallyComparableAdvisors = new ArrayList<>(advisors.size());
-		for (Advisor element : advisors) {
+		for (Advisor advisor : advisors) {
 			partiallyComparableAdvisors.add(
-					new PartiallyComparableAdvisorHolder(element, DEFAULT_PRECEDENCE_COMPARATOR));
+					new PartiallyComparableAdvisorHolder(advisor, DEFAULT_PRECEDENCE_COMPARATOR));
 		}
 		List<PartiallyComparableAdvisorHolder> sorted = PartialOrder.sort(partiallyComparableAdvisors);
 		if (sorted != null) {
@@ -86,8 +86,8 @@ protected List<Advisor> sortAdvisors(List<Advisor> advisors) {
 	}
 
 	/**
-	 * Adds an {@link ExposeInvocationInterceptor} to the beginning of the advice chain.
-	 * These additional advices are needed when using AspectJ expression pointcuts
+	 * Add an {@link ExposeInvocationInterceptor} to the beginning of the advice chain.
+	 * <p>This additional advice is needed when using AspectJ pointcut expressions
 	 * and when using AspectJ-style advice.
 	 */
 	@Override
@@ -110,7 +110,7 @@ protected boolean shouldSkip(Class<?> beanClass, String beanName) {
 
 
 	/**
-	 * Implements AspectJ PartialComparable interface for defining partial orderings.
+	 * Implements AspectJ's {@link PartialComparable} interface for defining partial orderings.
 	 */
 	private static class PartiallyComparableAdvisorHolder implements PartialComparable {
 
@@ -140,17 +140,19 @@ public Advisor getAdvisor() {
 
 		@Override
 		public String toString() {
-			StringBuilder sb = new StringBuilder();
 			Advice advice = this.advisor.getAdvice();
-			sb.append(ClassUtils.getShortName(advice.getClass()));
-			sb.append(": ");
+			StringBuilder sb = new StringBuilder(ClassUtils.getShortName(advice.getClass()));
+			boolean appended = false;
 			if (this.advisor instanceof Ordered) {
-				sb.append("order ").append(((Ordered) this.advisor).getOrder()).append(", ");
+				sb.append(": order = ").append(((Ordered) this.advisor).getOrder());
+				appended = true;
 			}
 			if (advice instanceof AbstractAspectJAdvice) {
+				sb.append(!appended ? ": " : ", ");
 				AbstractAspectJAdvice ajAdvice = (AbstractAspectJAdvice) advice;
+				sb.append("aspect name = ");
 				sb.append(ajAdvice.getAspectName());
-				sb.append(", declaration order ");
+				sb.append(", declaration order = ");
 				sb.append(ajAdvice.getDeclarationOrder());
 			}
 			return sb.toString();

spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java

@@ -27,20 +27,22 @@
 /**
  * Orders AspectJ advice/advisors by precedence (<i>not</i> invocation order).
  *
- * <p>Given two pieces of advice, {@code a} and {@code b}:
+ * <p>Given two pieces of advice, {@code A} and {@code B}:
  * <ul>
- * <li>if {@code a} and {@code b} are defined in different aspects, then the advice
- * in the aspect with the lowest order value has the highest precedence</li>
- * <li>if {@code a} and {@code b} are defined in the same aspect, then if one of
- * {@code a} or {@code b} is a form of after advice, then the advice declared last
- * in the aspect has the highest precedence. If neither {@code a} nor {@code b} is
- * a form of after advice, then the advice declared first in the aspect has the
- * highest precedence.</li>
+ * <li>If {@code A} and {@code B} are defined in different aspects, then the advice
+ * in the aspect with the lowest order value has the highest precedence.</li>
+ * <li>If {@code A} and {@code B} are defined in the same aspect, if one of
+ * {@code A} or {@code B} is a form of <em>after</em> advice, then the advice declared
+ * last in the aspect has the highest precedence. If neither {@code A} nor {@code B}
+ * is a form of <em>after</em> advice, then the advice declared first in the aspect
+ * has the highest precedence.</li>
  * </ul>
  *
- * <p>Important: Note that unlike a normal comparator a return of 0 means
- * we don't care about the ordering, not that the two elements must be sorted
- * identically. Used with AspectJ PartialOrder class.
+ * <p>Important: This comparator is used with AspectJ's
+ * {@link org.aspectj.util.PartialOrder PartialOrder} sorting utility. Thus, unlike
+ * a normal {@link Comparator}, a return value of {@code 0} from this comparator
+ * means we don't care about the ordering, not that the two elements must be sorted
+ * identically.
  *
  * @author Adrian Colyer
  * @author Juergen Hoeller
@@ -59,16 +61,16 @@ class AspectJPrecedenceComparator implements Comparator<Advisor> {
 
 
 	/**
-	 * Create a default AspectJPrecedenceComparator.
+	 * Create a default {@code AspectJPrecedenceComparator}.
 	 */
 	public AspectJPrecedenceComparator() {
 		this.advisorComparator = AnnotationAwareOrderComparator.INSTANCE;
 	}
 
 	/**
-	 * Create a AspectJPrecedenceComparator, using the given Comparator
+	 * Create an {@code AspectJPrecedenceComparator}, using the given {@link Comparator}
 	 * for comparing {@link org.springframework.aop.Advisor} instances.
-	 * @param advisorComparator the Comparator to use for Advisors
+	 * @param advisorComparator the {@code Comparator} to use for advisors
 	 */
 	public AspectJPrecedenceComparator(Comparator<? super Advisor> advisorComparator) {
 		Assert.notNull(advisorComparator, "Advisor comparator must not be null");
@@ -125,20 +127,20 @@ private boolean declaredInSameAspect(Advisor advisor1, Advisor advisor2) {
 				getAspectName(advisor1).equals(getAspectName(advisor2)));
 	}
 
-	private boolean hasAspectName(Advisor anAdvisor) {
-		return (anAdvisor instanceof AspectJPrecedenceInformation ||
-				anAdvisor.getAdvice() instanceof AspectJPrecedenceInformation);
+	private boolean hasAspectName(Advisor advisor) {
+		return (advisor instanceof AspectJPrecedenceInformation ||
+				advisor.getAdvice() instanceof AspectJPrecedenceInformation);
 	}
 
 	// pre-condition is that hasAspectName returned true
-	private String getAspectName(Advisor anAdvisor) {
-		AspectJPrecedenceInformation pi = AspectJAopUtils.getAspectJPrecedenceInformationFor(anAdvisor);
-		Assert.state(pi != null, "Unresolvable precedence information");
-		return pi.getAspectName();
+	private String getAspectName(Advisor advisor) {
+		AspectJPrecedenceInformation precedenceInfo = AspectJAopUtils.getAspectJPrecedenceInformationFor(advisor);
+		Assert.state(precedenceInfo != null, () -> "Unresolvable AspectJPrecedenceInformation for " + advisor);
+		return precedenceInfo.getAspectName();
 	}
 
-	private int getAspectDeclarationOrder(Advisor anAdvisor) {
-		AspectJPrecedenceInformation precedenceInfo = AspectJAopUtils.getAspectJPrecedenceInformationFor(anAdvisor);
+	private int getAspectDeclarationOrder(Advisor advisor) {
+		AspectJPrecedenceInformation precedenceInfo = AspectJAopUtils.getAspectJPrecedenceInformationFor(advisor);
 		return (precedenceInfo != null ? precedenceInfo.getDeclarationOrder() : 0);
 	}
 

spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAdvisorAutoProxyCreator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,11 +36,13 @@
  * also override the inherited {@link #shouldSkip} method to exclude certain
  * objects from auto-proxying.
  *
- * <p>Advisors or advices requiring ordering should implement the
+ * <p>Advisors or advices requiring ordering should be annotated with
+ * {@link org.springframework.core.annotation.Order @Order} or implement the
  * {@link org.springframework.core.Ordered} interface. This class sorts
- * Advisors by Ordered order value. Advisors that don't implement the
- * Ordered interface will be considered as unordered; they will appear
- * at the end of the advisor chain in undefined order.
+ * advisors using the {@link AnnotationAwareOrderComparator}. Advisors that are
+ * not annotated with {@code @Order} or don't implement the {@code Ordered}
+ * interface will be considered as unordered; they will appear at the end of the
+ * advisor chain in an undefined order.
  *
  * @author Rod Johnson
  * @author Juergen Hoeller