Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 335828 - getPictogramElementForBusinessObject(Object) returns sometimes wrong PictogramElement
Summary: getPictogramElementForBusinessObject(Object) returns sometimes wrong Pictogra...
Status: CLOSED FIXED
Alias: None
Product: Graphiti
Classification: Modeling
Component: Core (show other bugs)
Version: 0.7.0   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: 0.8.0   Edit
Assignee: Tim Kaiser CLA
QA Contact:
URL:
Whiteboard: Indigo M7 theme_round_offs
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-31 05:43 EST by Klaus S. CLA
Modified: 2014-08-27 08:15 EDT (History)
4 users (show)

See Also:
michael.wenz: indigo+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Klaus S. CLA 2011-01-31 05:43:47 EST
Build Identifier: 20100917-0705

If the link model contains link-entries with business objects which are equal in sense of EcoreUtil.equals(EObject, EObject), using org.eclipse.graphiti.features.impl.AbstractFeatureProvider.getPictogramElementForBusinessObject(Object) results in the odd behavior returning sometimes not the correct PictorgramElement.

A quick fix should be changing the equality-check of business objects from using EcoreUtils.equals() to comparing object-ids.

Same issue seems to persist in getAllPictogramElementsForBusinessObject(...)

Reproducible: Sometimes

Steps to Reproduce:
1. Create Business Model with
 a. 2 classes (class1, class2)
 b. ... with exactly the same attribute-values
 c. class1 and class2 are from the same type
2. Create 2 PictogramElements (picto1, picto2) and link picto1 with class1 respectively picto2 with class2
3. call getPictogramElementForBusinessObject(class1)
4. call getPictogramElementForBusinessObject(class2)

-> call (3) may return picto2 in place of picto1
-> call (4) may return picto1 in place of picto2
Comment 1 Michael Wenz CLA 2011-02-01 11:09:03 EST
Christian,

yes, this can happen in case all attributes of two EObjects are completely identical. The question that arises to me is, if that is really relevant for a real scenario? What would be the use case of having two structurally identical objects that are different in identity?

Besides, what is the ID of an EObject? By default this is not defined. Or do you mean comparing URIs? That will end up with issues as soon as different URI types (e.g. protocols) come up (this is an issue we already had).

In the end, using EcoreUtil.equals is the method of choice for comparing EObjects. Feel free and try to convince me of something else... ;-)

Michael
Comment 2 Klaus S. CLA 2011-02-02 09:11:59 EST
With comparing object ids I'm referring to comparing for reference equality. But this might be indeed not really a considerably option.

but for the use case, hmm lets assume the following simple model:
a directed graph with
nodes: node1, node2, node3
... and edges: edge1 (node1->node2), edge2 (node1->node3)

node2 and node3 are having identical attributes for whatever reason. As well as edge1 and edge2. EcoreUtuil.equals() would therefore evaluate node2 and node3 as equal.

It could be lead into trouble creating a diagram from this model like this:

First I would add the nodes and link them with PictogramElements.

The next step would be adding the edges like this:
for every edge in model do
  get source- and target node form edge (target, source).
  (*) get pictorgram elements (pictoSource, pictoTarget) of source and target (using getPictogramElementForBusinessObject()).
  get anchors of pictoSource, pictoTarget (sourceAnchor, targetAnchor).
  create addConnectionContext using sourceAnchor, targetAnchor, edge and targetDiagram.
  create AddFeature using addconnectionContext.
  execute addFeature.
enddo

If in step (*) the "wrong" pictorgramElements returns, either between node1 and node2 or node1 and node3 no edge will be added.

That node2 and node3 are structurally identical objects might seem a little odd but why should this not happen?
Maybe the model was initially created and forgotten adding node names or whatever to node2 and node3.
The model should be "displayable" anyhow.
Comment 3 Michael Wenz CLA 2011-02-03 07:57:54 EST
Ok, agreed that these kinds of models should somehow be displayable.

But, how would you distinguish between the structurally identical objects whithout using reference equality? All other ways I know of make use of structural differences.

Besides, with such objects you will most probably run into issues with other EMF-based tools or even EMF itself.

We could of course open the Graphiti framework so that there is one centralized method implementing the comparision (standard would be to use EmfUtil.equals) that the tool can override. But this won't help if there's no other feasible way to decide equality.
Comment 4 Klaus S. CLA 2011-02-05 14:37:51 EST
yes there is probably no other option then using reference equality.

Christian
Comment 5 Michael Wenz CLA 2011-03-14 04:39:27 EDT
There have by now been some request to use == for determining equality inside Graphiti instead of EcoreUtil.equals() for certain sceanrios. On the other hand in the default Graphiti usecases we so far know of == will definitely not work (e.g. because of reloading resources).

So coming back to my suggestion from above, I would vote for introducing a hook where tool builders can implement their own comparision mechanism while the standard Graphiti implementation continues to use EcoreUtil.equals(). Graphiti would use this hook in all relevant places.
Comment 6 Michael Wenz CLA 2011-03-24 10:19:38 EDT
My first idea was to add a method "isEqualForBusinessObjects" into the ToolBehaviorProvider with a default implementation that uses EcoreUtil.equals() and allow the user to override this to hook-in his own implementation of equals. The Graphiti framework would delegate to this method from all spots were business objects are checked for equality.
Unfortunatly this turned out not to be possible because there are some spots inside the Graphiti services (LinkService, PeService) were this delegation would be needed. The delegation there is not possible for two reasons:
1) The only information available there is the diagram and it is not possible with the means availble in Graphiti core to retrieve an instance of the ToolBehaviorProvider for that diagram
2) The services are global and valid for all Graphiti tools. If the user changed the behavior inside such a service it would affect all tools running in that IDE.

During the discussion we had the idea to use the independence solver mechanism to implement this usecase. Will dive into more investigations...

Michael
Comment 7 Michael Wenz CLA 2011-03-25 07:52:39 EDT
The idea to use an independance solver for this turned out to be implementable but not really usable. The interface IIndependanceSolver can be used for non-EMF domain models to do the linking between the domain and Graphiti objects. Currently it is not yet usable for EMF domain models but this could be easily implemented.
The disadvantage that you gain when using the independance solver mechanism for EMF domain models is that you do no longer have the EMF reference mechanism and instead would define you own one (inlcuding relsolving the targets of such a reference). This is hard to manage and may also lead to performance and other issues.
So, not really a suitable way to solve this. I put this back to the queue because I'll be on vacation next week.
Comment 8 Tim Kaiser CLA 2011-04-19 04:08:48 EDT
new method in IToolbehaviorProvider
            equalsBusinessObjects
standard implementation in DefaultToolbehaviorProvider 
based on EcoreUtil.equals.

Implementation detail: the diagramTypeProvider for each diagram
is cached in a WeakHashmap in EmfService. We checked (using Memory Analyzer)
that the garbage collector can reclaim closed diagrams etc.
Extra care needed to be taken since the values of our hashmap
(the DTP) have references to their keys. This could be solved by
wrapping them into WeakReferences.
Comment 9 Michael Wenz CLA 2011-05-27 09:25:15 EDT
Part of 0.8.0
Comment 10 Michael Wenz CLA 2011-06-24 09:13:20 EDT
Part of Graphiti Indigo 0.8.0
Comment 11 anudeep arya CLA 2014-06-25 07:15:57 EDT
Hi,

I face the same issue again in after migrating my code from graphiti 0.9 (indigo) to 0.10.2(kepler)...The object id's are different and objects are different.but in 

AbstractFeatureProvider class and getAllPrctogramElementsForBusinessObject method


if (getDiagramTypeProvider().getCurrentToolBehaviorProvider().equalsBusinessObjects(businessObject, obj)) 


this check returns true even if businessobj and obj are different.


Please revert back if you have any clue about this.I need it as soon as possible for my project

Regards,
Anudeep.
Comment 12 anudeep arya CLA 2014-06-25 07:16:23 EDT
Hi,

I face the same issue again in after migrating my code from graphiti 0.9 (indigo) to 0.10.2(kepler)...The object id's are different and objects are different.but in 

AbstractFeatureProvider class and getAllPrctogramElementsForBusinessObject method


if (getDiagramTypeProvider().getCurrentToolBehaviorProvider().equalsBusinessObjects(businessObject, obj)) 


this check returns true even if businessobj and obj are different.


Please revert back if you have any clue about this.I need it as soon as possible for my project

Regards,
Anudeep.
Comment 13 Michael Wenz CLA 2014-08-22 04:59:47 EDT
(In reply to anudeep arya from comment #12)
> I face the same issue again in after migrating my code from graphiti 0.9
> (indigo) to 0.10.2(kepler)...The object id's are different and objects are
> different.but in 

Are you sure this is the reason? This has been changed for Graphiti 0.8.0 and kept stable lateron...

Besides it might be an option for you to implement your own comparision logic in your ToolBehaviorProvider in the method equalsBusinessObjects(Object, Object).

Michael
Comment 14 anudeep arya CLA 2014-08-26 06:38:49 EDT
Hi Micheal,

  I think i still have the same problem and if if try to get all pictogramelemnts corresponding to a bo then it returns all the pictogram elements in the diagram.

couldn't figure out the reason but equals method for different objects returns true. i can see that while debugging.

Also i have one more question,i am implementing a feature for double click on a pictogram and in that when i iterate through all the pictogram elements in a diagram(required for business logic) ..the diagram scrolls to the end automatically which looks ugly.Is it reported anytime earlier.do you have a solution to stop the scroll.

Regards,
Anudeep.
Comment 15 Michael Wenz CLA 2014-08-26 09:51:29 EDT
(In reply to anudeep arya from comment #14)
>   I think i still have the same problem and if if try to get all
> pictogramelemnts corresponding to a bo then it returns all the pictogram
> elements in the diagram.
> 
> couldn't figure out the reason but equals method for different objects
> returns true. i can see that while debugging.

Hm, hard to tell without debugging into the equals coding. The standard implementation in Graphiti uses EcoreUtils.equals (can you check if that is called?) and that checks attribute equality, so the getAll... method would report
all BOs that have the same attribute values.

> 
> Also i have one more question,i am implementing a feature for double click
> on a pictogram and in that when i iterate through all the pictogram elements
> in a diagram(required for business logic) ..the diagram scrolls to the end
> automatically which looks ugly.Is it reported anytime earlier.do you have a
> solution to stop the scroll.

I would recommend not to reuse this bugzilla (it is a bug report for one specific issue) for more questions. Please post that as question in our forum. Please also provide the coding of what you do inside the iteration, because normally simply iterating over a list of pictogram elements will not let the editor scroll...
Comment 16 anudeep arya CLA 2014-08-26 10:09:24 EDT
(In reply to Michael Wenz from comment #15)
> (In reply to anudeep arya from comment #14)
> >   I think i still have the same problem and if if try to get all
> > pictogramelemnts corresponding to a bo then it returns all the pictogram
> > elements in the diagram.
> > 
> > couldn't figure out the reason but equals method for different objects
> > returns true. i can see that while debugging.
> 
> Hm, hard to tell without debugging into the equals coding. The standard
> implementation in Graphiti uses EcoreUtils.equals (can you check if that is
> called?) and that checks attribute equality, so the getAll... method would
> report
> all BOs that have the same attribute values.

Yes I can see Ecoreutil.equals method being called.attributes in the two objects.the name is different..but still equals returns true.
> 
> > 
> > Also i have one more question,i am implementing a feature for double click
> > on a pictogram and in that when i iterate through all the pictogram elements
> > in a diagram(required for business logic) ..the diagram scrolls to the end
> > automatically which looks ugly.Is it reported anytime earlier.do you have a
> > solution to stop the scroll.
> 
> I would recommend not to reuse this bugzilla (it is a bug report for one
> specific issue) for more questions. Please post that as question in our
> forum. Please also provide the coding of what you do inside the iteration,
> because normally simply iterating over a list of pictogram elements will not
> let the editor scroll...
Comment 17 Michael Wenz CLA 2014-08-27 08:15:11 EDT
(In reply to anudeep arya from comment #16)
> Yes I can see Ecoreutil.equals method being called.attributes in the two
> objects.the name is different..but still equals returns true.

Sorry, but I can only advise to debug into the equals method and find out what's wrong here...