eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[pivot] allInstances/implicit opposites domain may be more than a Resource #2147

Closed eclipse-ocl-bot closed 2 weeks ago

eclipse-ocl-bot commented 2 weeks ago

| --- | --- | | Bugzilla Link | 570894 | | Status | RESOLVED FIXED | | Importance | P3 normal | | Reported | Feb 04, 2021 00:39 EDT | | Modified | Jun 01, 2021 11:10 EDT | | See also | 543221, 529372, 570995, 571721 | | Reporter | Deniz Eren |

Description

Created attachment 285439\ Example model and profiles

Problem found regarding Papyrus static profile generated Java; OCL expression does not detect stereotypes applied to imported package elements.

Please find attached tar file containing example Papyrus static profile project "com.validationproblem.profile" and application "ValidationProblemApp" model ValidationProblem7.tar.gz.

The profile code com.validationproblem.profile contains the resources directory, which contains a profile directory with ValidationProblem.profile and a library directory with ValidationProblem-Library models.

The ValidationProblem-Library uml model applies the ValidationProblem.profile and defines a single class called LibraryChicken with the stereotype Chicken applied to it.

The application project ValidationProblemApp defines several classes that either apply the stereotype Chicken or Duck; one of the Chicken applied classes however is the LibraryChicken that is imported from the ValidationProblem-Library.

The profile com.validationproblem.profile defines the Farm stereotype with a number of derived properies, that are summarised here and you can find in the attached tar file projects:

/animal: Animal [0..*]\ let supplierElement : Set(UML::NamedElement) =\ Has.allInstances()->select(base_Dependency.client->includes(self.base_NamedElement)).base_Dependency.supplier->asSet()\ in Animal.allInstances()->select(supplierElement->includes(base_NamedElement))

/chicken: Chicken [0..*]\ self.animal->selectByKind(Chicken)

/duck: Duck [0..*]\ self.animal->selectByKind(Duck)

/allAnimals: NamedElement [0..*]\ Has.allInstances()->select(base_Dependency.client->includes(self.base_NamedElement)).base_Dependency.supplier->asSet()

/allAnimalsEverywhere: Animal [0..*]\ Animal.allInstances()

In the ValidationProblemApp we have defined classes:\ farm <> -> chicken1 <>\ farm <> -> chicken2 <>\ farm <> -> LibraryChicken <>\ farm <> -> duck1 <>

I have represented Dependency elements with stereotype "Has" applied as "->" above.

In the ValidationProblemApp we can see that the stereotype derived properies have the following values:\ /animal: Animal [0..] = {chicken1, chicken2, duck1}\ /chicken: Chicken [0..] = {chicken1, chicken2}\ /duck: Duck [0..] = {duck1}\ /allAnimals: NamedElement [0..] = {chicken1, duck1, LibraryChicken, chicken2}\ /allAnimalsEverywhere: Animal [0..*] = {chicken1, chicken2, duck1}

As you can see all this is pointing to the fact that the OCL expression Animal.allInstance() does not return "LibraryChicken"!

Note: ValidationProblem-Library Package is imported as PackageImport and class LibraryChicken is imported as ElementImport to no avail - still LibraryChicken doesn't get discovered by Animal.allInstances().

:compression: ValidationProblem7.tar.gz

eclipse-ocl-bot commented 2 weeks ago

By Ed Merks on Feb 04, 2021 00:45

I'm not sure this is a Papyrus problem or perhaps a UML2 problem, but I don't see an Ecore model so I can't comment on it.

And for future reference, if there is a problem that involves UML in any way to reproduce it, don't report it directly to EMF.

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 04, 2021 00:47

Hi Ed,\ It's confusing for me because I don't know exactly which system these static profile generating Java code issues would fall under. Should I report them as UML issues?

Best regards,\ Deniz

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 04, 2021 03:44

Almost certainly OCL.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 04, 2021 03:52

(In reply to Deniz Eren from comment #2)

It's confusing for me because

Yes. The general principle is if you don't know, make a guess and the 'wrong' project can move it to a batter guess. The one liner comments on the Forum are good since I get to pull the Bugzilla to OCL rather than waiting for someone else to push to OCL. I am subscribed to all OCL Bugzillas (and OMG OCL and UML issues) but not to all Papyrus or UML or EMF Bugzillas. I subscribe to the papyrus newsgroup because a lot of user problems have an OCL aspect.

As a rough guide:

anything to do with the UX is Papyrus

anything to do with model definition that can be reproduced in the UML Model Editor is UML2 but possibly EMF

anything to do with model 'execution' or OCL parsing is probably OCL but perhaps UML2 or EMF.

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 04, 2021 05:13

Thank you for pointers Ed, that helps.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 06, 2021 18:35

(In reply to Deniz Eren from comment #0)

In the ValidationProblemApp we have defined classes: farm <> -> chicken1 <> farm <> -> chicken2 <> farm <> -> LibraryChicken <> farm <> -> duck1 <>

Once I get the URI mappings right my test hraness executes Farm.animal to gove 3 (omitting LibraryChicken) animals as reported.

This turns out to be a legacy behaviour, the Pivot's LazyModelManager has the same comment and code as Classic OCL's LazyExtentMap.

/**

and ends up searching down from context.eResource().getContents().


Why one Resource rather than the whole ResourceSet, which includes all the meta-Rsources? Should Class.allInstances() return each meta-Class?

?? could model and metamodel resources be partitioned within/between ResourceSets ??


Why not follow the ElementImport? Not an Ecore functionality.

Why not follow all references? slow and may climb via Stereotypes into metaResources.


For QVTc/i/r where there are multiple input / output domains, each allInstances is scoped by the many Resources in a TypedModel.

?? just need to partition the ResourceSet into the model and metamodel 'TypedModel'. A utility/library model might be in both. ??

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 06, 2021 21:03

Ed, I checked the OCL standards but couldn’t find any directions for any such segregation; so I would say the standards expectation is that if a type is referenced and accessible by the model then it must be returned by the allInstances. Keep in mind also that extension_* returns undefined for objects from imported libraries.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 07, 2021 01:58

(In reply to Ed Willink from comment #6)

just need to partition the ResourceSet into the model and metamodel 'TypedModel'

Something better is clearly necessary since EMF allows use of a hierarchy of composed Resources for a big model.

Could try to follow all transitive non-meta-references (? meta-references are perhaps only provided by xsi:type / eClass()?)

But a more arbitrary family of Resources is acceptable. If I manually load 10 Resources that have no inter-references, surely instances from all 10 should contribute to allInstances()?

Workaround today is to create a fake parent Resource to compose all the roots.

Need to experiment with isolating the meta-Resources.

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 07, 2021 03:39

(In reply to Ed Willink from comment #8)

(In reply to Ed Willink from comment #6)

Workaround today is to create a fake parent Resource to compose all the roots.

Can you give example code; how would I manually load a Resource?

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 07, 2021 03:55

Bug 543221 and Bug 529372 claim that caching was improved.

A new EcoreExecutorManager and ModelManager are created at each validated element. This occurs despite occuring in a test case that can use a PivotDiagnostician.BasicDiagnosticWithRemove to control what is happening.

For a static profile we should really be able to do much much better. The QVTd transformation analysis can be emulated to identify at compile time what allInstances() and what implicit opposites (extension_*) are used by the compiled OCL. Some metadata in xxxTables can provide thsi.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 07, 2021 03:59

(In reply to Deniz Eren from comment #9)

Can you give example code; how would I manually load a Resource?

Sorry if you don't understnad this my time is better spent fixing the bug rather than explaining a dubious workaround.

If you don't understand this I am again worried that you are pursuing a crazy approach trying to use Stereotypes as Classes and Classes as Instances rather than Classes as Classes and Instances as Instances. Are you aware of the InstanceSpecification class?

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 07, 2021 04:11

(In reply to Ed Willink from comment #10)

A new EcoreExecutorManager and ModelManager are created at each validated element. This occurs despite occuring in a test case that can use a PivotDiagnostician.BasicDiagnosticWithRemove to control what is happening.

The instrumentation from Bug 529372 is still there and testValidate_Bug543187_xmi still uses it, so at least the introduced caching is still working. We have a new code path that evades caching.

Validating the user model creates

30 off AbstractModelManager\ 42 off ExecutorManager

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 07, 2021 04:26

(In reply to Ed Willink from comment #12)

We have a new code path that evades caching.

The old path was to PivotExecutorManager installs a lazy cache via a ResourceSet Adapter:

Thread [main] (Suspended (breakpoint at line 169 in ExecutorManager)) \ PivotExecutorManager$Adapter(ExecutorManager).(CompleteEnvironment) line: 169 \ PivotExecutorManager$Adapter(PivotExecutorManager).(EnvironmentFactory, EObject) line: 121 \ PivotExecutorManager$Adapter.(EnvironmentFactory, ResourceSet, EObject) line: 93 \ PivotExecutorManager$Adapter.(EnvironmentFactory, ResourceSet, EObject, PivotExecutorManager$Adapter) line: 92 \ PivotExecutorManager.createAdapter(EnvironmentFactory, EObject) line: 55 \ PivotUtil.basicGetExecutor(EObject, Map<Object,Object>) line: 246 \ OCLValidationDelegate.validatePivot(EClassifier, Object, DiagnosticChain, Map<Object,Object>, String, String, int) line: 308 \ OCLValidationDelegate.validate(EClass, EObject, Map<Object,Object>, String, String) line: 242 \ OCLValidationDelegateFactory$Global(OCLValidationDelegateFactory).validate(EClass, EObject, Map<Object,Object>, String, String) line: 77 \ EObjectValidator.validate(EClass, EObject, DiagnosticChain, Map<Object,Object>, String, String, String, int, String, int) line: 227

The new path is to EcoreExecutorManager which doesn't install itself:

Thread [main] (Suspended (breakpoint at line 169 in ExecutorManager)) \ EcoreExecutorManager(ExecutorManager).(CompleteEnvironment) line: 169 \ EcoreExecutorManager.(Object, ExecutorStandardLibrary) line: 65 \ PivotUtil.getExecutor(EObject) line: 1251 \ FarmImpl.getAnimal() line: 159 \ FarmImpl.eIsSet(int) line: 498 \ FarmImpl(BasicEObjectImpl).eIsSet(EStructuralFeature) line: 1280 \ ECrossReferenceEList$ResolvingFeatureIteratorImpl(EContentsEList$FeatureIteratorImpl).hasNext() line: 437 \ ECrossReferenceEList$ResolvingFeatureIteratorImpl(EContentsEList$FeatureIteratorImpl).next() line: 595 \ EObjectValidator.validate_EveryProxyResolves(EObject, DiagnosticChain, Map<Object,Object>) line: 747 \ EObjectValidator.validate_EveryDefaultConstraint(EObject, DiagnosticChain, Map<Object,Object>) line: 351 \ EObjectValidator$1(EObjectValidator$DynamicEClassValidator).validate(EClass, EObject, DiagnosticChain, Map<Object,Object>) line: 1426 \ EObjectValidator.validate(EClass, EObject, DiagnosticChain, Map<Object,Object>) line: 333

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 07, 2021 05:03

(In reply to Ed Willink from comment #11)

(In reply to Deniz Eren from comment #9)

Can you give example code; how would I manually load a Resource?

Sorry if you don't understnad this my time is better spent fixing the bug rather than explaining a dubious workaround.

No problem, I agree.

If you don't understand this I am again worried that you are pursuing a crazy approach trying to use Stereotypes as Classes and Classes as Instances rather than Classes as Classes and Instances as Instances. Are you aware of the InstanceSpecification class?

I’ve uploaded the failing examples, my use cases are not any different. Thanks for your work, let me know if you want me to retest anything.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 07, 2021 05:36

Ineffective caching by EcoreExecutorManager is Bug 570995.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 07, 2021 08:44

Trivial fix to treat enture ResourceSet as the extent available at

https://ci.eclipse.org/ocl/job/ocl-branch-tests/251/artifact/targetPlatform

Not pushed to master till we have caching via Bug 570995

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 07, 2021 19:00

Ed,\ With this fix version ocl-branch-tests #251 (6.14.0.v20210207-1253) - this Bug case works!

The results work for both allInstances() and extension_* use-case example tested in the sample model I had posted.

The new results look just as we would expect:

/animal: Animal [0..] = {chicken1, chicken2, duck1, LibraryChicken}\ /chicken: Chicken [0..] = {chicken1, chicken2, LibraryChicken}\ /duck: Duck [0..] = {duck1}\ /allAnimals: NamedElement [0..] = {chicken1, duck1, LibraryChicken, chicken2}\ /allAnimalsEverywhere: Animal [0..*] = {chicken1, chicken2, duck1, LibraryChicken}

Previously the LibraryChicken was missing from allInstances() and extension_* use-cases:

/animal: Animal [0..] = {chicken1, chicken2, duck1}\ /chicken: Chicken [0..] = {chicken1, chicken2}\ /duck: Duck [0..] = {duck1}\ /allAnimals: NamedElement [0..] = {chicken1, duck1, LibraryChicken, chicken2}\ /allAnimalsEverywhere: Animal [0..*] = {chicken1, chicken2, duck1}

You have definitely pin-pointed the source of this issue - thank you for your work.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 11, 2021 04:47

(In reply to Ed Willink from comment #6)

Why one Resource rather than the whole ResourceSet, which includes all the meta-Rsources? Should Class.allInstances() return each meta-Class?

Rubbish. The test harness re-used the ResourceSet for both Java synthesis and subsequent testing. It was therefore bloated by the synthesis resources.

Once a distinct ResourceSet is used the main UML resources are not present just:

platform:/resource/_OCL_UsageTests__testBug570894_uml/Bug570894.uml pathmap://VALIDATIONPROBLEM_LIBRARY/ValidationProblem-Library.uml\ pathmap://UML_LIBRARIES/UMLPrimitiveTypes.library.uml\ pathmap://UML_PROFILES/Ecore.profile.uml\ pathmap://VALIDATIONPROBLEM_PROFILE/ValidationProblem.profile.uml

Each of which is transitively imported from the root load. UMLPrimitiveTypes.library.uml is the consequence of gratuitous elementImports; it vanishes when the import is removed. Surely it should be a oompiled content? Yes, but it is so tiny that UML2 seems to have chosen not to provide a ststic version rather its individual elements are mapped to the Types package. An import of pathmap://UML_LIBRARIES/UMLPrimitiveTypes.library.uml is therefore user bloat.

Conclusion: there should be no serious downside to expanding the extent from Rssource to ResourceSet.


Note that if an allInstances() is compiled into a static profile, the search for an Executor via the ResourceSet may fail. The switch to a ThreadLocal in Bug 570995 should solve this.

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 11, 2021 19:27

Created attachment 285528 Bug demo profile file

I simplified the model, removed unnecessary items to focus on the issue I'm replicating at much as possible.

Firstly type Farm inherits FarmElement, which extends UML::Element, thus has base_Element property. Farm itself then extends UML::NamedElement, thus has base_NamedElement property, which then has Redefined property base_Element from FarmElement.

From within the properties of Farm, when referencing to self.base_Element, we get the Java error in the compiled code, requiring manual casting of an expression to NamedElement type:

+/animal_fails: FarmElement[*] -> Has.allInstances()->select( base_Dependency.client->includes( self.base_NamedElement ) ).base_Dependency.supplier.extension_FarmElement->asSet()

Java Error: Type mismatch: cannot convert from Element to NamedElement

Code: final /@NonInvalid/ NamedElement base_NamedElement = this.getBase_Element();

Instead when we refer to self.base_Element in the same expression there are no Java Errors requiring casting to NamedElement etc:

+/animal_works: FarmElement[*] -> Has.allInstances()->select( base_Dependency.client->includes( self.base_Element ) ).base_Dependency.supplier.extension_FarmElement->asSet()

I then tried to narrow down this issue with simpler OCL expressions by defining derived properties called animal1, animal2 and animal3, but they all worked fine, so it's a mystery to me why animal_fails property fails...

+/animal1: FarmElement[*] -> Has.allInstances().base_Dependency.supplier.extension_FarmElement->asSet()

+/animal2: FarmElement[*] -> UML::NamedElement.allInstances().extension_FarmElement->asSet()

+/animal3: FarmElement[*] -> UML::Element.allInstances().extension_FarmElement->asSet()

ValidationProblem.profile.uml

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 11, 2021 19:43

Typo earlier; referring to self.base_NamedElement in animal_fails is a problem I meant, referring to self.base_Element results in no Java errors in animal_works.

eclipse-ocl-bot commented 2 weeks ago

By Deniz Eren on Feb 11, 2021 20:31

Sorry Ed, I put these last two comments in the wrong Bugzilla ticket! I'll put them in the correct one, but not sure if I can delete these..

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 13, 2021 04:07

Once all ModelManagers use the new LazyEcoreModelManager rather than LazyModelManager and once LazyEcoreModelManager supports primarily EClass instances but also ClassId instances, test_stereotyped_allInstances_382981 is a problem, since it relies on a context object subselecting the Resources in their ResourceSet to scope to user M1 / user M2 excluding UML.

Now a targetted test for 5 hits fails with hundreds.

? abandon test\ ? special non-standard extent \ ? test for contains at least rather than exact

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 13, 2021 04:58

(In reply to Ed Willink from comment #22)

? special non-standard extent

Not a problem. Test already runs under a MyOCL. Easy enough to do

ocl3.setModelManager(new LazyEcoreModelManager(ocl3.mm.asEnglishClass.eResource().getContents(), null, null));

to construct a ModelManager with a custom extent. It's standard API, so we are testing that user could do it too.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Feb 22, 2021 13:33

Pushed to master for 6.14.0M3.

eclipse-ocl-bot commented 2 weeks ago

By Ed Willink on Jun 01, 2021 11:10

(In reply to Ed Willink from comment #22)

test_stereotyped_allInstances_382981 is a problem

So too is testConsole_UML, but once Bug 571721 provides per-thread EnvironmentFactory, the test no longer does allInstances() on a proxy (oops), so that with a ResourceSet Stereotype.allInstances() now retirns all the UML profiles.

Could chnage the expected results but(In reply to Ed Willink from comment #23)

(In reply to Ed Willink from comment #22) Test already runs under a MyOCL.

no MyOCL available but with a bit of work a testEnvironmentFactory can be used that imposes a single root LazyEcoreModelManager.