Author: steve.ebersole(a)jboss.com Date: 2007-10-17 01:08:57 -0400 (Wed, 17 Oct 2007) New Revision: 14092 Modified: core/branches/Branch_3_2/src/org/hibernate/engine/StatefulPersistenceContext.java core/branches/Branch_3_2/src/org/hibernate/property/BackrefPropertyAccessor.java core/branches/Branch_3_2/test/org/hibernate/test/unidir/BackrefTest.java core/branches/Branch_3_2/test/org/hibernate/test/unidir/Child.java Log: HHH-2864 : merge non-inverse collection when 'owner' is already proxied Modified: core/branches/Branch_3_2/src/org/hibernate/engine/StatefulPersistenceContext.java =================================================================== --- core/branches/Branch_3_2/src/org/hibernate/engine/StatefulPersistenceContext.java 2007-10-17 05:07:24 UTC (rev 14091) +++ core/branches/Branch_3_2/src/org/hibernate/engine/StatefulPersistenceContext.java 2007-10-17 05:08:57 UTC (rev 14092) @@ -1005,72 +1005,122 @@ } /** - * Search the persistence context for an owner for the child object, - * given a collection role. If <tt>mergeMap</tt> is non-null, also - * check the detached graph being merged for a parent. + * Search <tt>this</tt> persistence context for an associated entity instance which is considered the "owner" of + * the given <tt>childEntity</tt>, and return that owner's id value. This is performed in the scenario of a + * 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) { - - EntityPersister persister = session.getFactory() - .getEntityPersister(entity); - final CollectionPersister collectionPersister = session.getFactory() - .getCollectionPersister(entity + '.' + property); - + public Serializable getOwnerId(String entityName, String propertyName, Object childEntity, Map mergeMap) { + final String collectionRole = entityName + '.' + propertyName; + final EntityPersister persister = session.getFactory().getEntityPersister( entityName ); + final CollectionPersister collectionPersister = session.getFactory().getCollectionPersister( collectionRole ); + + // 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(); - EntityEntry ee = (EntityEntry) me.getValue(); - if ( persister.isSubclassEntityName( ee.getEntityName() ) ) { - Object instance = me.getKey(); + final Map.Entry me = ( Map.Entry ) entities.next(); + final EntityEntry entityEntry = ( EntityEntry ) me.getValue(); + // does this entity entry pertain to the entity persister in which we are interested (owner)? + if ( persister.isSubclassEntityName( entityEntry.getEntityName() ) ) { + final Object entityEntryInstance = me.getKey(); //check if the managed object is the parent - boolean found = isFoundInParent( - property, - childEntity, - persister, + boolean found = isFoundInParent( + propertyName, + childEntity, + persister, collectionPersister, - instance - ); + entityEntryInstance + ); - if (!found && mergeMap!=null) { + if ( !found && mergeMap != null ) { //check if the detached object being merged is the parent - Object unmergedInstance = mergeMap.get(instance); - Object unmergedChild = mergeMap.get(childEntity); - if ( unmergedInstance!=null && unmergedChild!=null ) { - found = isFoundInParent( - property, - unmergedChild, - persister, + Object unmergedInstance = mergeMap.get( entityEntryInstance ); + Object unmergedChild = mergeMap.get( childEntity ); + if ( unmergedInstance != null && unmergedChild != null ) { + found = isFoundInParent( + propertyName, + unmergedChild, + persister, collectionPersister, - unmergedInstance - ); + unmergedInstance + ); } } - + 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; } private boolean isFoundInParent( - String property, - Object childEntity, - EntityPersister persister, + String property, + Object childEntity, + EntityPersister persister, CollectionPersister collectionPersister, - Object potentialParent - ) { - Object collection = persister.getPropertyValue( - potentialParent, - property, - session.getEntityMode() - ); - return collection!=null && Hibernate.isInitialized(collection) && - collectionPersister.getCollectionType() - .contains(collection, childEntity, session); + Object potentialParent) { + Object collection = persister.getPropertyValue( + potentialParent, + property, + session.getEntityMode() + ); + return collection != null + && Hibernate.isInitialized( collection ) + && collectionPersister.getCollectionType().contains( collection, childEntity, session ); } /** Modified: core/branches/Branch_3_2/src/org/hibernate/property/BackrefPropertyAccessor.java =================================================================== --- core/branches/Branch_3_2/src/org/hibernate/property/BackrefPropertyAccessor.java 2007-10-17 05:07:24 UTC (rev 14091) +++ core/branches/Branch_3_2/src/org/hibernate/property/BackrefPropertyAccessor.java 2007-10-17 05:08:57 UTC (rev 14092) @@ -1,67 +1,112 @@ -//$Id$ +/* + * 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.SessionFactoryImplementor; import org.hibernate.engine.SessionImplementor; -import org.hibernate.engine.SessionFactoryImplementor; /** - * 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; } }; - + /** * 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 @@ /** - * 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 { - if (session==null) { + + /** + * {@inheritDoc} + */ + public Object getForInsert(Object target, Map mergeMap, SessionImplementor session) { + if ( session == null ) { return UNKNOWN; } else { - return session.getPersistenceContext() - .getOwnerId( entityName, propertyName, target, mergeMap ); + return session.getPersistenceContext().getOwnerId( entityName, propertyName, target, mergeMap ); } } - public Object get(Object target) { + /** + * {@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; } Modified: core/branches/Branch_3_2/test/org/hibernate/test/unidir/BackrefTest.java =================================================================== --- core/branches/Branch_3_2/test/org/hibernate/test/unidir/BackrefTest.java 2007-10-17 05:07:24 UTC (rev 14091) +++ core/branches/Branch_3_2/test/org/hibernate/test/unidir/BackrefTest.java 2007-10-17 05:08:57 UTC (rev 14092) @@ -3,6 +3,7 @@ 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 @@ 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(); + } } Modified: core/branches/Branch_3_2/test/org/hibernate/test/unidir/Child.java =================================================================== --- core/branches/Branch_3_2/test/org/hibernate/test/unidir/Child.java 2007-10-17 05:07:24 UTC (rev 14091) +++ core/branches/Branch_3_2/test/org/hibernate/test/unidir/Child.java 2007-10-17 05:08:57 UTC (rev 14092) @@ -8,19 +8,31 @@ public class Child { private String name; private int age; - Child() {} + + Child() { + } + 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; }