Bugzilla – Full Text Bug Listing
| Summary: | [Legacy] OCLQueryTest fails in all LEGACY scenarios | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Eike Stepper <stepper> | ||||||||||||||
| Component: | cdo.legacy | Assignee: | Martin Fluegge <martin.fluegge> | ||||||||||||||
| Status: | CLOSED FIXED | QA Contact: | Eike Stepper <stepper> | ||||||||||||||
| Severity: | normal | ||||||||||||||||
| Priority: | P3 | CC: | caspar_d | ||||||||||||||
| Version: | 4.0 | Flags: | stepper:
review+
|
||||||||||||||
| Target Milestone: | --- | ||||||||||||||||
| Hardware: | PC | ||||||||||||||||
| OS: | Windows 7 | ||||||||||||||||
| Whiteboard: | |||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Eike Stepper
Maybe some configuration of the server side is missing? I added skipConfig(LEGACY); Please investigate when you have time... The problem was introduced with the changes from Bug 334995. The AbstractCDOView (789) throws an exception when it tries to set the name, which should happen because this change is triggered from the view which actually does not allow modification of objects. Created attachment 188793 [details]
Patch v1
I attached a patch which fixes the problem by checking if the modification could work. Unfortunately this introduces a reference to CDOTransaction to the AbstractCDOView which I think is not a good design. But since this check is already done in the class this could be ok.
I would prefer a method like *isTranactional()* in the CDOView which would provide a cleaner seperation between Views and Transactions.
Eike, please let us discuss this.
Created attachment 188808 [details]
Patch B v1
I hit the road again and had a closer look why legacy tries to create the resource multiple times.
Finally found it in the recursion of legacy itself. The old problem that a legacy object is not registered to the view before the whole tree is build up popped up again. Actually legacy has mechanisms to prevent this but these failed since a CDOResource is no legacy object. I do not want to deliver a to detailed description since my had is still acing from all this recursive stuff ;)
I attached a patch to solve the problem. Named it "Patch B v1". Eike, I hope the naming is correct this time ;)
But I still believe the former patch should be discussed as well as the creation of a isTransactional() method. I cc'ed Caspar. maybe he has some opinions on this.
Created attachment 188813 [details]
Patch v2
Attached patch which uses isReadOnly() as increment for patch v1.
Martin, the patches are both not workspace relative. Note that Subversive does not remember the last patch format choice ;-( AbstractCDOView.newResourceInstance(InternalCDORevision): I'm not sure if simply ignoring the situation that is discovered by Caspar's new check is a good idea. Caspar, can you please review Martin's change? Created attachment 188827 [details]
Patch v3 - all in one
I'd suggest to split up the patches again so we can solve this issue with the legacy patch and move the other patch as base for a discussion to Bug 334995. Ok ;-) Created attachment 188835 [details]
Patch v4 - only legacy
Only the legacy changes. Please review.
Created attachment 188851 [details]
Patch v5
Forgot do include the re-enabling of the testrun for legacy in the patch. It's now included in the patch.
Committed revision 7070: - trunk/plugins/org.eclipse.emf.cdo - trunk/plugins/org.eclipse.emf.cdo.tests Resolved fixed. Available in R20110608-1407 |