Bug 291159

Summary:	umbrello creates many 'new_packages_xx" packages on file loading
Product:	[Applications] umbrello	Reporter:	Ralf Habacker <ralf.habacker>
Component:	general	Assignee:	Umbrello Development Group <umbrello-devel>
Status:	RESOLVED FIXED
Severity:	normal	CC:	okellogg
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Unlisted Binaries
OS:	Microsoft Windows
Latest Commit:		Version Fixed In:	4.8.1
Sentry Crash Report:

Description Ralf Habacker 2012-01-10 12:31:56 UTC

Version:           unspecified (using Devel) 
OS:                MS Windows

When opening the UML 1.4 spec from http://www.omg.org/spec/UML/20010967/01-02-15.xml umbrello detects already added package names and asks the user for a different package name. This happens 59 times. 



Reproducible: Always

Steps to Reproduce:
Download the mentioned url and open in umbrello

Actual Results:  
59 packages with name new_package_xx 

Expected Results:  
the packages with the name new_package_xx should not appear

umbrello prints out the following message 

loadFromXMI(UMLRole): id a47 is already in use!!! Please fix your XMI file.

This message is printed in UMLObject::loadFromXMI() although the related file do specify this id only once.

Further investigation let me assume, that umbrello "autocreates" the related UMLObject on an hidden place and then is surprised that the related object already exists 

The object are "official" added in bool UMLPackage::addObject(UMLObject *pObject) where it first searches if an UMLObject instance with the related package name is already present and if so ask the user for a new name. 

<excurse>
I doubt that this check is useful,because in a deep package hierachy there may be packages with the same name. 

As workaround i tried to change the line 

      while ( findObject( name ) != NULL  ) {

into 

      while ( (obj = findObject( name )) != NULL  ) {
         if (obj->id() == pObject->id())
            return false;

which checks the uniq object id and skip adding if the object is already present and the xmi file could be loaded without any problems. 

If the main issue of hidden "autocreating" of objects could not be solved, from my current umbrello knowledge I would say that the search should use the object id instead os using the name. 
</excurse>

Comment 1 Oliver Kellogg 2012-01-10 18:54:07 UTC

See also bug 56184, and in particular the XMI files from different tools attached there.

When changing loadFromXMI code, IMHO it would be a good idea to test loading those files. (It often happened that changing the code to load one particular XMI dialect ruined the loading of other XMI dialects.)

Also be aware that Umbrello's XMI loading code cannot handle forward declared objects.

Comment 2 Ralf Habacker 2012-01-10 19:08:54 UTC

I strongly suggest to add a quality audit to the source code and build system 

One way would be 
1. extend umbrello to have dbus methods for loading, importing, saving and dumping loaded files. 
2. add many xmi templates to the source 
3. extend the build system to run a test suite, which loads those files through qdbus, saves the files again or dumps the loaded content in an internal comparable format (something like the QDebug operator<<) and compare the result with expected files. 

This will make development more robust.

Comment 3 Oliver Kellogg 2012-02-29 06:18:28 UTC

SVN commit 1282856 by okellogg:

UMLDoc::loadFromXMI (tagEq(tag, "Package") ||
                     tagEq(tag, "Class") ||
                     tagEq(tag, "Interface")) :
Do not call loadUMLObjectsFromXMI() because that method again iterates over
the child nodes of the element (not appropriate here because we already
identified the exact tag to load.)
Instead, use Object_Factory::makeObjectFromXMI() for creating and
UMLObject::loadFromXMI() for loading the object.



 M  +1 -0      ChangeLog  
 M  +35 -3     umbrello/umldoc.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1282856

Comment 4 Ralf Habacker 2013-11-06 17:24:09 UTC

set version-fixed-in from 4.8.1 changelog