Hello Gordon,
Overriding UoW.mergeCloneWithReferences() causes a code duplication, and
introducing a new merge policy CLONE_INTO_WORKING_COPY_FORCE_CASCADE makes
another complexity to maintain this policy.
Instead of the approach I think introducing a new force-cascade flag is
cleaner solution. Also when I reconsider moving the force-cascade logic into
ObjectBuilder.mergeIntoObject(), it seems not a good idea because
mergeIntoObject() is called by several different places. So I left the logic
in MergeManager.
Please review below.
MergeManager:
/** Force cascade merge even if a clone is already registered */
// GF#1139 Cascade doesn't work when merging managed entity
protected boolean forceCascade;
...
protected Object mergeChangesOfCloneIntoWorkingCopy(Object rmiClone) {
ClassDescriptor descriptor = getSession().getDescriptor(rmiClone);
Object registeredObject =
registerObjectForMergeCloneIntoWorkingCopy(rmiClone);
if (registeredObject == rmiClone && !shouldForceCascade()) {
//need to find better better fix. prevents merging into itself.
return rmiClone;
}
try {
ObjectBuilder builder = descriptor.getObjectBuilder();
if (registeredObject != rmiClone &&
descriptor.usesVersionLocking()) {
VersionLockingPolicy policy = (VersionLockingPolicy)
descriptor.getOptimisticLockingPolicy();
if (policy.isStoredInObject()) {
Object currentValue =
builder.extractValueFromObjectForField(registeredObject,
policy.getWriteLockField(), session);
if (policy.isNewerVersion(currentValue, rmiClone,
session.keyFromObject(rmiClone), session)) {
throw
OptimisticLockException.objectChangedSinceLastMerge(rmiClone);
}
}
}
// Toggle change tracking during the merge.
descriptor.getObjectChangePolicy
().dissableEventProcessing(registeredObject);
if(registeredObject == rmiClone){
// GF#1139 Cascade merge operations even if it's already
registered
Vector mappings = descriptor.getMappings();
for (int index = 0; index < mappings.size(); index++) {
DatabaseMapping mapping =
(DatabaseMapping)mappings.get(index);
if(mapping instanceof ForeignReferenceMapping
&&
((ForeignReferenceMapping)mapping).shouldMergeCascadeParts(this)){
mapping.mergeIntoObject(registeredObject, false,
rmiClone, this);
}
}
} else {
// Merge into the clone from the original, use clone as
backup as anything different should be merged.
builder.mergeIntoObject(registeredObject, false, rmiClone,
this);
}
} finally {
descriptor.getObjectChangePolicy
().enableEventProcessing(registeredObject);
}
return registeredObject;
}
...
To set forceCascade flag, UnitOfWorkImpl and EntityManagerImpl are modified
as follows.
UnitOfWorkImpl:
public Object mergeCloneWithReferences(Object rmiClone, int
cascadePolicy) {
return mergeCloneWithReferences(rmiClone, cascadePolicy, false);
}
...
public Object mergeCloneWithReferences(Object rmiClone, int
cascadePolicy, boolean forceCascade) {
if (rmiClone == null) {
return null;
}
ClassDescriptor descriptor = getDescriptor(rmiClone);
if ((descriptor == null) || descriptor.isAggregateDescriptor() ||
descriptor.isAggregateCollectionDescriptor()) {
if (cascadePolicy == MergeManager.CASCADE_BY_MAPPING){
throw new IllegalArgumentException(
ExceptionLocalization.buildMessage("not_an_entity", new
Object[]{rmiClone}));
}
return rmiClone;
}
//CR#2272
logDebugMessage(rmiClone, "merge_clone_with_references");
ObjectBuilder builder = descriptor.getObjectBuilder();
Object implementation = builder.unwrapObject(rmiClone, this);
MergeManager manager = new MergeManager(this);
manager.mergeCloneWithReferencesIntoWorkingCopy();
manager.setCascadePolicy(cascadePolicy);
manager.setForceCascade(forceCascade);
Object mergedObject = manager.mergeChanges(implementation, null);
if (isSmartMerge()) {
return builder.wrapObject(mergedObject, this);
} else {
return mergedObject;
}
}
EntityManagerImpl:
protected Object mergeInternal(Object entity){
verifyOpen();
if (entity == null){
throw new IllegalArgumentException(
ExceptionLocalization.buildMessage("not_an_entity", new Object[] {entity}));
}
//gf830 - merging a removed entity should throw exception
if
(getActivePersistenceContext(checkForTransaction(!isExtended())).getDeletedObjects().contains(entity)){
throw new IllegalArgumentException(
ExceptionLocalization.buildMessage("cannot_merge_removed_entity", new
Object[]{entity}));
}
try {
return
getActivePersistenceContext(checkForTransaction(!isExtended())).mergeCloneWithReferences(entity,
MergeManager.CASCADE_BY_MAPPING, true);
} catch (
oracle.toplink.essentials.exceptions.OptimisticLockException ole) {
throw new javax.persistence.OptimisticLockException(ole);
}
}
Thanks
-Wonseok
On 9/26/06, Wonseok Kim <guruwons_at_gmail.com> wrote:
>
> Hello Gordon, thanks for review.
>
> Currently EntityManagerImpl is using UoW.mergeCloneWithReferences()
> instead of mergeClone(). So mergeCloneWithReferences() should be overrided
> as follows. right?
>
> public Object mergeCloneWithReferences(Object rmiClone, int
> cascadePolicy) {
> if (rmiClone == null) {
> return null;
> }
> ClassDescriptor descriptor = getDescriptor(rmiClone);
> if ((descriptor == null) || descriptor.isAggregateDescriptor() ||
> descriptor.isAggregateCollectionDescriptor()) {
> if (cascadePolicy == MergeManager.CASCADE_BY_MAPPING){
> throw new IllegalArgumentException(
> ExceptionLocalization.buildMessage("not_an_entity", new
> Object[]{rmiClone}));
> }
> return rmiClone;
> }
>
> //CR#2272
> // logDebugMessage(rmiClone, "merge_clone_with_references");
>
> ObjectBuilder builder = descriptor.getObjectBuilder();
> Object implementation = builder.unwrapObject(rmiClone, this);
>
> MergeManager manager = new MergeManager(this);
> manager.mergeCloneIntoWorkingCopyForceCascade();
> manager.setCascadePolicy(cascadePolicy);
> Object mergedObject = manager.mergeChanges(implementation, null);
> if (isSmartMerge()) {
> return builder.wrapObject(mergedObject, this);
> } else {
> return mergedObject;
> }
>
> }
>
> I'm curious, Why mergeCloneWithReferences() code is different from
> mergeClone()?
> It doesn't use SessionProfiler and handleException() as you see.
> Also, to override this private logDebugMessage() should have protected
> access.
>
> And the force-cascade logic which was in MergeManager can go to
> ObjectBuilder for avoding a duplicate like below.
>
> public void mergeIntoObject(Object target, boolean isUnInitialized,
> Object source, MergeManager mergeManager) {
> // PERF: Avoid synchronized enumerator as is concurrency
> bottleneck.
> Vector mappings = getDescriptor().getMappings();
> for (int index = 0; index < mappings.size(); index++) {
> DatabaseMapping mapping =
> (DatabaseMapping)mappings.get(index);
> if(target != source
> || (
> mergeManager.shouldMergeCloneIntoWorkingCopyForceCascade()
> && mapping instanceof ForeignReferenceMapping
> &&
> ((ForeignReferenceMapping)mapping).shouldMergeCascadeParts(mergeManager))){
> // GF#1139 Cascade merge operations even if it's already
> managed
> mapping.mergeIntoObject(target, isUnInitialized, source,
> mergeManager);
> }
> }
>
> ...
> }
>
> What do you think?
>
> -Wonseok
>
> On 9/25/06, Gordon Yorke <gordon.yorke_at_oracle.com> wrote:
> >
> > Hello Wonseok,
> > A slight change will have to be made to your code as it is not
> > backward compatible with current TopLink behaviour. Instead of altering the
> > MergeManger to always cascade the merge and duplicating code already found
> > within the ObjectBuilder the RepeatableWriteUnitOfWork (which is specific to
> > EJB3.0) should request the cascaded merge from the MergeManager while
> > the UnitOfWork continues to request the abortive merge.
> >
> > The following method should be added to the RepeatableWriteUnitOfWork :
> > /**
> > * INTERNAL:
> > * Merge the attributes of the clone into the unit of work copy.
> > */
> > public Object mergeClone(Object rmiClone, int cascadeDepth) {
> > if (rmiClone == null) {
> > return null;
> > }
> >
> > //CR#2272
> > logDebugMessage(rmiClone, "merge_clone");
> >
> > startOperationProfile(SessionProfiler.Merge);
> > ObjectBuilder builder =
> > getDescriptor(rmiClone).getObjectBuilder();
> > Object implementation = builder.unwrapObject(rmiClone, this);
> >
> > MergeManager manager = new MergeManager(this);
> > manager.mergeCloneIntoWorkingCopyForceCascade();
> > manager.setCascadePolicy(cascadeDepth);
> >
> > Object merged = null;
> > try {
> > merged = manager.mergeChanges(implementation, null);
> > } catch (RuntimeException exception) {
> > merged = handleException(exception);
> > }
> > endOperationProfile(SessionProfiler.Merge);
> >
> > return merged;
> > }
> > Then the MergeManager should have the following method and corresponding
> > property added:
> > /**
> > * This can be used by the user for merging clones from RMI into the
> > unit of work.
> > */
> > public void mergeCloneIntoWorkingCopyForceCascade() {
> > setMergePolicy(CLONE_INTO_WORKING_COPY_FORCE_CASCADE);
> > }
> >
> > Then update the MergeManager to check for this setting in two places:
> > mergeClone():
> > } else if (shouldMergeCloneIntoWorkingCopy() ||
> > shouldMergeCloneWithReferencesIntoWorkingCopy() ||
> > shouldMergeCloneIntoWorkingCopyForceCascade()) {
> > mergedObject = mergeChangesOfCloneIntoWorkingCopy(object);
> > and mergeChangesOfCloneIntoWorkingCopy() :
> > if (registeredObject== rmiClone && !
> > shouldMergeCloneIntoWorkingCopyForceCascade() ){
> > return rmiClone;
> > }
> >
> >
> > These changes preserve existing TopLink behaviour while updating the
> > functionality for EJB 3.0 support and the code continues to use the
> > current TopLink patterns for method call sequence.
> > --Gordon
> >
>