Bug 360072

Comment 1 Axel Uhl 2011-10-06 05:34:10 EDT

The problem seems related to the Pivot implementation. I can reproduce the NullPointerException. If I use the Sample Ecore Model Editor I change the annotations from .../OCL/Pivot into .../OCL then everything works fine. Without further analysis, I suppose that the Pivot evaluator implementation doesn't properly look for an OCL body to call but instead assumes that the respective operation exists on the EObject's Java class. This fails for dynamic EMF.

Comment 2 Axel Uhl 2011-10-06 05:49:29 EDT

Ah, here is what probably happens: When you use the "Interactive OCL Console", the Ecore (non-Pivot) evaluator is used. It can't find the operation body because it is not looking for the .../OCL/Pivot annotation URI. The annotation URI used by the Ecore evaluator is defined in OCLDelegateDomain (org.eclipse.emf.ecore.EcorePackage.eNS_URI + "/OCL"). With the OCLinEcore editor using the Pivot URI (org.eclipse.emf.ecore.EcorePackage.eNS_URI + "/OCL/Pivot"), InvocationBehavior.getOperationBody can't find the body expression.

Workarounds:

 - Either use the "Interactive Xtext OCL" console when defining your metamodels using the OCLinEcore text editors.

 - Or make sure your metamodels use org.eclipse.emf.ecore.EcorePackage.eNS_URI + "/OCL" as the delegate URIs as well as the body/constraint/derivation expression URIs which enables the Ecore evaluator to find them.

This bug suggests that working with two different delegate domains with different URIs is difficult for various reasons and may cause some confusion. I'm not sure what a good fix for this would be as there are also reasons in favor of using different URIs to distinguish between the two different OCL implementations with their sometimes subtle differences in semantics and execution behavior.

I would have expected the default OCLinEcore editor to still use the default, non-examples .../OCL URI and not the .../OCL/Pivot URI which leads to the OCL subsystem that is still under examples/. Maybe there should be two editor flavors as well, clearly indicating which OCL subsystem the resulting metamodels will be based on.

Comment 3 Ed Willink 2011-10-06 06:17:56 EDT

Well done. You've diagnosed the problem. See http://127.0.0.1:49757/help/topic/org.eclipse.ocl.doc/help/Delegates.html?path=18_5_8_1#OCLDelegateURI.

The triple URI is at least in Indigo a necessary preservation of Helios compatibility where there was no pivot evaluator. For Juno we will have to discuss whether we can remap the default .../OCL URI to .../OCL/Pivot rather than .../OCL/LPG.

However your problem clearly demonstrates that the current functionality can cause confusion.

We should either diagnose the mixed mode evaluation or convert it automatically to a mode that works.

Perhaps we should start populating the OCL preference page with options such as the project-specific choice of evaluator; then the OCLinEcore editor would not need to have its own built-in bias. [Of course not everything that the OCLinEcore editor edits can be re-parsed by the LPG parser; e.g. no interpretation of the import apparent in oclAsType(_'platform:/resource/my.project/model/Mine.ecore'::B::C).]

Comment 4 Ed Willink 2011-10-09 03:30:17 EDT

A delegate declaration has two aspects

a) choice of the evaluator
b) provision of the evaluation code

Since the evaluation code is Concrete Syntax OCL this is compatible between evaluators, except in very marginal troublesome corner cases requiring an import.

We could therefore solve this problem by changing e.g.

public boolean InvocationBehavior::appliesTo(EOperation operation) {
      	String annotation = EcoreUtil.getAnnotation(operation,
OCLDelegateDomain.OCL_DELEGATE_URI, BODY_CONSTRAINT_KEY);
		return annotation != null;
	}

Comment 5 Ed Willink 2011-10-09 03:39:48 EDT

(In reply to comment #4)
continuing

> We could therefore solve this problem by changing e.g.
> 
> public boolean InvocationBehavior::appliesTo(EOperation operation) {
>           String annotation = EcoreUtil.getAnnotation(operation,
> OCLDelegateDomain.OCL_DELEGATE_URI, BODY_CONSTRAINT_KEY);
>         return annotation != null;
>     }

to match 'any' OCL Delegate URI.

I suggest that 'any' is http://www.eclipse.org/emf/2002/Ecore/OCL with any /-prefixed suffix

i.e.

http://www.eclipse.org/emf/2002/Ecore/OCL
http://www.eclipse.org/emf/2002/Ecore/OCL/LPG
http://www.eclipse.org/emf/2002/Ecore/OCL/Pivot
http://www.eclipse.org/emf/2002/Ecore/OCL/someotherpurpose

but not

http://www.eclipse.org/emf/2002/Ecore/OCLsomeotherpurpose

This seems a safe enough fix for SR2.

Comment 6 Axel Uhl 2011-10-09 08:19:50 EDT

Sounds ok to me, also because the cached abstract syntax is specific to the delegate and therefore will match the metamodel expected.

Comment 7 Ed Willink 2011-10-09 13:11:20 EDT

bug/360072 branch supports mixed-mode delegate URIs, with tests to demonstrate
cross-invocation of the LPG/Pivot evaluator from the alternate invocation.

The tests are in the Pivot tests plugin since it can depend on LPG whereas LPG
cannot depend on Pivot.

Missing delegate standalone initialize routines are factored out.

Please review, then an SR2 patch without the extra routines, and probably
without the tests can be developed.

Comment 8 Adolfo Sanchez-Barbudo Herrera 2011-10-14 15:17:06 EDT

Ed,

One question:

Looking at OCLDelegateDomain::getDelegateAnnotation()... Does make sense having several annotations of different OCL delegate domain URIs ? Otherwise I don't understand the double loop...

Anyway looking at the o.e.o.ecore changes it looks like they don't change the current behaviour .... so feel free to include this changes in the master branch.

note: if you look for every where OCLDelegateDomain.OCL_DELEGATE_URI from ocl.ecore and OCLDelegateDomain.OCL_DELEGATE_URI_LPG/OCL_DELEGATE_URI_PIVOT from  examples.pivot are used using the JDT, you may find more places to exploit the new way to obtain the OCL delegate domain annotation, for instance:

org.eclipse.ocl.examples.impactanalyzer.benchmark.preparation.ocl.OCLExpressionFromModelPicker::searchAndParseExpressions()

Regards,
Adolfo.

Comment 9 Ed Willink 2011-10-15 14:34:08 EDT

(In reply to comment #8)
> Looking at OCLDelegateDomain::getDelegateAnnotation()... Does make sense having
> several annotations of different OCL delegate domain URIs ? Otherwise I don't
> understand the double loop...

The search is over all EAnnotations, not just OCL ones, so it seems worth cherry-picking the exact string match before the prefix match, which might go obsolete is we use a project preference to select evaluators.

> note: if you look for every where OCLDelegateDomain.OCL_DELEGATE_URI from
> ocl.ecore and OCLDelegateDomain.OCL_DELEGATE_URI_LPG/OCL_DELEGATE_URI_PIVOT
> from  examples.pivot are used using the JDT, you may find more places to
> exploit the new way to obtain the OCL delegate domain annotation, for instance:
> 
> org.eclipse.ocl.examples.impactanalyzer.benchmark.preparation.ocl.OCLExpressionFromModelPicker::searchAndParseExpressions()

Well spotted. Will do. Needs an isDelegateAnnotation() method too to support a further access from AbstractNavigationStep.java. This also fixes an NPE risk on a null EAnnotation.source which is legal.

Pushed to master.

Comment 10 Ed Willink 2011-10-15 16:39:29 EDT

bug/360072SR2 contains the rework for SR2.

Providing the tests and Impact Analyzer changes are challenging API-wise, so they're not provided. A little bit of cloning wouldn't be that hard though.

[The API checking seems unduly pedantic, but it seems that adding OCLDelegateDomain.getAnnotationDelegate as a public static requires filtering,
since pedantically another plugin using the public static would need a 3.1.2 rather than 3.1.0 dependency and maintenance API changes are not supported.]

Comment 11 Adolfo Sanchez-Barbudo Herrera 2011-10-17 05:17:44 EDT

(In reply to comment #9)
> (In reply to comment #8)
> > Looking at OCLDelegateDomain::getDelegateAnnotation()... Does make sense
> having
> > several annotations of different OCL delegate domain URIs ? Otherwise I don't
> > understand the double loop...
> 
> The search is over all EAnnotations, not just OCL ones, so it seems worth
> cherry-picking the exact string match before the prefix match, which might go
> obsolete is we use a project preference to select evaluators.

I'm not sure you have properly answered my question... I'll ellaborate the question in a different way...

Does make sense having a http://www.eclipse.org/emf/2002/Ecore/OCL/LPG and http://www.eclipse.org/emf/2002/Ecore/OCL/PIVOT sources of two eannotations on the same model element ?.... Perhaps it doesn't make sense, but it could be possible that somebody does that ?

> 
> [The API checking seems unduly pedantic, but it seems that adding
> OCLDelegateDomain.getAnnotationDelegate as a public static requires filtering,
> since pedantically another plugin using the public static would need a 3.1.2
> rather than 3.1.0 dependency and maintenance API changes are not supported.]

I think that new API is not expected for a SR .... just bug fixing of the current API, so I guess that API Tooling complains because of the new static methods.... I'm not against to the changes, though.

P.S: why OCLDelegateDomain from pivot misses the @since 3.0 annotation ? Don't the new static methods require the corresponding @since ? (If we add some new API non-expected-by-APITooling, I think that at least, we should comment when it was added).

Regards,
Adolfo.

Comment 12 Ed Willink 2011-10-17 14:31:53 EDT

(In reply to comment #10)
> Providing the tests and Impact Analyzer changes are challenging API-wise, so
> they're not provided. A little bit of cloning wouldn't be that hard though.

Indeed. Cloned routine added to the one IA access.

Please review bug/360072SR2.

Comment 13 Adolfo Sanchez-Barbudo Herrera 2011-10-21 05:37:43 EDT

Nice.

One last comment. In your last commit you changed the usage of OCLDelegateDomain.OCL_DELEGATE_URI. in

o.e.o.examples.impactanalyzer.instanceScope.AbstractNavigationStep.java

However there are more usages along the code.

Anyway, since it's for the maintenance stream, feel free to fix them up.

Cheers,
Adolfo.

Comment 14 Ed Willink 2011-10-21 05:50:55 EDT

(In reply to comment #13)
> However there are more usages along the code.

I think that you'll find that all other usages are in 'test' plugins; fixed on master; no change for maintenance.

Comment 15 Ed Willink 2011-10-21 06:33:00 EDT

Pushed to maintenance/R3_1 for SR2.

Comment 16 Ed Willink 2013-05-20 11:36:27 EDT

CLOSED after a year in the RESOLVED state.