Bug 117875 - Imported C++ classes not saved correctly in the XMI file
Summary: Imported C++ classes not saved correctly in the XMI file
Status: RESOLVED FIXED
Alias: None
Product: umbrello
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Fedora RPMs Linux
: NOR normal
Target Milestone: ---
Assignee: Oliver Kellogg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-12-07 17:51 UTC by Maciej Puzio
Modified: 2005-12-14 20:40 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
C++ Header file mentioned in the bug report (3.75 KB, application/x-bzip2)
2005-12-07 17:53 UTC, Maciej Puzio
Details
XMI file generated by Umbrello, as mentioned in the bug report (17.38 KB, application/x-bzip2)
2005-12-07 17:53 UTC, Maciej Puzio
Details
First screenshot mentioned in the bug report (178.75 KB, application/x-bzip2)
2005-12-07 17:54 UTC, Maciej Puzio
Details
Second screenshot mentioned in the bug report (94.85 KB, application/x-bzip2)
2005-12-07 17:55 UTC, Maciej Puzio
Details
classimport.cpp.diff (224 bytes, patch)
2005-12-14 01:43 UTC, Maciej Puzio
Details
cpptree2uml.cpp.diff (187 bytes, patch)
2005-12-14 01:44 UTC, Maciej Puzio
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Puzio 2005-12-07 17:51:41 UTC
Version:           1.4.3 (using KDE KDE 3.4.0)
Installed from:    Fedora RPMs
Compiler:          gcc (GCC) 4.0.2 20051125 (Red Hat 4.0.2-8) 
OS:                Linux

Importing a complex C++ header (attached zCDataTypes.H) works fine, but when the model is saved and then reopened, most of the classes are missing. If these classes were included in a class diagram, they are missing from the diagram as well, rendering it incomplete.

Steps to reproduce the problem:
1. Start Umbrello.
2. Import the attached zCDataTypes.H file by selecting Code | Import C++ Classes from the menu. I attach a screenshot (screenshot1.png) showing a list of classes (as well as a simple diagram that I created by dragging 3 classes from the list onto the diagram area).
3. Save the model by selecting File | Save As from the menu. I attach the test.xmi file that is the result of this step.
4. Close the model by selecting File | Close.
5. Reopen the model by using File | Open. (The same problem appears when I exit Umbrello and restart it.)
6. Note that several classes are missing from the "Logical View" list. Also 2 classes are missing from the diagram. I attach the screenshot (screenshot2.png).

I apologize for the complexity of the header file code. I am attempting to analyze a 100,000 line program written by another programmer by creating a map of classes, in order to redesign the inheritance structure. Unfortunately, the problem that I describe renders Umbrello unusable for this task.

Umbrello reports (in Help | About Umbrello UML Modeller):
Umbrello UML Modeller 1.4.2 (Using KDE 3.4.0-6 Red Hat)
However, the program was compiled and installed from the file umbrello-1.4.3.tar.bz2, which was the newest available at the time of this writing.

System (uname -a):
Linux nano.swmed.edu 2.6.14-1.1644_FC4smp #1 SMP Sun Nov 27 03:39:31 EST 2005 i686 i686 i386 GNU/Linux
Comment 1 Maciej Puzio 2005-12-07 17:53:09 UTC
Created attachment 13808 [details]
C++ Header file mentioned in the bug report
Comment 2 Maciej Puzio 2005-12-07 17:53:54 UTC
Created attachment 13809 [details]
XMI file generated by Umbrello, as mentioned in the bug report
Comment 3 Maciej Puzio 2005-12-07 17:54:40 UTC
Created attachment 13810 [details]
First screenshot mentioned in the bug report
Comment 4 Maciej Puzio 2005-12-07 17:55:07 UTC
Created attachment 13811 [details]
Second screenshot mentioned in the bug report
Comment 5 Maciej Puzio 2005-12-14 01:41:29 UTC
I have found two bugs in Umbrello that were causing the behavior that I described in this report. I have fixed these bugs and tested the program (patches attached).

The bugs were surfacing because of the sequence of class declarations in zCDataTypes.H. Several classes are declared as part of namespace zMol, first by forward declarations, and then by full class declarations (e.g. class zCAnySpaceData). As a result of 2 bugs that I've found, these classes were not correctly included in programs's data structures, and resultantly were omitted from the XMI file during the save operation. Below are the details.

The direct reason why some classes were not saved was that they had the package attribute (UMLObject::m_pUMLPackage) set correctly to the name of the namespace in which they were declared, but did not belong to the list of children of that package (UMLPackage::m_objects). Because of that they were skipped in both UMLDoc::saveToXMI (umldoc.cpp line 1636), and in UMLPackage::saveToXMI (package.cpp lines 172-177, called from UMLDoc::saveToXMI, line 1656). Code in both saveToXMI methods is correct, and the problem is the m_pUMLPackage/m_objects anomaly.

That anomaly was caused by the bug in ClassImport::createUMLObject (classimport.cpp lines 167-169). Class zCAnySpaceData (and other like it) are processed through createUMLObject twice, first as a result of a forward declaration, and then as a result of a full declaration. As a result of a second bug, that I describe later, namespace was ignored for the forward declaration. During the second pass through createUMLObject, namespace was provided correctly, and this resulted in UMLObject being reassigned to a different UMLPackage. However, only UMLObject::m_pUMLPackage was assigned, and the object was not added to the UMLPackage::m_object list. That created the anomaly that was preventing object from being saved.

The patch (also attached as classimport.cpp.diff):
168c168,171
< 		o->setUMLPackage(parentPkg);
---
>             if (o->getUMLPackage())
>                 o->getUMLPackage()->removeObject(o);
>             o->setUMLPackage(parentPkg);
>             parentPkg->addObject(o);

As a side note, I am not sure that reassigning classes from one namespace to another is a correct behavior. If there are 2 identically named classes declared in 2 namespaces, they should be present in the tree as 2 distinct objects, rather than one object in the namespace that was seen last, as the code appears to be doing now.

The second bug is in CppTree2Uml::parseElaboratedTypeSpecifier (classparser/cpptree2uml.cpp, line 404). The code ignores the current namespace (m_currentNamespace[m_nsCtl], which results in class being assigned to the empty namespace (NULL). 

Patch (also attached as cpptree2uml.cpp.diff):

404c404
<     UMLObject *o = m_importer->createUMLObject( Uml::ot_Class, text );
---
>     UMLObject *o = m_importer->createUMLObject( Uml::ot_Class, text, m_currentNamespace[m_nsCnt] );

Please forgive me any deficiencies in this report; I am a newbie to Umbrello, KDE and UML. I am not sure whether I should leave this report as UNCONFIRMED (it is quite clear that the bugs exist, and where), or change it to RESOLVED. While the issue has been resolved for the copy of Umbrello that I have, the bugs persist in the publicly available release. (?)
Comment 6 Maciej Puzio 2005-12-14 01:43:37 UTC
Created attachment 13905 [details]
classimport.cpp.diff
Comment 7 Maciej Puzio 2005-12-14 01:44:21 UTC
Created attachment 13906 [details]
cpptree2uml.cpp.diff
Comment 8 Oliver Kellogg 2005-12-14 20:18:04 UTC
Ah, those are the bugs I like - self fixing ;-)
Thanks for the excellent work tracking this down.
Comment 9 Oliver Kellogg 2005-12-14 20:40:48 UTC
SVN commit 488516 by okellogg:

CppTree2Uml::parseElaboratedTypeSpecifier(): Supply the m_currentNamespace
 at the call to Import_Utils::createUMLObject().
Import_Utils::createUMLObject(): Add missing call to parentPkg->addObject(o).
Thanks to Maciej Puzio for providing a very well written PR and solution.
BUG:117875


 M  +1 -1      ChangeLog  
 M  +1 -0      THANKS  
 M  +1 -1      umbrello/classparser/cpptree2uml.cpp  
 M  +3 -0      umbrello/import_utils.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/ChangeLog #488515:488516
@@ -1,7 +1,7 @@
 Version 1.5.1
 
 * Bugs fixed / wishes implemented (see http://bugs.kde.org)
-109963 117791
+109963 117875 117791
 
 Version 1.5
 
--- branches/KDE/3.5/kdesdk/umbrello/THANKS #488515:488516
@@ -52,6 +52,7 @@
 Dimitri Ognibene <ognibened @yahoo.it>
 Carsten Pfeiffer <pfeiffer @kde.org>
 Ivan Porres <iporres @abo.fi>
+Maciej Puzio <maciek @work.swmed.edu>
 Ruediger Ranft <kdebugs @rranft1.mail.htwm.de>
 John Ratke <jratke @comcast.net>
 Daniel Richard G. <skunk @iskunk.org>
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/classparser/cpptree2uml.cpp #488515:488516
@@ -406,7 +406,7 @@
     QString text = typeSpec->text();
     kdDebug() << "CppTree2Uml::parseElaboratedTypeSpecifier: text is " << text << endl;
     text.remove(QRegExp("^class\\s+"));
-    UMLObject *o = Import_Utils::createUMLObject( Uml::ot_Class, text );
+    UMLObject *o = Import_Utils::createUMLObject(Uml::ot_Class, text, m_currentNamespace[m_nsCnt]);
     flushTemplateParams( static_cast<UMLClassifier*>(o) );
 }
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/import_utils.cpp #488515:488516
@@ -200,7 +200,10 @@
             o = origType;
         }
     } else if (parentPkg && !bPutAtGlobalScope) {
+        if (o->getUMLPackage())
+            o->getUMLPackage()->removeObject(o);
         o->setUMLPackage(parentPkg);
+        parentPkg->addObject(o);
     }
     QString strippedComment = formatComment(comment);
     if (! strippedComment.isEmpty()) {