Home » Modeling » EMF » [CDO] CDOResources being EObjects confuses some APIs
| [CDO] CDOResources being EObjects confuses some APIs [message #910496] |
Sun, 09 September 2012 17:00  |
Eclipse User |
|
|
|
Hi,
The fact that CDOResources are EObjects confuses some APIs. For
example, it is often assumed that if an object is an EObject, then it
isn't a Resource and that therefore asking it for its eResource() makes
sense. This is a fairly common pattern in EContentAdapters. They are
attached notifiers of three kinds: ResourceSets, Resources, and
EObjects. So, sometimes, they'll check what kind of object sent a
notification and act accordingly.
One such case is Eclipse UML's CacheAdapter. It handles notifications
like this:
@Override
public void notifyChanged(Notification msg) {
super.notifyChanged(msg);
Object notifier = msg.getNotifier();
if (notifier instanceof EObject) {
// clear at resource scope iff not touch
if (!msg.isTouch()) {
clear(((EObject) notifier).eResource());
}
} else if (notifier instanceof Resource) {
// ... do Resource-ly things ...
}
}
Unfortunately, after a CDOTransaction has closed, the eResource() call
on a CDOResource results in an IllegalStateException (copied below).
Closing a transaction sets off this sequence of events:
- remove all CDOResources from the resource set
- for each resource being removed, its resourceSet inverse reference
is set to null
- so, each resource sends a Notification for its resourceSet changing to null
- the CacheAdapter gets this Notification and sees that the notifier
is an EObject
- because the notification is not a touch, the CacheAdapter tries to
clear all data
cached for the resource containing the EObject. So, it asks for
the object's
eResource()
- the CDOStore is responsible for answering the eResource(), but the
object can't
access it because the transaction isn't open. IllegalStateException
So, I can fix this in the CacheAdapter by changing the order in which
it tests the kind of object that sent the notification: check first
whether it's a Resource, then whether it's an EObject if it wasn't a
Resource.
However, this worries me. How much code out there is like the
CacheAdapter, checking the EObject case before the Resource case? How
much code out there doesn't expect that anything will be both an
EObject *and* a Resource? Lots of code that just checks for EObject
will have to be changed to check for EObject-and-not-Resource. And
this isn't just in content-adapters ...
This feels like a goldmine of trouble. What advice can you offer to
people porting applications using existing APIs from third parties onto
CDO?
Thanks,
Christian
-------- 8< --------
java.lang.IllegalStateException: Not active: Transaction 1 [MAIN]
at
org.eclipse.net4j.util.lifecycle.LifecycleUtil.checkActive(LifecycleUtil.java:87)
at org.eclipse.net4j.util.lifecycle.Lifecycle.checkActive(Lifecycle.java:194)
at
org.eclipse.emf.internal.cdo.view.AbstractCDOView.getStore(AbstractCDOView.java:178)
at org.eclipse.emf.internal.cdo.CDOObjectImpl.cdoStore(CDOObjectImpl.java:943)
at
org.eclipse.emf.internal.cdo.CDOObjectImpl.eDirectResource(CDOObjectImpl.java:481)
at
org.eclipse.emf.cdo.eresource.impl.CDOResourceImpl.eDirectResource(CDOResourceImpl.java:205)
at
org.eclipse.emf.ecore.impl.BasicEObjectImpl.eInternalResource(BasicEObjectImpl.java:925)
at
org.eclipse.emf.internal.cdo.CDOObjectImpl.eInternalResource(CDOObjectImpl.java:492)
at
org.eclipse.emf.ecore.impl.BasicEObjectImpl.eResource(BasicEObjectImpl.java:920)
at
org.eclipse.uml2.common.util.CacheAdapter.notifyChanged(CacheAdapter.java:374)
at
org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:374)
at
org.eclipse.emf.ecore.impl.EStructuralFeatureImpl$InternalSettingDelegateSingleData.dynamicSet(EStructuralFeatureImpl.java:2023)
at
org.eclipse.emf.ecore.impl.BasicEObjectImpl.eDynamicSet(BasicEObjectImpl.java:1127)
at
org.eclipse.emf.ecore.impl.BasicEObjectImpl.eSet(BasicEObjectImpl.java:1101)
at
org.eclipse.emf.ecore.impl.BasicEObjectImpl.eSet(BasicEObjectImpl.java:1071)
at
org.eclipse.emf.cdo.eresource.impl.CDOResourceImpl.setResourceSet(CDOResourceImpl.java:273)
at
org.eclipse.emf.cdo.eresource.impl.CDOResourceImpl.basicSetResourceSet(CDOResourceImpl.java:1356)
at
org.eclipse.emf.ecore.resource.impl.ResourceSetImpl$ResourcesEList.inverseRemove(ResourceSetImpl.java:604)
at
org.eclipse.emf.common.notify.impl.NotifyingListImpl.removeAll(NotifyingListImpl.java:945)
at
org.eclipse.emf.internal.cdo.view.CDOViewSetImpl.remove(CDOViewSetImpl.java:183)
at
org.eclipse.emf.internal.cdo.session.CDOViewContainerImpl.viewDetached(CDOViewContainerImpl.java:155)
at
org.eclipse.emf.internal.cdo.view.CDOViewImpl.doDeactivate(CDOViewImpl.java:1128)
at
org.eclipse.emf.internal.cdo.transaction.CDOTransactionImpl.doDeactivate(CDOTransactionImpl.java:2247)
at org.eclipse.net4j.util.lifecycle.Lifecycle.deactivate(Lifecycle.java:129)
at
org.eclipse.net4j.util.lifecycle.LifecycleUtil.deactivate(LifecycleUtil.java:221)
at
org.eclipse.net4j.util.lifecycle.LifecycleUtil.deactivate(LifecycleUtil.java:211)
at
org.eclipse.net4j.util.lifecycle.LifecycleUtil.deactivate(LifecycleUtil.java:237)
at
org.eclipse.emf.internal.cdo.view.AbstractCDOView.close(AbstractCDOView.java:1332)
. . .
|
|
|
| Re: [CDO] CDOResources being EObjects confuses some APIs [message #910743 is a reply to message #910496] |
Mon, 10 September 2012 06:20  |
| Eclipse User |
|
|
|
Am 09.09.2012 23:00, schrieb Christian W. Damus:
> Hi,
>
> The fact that CDOResources are EObjects confuses some APIs. For example, it is often assumed that if an object is an
> EObject, then it isn't a Resource and that therefore asking it for its eResource() makes sense. This is a fairly
> common pattern in EContentAdapters. They are attached notifiers of three kinds: ResourceSets, Resources, and
> EObjects. So, sometimes, they'll check what kind of object sent a notification and act accordingly.
>
> One such case is Eclipse UML's CacheAdapter. It handles notifications like this:
>
> @Override
> public void notifyChanged(Notification msg) {
> super.notifyChanged(msg);
>
> Object notifier = msg.getNotifier();
>
> if (notifier instanceof EObject) {
> // clear at resource scope iff not touch
> if (!msg.isTouch()) {
> clear(((EObject) notifier).eResource());
> }
> } else if (notifier instanceof Resource) {
> // ... do Resource-ly things ...
> }
> }
This is a known issue. I've searched org.eclipse.emf.ecore for "instanceof Resource" and counted the ones that test for
EObject *before* that check:
org.eclipse.emf.ecore.util.EContentAdapter.setTarget(Notifier)
org.eclipse.emf.ecore.util.EContentAdapter.unsetTarget(Object) - @Deprecated
org.eclipse.emf.ecore.util.EcoreUtil.ContentTreeIterator.getChildren(Object)
org.eclipse.emf.ecore.util.ECrossReferenceAdapter.selfAdapt(Notification)
org.eclipse.emf.ecore.util.ECrossReferenceAdapter.setTarget(Notifier)
org.eclipse.emf.ecore.util.ECrossReferenceAdapter.unsetTarget(Notifier)
Strange that this method does the Resource check first:
org.eclipse.emf.ecore.util.EContentAdapter.selfAdapt(Notification)
Unfortunately I know no other fix than to change CDO and make CDOResource no longer extend EObject. Certainly a very
breaking change and, similarly problematic, a huge effort ;-(
> Unfortunately, after a CDOTransaction has closed, the eResource() call on a CDOResource results in an
> IllegalStateException (copied below). Closing a transaction sets off this sequence of events:
>
> - remove all CDOResources from the resource set
> - for each resource being removed, its resourceSet inverse reference is set to null
> - so, each resource sends a Notification for its resourceSet changing to null
> - the CacheAdapter gets this Notification and sees that the notifier is an EObject
> - because the notification is not a touch, the CacheAdapter tries to clear all data
> cached for the resource containing the EObject. So, it asks for the object's
> eResource()
> - the CDOStore is responsible for answering the eResource(), but the object can't
> access it because the transaction isn't open. IllegalStateException
I think we could easily set eDeliver=false before CDO remoevs the resources from the resource set. But you still would
like to clean up your CacheAdapter. Maybe you'd prefer to use CDO API for this? CDOView/CDOTransaction emit, among
others, ILifecycleEvents:
public interface ILifecycleEvent extends IEvent
{
public ILifecycle getSource();
public Kind getKind();
public enum Kind
{
ABOUT_TO_ACTIVATE, ACTIVATED, ABOUT_TO_DEACTIVATE, DEACTIVATED
}
}
> So, I can fix this in the CacheAdapter by changing the order in which it tests the kind of object that sent the
> notification: check first whether it's a Resource, then whether it's an EObject if it wasn't a Resource.
Yes, for CDO this is generally useful/needed until we finally remove EObject from the set of super types of CDOResource.
> However, this worries me. How much code out there is like the CacheAdapter, checking the EObject case before the
> Resource case? How much code out there doesn't expect that anything will be both an EObject *and* a Resource? Lots of
> code that just checks for EObject will have to be changed to check for EObject-and-not-Resource. And this isn't just
> in content-adapters ...
>
> This feels like a goldmine of trouble. What advice can you offer to people porting applications using existing APIs
> from third parties onto CDO?
Until we agree that we want to break current CDO users and someone is willing to spend the effort or pay for it
consumers may need to customize their code or other code that they rely on. This topic has come up every once in a while
but not too frequently. In all cases the necessary changes were easy to find and to apply.
The places in EMF itself are mostly EContentAdapter and ECrossReferenceAdapter, two classes that tend to create trouble
anyway when used together with CDO managed models.
Cheers
/Eike
----
http://www.esc-net.de
http://thegordian.blogspot.com
http://twitter.com/eikestepper
|
|
|
| Re: [CDO] CDOResources being EObjects confuses some APIs [message #910826 is a reply to message #910743] |
Mon, 10 September 2012 08:58 |
| Eclipse User |
|
|
|
Hi, Eike,
Thanks for your reply! See some follow-up in-line.
cW
On 2012-09-10 10:20:38 +0000, Eike Stepper said:
> Am 09.09.2012 23:00, schrieb Christian W. Damus:
>> Hi,
>>
>> The fact that CDOResources are EObjects confuses some APIs. For
>> example, it is often assumed that if an object is an EObject, then it
>> isn't a Resource and that therefore asking it for its eResource() makes
>> sense. This is a fairly common pattern in EContentAdapters. They are
>> attached notifiers of three kinds: ResourceSets, Resources, and
>> EObjects. So, sometimes, they'll check what kind of object sent a
>> notification and act accordingly.
>>
>> One such case is Eclipse UML's CacheAdapter. It handles notifications
>> like this:
>>
>> @Override
>> public void notifyChanged(Notification msg) {
>> super.notifyChanged(msg);
>>
>> Object notifier = msg.getNotifier();
>>
>> if (notifier instanceof EObject) {
>> // clear at resource scope iff not touch
>> if (!msg.isTouch()) {
>> clear(((EObject) notifier).eResource());
>> }
>> } else if (notifier instanceof Resource) {
>> // ... do Resource-ly things ...
>> }
>> }
> This is a known issue. I've searched org.eclipse.emf.ecore for
> "instanceof Resource" and counted the ones that test for EObject
> *before* that check:
>
> org.eclipse.emf.ecore.util.EContentAdapter.setTarget(Notifier)
> org.eclipse.emf.ecore.util.EContentAdapter.unsetTarget(Object) -
> @Deprecated
> org.eclipse.emf.ecore.util.EcoreUtil.ContentTreeIterator.getChildren(Object)
> org.eclipse.emf.ecore.util.ECrossReferenceAdapter.selfAdapt(Notification)
> org.eclipse.emf.ecore.util.ECrossReferenceAdapter.setTarget(Notifier)
> org.eclipse.emf.ecore.util.ECrossReferenceAdapter.unsetTarget(Notifier)
>
> Strange that this method does the Resource check first:
>
> org.eclipse.emf.ecore.util.EContentAdapter.selfAdapt(Notification)
>
> Unfortunately I know no other fix than to change CDO and make
> CDOResource no longer extend EObject. Certainly a very breaking change
> and, similarly problematic, a huge effort ;-(
Yes, I certainly understand that there would be large consequences of
such a change. I'm not asking for that! Just hoping that there's
something I can do to make it easier to integrate code that I can't
modify. Although, for my purposes, I can recommend modifications in
some of the dependencies of Papyrus because the Papyrus team has some
influence with those other projects.
>> Unfortunately, after a CDOTransaction has closed, the eResource() call
>> on a CDOResource results in an IllegalStateException (copied below).
>> Closing a transaction sets off this sequence of events:
>>
>> - remove all CDOResources from the resource set
>> - for each resource being removed, its resourceSet inverse reference is
>> set to null
>> - so, each resource sends a Notification for its resourceSet changing to null
>> - the CacheAdapter gets this Notification and sees that the notifier is
>> an EObject
>> - because the notification is not a touch, the CacheAdapter tries to
>> clear all data
>> cached for the resource containing the EObject. So, it asks for the object's
>> eResource()
>> - the CDOStore is responsible for answering the eResource(), but the
>> object can't
>> access it because the transaction isn't open. IllegalStateException
> I think we could easily set eDeliver=false before CDO remoevs the
> resources from the resource set. But you still would like to clean up
> your CacheAdapter. Maybe you'd prefer to use CDO API for this?
> CDOView/CDOTransaction emit, among others, ILifecycleEvents:
Well, I think it's important to maintain the CDOResource's behaviour as
a Resource as much as possible, too. Suppressing delivery of
notifications will doubtless break something else. I would prefer to
substitute a CDO-specific CacheAdapter implementation in UML2
deployments with CDO if it should prove necessary; UML2 already
provides a mechanism to do that with external configuration. But, so
far it doesn't seem to be necessary.
Anyways, I've confirmed that simply rearranging the code a bit in the
CacheAdapter resolves my problem, so I'm happy with that solution.
> public interface ILifecycleEvent extends IEvent
> {
> public ILifecycle getSource();
> public Kind getKind();
> public enum Kind
> {
> ABOUT_TO_ACTIVATE, ACTIVATED, ABOUT_TO_DEACTIVATE, DEACTIVATED
> }
> }
>
>> So, I can fix this in the CacheAdapter by changing the order in which
>> it tests the kind of object that sent the notification: check first
>> whether it's a Resource, then whether it's an EObject if it wasn't a
>> Resource.
> Yes, for CDO this is generally useful/needed until we finally remove
> EObject from the set of super types of CDOResource.
It's easy to do this kind of change. I have no problem with that and
will include it in my plan.
>> However, this worries me. How much code out there is like the
>> CacheAdapter, checking the EObject case before the Resource case? How
>> much code out there doesn't expect that anything will be both an
>> EObject *and* a Resource? Lots of code that just checks for EObject
>> will have to be changed to check for EObject-and-not-Resource. And
>> this isn't just in content-adapters ...
>>
>> This feels like a goldmine of trouble. What advice can you offer to
>> people porting applications using existing APIs from third parties onto
>> CDO?
> Until we agree that we want to break current CDO users and someone is
> willing to spend the effort or pay for it consumers may need to
> customize their code or other code that they rely on. This topic has
> come up every once in a while but not too frequently. In all cases the
> necessary changes were easy to find and to apply.
Yes, as I said, I'm not asking for any change in CDO, just advice. So
far I'm doing fine with tweaks in the code, but I have this tendency to
worry about the code that I don't know about. :-)
> The places in EMF itself are mostly EContentAdapter and
> ECrossReferenceAdapter, two classes that tend to create trouble anyway
> when used together with CDO managed models.
Yeah, it's in the nature of these adapters to want to know every
object, and that's fine. It's what they were made for. I'm still
hoping that the CacheAdapter will be manageable as currently
implementated, because it looks like UML can work in legacy mode and
legacy mode doesn't support the lazy loading anyway. All in all, a
fairly consistent picture.
BTW, have I mentioned how thoroughly impressed I am with CDO? In all
these years, this is the first project in which I'm seriously working
with it, and I was worried about how UML and GMF will work. It didn't
take me long to lose that worry: the work that the CDO team has done is
simply amazing!
>
> Cheers
> /Eike
>
> ----
> http://www.esc-net.de
> http://thegordian.blogspot.com
> http://twitter.com/eikestepper
|
|
| | |
Goto Forum:
Current Time: Thu Nov 13 06:57:52 EST 2025
Powered by FUDForum. Page generated in 0.05111 seconds |
Home