Bugzilla – Full Text Bug Listing
| Summary: | [Legacy] Unidirectionnal non containment reference not properly set after commit for current client | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Steve Monnier <steve.monnier> | ||||
| Component: | cdo.legacy | Assignee: | Christian Damus <give.a.damus> | ||||
| Status: | NEW --- | QA Contact: | Eike Stepper <stepper> | ||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | alex.lagarde, Ed.Merks, laurent.redor, stepper | ||||
| Version: | 4.13 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Steve Monnier
Created attachment 208457 [details]
JUnit reproducing the issue
Here is a JUnit test to reproduce the issue with 2 Eclass and a super type reference
Happy new year! :) Do you have any idea how to fix/work around this issue? Thanks, Steve Sorry, I do not see any chance to have a look at this problem in the next weeks. Unfortunately it will take a while until I can care for it. Cheers, Martin Ok, after a short investigation the problem seems to be caused by the EClass implementation.
The CDOLegacyWrapper, in the cdoInternalPreCommit() method, clears all the structural features that are unset so that no default value is stored in the database.
Here is the CDOLegacyWrapper code that fulfills this task :
for (EStructuralFeature feature : CDOModelUtil.getAllPersistentFeatures(eClass)) {
if (feature.isUnsettable()) {
if (!instance.eIsSet(feature)) {
if (feature.isMany()) {
InternalEList<Object> list = (InternalEList<Object>)instance.eGet(feature);
clearEList(list);
}
In the case of an EClass c1 having an EClass c2 as supertype, when we test if the "EGenericSuperTypes" of c1 is set, it returns false (see the EClassImpl.getEGenericSuperTypes() method : if the GenericSuperTypes collection does not contain any Generictype having a non-null ETypeParameter or a non-empty ETypeArguments, it returns false).
Ok, no problem here. As this feature is unset, we clear it. But clearing the EGenericSuperTypes list also clears the ESuperTypes list, so after the call to cdoPreCommit(), c1 does not have any superTypes any more.
According to me, the EGenericSuperTypes collection should not use the same list as ESuperTypes, or maybe it should be defined as derived ?
===================================================
Here his a copy of Ed Merks answer :
Things like eSuperType and eGenericSuperType are "mutually derived". I.e., each can be derived from the other. Which one is consider set depends on which of the two needs to be serialized. If no use is made of generics, then the eSuperType can be serialized (to produce a serialization compatible with pre-generic Ecore). When generics are actually used, the eGenericType must be serialized to avoid loss of information. So only one of the two will ever be considered set, never both at once. That affects the serializer and even the copier, because otherwise they'd need special case Ecore support or serialize/copy redundantly. You'll have to live with this.
===================================================
Ed, I understand your answer, but the CDOLegacyWrapper code seems correct.
How should we deal with this issue ? We can always put the fix in the CDOLegacyWrapper, to add stronger condition before clearing the list, but it seems quite messy.
What do you think ?
It seems bad in general that an attempt to commit will change the state of the object. It's not clear how this approach works with Ecore in the first place. In some cases eSuperTypes is persisted and in others eGenericSuperTypes? That's what XML does, but what does CDO do for this case? As I mentioned, it's as if each is derived from the other, which is literally the implemented behavior. Changing either changes both. They can't be out of synch. Note that they're *not* the same list; they're separate lists with differently typed objects in them. But we can't mark them both as derived because then a copier will copy neither. Of course if we didn't have to support any kind of backward compatibility we'd have made eSuperTypes derived, or more likely not provided it at all, but such is the case for a model that's ten years old... Moving all open issues to 4.2. Open bugs can be ported to 4.1 maintenance after they've been fixed in master. Hi Christian, I assign this legacy mode zilla to you, just in case you're ineterested. Don't feel obliged ;-) We'll try to address open problems in 4.3 (master) first and then port fixes back to 4.2. Moving all open bugzillas to 4.5. Moving all unaddressed bugzillas to 4.6. Moving all open bugs to 4.7 Moving all unresolved issues to version 4.8- Moving all unresolved issues to version 4.9 Moving to 4.13. |