Bugzilla – Full Text Bug Listing
| Summary: | UMLUtil.safeApplyStereotype throws IllegalArgumentException on required stereotypes | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] MDT.UML2 | Reporter: | Paul Elder <pelder.eclipse> | ||||||||
| Component: | Core | Assignee: | Kenn Hussey <Kenn.Hussey> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | bruck.james | ||||||||
| Version: | 3.0.0 | Flags: | Kenn.Hussey:
pmc_approved+
|
||||||||
| Target Milestone: | --- | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Paul Elder
Hi Paul, Would it be possible to attach a small example that reproduces the situation. James. Created attachment 169349 [details]
JUnit4 test case
Here's a JUnit4 test case. I'll attach the test profile shortly.
Created attachment 169350 [details]
Test profile
Test profile. Should be located in the profiles directory of the JUnit project containing the test case.
Some notes about the attached test case: 1) it verifies applying both required and non-required stereotypes. 2) it verifies re-applying as well as initial application 3) of the four tests, only the initial application of a require stereotype fails. Thanks for the test cases. It looks like the stereotype gets applied twice and the second time the IllegalArgumentException gets thrown. Just by observing the code, I'm thinking the problem may ultimately be in ElementOperations::applyStereotype() where we throw the exception if the stereotype is already applied. It would seem that we need to return the stereotype application instead of throwing an exception. I'll investigate further to see if there are any other ramifications of changing this. It has long been part of the (implicit) API contract for stereotype application that attempting to apply a stereotype which is already applied will throw an exception. It has also been the case that it is invalid to attempt to explicitly apply a required stereotype. I'm concerned about changing this behavior at this late stage in the release, as there are likely clients that rely on the current behavior... (In reply to comment #6) > It has long been part of the (implicit) API contract for stereotype application > that attempting to apply a stereotype which is already applied will throw an > exception. It has also been the case that it is invalid to attempt to > explicitly apply a required stereotype. I'm concerned about changing this > behavior at this late stage in the release, as there are likely clients that > rely on the current behavior... Kenn, are you saying that Paul's situation is really not an error since the required stereotypes get applied automatically when the profile is applied? This API strikes me as a bit harsh (difficult to use)? I'm saying that is has always been the case that trying to apply a required stereotype, or one which has already been applied, will result in an exception. The "safe" utility method was introduced to handle the cases where you try to apply a stereotype before the profile has been applied (in which case it will apply the profile for you) or where you try to apply one which has already been applied (in which case it will return the stereotype application). I suppose we could change the logic (in the utility method only) to also handle the case where you try to apply a required stereotype, but that doesn't change the fact that we have always considered it invalid to attempt to explicitly apply a required stereotype in the first place (since it's done for you automatically)... I can go with Kenn's interpretation (i.e. it shouldn't work on required stereotypes), but in that case the behavior is still inconsistent, since passing an 'already-applied' required stereotype raises no exception. Also, if no-required-stereotypes is to be the rule, then the documentation needs to state this. Created attachment 169847 [details]
proposed fix
Here's a proposed fix. Paul, please verify that this will meet your needs. Jame, please take a look to see if this looks OK.
(In reply to comment #10) > Created an attachment (id=169847) [details] > proposed fix > > Here's a proposed fix. Paul, please verify that this will meet your needs. > Jame, please take a look to see if this looks OK. That looks good to me. Kenn: Looks good to me. The fix has been committed to CVS. The changes are available in the RC3 build. |