diff --git a/autotests/dbusrunnertest.cpp b/autotests/dbusrunnertest.cpp --- a/autotests/dbusrunnertest.cpp +++ b/autotests/dbusrunnertest.cpp @@ -43,33 +43,36 @@ void initTestCase(); void cleanupTestCase(); void testMatch(); + void testMulti(); private: - QProcess *m_process; QStringList m_filesForCleanup; }; DBusRunnerTest::DBusRunnerTest() - : QObject(), - m_process(new QProcess(this)) + : QObject() { - m_process->start(QFINDTESTDATA("testremoterunner")); - QVERIFY(m_process->waitForStarted()); qRegisterMetaType >(); } DBusRunnerTest::~DBusRunnerTest() { - m_process->kill(); - m_process->waitForFinished(); } void DBusRunnerTest::initTestCase() { QStandardPaths::setTestModeEnabled(true); QDir(QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation)).mkpath(QStringLiteral("kservices5")); - const QString fakeServicePath = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + QStringLiteral("/kservices5/dbusrunnertest.desktop"); - QFile::copy(QFINDTESTDATA("dbusrunnertest.desktop"), fakeServicePath); - m_filesForCleanup << fakeServicePath; + { + const QString fakeServicePath = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + QStringLiteral("/kservices5/dbusrunnertest.desktop"); + QFile::copy(QFINDTESTDATA("dbusrunnertest.desktop"), fakeServicePath); + m_filesForCleanup << fakeServicePath; + + } + { + const QString fakeServicePath = QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + QStringLiteral("/kservices5/dbusrunnertestmulti.desktop"); + QFile::copy(QFINDTESTDATA("dbusrunnertestmulti.desktop"), fakeServicePath); + m_filesForCleanup << fakeServicePath; + } KSycoca::self()->ensureCacheValid(); } @@ -82,6 +85,12 @@ void DBusRunnerTest::testMatch() { + QProcess process; + process.start(QFINDTESTDATA("testremoterunner"), QStringList({"net.krunnertests.dave"})); + QVERIFY(process.waitForStarted()); + + QTest::qSleep(500); + RunnerManager m; auto s = KService::serviceByDesktopPath(QStringLiteral("dbusrunnertest.desktop")); QVERIFY(s); @@ -109,15 +118,54 @@ QCOMPARE(action->text(), QStringLiteral("Action 1")); - QSignalSpy processSpy(m_process, &QProcess::readyRead); + QSignalSpy processSpy(&process, &QProcess::readyRead); m.run(result); processSpy.wait(); - QCOMPARE(m_process->readAllStandardOutput().trimmed(), QByteArray("Running:id1:")); + QCOMPARE(process.readAllStandardOutput().trimmed(), QByteArray("Running:id1:")); result.setSelectedAction(action); m.run(result); processSpy.wait(); - QCOMPARE(m_process->readAllStandardOutput().trimmed(), QByteArray("Running:id1:action1")); + QCOMPARE(process.readAllStandardOutput().trimmed(), QByteArray("Running:id1:action1")); + + process.kill(); + process.waitForFinished(); +} + +void DBusRunnerTest::testMulti() +{ + QProcess process1; + process1.start(QFINDTESTDATA("testremoterunner"), QStringList({"net.krunnertests.multi.a1"})); + QVERIFY(process1.waitForStarted()); + + QProcess process2; + process2.start(QFINDTESTDATA("testremoterunner"), QStringList({"net.krunnertests.multi.a2"})); + QVERIFY(process2.waitForStarted()); + + QTest::qSleep(500); + + RunnerManager m; + auto s = KService::serviceByDesktopPath(QStringLiteral("dbusrunnertestmulti.desktop")); + QVERIFY(s); + m.loadRunner(s); + m.launchQuery(QStringLiteral("foo")); + + QSignalSpy spy(&m, &RunnerManager::matchesChanged); + QVERIFY(spy.wait()); + + //verify matches, must be one from each + QCOMPARE(m.matches().count(), 2); + + QString first = m.matches().at(0).data().toString(); + QString second = m.matches().at(1).data().toString(); + QVERIFY(first != second); + QVERIFY(first == "net.krunnertests.multi.a1" || first == "net.krunnertests.multi.a2"); + QVERIFY(second == "net.krunnertests.multi.a1" || second == "net.krunnertests.multi.a2"); + + process1.kill(); + process2.kill(); + process1.waitForFinished(); + process2.waitForFinished(); } diff --git a/autotests/dbusrunnertest.desktop b/autotests/dbusrunnertest.desktop --- a/autotests/dbusrunnertest.desktop +++ b/autotests/dbusrunnertest.desktop @@ -11,5 +11,5 @@ X-KDE-PluginInfo-License=LGPL X-KDE-PluginInfo-EnabledByDefault=true X-Plasma-API=DBus -X-Plasma-DBusRunner-Service=net.dave +X-Plasma-DBusRunner-Service=net.krunnertests.dave X-Plasma-DBusRunner-Path=/dave diff --git a/autotests/dbusrunnertest.desktop b/autotests/dbusrunnertestmulti.desktop copy from autotests/dbusrunnertest.desktop copy to autotests/dbusrunnertestmulti.desktop --- a/autotests/dbusrunnertest.desktop +++ b/autotests/dbusrunnertestmulti.desktop @@ -11,5 +11,5 @@ X-KDE-PluginInfo-License=LGPL X-KDE-PluginInfo-EnabledByDefault=true X-Plasma-API=DBus -X-Plasma-DBusRunner-Service=net.dave +X-Plasma-DBusRunner-Service=net.krunnertests.multi.* X-Plasma-DBusRunner-Path=/dave diff --git a/autotests/testremoterunner.h b/autotests/testremoterunner.h --- a/autotests/testremoterunner.h +++ b/autotests/testremoterunner.h @@ -7,7 +7,7 @@ { Q_OBJECT public: - TestRemoteRunner(); + TestRemoteRunner(const QString &serviceName); public Q_SLOTS: RemoteActions Actions(); diff --git a/autotests/testremoterunner.cpp b/autotests/testremoterunner.cpp --- a/autotests/testremoterunner.cpp +++ b/autotests/testremoterunner.cpp @@ -27,14 +27,14 @@ //Test DBus runner, if the search term contains "foo" it returns a match, otherwise nothing //Run prints a line to stdout -TestRemoteRunner::TestRemoteRunner() +TestRemoteRunner::TestRemoteRunner(const QString &serviceName) { new Krunner1Adaptor(this); qDBusRegisterMetaType(); qDBusRegisterMetaType(); qDBusRegisterMetaType(); qDBusRegisterMetaType(); - QDBusConnection::sessionBus().registerService(QStringLiteral("net.dave")); + QDBusConnection::sessionBus().registerService(serviceName); QDBusConnection::sessionBus().registerObject(QStringLiteral("/dave"), this); } @@ -73,6 +73,7 @@ int main(int argc, char ** argv) { QCoreApplication app(argc, argv); - TestRemoteRunner r; + Q_ASSERT(app.arguments().count() == 2); + TestRemoteRunner r(app.arguments()[1]); app.exec(); } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -10,7 +10,6 @@ runnersyntax.cpp) ecm_qt_declare_logging_category(KF5Runner_SRCS HEADER krunner_debug.h IDENTIFIER KRUNNER CATEGORY_NAME org.kde.krunner) set_property(SOURCE "data/org.kde.krunner1.xml" PROPERTY INCLUDE dbusutils_p.h) -qt5_add_dbus_interface(KF5Runner_SRCS "data/org.kde.krunner1.xml" krunner_iface) add_library(KF5Runner ${KF5Runner_SRCS}) diff --git a/src/dbusrunner.cpp b/src/dbusrunner.cpp --- a/src/dbusrunner.cpp +++ b/src/dbusrunner.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2017 David Edmundson + * Copyright (C) 2017,2018 David Edmundson * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU Library General Public License version 2 as @@ -24,105 +24,170 @@ #include #include #include +#include #include #include #include +#include #include #include "krunner_debug.h" -#include "krunner_iface.h" +#include "dbusutils_p.h" + +#define IFACE_NAME "org.kde.krunner1" DBusRunner::DBusRunner(const KService::Ptr service, QObject *parent) : Plasma::AbstractRunner(service, parent) + , m_mutex(QMutex::NonRecursive) { qDBusRegisterMetaType(); qDBusRegisterMetaType(); qDBusRegisterMetaType(); qDBusRegisterMetaType(); - QString serviceName = service->property(QStringLiteral("X-Plasma-DBusRunner-Service")).toString(); - QString path = service->property(QStringLiteral("X-Plasma-DBusRunner-Path")).toString(); + QString requestedServiceName = service->property(QStringLiteral("X-Plasma-DBusRunner-Service")).toString(); + m_path = service->property(QStringLiteral("X-Plasma-DBusRunner-Path")).toString(); - if (serviceName.isEmpty() || path.isEmpty()) { + if (requestedServiceName.isEmpty() || m_path.isEmpty()) { qCWarning(KRUNNER) << "Invalid entry:" << service->name(); return; } - m_interface = new OrgKdeKrunner1Interface(serviceName, path, QDBusConnection::sessionBus(), this); + if (requestedServiceName.endsWith(QLatin1Char('*'))) { + requestedServiceName.chop(1); + //find existing matching names + auto names = QDBusConnection::sessionBus().interface()->registeredServiceNames(); + if (names.isValid()) { + for(const QString serviceName : names.value()) { + if (serviceName.startsWith(requestedServiceName)) { + m_matchingServices << serviceName; + } + } + } + //and watch for changes + connect(QDBusConnection::sessionBus().interface(), &QDBusConnectionInterface::serviceOwnerChanged, + this, [this, requestedServiceName](const QString &serviceName, const QString &oldOwner, const QString &newOwner) { + if (!serviceName.startsWith(requestedServiceName)) { + return; + } + if (!oldOwner.isEmpty() && !newOwner.isEmpty()) { + //changed owner, but service still exists. Don't need to adjust anything + return; + } + QMutexLocker lock(&m_mutex); + if (!newOwner.isEmpty()) { + m_matchingServices.insert(serviceName); + } + if (!oldOwner.isEmpty()) { + m_matchingServices.remove(serviceName); + } + }); + } else { + //don't check when not wildcarded, as it could be used with DBus-activation + m_matchingServices << requestedServiceName; + } connect(this, &AbstractRunner::prepare, this, &DBusRunner::requestActions); } DBusRunner::~DBusRunner() = default; void DBusRunner::requestActions() { - auto reply = m_interface->Actions(); - auto watcher = new QDBusPendingCallWatcher(reply); - connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher]() { - watcher->deleteLater(); - clearActions(); - QDBusReply reply = *watcher; - if (!reply.isValid()) { - return; - } - for(const RemoteAction &action: reply.value()) { - auto a = addAction(action.id, QIcon::fromTheme(action.iconName), action.text); - a->setData(action.id); - } - }); + clearActions(); + m_actions.clear(); + + //in the multi-services case, register separate actions from each plugin in case they happen to be somehow different + //then match together in matchForAction() + + for (const QString &service: m_matchingServices) { + auto getActionsMethod = QDBusMessage::createMethodCall(service, m_path, QStringLiteral(IFACE_NAME), QStringLiteral("Actions")); + QDBusPendingReply reply = QDBusConnection::sessionBus().asyncCall(getActionsMethod); + + auto watcher = new QDBusPendingCallWatcher(reply); + connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher, service]() { + watcher->deleteLater(); + QDBusReply reply = *watcher; + if (!reply.isValid()) { + return; + } + for(const RemoteAction &action: reply.value()) { + auto a = addAction(action.id, QIcon::fromTheme(action.iconName), action.text); + a->setData(action.id); + m_actions[service].append(a); + } + }); + } } void DBusRunner::match(Plasma::RunnerContext &context) { - if (!m_interface) { - return; + QSet services; + { + QMutexLocker lock(&m_mutex); + services = m_matchingServices; } - - auto reply = m_interface->Match(context.query()); - reply.waitForFinished(); //AbstractRunner::match is called in a new thread, may as well block - if (reply.isError()) { - qCDebug(KRUNNER) << "Error calling" << m_interface->service() << " :" << reply.error().name() << reply.error().message(); - return; + //we scope watchers to make sure the lambda that captures context by reference definitely gets disconnected when this function ends + QList> watchers; + + for (const QString service : services) { + auto matchMethod = QDBusMessage::createMethodCall(service, m_path, QStringLiteral(IFACE_NAME), QStringLiteral("Match")); + matchMethod.setArguments(QList({context.query()})); + QDBusPendingReply reply = QDBusConnection::sessionBus().asyncCall(matchMethod); + + auto watcher = new QDBusPendingCallWatcher(reply); + watchers << QSharedPointer(watcher); + connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, service, &context, reply]() { + if (reply.isError()) { + qCDebug(KRUNNER) << "Error calling" << service << " :" << reply.error().name() << reply.error().message(); + return; + } + for(const RemoteMatch &match: reply.value()) { + Plasma::QueryMatch m(this); + + m.setText(match.text); + m.setId(match.id); + m.setData(service); + m.setIconName(match.iconName); + m.setType(match.type); + m.setRelevance(match.relevance); + + //split is essential items are as native DBus types, optional extras are in the property map (which is obviously a lot slower to parse) + m.setUrls(QUrl::fromStringList(match.properties.value(QStringLiteral("urls")).toStringList())); + m.setMatchCategory(match.properties.value(QStringLiteral("category")).toString()); + m.setSubtext(match.properties.value(QStringLiteral("subtext")).toString()); + + context.addMatch(m); + }; + }); } - for(const RemoteMatch &match: reply.value()) { - Plasma::QueryMatch m(this); - - m.setText(match.text); - m.setData(match.id); - m.setIconName(match.iconName); - m.setType(match.type); - m.setRelevance(match.relevance); - - //split is essential items are as native DBus types, optional extras are in the property map (which is obviously a lot slower to parse) - m.setUrls(QUrl::fromStringList(match.properties.value(QStringLiteral("urls")).toStringList())); - m.setMatchCategory(match.properties.value(QStringLiteral("category")).toString()); - m.setSubtext(match.properties.value(QStringLiteral("subtext")).toString()); - - context.addMatch(m); + //we're done matching when every service replies + for (auto w : watchers) { + w->waitForFinished(); } } QList DBusRunner::actionsForMatch(const Plasma::QueryMatch &match) { Q_UNUSED(match) - return actions().values(); + const QString service = match.data().toString(); + return m_actions.value(service); } void DBusRunner::run(const Plasma::RunnerContext &context, const Plasma::QueryMatch &match) { Q_UNUSED(context); - if (!m_interface) { - return; - } QString actionId; - QString matchId = match.data().toString(); + QString matchId = match.id().mid(id().length() + 1); //QueryMatch::setId mangles the match ID with runnerID + '_'. This unmangles it + QString service = match.data().toString(); if (match.selectedAction()) { actionId = match.selectedAction()->data().toString(); } - m_interface->Run(matchId, actionId); //don't wait for reply before returning to process + auto runMethod = QDBusMessage::createMethodCall(service, m_path, QStringLiteral(IFACE_NAME), QStringLiteral("Run")); + runMethod.setArguments(QList({matchId, actionId})); + QDBusConnection::sessionBus().call(runMethod, QDBus::NoBlock); } diff --git a/src/dbusrunner_p.h b/src/dbusrunner_p.h --- a/src/dbusrunner_p.h +++ b/src/dbusrunner_p.h @@ -22,6 +22,10 @@ #include "dbusutils_p.h" class OrgKdeKrunner1Interface; +#include +#include +#include +#include class DBusRunner : public Plasma::AbstractRunner { @@ -38,5 +42,8 @@ private: void requestActions(); void setActions(const RemoteActions &remoteActions); - OrgKdeKrunner1Interface *m_interface = nullptr; + QMutex m_mutex; //needed round any variable also accessed from Match + QString m_path; + QSet m_matchingServices; + QHash > m_actions; };