243559 - [hotbug] Exchanging entity objects contents when renaming a certain entity

6 files changed, 160 insertions, 57 deletions

diff --git a/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/java/GenericJavaPersistentType.java b/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/java/GenericJavaPersistentType.java
index feb54cc1fc..07e92585fd 100644
--- a/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/java/GenericJavaPersistentType.java
+++ b/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/java/GenericJavaPersistentType.java
@@ -167,14 +167,18 @@ public class GenericJavaPersistentType extends AbstractJavaJpaContextNode implem
 		return this.attributes.size();
 	}
 	
-	private void addAttribute(JavaPersistentAttribute attribute) {
-		addItemToList(attribute, this.attributes, PersistentType.SPECIFIED_ATTRIBUTES_LIST);
+	private void addAttribute(int index, JavaPersistentAttribute attribute) {
+		addItemToList(index, attribute, this.attributes, PersistentType.SPECIFIED_ATTRIBUTES_LIST);
 	}
 
 	private void removeAttribute(JavaPersistentAttribute attribute) {
 		removeItemFromList(attribute, this.attributes, PersistentType.SPECIFIED_ATTRIBUTES_LIST);
 	}
 	
+	private void moveAttribute(int index, JavaPersistentAttribute attribute) {
+		moveItemInList(index, this.attributes.indexOf(attribute), this.attributes, PersistentType.SPECIFIED_ATTRIBUTES_LIST);
+	}
+	
 	public Iterator<String> attributeNames() {
 		return this.attributeNames(this.attributes());
 	}
@@ -417,24 +421,38 @@ public class GenericJavaPersistentType extends AbstractJavaJpaContextNode implem
 	}
 
 	protected void updatePersistentAttributes(JavaResourcePersistentType jrpt) {
-		ListIterator<JavaPersistentAttribute> contextAttributes = attributes();
 		Iterator<JavaResourcePersistentAttribute> resourceAttributes = jrpt.fields();
 		if (getAccess() == AccessType.PROPERTY) {
 			resourceAttributes = jrpt.properties();
-		}		
+		}
+		Collection<JavaPersistentAttribute> contextAttributesToRemove = CollectionTools.collection(attributes());
+		Collection<JavaPersistentAttribute> contextAttributesToUpdate = new ArrayList<JavaPersistentAttribute>();
+		int resourceIndex = 0;
 		
-		while (contextAttributes.hasNext()) {
-			JavaPersistentAttribute persistentAttribute = contextAttributes.next();
-			if (resourceAttributes.hasNext()) {
-				persistentAttribute.update(resourceAttributes.next());
+		while (resourceAttributes.hasNext()) {
+			JavaResourcePersistentAttribute resourceAttribute = resourceAttributes.next();
+			boolean contextAttributeFound = false;
+			for (JavaPersistentAttribute contextAttribute : contextAttributesToRemove) {
+				if (contextAttribute.getResourcePersistentAttribute() == resourceAttribute) {
+					moveAttribute(resourceIndex, contextAttribute);
+					contextAttributesToRemove.remove(contextAttribute);
+					contextAttributesToUpdate.add(contextAttribute);
+					contextAttributeFound = true;
+					break;
+				}
 			}
-			else {
-				removeAttribute(persistentAttribute);
+			if (!contextAttributeFound) {
+				addAttribute(resourceIndex, createAttribute(resourceAttribute));
 			}
+			resourceIndex++;
 		}
-		
-		while (resourceAttributes.hasNext()) {
-			addAttribute(createAttribute(resourceAttributes.next()));
+		for (JavaPersistentAttribute contextAttribute : contextAttributesToRemove) {
+			removeAttribute(contextAttribute);
+		}
+		//first handle adding/removing of the persistent attributes, then update the others last, 
+		//this causes less churn in the update process
+		for (JavaPersistentAttribute contextAttribute : contextAttributesToUpdate) {
+			contextAttribute.update(contextAttribute.getResourcePersistentAttribute());
 		}
 	}
 	
diff --git a/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/persistence/GenericPersistenceUnit.java b/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/persistence/GenericPersistenceUnit.java
index 395bc9c04f..8dc2406c7c 100644
--- a/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/persistence/GenericPersistenceUnit.java
+++ b/jpa/plugins/org.eclipse.jpt.core/src/org/eclipse/jpt/core/internal/context/persistence/GenericPersistenceUnit.java
@@ -798,7 +798,7 @@ public class GenericPersistenceUnit extends AbstractPersistenceJpaContextNode
 		updateMappingFileRefs(persistenceUnit);
 		//update implied classRefs last since they depend on both
 		//specified classRefs and mappingFileRefs
-		updateImpliedClassRefs(persistenceUnit);
+		updateImpliedClassRefs();
 		updateExcludeUnlistedClasses(persistenceUnit);
 		updateProperties(persistenceUnit);
 		updatePersistenceUnitDefaults();
@@ -906,57 +906,66 @@ public class GenericPersistenceUnit extends AbstractPersistenceJpaContextNode
 		return getJpaFactory().buildMappingFileRef(this, xmlMappingFileRef);
 	}
 	
+	//this is not being changed to match updateImpliedClassRefs.  In the xml,
+	//changing the class name does not imply that a new object needs to be created.
+	//If anything we should be matching ClassRefs with the corresponding XmlJavaClassRef
+	//and if there is not one the ClassRef would be removed.  We would not want to 
+	//do a name match as is done for the implied class refs.
 	protected void updateSpecifiedClassRefs(XmlPersistenceUnit persistenceUnit) {
-		Iterator<ClassRef> stream = specifiedClassRefs();
-		Iterator<XmlJavaClassRef> stream2 = new CloneIterator<XmlJavaClassRef>(persistenceUnit.getClasses());//prevent ConcurrentModificiationException
+		Iterator<ClassRef> contextClassRefs = specifiedClassRefs();
+		Iterator<XmlJavaClassRef> resourceClassRefs = new CloneIterator<XmlJavaClassRef>(persistenceUnit.getClasses());//prevent ConcurrentModificiationException
 		
-		while (stream.hasNext()) {
-			ClassRef classRef = stream.next();
-			if (stream2.hasNext()) {
-				classRef.update(stream2.next());
+		while (contextClassRefs.hasNext()) {
+			ClassRef contextClassRef = contextClassRefs.next();
+			if (resourceClassRefs.hasNext()) {
+				contextClassRef.update(resourceClassRefs.next());
 			}
 			else {
-				removeSpecifiedClassRef_(classRef);
+				removeSpecifiedClassRef_(contextClassRef);
 			}
 		}
 		
-		while (stream2.hasNext()) {
-			addSpecifiedClassRef_(buildClassRef(stream2.next()));
+		while (resourceClassRefs.hasNext()) {
+			addSpecifiedClassRef_(buildClassRef(resourceClassRefs.next()));
 		}
 	}
 	
-	protected void updateImpliedClassRefs(XmlPersistenceUnit persistenceUnit) {
-		Iterator<ClassRef> impliedRefs = impliedClassRefs();
-		Iterator<String> annotatedClassNames = getJpaProject().annotatedClassNames();
+	protected void updateImpliedClassRefs() {
+		if (isExcludeUnlistedClasses()) {
+			for (ClassRef classRef : CollectionTools.iterable(impliedClassRefs())) {
+				removeImpliedClassRef(classRef);
+			}
+			return;
+		}
 		
+		Iterator<String> annotatedClassNames = getJpaProject().annotatedClassNames();
+		Collection<ClassRef> impliedRefsToRemove = CollectionTools.collection(impliedClassRefs());
+		Collection<ClassRef> impliedRefsToUpdate = new ArrayList<ClassRef>();
 		
-		if ( ! isExcludeUnlistedClasses()) {
-			while (impliedRefs.hasNext()) {
-				ClassRef classRef = impliedRefs.next();
-				boolean updated = false;
-				while (! updated && annotatedClassNames.hasNext()) {
-					String annotatedClassName = annotatedClassNames.next();
-					if ( ! classIsSpecified(annotatedClassName)) {
-						classRef.update(annotatedClassName);
-						updated = true;
+		while (annotatedClassNames.hasNext()) {
+			String annotatedClassName = annotatedClassNames.next();
+			boolean impliedRefFound = false;
+			if (!classIsSpecified(annotatedClassName)) {
+				for (ClassRef classRef : impliedRefsToRemove) {
+					if (annotatedClassName.equals(classRef.getClassName())) {
+						impliedRefsToRemove.remove(classRef);
+						impliedRefsToUpdate.add(classRef);
+						impliedRefFound = true;
+						break;
 					}
 				}
-				if (! annotatedClassNames.hasNext() && ! updated) {
-					removeImpliedClassRef(classRef);
-				}
-			}
-			
-			while (annotatedClassNames.hasNext()) {
-				String annotatedClassName = annotatedClassNames.next();
-				if ( ! classIsSpecified(annotatedClassName)) {
+				if (!impliedRefFound) {
 					addImpliedClassRef(annotatedClassName);
 				}
 			}
 		}
-		else {
-			for (ClassRef classRef : CollectionTools.iterable(impliedClassRefs())) {
-				removeImpliedClassRef(classRef);
-			}
+		for (ClassRef classRef : impliedRefsToRemove) {
+			removeImpliedClassRef(classRef);
+		}
+		//first handle adding/removing of the implied class refs, then update the others last, 
+		//this causes less churn in the update process
+		for (ClassRef classRef : impliedRefsToUpdate) {
+			classRef.update(classRef.getClassName());
 		}
 	}
 	
diff --git a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/JpaFileTests.java b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/JpaFileTests.java
index 745987618b..cdb4ec56d1 100644
--- a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/JpaFileTests.java
+++ b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/JpaFileTests.java
@@ -13,6 +13,7 @@ import java.util.Iterator;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.jdt.core.ICompilationUnit;
 import org.eclipse.jpt.core.JpaFile;
+import org.eclipse.jpt.core.JpaStructureNode;
 import org.eclipse.jpt.core.JptCorePlugin;
 import org.eclipse.jpt.core.MappingKeys;
 import org.eclipse.jpt.core.context.java.JavaPersistentType;
@@ -254,8 +255,11 @@ public class JpaFileTests extends ContextModelTestCase
 		
 		addXmlClassRef(FULLY_QUALIFIED_TYPE_NAME);
 		JavaPersistentType javaPersistentType = javaPersistentType();
-		assertEquals(javaPersistentType, javaJpaFile.rootStructureNodes().next());
-		assertEquals(entityMappings().getPersistenceUnit().specifiedClassRefs().next(), javaJpaFile.rootStructureNodes().next().getParent());
+		Iterator<JpaStructureNode> rootStructureNodes = javaJpaFile.rootStructureNodes();
+		JpaStructureNode rootStructureNode = rootStructureNodes.next();
+		assertEquals(javaPersistentType, rootStructureNode);
+		assertEquals(entityMappings().getPersistenceUnit().specifiedClassRefs().next(), rootStructureNode.getParent());
+		assertFalse(rootStructureNodes.hasNext());
 		
 		removeXmlClassRef(FULLY_QUALIFIED_TYPE_NAME);
 		assertNotSame(javaPersistentType, javaJpaFile.rootStructureNodes().next());
diff --git a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/java/JavaPersistentTypeTests.java b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/java/JavaPersistentTypeTests.java
index 539bfede5c..337a249c91 100644
--- a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/java/JavaPersistentTypeTests.java
+++ b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/java/JavaPersistentTypeTests.java
@@ -12,6 +12,7 @@ package org.eclipse.jpt.core.tests.internal.context.java;
 import java.util.Iterator;
 import java.util.ListIterator;
 import org.eclipse.jdt.core.ICompilationUnit;
+import org.eclipse.jdt.core.IField;
 import org.eclipse.jpt.core.JptCorePlugin;
 import org.eclipse.jpt.core.MappingKeys;
 import org.eclipse.jpt.core.context.AccessType;
@@ -698,6 +699,32 @@ public class JavaPersistentTypeTests extends ContextModelTestCase
 		assertNull(javaPersistentType().getAttributeNamed("foo"));
 	}
 	
+	public void testRenameAttribute() throws Exception {
+		ICompilationUnit testType = createTestEntityAnnotatedField();
+		addXmlClassRef(FULLY_QUALIFIED_TYPE_NAME);
+		
+		ListIterator<JavaPersistentAttribute> attributes = javaPersistentType().attributes();
+		JavaPersistentAttribute idAttribute = attributes.next();
+		JavaPersistentAttribute nameAttribute = attributes.next();
+		
+		
+		assertEquals("id", idAttribute.getName());
+		assertEquals("name", nameAttribute.getName());
+		
+		IField idField = testType.findPrimaryType().getField("id");
+		idField.rename("id2", false, null);
+		
+		attributes = javaPersistentType().attributes();
+		JavaPersistentAttribute nameAttribute2 = attributes.next();
+		JavaPersistentAttribute id2Attribute = attributes.next();
+
+		assertNotSame(idAttribute, id2Attribute);
+		assertEquals("id2", id2Attribute.getName());
+		assertEquals(nameAttribute, nameAttribute2);
+		assertEquals("name", nameAttribute2.getName());
+		assertFalse(attributes.hasNext());
+	}
+
 	public void testParentPersistentTypeGeneric() throws Exception {
 		createTestGenericEntity();
 		createTestGenericMappedSuperclass();
diff --git a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/ClassRefTests.java b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/ClassRefTests.java
index fdf5a60c78..69ce756519 100644
--- a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/ClassRefTests.java
+++ b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/ClassRefTests.java
@@ -39,21 +39,19 @@ public class ClassRefTests extends ContextModelTestCase
 		// set xml class name to different name, test equality
 		xmlClassRef.setJavaClass("com.bar.Foo");
 		
+		classRef = persistenceUnit.specifiedClassRefs().next();
 		assertEquals(classRef.getClassName(), xmlClassRef.getJavaClass());
 		
 		// set class name to empty string, test equality
 		xmlClassRef.setJavaClass("");
 		
+		classRef = persistenceUnit.specifiedClassRefs().next();
 		assertEquals(classRef.getClassName(), xmlClassRef.getJavaClass());
 		
-		// set class name to null, test equality
-		xmlClassRef.setJavaClass(null);
-		
-		assertEquals(classRef.getClassName(), xmlClassRef.getJavaClass());
-		
-		// set name back to non-null, test equality
+		// set name back to non empty string, test equality
 		xmlClassRef.setJavaClass("com.foo.Bar");
 		
+		classRef = persistenceUnit.specifiedClassRefs().next();
 		assertEquals(classRef.getClassName(), xmlClassRef.getJavaClass());
 	}
 	
diff --git a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/PersistenceUnitTests.java b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/PersistenceUnitTests.java
index 08cd41c918..11314f9c01 100644
--- a/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/PersistenceUnitTests.java
+++ b/jpa/tests/org.eclipse.jpt.core.tests/src/org/eclipse/jpt/core/tests/internal/context/persistence/PersistenceUnitTests.java
@@ -16,6 +16,7 @@ import org.eclipse.jdt.core.ICompilationUnit;
 import org.eclipse.jpt.core.JptCorePlugin;
 import org.eclipse.jpt.core.MappingKeys;
 import org.eclipse.jpt.core.context.AccessType;
+import org.eclipse.jpt.core.context.java.JavaPersistentType;
 import org.eclipse.jpt.core.context.orm.OrmPersistentType;
 import org.eclipse.jpt.core.context.persistence.ClassRef;
 import org.eclipse.jpt.core.context.persistence.PersistenceUnit;
@@ -40,6 +41,10 @@ public class PersistenceUnitTests extends ContextModelTestCase
 	protected static final String INNER_CLASS_NAME = "InnerAnnotationTestType";
 	protected static final String FULLY_QUALIFIED_INNER_CLASS_NAME = PACKAGE_NAME + "." + TYPE_NAME + "." + INNER_CLASS_NAME;
 
+	public static final String OTHER_TYPE_NAME = "OtherTestType";
+	public static final String FULLY_QUALIFIED_OTHER_TYPE_NAME = PACKAGE_NAME + "." + OTHER_TYPE_NAME;
+
+
 	public PersistenceUnitTests(String name) {
 		super(name);
 	}
@@ -653,8 +658,8 @@ public class PersistenceUnitTests extends ContextModelTestCase
 		
 		removeXmlClassRef(FULLY_QUALIFIED_TYPE_NAME);
 		classRefs = persistenceUnit().impliedClassRefs();
-		assertEquals(FULLY_QUALIFIED_TYPE_NAME, classRefs.next().getClassName());
 		assertEquals(FULLY_QUALIFIED_INNER_CLASS_NAME, classRefs.next().getClassName());
+		assertEquals(FULLY_QUALIFIED_TYPE_NAME, classRefs.next().getClassName());
 		
 		entityMappings().addOrmPersistentType(MappingKeys.ENTITY_TYPE_MAPPING_KEY, FULLY_QUALIFIED_TYPE_NAME);
 		classRefs = persistenceUnit().impliedClassRefs();
@@ -675,7 +680,35 @@ public class PersistenceUnitTests extends ContextModelTestCase
 		assertFalse(classRefs.hasNext());
 	}
 	
-	public void testUpdateExcludeUnlistedClasses() {
+	public void testRenamePersistentTypeImpliedClassRefs() throws Exception {
+		getJavaProject().getJpaProject().setDiscoversAnnotatedClasses(true);
+		ICompilationUnit testType = createTestEntity();
+		ICompilationUnit otherTestType = this.createTestOtherTypeEntity();
+		
+		ListIterator<ClassRef> classRefs = persistenceUnit().impliedClassRefs();
+		ClassRef testTypeClassRef = classRefs.next();
+		ClassRef otherTestTypeClassRef = classRefs.next();
+		
+		assertEquals(FULLY_QUALIFIED_TYPE_NAME, testTypeClassRef.getClassName());
+		assertEquals(FULLY_QUALIFIED_OTHER_TYPE_NAME, otherTestTypeClassRef.getClassName());
+		
+		JavaPersistentType testJavaPersistentType = testTypeClassRef.getJavaPersistentType();
+		JavaPersistentType otherTestJavaPersistentType = otherTestTypeClassRef.getJavaPersistentType();
+		
+		testType.findPrimaryType().rename("TestType2", false, null);
+		
+		classRefs = persistenceUnit().impliedClassRefs();
+		otherTestTypeClassRef = classRefs.next();
+		testTypeClassRef = classRefs.next();
+		
+		assertEquals(FULLY_QUALIFIED_OTHER_TYPE_NAME, otherTestTypeClassRef.getClassName());
+		assertEquals("test.TestType2", testTypeClassRef.getClassName());
+		
+		assertEquals(otherTestJavaPersistentType, otherTestTypeClassRef.getJavaPersistentType());
+		assertNotSame(testJavaPersistentType, testTypeClassRef.getJavaPersistentType());
+	}
+	
+	public void testUpdateExcludeUnlistedClasses() throws Exception {
 		XmlPersistenceUnit xmlPersistenceUnit = xmlPersistenceUnit();
 		PersistenceUnit persistenceUnit = persistenceUnit();
 		
@@ -1064,6 +1097,20 @@ public class PersistenceUnitTests extends ContextModelTestCase
 		});
 	}
 	
+	private ICompilationUnit createTestOtherTypeEntity() throws Exception {
+		return this.createTestType(PACKAGE_NAME, OTHER_TYPE_NAME + ".java", OTHER_TYPE_NAME, new DefaultAnnotationWriter() {
+			@Override
+			public Iterator<String> imports() {
+				return new ArrayIterator<String>(JPA.ENTITY);
+			}
+				@Override
+			public void appendTypeAnnotationTo(StringBuilder sb) {
+				sb.append("@Entity");
+			}
+
+		});
+	}
+
 	private ICompilationUnit createTestEntityWithPersistentInnerClass() throws Exception {
 		createEntityAnnotation();