Bugzilla – Full Text Bug Listing
| Summary: | [Papyrus DSML Validation] The validation plugin generator silently ignores constraints without ValidationRule stereotype | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] Papyrus | Reporter: | Ansgar Radermacher <ansgar.radermacher> | ||||
| Component: | Others | Assignee: | Ansgar Radermacher <ansgar.radermacher> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | johan, klaas.gadeyne, papyrus-bugs, peter.cigehn, rschnekenburger | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| See Also: |
https://git.eclipse.org/r/60779 https://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=d3f91acb5d63c00dfe37b8cabf540a8fd3360029 https://git.eclipse.org/r/60785 https://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=56f509bd34c12e527ce69271da7f8b623ed79032 |
||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Ansgar Radermacher
Fixed with commit 6b8a3294808aaf6acff80b810989756ed8bc388e for the master. The generator tells the user that customization is possible via the DSML validation profile. All constraints are now taken into account. If the ValidationRule stereotype is not applied, a default message (<constraint named> not valid) and error severity is assumed. I have been searching for a long time why my first trials with the Papyrus DSML plugin always resulted in invalid constraints... till I finally discovered that this is the default message generated if you don't fill the optional message yourself. IMHO, the default message should be changed. * "Constraint1 not valid" means that the constraint has issues in its definition or is not applicable in the given context. * The correct way to formulate this is "Constraint1 is violated". Hi Johan, I agree that the message might be confusing. Thus, ok to re-open. Note that the message has already been discussed in bug 464387. Gathering your propositions, we have 1. '<constraint name> is violated' 2. Constraint '<constraint name> is violated' 3. <constraint name> Advantage: profile developer can fully control the message, but might also be confusing if the constraint name has not been carefully chosen. Therefore, another option is 4. choose 2 by default, choose 3, if the constraint name has a specific contents, e.g. starts with ' or is enclosed in "< >" What is your favourite? PS: It is already possible to apply the DSML stereotype to the constraint and set a custom message (with the risk of forgetting to align the name whenever the constraint changes, in particular after a copy&paste of a constraint). (In reply to Ansgar Radermacher from comment #4) > Gathering your propositions, we have > > 1. '<constraint name> is violated' > > 2. Constraint '<constraint name> is violated' > > 3. <constraint name> > Advantage: profile developer can fully control the message, but might also > be confusing if the constraint name has not been carefully chosen. > Therefore, another option is > > 4. choose 2 by default, choose 3, if the constraint name has a specific > contents, e.g. starts with ' or is enclosed in "< >" > > What is your favourite? > Personally I think that alternative 2 is too "talky". If you have lots of errors/warnings in the model validation view, which all starts with the same text, i.e. "Constraint..." it will be a lot of repeated, and seemingly superfluous information/text in the model validation view and it will be harder to distinguish/see the different types of constraints being violated since all messages starts exactly the same. I would really prefer alternative 3 to be the default (or possibly alternative 1). As Ansgar wrote, I think we should distinguish two situations: Case 1/ The developer didn't specify any message for the constraint In that case, we only have its name to work with. I would propose the construct: Constraint 'constraint name' is violated. I agree this is verbose, but this is a fallback-situation and should therefore be as clear as possible: no confusion allowed. As Peter stated in https://bugs.eclipse.org/bugs/show_bug.cgi?id=464387#c14, I also was confused about the "not valid" part. Just the name is not an option. Developers will typically choose names like "LinksMustBePresent" which is not that clear when it just pops up in the Model Validation view. Moreover, this default is only used if the developer didn't specify any message, so the verboseness can always be avoided by the developer. Case 2/ The developer did specify a message In that case, I would propose to use just the message. It is the developer's responsibility to use a sensible message. The constraint's name seems irrelevant to me here. Conclusion: I follow Ansgar's 4th proposal, with the exception that I propose to use the "Message" instead of the "Name". I guess that is also what Ansgar intended. (In reply to Johan Van Noten from comment #6) >... > > Conclusion: I follow Ansgar's 4th proposal, with the exception that I > propose to use the "Message" instead of the "Name". I guess that is also > what Ansgar intended. Hi Johan, no, I actually meant the name. We had the problem in the past that message specification via the stereotype became inconsistent with the constraint. The main reason was that the domain expert who worked on the profile was not an experienced tool user (and since we don't automatically always show the stereotype content in the diagram). So, it would be nice to have an option to specify only the constraint name and still get a useful message. My current favourite is thus: - constraint message, if a message is specified - constraint name without prefix, if it starts with a "magic" prefix, I thought of a simple "=" since that does not harm the readability of the constraint within the profile and seems a good indication that the name should be used as it is. - "<constraint name> is violated", otherwise Definitively agree that the two different cases shall be treated differently. I have a strong feeling that we are in a situation where one-size-fits-all does not work. As can be seen in comments of Bug 464387, we started out with case 2, i.e. the UML-RT profile had explicit messages defined. But then the profile was updated and the messages got out of sync with both the names and the semantics of the constraints. Thus it was decided to go for case 1 instead, to avoid the inconsistency problems. So the question is which automatically generated message that shall be used for case 1. I do not see it feasible to force everyone to go for case 2 (since that was the cause of the inconsistency issues that Bug 464387 is all about). One possible solution here is to introduce a new setting, e.g. a new stereotype which is applied on the profile itself, which has some property (or properties) that can control how the automatically generated message will look like for case 1. For example with the UML-RT profile, I would prefer if the message was simply the same as the name of the constraint. For the specific profile we will then just ensure that the name of the constraint is good enough as a message as well. For other profiles it might be that the names are not that good, and then the pattern of "Constraint <constraint name> is violated" might be better. But then that can be controlled by a specific property of a stereotype applied on the profile itself. With this approach you could even have a property where you freely can specify how the automatically generated message should look like, e.g. using a template string "Constraint %1 is violated" or any other template. Created attachment 257961 [details] Screen shot showing the constraints in the preferences dialog (In reply to Ansgar Radermacher from comment #7) > - constraint name without prefix, if it starts with a "magic" prefix, I > thought of a simple "=" since that does not harm the readability of the > constraint within the profile and seems a good indication that the name > should be used as it is. The problem with this kind of "trick" is that the "magic" prefix I assume will be visible to end-users also when they look in the preferences dialog under Model Validation > Constraints (see the attached screen shot). I would really not like to have such "magic" prefixes or similar visible to end-users. Better then to have the controlling of how messages shall be generated as a separate property (or properties). I would also avoid the use of "magic"... there is already enough magic in Papyrus :-) In fact, I also dislike the (ab-)use of a name as a message. I would expect the name to be a valid identifier (no spaces, strange characters, etc), rather than a message intended for the end-user. If I understand it correctly, the driver for doing this was to avoid inconsistency between the constraint and the "invisible" message. Does Papyrus have a way for a profile to say that by default some of its properties should be shown on the element it is applied to? (In reply to Johan Van Noten from comment #10) > In fact, I also dislike the (ab-)use of a name as a message. I would expect > the name to be a valid identifier (no spaces, strange characters, etc), > rather than a message intended for the end-user. It is exactly this that makes me feel that the generation of automatic message for case 1 needs to be configurable. For those like Johan, who wants the name of the constraint to only be seen as an identifier, you should be able to configure the message to be for example "Constraint <constraint identifier> is violated" but for the profiles where the name has a more "relaxed" approach and the name of the constraint actually makes sense to also have as the message then the the message simply can be the same as the constraint name (which I feel is applicable for the UML-RT profile as discussed in Bug 464387). So the tooling really needs to solve both approaches in a simple (and non-magic) way, e.g. by the use of additional (explicit) properties that can control how the automatically generated message looks like, maybe even configurable with a template string as proposed in Comment 8. (In reply to Peter Cigehn from comment #11) > (In reply to Johan Van Noten from comment #10) > > In fact, I also dislike the (ab-)use of a name as a message. I would expect > > the name to be a valid identifier (no spaces, strange characters, etc), > > rather than a message intended for the end-user. > > It is exactly this that makes me feel that the generation of automatic > message for case 1 needs to be configurable. For those like Johan, who wants > the name of the constraint to only be seen as an identifier, you should be > able to configure the message to be for example "Constraint <constraint > identifier> is violated" but for the profiles where the name has a more > "relaxed" approach and the name of the constraint actually makes sense to > also have as the message then the the message simply can be the same as the > constraint name (which I feel is applicable for the UML-RT profile as > discussed in Bug 464387). > Hi Peter, I propose to add an additional stereotype to the DSML validation profile that can be applied to a profile or sub-package. It has a boolean attribute "treat name as message". If true (and no message is present) the constraint name will be used as message without any changes, otherwise, the message is "Constraint <name> is violated". (In reply to Ansgar Radermacher from comment #12) > I propose to add an additional stereotype to the DSML validation profile > that can be applied to a profile or sub-package. It has a boolean attribute > "treat name as message". If true (and no message is present) the constraint > name will be used as message without any changes, otherwise, the message is > "Constraint <name> is violated". That sounds like a good solution and it will solve both Johan's needs as well as the needs for the UML-RT profile, without any tricks. Have you considered to make it even more flexible to also allow the user to provide a template for the generated message, i.e. in case someones wants to a generated message to look different than the proposed template of "Constraint <name> is violated"? Should be fairly easy to just have an additional string property on this stereotype where you can provide a string like "%1 is violated" if you want to have a shorter version of the template? Or did you find that to be overkill? New Gerrit change created: https://git.eclipse.org/r/60779 (In reply to Eclipse Genie from comment #14) > New Gerrit change created: https://git.eclipse.org/r/60779 > [...] > Have you considered to make it even more flexible to also allow the user to > provide a template for the generated message, i.e. in case someones wants to > a generated message to look different than the proposed template of > "Constraint <name> is violated"? Should be fairly easy to just have an > additional string property on this stereotype where you can provide a string > like "%1 is violated" if you want to have a shorter version of the template? > Or did you find that to be overkill? It's a bit of an overkill ;-) But it maybe useful in same cases. It is taken into account in the gerrit proposition above. Instead of a boolean attribute, there is an enumeration with the options: default, name-is-message and custom. In case of the latter, an additional template string must be filled. The template string can contain a %s field for the name (documentation needs to be updated). Should we modify the umlrt-profile accordingly (with the message = name option)? (In reply to Ansgar Radermacher from comment #15) > Should we modify the umlrt-profile accordingly (with the message = name > option)? Yes, I would like that. The naming-scheme for the constraints in the UML-RT profile makes perfect sense to have them directly as the message. Bran is planning to make an update of the UML-RT profile and document, including some new constraints. So it would be a good timing to make that adjustment of this at the same time as the planned update which should be done before the 0.8.0 release of Papyrus-RT. Remi mentioned that he intended to create a tracking Bugzilla for that update, but I have not seen it yet. Please check with Remi regarding the planned update of the UML-RT profile. Gerrit change https://git.eclipse.org/r/60779 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=d3f91acb5d63c00dfe37b8cabf540a8fd3360029 New Gerrit change created: https://git.eclipse.org/r/60785 Gerrit change https://git.eclipse.org/r/60785 was merged to [streams/1.1-maintenance]. Commit: http://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=56f509bd34c12e527ce69271da7f8b623ed79032 I set the bug to fixed, since the gerrit contributions for master and 1.1-maintenance have been merged. |