a slew of clean ups and improvements to the code

DataSource still needs to be made to not keep a private copy of all the data

declarativeimports/core/dataengineconsumer.cpp

@@ -42,33 +42,32 @@ ServiceMonitor::~ServiceMonitor()
 
 void ServiceMonitor::slotJobFinished(Plasma::ServiceJob *job)
 {
-    kDebug() << "engine ready!";
+    //kDebug() << "engine ready!";
     QString engineName = job->parameters()["EngineName"].toString();
     QString location = job->destination();
     QPair<QString, QString> pair(location, engineName);
-    kDebug() << "pair = " << pair;
+    //kDebug() << "pair = " << pair;
     if (!m_consumer->m_remoteEngines.contains(pair)) {
         kDebug() << "engine does not exist yet!";
     } else {
         KUrl engineLocation(location);
         engineLocation.setFileName(job->result().toString());
-        kDebug() << "setting location : "
+        //kDebug() << "setting location : " << engineLocation.prettyUrl();
-                 << engineLocation.prettyUrl();
       //  m_consumer->m_remoteEngines[pair]->setLocation(engineLocation);
     }
 }
 
 void ServiceMonitor::slotServiceReady(Plasma::Service *plasmoidService)
 {
-    kDebug() << "service ready!";
+    //kDebug() << "service ready!";
     if (!m_consumer->m_engineNameForService.contains(plasmoidService)) {
         kDebug() << "no engine name for service!";
         kDebug() << "amount of services in map: " << m_consumer->m_engineNameForService.count();
     } else {
-        kDebug() << "value = " << m_consumer->m_engineNameForService.value(plasmoidService);
+        //kDebug() << "value = " << m_consumer->m_engineNameForService.value(plasmoidService);
     }
 
-    kDebug() << "requesting dataengine!";
+    //kDebug() << "requesting dataengine!";
     KConfigGroup op = plasmoidService->operationDescription("DataEngine");
     op.writeEntry("EngineName", m_consumer->m_engineNameForService.value(plasmoidService));
     plasmoidService->startOperationCall(op);
@@ -90,6 +89,13 @@ DataEngineConsumer::~DataEngineConsumer()
     delete m_monitor;
 }
 
+void DataEngineConsumer::finishedWithEngine(const QString &name)
+{
+    if (m_loadedEngines.contains(name)) {
+        DataEngineManager::self()->unloadEngine(name);
+    }
+}
+
 DataEngine *DataEngineConsumer::dataEngine(const QString &name)
 {
     if (m_loadedEngines.contains(name)) {

declarativeimports/core/dataengineconsumer_p.h

@@ -57,6 +57,7 @@ public:
     ~DataEngineConsumer();
     DataEngine *dataEngine(const QString &name);
     DataEngine *remoteDataEngine(const KUrl &location, const QString &name);
+    void finishedWithEngine(const QString &name);
 
 private:
     QSet<QString> m_loadedEngines;

declarativeimports/core/datamodel.cpp

@@ -151,53 +151,44 @@ DataModel::~DataModel()
 
 void DataModel::dataUpdated(const QString &sourceName, const Plasma::DataEngine::Data &data)
 {
-    if (!m_keyRoleFilter.isEmpty()) {
+    if (!m_sourceFilter.isEmpty() && m_sourceFilterRE.isValid() && !m_sourceFilterRE.exactMatch(sourceName)) {
-        QRegExp sourceRegExp(m_sourceFilter);
+        return;
-        //a key that matches the one we want exists and is a list of DataEngine::Data
+    }
-        if (data.contains(m_keyRoleFilter) &&
+
-            data.value(m_keyRoleFilter).canConvert<QVariantList>() &&
+    if (m_keyRoleFilter.isEmpty()) {
-            //matches the source filter regexp?
+        //an item is represented by a source: keys are roles m_roleLevel == FirstLevel
-            (m_sourceFilter.isEmpty() || (sourceRegExp.isValid() &&  sourceRegExp.exactMatch(sourceName)))) {
-            setItems(sourceName, data.value(m_keyRoleFilter).value<QVariantList>());
-        //try to match the key we want with a regular expression if set
-        } else {
-            QRegExp regExp(m_keyRoleFilter);
-            if (regExp.isValid() &&
-                //matches the source filter regexp?
-                (m_sourceFilter.isEmpty() || (sourceRegExp.isValid() && sourceRegExp.exactMatch(sourceName)))) {
-                QHash<QString, QVariant>::const_iterator i;
-                QVariantList list;
-                for (i = data.constBegin(); i != data.constEnd(); ++i) {
-                    if (regExp.exactMatch(i.key())) {
-                        list.append(i.value());
-                    }
-                }
-                setItems(sourceName, list);
-            }
-        }
-    //an item is represented by a source: keys are roles m_roleLevel == FirstLevel
-    } else {
         QVariantList list;
 
         if (!m_dataSource->data().isEmpty()) {
-            QVariantMap::const_iterator i = m_dataSource->data().constBegin();
+            QVariantHash::const_iterator i = m_dataSource->data().constBegin();
-            QRegExp sourceRegExp(m_sourceFilter);
 
             while (i != m_dataSource->data().constEnd()) {
                 QVariant value = i.value();
-                if (value.isValid() &&
+                if (value.isValid() && value.canConvert<Plasma::DataEngine::Data>()) {
-                    value.canConvert<Plasma::DataEngine::Data>() &&
-                    //matches the source filter regexp?
-                    (m_sourceFilter.isEmpty() || (sourceRegExp.isValid() && sourceRegExp.exactMatch(i.key())))) {
                     Plasma::DataEngine::Data data = value.value<Plasma::DataEngine::Data>();
                     data["DataEngineSource"] = i.key();
-
                     list.append(data);
                 }
                 ++i;
             }
         }
         setItems(QString(), list);
+    } else {
+        //a key that matches the one we want exists and is a list of DataEngine::Data
+        if (data.contains(m_keyRoleFilter) &&
+            data.value(m_keyRoleFilter).canConvert<QVariantList>()) {
+            setItems(sourceName, data.value(m_keyRoleFilter).value<QVariantList>());
+        } else if (m_keyRoleFilterRE.isValid()) {
+            //try to match the key we want with a regular expression if set
+            QVariantList list;
+            QHash<QString, QVariant>::const_iterator i;
+            for (i = data.constBegin(); i != data.constEnd(); ++i) {
+                if (m_keyRoleFilterRE.exactMatch(i.key())) {
+                    list.append(i.value());
+                }
+            }
+            setItems(sourceName, list);
+        }
     }
 }
 
@@ -212,11 +203,15 @@ void DataModel::setDataSource(QObject *object)
         return;
     }
 
-    disconnect(m_dataSource, 0, this, 0);
+    if (m_dataSource) {
+        disconnect(m_dataSource, 0, this, 0);
+    }
+
     m_dataSource = source;
 
-    QMap<QString, QVariant>::const_iterator i = source->data().constBegin();
+    const QHash<QString, QVariant> data = source->data();
-    while (i != source->data().constEnd()) {
+    QHash<QString, QVariant>::const_iterator i = data.constBegin();
+    while (i != data.constEnd()) {
         dataUpdated(i.key(), i.value().value<Plasma::DataEngine::Data>());
         ++i;
     }
@@ -234,11 +229,24 @@ QObject *DataModel::dataSource() const
 
 void DataModel::setKeyRoleFilter(const QString& key)
 {
+    // the "key role filter" can be used in one of three ways:
+    //
+    // 1) empty string -> all data is used, each source is one row in the model
+    // 2) matches a key in the data exactly -> only that key/value pair is used, and the value is
+    //    treated as a collection where each item in the collection becomes a row in the model
+    // 3) regular expression -> matches zero or more keys in the data, and each matching key/value
+    //    pair becomes a row in the model
     if (m_keyRoleFilter == key) {
         return;
     }
 
     m_keyRoleFilter = key;
+    m_keyRoleFilterRE = QRegExp(m_keyRoleFilter);
+}
+
+QString DataModel::keyRoleFilter() const
+{
+    return m_keyRoleFilter;
 }
 
 void DataModel::setSourceFilter(const QString& key)
@@ -248,6 +256,14 @@ void DataModel::setSourceFilter(const QString& key)
     }
 
     m_sourceFilter = key;
+    m_sourceFilterRE = QRegExp(key);
+    /*
+     FIXME: if the user changes the source filter, it won't immediately be reflected in the
+     available data
+    if (m_sourceFilterRE.isValid()) {
+        .. iterate through all items and weed out the ones that don't match ..
+    }
+    */
 }
 
 QString DataModel::sourceFilter() const
@@ -255,11 +271,6 @@ QString DataModel::sourceFilter() const
     return m_sourceFilter;
 }
 
-QString DataModel::keyRoleFilter() const
-{
-    return m_keyRoleFilter;
-}
-
 void DataModel::setItems(const QString &sourceName, const QVariantList &list)
 {
     beginResetModel();
@@ -270,7 +281,11 @@ void DataModel::setItems(const QString &sourceName, const QVariantList &list)
     if (!list.isEmpty()) {
         if (list.first().canConvert<QVariantHash>()) {
             foreach (const QVariant &item, list) {
-                foreach (const QString& roleName, item.value<QVariantHash>().keys()) {
+                const QVariantHash &vh = item.value<QVariantHash>();
+                QHashIterator<QString, QVariant> it(vh);
+                while (it.hasNext()) {
+                    it.next();
+                    const QString &roleName = it.key();
                     if (!m_roleIds.contains(roleName)) {
                         ++m_maxRoleId;
                         m_roleNames[m_maxRoleId] = roleName.toLatin1();
@@ -280,7 +295,11 @@ void DataModel::setItems(const QString &sourceName, const QVariantList &list)
             }
         } else {
             foreach (const QVariant &item, list) {
-                foreach (const QString& roleName, item.value<QVariantMap>().keys()) {
+                const QVariantMap &vh = item.value<QVariantMap>();
+                QMapIterator<QString, QVariant> it(vh);
+                while (it.hasNext()) {
+                    it.next();
+                    const QString &roleName = it.key();
                     if (!m_roleIds.contains(roleName)) {
                         ++m_maxRoleId;
                         m_roleNames[m_maxRoleId] = roleName.toLatin1();
@@ -305,15 +324,8 @@ void DataModel::removeSource(const QString &sourceName)
     //FIXME: this could be way more efficient by not resetting the whole model
     //FIXME: find a way to remove only the proper things also in the case where sources are items
 
-    //source name as key of the map
+    if (m_keyRoleFilter.isEmpty()) {
-    if (!m_keyRoleFilter.isEmpty()) {
+        //source name in the map, linear scan
-        if (m_items.contains(sourceName)) {
-            beginResetModel();
-            m_items.remove(sourceName);
-            endResetModel();
-        }
-    //source name in the map, linear scan
-    } else {
         for (int i = 0; i < m_items.value(QString()).count(); ++i) {
             if (m_items.value(QString())[i].value<QVariantHash>().value("DataEngineSource") == sourceName) {
                 beginResetModel();
@@ -322,6 +334,13 @@ void DataModel::removeSource(const QString &sourceName)
                 break;
             }
         }
+    } else {
+        //source name as key of the map
+        if (m_items.contains(sourceName)) {
+            beginResetModel();
+            m_items.remove(sourceName);
+            endResetModel();
+        }
     }
 }
 
@@ -349,10 +368,9 @@ QVariant DataModel::data(const QModelIndex &index, int role) const
 
     //is it the reserved role: DataEngineSource ?
     //also, if each source is an item DataEngineSource is a role between all the others, otherwise we know it from the role variable
+    //finally, sub items are some times QVariantHash some times QVariantMaps
     if (!m_keyRoleFilter.isEmpty() && m_roleNames.value(role) == "DataEngineSource") {
         return source;
-
-    //sub items are some times QVariantHash some times QVariantMaps
     } else if (m_items.value(source).value(actualRow).canConvert<QVariantHash>()) {
         return m_items.value(source).value(actualRow).value<QVariantHash>().value(m_roleNames.value(role));
     } else {

declarativeimports/core/datamodel.h

@@ -105,8 +105,7 @@ public:
     QString keyRoleFilter() const;
 
     /**
-     * Include only items with a source name that matches this regexp in the model
+     * Include only sources that matches this regexp in the model
-     * @since 4.7
      */
     void setSourceFilter(const QString& key);
     QString sourceFilter() const;
@@ -139,7 +138,9 @@ private Q_SLOTS:
 private:
     DataSource *m_dataSource;
     QString m_keyRoleFilter;
+    QRegExp m_keyRoleFilterRE;
     QString m_sourceFilter;
+    QRegExp m_sourceFilterRE;
     QMap<QString, QVector<QVariant> > m_items;
     QHash<int, QByteArray> m_roleNames;
     QHash<QString, int> m_roleIds;

declarativeimports/core/datasource.cpp

@@ -36,31 +36,35 @@ DataSource::DataSource(QObject* parent)
       m_dataEngine(0)
 {
     setObjectName("DataSource");
-
-    connect(this, SIGNAL(engineChanged()),
-            this, SLOT(setupData()));
-    connect(this, SIGNAL(connectedSourcesChanged()),
-            this, SLOT(setupData()), Qt::QueuedConnection);
-    connect(this, SIGNAL(intervalChanged()),
-            this, SLOT(setupData()));
 }
 
 void DataSource::setConnectedSources(const QStringList &sources)
 {
+    bool sourcesChanged = false;
     foreach (const QString &source, sources) {
         if (!m_connectedSources.contains(source)) {
-            m_newSources.append(source);
+            sourcesChanged = true;
-        }
+            if (m_dataEngine) {
-    }
+                m_connectedSources.append(source);
-    foreach (const QString &source, m_connectedSources) {
+                m_dataEngine->connectSource(source, this, m_interval);
-        if (!sources.contains(source)) {
+                emit sourceConnected(source);
-            m_oldSources.append(source);
+            }
         }
     }
 
-    if (!m_newSources.isEmpty() || !m_oldSources.isEmpty()) {
+    foreach (const QString &source, m_connectedSources) {
+        if (!sources.contains(source)) {
+            m_data.remove(source);
+            sourcesChanged = true;
+            if (m_dataEngine) {
+                m_dataEngine->disconnectSource(source, this);
+                emit sourceDisconnected(source);
+            }
+        }
+    }
+
+    if (sourcesChanged) {
         m_connectedSources = sources;
-        m_changes |= SourcesChanged;
         emit connectedSourcesChanged();
     }
 }
@@ -70,9 +74,9 @@ void DataSource::setEngine(const QString &e)
     if (e == m_engine) {
         return;
     }
-    m_engine = e;
 
-    m_changes |= DataEngineChanged;
+    m_engine = e;
+    setupData();
     emit engineChanged();
 }
 
@@ -81,61 +85,44 @@ void DataSource::setInterval(const int interval)
     if (interval == m_interval) {
         return;
     }
+
     m_interval = interval;
-    m_changes |= DataEngineChanged;
+    setupData();
     emit intervalChanged();
 }
 
 //TODO: event compression for this
 void DataSource::setupData()
 {
+    //FIXME: should all services be deleted just because we're changing the interval, etc?
+    qDeleteAll(m_services);
+    m_services.clear();
 
-    if (m_changes & DataEngineChanged) {
+    Plasma::DataEngine *engine = dataEngine(m_engine);
+    if (!engine) {
+        kWarning() << "DataEngine" << m_engine << "not found";
+        return;
+    }
+
+    if (engine != m_dataEngine) {
         if (m_dataEngine) {
-            foreach (const QString &source, m_connectedSources) {
+            m_dataEngine->disconnect(this);
-                m_dataEngine->disconnectSource(source, this);
+            finishedWithEngine(m_dataEngine->pluginName());
-            }
         }
 
-        qDeleteAll(m_services);
+        m_dataEngine = engine;
-        m_services.clear();
-
-        m_dataEngine = dataEngine(m_engine);
-        if (!m_dataEngine) {
-            kWarning() << "DataEngine" << m_engine << "not found";
-            return;
-        }
         connect(m_dataEngine, SIGNAL(sourceAdded(const QString&)), this, SIGNAL(sourcesChanged()));
         connect(m_dataEngine, SIGNAL(sourceRemoved(const QString&)), this, SIGNAL(sourcesChanged()));
 
         connect(m_dataEngine, SIGNAL(sourceAdded(const QString&)), this, SIGNAL(sourceAdded(const QString&)));
         connect(m_dataEngine, SIGNAL(sourceRemoved(const QString&)), this, SLOT(removeSource(const QString&)));
         connect(m_dataEngine, SIGNAL(sourceRemoved(const QString&)), this, SIGNAL(sourceRemoved(const QString&)));
-
-        //TODO: remove after event compression done
-        if (!(m_changes & SourcesChanged)) {
-            foreach (const QString &source, m_connectedSources) {
-                m_dataEngine->connectSource(source, this, m_interval);
-                emit sourceConnected(source);
-            }
-        }
     }
 
-    if (m_changes & SourcesChanged) {
+    foreach (const QString &source, m_connectedSources) {
-        if (m_dataEngine) {
+        m_dataEngine->connectSource(source, this, m_interval);
-            foreach (const QString &source, m_oldSources) {
+        emit sourceConnected(source);
-                m_dataEngine->disconnectSource(source, this);
-                emit sourceDisconnected(source);
-            }
-            foreach (const QString &source, m_newSources) {
-                m_dataEngine->connectSource(source, this, m_interval);
-                emit sourceConnected(source);
-            }
-            m_oldSources.clear();
-            m_newSources.clear();
-        }
     }
-    m_changes = NoChange;
 }
 
 void DataSource::dataUpdated(const QString &sourceName, const Plasma::DataEngine::Data &data)
@@ -157,15 +144,12 @@ void DataSource::removeSource(const QString &source)
 
     //TODO: emit those signals as last thing
     if (m_connectedSources.contains(source)) {
+        m_connectedSources.removeAll(source);
         emit sourceDisconnected(source);
         emit connectedSourcesChanged();
     }
 
     if (m_dataEngine) {
-        m_connectedSources.removeAll(source);
-        m_newSources.removeAll(source);
-        m_oldSources.removeAll(source);
-
         QHash<QString, Plasma::Service *>::iterator it = m_services.find(source);
         if (it != m_services.end()) {
             delete it.value();
@@ -174,14 +158,6 @@ void DataSource::removeSource(const QString &source)
     }
 }
 
-QStringList DataSource::keysForSource(const QString &source) const
-{
-    if (!m_data.contains(source)) {
-        return QStringList();
-    }
-    return m_data.value(source).value<Data>().keys();
-}
-
 Plasma::Service *DataSource::serviceForSource(const QString &source)
 {
     if (!m_services.contains(source)) {
@@ -201,20 +177,22 @@ void DataSource::connectSource(const QString &source)
         return;
     }
 
-    m_newSources.append(source);
     m_connectedSources.append(source);
-    m_changes |= SourcesChanged;
+    if (m_dataEngine) {
-    emit sourceConnected(source);
+        m_dataEngine->connectSource(source, this, m_interval);
-    emit connectedSourcesChanged();
+        emit sourceConnected(source);
+        emit connectedSourcesChanged();
+    }
 }
 
 void DataSource::disconnectSource(const QString &source)
 {
-    m_oldSources.append(source);
+    if (m_dataEngine && m_connectedSources.contains(source)) {
-    m_connectedSources.removeAll(source);
+        m_connectedSources.removeAll(source);
-    m_changes |= SourcesChanged;
+        m_dataEngine->disconnectSource(source, this);
-    emit sourceDisconnected(source);
+        emit sourceDisconnected(source);
-    emit connectedSourcesChanged();
+        emit connectedSourcesChanged();
+    }
 }
 
 }

declarativeimports/core/datasource_p.h

@@ -73,10 +73,9 @@ public:
     Q_PROPERTY(QStringList sources READ sources NOTIFY sourcesChanged)
     QStringList sources() const {if (m_dataEngine) return m_dataEngine->sources(); else return QStringList();}
 
-    Q_PROPERTY(QVariantMap data READ data NOTIFY dataChanged);
+    Q_PROPERTY(QVariantHash data READ data NOTIFY dataChanged);
-    QVariantMap data() const {return m_data;}
+    QVariantHash data() const {return m_data;}
 
-    Q_INVOKABLE QStringList keysForSource(const QString &source) const;
     Q_INVOKABLE Plasma::Service *serviceForSource(const QString &source);
 
     Q_INVOKABLE void connectSource(const QString &source);
@@ -103,7 +102,7 @@ private:
     QString m_id;
     int m_interval;
     QString m_engine;
-    QVariantMap m_data;
+    QVariantHash m_data;
     Plasma::DataEngine* m_dataEngine;
     QStringList m_connectedSources;
     QStringList m_oldSources;