Consistent equality check for parent name and indexed arguments

Closes gh-23593

spring-beans/src/main/java/org/springframework/beans/factory/config/ConstructorArgumentValues.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.
@@ -408,7 +408,7 @@ public class ConstructorArgumentValues {
 		for (Map.Entry<Integer, ValueHolder> 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;
 			}
 		}

spring-beans/src/main/java/org/springframework/beans/factory/support/GenericBeanDefinition.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 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.
@@ -18,6 +18,7 @@ package org.springframework.beans.factory.support;
 
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.lang.Nullable;
+import org.springframework.util.ObjectUtils;
 
 /**
  * GenericBeanDefinition is a one-stop shop for standard bean definition purposes.
@@ -84,7 +85,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

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

@@ -794,6 +794,18 @@ public class DefaultListableBeanFactoryTests {
 		lbf.registerAlias("test", "test3");
 	}
 
+	@Test
+	public void testAliasChaining() {
+		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"));
+	}
+
 	@Test
 	public void testBeanDefinitionOverriding() {
 		lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class));
@@ -804,6 +816,45 @@ public class DefaultListableBeanFactoryTests {
 		assertTrue(lbf.getBean("test2") instanceof NestedTestBean);
 	}
 
+	@Test
+	public void testBeanDefinitionOverridingNotAllowed() {
+		lbf.setAllowBeanDefinitionOverriding(false);
+		BeanDefinition oldDef = new RootBeanDefinition(TestBean.class);
+		BeanDefinition newDef = new RootBeanDefinition(NestedTestBean.class);
+		lbf.registerBeanDefinition("test", oldDef);
+		try {
+			lbf.registerBeanDefinition("test", newDef);
+			fail("Should have thrown BeanDefinitionOverrideException");
+		}
+		catch (BeanDefinitionOverrideException ex) {
+			assertEquals("test", ex.getBeanName());
+			assertSame(newDef, ex.getBeanDefinition());
+			assertSame(oldDef, ex.getExistingDefinition());
+		}
+	}
+
+	@Test
+	public void testBeanDefinitionOverridingWithAlias() {
+		lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class));
+		lbf.registerAlias("test", "testAlias");
+		lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class));
+		lbf.registerAlias("test", "testAlias");
+		assertTrue(lbf.getBean("test") instanceof NestedTestBean);
+		assertTrue(lbf.getBean("testAlias") instanceof NestedTestBean);
+	}
+
+	@Test
+	public void beanDefinitionOverridingWithConstructorArgumentMismatch() {
+		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() {
 		lbf.setAllowBeanDefinitionOverriding(false);
@@ -844,45 +895,6 @@ public class DefaultListableBeanFactoryTests {
 		lbf.removeBeanDefinition(name);
 	}
 
-	@Test
-	public void testBeanDefinitionOverridingNotAllowed() {
-		lbf.setAllowBeanDefinitionOverriding(false);
-		BeanDefinition oldDef = new RootBeanDefinition(TestBean.class);
-		BeanDefinition newDef = new RootBeanDefinition(NestedTestBean.class);
-		lbf.registerBeanDefinition("test", oldDef);
-		try {
-			lbf.registerBeanDefinition("test", newDef);
-			fail("Should have thrown BeanDefinitionOverrideException");
-		}
-		catch (BeanDefinitionOverrideException ex) {
-			assertEquals("test", ex.getBeanName());
-			assertSame(newDef, ex.getBeanDefinition());
-			assertSame(oldDef, ex.getExistingDefinition());
-		}
-	}
-
-	@Test
-	public void testBeanDefinitionOverridingWithAlias() {
-		lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class));
-		lbf.registerAlias("test", "testAlias");
-		lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class));
-		lbf.registerAlias("test", "testAlias");
-		assertTrue(lbf.getBean("test") instanceof NestedTestBean);
-		assertTrue(lbf.getBean("testAlias") instanceof NestedTestBean);
-	}
-
-	@Test
-	public void testAliasChaining() {
-		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"));
-	}
-
 	@Test
 	public void testBeanReferenceWithNewSyntax() {
 		Properties p = new Properties();

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,27 @@ 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);
+		boolean condition1 = !bd.equals(otherBd);
+		assertTrue(condition1);
+		boolean condition = !otherBd.equals(bd);
+		assertTrue(condition);
+		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);