From 81634303b2d3f37d9bdd0d1a35508e9f00645700 Mon Sep 17 00:00:00 2001 From: Aleix Pol Date: Wed, 6 Jul 2016 17:37:22 +0200 Subject: [PATCH] Make capabilities static As discussed with Albert, move the population of capabilities into the identity package, making them static along the execution of the link. When we receive the identityPackage, we collect the plugins we can use with the device and stick to those. This should simplify the implementation and remove the possibility to lose packages if packages are received before the capabilities are processed in the former approach. REVIEW: 128386 --- core/device.cpp | 103 +++++++++++-------------------------- core/device.h | 13 ++--- core/networkpackage.cpp | 2 + core/networkpackagetypes.h | 1 - core/pluginloader.cpp | 23 ++++++++- core/pluginloader.h | 6 ++- kcm/kcm.cpp | 12 ++--- kcm/kcm.h | 2 +- tests/pluginloadtest.cpp | 4 +- 9 files changed, 69 insertions(+), 97 deletions(-) diff --git a/core/device.cpp b/core/device.cpp index ec2d1f5af..b5a0abdec 100644 --- a/core/device.cpp +++ b/core/device.cpp @@ -40,8 +40,6 @@ #include "kdeconnectconfig.h" #include "daemon.h" -#define MIN_VERSION_WITH_CAPPABILITIES_SUPPORT 6 - Q_LOGGING_CATEGORY(KDECONNECT_CORE, "kdeconnect.core") static void warn(const QString &info) @@ -69,8 +67,6 @@ Device::Device(QObject* parent, const NetworkPackage& identityPackage, DeviceLin : QObject(parent) , m_deviceId(identityPackage.get("deviceId")) , m_deviceName(identityPackage.get("deviceName")) - , m_deviceType(str2type(identityPackage.get("deviceType"))) - , m_protocolVersion(identityPackage.get("protocolVersion", -1)) { addLink(identityPackage, dl); @@ -99,45 +95,17 @@ QStringList Device::loadedPlugins() const void Device::reloadPlugins() { QHash newPluginMap; - QMultiMap newPluginsByIncomingInterface; - QMultiMap newPluginsByOutgoingInterface; - QSet supportedIncomingInterfaces; - QSet supportedOutgoingInterfaces; - QStringList unsupportedPlugins; + QMultiMap newPluginsByIncomingCapability; if (isTrusted() && isReachable()) { //Do not load any plugin for unpaired devices, nor useless loading them for unreachable devices PluginLoader* loader = PluginLoader::instance(); - const bool capabilitiesSupported = (m_protocolVersion >= MIN_VERSION_WITH_CAPPABILITIES_SUPPORT); - Q_FOREACH (const QString& pluginName, loader->getPluginList()) { + Q_FOREACH (const QString& pluginName, m_supportedPlugins) { const KPluginMetaData service = loader->getPluginInfo(pluginName); - const QSet incomingInterfaces = KPluginMetaData::readStringList(service.rawData(), "X-KdeConnect-SupportedPackageType").toSet(); - const QSet outgoingInterfaces = KPluginMetaData::readStringList(service.rawData(), "X-KdeConnect-OutgoingPackageType").toSet(); const bool pluginEnabled = isPluginEnabled(pluginName); - - if (pluginEnabled) { - supportedIncomingInterfaces += incomingInterfaces; - supportedOutgoingInterfaces += outgoingInterfaces; - } - - const bool pluginNeedsCapabilities = !incomingInterfaces.isEmpty() || !outgoingInterfaces.isEmpty(); - - //If we don't find intersection with the received on one end and the sent on the other, we don't - //let the plugin stay - if (capabilitiesSupported && pluginNeedsCapabilities - && (m_incomingCapabilities & outgoingInterfaces).isEmpty() - && (m_outgoingCapabilities & incomingInterfaces).isEmpty() - ) { - if (!m_incomingCapabilities.isEmpty() || !m_outgoingCapabilities.isEmpty()) { - qCWarning(KDECONNECT_CORE) << "not loading" << pluginName << "because of unmatched capabilities" << - outgoingInterfaces << incomingInterfaces; - } - - unsupportedPlugins.append(pluginName); - continue; - } + const QSet incomingCapabilities = KPluginMetaData::readStringList(service.rawData(), "X-KdeConnect-SupportedPackageType").toSet(); if (pluginEnabled) { KdeConnectPlugin* plugin = m_plugins.take(pluginName); @@ -145,12 +113,10 @@ void Device::reloadPlugins() if (!plugin) { plugin = loader->instantiatePluginForDevice(pluginName, this); } + Q_ASSERT(plugin); - Q_FOREACH (const QString& interface, incomingInterfaces) { - newPluginsByIncomingInterface.insert(interface, plugin); - } - Q_FOREACH (const QString& interface, outgoingInterfaces) { - newPluginsByOutgoingInterface.insert(interface, plugin); + Q_FOREACH (const QString& interface, incomingCapabilities) { + newPluginsByIncomingCapability.insert(interface, plugin); } newPluginMap[pluginName] = plugin; @@ -158,31 +124,20 @@ void Device::reloadPlugins() } } + const bool differentPlugins = m_plugins != newPluginMap; + //Erase all left plugins in the original map (meaning that we don't want //them anymore, otherwise they would have been moved to the newPluginMap) - const QStringList newSupportedIncomingInterfaces = supportedIncomingInterfaces.toList(); - const QStringList newSupportedOutgoingInterfaces = supportedOutgoingInterfaces.toList(); - const bool capabilitiesChanged = (m_pluginsByOutgoingInterface != newPluginsByOutgoingInterface - || m_supportedIncomingInterfaces != newSupportedIncomingInterfaces); qDeleteAll(m_plugins); m_plugins = newPluginMap; - m_supportedIncomingInterfaces = newSupportedIncomingInterfaces; - m_supportedOutgoingInterfaces = newSupportedOutgoingInterfaces; - m_pluginsByOutgoingInterface = newPluginsByOutgoingInterface; - m_pluginsByIncomingInterface = newPluginsByIncomingInterface; - m_unsupportedPlugins = unsupportedPlugins; + m_pluginsByIncomingCapability = newPluginsByIncomingCapability; + //TODO: see how it works in Android (only done once, when created) Q_FOREACH(KdeConnectPlugin* plugin, m_plugins) { plugin->connected(); } - Q_EMIT pluginsChanged(); - - if (capabilitiesChanged && isReachable() && isTrusted()) - { - NetworkPackage np(PACKAGE_TYPE_CAPABILITIES); - np.set("IncomingCapabilities", newSupportedIncomingInterfaces); - np.set("OutgoingCapabilities", newSupportedOutgoingInterfaces); - sendPackage(np); + if (differentPlugins) { + Q_EMIT pluginsChanged(); } } @@ -271,13 +226,21 @@ void Device::addLink(const NetworkPackage& identityPackage, DeviceLink* link) qSort(m_deviceLinks.begin(), m_deviceLinks.end(), lessThan); - if (m_deviceLinks.size() == 1) { - reloadPlugins(); //Will load the plugins - Q_EMIT reachableStatusChanged(); + const bool capabilitiesSupported = identityPackage.has("incomingCapabilities") || identityPackage.has("outgoingCapabilities"); + if (capabilitiesSupported) { + const QSet outgoingCapabilities = identityPackage.get("outgoingCapabilities").toSet() + , incomingCapabilities = identityPackage.get("incomingCapabilities").toSet(); + + m_supportedPlugins = PluginLoader::instance()->pluginsForCapabilities(incomingCapabilities, outgoingCapabilities); + qDebug() << "new plugins for" << m_deviceName << m_supportedPlugins << incomingCapabilities << outgoingCapabilities; } else { - Q_FOREACH(KdeConnectPlugin* plugin, m_plugins) { - plugin->connected(); - } + m_supportedPlugins = PluginLoader::instance()->getPluginList().toSet(); + } + + reloadPlugins(); + + if (m_deviceLinks.size() == 1) { + Q_EMIT reachableStatusChanged(); } connect(link, &DeviceLink::pairStatusChanged, this, &Device::pairStatusChanged); @@ -296,6 +259,7 @@ void Device::removeLink(DeviceLink* link) //qCDebug(KDECONNECT_CORE) << "RemoveLink" << m_deviceLinks.size() << "links remaining"; if (m_deviceLinks.isEmpty()) { + m_supportedPlugins.clear(); reloadPlugins(); Q_EMIT reachableStatusChanged(); } @@ -317,17 +281,8 @@ bool Device::sendPackage(NetworkPackage& np) void Device::privateReceivedPackage(const NetworkPackage& np) { Q_ASSERT(np.type() != PACKAGE_TYPE_PAIR); - if (np.type() == PACKAGE_TYPE_CAPABILITIES) { - QSet newIncomingCapabilities = np.get("IncomingCapabilities", QStringList()).toSet(); - QSet newOutgoingCapabilities = np.get("OutgoingCapabilities", QStringList()).toSet(); - - if (newOutgoingCapabilities != m_outgoingCapabilities || newIncomingCapabilities != m_incomingCapabilities) { - m_incomingCapabilities = newIncomingCapabilities; - m_outgoingCapabilities = newOutgoingCapabilities; - reloadPlugins(); - } - } else if (isTrusted()) { - const QList plugins = m_pluginsByIncomingInterface.values(np.type()); + if (isTrusted()) { + const QList plugins = m_pluginsByIncomingCapability.values(np.type()); if (plugins.isEmpty()) { qWarning() << "discarding unsupported package" << np.type() << "for" << name(); } diff --git a/core/device.h b/core/device.h index f46685624..0e56723a3 100644 --- a/core/device.h +++ b/core/device.h @@ -43,7 +43,7 @@ class KDECONNECTCORE_EXPORT Device Q_PROPERTY(QString statusIconName READ statusIconName) Q_PROPERTY(bool isReachable READ isReachable NOTIFY reachableStatusChanged) Q_PROPERTY(bool isTrusted READ isTrusted NOTIFY trustedChanged) - Q_PROPERTY(QStringList unsupportedPlugins READ unsupportedPlugins NOTIFY pluginsChanged) + Q_PROPERTY(QStringList supportedPlugins READ supportedPlugins NOTIFY pluginsChanged) public: @@ -82,7 +82,6 @@ public: QString type() const { return type2str(m_deviceType); } QString iconName() const; QString statusIconName() const; - QStringList unsupportedPlugins() const { return m_unsupportedPlugins; } Q_SCRIPTABLE QString encryptionInfo() const; //Add and remove links @@ -106,6 +105,7 @@ public: void cleanUnneededLinks(); int protocolVersion() { return m_protocolVersion; } + QStringList supportedPlugins() const { return m_supportedPlugins.toList(); } public Q_SLOTS: ///sends a @p np package to the device @@ -147,13 +147,8 @@ private: //Fields (TODO: dPointer!) QHash m_plugins; //Capabilities stuff - QMultiMap m_pluginsByIncomingInterface; - QMultiMap m_pluginsByOutgoingInterface; - QSet m_incomingCapabilities; - QSet m_outgoingCapabilities; - QStringList m_supportedIncomingInterfaces; - QStringList m_supportedOutgoingInterfaces; - QStringList m_unsupportedPlugins; + QMultiMap m_pluginsByIncomingCapability; + QSet m_supportedPlugins; }; Q_DECLARE_METATYPE(Device*) diff --git a/core/networkpackage.cpp b/core/networkpackage.cpp index 2498aa5cc..0678c850d 100644 --- a/core/networkpackage.cpp +++ b/core/networkpackage.cpp @@ -67,6 +67,8 @@ void NetworkPackage::createIdentityPackage(NetworkPackage* np) np->set("deviceName", config->name()); np->set("deviceType", config->deviceType()); np->set("protocolVersion", NetworkPackage::ProtocolVersion); + np->set("incomingCapabilities", PluginLoader::instance()->incomingCapabilities()); + np->set("outgoingCapabilities", PluginLoader::instance()->outgoingCapabilities()); //qCDebug(KDECONNECT_CORE) << "createIdentityPackage" << np->serialize(); } diff --git a/core/networkpackagetypes.h b/core/networkpackagetypes.h index 02f0e8b71..b3c0cec90 100644 --- a/core/networkpackagetypes.h +++ b/core/networkpackagetypes.h @@ -23,6 +23,5 @@ #define PACKAGE_TYPE_IDENTITY QLatin1String("kdeconnect.identity") #define PACKAGE_TYPE_PAIR QLatin1String("kdeconnect.pair") -#define PACKAGE_TYPE_CAPABILITIES QLatin1String("kdeconnect.capabilities") #endif // NETWORKPACKAGETYPES_H diff --git a/core/pluginloader.cpp b/core/pluginloader.cpp index 9608d4652..442c369a1 100644 --- a/core/pluginloader.cpp +++ b/core/pluginloader.cpp @@ -83,7 +83,7 @@ KdeConnectPlugin* PluginLoader::instantiatePluginForDevice(const QString& plugin return ret; } -QStringList PluginLoader::incomingInterfaces() const +QStringList PluginLoader::incomingCapabilities() const { QSet ret; Q_FOREACH (const KPluginMetaData& service, plugins) { @@ -92,7 +92,7 @@ QStringList PluginLoader::incomingInterfaces() const return ret.toList(); } -QStringList PluginLoader::outgoingInterfaces() const +QStringList PluginLoader::outgoingCapabilities() const { QSet ret; Q_FOREACH (const KPluginMetaData& service, plugins) { @@ -100,3 +100,22 @@ QStringList PluginLoader::outgoingInterfaces() const } return ret.toList(); } + +QSet PluginLoader::pluginsForCapabilities(const QSet& incoming, const QSet& outgoing) +{ + QSet ret; + + Q_FOREACH (const KPluginMetaData& service, plugins) { + const QSet pluginIncomingCapabilities = KPluginMetaData::readStringList(service.rawData(), "X-KdeConnect-SupportedPackageType").toSet(); + const QSet pluginOutgoingCapabilities = KPluginMetaData::readStringList(service.rawData(), "X-KdeConnect-OutgoingPackageType").toSet(); + + if ((pluginIncomingCapabilities.isEmpty() && pluginOutgoingCapabilities.isEmpty()) + || incoming.intersects(pluginOutgoingCapabilities) || outgoing.intersects(pluginIncomingCapabilities)) { + ret += service.pluginId(); + } else { + qDebug() << "discarding..." << service.pluginId(); + } + } + + return ret; +} diff --git a/core/pluginloader.h b/core/pluginloader.h index f1722ab91..d1a538f82 100644 --- a/core/pluginloader.h +++ b/core/pluginloader.h @@ -36,12 +36,14 @@ class PluginLoader public: static PluginLoader* instance(); - QStringList incomingInterfaces() const; - QStringList outgoingInterfaces() const; QStringList getPluginList() const; KPluginMetaData getPluginInfo(const QString& name) const; KdeConnectPlugin* instantiatePluginForDevice(const QString& name, Device* device) const; + QStringList incomingCapabilities() const; + QStringList outgoingCapabilities() const; + QSet pluginsForCapabilities(const QSet &incoming, const QSet &outgoing); + private: PluginLoader(); QHash plugins; diff --git a/kcm/kcm.cpp b/kcm/kcm.cpp index 72ebd93fa..34b755d95 100644 --- a/kcm/kcm.cpp +++ b/kcm/kcm.cpp @@ -199,9 +199,9 @@ void KdeConnectKcm::deviceSelected(const QModelIndex& current) void KdeConnectKcm::resetCurrentDevice() { - const QStringList unsupportedPluginNames = currentDevice->unsupportedPlugins(); + const QStringList supportedPluginNames = currentDevice->supportedPlugins(); - if (m_oldUnsupportedPluginNames != unsupportedPluginNames) { + if (m_oldSupportedPluginNames != supportedPluginNames) { resetDeviceView(); } } @@ -222,12 +222,12 @@ void KdeConnectKcm::resetDeviceView() QList availablePluginInfo; QList unsupportedPluginInfo; - m_oldUnsupportedPluginNames = currentDevice->unsupportedPlugins(); + m_oldSupportedPluginNames = currentDevice->supportedPlugins(); for (auto it = pluginInfo.cbegin(), itEnd = pluginInfo.cend(); it!=itEnd; ++it) { - if (m_oldUnsupportedPluginNames.contains(it->pluginName())) { - unsupportedPluginInfo.append(*it); - } else { + if (m_oldSupportedPluginNames.contains(it->pluginName())) { availablePluginInfo.append(*it); + } else { + unsupportedPluginInfo.append(*it); } } diff --git a/kcm/kcm.h b/kcm/kcm.h index 32fce5ef5..38f0dba1e 100644 --- a/kcm/kcm.h +++ b/kcm/kcm.h @@ -70,7 +70,7 @@ private: DevicesSortProxyModel* sortProxyModel; DeviceDbusInterface* currentDevice; QModelIndex currentIndex; - QStringList m_oldUnsupportedPluginNames; + QStringList m_oldSupportedPluginNames; public Q_SLOTS: void unpair(); diff --git a/tests/pluginloadtest.cpp b/tests/pluginloadtest.cpp index a663547fa..73fe2370a 100644 --- a/tests/pluginloadtest.cpp +++ b/tests/pluginloadtest.cpp @@ -68,11 +68,11 @@ class PluginLoadTest : public QObject d->setPluginEnabled("kdeconnect_mousepad", false); QCOMPARE(d->isPluginEnabled("kdeconnect_mousepad"), false); - QVERIFY(d->unsupportedPlugins().contains("kdeconnect_remotecontrol")); + QVERIFY(d->supportedPlugins().contains("kdeconnect_remotecontrol")); d->setPluginEnabled("kdeconnect_mousepad", true); QCOMPARE(d->isPluginEnabled("kdeconnect_mousepad"), true); - QVERIFY(!d->unsupportedPlugins().contains("kdeconnect_remotecontrol")); + QVERIFY(d->supportedPlugins().contains("kdeconnect_remotecontrol")); } private: