Comment 1 smaoui asma 2016-03-29 06:37:56 EDT

Hello,

I analysed this bug and found that the display of a decorator in the lower left corner of the capsule structure diagram is due to the creation of 2 different views (shapes) for the same model element (the capsule part being copied). In fact, if we open the .notation file after the copy paste we noticed that 2 shapes were created for the same uml:property element: the capsule part: one inside the basic compartment (the rignt one, visible on the diagram) and another outside the basic compartment of the containing capsule.
if we remove the second one, the decorator outside the capsule is deleted.


The NPE exception is caused again by applying the "Comment" stereotype. As in Bug 489624 

org.eclipse.papyrus.uml.diagram.stereotype.edition.editpolicies.AppliedStereotypeCommentEditPolicy.java is figuring in the NPE Stack of both Bug (this one and 489624). may be this class should be reviewed by papyrus developper team.

I will try to fix the decoration staff (did not create the second shape), at the begining, I thought that the decorator implementation have a problem, and dislplay the decorator twice but it seems that the decorator works well but the editpart sychronisation staff is creating somewhere 2 figures for the same capsule part.

Asma

Comment 2 smaoui asma 2016-03-31 11:41:59 EDT

Hello,

I created a Papyrus Bug: Bug 490804 dealing with copy/paste a property in a composite structure diagram. The decorator displayed outside the basic compartment of the Class is due to the creation of a view referencing the newly created property outside the basic compartment of the class structure.

as mentioned by Peter in his comment to the bug Bug 490804, this is only the case if we select the Class view itself (not the compartment) before the paste.

Then, for this bug: the decorator shown outside the class view is only shown in case of a paste while selecting the class view (not the compartment).

Since the Papyrus Bug will not be fixed (it is not yet confirmed as a bug) I propose this gerrit to not install a decorator (not provide an RTPropertyEditPart at all) for the element created outside the Class View.

This fix should be refixed when the Papyrus bug will be fixed :)

Still have the NPE dealing with CommentSterotype application, that I can not reproduce outside Papyrus RT. 

Asma

Comment 3 Eclipse Genie 2016-03-31 12:07:50 EDT

New Gerrit change created: https://git.eclipse.org/r/69640

Comment 4 Peter Cigehn 2016-04-01 02:59:09 EDT

(In reply to smaoui asma from comment #2)
> Still have the NPE dealing with CommentSterotype application, that I can not
> reproduce outside Papyrus RT. 

Regarding this issue, I have written a separate Bugzilla (which I focused on the port case, but I guess it could be generalised to also cover the capsule part case), see Bug 490775.

Comment 5 smaoui asma 2016-04-05 09:51:51 EDT

Hello,

I can reproduce the same NPE with SysML :) this means that this NPE should be fixed in Papyrus and not in Papyrus RT.


I think that the PapyrusCanonicalEditPolicy should be fixed inorder to not delete Comment view attached to part (The comment view semantic element point to null: and this is a choice that papyrus developepr made to ensure that if we delete the comment from the model we did not delete the semantic element attached to it).


In fact, to reproduce the bug, we should activate the sychronisation mode. 

we should have a profiled model because the NPE is dealing with the application of the sterotype "Comment". 

steps to reproduce the same NPE in SysML
1) create a Block in a composite structure diagram
2) create a part in the Block (sterotype it with "FlowProperty" for example)
3) copy the FlowProperty view and Paste it in the Block view
4) the same NPE is raised: 

java.lang.NullPointerException
	at org.eclipse.papyrus.infra.gmfdiag.css.CSSShapeImpl.getEngine(CSSShapeImpl.java:64)
	at org.eclipse.papyrus.infra.gmfdiag.css.CSSShapeImpl.getCSSView(CSSShapeImpl.java:57)
	at org.eclipse.papyrus.infra.gmfdiag.css.CSSShapeImpl.getCSSNamedStyle(CSSShapeImpl.java:519)
	at org.eclipse.papyrus.infra.gmfdiag.css.CSSShapeImpl.getNamedStyle(CSSShapeImpl.java:509)
	at org.eclipse.papyrus.infra.gmfdiag.common.model.NotationUtils.getEObjectValue(NotationUtils.java:358)
	at org.eclipse.papyrus.uml.diagram.common.stereotype.display.helper.StereotypeDisplayUtil.getCommentSemanticElement(StereotypeDisplayUtil.java:113)
	at org.eclipse.papyrus.uml.diagram.stereotype.edition.editpolicies.AppliedStereotypeCommentEditPolicy.notifyChanged(AppliedStereotypeCommentEditPolicy.java:116)
	at org.eclipse.gmf.runtime.diagram.core.listener.DiagramEventBroker.fireNotification(DiagramEventBroker.java:504)
......
......

Asma

Comment 6 smaoui asma 2016-04-05 10:09:39 EDT

I created a Papyrus Bug 491083 that trakcs the fact that we loose the Comment sterotype attached to property of Class in a composite structure diagram. this could be related to this bug.

Comment 7 Peter Cigehn 2016-04-05 10:15:01 EDT

I made that (In reply to smaoui asma from comment #6)
> I created a Papyrus Bug 491083 that trakcs the fact that we loose the
> Comment sterotype attached to property of Class in a composite structure
> diagram. this could be related to this bug.

To make it more clear, I made that Bugzilla block this one. Should we do the same for the other similar cases, e.g. Bug 490775 and Bug 489624?

Comment 8 smaoui asma 2016-04-06 04:03:48 EDT

I do not think so since I did not succed in reproducing the same NPE when deleting port or parts in SysML even in synchronized mode.

I think that this bug should be more analyzed at the Papyrus RT side.

Asma

Comment 9 smaoui asma 2016-04-06 10:53:12 EDT

Hello,
I finally fixed this strange NPE
It is definitively related to Papyrus RT implementation and not to Papyrus itself.

In fact, the NPE is raised when Papyrus tries to add a Comment Sterotype to the deleted element. 

when analyzing the code, I noticed that the sterotype referenced by the Comment is not the CapsulePart (as we expect) it is instead the Capsule Sterotype.

In fact, the semantic element of the RTPropertyEditPart is not null (as we can expect after the deletion of the view), it is instead set to the container of the last semantic element : the Capsule that owns the property. 

This strange set of the editpart semantic element is the cause of the NPE: the smeantic element is not set to null, it has a sterotype (Capsule) and then the AppliedCommentEditPolicy tries to create a comment for the Capsule Sterotype...

after long debug, I found that the implementation of our RTPropertyEditPart and RTClassPropertyEditPart contains a call to the method getUMLElement() to retrieve the semantic element attached to the editpart. However this method returns the direct semantic element of the view if it is set, otherwise it returns the Container semantic element. BUT it ALSO modify the value of the element attribute and set it to the new retrived value, in our case, the RTPropertyEditPart semantic element is set to the capsule element owning the deleted property. 


I fixed this NPE by testing the old value of the RTPropetyEditPart semantic element before calling getUMLElement(): if the element is already set to null, we should not add listerners to  already removed elements and then we should not call getUMLElement().


Asma

Comment 10 Peter Cigehn 2016-04-06 10:57:35 EDT

(In reply to smaoui asma from comment #9)
> when analyzing the code, I noticed that the sterotype referenced by the
> Comment is not the CapsulePart (as we expect) it is instead the Capsule
> Sterotype.
> 
> In fact, the semantic element of the RTPropertyEditPart is not null (as we
> can expect after the deletion of the view), it is instead set to the
> container of the last semantic element : the Capsule that owns the property. 
> 
> This strange set of the editpart semantic element is the cause of the NPE:
> the smeantic element is not set to null, it has a sterotype (Capsule) and
> then the AppliedCommentEditPolicy tries to create a comment for the Capsule
> Sterotype...

Nice to see the same analysis that I did when I tried to learn what was going on. And I could also see these strange anomalies, but since I did not know what to expect, I could just conclude that it looked really strange. Good to know that I was on the right track up until here at least... :)

Comment 11 smaoui asma 2016-04-08 04:06:47 EDT

hello, I created a Papyrus bug (Bug 491300) since the NPE raised while copy paste the capsule part is raised whenever we copy paste a sterotyped element in a composite structure diagram (I reproduced the same bug using MARTE and SysML profiles)

thus, the gerrit that I will propose here (for mars) fix the decoration isuues but not the NPE since the NPE should be fixed in Papyrus itself.

Asma

Comment 12 Eclipse Genie 2016-04-08 04:48:01 EDT

New Gerrit change created: https://git.eclipse.org/r/70212

Comment 13 Christian Damus 2016-05-17 09:21:44 EDT

In the latest build, I can copy and paste capsule-parts in a Capsule Structure Diagram without the NullPointerException.  I do still see the rogue capsule icon in the bottom-left corner of the diagram (will try Asma's Gerrit patch next).

But there's a worse problem:  we have a new undo exception.  Attempting to undo the paste of the capsule-part gives me this, which rolls back the undo transaction:

!ENTRY org.eclipse.ui 4 0 2016-05-17 09:18:01.366
!MESSAGE index=8, size=8
!STACK 0
org.eclipse.emf.common.util.BasicEList$BasicIndexOutOfBoundsException: index=8, size=8
	at org.eclipse.emf.common.util.BasicEList.get(BasicEList.java:346)
	at org.eclipse.emf.ecore.change.impl.ListChangeImpl.process(ListChangeImpl.java:534)
	at org.eclipse.emf.ecore.change.impl.ListChangeImpl.applyAndReverse(ListChangeImpl.java:486)
	at org.eclipse.emf.ecore.change.impl.ChangeDescriptionImpl.preApply(ChangeDescriptionImpl.java:818)
	at org.eclipse.emf.ecore.change.impl.ChangeDescriptionImpl.applyAndReverse(ChangeDescriptionImpl.java:324)
	at org.eclipse.emf.transaction.util.CompositeChangeDescription.applyAndReverse(CompositeChangeDescription.java:149)
	at org.eclipse.emf.transaction.RecordingCommand.undo(RecordingCommand.java:213)
	at org.eclipse.emf.common.command.CompoundCommand.undo(CompoundCommand.java:327)
	at org.eclipse.emf.common.command.CompoundCommand.undo(CompoundCommand.java:327)
	at org.eclipse.emf.common.command.CompoundCommand.undo(CompoundCommand.java:327)
	at org.eclipse.emf.common.command.CompoundCommand.undo(CompoundCommand.java:327)
	at org.eclipse.emf.workspace.EMFCommandOperation.doUndo(EMFCommandOperation.java:150)
	at org.eclipse.emf.workspace.AbstractEMFOperation.undo(AbstractEMFOperation.java:370)
	at org.eclipse.core.commands.operations.DefaultOperationHistory.doUndo(DefaultOperationHistory.java:399)
	at org.eclipse.core.commands.operations.DefaultOperationHistory.undo(DefaultOperationHistory.java:1149)
	at org.eclipse.ui.operations.UndoActionHandler.runCommand(UndoActionHandler.java:87)
	...

Comment 14 Christian Damus 2016-05-17 09:36:22 EDT

(In reply to Eclipse Genie from comment #3)
> New Gerrit change created: https://git.eclipse.org/r/69640

I rebased this on the latest Neon base, which was quite drastically refactored (mostly by me) since the original patch.

Comment 15 smaoui asma 2016-05-17 09:57:40 EDT

hello,

I created a Papyrus Bug, Bug 490804 to track the fact that when copy/paste properties in Composite Structure Diagram, the property view is created in the wrong place (not in the basic compartment of the Composite Class).

My patch only avoid the display of the decorator: do not allow to create a capsule part view outside the basic compatment.

However, I think that this bug should be fixed in Papyrus.  I just provide a workarround in Papyrus RT.

A++

Comment 16 Eclipse Genie 2016-05-17 10:33:44 EDT

Gerrit change https://git.eclipse.org/r/69640 was merged to [master].
Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=b36d46a9419e7d7beb6d2fd9299df3937fc027fd

Comment 17 Christian Damus 2016-05-17 10:35:39 EDT

(In reply to Eclipse Genie from comment #16)
> Gerrit change https://git.eclipse.org/r/69640 was merged to [master].

Thanks for the contribution!

The undo bug is unrelated and is tracked by bug 493812.

Comment 18 smaoui asma 2016-05-17 10:42:45 EDT

Thanks ;) but I am talking about copy paste bug in papayrus Bug 490804 and not an undo bug.
I saw that you have merged this contribution in master, but it only corrects the copy paste issue in Papyrus RT (prohibit the creation of a capsule part outside the capsule basic compartment) but the bug still exist in Papyrus !

Comment 19 Peter Cigehn 2016-05-17 11:33:40 EDT

I have tested this in the latest build of Papyrus-RT, and it do works to copy-paste without anything being logged in the error log.

The aspects identified in Bug 490804 are there though, i.e. depending on what you have selected prior to pasting you get three different behaviors/results:

1) If you select the structure compartment of the capsule prior to pasting, the copy ends up stacked on top the existing capsule part
2) If you select the capsule itself prior to pasting, the copy ends up at the "auto-layuout" position, i.e. the corresponding position that you would have gotten if you used the new child menu in the model explorer to create an additional capsule part.
3) If you don't have anything selected at all in the diagram prior to pasting, then you actually get two views of the same capsule part in the diagram. Removing the capsule part, removes both views. One of the views gets positioned at the "auto-layout" position (as in 2), and the other one ends up above the existing capsule part. It even looks like that the one above, is placed outside the structure compartment (if you move the capsule itself, then that view for the capsule part don't move, as the other one).

I am not sure how we shall track these remaining issues, that probably originates from the issues reported in Bug 490804. If we close this one, then we need another Papyrus-RT specific Bugzilla that depends on Bug 490804 so that we can track the remaining issues in base Papryus on the Papyrus-RT side.

Any opinions if we shall reopen this one, or write a new "tracking" Bugzilla that only trackes Bug 490804?

PS. Just for the sake of it, I also tried the case with copy-pasting a port in the diagram, and that fails with an expection. So that is also a case we need to write a Bugzilla for. DS.

Comment 20 Peter Cigehn 2016-05-24 08:20:17 EDT

(In reply to Peter Cigehn from comment #19)
> Any opinions if we shall reopen this one, or write a new "tracking" Bugzilla
> that only trackes Bug 490804?

I wrote Bug 494401 to track the remaining issues with the inconsistent behavior of what is selected prior to pasting.

I put this one into verified since the originally reported exceptions no longer are thrown.

Comment 21 Peter Cigehn 2016-10-20 04:54:30 EDT

Closing as verified fixed.