Bug 337792

Comment 1 Ed Willink 2011-02-22 02:44:33 EST

Created attachment 189459 [details]
Enhanced ValidationDelegate API

For reference:

The corresponding OCL in Ecore support recognises an EAnnotation.Detail key with a $message suffix as a String-valued OCL expression for the custom message to accompany the Boolean-valued OCL expression for the validation.

The corresponding OCL in Ecore concrete syntax supports:

invariant InvariantName(StringValuedExpression): BooleanValuedExpression;

The StringValuedExpression may use variables from the BooleanValuedExpression.

Comment 2 Ed Merks 2011-05-12 16:59:06 EDT

This approach also breaks an aspect of the EObjectValidator's design, i.e., all diagnostics are created by calls to overrideable methods and all translated Strings are loaded by calls like getString so that in all cases, it's possible to override what's presented to the client. This patch bypasses that and that seems like a bad thing...

Could you explain how the OCL design you've outlined supports translation to different locales?  It's hard to imagine how that's done without *.properties files...

Comment 3 Ed Willink 2011-05-13 01:17:40 EDT

(In reply to comment #2)
> Could you explain how the OCL design you've outlined supports translation to
> different locales?  It's hard to imagine how that's done without *.properties
> files...

The OCL language as specified by OMG has no support for internationalization; just like Java.

Java is internationalized in Eclipse by NLS.bind, which it is the responsibility of the application programmer or tool chain to invoke. The same could be true in OCL once a counterpart NLS.bind facility is provided. So rather than the OCLinEcore

invariant MyInvariant(
    'The length of ' + name + ' is only ' + name.size() + ' characters'):
        name.size >= 8;

the internationalizable user could write 

invariant MyInvariant('MyInvariantMessage'.bind(name, name.size())):
        name.size >= 8;

and String::bind() would be responsible for instantiating the MyInvariantMessage template.

So when the responsibility for providing the message is handed to the user by the proposed API enhancement, the responsibility for internationalization is as well.

Comment 4 Ed Willink 2013-09-03 09:41:18 EDT

Is there any chance of some progress on this?

I get a steady stream of of users who fail to read the documentation expliaing how they must manually change the generated Validator class manually to extend OCLinEcoreEObjectValidator. I expect this to get worse now that CEA LIST are promoting the custom messages for UML/Papyrus and Obeo/Airbus are promoting them for Topcased migration.

I could workaround this by providing a custom ValidatorClass.javajet and a custom GenModel Generator that diagnoses the failure to use the custom template, but this is just avoiding a useful enhancement to EMF.

Please support custom messages (and severities).

Comment 5 Ed Merks 2013-10-11 04:06:02 EDT

So the proposal is simple and nondisruptive, which is very nice and I like that, but as I said, it bothers me a lot that currently all diagnostic creation is funneled through the following EObjectValidator method

  protected BasicDiagnostic createDiagnostic
    (int severity, String source, int code, String messageKey, Object[] messageSubstitutions, Object[] data, Map<Object, Object> context)
  {
    String message =
      DIAGNOSTIC_SOURCE.equals(source) ?
        getEcoreString(messageKey, messageSubstitutions) :
        getString(messageKey, messageSubstitutions);
    return new BasicDiagnostic(severity, source, code, message, data);
  }

and that also funnels all text fetching (key-to-translated-message mapping) though those get*String methods.  

If you look at validateEClassifier_WellFormedInstanceTypeName you can see that even this is designed to do that same delegation, merely for the purpose of ensuring the delegation is in place.

So I keep thinking there must be some design where the delegate would call out to these methods.  Maybe some approach similar to SubstitutionLabelProvider where the API for the call out is available via the context. Of course this is further complicated by the fact that the delegate itself specifies the message key and must provide the key-to-translated-message mapping in a way that still allows a derived generated validator to intercept the mapping process...

I'll study the issue further and see if I can come up with something clean like what you proposed, while preserving the delegation pattern...

Comment 6 Ed Merks 2013-10-11 05:00:37 EDT

Hmmm, in fact I see that all the report*Delegate* method don't delegate properly to the createDiagnostic method.  That could be fixed, but no one has complained to date either...

Comment 7 Ed Willink 2013-10-11 05:43:35 EDT

(In reply to Ed Merks from comment #5)
> I'll study the issue further and see if I can come up with something clean
> like what you proposed, while preserving the delegation pattern...

Thanks. While you're thinking in this area you might care to consider a couple of related limitations.

Validation has limited extensibility. So Xtext introduces a CompositeEValidator. It would be good to make this part of the standard API, so that anyone can promote the installed EValidator into a Composite and thereafter everyone extends the one Composite.

The validation registry has no two-level counterpart like the package and factory registries allowing applications to add/compose/replace validation locally without affecting global registrations.

Comment 8 Ed Merks 2013-10-11 06:08:32 EDT

Sorry, but I'm not sure specifically what you want from CompositeEValidator that isn't already that's not provided by Diagnostician.  In fact, I'm not sure I fully understand what that pattern is doing and what part you feel must be moved to the core.  CompositeEValidator seems to be a composition of EValidator implementations that can each validate things from any model, rather then being a composition of model-specific validators which dispatches to the model-appropriate validator as Diagnostician does.  That's of course fine (and so is using dependency injection) but I don't see how EMF's generated validators fit into that type of arbitrary composition pattern, and I'm not sure that it's appropriate to introduce dependency injection into the core...

Via org.eclipse.emf.ecore.util.Diagnostician.Diagnostician(Registry) a diagnostician can be constructed with a local registry and via org.eclipse.emf.ecore.impl.EValidatorRegistryImpl.EValidatorRegistryImpl(Registry) a delegating registry with local overrides is supported, e.g., overrides for extended model-specific validators. Isn't that sufficient?

Comment 9 Ed Willink 2013-10-11 06:42:19 EDT

I'm certainly not advocating DI in EMF Core here (for EObject it would be wrong, for Resource and ResourceSet and AdapterFactory it might be interesting).

CompositeEValidator discussion continues in Bug 419215.

Comment 10 Lars-Ola Österlund 2019-12-03 14:04:26 EST

I try to generate custom messages but fail, started with the example from the library tutorial:
invariant SufficientCopies('There are '
+ library.loans->select((book = self))->size().toString()
+ ' loans for the ' + copies.toString() + ' copies of \'' + name + '\''):
library.loans->select((book = self))->size() <= copies;
Generated the code for the library tutorial and did a test run resulting in the built in report.
The documentation says the generated validator class must extend OCLinEcoreEObjectValidator instead of EObjectValidator so I changed that and added 
"import org.eclipse.ocl.xtext.oclinecore.validation.OCLinEcoreEObjectValidator;"
and got the error 
"The import org.eclipse.ocl.xtext cannot be resolved"
A bugreport 337792 has a warning "This class may go obsolete once Bug 337792 resolved".
Another solved bugreport 471948 says a call to OCLinEcoreEObjectValidator is now working.
Then I debugged the existing code and found that the EObjectValidator validate method has an expression string with a "Tuple …" containing the above message. But the message string is not transferred to the reportConstraintDelegateViolation method that generates the standard message.
Did I make a mistake somewhere or is the custom messaging not working?
I am using latest Eclipse (2019-09) and latest OCL downloads.
Would much appreciate help on this.
Lars-Ola Österlund

Comment 11 Ed Willink 2019-12-03 14:53:45 EST

(In reply to Lars-Ola Österlund from comment #10)
> Did I make a mistake somewhere or is the custom messaging not working?

The code is very much working.

https://git.eclipse.org/c/ocl/org.eclipse.ocl.git/tree/plugins/org.eclipse.ocl.pivot/model/Pivot.ocl

at line 420 has a ShadowExp::InitializesAllClassProperties which has a non-trivial error message that enumerates missed properties.

https://git.eclipse.org/c/ocl/org.eclipse.ocl.git/tree/tests/org.eclipse.ocl.examples.xtext.tests/models/ecore/ConstraintMessages.ecore

is part of the release test suite.

You appear to be starting from some seriously vintage documentation. THe current documentation refers to "org.eclipse.ocl.xtext" only as a prefix for a family of 12 plugins.

Please raise an OCL Bugzilla detailing exactly where you started from.