HHH-2864 : merge non-inverse collection when 'owner' is already proxied

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@14091 1b8cb986-b30d-0410-93ca-fae66ebed9b2

core/src/main/java/org/hibernate/engine/StatefulPersistenceContext.java

@@ -1005,40 +1005,56 @@ public class StatefulPersistenceContext implements PersistenceContext {
 	}
 
 	/**
-	 * Search the persistence context for an owner for the child object,
+	 * Search <tt>this</tt> persistence context for an associated entity instance which is considered the "owner" of
-	 * given a collection role. If <tt>mergeMap</tt> is non-null, also
+	 * the given <tt>childEntity</tt>, and return that owner's id value.  This is performed in the scenario of a
-	 * check the detached graph being merged for a parent.
+	 * uni-directional, non-inverse one-to-many collection (which means that the collection elements do not maintain
+	 * a direct reference to the owner).
+	 * <p/>
+	 * As such, the processing here is basically to loop over every entity currently associated with this persistence
+	 * context and for those of the correct entity (sub) type to extract its collection role property value and see
+	 * if the child is contained within that collection.  If so, we have found the owner; if not, we go on.
+	 * <p/>
+	 * Also need to account for <tt>mergeMap</tt> which acts as a local copy cache managed for the duration of a merge
+	 * operation.  It represents a map of the detached entity instances pointing to the corresponding managed instance.
+	 *
+	 * @param entityName The entity name for the entity type which would own the child
+	 * @param propertyName The name of the property on the owning entity type which would name this child association.
+	 * @param childEntity The child entity instance for which to locate the owner instance id.
+	 * @param mergeMap A map of non-persistent instances from an on-going merge operation (possibly null).
+	 *
+	 * @return The id of the entityName instance which is said to own the child; null if an appropriate owner not
+	 * located.
 	 */
-	public Serializable getOwnerId(String entity, String property, Object childEntity, Map mergeMap) {
+	public Serializable getOwnerId(String entityName, String propertyName, Object childEntity, Map mergeMap) {
-		
+		final String collectionRole = entityName + '.' + propertyName;
-		EntityPersister persister = session.getFactory()
+		final EntityPersister persister = session.getFactory().getEntityPersister( entityName );
-				.getEntityPersister(entity);
+		final CollectionPersister collectionPersister = session.getFactory().getCollectionPersister( collectionRole );
-		final CollectionPersister collectionPersister = session.getFactory()
-				.getCollectionPersister(entity + '.' + property);
 
+		// iterate all the entities currently associated with the persistence context.
 		Iterator entities = entityEntries.entrySet().iterator();
 		while ( entities.hasNext() ) {
-			Map.Entry me = (Map.Entry) entities.next();
+			final Map.Entry me = ( Map.Entry ) entities.next();
-			EntityEntry ee = (EntityEntry) me.getValue();
+			final EntityEntry entityEntry = ( EntityEntry ) me.getValue();
-			if ( persister.isSubclassEntityName( ee.getEntityName() ) ) {
+			// does this entity entry pertain to the entity persister in which we are interested (owner)?
-				Object instance = me.getKey();
+			if ( persister.isSubclassEntityName( entityEntry.getEntityName() ) ) {
+				final Object entityEntryInstance = me.getKey();
 
 				//check if the managed object is the parent
 				boolean found = isFoundInParent(
-						property, 
+						propertyName,
 						childEntity,
 						persister,
 						collectionPersister,
-						instance 
+						entityEntryInstance
 				);
 
 				if ( !found && mergeMap != null ) {
 					//check if the detached object being merged is the parent
-					Object unmergedInstance = mergeMap.get(instance);
+					Object unmergedInstance = mergeMap.get( entityEntryInstance );
 					Object unmergedChild = mergeMap.get( childEntity );
 					if ( unmergedInstance != null && unmergedChild != null ) {
 						found = isFoundInParent(
-								property, 
+								propertyName,
 								unmergedChild,
 								persister,
 								collectionPersister,
@@ -1048,11 +1064,46 @@ public class StatefulPersistenceContext implements PersistenceContext {
 				}
 
 				if ( found ) {
-					return ee.getId();
+					return entityEntry.getId();
 				}
 
 			}
 		}
+
+		// if we get here, it is possible that we have a proxy 'in the way' of the merge map resolution...
+		// 		NOTE: decided to put this here rather than in the above loop as I was nervous about the performance
+		//		of the loop-in-loop especially considering this is far more likely the 'edge case'
+		if ( mergeMap != null ) {
+			Iterator mergeMapItr = mergeMap.entrySet().iterator();
+			while ( mergeMapItr.hasNext() ) {
+				final Map.Entry mergeMapEntry = ( Map.Entry ) mergeMapItr.next();
+				if ( mergeMapEntry.getKey() instanceof HibernateProxy ) {
+					final HibernateProxy proxy = ( HibernateProxy ) mergeMapEntry.getKey();
+					if ( persister.isSubclassEntityName( proxy.getHibernateLazyInitializer().getEntityName() ) ) {
+						boolean found = isFoundInParent(
+								propertyName,
+								childEntity,
+								persister,
+								collectionPersister,
+								mergeMap.get( proxy )
+						);
+						if ( !found ) {
+							found = isFoundInParent(
+									propertyName,
+									mergeMap.get( childEntity ),
+									persister,
+									collectionPersister,
+									mergeMap.get( proxy )
+							);
+						}
+						if ( found ) {
+							return proxy.getHibernateLazyInitializer().getIdentifier();
+						}
+					}
+				}
+			}
+		}
+
 		return null;
 	}
 
@@ -1061,16 +1112,15 @@ public class StatefulPersistenceContext implements PersistenceContext {
 			Object childEntity,
 			EntityPersister persister,
 			CollectionPersister collectionPersister,
-			Object potentialParent
+			Object potentialParent) {
-	) {
 		Object collection = persister.getPropertyValue(
 				potentialParent,
 				property,
 				session.getEntityMode()
 		);
-		return collection!=null && Hibernate.isInitialized(collection) &&
+		return collection != null
-				collectionPersister.getCollectionType()
+				&& Hibernate.isInitialized( collection )
-						.contains(collection, childEntity, session);
+				&& collectionPersister.getCollectionType().contains( collection, childEntity, session );
 	}
 
 	/**

core/src/main/java/org/hibernate/property/BackrefPropertyAccessor.java

@@ -1,30 +1,56 @@
-//$Id: BackrefPropertyAccessor.java 7516 2005-07-16 22:20:48Z oneovthafew $
+/*
+ * Copyright (c) 2007, Red Hat Middleware, LLC. All rights reserved.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, v. 2.1. This program is distributed in the
+ * hope that it will be useful, but WITHOUT A WARRANTY; without even the implied
+ * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details. You should have received a
+ * copy of the GNU Lesser General Public License, v.2.1 along with this
+ * distribution; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * Red Hat Author(s): Gavin King, Steve Ebersole
+ */
 package org.hibernate.property;
 
+import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.util.Map;
-import java.io.Serializable;
 
-import org.hibernate.HibernateException;
-import org.hibernate.engine.SessionImplementor;
 import org.hibernate.engine.SessionFactoryImplementor;
+import org.hibernate.engine.SessionImplementor;
 
 /**
- * Represents a "back-reference" to the id of a collection owner.
+ * Represents a "back-reference" to the id of a collection owner.  A "back-reference" is pertinent in mapping scenarios
+ * where we have a uni-directional one-to-many association in which only the many side is mapped.  In this case it is
+ * the collection itself which is responsible for the FK value.
+ * <p/>
+ * In this scenario, the one side has no inherent knowledge of its "owner".  So we introduce a synthetic property into
+ * the one side to represent the association; a so-called back-reference.
  *
  * @author Gavin King
+ * @author Steve Ebersole
  */
 public class BackrefPropertyAccessor implements PropertyAccessor {
 
 	private final String propertyName;
 	private final String entityName;
 
+	// cache these since they are stateless
+	private final BackrefSetter setter; // this one could even be static...
+	private final BackrefGetter getter;
+
 	/**
 	 * A placeholder for a property value, indicating that
 	 * we don't know the value of the back reference
 	 */
 	public static final Serializable UNKNOWN = new Serializable() {
-		public String toString() { return "<unknown>"; }
+		public String toString() {
+			return "<unknown>";
+		}
+
 		public Object readResolve() {
 			return UNKNOWN;
 		}
@@ -34,34 +60,53 @@ public class BackrefPropertyAccessor implements PropertyAccessor {
 	 * Constructs a new instance of BackrefPropertyAccessor.
 	 *
 	 * @param collectionRole The collection role which this back ref references.
+	 * @param entityName The owner's entity name.
 	 */
 	public BackrefPropertyAccessor(String collectionRole, String entityName) {
 		this.propertyName = collectionRole.substring( entityName.length() + 1 );
 		this.entityName = entityName;
+
+		this.setter = new BackrefSetter();
+		this.getter = new BackrefGetter();
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	public Setter getSetter(Class theClass, String propertyName) {
-		return new BackrefSetter();
+		return setter;
 	}
 
+	/**
+	 * {@inheritDoc}
+	 */
 	public Getter getGetter(Class theClass, String propertyName) {
-		return new BackrefGetter();
+		return getter;
 	}
 
 
 	/**
-	 * The Setter implementation for id backrefs.
+	 * Internal implementation of a property setter specific to these back-ref properties.
 	 */
 	public static final class BackrefSetter implements Setter {
 
+		/**
+		 * {@inheritDoc}
+		 */
 		public Method getMethod() {
 			return null;
 		}
 
+		/**
+		 * {@inheritDoc}
+		 */
 		public String getMethodName() {
 			return null;
 		}
 
+		/**
+		 * {@inheritDoc}
+		 */
 		public void set(Object target, Object value, SessionFactoryImplementor factory) {
 			// this page intentionally left blank :)
 		}
@@ -70,33 +115,46 @@ public class BackrefPropertyAccessor implements PropertyAccessor {
 
 
 	/**
-	 * The Getter implementation for id backrefs.
+	 * Internal implementation of a property getter specific to these back-ref properties.
 	 */
 	public class BackrefGetter implements Getter {
 
-		public Object getForInsert(Object target, Map mergeMap, SessionImplementor session)
+		/**
-		throws HibernateException {
+		 * {@inheritDoc}
+		 */
+		public Object getForInsert(Object target, Map mergeMap, SessionImplementor session) {
 			if ( session == null ) {
 				return UNKNOWN;
 			}
 			else {
-				return session.getPersistenceContext()
+				return session.getPersistenceContext().getOwnerId( entityName, propertyName, target, mergeMap );
-						.getOwnerId( entityName, propertyName, target, mergeMap );
 			}
 		}
 
+		/**
+		 * {@inheritDoc}
+		 */
 		public Object get(Object target) {
 			return UNKNOWN;
 		}
 
+		/**
+		 * {@inheritDoc}
+		 */
 		public Method getMethod() {
 			return null;
 		}
 
+		/**
+		 * {@inheritDoc}
+		 */
 		public String getMethodName() {
 			return null;
 		}
 
+		/**
+		 * {@inheritDoc}
+		 */
 		public Class getReturnType() {
 			return Object.class;
 		}

testsuite/src/test/java/org/hibernate/test/unidir/BackrefTest.java

@@ -1,8 +1,9 @@
-//$Id: BackrefTest.java 10977 2006-12-12 23:28:04Z steve.ebersole@jboss.com $
+//$Id: BackrefTest.java 10976 2006-12-12 23:22:26Z steve.ebersole@jboss.com $
 package org.hibernate.test.unidir;
 
 import junit.framework.Test;
 
+import org.hibernate.Hibernate;
 import org.hibernate.Session;
 import org.hibernate.Transaction;
 import org.hibernate.junit.functional.FunctionalTestCase;
@@ -84,5 +85,37 @@ public class BackrefTest extends FunctionalTestCase {
 		t.commit();
 		s.close();
 	}
+
+	public void testBackRefToProxiedEntityOnMerge() {
+		Session s = openSession();
+		s.beginTransaction();
+		Parent me = new Parent( "Steve" );
+		me.getChildren().add( new Child( "Joe" ) );
+  		s.persist( me );
+		s.getTransaction().commit();
+		s.close();
+
+		// while detached, add a new element
+		me.getChildren().add( new Child( "Cece" ) );
+		me.getChildren().add( new Child( "Austin" ) );
+
+		s = openSession();
+		s.beginTransaction();
+		// load 'me' to associate it with the new session as a proxy (this may have occurred as 'prior work'
+		// to the reattachment below)...
+		Object meProxy = s.load( Parent.class, me.getName() );
+		assertFalse( Hibernate.isInitialized( meProxy ) );
+		// now, do the reattchment...
+		s.merge( me );
+		s.getTransaction().commit();
+		s.close();
+
+		s = openSession();
+		s.beginTransaction();
+		s.createQuery( "delete from Child" ).executeUpdate();
+		s.createQuery( "delete from Parent" ).executeUpdate();
+		s.getTransaction().commit();
+		s.close();
+	}
 }
 

testsuite/src/test/java/org/hibernate/test/unidir/Child.java

@@ -8,19 +8,31 @@ package org.hibernate.test.unidir;
 public class Child {
 	private String name;
 	private int age;
-	Child() {}
+
-	public Child(String name) {
+	Child() {
-		this.name = name;
 	}
+
+	public Child(String name) {
+		this( name, 0 );
+	}
+
+	public Child(String name, int age) {
+		this.name = name;
+		this.age = age;
+	}
+
 	public String getName() {
 		return name;
 	}
+
 	public void setName(String name) {
 		this.name = name;
 	}
+
 	public int getAge() {
 		return age;
 	}
+
 	public void setAge(int age) {
 		this.age = age;
 	}