[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
RE: [eclipselink-users] Application DeveloperResponsibilitieswhenoverridingjava.lang.Object.clone()in EclipseLink JPA mapped classes?
|
Morning Tom,
Thanks. I was wondering about that as well due to the .zip attachment.
Ok, I did just enter a defect against EclipseLink 1.0.1 for this issue,
defect 255990.
Thank you for the help,
Doug
-----Original Message-----
From: eclipselink-users-bounces@xxxxxxxxxxx
[mailto:eclipselink-users-bounces@xxxxxxxxxxx] On Behalf Of Tom Ware
Sent: Thursday, November 20, 2008 5:58 AM
To: EclipseLink User Discussions
Subject: Re: [eclipselink-users] Application
DeveloperResponsibilitieswhenoverridingjava.lang.Object.clone()in
EclipseLink JPA mapped classes?
Hi Doug,
I am glad you pinged me about this issue. For some reason, I never
received
the previous email. I suspect it has something to do with how our mail
system
here deals with zip files.
I think the best way to proceed is to enter a bug and attach your
test case.
I will recommend to the team that it be addressed for our 1.1 release.
BTW: if you want to send an archive in the future, send it as something
other
than a zip, a jar for instance.
-Tom
Gschwind, Doug wrote:
> 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
_______________________________________________
eclipselink-users mailing list
eclipselink-users@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-users
