Comment 1 Eike Stepper 2011-11-15 02:09:03 EST

Note for Martin: Can bug 363613 be closed as invalid or duplicate of this bug?

Comment 2 Dan Pollitt 2012-01-09 10:13:47 EST

I have just come up against this problem with containment references to Ecore package types from my CDO-enabled model. Using the db store, I have an "edatatype" table but it's empty after adding in some instances of EClass with contained EStructuralFeatures (attributes and references).

Has there been any initial investigation into this or could someone give me a pointer to get started looking at a patch?

I am using CDO version: 4.0.1.v20110831-1303

Thanks,
Dan

Comment 3 Martin Fluegge 2012-01-09 11:41:09 EST

Dan, sorry currently there is not much time. It is likely that the problem comes from the CDOLegacyWrapper. I succest setting a break point at revisionToInstance() and check if all values are set correctly.

Comment 4 Dan Pollitt 2012-01-10 09:20:07 EST

As Tim reported in his original newsgroup posting for this issue, the EStructuralFeature "eType" (a reference) of the EClass "ETypedElement" is identified as not persistent by EMFUtil.isPersistent(EStructuralFeature feature) due to the following initial check:

...
if (feature == ECLASS_ESUPER_TYPES || feature == ETYPED_ELEMENT_ETYPE || feature == EOPERATION_EEXCEPTIONS)
    {
      // http://www.eclipse.org/newsportal/article.php?id=26780&group=eclipse.tools.emf#26780
      return false;
    }
...

where ETYPED_ELEMENT_ETYPE = EcorePackage.eINSTANCE.getETypedElement_EType();

If I change this check to exclude testing for ETYPED_ELEMENT_ETYPE, therefore returning true to EMFUtil.isPersistent(..) and run with this change in client and server I see that I have an eType column now on ereference and eattribute tables in the database. This column is populated after adding in an Ecore model, however it looks like the values in the eType column of eattribute are foreign keys to entries in the cdo_external_refs table, e.g. with URI : "http://www.eclipse.org/emf/2003/XMLType#//Double"

When I look at an EAttribute using the CDO Editor raised from the CDO Session view the eType is null (the package: http://www.eclipse.org/emf/2003/XMLType is not registered), presumuably because nothing is resolving external refs? 

I haven't looked at the internals of CDO before, can anyone help:

a) Understand why ETYPED_ELEMENT_ETYPE was set to be non-persistent
b) Tell whether not storing eType is a regression issue
c) Offer some advice on how to proceed to a fix/workaround

I would like to explore the model retrieved from CDO via a CDOView that I can get and look at the eType of EStructuralFeatures of EClasses that are persisted instances of Ecore model elements.

Comment 5 Eike Stepper 2012-01-10 09:45:02 EST

(In reply to comment #4)
>       //
> http://www.eclipse.org/newsportal/article.php?id=26780&group=eclipse.tools.emf#26780

A mess: the link does not seem to work anymore ;-(

Comment 6 Eike Stepper 2012-01-10 09:51:32 EST

> model, however it looks like the values in the eType column of eattribute are
> foreign keys to entries in the cdo_external_refs table, e.g. with URI :
> "http://www.eclipse.org/emf/2003/XMLType#//Double"

Yeah, that URI will only look the package up in the package registry. 

> When I look at an EAttribute using the CDO Editor raised from the CDO Session
> view the eType is null (the package: http://www.eclipse.org/emf/2003/XMLType is
> not registered), presumuably because nothing is resolving external refs? 

In which registry have you looked?

> I haven't looked at the internals of CDO before, can anyone help:
> 
> a) Understand why ETYPED_ELEMENT_ETYPE was set to be non-persistent

Hardly, because I don't see the forum post anymore that may explain it ;-(

> b) Tell whether not storing eType is a regression issue

Personally I suspect that not so many users are using the legacy mode with ecore models. It may well be that this has never worked perfectly. Maybe I don't understand it fully, yet.

> c) Offer some advice on how to proceed to a fix/workaround

First we need to find out why EMFUtil does these checks...

Comment 7 Dan Pollitt 2012-01-26 10:31:18 EST

I'm still looking to be able to persist and access the eType of EAttributes and EReferences persisted to CDO.

The Ecore model instances I'm attempting to store are created using the org.eclipse.wst.wsdl to read in a WSDL and associated XMLSchema (it's this XMLSchema that is used to create the ECore which is why EDataTypes from the XMLTypePackage are used).

I've done the following:

Changed EMFUtil.isPersistent(EStructuralFeature feature) so that EcorePackage.eINSTANCE.getETypedElement_EType() is reported as a persistent feature. This does mean that I have an eType column now on the ereference and eattribute tables in the database. This column is populated after adding in an Ecore model, however it looks like the values in the eType column of eattribute are foreign keys to entries in the cdo_external_refs table.

Changed org.eclipse.emf.cdo.internal.server.Repository.initSystemPackages() to add an additional package for XMLTypePackage:
units.add(initSystemPackage(XMLTypePackage.eINSTANCE));
I don't think this should be needed but just trying things...

Added the XMLTypePackage.eINSTANCE to the package registry of my org.eclipse.emf.cdo.session.CDOSession in client code.

However I still see the same behaviour when I open a CDO Editor raised from the CDO Session - the EType for EAttributes and EReferences of EClasses in the model are reported as null in the properties view.

Looking at the Package Registry dialog on the CDOSession reports the http://www.eclipse.org/emf/2003/XMLType package to be:

State: LOADED
Type: LEGACY
Original: LEGACY

Can someone suggest a starting point for debugging where attempting to navigate from an EAttribute or EReference to it's EType occurs?

Thanks,
Dan

Comment 8 Eike Stepper 2012-06-05 07:30:35 EDT

Moving all open bug reports to 4.1 because the release is very near and it's hghly unlikely that there will be spare time to address 4.0 problems.

Please make sure that your patches can be applied against the master branch and that your problem is not already fixed there!!!

Comment 9 Eike Stepper 2012-08-14 22:56:28 EDT

Moving all open issues to 4.2. Open bugs can be ported to 4.1 maintenance after they've been fixed in master.

Comment 10 Christian Damus 2012-09-08 11:53:06 EDT

Solving this problem will be necessary to support UML Profiles in legacy mode.  A profile can be defined and stored in the repository before any of its stereotypes are applied to a model.  Only then would CDO copy the profile's Ecore definition to the package registry, and by then the eTypes would all have been lost.

Comment 11 Christian Damus 2012-09-08 12:01:16 EDT

Ed Merks made a comment on the newsgroup that may shed some light on this issue.

The eType reference and eGenericType containment reference are mutually derived in ETypedElements.  At run-time, the eType is actually stored as a reference in a contained EGenericType instance.  However, in the most common case that the EGenericType simply references a naked EClassifier (no generic type parameters or wildcards involved), then Ecore serializes the ETypedElement::eType reference and omits the EGenericType object.

Perhaps the EMFUtil's decision to exclude eType from persistence was based on the fact that the EGenericType holds the reference, so it would be redundant.  But then the question is why the EGenericType isn't persisted, either.

Comment 12 Ed Merks 2012-09-09 01:34:16 EDT

I expect it's good to ignore those three features given that they can always be derived from the corresponding eGeneric* versions of those feature and, moreover, given that those features generally can store more information, i.e., the derivation from eGenericType to eType is lossy which is by design because it represents the erasure of generics.

It seems likely though that the rest of CDO will consider eIsSet before storing the generic versions of those features and when a model makes no actual use of generics, i.e., when the generic type and the erased type are the same, those will all return false.

We can see other places that we need special case handling such as in BasicChangeRecorder

  protected boolean shouldRecord(EStructuralFeature feature, EObject eObject)
  {
    return isRecording() &&
      !feature.isDerived() &&
      (isRecordingTransientFeatures() || !feature.isTransient()) &&
      feature != EcorePackage.Literals.ECLASS__ESUPER_TYPES &&
      feature != EcorePackage.Literals.ETYPED_ELEMENT__ETYPE &&
      feature != EcorePackage.Literals.EOPERATION__EEXCEPTIONS &&
      feature != EcorePackage.Literals.ECLASSIFIER__INSTANCE_CLASS_NAME;
  }

where we really don't want to record changes to the features derived from erased types.  Note that it has one more case, i.e., the instance class name is derived via erasure from the instance type name.

So I suspect that somewhere there needs to be a special case for the corresponding generic versions of the above to persist the (non-null/non-empty) value even when eIsSet returns false.

Comment 13 Eike Stepper 2012-09-14 04:45:46 EDT

Ed and I could hunt down a couple issues in the legacy code...

Comment 14 Eike Stepper 2012-09-14 07:28:21 EDT

Fixed.

I also fixed an issue with eUnsetting primitive-typed EAttributes.

commit 86b9f0f9b6f0ade19472328e86b899f58838fd0a

Comment 15 Christian Damus 2012-09-14 14:35:42 EDT

Thanks!  I can confirm that your fix works for me.

Comment 16 Dan Pollitt 2013-01-19 04:27:52 EST

Could this fix be ported back to the 4.1 stream? I'd like to employ this functionality in an app using Juno/EMF 2.8...

Comment 17 Eike Stepper 2013-01-22 01:17:41 EST

Ok ;-)

Port to 4.1 via bug 398704.

Comment 18 Dan Pollitt 2013-01-22 06:41:04 EST

That's fantastic - much appreciated, thank you.

Comment 19 Eike Stepper 2013-06-27 03:31:42 EDT

Available in R20130613-1157 (4.2)