Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Modeling » EMF » [EMF] MinimalEStoreEObjectImpl
[EMF] MinimalEStoreEObjectImpl [message #1470805] Wed, 12 November 2014 16:47 Go to next message
Esteban Dugueperoux is currently offline Esteban DugueperouxFriend
Messages: 472
Registered: July 2009
Senior Member
Hi EMF and CDO community,

I would like talk about
https://www.eclipse.org/forums/index.php/t/841521/ which seems due to a
EMF bug as said by Laurent Fasani.
The bug is only reproductible with CDO as it has an implementation of
MinimalEStoreEObjectImpl, i.e. CDOObject.

Outside CDO, when a list is created for a EReference and we call unset()
on it a clear() is done. Like that if it is a containment reference,
inverseRemove() is called for each object of the list to have them
detached of the resource.

But in CDO context, we have BasicEStoreEList created through
MinimalEStoreEObjectImpl.createList(). But when calling unset() on it,
EStore.unset() is called but objects of the list are not detached.
Could you confirm that as a bug?
We can't provide a EMF test as the only concrete implementation of
MinimalEStoreEObjectImpl is CDOObjectImpl, but we have a CDO JUnit test
: Bugzilla_429659_Test.testUndoSeveralElementsCreation().

Then if you confirm it is a EMF bug, where to call the clear()? In
BasicEObjectImpl.eUnset(), in BasicEStoreEList.unset(), or elsewhere?


Best Regards.

--
Esteban Dugueperoux - Obeo

Need professional services for Sirius?
http://www.obeodesigner.com/sirius
Re: [EMF] MinimalEStoreEObjectImpl [message #1471721 is a reply to message #1470805] Thu, 13 November 2014 09:35 Go to previous messageGo to next message
Ed Merks is currently offline Ed MerksFriend
Messages: 28973
Registered: July 2009
Senior Member
Esteban,

Comments below.

On 12/11/2014 5:47 PM, Esteban Dugueperoux wrote:
> Hi EMF and CDO community,
>
> I would like talk about
> https://www.eclipse.org/forums/index.php/t/841521/ which seems due to
> a EMF bug as said by Laurent Fasani.
> The bug is only reproductible with CDO as it has an implementation of
> MinimalEStoreEObjectImpl, i.e. CDOObject.
>
> Outside CDO, when a list is created for a EReference and we call
> unset() on it a clear() is done. Like that if it is a containment
> reference, inverseRemove() is called for each object of the list to
> have them detached of the resource.
>
> But in CDO context, we have BasicEStoreEList created through
> MinimalEStoreEObjectImpl.createList(). But when calling unset() on it,
> EStore.unset() is called but objects of the list are not detached.
> Could you confirm that as a bug?
A few things don't look quite right... E.g., in the transient case we
just throw the list away, but we don't unset/clear it...

@Override
public void dynamicUnset(int dynamicFeatureID)
{
Object[] eSettings = eDynamicSettings();
EStructuralFeature eStructuralFeature =
eDynamicFeature(dynamicFeatureID);
if (eStructuralFeature.isTransient())
{
eSettings[dynamicFeatureID] = null;
}
else
{
eStore().unset(this, eStructuralFeature);
eSettings[dynamicFeatureID] = null;
}
}

Also, in the "example testing store implementation" this doesn't clear
the list

public void clear(InternalEObject eObject, EStructuralFeature feature)
{
Entry entry = new Entry(eObject, feature);
map.remove(entry);
//getList(entry).clear();
}

And this does nothing either:

public void unset(InternalEObject eObject, EStructuralFeature feature)
{
Entry entry = new Entry(eObject, feature);
map.remove(entry);
}

But yes, ultimately a real EStore implementation must do the correct
things, i.e., clear the list and in the case of unsettable lists clears
it and restore the unset state. And of course when bidirectional things
are involved it must ensure that those things are done properly as well.

> We can't provide a EMF test as the only concrete implementation of
> MinimalEStoreEObjectImpl is CDOObjectImpl, but we have a CDO JUnit
> test : Bugzilla_429659_Test.testUndoSeveralElementsCreation().
>
> Then if you confirm it is a EMF bug, where to call the clear()? In
> BasicEObjectImpl.eUnset(), in BasicEStoreEList.unset(), or elsewhere?
In a real store implementation you must do the appropriate/correct
things. The only thing that appears incorrect in the basic
infrastructure is the transient handling in
org.eclipse.emf.ecore.impl.MinimalEStoreEObjectImpl.dynamicUnset(int).
Is that the fundamental problem? The rest is really up to the store
implementation so if that's where the problem lies that's where it needs
to be solved.
>
>
> Best Regards.
>
Re: [EMF] MinimalEStoreEObjectImpl [message #1471820 is a reply to message #1471721] Thu, 13 November 2014 11:05 Go to previous message
Esteban Dugueperoux is currently offline Esteban DugueperouxFriend
Messages: 472
Registered: July 2009
Senior Member
Hi Ed,

Comments below.

Le 13/11/2014 10:35, Ed Merks a écrit :
> Esteban,
>
> Comments below.
>
> On 12/11/2014 5:47 PM, Esteban Dugueperoux wrote:
>> Hi EMF and CDO community,
>>
>> I would like talk about
>> https://www.eclipse.org/forums/index.php/t/841521/ which seems due to
>> a EMF bug as said by Laurent Fasani.
>> The bug is only reproductible with CDO as it has an implementation of
>> MinimalEStoreEObjectImpl, i.e. CDOObject.
>>
>> Outside CDO, when a list is created for a EReference and we call
>> unset() on it a clear() is done. Like that if it is a containment
>> reference, inverseRemove() is called for each object of the list to
>> have them detached of the resource.
>>
>> But in CDO context, we have BasicEStoreEList created through
>> MinimalEStoreEObjectImpl.createList(). But when calling unset() on it,
>> EStore.unset() is called but objects of the list are not detached.
>> Could you confirm that as a bug?
> A few things don't look quite right... E.g., in the transient case we
> just throw the list away, but we don't unset/clear it...
>
> @Override
> public void dynamicUnset(int dynamicFeatureID)
> {
> Object[] eSettings = eDynamicSettings();
> EStructuralFeature eStructuralFeature =
> eDynamicFeature(dynamicFeatureID);
> if (eStructuralFeature.isTransient())
> {
> eSettings[dynamicFeatureID] = null;
> }
> else
> {
> eStore().unset(this, eStructuralFeature);
> eSettings[dynamicFeatureID] = null;
> }
> }
>
> Also, in the "example testing store implementation" this doesn't clear
> the list
>
> public void clear(InternalEObject eObject, EStructuralFeature feature)
> {
> Entry entry = new Entry(eObject, feature);
> map.remove(entry);
> //getList(entry).clear();
> }
>
> And this does nothing either:
>
> public void unset(InternalEObject eObject, EStructuralFeature feature)
> {
> Entry entry = new Entry(eObject, feature);
> map.remove(entry);
> }
>
Ok then unrelated to our issue, there is a bug in the "example testing
store implementation", it's not a problem as we don't use it.
> But yes, ultimately a real EStore implementation must do the correct
> things, i.e., clear the list and in the case of unsettable lists clears
> it and restore the unset state. And of course when bidirectional things
> are involved it must ensure that those things are done properly as well.
>
In our specific case model generated for CDO, i.e. with feature
delegation Reflective and having all model elements inheriting from
CDOObjectImpl, i.e. MinimalEStoreEObjectImpl, we have a many containment
reference, for which we call EObject.eUnset(). After we have
BasicEStoreEList.unset() called which is similar to Generic.unset()
except that instead of calling super.unset() EStore.unset() is called.
Then in addition to do a unset, we must call EList.clear() to have same
EMF notifications as when DelegatingEcoreEList.unset() is called. Ok
I'll talk with Eike, how to do that correctly in CDO EStore
implementations, i.e. TransientStore and CDOStoreImpl.

>> We can't provide a EMF test as the only concrete implementation of
>> MinimalEStoreEObjectImpl is CDOObjectImpl, but we have a CDO JUnit
>> test : Bugzilla_429659_Test.testUndoSeveralElementsCreation().
>>
>> Then if you confirm it is a EMF bug, where to call the clear()? In
>> BasicEObjectImpl.eUnset(), in BasicEStoreEList.unset(), or elsewhere?
> In a real store implementation you must do the appropriate/correct
> things. The only thing that appears incorrect in the basic
> infrastructure is the transient handling in
> org.eclipse.emf.ecore.impl.MinimalEStoreEObjectImpl.dynamicUnset(int).
> Is that the fundamental problem? The rest is really up to the store
> implementation so if that's where the problem lies that's where it needs
> to be solved.
>>
>>
>> Best Regards.
>>
>


Thanks.


--
Esteban Dugueperoux - Obeo

Need professional services for Sirius?
http://www.obeodesigner.com/sirius
Previous Topic:[CDO] Invalid signalID on commit
Next Topic:[CDO] Is CDOMergingConflictResolver really called back when conflicts occur ?
Goto Forum:
  


Current Time: Wed Nov 22 18:47:32 GMT 2017

Powered by FUDForum. Page generated in 0.02022 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software