From 5d5867603a4e812d7247760530303fc3157f361d Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 25 Sep 2019 12:07:29 +0200 Subject: [PATCH] Consistent equality check for parent name and indexed arguments Closes gh-23593 --- .../config/ConstructorArgumentValues.java | 4 +- .../support/GenericBeanDefinition.java | 12 +++- .../DefaultListableBeanFactoryTests.java | 55 ++++++++++++------- .../factory/support/BeanDefinitionTests.java | 21 ++++++- 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/config/ConstructorArgumentValues.java b/spring-beans/src/main/java/org/springframework/beans/factory/config/ConstructorArgumentValues.java index 9a4dd6b094..7baa09b76b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/config/ConstructorArgumentValues.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/config/ConstructorArgumentValues.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2019 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. @@ -401,7 +401,7 @@ public class ConstructorArgumentValues { for (Map.Entry entry : this.indexedArgumentValues.entrySet()) { ValueHolder vh1 = entry.getValue(); ValueHolder vh2 = that.indexedArgumentValues.get(entry.getKey()); - if (!vh1.contentEquals(vh2)) { + if (vh2 == null || !vh1.contentEquals(vh2)) { return false; } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/GenericBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/GenericBeanDefinition.java index f15bded6b6..4f8f06a375 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/GenericBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/GenericBeanDefinition.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2019 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. @@ -17,6 +17,7 @@ package org.springframework.beans.factory.support; import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.util.ObjectUtils; /** * GenericBeanDefinition is a one-stop shop for standard bean definition purposes. @@ -81,7 +82,14 @@ public class GenericBeanDefinition extends AbstractBeanDefinition { @Override public boolean equals(Object other) { - return (this == other || (other instanceof GenericBeanDefinition && super.equals(other))); + if (this == other) { + return true; + } + if (!(other instanceof GenericBeanDefinition)) { + return false; + } + GenericBeanDefinition that = (GenericBeanDefinition) other; + return (ObjectUtils.nullSafeEquals(this.parentName, that.parentName) && super.equals(other)); } @Override 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 6fc3205c46..cc5da315fd 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -811,26 +811,24 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testBeanDefinitionOverriding() { + public void testAliasChaining() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); - lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class)); lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class)); - lbf.registerAlias("otherTest", "test2"); - lbf.registerAlias("test", "test2"); - assertTrue(lbf.getBean("test") instanceof NestedTestBean); - assertTrue(lbf.getBean("test2") instanceof NestedTestBean); + lbf.registerAlias("test", "testAlias"); + lbf.registerAlias("testAlias", "testAlias2"); + lbf.registerAlias("testAlias2", "testAlias3"); + Object bean = lbf.getBean("test"); + assertSame(bean, lbf.getBean("testAlias")); + assertSame(bean, lbf.getBean("testAlias2")); + assertSame(bean, lbf.getBean("testAlias3")); } @Test - public void testBeanDefinitionRemoval() { + public void testBeanDefinitionOverriding() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); - lbf.setAllowBeanDefinitionOverriding(false); lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class)); - lbf.registerAlias("test", "test2"); - lbf.preInstantiateSingletons(); - lbf.removeBeanDefinition("test"); - lbf.removeAlias("test2"); lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class)); + lbf.registerAlias("otherTest", "test2"); lbf.registerAlias("test", "test2"); assertTrue(lbf.getBean("test") instanceof NestedTestBean); assertTrue(lbf.getBean("test2") instanceof NestedTestBean); @@ -863,16 +861,31 @@ public class DefaultListableBeanFactoryTests { } @Test - public void testAliasChaining() { + public void beanDefinitionOverridingWithConstructorArgumentMismatch() { DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); + RootBeanDefinition bd1 = new RootBeanDefinition(NestedTestBean.class); + bd1.getConstructorArgumentValues().addIndexedArgumentValue(1, "value1"); + lbf.registerBeanDefinition("test", bd1); + RootBeanDefinition bd2 = new RootBeanDefinition(NestedTestBean.class); + bd2.getConstructorArgumentValues().addIndexedArgumentValue(0, "value0"); + lbf.registerBeanDefinition("test", bd2); + assertTrue(lbf.getBean("test") instanceof NestedTestBean); + assertEquals("value0", lbf.getBean("test", NestedTestBean.class).getCompany()); + } + + @Test + public void testBeanDefinitionRemoval() { + DefaultListableBeanFactory lbf = new DefaultListableBeanFactory(); + lbf.setAllowBeanDefinitionOverriding(false); + lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class)); + lbf.registerAlias("test", "test2"); + lbf.preInstantiateSingletons(); + lbf.removeBeanDefinition("test"); + lbf.removeAlias("test2"); lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class)); - lbf.registerAlias("test", "testAlias"); - lbf.registerAlias("testAlias", "testAlias2"); - lbf.registerAlias("testAlias2", "testAlias3"); - Object bean = lbf.getBean("test"); - assertSame(bean, lbf.getBean("testAlias")); - assertSame(bean, lbf.getBean("testAlias2")); - assertSame(bean, lbf.getBean("testAlias3")); + lbf.registerAlias("test", "test2"); + assertTrue(lbf.getBean("test") instanceof NestedTestBean); + assertTrue(lbf.getBean("test2") instanceof NestedTestBean); } @Test diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanDefinitionTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanDefinitionTests.java index a2df91991b..f7d5cb1d4c 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanDefinitionTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanDefinitionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2019 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. @@ -100,6 +100,25 @@ public class BeanDefinitionTests { assertTrue(bd.hashCode() == otherBd.hashCode()); } + @Test + public void genericBeanDefinitionEquality() { + GenericBeanDefinition bd = new GenericBeanDefinition(); + bd.setParentName("parent"); + bd.setScope("request"); + bd.setAbstract(true); + bd.setLazyInit(true); + GenericBeanDefinition otherBd = new GenericBeanDefinition(); + otherBd.setScope("request"); + otherBd.setAbstract(true); + otherBd.setLazyInit(true); + assertTrue(!bd.equals(otherBd)); + assertTrue(!otherBd.equals(bd)); + otherBd.setParentName("parent"); + assertTrue(bd.equals(otherBd)); + assertTrue(otherBd.equals(bd)); + assertTrue(bd.hashCode() == otherBd.hashCode()); + } + @Test public void beanDefinitionHolderEquality() { RootBeanDefinition bd = new RootBeanDefinition(TestBean.class); -- GitLab