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

Bug 362620

Summary:	OclInEcoreEditor removes namespace prefix
Product:	[Modeling] OCL	Reporter:	Jörn Guy Süß <jgsuess>
Component:	Core	Assignee:	OCL Inbox <mdt-ocl-inbox>
Status:	CLOSED FIXED	QA Contact:	Ed Willink <ed>
Severity:	normal
Priority:	P3	CC:	ed
Version:	unspecified
Target Milestone:	M3
Hardware:	All
OS:	All
Whiteboard:

Description Jörn Guy Süß

2011-11-01 22:34:07 EDT

Build Identifier: 20110916-0149

Opening a validated ecore file that contains embedded subpackages.

module _'faulty.ecore'
package main : m = 'http://main'
{
	class M
	{
		property p : sub::subsub::SSC;
	}
	package sub : s = 'http://sub'
	{
		package subsub : ss = 'http://subsub'
		{
			class SSC;
		}
	}
}

after save, close, load turns into:

module _'faulty.ecore'
package main : m = 'http://main'
{
	class M
	{
		property p : subsub::SSC[1];
	}
	package sub : s = 'http://sub'
	{
		package subsub : ss = 'http://subsub'
		{
			class SSC;
		}
	}
}

Reproducible: Always

Steps to Reproduce:
1. Create a model in graphical ecore Editor where a class references another type in a package nested at least two levels deep. (See above)
2. Validate the ecore.
3. Open the file in OclInEcore Editor
4. Observe that the package prefix has been truncated to the last package of the target type. (i.e. sub::subsub:: becomes just subsub::)
The editor reports an error.
5. Fix the error.
6. Close the editor.
7. Reopen the oclInecore editor. The file is broken again.

Comment 1 Jörn Guy Süß

2011-11-01 23:53:00 EDT

The problem is made more difficult because the change also breaks the genmodel and code generation. This is a real blocker.

Comment 2 Ed Willink

2011-11-02 02:31:38 EDT

This is a model to text serialization bug; an error-free model is saved correctly but loads wrong. The workaround is just to repair the loaded text before saving again. There should therefore be no impact on genmodel or codegen since the save is correct.

I assume that you loaded, kept the errors, then saved again with errors.

Changing to major since there is a workaround, and the problem does not occur during save.

Comment 3 Ed Willink

2011-11-02 03:13:03 EDT

Problem is that the serializer thinks that subsub for the inner package is also an external alias; this is already fixed on the bug/349962 where aliases are suppressed for nested packages.

Pushed to master for M3. SR2 to follow.

Comment 4 Ed Willink

2011-11-02 03:47:39 EDT

Same fix for SR2 pushed to bug/362620SR2 branch. Please review.

Comment 5 Adolfo Sanchez-Barbudo Herrera

2012-01-31 11:27:51 EST

Ed,

When I spent some time in reviewing your changes last week, I didn't understand very well the code. I expected to make a deeper analysis of it, with some questions to you to understand it, but this has not occurred yet and RC2 has passed.

If I'm not wrong, since this is code from examples, I think you don't strictly need a +1 from me, do you ?. Please, if I have not done the review during this week, committ the changes before the RC3.

Regards,
Adolfo.

Comment 6 Ed Willink

2012-01-31 11:38:06 EST

(In reply to comment #5)
> If I'm not wrong, since this is code from examples, I think you don't strictly
> need a +1 from me, do you ?.
All maintenance changes need review.

For examples it is sufficient for you to feel comfortable that the potential impact is limited to examples functionality. i.e. no new extension points, global registry manipulation, ...

Comment 7 Adolfo Sanchez-Barbudo Herrera

2012-02-03 12:45:25 EST

Changes mainly applies to AnalysisAlias Class and computePackages private method. Some comments

1. nestedPackages are only used privately when computing packages. No need to be a private field, but a local variable of such a method.
2. There is an extra "if (eObject instanceof org.eclipse.ocl.examples.pivot.Package)" in such a method.
3. I need comments to understand what the method exactly does, I don't' clearly see its intention when reading the code.
4. Iterating on every object contained in a resource and then transitevely navigating through its containers doesn't look performance-wise. It will probably have a technical reason, but some comments could help to the code reader.

On the other hand, I feel comfortable with the change since I don't foresee any impact to OCL Core (+ 1). I don't want to block this interesting fix for Indigo SR2 due to my misunderstanding of such a lass. 1 and 2 are sensible easy changes which merit to be incorporated, though.

Regards,
Adolfo.

Comment 8 Ed Willink

2012-02-03 16:51:12 EST

(In reply to comment #7)
Thanks for the review.

1,2 done before pushing to maintenance/R3_1 for SR2.

(2) was already done on master; I was just minimizing changes.

1,3,4 done and pushed to master

Avoiding alias clashes with all names is safer that an intelligent/buggy avoidance of relevant names. Unfortunately all names needs a total search.

> Changes mainly applies to AnalysisAlias Class and computePackages private
> method. Some comments
> 
> 1. nestedPackages are only used privately when computing packages. No need to
> be a private field, but a local variable of such a method.
> 2. There is an extra "if (eObject instanceof
> org.eclipse.ocl.examples.pivot.Package)" in such a method.
> 3. I need comments to understand what the method exactly does, I don't' clearly
> see its intention when reading the code.
> 4. Iterating on every object contained in a resource and then transitevely
> navigating through its containers doesn't look performance-wise. It will
> probably have a technical reason, but some comments could help to the code
> reader.
> 
> On the other hand, I feel comfortable with the change since I don't foresee any
> impact to OCL Core (+ 1). I don't want to block this interesting fix for Indigo
> SR2 due to my misunderstanding of such a lass. 1 and 2 are sensible easy
> changes which merit to be incorporated, though.
> 
> Regards,
> Adolfo.

Comment 9 Ed Willink

2013-05-20 11:37:21 EDT

CLOSED after a year in the RESOLVED state.