Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
RE: [eclipselink-users] Application Developer Responsibilitieswhenoverridingjava.lang.Object.clone() in EclipseLink JPA mapped classes?

Hi Tom,

I am sure you EclipseLink developers always have many competing
priorities, but am wondering when you or someone else on the team will
have a chance to review this problem I am seeing?

I also wanted to confirm that you had in fact received the .zip file
attachment I sent on 14 November.

Thanks,

Doug

-----Original Message-----
From: Gschwind, Doug 
Sent: Friday, November 14, 2008 11:11 AM
To: 'EclipseLink User Discussions'
Subject: RE: [eclipselink-users] Application Developer
Responsibilitieswhenoverridingjava.lang.Object.clone() in EclipseLink
JPA mapped classes?

Hi Tom,

Thanks for your response. I was optimistic that your suggestion would in
fact resolve the issue, but alas, that was not the case, the error
persists even after using clonedEmployee.setDepartment(null); within the
clone() implementation in Employee.java.

I am now wondering if there is in fact a defect in EclipseLink 1.0.1 in
this area. Since we are using field based access due to the placement of
annotations on the fields themselves, I would think
clonedEmployee.department = null or clonedEmployee.setDepartment(null)
would work, but neither do.

I have attached a .zip file that contains a JUnit test that demonstrates
this problem. The source files included mimic the class hierarchy where
we saw the original problem in our code base. The class names were
changed to Department and Employee to pick a simple real life example.
Upon extraction of the .zip file, you will find a README.txt file that
will provide the remainder of instruction necessary to reproduce my test
results. You could just review the clone() implementations in Department
and Employee respectively to start if you like, they are quite simple.

I am happy to create a defect against EclipseLink 1.0.1, but was
deferring that exercise until I was certain it wasn't a faulty
implementation in one or more of our overriding clone() methods.

Please take a look when you have a chance and let me know how I should
proceed.

Thanks again,

Doug

-----Original Message-----
From: eclipselink-users-bounces@xxxxxxxxxxx
[mailto:eclipselink-users-bounces@xxxxxxxxxxx] On Behalf Of Tom Ware
Sent: Friday, November 14, 2008 6:24 AM
To: EclipseLink User Discussions
Subject: Re: [eclipselink-users] Application Developer
Responsibilitieswhenoverridingjava.lang.Object.clone() in EclipseLink
JPA mapped classes?

Hi Doug,

   The reason you are seeing an issue is fairly subtle.  Your clone
methods do 
something that is not recommended by the JPA specification.  They
directly 
access instance variables of an instance of an object from outside of
that 
object.  Here is what I mean:

   In this line:

clonedEmployee.department = (Department) this.department.clone();

   You are in an instance of "this", but you are directly accessing an
instance 
variable of "clonedEmployee".  We cannot weave this code correctly.  I
suggest 
changing to this:

clonedEmployee.setDepartment((Department) this.department.clone());

   With this change, the weaved code in the setDepartment() method will
take 
care of the valueholders and the instance variable access will be within

clonedEmployee through it's setDepartment() method.

-Tom

Gschwind, Doug wrote:
> Hi Tom,
> 
> I have some further refinements on topic to discuss. My hypothesis is
> that one of our clone() methods does not provide the opportunity for
the
> EclipseLink weaving to execute, and thus yields undesired results. So,
I
> am hoping you can review the following and let me know if you think
this
> clone() method is incorrect, or if there is some possible defect in
> EclipseLink (we are using 1.0.1).
> 
> Suppose we have a OneToMany from Department to Employee, and a back
> pointing ManyToOne from Employee to Department, and EclipseLink
weaving
> is enabled for both of these JPA mapped classes. Lets also assume that
> both Department and Employee extend Object (but in my test case they
are
> a level or two subclassed from Object). Consider the following
> overriding clone() methods on Department and Employee:
> 
> Department.java
> ---------------
> @OneToMany
> private List<Employee> employees;
> 
> @Override
> public Object clone()
> {
>     Department clonedDepartment = (Department) super.clone();
> 
>     clonedDepartment.employees = new ArrayList<Employee>();
> 
>     for (Employee sourceEmployee : this.employees)
>     {
>         Employee clonedEmployee = (Employee) sourceEmployee.clone();
>         clonedDepartment.addEmployee(clonedEmployee);
>         clonedEmployee.setDepartment(result);
>     }
> 
>     return clonedDepartment;
> }
> 
> Employee.java
> -------------
> @ManyToOne
> private Department department;
> 
> @Override
> public Object clone()
> {
>     Employee clonedEmployee = (Employee) super.clone();
> 
>     // Cannot include the following line due to infinite recursion
>     // but it seems we need a way to allow the underlying ValueHolder
>     // that supports the department instance to be cloned. How to do
>     // that without calling clone() against the Department instance?
>     //clonedEmployee.department = (Department)
this.department.clone();
> 
>     return clonedEmployee;
> }
> 
> When a Department (D) with three Employees (E1,E2,E3) is cloned, the
> cloned Department (D') instance has three Employee clone instances
> (E1',E2',E3'), but the original three Employee instances all now point
> to the Department clone (D') instead of the original Department
instance
> (D). The original Department instance should be treated as read only.
> 
> This would make sense if the ValueHolder for the Department in each
> Employee instance is in fact the same ValueHolder reference due to the
> shallow copy results of super.clone() in Employee.clone(). Changing
the
> department reference in one (e.g. E1, E1') would change the reference
> for both.
> 
> Clearly if the Employee clone() method were implemented to construct a
> new Employee instance via an Employee constructor for instance, this
> problem would not be possible due to the different ValueHolder
instances
> held in the newly constructed Employee instance. We have some
investment
> in the clone() framework though, so we are attempting to not disturb
> that if possible.
> 
> Let me know your thoughts. It feels to me that the ValueHolder on the
> Employee class for the Department needs to be copied (but not its
> contents as we will be resetting the Department reference immediately
> after Employee.clone() returns control to Department.clone()), but not
> sure how to get there.
> 
> Thanks,
> 
> Doug
> 
> -----Original Message-----
> From: eclipselink-users-bounces@xxxxxxxxxxx
> [mailto:eclipselink-users-bounces@xxxxxxxxxxx] On Behalf Of Tom Ware
> Sent: Thursday, November 13, 2008 10:57 AM
> To: EclipseLink User Discussions
> Subject: Re: [eclipselink-users] Application Developer
Responsibilities
> whenoverriding java.lang.Object.clone() in EclipseLink JPA mapped
> classes?
> 
> Hi Doug,
> 
>    As long as the clone methods on the targets of your relationships
are
> 
> implemented correctly, you should be fine with cloning the way you
> suggest.
> 
>    When weaving is enabled, EclipseLink does weave in a 
> _persistence_post_clone() method.  That code is added after the call
to 
> super.clone() and does some clean-up of the weaved code.
> 
> Are you seeing problems?
> 
> -Tom
> 
> Gschwind, Doug wrote:
>> Hello everyone,
>>
>>  
>>
>> I didn't find any information on the wiki on this topic, and that may

>> simply be due to no change in the contract regarding how overriding 
>> implementations of clone() should be implemented in EclipseLink JPA 
>> mapped classes. However, I am looking for confirmation of that.
>>
>>  
>>
>> Suppose I have a JPA mapped class, with relationships to other JPA 
>> mapped classes as follows:
>>
>>  
>>
>> @Entity
>>
>> public class Container extends Object
>>
>> {
>>
>>     @OneToOne
>>
>>     private A a;
>>
>>  
>>
>>     @ManyToOne
>>
>>     private B b;
>>
>>  
>>
>>     @OneToMany
>>
>>     private List<C> cs;
>>
>>  
>>
>>     @ManyToMany
>>
>>     private List<D> ds;
>>
>> }
>>
>>  
>>
>> Now suppose I wish to create instances of Container by cloning 
>> persistent instances. What I am looking for are any differences in
the
> 
>> overriding clone() implementation if the Container class were defined
> as 
>> above, or if the Container class was not JPA mapped and had none of
> the 
>> above annotations. Would the following method suffice within the JPA 
>> mapped Container class, assuming the overriding implementation of 
>> clone() on the JPA mapped A, B, C, and D classes were equally
> appropriate:
>>  
>>
>> @Override
>>
>> public Object clone()
>>
>> {
>>
>>     Container result = (Container) super.clone();
>>
>>  
>>
>>     result.a = (A) this.a.clone();
>>
>>     result.b = (B) this.b.clone();
>>
>>  
>>
>>     result.cs = new ArrayList<C>(this.cs.size());
>>
>>     for (C sourceC : this.cs)
>>
>>     {
>>
>>         C cloneC = (C) sourceC.clone();
>>
>>         result.cs.add(cloneC);
>>
>>         cloneC.setContainer(result);
>>
>>     }
>>
>>  
>>
>>     result.ds = new ArrayList<D>(this.ds.size());
>>
>>     for (D sourceD : this.ds)
>>
>>     {
>>
>>         D cloneD = (D) sourceD.clone();
>>
>>         result.ds.add(cloneD);
>>
>>         cloneD.setContainer(result);
>>
>>     }
>>
>> }
>>
>>  
>>
>> If not, what differences in the above implementation would be 
>> recommended for use in the JPA mapped Container class?
>>
>>  
>>
>> Thanks,
>>
>>  
>>
>> Doug
>>
>>  
>>
>>
>>
>>
>>
>>
>> The contents of this electronic mail message and any attachments are
> confidential, possibly privileged and intended
>> for the addressee(s) only. Only the addressee(s) may read,
> disseminate, retain or otherwise use this message. If
>> received in error, please immediately inform the sender and then
> delete this message without disclosing its contents
>> to anyone.
>>
>>
>>
>
------------------------------------------------------------------------
>> _______________________________________________
>> eclipselink-users mailing list
>> eclipselink-users@xxxxxxxxxxx
>> https://dev.eclipse.org/mailman/listinfo/eclipselink-users
> _______________________________________________
> eclipselink-users mailing list
> eclipselink-users@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/eclipselink-users
> _______________________________________________
> eclipselink-users mailing list
> eclipselink-users@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/eclipselink-users
_______________________________________________
eclipselink-users mailing list
eclipselink-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-users


Back to the top