Bug 130172

Created attachment 16869 [details]
A diagram with some duplicated labels.

Yes, while working on other models I saw this too

SVN commit 651654 by okellogg:

Do not emit the modified signal during load from XMI.
BUG:130172


 M  +7 -12     ChangeLog  
 M  +3 -1      umbrello/association.cpp  
 M  +3 -1      umbrello/attribute.cpp  
 M  +46 -16    umbrello/classifier.cpp  
 M  +6 -2      umbrello/classifierlistitem.cpp  
 M  +19 -7     umbrello/entity.cpp  
 M  +19 -7     umbrello/enum.cpp  
 M  +3 -1      umbrello/operation.cpp  
 M  +1 -1      umbrello/umlcanvasobject.cpp  
 M  +24 -8     umbrello/umlobject.cpp  
 M  +9 -3      umbrello/umlrole.cpp  
 M  +2 -0      umbrello/umlview.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/ChangeLog #651653:651654
@@ -2,18 +2,16 @@
 
 * Bugs/wishes from http://bugs.kde.org:
 * C# Code Generation and export (53368)
-* Java interface inheritance, abstract classes and generics in code generation
-  (53376)
+* Java interface inheritance, abstract classes and generics in code generation (53376)
 * Code generation ignores unidirectional association (72042)
 * Design bug in advanced code generators (84739)
 * %date% and %time% not being parsed (96612)
-* Operations of the Interface are not implemented in the class automatically
-  (111593)
+* Operations of the Interface are not implemented in the class automatically (111593)
 * Relationships for entities do not live outside of the diagram (125146)
 * Crashes while loading file (Advanced Code Generators enabled) (126262)
 * Multiplicity labels often are placed incorrectly (127628)
-* Crash on adding operation to class with Advanced Code Generators enabled
-  (131528)
+* Association role labels are duplicated (130172)
+* Crash on adding operation to class with Advanced Code Generators enabled (131528)
 * Javascript wrong Code Generation (135527)
 * Javascript Code Generation creates bad format methods (135540)
 * Incorrect Association Properties text (139872)
@@ -22,14 +20,11 @@
 * Associations not updated during move of class on diagram (140709)
 * Crash when deleting the link between a package and a class (141602)
 * Ada95 Code Generation Errors for Aggregation (141644)
-* Unable to delete multiplicity information or label from an association
-  (141813)
+* Unable to delete multiplicity information or label from an association (141813)
 * Reinstate code generation options for C++ (141875)
 * C++ code generator does not correctly define namespaces (141876)
-* Ada code generator generates "withs" in both directions for certain
-  associations (141956)
-* Ada code generator always generates methods abstract even if abstract box
-  not checked (142093)
+* Ada code generator generates "withs" in both directions for certain associations (141956)
+* Ada code generator always generates methods abstract even if abstract box not checked (142093)
 * Missing "with" on Ada code generation for aggregation (142392)
 * Operation Properties "Type" combo box too small (143319)
 * Support duplication of diagrams (143581)
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/association.cpp #651653:651654
@@ -514,7 +514,9 @@
         kDebug() << " A new uni-association has been created." << endl;
 #endif
     }
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLAssociation::setObject(UMLObject *obj, Role_Type role) {
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/attribute.cpp #651653:651654
@@ -59,7 +59,9 @@
 void UMLAttribute::setInitialValue(const QString &iv) {
     if(m_InitialValue != iv) {
         m_InitialValue = iv;
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading())
+            emit modified();
     }
 }
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/classifier.cpp #651653:651654
@@ -244,8 +244,11 @@
         kDebug() << "  UMLClassifier::addOperation list after change: " << buf << endl;
      } else
         m_List.append( op );
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
     emit operationAdded(op);
-    emit modified();
     connect(op,SIGNAL(modified()),this,SIGNAL(modified()));
     return true;
 }
@@ -273,8 +276,11 @@
     // disconnection needed.
     // note that we don't delete the operation, just remove it from the Classifier
     disconnect(op,SIGNAL(modified()),this,SIGNAL(modified()));
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
     emit operationRemoved(op);
-    emit modified();
     return m_List.count();
 }
 
@@ -548,9 +554,12 @@
     Uml::Visibility scope = Settings::getOptionState().classState.defaultAttributeScope;
     UMLAttribute *a = new UMLAttribute(this, name, id, scope);
     m_List.append(a);
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
+    emit attributeAdded(a);
     connect(a,SIGNAL(modified()),this,SIGNAL(modified()));
-    emit attributeAdded(a);
     return a;
 }
 
@@ -562,9 +571,12 @@
     if (type)
         a->setType(type);
     m_List.append(a);
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
+    emit attributeAdded(a);
     connect(a,SIGNAL(modified()),this,SIGNAL(modified()));
-    emit attributeAdded(a);
     return a;
 }
 
@@ -577,9 +589,12 @@
             m_List.insert(position, att);
         else
             m_List.append(att);
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading()) {
+            emit modified();
+        }
+        emit attributeAdded(att);
         connect(att, SIGNAL(modified()), this, SIGNAL(modified()));
-        emit attributeAdded(att);
         return true;
     } else if (Log) {
         Log->removeChangeByNewID(att->getID());
@@ -593,8 +608,11 @@
         kDebug() << "can't find att given in list" << endl;
         return -1;
     }
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
     emit attributeRemoved(a);
-    emit modified();
     // If we are deleting the object, then we don't need to disconnect..this is done auto-magically
     // for us by QObject. -b.t.
     // disconnect(a,SIGNAL(modified()),this,SIGNAL(modified()));
@@ -669,9 +687,12 @@
         return t;
     t = new UMLTemplate(this, name, id);
     m_List.append(t);
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
+    emit templateAdded(t);
     connect(t, SIGNAL(modified()), this, SIGNAL(modified()));
-    emit templateAdded(t);
     return t;
 }
 
@@ -681,9 +702,12 @@
         newTemplate->parent()->removeChild(newTemplate);
         this->insertChild(newTemplate);
         m_List.append(newTemplate);
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading()) {
+            emit modified();
+        }
+        emit templateAdded(newTemplate);
         connect(newTemplate,SIGNAL(modified()),this,SIGNAL(modified()));
-        emit templateAdded(newTemplate);
         return true;
     } else if (log) {
         log->removeChangeByNewID( newTemplate->getID() );
@@ -702,9 +726,12 @@
             m_List.insert(position,Template);
         else
             m_List.append(Template);
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading()) {
+            emit modified();
+        }
+        emit templateAdded(Template);
         connect(Template,SIGNAL(modified()),this,SIGNAL(modified()));
-        emit templateAdded(Template);
         return true;
     }
     //else
@@ -716,8 +743,11 @@
         kWarning() << "can't find att given in list" << endl;
         return -1;
     }
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
     emit templateRemoved(umltemplate);
-    emit modified();
     disconnect(umltemplate,SIGNAL(modified()),this,SIGNAL(modified()));
     return m_List.count();
 }
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/classifierlistitem.cpp #651653:651654
@@ -65,7 +65,9 @@
 void UMLClassifierListItem::setType(UMLObject *type) {
     if (m_pSecondary != type) {
         m_pSecondary = type;
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading())
+            emit modified();
     }
 }
 
@@ -87,7 +89,9 @@
             m_SecondaryId = type;
         }
     }
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/entity.cpp #651653:651654
@@ -97,9 +97,12 @@
 UMLObject* UMLEntity::addEntityAttribute(const QString& name, Uml::IDType id) {
     UMLEntityAttribute* literal = new UMLEntityAttribute(this, name, id);
     m_List.append(literal);
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
+    emit entityAttributeAdded(literal);
     connect(literal,SIGNAL(modified()),this,SIGNAL(modified()));
-    emit entityAttributeAdded(literal);
     return literal;
 }
 
@@ -109,9 +112,12 @@
         attribute->parent()->removeChild(attribute);
         this->insertChild(attribute);
         m_List.append(attribute);
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading()) {
+            emit modified();
+        }
+        emit entityAttributeAdded(attribute);
         connect(attribute,SIGNAL(modified()),this,SIGNAL(modified()));
-        emit entityAttributeAdded(attribute);
         return true;
     } else if (Log) {
         Log->removeChangeByNewID( attribute->getID() );
@@ -130,9 +136,12 @@
         } else {
             m_List.append(attribute);
         }
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading()) {
+            emit modified();
+        }
+        emit entityAttributeAdded(attribute);
         connect(attribute,SIGNAL(modified()),this,SIGNAL(modified()));
-        emit entityAttributeAdded(attribute);
         return true;
     }
     return false;
@@ -143,7 +152,10 @@
         kDebug() << "can't find att given in list" << endl;
         return -1;
     }
-    emit entityAttributeRemoved(literal);
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit entityAttributeRemoved(literal);
+    }
     emit modified();
     // If we are deleting the object, then we don't need to disconnect..this is done auto-magically
     // for us by QObject. -b.t.
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/enum.cpp #651653:651654
@@ -89,9 +89,12 @@
     }
     UMLEnumLiteral* literal = new UMLEnumLiteral(this, name, id);
     m_List.append(literal);
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
+    emit enumLiteralAdded(literal);
     connect(literal,SIGNAL(modified()),this,SIGNAL(modified()));
-    emit enumLiteralAdded(literal);
     return literal;
 }
 
@@ -101,9 +104,12 @@
         literal->parent()->removeChild(literal);
         this->insertChild(literal);
         m_List.append(literal);
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading()) {
+            emit modified();
+        }
+        emit enumLiteralAdded(literal);
         connect(literal,SIGNAL(modified()),this,SIGNAL(modified()));
-        emit enumLiteralAdded(literal);
         return true;
     } else if (Log) {
         Log->removeChangeByNewID( literal->getID() );
@@ -122,9 +128,12 @@
         } else {
             m_List.append(literal);
         }
-        emit modified();
+        UMLDoc *umldoc = UMLApp::app()->getDocument();
+        if (! umldoc->loading()) {
+            emit modified();
+        }
+        emit enumLiteralAdded(literal);
         connect(literal,SIGNAL(modified()),this,SIGNAL(modified()));
-        emit enumLiteralAdded(literal);
         return true;
     }
     return false;
@@ -135,8 +144,11 @@
         kDebug() << "can't find att given in list" << endl;
         return -1;
     }
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading()) {
+        emit modified();
+    }
     emit enumLiteralRemoved(literal);
-    emit modified();
     // If we are deleting the object, then we don't need to disconnect..this is done auto-magically
     // for us by QObject. -b.t.
     // disconnect(a,SIGNAL(modified()),this,SIGNAL(modified()));
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/operation.cpp #651653:651654
@@ -180,7 +180,9 @@
         m_List.insert(position,parameter);
     else
         m_List.append( parameter );
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
     connect(parameter,SIGNAL(modified()),this,SIGNAL(modified()));
 }
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlcanvasobject.cpp #651653:651654
@@ -69,8 +69,8 @@
         UMLDoc *umldoc = UMLApp::app()->getDocument();
         if (! umldoc->loading()) {
             emit modified();
-            emit sigAssociationEndAdded(assoc);
         }
+        emit sigAssociationEndAdded(assoc);
         return true;
     }
     return false;
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlobject.cpp #651653:651654
@@ -94,12 +94,16 @@
 
 void UMLObject::setID(Uml::IDType NewID) {
     m_nId = NewID;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLObject::setName(const QString &strName) {
     m_Name = strName;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 QString UMLObject::getName() const {
@@ -211,7 +215,9 @@
 
 void UMLObject::setAbstract(bool bAbstract) {
     m_bAbstract = bAbstract;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLObject::setInPaste(bool bInPaste /* =true */) {
@@ -227,12 +233,16 @@
 void UMLObject::setStatic(bool bStatic)
 {
     m_bStatic = bStatic;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLObject::emitModified()
 {
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLObject::setDoc(const QString &d) {
@@ -262,7 +272,9 @@
 
 void UMLObject::setVisibility(Uml::Visibility s) {
     m_Vis = s;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLObject::setUMLStereotype(UMLStereotype *stereo) {
@@ -281,7 +293,9 @@
     }
     m_pStereotype = stereo;
     // TODO: don't emit modified() if predefined folder
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLObject::setStereotype(const QString &_name) {
@@ -319,7 +333,9 @@
 
 void UMLObject::setUMLPackage(UMLPackage* pPkg) {
     m_pUMLPackage = pPkg;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 const UMLStereotype * UMLObject::getUMLStereotype() {
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlrole.cpp #651653:651654
@@ -65,17 +65,23 @@
     }
 
     m_pSecondary = obj;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLRole::setChangeability (Uml::Changeability_Type value) {
     m_Changeability = value;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 void UMLRole::setMultiplicity ( const QString &multi ) {
     m_Multi = multi;
-    emit modified();
+    UMLDoc *umldoc = UMLApp::app()->getDocument();
+    if (! umldoc->loading())
+        emit modified();
 }
 
 Uml::Role_Type UMLRole::getRole() {
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlview.cpp #651653:651654
@@ -1444,6 +1444,8 @@
     if( !pWidget ) {
         return false;
     }
+    if (pWidget->getName() == "bacx def")
+        kDebug() << "bacx: id=" << ID2STR(pWidget->getID()) << endl;
     if (!isPasteOperation && findWidget(pWidget->getID())) {
         kError() << "UMLView::addWidget: Not adding "
                   << "(id=" << ID2STR(pWidget->getID())

SVN commit 672632 by okellogg:

findAssocWidget(UMLWidget*,UMLWidget*): Add a further arg, the role B name.
This fixes association label duplications when adding multiple attributes
of the same type at a classifier, when both that classifier and the
attribute type classifier are present on a class diagram.
CCBUG:130172


 M  +21 -23    umlview.cpp  
 M  +12 -3     umlview.h  


--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlview.cpp #672631:672632
@@ -764,17 +764,24 @@
     return 0;
 }
 
-AssociationWidget * UMLView::findAssocWidget(UMLWidget *pWidgetA, UMLWidget *pWidgetB) {
-    static QValueList<Association_Type> assocTypes;
-    if (assocTypes.isEmpty()) {
-        assocTypes << Uml::at_Aggregation << Uml::at_Composition << Uml::at_Containment;
+AssociationWidget * UMLView::findAssocWidget(UMLWidget *pWidgetA,
+                                             UMLWidget *pWidgetB, const QString& roleNameB) {
+    AssociationWidget *assoc;
+    AssociationWidgetListIt it(m_AssociationList);
+    while ((assoc = it.current()) != 0) {
+        ++it;
+        const Association_Type testType = assoc->getAssocType();
+        if (testType != Uml::at_Association &&
+            testType != Uml::at_UniAssociation &&
+            testType != Uml::at_Composition &&
+            testType != Uml::at_Aggregation)
+            continue;
+        if (pWidgetA->getID() == assoc->getWidgetID(A) &&
+            pWidgetB->getID() == assoc->getWidgetID(B) &&
+            assoc->getRoleName(Uml::B) == roleNameB)
+            return assoc;
     }
-    AssociationWidget* retval = NULL;
-    for (uint i=0; i < assocTypes.size(); ++i) {
-        retval = findAssocWidget(assocTypes[i], pWidgetA, pWidgetB);
-        if (retval != NULL) return retval;
-    }
-    return retval;
+    return 0;
 }
 
 
@@ -790,15 +797,6 @@
         if (pWidgetA->getID() == assoc->getWidgetID(A) &&
                 pWidgetB->getID() == assoc->getWidgetID(B))
             return assoc;
-        // Allow for the swapped roles of generalization/realization assocwidgets.
-        // When the swapped roles bug is fixed, this code can disappear.
-        if (pWidgetA->getID() == assoc->getWidgetID(B) &&
-                pWidgetB->getID() == assoc->getWidgetID(A)) {
-            kDebug() << "UMLView::findAssocWidget: found assoctype " << at
-                      << "with swapped roles (A: " << pWidgetA->getName()
-                      << ", B: " << pWidgetB->getName() << ")" << endl;
-            return assoc;
-        }
     }
     return 0;
 }
@@ -2147,7 +2145,7 @@
     UMLAttributeList attrList = klass->getAttributeList();
     for (UMLAttributeListIt ait(attrList); ait.current(); ++ait) {
         UMLAttribute *attr = ait.current();
-        createAutoAttributeAssociation(attr->getType(), attr,widget);
+        createAutoAttributeAssociation(attr->getType(), attr, widget);
         /*
          * The following code from attachment 19935 [details] of http://bugs.kde.org/140669
          * creates Aggregation/Composition to the template parameters.
@@ -2174,8 +2172,8 @@
     AssociationWidget *aw = NULL;
     // if the attribute type has a widget representation on this view
     if (w) {
-        aw = findAssocWidget(widget, w) ;
-        if ( ( aw == NULL || aw->getRoleName(Uml::B) != attr->getName() ) &&
+        aw = findAssocWidget(widget, w, attr->getName());
+        if ( aw == NULL &&
                // if the current diagram type permits compositions
                AssocRules::allowAssociation(assocType, widget, w, false) ) {
             // Create a composition AssocWidget, or, if the attribute type is
@@ -2206,7 +2204,7 @@
             UMLWidget *w = c ? findWidget( c->getID() ) : 0;
             // if the referenced type has a widget representation on this view
             if (w) {
-                aw = findAssocWidget(widget, w);
+                aw = findAssocWidget(widget, w, attr->getName());
                 if (aw == NULL &&
                     // if the current diagram type permits aggregations
                     AssocRules::allowAssociation(at_Aggregation, widget, w, false)) {
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umlview.h #672631:672632
@@ -399,10 +399,19 @@
                                         UMLWidget *pWidgetA, UMLWidget *pWidgetB);
 
     /**
-     * calls findAssocWidget on three possible types:
-     * Uml::at_Aggregation << Uml::at_Composition << Uml::at_Containment
+     * Finds an association widget with the given widgets and the given role B name.
+     * Considers the following association types:
+     *  at_Association, at_UniAssociation, at_Composition, at_Aggregation
+     * This is used for seeking an attribute association.
+     *
+     * @param pWidgetA  Pointer to the UMLWidget of role A.
+     * @param pWidgetB  Pointer to the UMLWidget of role B.
+     * @param roleNameB Name at the B side of the association (the attribute name)
+     *
+     * @return Returns the widget found, returns 0 if no widget found.
      */
-    AssociationWidget * findAssocWidget(UMLWidget *pWidgetA, UMLWidget *pWidgetB);
+    AssociationWidget * findAssocWidget(UMLWidget *pWidgetA,
+                                        UMLWidget *pWidgetB, const QString& roleNameB);
 
     /**
      * Remove a widget from view.