[pivot] ValidityView fails to migrate to selected EnvironmentFactory

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 582722 | | Status | REOPENED | | Importance | P3 normal | | Reported | Dec 10, 2023 05:07 EDT | | Modified | Feb 22, 2024 03:25 EDT | | See also | 582494, 581266, 571721, 574041, 582958 |

[eclipse-ocl-bot]

By Ed Willink on Dec 10, 2023 05:12

https://www.eclipse.org/forums/index.php?t=rview&goto=1862533#msg_1862533 reports that the OCL functionality of the ValidityView is missing in 6.19.0 2023-12.

Running through the "Validation tutorial" hits a similar problem very rapidly. The load or ExtraEcoreValidation.ocl does not update the Ecore Editor.

(Probably a side effect of the move to a local validation registry see Bug 582494.)

[eclipse-ocl-bot]

By Ed Willink on Dec 10, 2023 07:09

(In reply to Ed Willink from comment #1)

Running through the "Validation tutorial" hits a similar problem very rapidly. The load or ExtraEcoreValidation.ocl does not update the Ecore Editor.

Something weird going on. 'repro' seems to come and go; many oppportunities for developer finger trouble.

An 'old' nested Eclipse on 6.19.0 shows many 'impossible' problems.

Creating a new nested Eclipse on 6.19.0 and the loads do display, but the validate errors are missing with Validate. OCL->Validate is greyed out.

Creating a new nested Eclipse using 6.18.0 on 6.19.0 and the loads display and again the validate errors are missing with Validate. OCL->Validate is greyed out.

[eclipse-ocl-bot]

By Ed Willink on Dec 10, 2023 07:38

(In reply to Ed Willink from comment #2)\

Something weird going on. 'repro' seems to come and go.

An OCL 6.17.0 (2021-11) installation on Eclipse 4.22 (2021-12) seems plausible OCL->Validate-wise.

(confusion: 6.17.0 has some x.18.0 content.)

(confusion: the first Validate content comes from ExtraXMIValidation.ocl, so loading just ExtraEcoreValidation.ocl is pointless.)

[eclipse-ocl-bot]

By Ed Willink on Dec 10, 2023 10:34

Why does OCL->Validate get greyed out on 6.19.0 (? and 6.18.0) but not 6.17.0?

Inherited functionality of org.eclipse.emf.edit.ui.action.ValidateAction changed.\ ---\ commit 36cee1a2afaf3f5c23290cf5acce02710534d448\ Author: Ed Merks ed.merks@gmail.com 2023-01-11 14:23:51

[581266] Validation action should be disabled until there is a selection

Otherwise we get this when the action is run without a selection:\ NullPointerException: Cannot invoke "java.util.List.size()" because\ "this.selectedObjects" is null\

EMF's ValidateAction is part of the old deprecated Action framework.

OCL's ValidateCommand reuses ValidateAction as part of the new Command/Handler framework.

EMF's ValidateAction is a top level menu entry. It can just be invoked with an empty selection provoking the fix.

OCL's re-use is second level so lazily created and so even harder to invoke with an empty selection.

We can probably just do a simple setEnabled(true) in the derived ctor. But we should see if anything can go wrong with the start up empty selection. (Instrumenting selectionChanged we only find activity in ValidateAction but nothing in ValidateHandler/Command).

(Is it ever possible to 'create' an empty selection other than by restarting? Yes. Treating the Ecore Editor bas a multi-selector ctrl-select can end up with zerio selections. EMF's Validate greys out accordingly. The whole OCL menu vanishes.)

[eclipse-ocl-bot]

By Steve Hickman on Dec 11, 2023 17:54

2023-09 with OCL 6.17.1 works

[eclipse-ocl-bot]

By Ed Willink on Dec 12, 2023 03:51

(In reply to Ed Willink from comment #1)

(Probably a side effect of the move to a local validation registry see Bug 582494.)

No. A side effect of the move to ThreadLocalExecutor. The old working code uses PivotUtilInternal.findEnvironmentFactory which, after failing to find a thread local EnvironmentFactory, falls back to search the deprecated Resource/ResourceSet eAdapters. The new broken code requires a thread local EnvironmentFactory as a pre-requisite to finding constraints.

Probable fix: just initialize the Validation View worker thread properly to re-use the UI thread EnvironmentFactory.

BUT. Why did no JUnit test find this?

The org.eclipse.ocl.examples.validity.test suite, although it has UI dependencies, has no UI tests; getting close to needing swtbot.

TODO: EValidatorConstraintLocator uses EValidator.Registry.INSTANCE. Probably needs local/global/local+global variants.

[eclipse-ocl-bot]

By Ed Willink on Dec 12, 2023 04:40

(In reply to Ed Willink from comment #6)

No. A side effect of the move to ThreadLocalExecutor. The old working code uses PivotUtilInternal.findEnvironmentFactory which, after failing to find a thread local EnvironmentFactory, falls back to search the deprecated Resource/ResourceSet eAdapters. The new broken code requires a thread local EnvironmentFactory as a pre-requisite to finding constraints.

Probable fix: just initialize the Validation View worker thread properly to re-use the UI thread EnvironmentFactory.

The old EnvironmentFactoryAdapter enabled sub-applications to re-use the OCL support via sharing of the ResourceSet with some other application. This could lread to concurrency surprises as the other application might not be properly synchronized. (Is any EMF / OCL code properly synchronized ever?) In summary there could be too few EnvironmentFactory creations to isolate OCL applications.

The new ThreadLocalExecutor, bug 571721, only supports sharing between sub-applications that share the same thread, and in the case of UI the UI thread, the same Part too. This prevents sub-applications sharing the OCL support and may lead to inadequate recreation of OCL support. To avoid this the sub-application must take care to configure its part-thread to share the support from the primary application's part-thread. In summary, there can be too many EnvironmentFactory creations and undesirably isolated OCL applications. Better/safer so we 'just' need to fixup a rough edge.

The ValidityView is a sub-application whose primary application may change as the selection changes between editors. How does the ValidityView convert the selection's ResourceSet to the appropriate EnvironmentFactory without using the old-style EnvironmentFactoryAdapter? Worse, the primary application may never have created an EnvironmentFactory, so how do we ensure that a 'tardy' primary application re-uses the EnvironmentFactory when first created by the sub-application?

Perhaps the ThreadLocalExecutorUI supports registration of unique/shared EnvironmentFactory for the UI thread. By default registration for a UI part-thread can re-use for the same ResoureceSet. Explicit registration of a unique EnvironmentFactory can only be shared when configured explicitly.

[eclipse-ocl-bot]

By Ed Willink on Feb 12, 2024 08:29

I took a break to work on Bug 582833 where Sirius demands to use its own ResourceSet, consequently the selection ResourceSet cannot be shared. The selection URI must therefore be re-loaded into the Sirius (sub-application) ResourceSet. We could do the same for the ValidityView and so avoid any additional VV loads impacting on the source ResourceSet.

[eclipse-ocl-bot]

By Ed Willink on Feb 14, 2024 11:00

(In reply to Ed Willink from comment #6)

Probable fix: just initialize the Validation View worker thread properly to re-use the UI thread EnvironmentFactory.

No. While it is attractive to avoid re-loading models / re-initializing OCL for the ValidityView, the VV is a concurrent sub-application so any mutation of parasitic resources such as UML profiles, or caches more generally is unsafe. The old ResourceSet EnvironmentFactoryAdapter usafe was unsage. The suggested new Thread to ResourceSet to EnvironmentFactiory mapping is similarly unsafe.

(The sharing of an EnvironmentFactory across threads is ok so long as there is just a single OCL flow of control that migrates between threads as workers/runnables sequence activity in background/foreground contexts.)

The concurrent VV cannot re-use the selected EObject/Resource/ResourceSet until we have threadsafe OCL.

The VV sub-application must be indistinguishable from a re-load from the URI of the selection. However we should clone the ResourceSet registries to ensure that re-loads use the correct ResourceFactory etc.

[eclipse-ocl-bot]

By Ed Willink on Feb 14, 2024 12:24

(In reply to Ed Willink from comment #6)

BUT. Why did no JUnit test find this?

The org.eclipse.ocl.examples.validity.test suite, although it has UI dependencies, has no UI tests; getting close to needing swtbot.

Testing the IDEValidityManager.ValidityViewJob needs swtbot. Promoting its internals that clone StandaloneValidityManager.runValidation to ValidityManager.runValidationshould enable the testStandaloneExecution_validate_xxx tests to do more.

[eclipse-ocl-bot]

By Ed Willink on Feb 15, 2024 11:33

Doh! There is a much simpler solution.

The problem is that the VV Thread has no EnvironmentFactory so the basicGet polled access uses null and so finds no OCL-based constraints. Changing the basicGet to a proper lazy creation and all is ok. Concurrency issues are a bit academic since the VV has its own OCL, although it does share the incoming EObjects. There are some attempts at synchronize.

Adjusting the standalone VV tests to wrap the extracted runValidation in a worker thread demonstrates that we get failures if the lazy get EnvironmentFactory fails.

Pushed to master for M3.

[eclipse-ocl-bot]

By Ed Willink on Feb 17, 2024 03:17

Not so simple. OCL constraints involving type functionality fail to resolve ids.

Perhaps the EnvironmentFactory tidy up went too far, see Bug 582947.

Reproducible using the Complete OCL Tutorial.

Load XMITestFIle.xmi\ OCL->Load ExtraEcoreValidation.ocl, ExtraXMIValidation.ocl\ OCL->Show Validity View\ ?? VV->Run

=> UnsupportedOperationException

Thread [Worker-2: Validity View Validation] (Suspended (exception UnsupportedOperationException)) \ UMLIdResolver(AbstractIdResolver).visitClassId(ClassId) line: 1672 \ UMLIdResolver(AbstractIdResolver).visitClassId(ClassId) line: 1 \ GeneralizedClassIdImpl.accept(IdVisitor) line: 78 \ UMLIdResolver(AbstractIdResolver).getType(TypeId) line: 1197 \ UMLIdResolver(AbstractIdResolver).getCollectionType(CollectionTypeId, boolean, IntegerValue, UnlimitedNaturalValue) line: 825 \ UMLIdResolver(AbstractIdResolver).getCollectionType(CollectionTypeId) line: 795 \ UMLIdResolver(AbstractIdResolver).visitCollectionTypeId(CollectionTypeId) line: 1689 \ UMLIdResolver(AbstractIdResolver).visitCollectionTypeId(CollectionTypeId) line: 1 \ SpecializedCollectionTypeIdImpl.accept(IdVisitor) line: 31 \ UMLIdResolver(AbstractIdResolver).getClass(TypeId, Object) line: 788 \ BasicEvaluationVisitor.visitIteratorExp(IteratorExp) line: 494 \ IteratorExpImpl.accept(Visitor) line: 2041 \ BasicEvaluationVisitor.visitOperationCallExp(OperationCallExp) line: 665 \ OperationCallExpImpl.accept(Visitor) line: 572 \ BasicEvaluationVisitor.evaluate(OCLExpression) line: 146 \ BasicOCLExecutor(AbstractExecutor).evaluate(OCLExpression) line: 215 \ ConstrainedProperty.evaluate(Executor, TypeId, Object) line: 67 \ BasicOCLExecutor(AbstractExecutor).internalExecuteNavigationCallExp(NavigationCallExp, Property, Object) line: 393 \ BasicEvaluationVisitor.visitPropertyCallExp(PropertyCallExp) line: 788 \ PropertyCallExpImpl.accept(Visitor) line: 445 \ BasicEvaluationVisitor.visitOperationCallExp(OperationCallExp) line: 669 \ OperationCallExpImpl.accept(Visitor) line: 572 \ PivotEObjectValidator$1(AbstractConstraintEvaluator).evaluate(EvaluationVisitor) line: 86 \ PivotEObjectValidator.validate(EnvironmentFactoryInternal, Constraint, Object, Map<Object,Object>) line: 333 \ PivotEObjectValidator.validate(Constraint, Object, Map<Object,Object>) line: 356 \ CompleteOCLCSUIConstraintLocator(CompleteOCLCSConstraintLocator).validate(Result, ValidityManager, Monitor) line: 131 \ IDEValidityManager(ValidityManager).runValidation(Set, IProgressMonitor) line: 582 \ IDEValidityManager$ValidityViewJob.run(IProgressMonitor) line: 94 \ Worker.run() line: 63