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
_______________________________________________
eclipselink-users mailing list
eclipselink-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-users
