QmlDesigner: Extract unique property name by collection name

- The property names of the data store are defined after converting
collection names to a proper unique property name.
- Now, Spaces are supported in the collection names.
- Collection property names will remain the same by collection
renames, and only the modelName will change

Task-number: QDS-11462
Change-Id: I2031c2e0a9afc5388386dc6e54c66e75f0d13ded
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
Reviewed-by: Qt CI Patch Build Bot <ci_patchbuild_bot@qt.io>
Reviewed-by: Shrief Gabr <shrief.gabr@qt.io>

share/qtcreator/qmldesigner/collectionEditorQmlSource/ImportDialog.qml

@@ -120,7 +120,7 @@ StudioControls.Dialog {
             actionIndicator.visible: false
             translationIndicator.visible: false
             validator: RegularExpressionValidator {
-                regularExpression: /^\w+$/
+                regularExpression: /^[\w ]+$/
             }
 
             Keys.onEnterPressed: btnImport.onClicked()

share/qtcreator/qmldesigner/collectionEditorQmlSource/NewCollectionDialog.qml

@@ -87,7 +87,7 @@ StudioControls.Dialog {
             actionIndicator.visible: false
             translationIndicator.visible: false
             validator: RegularExpressionValidator {
-                regularExpression: /^\w+$/
+                regularExpression: /^[\w ]+$/
             }
 
             Keys.onEnterPressed: btnCreate.onClicked()

src/plugins/qmldesigner/components/collectioneditor/collectioneditorutils.cpp

@@ -3,11 +3,9 @@
 
 #include "collectioneditorutils.h"
 
-#include "abstractview.h"
+#include "model.h"
-#include "bindingproperty.h"
 #include "nodemetainfo.h"
 #include "propertymetainfo.h"
-#include "variantproperty.h"
 
 #include <variant>
 
@@ -137,33 +135,6 @@ QString getSourceCollectionType(const ModelNode &node)
     return {};
 }
 
-void assignCollectionToNode(AbstractView *view,
-                            const ModelNode &modelNode,
-                            const ModelNode &collectionSourceNode,
-                            const QString &collectionName)
-{
-    QTC_ASSERT(modelNode.isValid() && collectionSourceNode.isValid(), return);
-
-    QString sourceId = isDataStoreNode(collectionSourceNode) ? "DataStore"
-                                                             : collectionSourceNode.id();
-
-    if (sourceId.isEmpty() || !canAcceptCollectionAsModel(modelNode))
-        return;
-
-    VariantProperty sourceProperty = collectionSourceNode.variantProperty(collectionName.toLatin1());
-    if (!sourceProperty.exists())
-        return;
-
-    BindingProperty modelProperty = modelNode.bindingProperty("model");
-
-    QString identifier = QString("%1.%2").arg(sourceId, QString::fromLatin1(sourceProperty.name()));
-
-    view->executeInTransaction("CollectionEditor::assignCollectionToNode",
-                               [&modelProperty, &identifier]() {
-                                   modelProperty.setExpression(identifier);
-                               });
-}
-
 Utils::FilePath dataStoreJsonFilePath()
 {
     return collectionPath("models.json");

src/plugins/qmldesigner/components/collectioneditor/collectioneditorutils.h

@@ -24,11 +24,6 @@ QString getSourceCollectionType(const QmlDesigner::ModelNode &node);
 
 QString getSourceCollectionPath(const QmlDesigner::ModelNode &dataStoreNode);
 
-void assignCollectionToNode(AbstractView *view,
-                            const ModelNode &modelNode,
-                            const ModelNode &collectionSourceNode,
-                            const QString &collectionName);
-
 Utils::FilePath dataStoreJsonFilePath();
 
 Utils::FilePath dataStoreQmlFilePath();

src/plugins/qmldesigner/components/collectioneditor/collectionsourcemodel.cpp

@@ -403,8 +403,7 @@ void CollectionSourceModel::onSelectedCollectionChanged(CollectionListModel *col
 
         m_previousSelectedList = collectionList;
 
-        emit collectionSelected(collectionList->sourceNode(),
+        emit collectionSelected(collectionList->collectionNameAt(collectionIndex));
-                                collectionList->collectionNameAt(collectionIndex));
 
         selectSourceIndex(sourceIndex(collectionList->sourceNode()));
     }
@@ -494,6 +493,7 @@ void CollectionSourceModel::onCollectionNameChanged(CollectionListModel *collect
             return;
         }
 
+        emit collectionRenamed(oldName, newName);
         updateCollectionList(nodeIndex);
     }
 }
@@ -547,10 +547,12 @@ void CollectionSourceModel::onCollectionsRemoved(CollectionListModel *collection
     if (document.isObject()) {
         QJsonObject rootObject = document.object();
 
+        QStringList collectionsRemovedFromDocument;
         for (const QString &collectionName : removedCollections) {
             bool sourceContainsCollection = rootObject.contains(collectionName);
             if (sourceContainsCollection) {
                 rootObject.remove(collectionName);
+                collectionsRemovedFromDocument << collectionName;
             } else {
                 emitDeleteWarning(tr("The model group doesn't contain the model name (%1).")
                                       .arg(sourceContainsCollection));
@@ -572,6 +574,9 @@ void CollectionSourceModel::onCollectionsRemoved(CollectionListModel *collection
             return;
         }
 
+        for (const QString &collectionName : std::as_const(collectionsRemovedFromDocument))
+            emit this->collectionRemoved(collectionName);
+
         updateCollectionList(nodeIndex);
     }
 }
@@ -602,7 +607,7 @@ void CollectionSourceModel::setSelectedIndex(int idx)
             } else if (m_previousSelectedList) {
                 m_previousSelectedList->selectCollectionIndex(-1);
                 m_previousSelectedList = {};
-                emit this->collectionSelected(sourceNodeAt(idx), {});
+                emit this->collectionSelected({});
             }
         }
     }
@@ -626,12 +631,12 @@ void CollectionSourceModel::updateCollectionList(QModelIndex index)
         return;
 
     ModelNode sourceNode = sourceNodeAt(index.row());
-    QSharedPointer<CollectionListModel> currentList = m_collectionList.at(index.row());
+    QSharedPointer<CollectionListModel> oldList = m_collectionList.at(index.row());
-    QSharedPointer<CollectionListModel> newList = loadCollection(sourceNode, currentList);
+    QSharedPointer<CollectionListModel> newList = loadCollection(sourceNode, oldList);
-    if (currentList != newList) {
+    if (oldList != newList) {
         m_collectionList.replace(index.row(), newList);
         emit dataChanged(index, index, {CollectionsRole});
-        emit collectionNamesChanged(sourceNode, newList->stringList());
+        registerCollection(newList);
     }
 }
 
@@ -656,8 +661,8 @@ void CollectionSourceModel::registerCollection(const QSharedPointer<CollectionLi
             onCollectionsRemoved(collectionList, removedCollections);
         }, Qt::UniqueConnection);
 
-    if (collection->sourceNode())
+    if (collectionList->sourceNode().isValid())
-        emit collectionNamesChanged(collection->sourceNode(), collection->stringList());
+        emit collectionNamesInitialized(collection->stringList());
 }
 
 QModelIndex CollectionSourceModel::indexOfNode(const ModelNode &node) const

src/plugins/qmldesigner/components/collectioneditor/collectionsourcemodel.h

@@ -70,8 +70,11 @@ public:
 
 signals:
     void selectedIndexChanged(int idx);
-    void collectionSelected(const ModelNode &sourceNode, const QString &collectionName);
+    void collectionSelected(const QString &collectionName);
-    void collectionNamesChanged(const ModelNode &sourceNode, QStringList collections);
+    void collectionNamesInitialized(const QStringList &initialList);
+    void collectionRenamed(const QString &oldname, const QString &newName);
+    void collectionRemoved(const QString &collectionName);
+
     void isEmptyChanged(bool);
     void warning(const QString &title, const QString &body);
 

src/plugins/qmldesigner/components/collectioneditor/collectionview.cpp

@@ -70,8 +70,8 @@ QmlDesigner::WidgetInfo CollectionView::widgetInfo()
         connect(sourceModel,
                 &CollectionSourceModel::collectionSelected,
                 this,
-                [this](const ModelNode &sourceNode, const QString &collection) {
+                [this](const QString &collection) {
-                    m_widget->collectionDetailsModel()->loadCollection(sourceNode, collection);
+                    m_widget->collectionDetailsModel()->loadCollection(dataStoreNode(), collection);
                 });
 
         connect(sourceModel, &CollectionSourceModel::isEmptyChanged, this, [this](bool isEmpty) {
@@ -80,12 +80,25 @@ QmlDesigner::WidgetInfo CollectionView::widgetInfo()
         });
 
         connect(sourceModel,
-                &CollectionSourceModel::collectionNamesChanged,
+                &CollectionSourceModel::collectionNamesInitialized,
                 this,
-                [this](const ModelNode &sourceNode, const QStringList &collectionNames) {
+                [this](const QStringList &collectionNames) {
-                    if (sourceNode == m_dataStore->modelNode())
                     m_dataStore->setCollectionNames(collectionNames);
                 });
+
+        connect(sourceModel,
+                &CollectionSourceModel::collectionRenamed,
+                this,
+                [this](const QString &oldName, const QString &newName) {
+                    m_dataStore->renameCollection(oldName, newName);
+                });
+
+        connect(sourceModel,
+                &CollectionSourceModel::collectionRemoved,
+                this,
+                [this](const QString &collectionName) {
+                    m_dataStore->removeCollection(collectionName);
+                });
     }
 
     return createWidgetInfo(m_widget.data(),
@@ -200,6 +213,12 @@ void CollectionView::addResource(const QUrl &url, const QString &name, const QSt
     });
 }
 
+void CollectionView::assignCollectionToSelectedNode(const QString &collectionName)
+{
+    QTC_ASSERT(dataStoreNode() && hasSingleSelectedModelNode(), return);
+    m_dataStore->assignCollectionToNode(this, singleSelectedModelNode(), collectionName);
+}
+
 void CollectionView::registerDeclarativeType()
 {
     CollectionDetails::registerDeclarativeType();

src/plugins/qmldesigner/components/collectioneditor/collectionview.h

@@ -43,6 +43,8 @@ public:
 
     void addResource(const QUrl &url, const QString &name, const QString &type);
 
+    void assignCollectionToSelectedNode(const QString &collectionName);
+
     static void registerDeclarativeType();
 
     void resetDataStoreNode();

src/plugins/qmldesigner/components/collectioneditor/collectionwidget.cpp

@@ -337,17 +337,7 @@ bool CollectionWidget::addCollectionToDataStore(const QString &collectionName)
 
 void CollectionWidget::assignCollectionToSelectedNode(const QString collectionName)
 {
-    ModelNode dsNode = dataStoreNode();
+    m_view->assignCollectionToSelectedNode(collectionName);
-    ModelNode targetNode = m_view->singleSelectedModelNode();
-
-    QTC_ASSERT(dsNode.isValid() && targetNode.isValid(), return);
-
-    if (dsNode.id().isEmpty()) {
-        warn(tr("Assigning the model"), tr("The model must have a valid id to be assigned."));
-        return;
-    }
-
-    CollectionEditor::assignCollectionToNode(m_view, targetNode, dsNode, collectionName);
 }
 
 void CollectionWidget::ensureDataStoreExists()

src/plugins/qmldesigner/components/collectioneditor/datastoremodelnode.cpp

@@ -3,6 +3,7 @@
 
 #include "datastoremodelnode.h"
 
+#include "abstractview.h"
 #include "collectioneditorconstants.h"
 #include "collectioneditorutils.h"
 #include "model/qmltextgenerator.h"
@@ -42,6 +43,19 @@ QmlDesigner::PropertyNameList createNameList(const QmlDesigner::ModelNode &node)
     return defaultsNodeProps + dynamicPropertyNames;
 }
 
+bool isValidCollectionPropertyName(const QString &collectionId)
+{
+    static const QmlDesigner::PropertyNameList reservedKeywords = {
+        QmlDesigner::CollectionEditor::SOURCEFILE_PROPERTY,
+        QmlDesigner::CollectionEditor::JSONBACKEND_TYPENAME,
+        "backend",
+        "models",
+    };
+
+    return QmlDesigner::ModelNode::isValidId(collectionId)
+           && !reservedKeywords.contains(collectionId.toLatin1());
+}
+
 } // namespace
 
 namespace QmlDesigner {
@@ -85,15 +99,13 @@ void DataStoreModelNode::reloadModel()
 
     m_dataRelativePath = dataStoreJsonPath.relativePathFrom(dataStoreQmlPath).toFSPathString();
 
-    if (forceUpdate) {
+    if (forceUpdate)
-        updateDataStoreProperties();
+        update();
-        updateSingletonFile();
-    }
 }
 
 QStringList DataStoreModelNode::collectionNames() const
 {
-    return m_collectionNames;
+    return m_collectionPropertyNames.keys();
 }
 
 Model *DataStoreModelNode::model() const
@@ -137,23 +149,60 @@ void DataStoreModelNode::updateDataStoreProperties()
 
     static TypeName childNodeTypename = "ChildListModel";
 
+    QSet<QString> collectionNamesToBeAdded;
+    const QStringList allCollectionNames = m_collectionPropertyNames.keys();
+    for (const QString &collectionName : allCollectionNames)
+        collectionNamesToBeAdded << collectionName;
+
     const QList<AbstractProperty> formerPropertyNames = rootNode.dynamicProperties();
-    for (const AbstractProperty &property : formerPropertyNames)
+
+    // Remove invalid collection names from the properties
+    for (const AbstractProperty &property : formerPropertyNames) {
+        if (!property.isNodeProperty())
+            continue;
+
+        NodeProperty nodeProprty = property.toNodeProperty();
+        if (!nodeProprty.hasDynamicTypeName(childNodeTypename))
+            continue;
+
+        ModelNode childNode = nodeProprty.modelNode();
+        if (childNode.hasProperty(CollectionEditor::JSONCHILDMODELNAME_PROPERTY)) {
+            QString modelName = childNode.property(CollectionEditor::JSONCHILDMODELNAME_PROPERTY)
+                                    .toVariantProperty()
+                                    .value()
+                                    .toString();
+            if (collectionNamesToBeAdded.contains(modelName)) {
+                m_collectionPropertyNames.insert(modelName, property.name());
+                collectionNamesToBeAdded.remove(modelName);
+            } else {
                 rootNode.removeProperty(property.name());
+            }
+        } else {
+            rootNode.removeProperty(property.name());
+        }
+    }
 
     rootNode.setIdWithoutRefactoring("models");
 
-    for (const QString &collectionName : std::as_const(m_collectionNames)) {
+    QStringList collectionNamesLeft = collectionNamesToBeAdded.values();
-        PropertyName newName = collectionName.toLatin1();
+    Utils::sort(collectionNamesLeft);
+    for (const QString &collectionName : std::as_const(collectionNamesLeft)) {
+        PropertyName newPropertyName = getUniquePropertyName(collectionName);
+        if (newPropertyName.isEmpty()) {
+            qWarning() << __FUNCTION__ << __LINE__
+                       << QString("The property name cannot be generated from \"%1\"").arg(collectionName);
+            continue;
+        }
 
         ModelNode collectionNode = model()->createModelNode(childNodeTypename);
-
         VariantProperty modelNameProperty = collectionNode.variantProperty(
             CollectionEditor::JSONCHILDMODELNAME_PROPERTY);
-        modelNameProperty.setValue(newName);
+        modelNameProperty.setValue(collectionName);
 
-        NodeProperty nodeProp = rootNode.nodeProperty(newName);
+        NodeProperty nodeProp = rootNode.nodeProperty(newPropertyName);
         nodeProp.setDynamicTypeNameAndsetModelNode(childNodeTypename, collectionNode);
+
+        m_collectionPropertyNames.insert(collectionName, newPropertyName);
     }
 
     // Backend Property
@@ -186,13 +235,127 @@ void DataStoreModelNode::updateSingletonFile()
     file.finalize();
 }
 
-void DataStoreModelNode::setCollectionNames(const QStringList &newCollectionNames)
+void DataStoreModelNode::update()
 {
-    if (m_collectionNames != newCollectionNames) {
-        m_collectionNames = newCollectionNames;
     updateDataStoreProperties();
     updateSingletonFile();
+}
+
+PropertyName DataStoreModelNode::getUniquePropertyName(const QString &collectionName)
+{
+    ModelNode dataStoreNode = modelNode();
+    QTC_ASSERT(!collectionName.isEmpty() && dataStoreNode.isValid(), return {});
+
+    QString newProperty;
+
+    // convert to camel case
+    QStringList nameWords = collectionName.split(' ');
+    nameWords[0] = nameWords[0].at(0).toLower() + nameWords[0].mid(1);
+    for (int i = 1; i < nameWords.size(); ++i)
+        nameWords[i] = nameWords[i].at(0).toUpper() + nameWords[i].mid(1);
+    newProperty = nameWords.join("");
+
+    // if id starts with a number prepend an underscore
+    if (newProperty.at(0).isDigit())
+        newProperty.prepend('_');
+
+    // If the new id is not valid (e.g. qml keyword match), prepend an underscore
+    if (!isValidCollectionPropertyName(newProperty))
+        newProperty.prepend('_');
+
+    static const QRegularExpression rgx("\\d+$"); // matches a number at the end of a string
+    while (dataStoreNode.hasProperty(newProperty.toLatin1())) { // id exists
+        QRegularExpressionMatch match = rgx.match(newProperty);
+        if (match.hasMatch()) {                                 // ends with a number, increment it
+            QString numStr = match.captured();
+            int num = numStr.toInt() + 1;
+            newProperty = newProperty.mid(0, match.capturedStart()) + QString::number(num);
+        } else {
+            newProperty.append('1');
+        }
+    }
+
+    return newProperty.toLatin1();
+}
+
+void DataStoreModelNode::setCollectionNames(const QStringList &newCollectionNames)
+{
+    m_collectionPropertyNames.clear();
+    for (const QString &collectionName : newCollectionNames)
+        m_collectionPropertyNames.insert(collectionName, {});
+    update();
+}
+
+void DataStoreModelNode::renameCollection(const QString &oldName, const QString &newName)
+{
+    ModelNode dataStoreNode = modelNode();
+    QTC_ASSERT(dataStoreNode.isValid(), return);
+
+    if (m_collectionPropertyNames.contains(oldName)) {
+        const PropertyName oldPropertyName = m_collectionPropertyNames.value(oldName);
+        if (!oldPropertyName.isEmpty() && dataStoreNode.hasProperty(oldPropertyName)) {
+            NodeProperty collectionNode = dataStoreNode.property(oldPropertyName).toNodeProperty();
+            if (collectionNode.isValid()) {
+                VariantProperty modelNameProperty = collectionNode.modelNode().variantProperty(
+                    CollectionEditor::JSONCHILDMODELNAME_PROPERTY);
+                modelNameProperty.setValue(newName);
+                m_collectionPropertyNames.remove(oldName);
+                m_collectionPropertyNames.insert(newName, collectionNode.name());
+                update();
+                return;
+            }
+            qWarning() << __FUNCTION__ << __LINE__
+                       << "There is no valid node for the old collection name";
+            return;
+        }
+        qWarning() << __FUNCTION__ << __LINE__ << QString("Invalid old property name")
+                   << oldPropertyName;
+        return;
+    }
+    qWarning() << __FUNCTION__ << __LINE__
+               << QString("There is no old collection name registered with this name \"%1\"").arg(oldName);
+}
+
+void DataStoreModelNode::removeCollection(const QString &collectionName)
+{
+    if (m_collectionPropertyNames.contains(collectionName)) {
+        m_collectionPropertyNames.remove(collectionName);
+        update();
     }
 }
 
+void DataStoreModelNode::assignCollectionToNode(AbstractView *view,
+                                                const ModelNode &targetNode,
+                                                const QString &collectionName)
+{
+    QTC_ASSERT(targetNode.isValid(), return);
+
+    if (!CollectionEditor::canAcceptCollectionAsModel(targetNode))
+        return;
+
+    if (!m_collectionPropertyNames.contains(collectionName)) {
+        qWarning() << __FUNCTION__ << __LINE__ << "Collection doesn't exist in the DataStore"
+                   << collectionName;
+        return;
+    }
+
+    PropertyName propertyName = m_collectionPropertyNames.value(collectionName);
+
+    const ModelNode dataStore = modelNode();
+    VariantProperty sourceProperty = dataStore.variantProperty(propertyName);
+    if (!sourceProperty.exists()) {
+        qWarning() << __FUNCTION__ << __LINE__
+                   << "The source property doesn't exist in the DataStore.";
+        return;
+    }
+
+    BindingProperty modelProperty = targetNode.bindingProperty("model");
+
+    QString identifier = QString("DataStore.%1").arg(QString::fromLatin1(sourceProperty.name()));
+
+    view->executeInTransaction("assignCollectionToNode", [&modelProperty, &identifier]() {
+        modelProperty.setExpression(identifier);
+    });
+}
+
 } // namespace QmlDesigner

src/plugins/qmldesigner/components/collectioneditor/datastoremodelnode.h

@@ -5,6 +5,8 @@
 
 #include <modelnode.h>
 
+#include <QMap>
+
 namespace QmlDesigner {
 
 class Model;
@@ -21,6 +23,12 @@ public:
     ModelNode modelNode() const;
 
     void setCollectionNames(const QStringList &newCollectionNames);
+    void renameCollection(const QString &oldName, const QString &newName);
+    void removeCollection(const QString &collectionName);
+
+    void assignCollectionToNode(AbstractView *view,
+                                const ModelNode &targetNode,
+                                const QString &collectionName);
 
 private:
     QString getModelQmlText();
@@ -28,9 +36,11 @@ private:
     void reset();
     void updateDataStoreProperties();
     void updateSingletonFile();
+    void update();
+    PropertyName getUniquePropertyName(const QString &collectionName);
 
     ModelPointer m_model;
-    QStringList m_collectionNames;
+    QMap<QString, PropertyName> m_collectionPropertyNames;
     QString m_dataRelativePath;
 };