From b198cad58e8f9a0316f2d7fafce12b3a07cf5071 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 25 Aug 2015 17:06:23 +0200 Subject: [PATCH] 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 --- .../DefaultListableBeanFactoryTests.java | 11 +++ .../springframework/core/AliasRegistry.java | 6 +- .../core/SimpleAliasRegistry.java | 67 +++++++++++++------ 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index 1c6bab2058..4f8f7f26b7 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/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 diff --git a/spring-core/src/main/java/org/springframework/core/AliasRegistry.java b/spring-core/src/main/java/org/springframework/core/AliasRegistry.java index 0e1e18bdaf..921b05caba 100644 --- a/spring-core/src/main/java/org/springframework/core/AliasRegistry.java +++ b/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. diff --git a/spring-core/src/main/java/org/springframework/core/SimpleAliasRegistry.java b/spring-core/src/main/java/org/springframework/core/SimpleAliasRegistry.java index 1dfe2db56d..16c73b5f41 100644 --- a/spring-core/src/main/java/org/springframework/core/SimpleAliasRegistry.java +++ b/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); - if (registeredName != null && !registeredName.equals(name)) { + String registeredName = this.aliasMap.get(alias); + if (registeredName != null) { + 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 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"); - } - } - }