Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 349958

Summary: Revision values are not restored to previous
Product: [Modeling] EMF Reporter: Egidijus Vaisnora <vaisegid>
Component: cdo.coreAssignee: Egidijus Vaisnora <vaisegid>
Status: CLOSED WONTFIX QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 CC: saulius.tvarijonas
Version: 4.1Flags: stepper: review-
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
test case v1
none
TestCase+Patch v1
none
patch v2 none

Description Egidijus Vaisnora CLA 2011-06-21 11:17:20 EDT
I have element A and B. Both references each other  via opposite. B element has value of A referenced by CDOID. If I remove A from the container and move back, B element has now referencing value A by object reference. 
Expected behavior is to have B referencing to A by its CDOID, as it was before removing from container. Perhaps even better is not to change other references, if containment is modified...
Comment 1 Egidijus Vaisnora CLA 2011-06-21 11:18:55 EDT
Created attachment 198342 [details]
test case v1
Comment 2 Egidijus Vaisnora CLA 2011-06-22 04:12:13 EDT
Created attachment 198376 [details]
TestCase+Patch v1

Added on attachement opposite adjustment (like it is done on detach). It should change references from object to CDOID back
Comment 3 Egidijus Vaisnora CLA 2011-06-28 09:50:21 EDT
Created attachment 198725 [details]
patch v2

Updated test case and fix
Comment 4 Eike Stepper CLA 2011-07-02 06:14:18 EDT
Changing to 4.1 to ensure that the fix will "last". Please clone this bugzilla
to 4.0 if you want a maintenance fix, too.
Comment 5 Eike Stepper CLA 2011-07-02 06:38:32 EDT
There seems to be a misunderstanding. To clarify it I'll copy over a Skype discussion below.

Egidijus, please submit a separate bugzilla to fix your original problem in the compare() mathod!

[12:24:40] Eike Stepper: the revision values for references are not always required to be CDOIDs
[12:24:57] Eike Stepper: in local dirty transactions they're often EObjects
[12:25:17] Eike Stepper: which get converted to CDOIDs during commit
[12:25:52] Eike Stepper: since your test fails when explicitely asserting for CDOID i'm inclined to reject that bug
[12:26:01] Eike Stepper: can you explain why you submitted it?
[12:26:06] Egidijus Vaishnora: original problem was that CDORevision compare gived incorrect deltas
[12:26:21] Eike Stepper: then we should fix the compare
[12:26:37] Eike Stepper: even with your current fix there can be EObjects!
[12:27:37] Egidijus Vaishnora: Yes it could be EObjects - like new objects
[12:28:22] Egidijus Vaishnora: I just took aproach that if before was CDOIDs, then after attache-detache it is expected to bring to the same state
[12:28:27] Eike Stepper: it's more about external references
[12:28:42] Eike Stepper: you can xref to yet unattached objects
[12:29:02 | Bearbeitet 12:30:28] Eike Stepper: there is no such expectation like you said (at least not for dirty transactions)
[12:30:28] Egidijus Vaishnora: ok then, it is my bad. Then perhaps revision compare should be adjusted to be aware of situation that in the list could be objects and OID. I just doubt that will be rest of the code which won't take this into account
[12:31:13] Eike Stepper: i think all places that do not yet consider it should be changed
[12:31:46] Eike Stepper: it would be extremely unperformant to always check all crossreferences on each model modification
[12:32:01] Eike Stepper: so we defer these checks/actions to commit time
[12:32:23] Eike Stepper: unless we decide to change that mechansim we must consider the consequences everywhere
[12:33:26] Egidijus Vaishnora: ok - we assume that CDOID and Objects with persisted CDOID can be mixed.
[12:33:42] Egidijus Vaishnora: What do you ment by "in local dirty transactions they're often EObjects"?
[12:35:46] Eike Stepper: this entire situation (revision contains eobject as refrence target) can only happen in response to local modifications. i.e. it will never appear in read only views or in transactions that have isDirty()==false
Comment 6 Eike Stepper CLA 2012-09-21 07:17:50 EDT
Closing.