Simplify Device::id management

Removes the id->row mapping. It was broken as the indices weren't updated
when a row was added or removed.
Stop exposing Device::id through dbus, just store it in the interface
class.
* It's redundant as it's part of the interface definition
* If we keep it it will save us quite some dbus round-trips
* It will be reliable, as nowadays id() sometimes QString() if the dbus
connection is invalid.

REVIEW: 124158

core/device.h

@@ -39,7 +39,6 @@ class KDECONNECTCORE_EXPORT Device
 {
     Q_OBJECT
     Q_CLASSINFO("D-Bus Interface", "org.kde.kdeconnect.device")
-    Q_PROPERTY(QString id READ id CONSTANT)
     Q_PROPERTY(QString type READ type CONSTANT)
     Q_PROPERTY(QString name READ name NOTIFY nameChanged)
     Q_PROPERTY(QString iconName READ iconName CONSTANT)

interfaces/dbusinterfaces.cpp

@@ -39,6 +39,7 @@ DaemonDbusInterface::~DaemonDbusInterface()
 
 DeviceDbusInterface::DeviceDbusInterface(const QString& id, QObject* parent)
     : OrgKdeKdeconnectDeviceInterface(activatedService(), "/modules/kdeconnect/devices/"+id, QDBusConnection::sessionBus(), parent)
+    , m_id(id)
 {
     connect(this, &OrgKdeKdeconnectDeviceInterface::pairingChanged, this, &DeviceDbusInterface::pairingChangedProxy);
 }
@@ -48,6 +49,11 @@ DeviceDbusInterface::~DeviceDbusInterface()
 
 }
 
+QString DeviceDbusInterface::id() const
+{
+    return m_id;
+}
+
 void DeviceDbusInterface::pluginCall(const QString &plugin, const QString &method)
 {
     QDBusMessage msg = QDBusMessage::createMethodCall("org.kde.kdeconnect", "/modules/kdeconnect/devices/"+id()+'/'+plugin, "org.kde.kdeconnect.device."+plugin, method);

interfaces/dbusinterfaces.h

@@ -51,14 +51,21 @@ class KDECONNECTINTERFACES_EXPORT DeviceDbusInterface
 //  TODO: Workaround because OrgKdeKdeconnectDeviceInterface is not generating
 //  the signals for the properties
     Q_PROPERTY(bool isPaired READ isPaired NOTIFY pairingChangedProxy)
+
+    /** @returns an id even if the interface isn't valid */
+    Q_PROPERTY(QString id READ id CONSTANT)
 public:
     DeviceDbusInterface(const QString& deviceId, QObject* parent = 0);
     virtual ~DeviceDbusInterface();
 
+    QString id() const;
     Q_SCRIPTABLE void pluginCall(const QString &plugin, const QString &method);
 
 Q_SIGNALS:
     void pairingChangedProxy(bool paired);
+
+private:
+    const QString m_id;
 };
 
 class KDECONNECTINTERFACES_EXPORT DeviceBatteryDbusInterface

interfaces/devicesmodel.cpp

@@ -70,19 +70,31 @@ DevicesModel::~DevicesModel()
 {
 }
 
+int DevicesModel::rowForDevice(const QString& id) const
+{
+    for (int i = 0, c=m_deviceList.size(); i<c; ++i) {
+        if (m_deviceList[i]->id() == id) {
+            return i;
+        }
+    }
+    return -1;
+}
+
 void DevicesModel::deviceAdded(const QString& id)
 {
-    if (m_deviceIndexById.contains(id)) {
+    if (rowForDevice(id) >= 0) {
-        Q_ASSERT(false); //This should only happen for new devices
+        Q_ASSERT_X(false, "deviceAdded", "Trying to add a device twice");
         return;
     }
 
     DeviceDbusInterface* dev = new DeviceDbusInterface(id, this);
+    Q_ASSERT(dev->isValid());
 
     bool onlyPaired = (m_displayFilter & StatusFilterFlag::Paired);
     bool onlyReachable = (m_displayFilter & StatusFilterFlag::Reachable);
 
     if ((onlyReachable && !dev->isReachable()) || (onlyPaired && !dev->isPaired())) {
+        delete dev;
         return;
     }
 
@@ -93,10 +105,8 @@ void DevicesModel::deviceAdded(const QString& id)
 
 void DevicesModel::deviceRemoved(const QString& id)
 {
-    QMap<QString, int>::iterator it = m_deviceIndexById.find(id);
+    int row = rowForDevice(id);
-    if (it != m_deviceIndexById.end()) {
+    if (row>=0) {
-        int row = *it;
-        m_deviceIndexById.erase(it);
         beginRemoveRows(QModelIndex(), row, row);
         delete m_deviceList.takeAt(row);
         endRemoveRows();
@@ -105,9 +115,9 @@ void DevicesModel::deviceRemoved(const QString& id)
 
 void DevicesModel::deviceUpdated(const QString& id)
 {
-    QMap<QString, int>::iterator it = m_deviceIndexById.find(id);
+    int row = rowForDevice(id);
-    if (it != m_deviceIndexById.end()) {
+    if (row >= 0) {
-        const QModelIndex idx = index(it.value());
+        const QModelIndex idx = index(row);
         Q_EMIT dataChanged(idx, idx);
     }
 }
@@ -152,7 +162,6 @@ void DevicesModel::receivedDeviceList(QDBusPendingCallWatcher* watcher)
     }
 
     Q_ASSERT(m_deviceList.isEmpty());
-    Q_ASSERT(m_deviceIndexById.isEmpty());
     const QStringList deviceIds = pendingDeviceIds.value();
 
     if (deviceIds.isEmpty())
@@ -167,7 +176,6 @@ void DevicesModel::receivedDeviceList(QDBusPendingCallWatcher* watcher)
 
 void DevicesModel::appendDevice(DeviceDbusInterface* dev)
 {
-    m_deviceIndexById.insert(dev->id(), m_deviceList.size());
     m_deviceList.append(dev);
     connect(dev, SIGNAL(nameChanged(QString)), SLOT(nameChanged(QString)));
 }
@@ -185,23 +193,23 @@ void DevicesModel::clearDevices()
         beginRemoveRows(QModelIndex(), 0, m_deviceList.size() - 1);
         qDeleteAll(m_deviceList);
         m_deviceList.clear();
-        m_deviceIndexById.clear();
         endRemoveRows();
     }
 }
 
 QVariant DevicesModel::data(const QModelIndex& index, int role) const
 {
-    if (!m_dbusInterface->isValid()
+    if (!index.isValid()
-        || !index.isValid()
         || index.row() < 0
-        || index.row() >= m_deviceList.size()
+        || index.row() >= m_deviceList.size())
-        || !m_deviceList[index.row()]->isValid())
     {
         return QVariant();
     }
 
+    Q_ASSERT(m_dbusInterface->isValid());
+
     DeviceDbusInterface* device = m_deviceList[index.row()];
+    Q_ASSERT(device->isValid());
 
     //This function gets called lots of times, producing lots of dbus calls. Add a cache?
     switch (role) {

interfaces/devicesmodel.h

@@ -81,12 +81,12 @@ Q_SIGNALS:
     void rowsChanged();
 
 private:
+    int rowForDevice(const QString& id) const;
     void clearDevices();
     void appendDevice(DeviceDbusInterface* dev);
 
     DaemonDbusInterface* m_dbusInterface;
-    QList<DeviceDbusInterface*> m_deviceList;
+    QVector<DeviceDbusInterface*> m_deviceList;
-    QMap<QString, int> m_deviceIndexById;
     StatusFilterFlag m_displayFilter;
 };