eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

OCL's AnyType classifier make org.eclipse.emf.ecore.util.BasicExtendedMetaData.createEClassifierExtendedMetaData(EClassifier) crash #2204

Closed eclipse-ocl-bot closed 1 month ago

eclipse-ocl-bot commented 1 month ago

| --- | --- | | Bugzilla Link | 576583 | | Status | RESOLVED WONTFIX | | Importance | P3 normal | | Reported | Oct 12, 2021 12:51 EDT | | Modified | Oct 13, 2021 07:35 EDT | | Reporter | Vincent HEMERY |

Description

With a target platform based on Eclipse 2021-06, a ClassCastException occurs in org.eclipse.emf.ecore.util.BasicExtendedMetaData.createEClassifierExtendedMetaData(EClassifier) when the http://www.eclipse.org/ocl/1.1.0/oclstdlib.ecore package has been loaded in registry. (while calling org.eclipse.emf.edit.provider.ReflectiveItemProvider.gatherAllMetaData(EObject))

Indeed, this package contains the OclAny classifier, of type org.eclipse.ocl.ecore.impl.AnyTypeImpl. This is an EClassifier, but neither an EClass nor an EDataType, which is not expected by org.eclipse.emf.edit.provider.ReflectiveItemProvider...

I'm not sure whether we should make org.eclipse.emf.edit.provider.ReflectiveItemProvider more robust on this or fix the OclAny EClassifier...

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Oct 12, 2021 13:43

There are many observations to make.

You provide a filletted stack trace making the crash flow needlessly hard to follow. The local call of createEClassifierExtendedMetaData and the michanism needing ItemProviders are missing. Paste stack traces.

http://www.eclipse.org/ocl/1.1.0/oclstdlib.ecore should never be loaded. It is for modeling purposes.

Traversals of the EPackage.Registry should always be silently tolerant of bad models. Many projects have dubious registrations. (Such traversals are very undesirable practice since they load the world.)

http://www.eclipse.org/ocl/1.1.0/oclstdlib.ecore has been pretty stable for 10 years so is unlikely to change.

http://www.eclipse.org/ocl/1.1.0/oclstdlib.ecore demonstrates the wisdom of Ed Merks' guidance; never extend Ecore. The new Pivot-based OCL avoids this bad practice.

Whether ReflectiveItemProvider should be more robust is debateable. Extension of Ecore is not really supported so the problem is bad derived code, for which a derived BasicExtendedMetaData should provide a createEClassifierExtendedMetaData override. No idea whether it is available or whether you just neglected to install it. Which returns to the need for traversals to be silently tolerant.

eclipse-ocl-bot commented 1 month ago

By Vincent HEMERY on Oct 13, 2021 05:46

Created attachment 287310 Test Case producing the ClassCastException

My stack trace is not relevant. It doesn't provide enough information, and too many custom classes are in stake.

On the other hand, I tried and made a small Junit Plugin test case to reproduce it (with much more relevant information and easy to reproduce).

What I ended up discovering is that :

So my question is : Is it legal to make direct use of "EPackage.Registry.INSTANCE" in a ResourceSet, or should on always use "new EPackageRegistryImpl(EPackage.Registry.INSTANCE)" ?

If it is legal, then there definitely is an issue (either in OclStdLib, in EMF Cloud Jackson or in ReflectiveItemProvider). If not, then it's only my mistake.

:compression: 576583.zip

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Oct 13, 2021 07:35

(In reply to Vincent HEMERY from comment #2)

Created attachment 287310 [details] Test Case producing the ClassCastException

Thanks. Reproduces as a JUnit plugin test. Minor simplification and it reproduces as a direct JUnit test.

  • The OclStdLib package is loaded by EMF Cloud Jackson which loads all packages from descriptors in the global package registry.

This is a bad unscalable practice, but not the real problem.

  • The ClassCastException in ReflectiveItemProvider occurs only because I make direct use of "EPackage.Registry.INSTANCE"

No. The CCE is because AnyTypeImpl inherits EClassifierImpl that is not supported by ReflectiveItemProvider.

1) 5 Ecore OCL inheritances from EClassifierImpl could be changed to EDataTypeImpl. But this is very stable code that often causes pain and surprises when changed.

2) Don't call ReflectiveItemProvider \ a) ensure that the Ecore OCL registrations are in place via org.eclipse.ocl.ecore.provider.EcoreItemProviderAdapterFactory so that you use AnyTypeItemProvider.

b) accept that ReflectiveItemProvider is inadequate and catch and ignore the exception.

Is it legal to make direct use of "EPackage.Registry.INSTANCE" in a ResourceSet

You're just doing what happens anyway be delegation after checking the ResourceSet specific override. So your code gives a small performance improvement at the expense of breaking EMF semantics since any local registration will be redirected to global.

In general a ResourceSet is the point at which diverse EMF applications share their models. If you have a custom ResourceSet you run into trouble with any other application that needs a differently custom ResourceSet.

I strongly recommend never extending ResourceSetImpl; just use its eAdapters for extension.