Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 326452

Summary: [Diagram] Diagram synchronization doesn't work in certain cases
Product: [Technology] Tigerstripe Reporter: Navid Mehregani <nmehrega>
Component: UIAssignee: Daniel Johnson <danijoh2>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P3 CC: chrhartl, danijoh2, skanton
Version: unspecified   
Target Milestone: 0.5M0   
Hardware: PC   
OS: Mac OS X - Carbon (unsup.)   
Whiteboard:
Attachments:
Description Flags
patch_326452.txt
none
patch_326452.txt
none
refactor_patch.txt none

Description Navid Mehregani CLA 2010-09-28 14:59:51 EDT 
Build Identifier: 

Diagram synchronization doesn't work in some cases where the model is updated from TS explorer or the association pop-up dialog is used.  All the indicated cases should be tested with an OPEN and a CLOSED diagram.  The synchronization algorithm used for open/closed diagrams is different and they should both be verified when fixing this defect. 

Reproducible: Always

Steps to Reproduce:
1. Create two entities with some attributes in each entity
2. Create an association between the two entities
3. Create two class diagrams and move all the artifacts on the diagrams
4. Open one diagram file and close the other

The following actions will not update one or both of the diagram files:
- Using TS explorer, move attributes from one entity onto the other
- Using TS explorer, delete an attribute from one of the entities
- Use the association pop-up dialog to change properties of the association.  The open diagram file is updated, but the closed one isn't
Comment 1 Daniel Johnson CLA 2010-10-08 18:15:28 EDT 
Created attachment 180524 [details]
patch_326452.txt

This is a proposed patch to fix this issue. Major change include making commands use a working copy of an artifact before editing it, instead of directly working with the cached copy that the synchronizers were trying to compare changes against. Also updated the synchronizer for closed diagrams that was comparing against the wrong list of resources for changes and therefore was never accepting a change.
Classes Changed:
AnnotationAddFeatureRequest.java
ArtifactAddFeatureRequest.java
ArtifactRemoveFeatureRequest.java
ArtifactSetFeatureRequest.java
AttributeCreateRequest.java
AttributeRemoveRequest.java
AttributeSetRequest.java
LiteralCreateRequest.java
LiteralRemoveRequest.java
LiteralSetRequest.java
MethodAddFeatureRequest.java
MethodCreateRequest.java
MethodRemoveRequest.java
MethodSetRequest.java
StereotypeAddFeatureRequest.java

IAbstractArtifact.java
ProjectDiagramsSynchronizer.java
Comment 2 Chris Hartley CLA 2010-10-10 18:53:08 EDT 
Excellent - this patch should fix a number of issues
Comment 3 Daniel Johnson CLA 2010-10-11 19:01:01 EDT 
Created attachment 180623 [details]
patch_326452.txt

Though my patch offered a lot of enhancements regarding synchronizing open and closed diagrams, it was still not synchronizing with TS Explorer, which was the main point of the bug. This patch fixes all test cases outlined by this bug, and should fix all synchronization issues.
Also, Comparer class had a comment saying that exceptions have nothing to compare, but they are allowed to have Fields both in TS Explorer and Diagrams, is this a bug? Or was the Comparer class just outdated?
Classes Changed:
AnnotationAddFeatureRequest.java
ArtifactAddFeatureRequest.java
ArtifactRemoveFeatureRequest.java
ArtifactSetFeatureRequest.java
AttributeCreateRequest.java
AttributeRemoveRequest.java
AttributeSetRequest.java
LiteralCreateRequest.java
LiteralRemoveRequest.java
LiteralSetRequest.java
MethodAddFeatureRequest.java
MethodCreateRequest.java
MethodRemoveRequest.java
MethodSetRequest.java
StereotypeAddFeatureRequest.java

ArtifactComponentTransferDropAdapter.java
TSDeleteAction.java

IAbstractArtifact.java
AbstractArtifact.java

Also made it so AssociationArtifact no longer relies on IMarkDirty, since other changes now make Associations update correctly without having to be marked dirty.
IAssociationArtifact.java
AssociationArtifact.java
AssociationUpdateCommand.java

ProjectDiagramsSynchronizer.java
ClassDiagramSynchronizer.java

Comparer.java

I sometimes get errors that break the whole diagram, but I can't pinpoint what is causing the problem. I was also getting these errors occasionally before I made any changes as well though.
Comment 4 Chris Hartley CLA 2010-10-11 20:24:38 EDT 
Exceptions can have fields to store exception message related info.

I suggest you fix exceptions only if this is not a lot of work.
Comment 5 Anton Salnik CLA 2010-10-12 09:34:18 EDT 
Hi Daniel, could you also check the following case please:
1. Open /org.eclipse.tigerstripe/samples/test-models/model-refactoring project; 
2. Rename simple.Ent1 artifact (with using Refactor Model -> Rename...); 
3. Open simple.default diagram. 
The artifact has new name, but references from other artifacts are still pointed to Ent1 (Ent2: +method0(Ent1):Ent1[0..1]).

I tested the case with your patch and it doesn't solve the problem.
Comment 6 Daniel Johnson CLA 2010-10-12 11:16:03 EDT 
(In reply to comment #5)
> Hi Daniel, could you also check the following case please:
> 1. Open /org.eclipse.tigerstripe/samples/test-models/model-refactoring project; 
> 2. Rename simple.Ent1 artifact (with using Refactor Model -> Rename...); 
> 3. Open simple.default diagram. 
> The artifact has new name, but references from other artifacts are still
> pointed to Ent1 (Ent2: +method0(Ent1):Ent1[0..1]).
> 
> I tested the case with your patch and it doesn't solve the problem.

Hey Anton, I do not believe this error is part of the same problem, so I would rather close this bug and work on Bug 327000 separately, though I will start looking into that as soon as possible.
Comment 7 Daniel Johnson CLA 2010-10-12 12:54:37 EDT 
(In reply to comment #6)
> Hey Anton, I do not believe this error is part of the same problem, so I would
> rather close this bug and work on Bug 327000 separately, though I will start
> looking into that as soon as possible.

I thought this bug was about the same problem, but did not realize the 3rd step was different. Let me spend some time today looking into it, and if I find it is because of different code than I will create a separate bug to track it under.
Comment 8 Daniel Johnson CLA 2010-10-12 19:10:13 EDT 
Created attachment 180714 [details]
refactor_patch.txt

(In reply to comment #5)
> Hi Daniel, could you also check the following case please:
> 1. Open /org.eclipse.tigerstripe/samples/test-models/model-refactoring project; 
> 2. Rename simple.Ent1 artifact (with using Refactor Model -> Rename...); 
> 3. Open simple.default diagram. 
> The artifact has new name, but references from other artifacts are still
> pointed to Ent1 (Ent2: +method0(Ent1):Ent1[0..1]).
> 
> I tested the case with your patch and it doesn't solve the problem.

Hey Anton, this patch should fix that issue. It was somewhat related to the rest of the changes.
Comment 9 Anton Salnik CLA 2010-10-12 22:50:23 EDT 
(In reply to comment #8)
> Created an attachment (id=180714) [details]
> refactor_patch.txt
> 
> Hey Anton, this patch should fix that issue. It was somewhat related to the
> rest of the changes.

Hi Daniel, thanks for the patch, it has solved described problem. But with the patch i have another problems, refactoring doesn't work correctly for some artifacts. Try to rename DataMiddle or Enumeration0 artifact from 'simple' package of 'model-refactoring' project and you will get "..cannot be resolved to a type" errors.
Comment 10 Navid Mehregani CLA 2010-10-13 14:24:31 EDT 
Thanks Dan! I've reviewed and committed your patch under comment#3.
I've filed a separate defect to address the problem noted by Anton: bug#327698.  This will be handled in the next iteration.