SimpleAliasRegistry detects circles between non-canonical aliases as well (through hasAlias)

Additionally, as an optimization, we skip the re-registration step for existing aliases now.

Issue: SPR-13390

spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java

@@ -788,6 +788,15 @@ public class DefaultListableBeanFactoryTests {
 		DefaultListableBeanFactory lbf = new DefaultListableBeanFactory();
 		lbf.registerAlias("test", "test2");
 		lbf.registerAlias("test2", "test3");
+
+		try {
+			lbf.registerAlias("test3", "test2");
+			fail("Should have thrown IllegalStateException");
+		}
+		catch (IllegalStateException ex) {
+			// expected
+		}
+
 		try {
 			lbf.registerAlias("test3", "test");
 			fail("Should have thrown IllegalStateException");
@@ -795,6 +804,8 @@ public class DefaultListableBeanFactoryTests {
 		catch (IllegalStateException ex) {
 			// expected
 		}
+
+		lbf.registerAlias("test", "test3");
 	}
 
 	@Test

spring-core/src/main/java/org/springframework/core/AliasRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -44,10 +44,10 @@ public interface AliasRegistry {
 	/**
 	 * Determine whether this given name is defines as an alias
 	 * (as opposed to the name of an actually registered component).
-	 * @param beanName the bean name to check
+	 * @param name the name to check
 	 * @return whether the given name is an alias
 	 */
-	boolean isAlias(String beanName);
+	boolean isAlias(String name);
 
 	/**
 	 * Return the aliases for the given name, if defined.

spring-core/src/main/java/org/springframework/core/SimpleAliasRegistry.java

@@ -49,9 +49,13 @@ public class SimpleAliasRegistry implements AliasRegistry {
 			this.aliasMap.remove(alias);
 		}
 		else {
-			if (!allowAliasOverriding()) {
+			String registeredName = this.aliasMap.get(alias);
-				String registeredName = this.aliasMap.get(alias);
+			if (registeredName != null) {
-				if (registeredName != null && !registeredName.equals(name)) {
+				if (registeredName.equals(name)) {
+					// An existing alias - no need to re-register
+					return;
+				}
+				if (!allowAliasOverriding()) {
 					throw new IllegalStateException("Cannot register alias '" + alias + "' for name '" +
 							name + "': It is already registered for name '" + registeredName + "'.");
 				}
@@ -69,6 +73,23 @@ public class SimpleAliasRegistry implements AliasRegistry {
 		return true;
 	}
 
+	/**
+	 * Determine whether the given name has the given alias registered.
+	 * @param name the name to check
+	 * @param alias the alias to look for
+	 * @since 4.2.1
+	 */
+	public boolean hasAlias(String name, String alias) {
+		for (Map.Entry<String, String> entry : this.aliasMap.entrySet()) {
+			String registeredName = entry.getValue();
+			if (registeredName.equals(name)) {
+				String registeredAlias = entry.getKey();
+				return (registeredAlias.equals(alias) || hasAlias(registeredAlias, alias));
+			}
+		}
+		return false;
+	}
+
 	@Override
 	public void removeAlias(String alias) {
 		String name = this.aliasMap.remove(alias);
@@ -127,7 +148,12 @@ public class SimpleAliasRegistry implements AliasRegistry {
 				}
 				else if (!resolvedAlias.equals(alias)) {
 					String existingName = this.aliasMap.get(resolvedAlias);
-					if (existingName != null && !existingName.equals(resolvedName)) {
+					if (existingName != null) {
+						if (existingName.equals(resolvedName)) {
+							// Pointing to existing alias - just remove placeholder
+							this.aliasMap.remove(alias);
+							break;
+						}
 						throw new IllegalStateException(
 								"Cannot register resolved alias '" + resolvedAlias + "' (original: '" + alias +
 								"') for name '" + resolvedName + "': It is already registered for name '" +
@@ -144,6 +170,23 @@ public class SimpleAliasRegistry implements AliasRegistry {
 		}
 	}
 
+	/**
+	 * Check whether the given name points back to the given alias as an alias
+	 * in the other direction already, catching a circular reference upfront
+	 * and throwing a corresponding IllegalStateException.
+	 * @param name the candidate name
+	 * @param alias the candidate alias
+	 * @see #registerAlias
+	 * @see #hasAlias
+	 */
+	protected void checkForAliasCircle(String name, String alias) {
+		if (hasAlias(alias, name)) {
+			throw new IllegalStateException("Cannot register alias '" + alias +
+					"' for name '" + name + "': Circular reference - '" +
+					name + "' is a direct or indirect alias for '" + alias + "' already");
+		}
+	}
+
 	/**
 	 * Determine the raw name, resolving aliases to canonical names.
 	 * @param name the user-specified name
@@ -163,20 +206,4 @@ public class SimpleAliasRegistry implements AliasRegistry {
 		return canonicalName;
 	}
 
-	/**
-	 * Check whether the given name points back to given alias as an alias
-	 * in the other direction, catching a circular reference upfront and
-	 * throwing a corresponding IllegalStateException.
-	 * @param name the candidate name
-	 * @param alias the candidate alias
-	 * @see #registerAlias
-	 */
-	protected void checkForAliasCircle(String name, String alias) {
-		if (alias.equals(canonicalName(name))) {
-			throw new IllegalStateException("Cannot register alias '" + alias +
-					"' for name '" + name + "': Circular reference - '" +
-					name + "' is a direct or indirect alias for '" + alias + "' already");
-		}
-	}
-
 }