| Summary: | Revision values are not restored to previous | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Egidijus Vaisnora <vaisegid> | ||||||||
| Component: | cdo.core | Assignee: | Egidijus Vaisnora <vaisegid> | ||||||||
| Status: | CLOSED WONTFIX | QA Contact: | Eike Stepper <stepper> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | saulius.tvarijonas | ||||||||
| Version: | 4.1 | Flags: | stepper:
review-
|
||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Egidijus Vaisnora
Created attachment 198342 [details]
test case v1
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
Created attachment 198725 [details]
patch v2
Updated test case and fix
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. 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 Closing. |