StateTransitions incorrectly asserts that field access in a deleted instance will throw an exception

• Key: JDO-419
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Minor
• Assignee: Craig Russell
• Reporter: Marc Prud'hommeaux
• Component/s: tck2
• Affects Version/s: JDO 2 final
• Fix Version/s: JDO 2 maintenance release 1, JDO 2 TCK challenge fixes

Description

The series of states marked with "read field with active datastore transaction" (element 14 of the "transitions" field of org.apache.jdo.tck.lifecycle.StateTransitions) asserts that accessing a field of a deleted instance will throw an exception. However, this is not mandated by the spec, only suggested that it might happen. Section 5.5.6 reads: "Any other access to persistent fields is not supported and might throw a JDOUserException".

The only easy fix is to just remove the "read field with active datastore transaction" test specifications.

Comments

[Marc Prud'hommeaux]

This also affects the "read field with active optimistic transaction" state list. Another way to fix it is to change elements 7 & 8 from ERROR to IMPOSSIBLE in both the "read field with active optimistic transaction" and "read field with active datastore transaction" arrays.

[Marc Prud'hommeaux]

Attached is a patch to fix the incorrect state transition assertions.

[Craig Russell]

The patch is an elegant solution to the issue.

The only other option is to require an exception to be thrown in case the implementation is BinaryCompatible, which would require a specification change. This would also require a more extensive change to the test. I'm not sure that it's worth it.

Other opinions?

[Marc Prud'hommeaux]

My opinion is that it is useful to be left up to the vendor to decide whether to throw an exception or not (or only throw an exception if the field is hollow).

[Craig Russell]

In JDO 1, it was a requirement to throw an exception to access a field other than the primary key field in a deleted instance. We changed it in JDO 2 to accommodate implementations that didn't get control on each field access (reflective implementations).

Hi Craig,

Your last comment is very interesting. I didn't know that this is the new policy. Actually ObjectDB 2.0 supports dual mode - enhancement when available (but with no binary compatibility) and reflection as a backup for persistence capable classes that are not enhanced.

Currently, out of the 445 TCK tests that ObjectDB passes - 6 tests fail in reflection mode:

1. IsTransactionalFalse.testIsTransactionalFalse (access in line 86 is not detected)
2. MakeTransactionalPriorToTransactionRolledback.testTransactionalInst (modifications in lines 115, 117 are not detected)
3. InstanceLifecycleListenerLoad.testLoad (access in line 101 is not detected)
4. NoAccessToFieldsAfterPredelete.test (access in line 115, for instance, is not detected)
5. StateTransitions - as discussed in this issue.
6. WhenNontransactionalReadIsFalse.test (access in line 116, for instance, is not detected)

My plan was to offer the reflection mode as a non compatible extension, but if JDO can support reflection mode and these tests can be fixed it could be much better.

Regards, Ilan

[Craig Russell]

Please review this patch.

This patch uses a different flag, UNSPECIFIED, to bypass running the test. The semantic difference is that UNSPECIFIED means that the behavior is not specified, while IMPOSSIBLE means that the conditions for the test cannot be created.

This allows us to update the life cycle table in the specification to be consistent.

[Michael Bouschen]

The patch looks good!

[Craig Russell]

svn commit -m "JDO-419 JDO-424 Changed state transition table to use UNSPECIFIED where the specification is unclear" trunk/tck20/src/java/org/apache/jdo/tck/lifecycle/StateTransitions.java branches/2.0.1/tck20/src/java/org/apache/jdo/tck/lifecycle/StateTransitions.java
Sending branches/2.0.1/tck20/src/java/org/apache/jdo/tck/lifecycle/StateTransitions.java
Sending trunk/tck20/src/java/org/apache/jdo/tck/lifecycle/StateTransitions.java
Transmitting file data ..
Committed revision 454538.