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

Bug 340992

Summary: Graph doesn't match, probably because of incorrectly populated EmfGraph.inheritanceMap
Product: [Modeling] EMFT.Henshin Reporter: Bernhard Stadler <bstadler.eclipsebugzilla>
Component: CoreAssignee: Enrico Biermann <enrico>
Status: CLOSED WORKSFORME QA Contact:
Severity: major    
Priority: P3 CC: bstadler.eclipsebugzilla
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
Test project
none
Patch that eliminates this bug
none
Test project (corrected)
none
Patch for henshin.common none

Description Bernhard Stadler CLA 2011-03-25 14:54:22 EDT 
Build Identifier: M20110210-1200

Hello,

I have been trying for some time to get Henshin to run on a simple test model written in XMI, but can't get it to work.

Because I haven't found enough documentation to be sure I didn't make a mistake in defining the transformation system, I used the Eclipse debugger to understand how it works and why I don't get a match.

What I found out makes me think that it's a bug (a major one because it breaks the main functionality), so I'm reporting it here:
In the method "EmfGraph.getDomainForType(EClass type)", the HashMap "inheritanceMap" is used to get all instances of "type".
In my debug run, during an invocation of "getDomainForType", "type" was an EClass representing "Test", a class from my model.
In "inheritanceMap", there was also an EClass representing "Test", but not the same one.
As EClass and its ancestors appear to override neither Object.hashCode() nor Object.equals(), these two EClasses are not considered equal and null is returned.

My explanation is that the "Test" instantiated twice - once for the XMI model and once for the LHS of the rule.
Is this behaviour expected?

I will attach the test ecore model and henshin file.
As I said I'm not entirely sure whether the transformation system is defined correctly - I'm not sure what these parameters are about and whether I have to define them with the same name as a node or not.

Sincerely,
Bernhard Stadler

Reproducible: Always

Steps to Reproduce:
1. Run in debug mode from Henshin development workspace
2. In launched instance, import attached project, register test.henshin
2. In Henshin development workspace, set Breakpoint at org.eclipse.emf.henshin.common.util.EmfGraph.getDomainForType
3. Apply transformation unit MyHenshinTest.AB2C
4. Breakpoint will trigger
5. Inspect "type" (for me, this has ben an EMFClass instance for "Test" every time) and "inheritanceMap": inheritanceMap will contain a different EMFClass instance for the same EMF class.
Comment 1 Bernhard Stadler CLA 2011-03-25 14:55:32 EDT 
Created attachment 191932 [details]
Test project
Comment 2 Bernhard Stadler CLA 2011-03-25 16:41:59 EDT 
Forgot to mention that I was using the 0.7.0 tag from SVN
Comment 3 Bernhard Stadler CLA 2011-03-26 06:45:54 EDT 
I tried replacing inheritanceMap and domainMap with a TreeMap using a comparator that looks only at the classes' names in order to verify that this is the real problem. 

However, it still doesn't work because there is a similar problem at TypeConstraint.check(), which uses the Ecore method EClassImpl.isSupertypeOf, which of course doesn't respect the inheritanceMap.

I think that the problem is at a earlier stage, at the point where the Ecore model is loaded - it should be instatiated only once, and reused later when it is loaded another time, so that identical Ecore classes also mean identical EClass objects.


Another question: Is inheritanceMap really necessary although Ecore already provides EClass.getEAllSuperTypes()?

Sincerely,
Bernhard Stadler
Comment 4 Bernhard Stadler CLA 2011-03-26 09:20:17 EDT 
Created attachment 191953 [details]
Patch that eliminates this bug

I took the SVN source and changed org.eclipse.emf.henshin.interpreter.util.HenshinRegistry to remember the ResourceSets and changed org.eclipse.emf.henshin.internal.interpreter.VariableInfo, org.eclipse.emf.henshin.interpreter.ui.ApplyTrafoUnit and org.eclipse.emf.henshin.interpreter.ui.RegisterTrafoSystem accordingly.

That way, each .henshin file is associated with a ResourceSet, which is reused every time one of its transformation systems is applied to a model.
I'm not sure whether this is conceptually right, but it works for me now.

(I also added a few lines to RegisterTrafoSystem in order to eliminate duplicates because one could not register a transformation system twice without causing errors)

Now, a corrected version of my transformation systems works (will follow)
Comment 5 Bernhard Stadler CLA 2011-03-26 09:25:18 EDT 
Created attachment 191954 [details]
Test project (corrected)

This is a corrected version of the test project.
When the previously presented patch is applied to Henshin, applying the TS from test.henshin to test2 will cause "c" references to be added to the "xes" objects, as supposed.
Comment 6 Enrico Biermann CLA 2011-04-04 07:41:19 EDT 
Hello,

thanks for the detailed explanation and the patch.
You are correct, that there are two different EClasses associated with "Test".
In the attached project, "Test" from the xmi-files is created together with a new EPackage because a registry lookup to "http://test.com" fails.

I will see if I can find a more general solution to recognize the correct EClass because a check by name can easily lead to collisions. 

About your question about the inheritance map: It is used to look up "compatible" aka child types not super types. Such a lookup can be very difficult when inheritance is used between EClasses of different packages.

Can you please attach a patch of your changes to common? Your patch files contain only changes to interpreter and interpreter.ui but there should be changes in common, at least for Variable and likely also to EmfGraph.
I am also curious why you want node.getName() as a second constructor parameter for Variable because it does not contain any type related information (type name would be node.getType().getName())
Comment 7 Bernhard Stadler CLA 2011-04-04 07:54:02 EDT 
Created attachment 192450 [details]
Patch for henshin.common
Comment 8 Bernhard Stadler CLA 2011-04-04 08:06:20 EDT 
Hello,

(In reply to comment #6)
> I will see if I can find a more general solution to recognize the correct
> EClass because a check by name can easily lead to collisions. 

As I said - I'm not sure whether the change is conceptually right. 
The patch was meant more as an example, less as a solution.

> Can you please attach a patch of your changes to common? Your patch files
> contain only changes to interpreter and interpreter.ui but there should be
> changes in common, at least for Variable and likely also to EmfGraph.
> I am also curious why you want node.getName() as a second constructor parameter
> for Variable because it does not contain any type related information (type
> name would be node.getType().getName())

Forgot that. Patch attached - but I only made that change for debugging purposes. I wanted to keep track of variables more easily during the debug session so I just added that attribute. 
After I had my "diagnosis", it wasn't relevant anymore. That's why I didn't originally include the henshin.common patch, but I forgot that I had changed the constructor.

Regards,
Bernhard Stadler
Comment 9 Christian Krause CLA 2012-07-20 14:47:11 EDT 
I think the problem occurs only if multiple ResourceSets are used for loading the instance model and the transformation model. If everything is loaded using a single ResourceSet (e.g. a HenshinResoureSet), the problem should not occur.