Index: src/java/org/datanucleus/ObjectManagerImpl.java =================================================================== --- src/java/org/datanucleus/ObjectManagerImpl.java (revision 9525) +++ src/java/org/datanucleus/ObjectManagerImpl.java (working copy) @@ -1988,24 +1988,13 @@ thePC = findObject(getApiAdapter().getIdForObject(thePC), false, true, null); } - Object detached = ((DetachState)state).getDetachedCopyObject(thePC); - if (detached == null) + StateManager sm = findStateManager(thePC); + if (sm == null) { - StateManager sm = findStateManager(thePC); - if (sm == null) - { - throw new NucleusUserException(LOCALISER.msg("010007", getApiAdapter().getIdForObject(thePC))); - } - - detached = sm.detachCopy(state); - ((DetachState)state).setDetachedCopyObject(detached, sm.getExternalObjectId(sm.getObject())); + throw new NucleusUserException(LOCALISER.msg("010007", getApiAdapter().getIdForObject(thePC))); } - else - { - // What if the fetch-group here implies a different depth through this object ? we need to continue detaching - // TODO Check the detach and proceed further where necessary - } - return detached; + + return sm.detachCopy(state); } finally { @@ -4457,4 +4446,4 @@ { return executionContext; } -} \ No newline at end of file +} Index: src/java/org/datanucleus/jdo/state/JDOStateManagerImpl.java =================================================================== --- src/java/org/datanucleus/jdo/state/JDOStateManagerImpl.java (revision 9525) +++ src/java/org/datanucleus/jdo/state/JDOStateManagerImpl.java (working copy) @@ -81,6 +81,7 @@ import org.datanucleus.sco.UnsetOwners; import org.datanucleus.state.AbstractStateManager; import org.datanucleus.state.ActivityState; +import org.datanucleus.state.DetachState; import org.datanucleus.state.FetchPlanState; import org.datanucleus.state.LifeCycleState; import org.datanucleus.state.ObjectProviderImpl; @@ -3780,7 +3781,29 @@ return referencedPC; } - PersistenceCapable detachedPC = myPC.jdoNewInstance(this); + // Look for an existing detached copy + DetachState detachState = (DetachState) state; + DetachState.Entry existingDetached = detachState.getDetachedCopyEntry(myPC); + + PersistenceCapable detachedPC; + if (existingDetached == null) + { + // No existing detached copy - create new one + detachedPC = myPC.jdoNewInstance(this); + detachState.setDetachedCopyEntry(myPC, detachedPC); + } + else + { + // Found one - if it's sufficient for current FetchPlanState, return it immediately + detachedPC = (PersistenceCapable) existingDetached.getDetachedCopyObject(); + if (existingDetached.checkCurrentState()) + { + return detachedPC; + } + + // Need to process the detached copy using current FetchPlanState + } + referencedPC = detachedPC; // Check if detachable ... if so then we detach a copy, otherwise we return a transient copy @@ -3823,6 +3846,12 @@ smDetachedPC.initialiseForDetached(detachedPC, getExternalObjectId(myPC), getVersion(myPC)); smDetachedPC.referencedPC = myPC; + // If detached copy already existed, take note of fields previously loaded + if (existingDetached != null) + { + smDetachedPC.retrieveDetachState(smDetachedPC); + } + smDetachedPC.replaceFields(getFieldsNumbersToDetach(), new DetachFieldManager(this, getSecondClassMutableFields(), myFP, state, true)); Index: src/java/org/datanucleus/state/DetachState.java =================================================================== --- src/java/org/datanucleus/state/DetachState.java (revision 9525) +++ src/java/org/datanucleus/state/DetachState.java (working copy) @@ -18,7 +18,11 @@ **********************************************************************/ package org.datanucleus.state; +import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; import java.util.Map; import org.datanucleus.api.ApiAdapter; @@ -30,7 +34,7 @@ public class DetachState extends FetchPlanState { /** a map for the current execution of detachCopy with detached objects keyed by the object id **/ - private Map detachedObjectById = new HashMap(); + private Map detachedObjectById = new HashMap(); /** Adapter for the API being used. */ private ApiAdapter api; @@ -49,35 +53,93 @@ * @param detachedPC the Detached persistable object * @param id The id to key against. If this is null, we treat as embedded */ - public void setDetachedCopyObject(Object detachedPC, Object id) + public void setDetachedCopyEntry(Object pc, Object detachedPC) { - if (id == null) - { - // embedded element (NO ids) - detachedObjectById.put(StringUtils.toJVMIDString(detachedPC), detachedPC); - } - else - { - detachedObjectById.put(id, detachedPC); - } + detachedObjectById.put(getKey(pc), new Entry(detachedPC)); } /** - * Set to the current state a detached copy object + * Get any existing detached copy object for the passed in PersistenceCapable * @param pc the PersistenceCapable of the object searched * @return the Detached PC */ - public Object getDetachedCopyObject(Object pc) + public Entry getDetachedCopyEntry(Object pc) { + return detachedObjectById.get(getKey(pc)); + } + + private Object getKey(Object pc) + { Object id = api.getIdForObject(pc); if (id == null) { // embedded element (NO ids) - return detachedObjectById.get(StringUtils.toJVMIDString(pc)); + return StringUtils.toJVMIDString(pc); } else { - return detachedObjectById.get(id); + return id; } } + + public class Entry + { + private Object detachedPC; + private List> detachStates = new LinkedList>(); + + Entry(Object detachedPC) + { + this.detachedPC = detachedPC; + this.detachStates.add(getCurrentState()); + } + + public Object getDetachedCopyObject() + { + return detachedPC; + } + + /** + * Determine whether the current state is "dominated" by any previous + * detach state for this entry, in which case we know that all the required + * fields will already be in the detached copy. + * (Dominance is transitive, so we can remove redundant entries) + * @return true if we can prove the current state is fully detached already + */ + public boolean checkCurrentState() + { + List currentState = getCurrentState(); + + Iterator> iter = detachStates.iterator(); + while (iter.hasNext()) + { + List detachState = iter.next(); + if (dominates(detachState, currentState)) + { + return true; + } + if (dominates(currentState, detachState)) + { + iter.remove(); + } + } + detachStates.add(currentState); + return false; + } + + private List getCurrentState() + { + return new ArrayList(fetchFieldNames); + } + + private boolean dominates(List candidate, List target) + { + if (candidate.size() == 0) + return true; + if (candidate.size() > target.size()) + return false; + String fieldName = target.get(target.size() - 1); + // TODO If fieldName is not a recursive field, then return true; + return calculateObjectDepthForType(candidate, fieldName) <= calculateObjectDepthForType(target, fieldName); + } + } } \ No newline at end of file Index: src/java/org/datanucleus/state/FetchPlanState.java =================================================================== --- src/java/org/datanucleus/state/FetchPlanState.java (revision 9525) +++ src/java/org/datanucleus/state/FetchPlanState.java (working copy) @@ -35,7 +35,7 @@ * List of field names in the graph. The first is the root of the tree, and fields are added as they are encountered * and removed when they are finished with. */ - protected List fetchFieldNames = new ArrayList(); + protected List fetchFieldNames = new ArrayList(); /** * Method to add an field name to the list since it is being processed @@ -70,7 +70,12 @@ */ public int getObjectDepthForType(String fieldName) { - ListIterator iter = fetchFieldNames.listIterator(fetchFieldNames.size()); // Start at the end + return calculateObjectDepthForType(fetchFieldNames, fieldName); + } + + protected static int calculateObjectDepthForType(List fieldNames, String fieldName) + { + ListIterator iter = fieldNames.listIterator(fieldNames.size()); // Start at the end int number = 0; while (iter.hasPrevious()) { Index: src/java/org/datanucleus/store/fieldmanager/DetachFieldManager.java =================================================================== --- src/java/org/datanucleus/store/fieldmanager/DetachFieldManager.java (revision 9525) +++ src/java/org/datanucleus/store/fieldmanager/DetachFieldManager.java (working copy) @@ -269,19 +269,18 @@ if (api.isPersistable(value)) { - Object detached = null; if (copy) { - detached = ((DetachState)state).getDetachedCopyObject(value); + DetachState.Entry entry = ((DetachState)state).getDetachedCopyEntry(value); + if (entry != null) + { + // While we are at the end of a branch and this would go beyond the depth limits, + // the object here *is* already detached so just return it + return entry.getDetachedCopyObject(); + } } - if (detached != null) + else if (sm.getObjectManager().getApiAdapter().isDetached(value)) { - // While we are at the end of a branch and this would go beyond the depth limits, - // the object here *is* already detached so just return it - return detached; - } - if (!copy && sm.getObjectManager().getApiAdapter().isDetached(value)) - { return value; } }