Hi Tom,
Tom Ware wrote:
> Hi Markus,
>
> Comments inline
>
> Markus Fuchs wrote:
>
>> Hi Tom!
>>
>> [Including the persistence alias in the discussion]
>>
>> I hope you had a great vacation!
>>
>> While investigating issue 879 I noticed, that the class
>> OneToManyMapping doesn't have to implement method
>> hasInverseConstraintDependency().
>>
>> The dependencies corresponding to a OneToMany/ManyToOne relationship
>> are always added by the owning side, which is represented by the class
>> OneToOneMapping in TopLink, correct?
>>
>> So, the dependencies are added twice, once from the owning, and once
>> from the non-owning side. It should be possible to remove this method,
>> letting OneToManyMapping fall back to the default implementation
>> returning 'false'. Would you agree?
>>
>>
> If we make this change, how will the commit order calculation logic
> work with we are processing a OneToMany? With the code as it is, it
> looks to me like if we have OneToManyMapping return false from this
> method, in recordMappingDependencies() we will get end up adding a
> dependancy that is in the incorrect order (because we will get into
> the "if" rather than the "else")
>
I understand the handling of the two cases in recordMappingDependencies()
as follows:
(1) (ForeignReferenceMapping)mapping).hasConstraintDependency() == true
This is for the owning side. The referenced class is the owned class
and is added with all its subclasses to the owner's list relatedNodes.
(2) ((ForeignReferenceMapping)mapping).hasInverseConstraintDependency()
== true
This is for the non-owning side. The current class is the owned class and
is added with all its subclasses to the owner's list of relatedNodes.
recordMappingDependencies() uses an exclusive if ("if" ... "else if")
construct. So if the two methods hasConstraintDependency() and
hasInverseConstraintDependency() are implemented consistently for each
mapping, there is no problem of adding "reversed" dependencies.
The implementation of hasInverseConstraintDependency() for
OneToManyMapping is *correct*, and shouldn't be changed. Rather, one
could consider changing the implementation of
recordMappingDependencies() to evaluate only the owning side (case 1)
for calculating the dependencies. But it's not a big deal anyway. I
withdraw my proposal to change OneToManyMapping.
>> Could you please also comment on my evaluation of issue 879? The
>> change in class ManyToManyMapping could be the fix for this issue.
>>
>>
>
> I'll add comments to the bug shortly.
>
>>
>> For issue 699, I changed UnitOfWorkImpl to remove the ObjectChangeSets
>> for all objects that are to be deleted before updates/inserts are
>> processed in any case. I got the impression that the updates for
>> deleted objects are executed in order to prevent foreign key (unique
>> key) violations. Is this correct?
>>
>> There's no database constraint forcing to nullify foreign keys for for
>> OneToMany relationships, before other updates happen. The same is true
>> for join table entires in case of ManyToMany relationships, which
>> don't need to be removed. The only case where nulling out the foreign
>> key is important is for OneToOne relationships, which might have a
>> unique constraint on the foreign key in the database. But as
>> dependencies within the same class can't be determinated anyway,
>> removing those updates should not make a difference. Please see the
>> attached UnitOfWorkImpl class for my change.
>>
>>
> I am concerned about the change to UnitOfWorkImpl and will need to
> take a closer look at the issue. Here is my concern: It looks like
> the way we are solving this issue is to go through the change sets as
> usual and build a change set as normal and them make a second pass to
> "fix" the change set. Ideally, I think we would build a change set
> that we could use correctly like from the beginning. I'll take a look
> at this issue in some more depth and get back to you.
>
Thanks, I'll wait for your update!
-- markus.
>> I ran entity-persistence-tests w/o regressions with my changes. What
>> do you think?
>>
>> Thanks very much,
>>
>> -- markus.
>>
>>
>> Tom Ware wrote:
>>
>>
>>
>>> Hi Markus,
>>>
>>> I was on vacation, but it looks like you got your answer. Feel
>>> free to contact me with questions.
>>>
>>> -Tom
>>>
>>> Markus Fuchs wrote:
>>>
>>>
>>>
>>>> Hi Tom,
>>>>
>>>> is it ok if I look into this?
>>>>
>>>> Thanks!
>>>>
>>>> -- markus.
>>>>
>>>>
>>>>
>>>
>