Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 319426 - Original and copy share ValueHolder
Summary: Original and copy share ValueHolder
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Eclipselink (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---   Edit
Assignee: Andrei Ilitchev CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-09 14:58 EDT by Andrei Ilitchev CLA
Modified: 2022-06-09 10:08 EDT (History)
1 user (show)

See Also:


Attachments
suggested patch (2.72 KB, patch)
2010-07-13 11:05 EDT, Andrei Ilitchev CLA
no flags Details | Diff
updated patch (4.46 KB, patch)
2010-07-14 11:34 EDT, Andrei Ilitchev CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Ilitchev CLA 2010-07-09 14:58:38 EDT
To reproduce: 
using fieldaccess.advanced model read an Employee that has an address;
then copy it with CASCADE_ALL_PARTS cascade depth:
  CopyGroup group = new CopyGroup();
  group.cascadeAllParts();
  empCopy = (Employee)em.unwrap(JpaEntityManager.class).copy(emp, group);
Result: both emp and empCopy have the same Address

The original and the copy share the ValueHolder, therefore when the copy of the Address is assigned to copy Employee it also lands into the original.

The culprit is PersistenceEntityCopyPolicy.buldClone method:
  public Object buildClone(Object object, Session session) throws DescriptorException {
      return ((PersistenceObject)object)._persistence_shallow_clone();
  }

_persistence_shallow_clone is a new method added by weaving:
  public Object _persistence_shallow_clone()
  {
      return super.clone();
  }
that's why value holders are shared by source and copy.

Note that a similar problem is succefully resolved in a woven clone method, here's the woven code for org.eclipse.persistence.testing.models.jpa.fieldaccess.advanced.Employee:

    public Employee clone()
    {
        try
        {
            return (Employee)((Employee)super.clone())._persistence_post_clone();
        }
        catch(CloneNotSupportedException exception)
        {
            throw new InternalError(exception.toString());
        }
    }

where:

    public Object _persistence_post_clone()
    {
        if(_persistence_manager_vh != null)
            _persistence_manager_vh = (WeavedAttributeValueHolderInterface)_persistence_manager_vh.clone();
        if(_persistence_address_vh != null)
            _persistence_address_vh = (WeavedAttributeValueHolderInterface)_persistence_address_vh.clone();
        _persistence_listener = null;
        _persistence_fetchGroup = null;
        _persistence_session = null;
        _persistence_primaryKey = null;
        return this;
    }

Suggested solution - add a similar method to _persistence_shallow_clone:
  public Object _persistence_shallow_clone()
  {
      return super.clone()._persistence_post_super_clone();
  }

    public Object _persistence_post_super_clone()
    {
        _persistence_manager_vh = null;
        _persistence_address_vh = null;
        _persistence_listener = null;
        _persistence_fetchGroup = null;
        _persistence_session = null;
        _persistence_primaryKey = null;
        return this;
    }

To test uncomment copyCascadeAllParts in org.eclipse.persistence.testing.tests.jpa.fetchgroups.SimpleSerializeFetchGroupTests: 
currently it fails with:
junit.framework.AssertionFailedError: address has not been copied.

After this bug is fixed the test should pass.

Note that the same problem also affects PersistenceEntityCopyPolicy.buildWorkingCopyClone method.
Comment 1 Andrei Ilitchev CLA 2010-07-13 11:05:12 EDT
Created attachment 174160 [details]
suggested patch

The shallow copy is working properly, the responsibility to clear the attributes woven into the class is on ObjectBuilder (instantiateClone method), to set a new ValueHolder - is on the corresponding mapping (ObjectReferenceMapping.getCopy).
Comment 2 Andrei Ilitchev CLA 2010-07-14 11:34:53 EDT
Created attachment 174305 [details]
updated patch

Added calling reset on indirection policy for CollectionMapping and DirectCollectionMapping.
Comment 3 Andrei Ilitchev CLA 2010-07-14 11:52:44 EDT
The patch checked into trunk, 2.1.1 is pending.
Comment 4 Andrei Ilitchev CLA 2010-07-15 12:17:51 EDT
Backported to 2.1.1.
Comment 5 Eclipse Webmaster CLA 2022-06-09 10:08:07 EDT
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink