Hello Markus,
Good catch on the updateWriteLockValueForWrite method setting of the
initialWriteLockValue should only be done for an insert moving the
update into the setupWriteFieldsForInsert method would be better. If
setInitialWriteLockValue does not exist then it should be created. It
should be a simple mutator . I do not believe there is current support
for detecting if a version value has been set to null after it has been
created. It will need to be implemented and should be done as part of
this bug.
-- Gordon
Markus Fuchs wrote:
> Hi Gordon,
>
> This fixes my test case. Great!
>
> The new method
> setInitialWriteLockValue(lockValue)
> would be implemented in ObjectChangeSet, correct?
>
> This solution sets the initialWriteLockValue every time the version is
> increased. I'm not sure if that's an expected behavior, the initial
> value should be set only once. But that also depends on the
> implementation of setInitialWriteLockValue(lockValue) and should be
> documented in the JavaDoc. Could you send me that implementation?
>
> I agree, we absolutely need to detect if a user updates an object to
> have a null version. Is this implemented?
>
> If you still want the method
> public int compareWriteLockValues(Object value1, Object value2);
>
> to throw an exception if a null value is passed, we might want to
> enhance this behaviour to throw an application error rather than a
> NullPointerException.
>
> Thanks,
>
> -- markus.
>
>
> Gordon Yorke wrote:
>> Hello Markus,
>> With new objects this problem can be easily solved by adding the following line to VersionLockingPolicy.updateWriteLockValueForWrite(...)
>> if (objectChangeSet != null) {
>> objectChangeSet.setWriteLockValue(lockValue);
>> objectChangeSet.setInitialWriteLockValue(lockValue); // <-- *** new line.
>> }
>>
>> If the user updates an object to have a null version value we should probably detect this and throw an exception that notifies the user of the error they have made.
>> What do you think,
>> --Gordon
>>
>> -----Original Message-----
>> From: Markus.Fuchs_at_Sun.COM [mailto:Markus.Fuchs_at_Sun.COM]On Behalf Of
>> Markus Fuchs
>> Sent: Thursday, May 11, 2006 2:15 PM
>> To: Gordon Yorke
>> Cc: persistence_at_glassfish.com
>> Subject: Re: [Fwd: [Issue 635] Toplink's VersionLockingPolicy became to
>> throw a NullPointerException]
>>
>>
>> Hi Gordon,
>>
>> Gordon Yorke wrote:
>>
>>
>>> Hello Markus,
>>> Can you explain how initialWriteLockValue is null? As the object is inserted into the database TopLink should set an initial value into the object and the ChangeSet. Is an instance value being inserted into the database?
>>>
>>>
>> I attached my testbase to the bug report. To run it, please unjar
>> issue635.jar under entity-persistence-tests. Then run the newly created
>>
>> oracle.toplink.essentials.testing.tests.cmp3.advanced.EntityMappingsAdvancedJUnitTestCase
>>
>>
>> Junit test. The situation is:
>>
>> An Employee is persisted, and in the same transaction two previously
>> created Projects are added to the Employee. The NullPointerException
>> happens at the second commit.
>>
>>
>>> Also the method you mention "setOptimisticLockingPolicyAndInitialWriteLockValue" that initializes the value does not exist in TopLink?
>>>
>>>
>>>
>> I was referring to
>>
>> public void
>> setOptimisticLockingPolicyAndInitialWriteLockValue(OptimisticLockingPolicy
>> optimisticLockingPolicy, AbstractSession session) in ObjectChangeSet
>>
>> Thanks,
>>
>> -- markus.
>>
>>
>>> --Gordon
>>>
>>> -----Original Message-----
>>> From: Markus.Fuchs_at_Sun.COM [mailto:Markus.Fuchs_at_Sun.COM]On Behalf Of
>>> Markus Fuchs
>>> Sent: Wednesday, May 10, 2006 10:39 PM
>>> To: ejb3-toplink-ext_at_sun.com
>>> Subject: [Fwd: [Issue 635] Toplink's VersionLockingPolicy became to
>>> throw a NullPointerException]
>>>
>>>
>>> Hi TopLink team,
>>>
>>> while investigating issue 635, I noticed that the assumption given in
>>> ObjectChangeSet (in mergeObjectChanges, line 540 of the FCS version), is
>>> not always true:
>>>
>>> // optimisticLockingPolicy != null guarantees initialWriteLockValue !=
>>> null
>>>
>>> If the instance has an Object type version field (the spec allows
>>> Integer, Short, Long, and Timestamp) following can be true:
>>>
>>> // optimisticLockingPolicy != null && initialWriteLockValue == null
>>>
>>> which leads to the NullPointerException described in issue 635.
>>>
>>> The member "initialWriteLockValue" can be null for an Object typed
>>> version field, as it may be initialized by getting the version field's
>>> value in
>>>
>>> public void
>>> setOptimisticLockingPolicyAndInitialWriteLockValue(OptimisticLockingPolicy
>>> optimisticLockingPolicy, AbstractSession session)
>>>
>>> in class ObjectChangeSet.
>>>
>>> ===================================================
>>>
>>> I see following the alternative fixes:
>>>
>>> 1. Let the two implementations of method
>>>
>>> public int compareWriteLockValues(Object value1, Object value2);
>>>
>>> defined in the interface OptimisticLockingPolicy handle null values
>>> correctly. The two implementations are in VersionLockingPolicy and
>>> TimestampLockingPolicy. This seems the preferred solution as long as
>>> there are no other problems associated with null values in version fields.
>>>
>>> 2. Add a method to OptimisticLockingPolicy
>>>
>>> public Object getInitialWriteLockValue(Object domainObject,
>>> java.util.Vector primaryKey, AbstractSession session);
>>>
>>> which makes sure the return value is never null. This method would only
>>> be called to initialize the variable ObjectChangeSet#initialWriteLockValue
>>>
>>> I couldn't think of an appropriate default for Timestamp version fields,
>>> maybe new Timestamp(0)? There might be other problems with
>>> "initialWriteLockValue" *not* being null in some cases.
>>>
>>> What do you think? I would be happy to implement one of the above
>>> proposals if the TopLink team agrees.
>>>
>>> Thanks!
>>>
>>> -- markus.
>>>
>>>
>>>
>>
>>