From ba7128b30850594c7efb258d1794e377eede364a Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Sun, 1 Jan 2017 13:37:19 +0100 Subject: Instead of using the action system we use controllers only. It's simpler, and the action system was just too complex to use in a typesafe way. --- framework/actions/CMakeLists.txt | 4 +- framework/actions/actionbroker.cpp | 5 ++ framework/actions/actionbroker.h | 1 + framework/actions/actionhandler.cpp | 17 ++++++ framework/actions/actionhandler.h | 55 +++++++++++++++--- framework/actions/context.cpp | 34 +++++++++++ framework/actions/context.h | 27 +++++++-- framework/actions/tests/CMakeLists.txt | 6 ++ framework/actions/tests/actiontest.cpp | 102 +++++++++++++++++++++++++++++++++ 9 files changed, 238 insertions(+), 13 deletions(-) create mode 100644 framework/actions/tests/CMakeLists.txt create mode 100644 framework/actions/tests/actiontest.cpp (limited to 'framework/actions') diff --git a/framework/actions/CMakeLists.txt b/framework/actions/CMakeLists.txt index 9cf0acd1..9fc43b9b 100644 --- a/framework/actions/CMakeLists.txt +++ b/framework/actions/CMakeLists.txt @@ -9,8 +9,10 @@ set(SRCS add_library(actionplugin SHARED ${SRCS}) -target_link_libraries(actionplugin KF5::Async) +target_link_libraries(actionplugin KF5::Async sink) qt5_use_modules(actionplugin Core Quick Qml) install(TARGETS actionplugin DESTINATION ${QML_INSTALL_DIR}/org/kube/framework/actions) install(FILES qmldir DESTINATION ${QML_INSTALL_DIR}/org/kube/framework/actions) + +add_subdirectory(tests) diff --git a/framework/actions/actionbroker.cpp b/framework/actions/actionbroker.cpp index 17145440..f6bfdd8e 100644 --- a/framework/actions/actionbroker.cpp +++ b/framework/actions/actionbroker.cpp @@ -94,3 +94,8 @@ void ActionBroker::registerHandler(const QByteArray &actionId, ActionHandler *ha { mHandler.insert(actionId, handler); } + +void ActionBroker::unregisterHandler(const QByteArray &actionId, ActionHandler *handler) +{ + mHandler.remove(actionId, handler); +} diff --git a/framework/actions/actionbroker.h b/framework/actions/actionbroker.h index 84678c16..d893a3e7 100644 --- a/framework/actions/actionbroker.h +++ b/framework/actions/actionbroker.h @@ -36,6 +36,7 @@ public: ActionResult executeAction(const QByteArray &actionId, Context *context, const QList> &preHandler, const QList> &postHandler); void registerHandler(const QByteArray &actionId, ActionHandler *handler); + void unregisterHandler(const QByteArray &actionId, ActionHandler *handler); Q_SIGNALS: void readyChanged(); diff --git a/framework/actions/actionhandler.cpp b/framework/actions/actionhandler.cpp index dc9edeca..eb7b3224 100644 --- a/framework/actions/actionhandler.cpp +++ b/framework/actions/actionhandler.cpp @@ -31,6 +31,11 @@ ActionHandler::ActionHandler(QObject *parent) } +ActionHandler::~ActionHandler() +{ + ActionBroker::instance().unregisterHandler(mActionId, this); +} + bool ActionHandler::isActionReady(Context *context) { if (context) { @@ -67,6 +72,8 @@ ActionResult ActionHandler::execute(Context *context) void ActionHandler::setActionId(const QByteArray &actionId) { + //Reassigning the id is not supported + Q_ASSERT(mActionId.isEmpty()); mActionId = actionId; ActionBroker::instance().registerHandler(actionId, this); } @@ -76,6 +83,16 @@ QByteArray ActionHandler::actionId() const return mActionId; } +void ActionHandler::setRequiredProperties(const QSet &requiredProperties) +{ + mRequiredProperties = requiredProperties; +} + +QSet ActionHandler::requiredProperties() const +{ + return mRequiredProperties; +} + ActionHandlerHelper::ActionHandlerHelper(const Handler &handler) : ActionHandler(nullptr), diff --git a/framework/actions/actionhandler.h b/framework/actions/actionhandler.h index 09ed13c6..5ccf0ac7 100644 --- a/framework/actions/actionhandler.h +++ b/framework/actions/actionhandler.h @@ -24,9 +24,9 @@ #include #include "actionresult.h" +#include "context.h" namespace Kube { -class Context; class ActionHandler : public QObject { @@ -35,6 +35,7 @@ class ActionHandler : public QObject public: ActionHandler(QObject *parent = 0); + virtual ~ActionHandler(); virtual bool isActionReady(Context *context); @@ -43,25 +44,65 @@ public: void setActionId(const QByteArray &); QByteArray actionId() const; + void setRequiredProperties(const QSet &requiredProperties); + QSet requiredProperties() const; + private: QByteArray mActionId; + QSet mRequiredProperties; +}; + +template +class ActionHandlerBase : public ActionHandler +{ +public: + ActionHandlerBase(const QByteArray &actionId) + : ActionHandler{} + { + setActionId(actionId); + } + + bool isActionReady(Context *c) Q_DECL_OVERRIDE + { + auto wrapper = ContextType{*c}; + return isActionReady(wrapper); + } + + ActionResult execute(Context *c) Q_DECL_OVERRIDE + { + ActionResult result; + auto wrapper = ContextType{*c}; + execute(wrapper) + .template syncThen([=](const KAsync::Error &error) { + auto modifyableResult = result; + if (error) { + qWarning() << "Job failed: " << error.errorCode << error.errorMessage; + modifyableResult.setError(1); + } + modifyableResult.setDone(); + }).exec(); + return result; + } +protected: + + virtual bool isActionReady(ContextType &) { return true; } + virtual KAsync::Job execute(ContextType &) = 0; }; class ActionHandlerHelper : public ActionHandler { - Q_OBJECT public: - typedef std::function IsReadyFunction; - typedef std::function Handler; - typedef std::function(Context*)> JobHandler; + typedef std::function IsReadyFunction; + typedef std::function Handler; + typedef std::function(Context *)> JobHandler; ActionHandlerHelper(const Handler &); ActionHandlerHelper(const IsReadyFunction &, const Handler &); ActionHandlerHelper(const QByteArray &actionId, const IsReadyFunction &, const Handler &); ActionHandlerHelper(const QByteArray &actionId, const IsReadyFunction &, const JobHandler &); - bool isActionReady(Context *context) Q_DECL_OVERRIDE; - ActionResult execute(Context *context) Q_DECL_OVERRIDE; + bool isActionReady(Context *) Q_DECL_OVERRIDE; + ActionResult execute(Context *) Q_DECL_OVERRIDE; private: const IsReadyFunction isReadyFunction; const Handler handlerFunction; diff --git a/framework/actions/context.cpp b/framework/actions/context.cpp index 8f370a0b..45b660a9 100644 --- a/framework/actions/context.cpp +++ b/framework/actions/context.cpp @@ -29,6 +29,20 @@ Context::Context(QObject *parent) } +Context::Context(const Context &other) + : QObject() +{ + *this = other; +} + +Context &Context::operator=(const Context &other) +{ + for (const auto &p : other.availableProperties()) { + setProperty(p, other.property(p)); + } + return *this; +} + void Context::clear() { auto meta = metaObject(); @@ -41,6 +55,20 @@ void Context::clear() } } +QSet Context::availableProperties() const +{ + QSet names; + auto meta = metaObject(); + for (auto i = meta->propertyOffset(); i < meta->propertyCount(); i++) { + auto property = meta->property(i); + names << property.name(); + } + for (const auto &p : dynamicPropertyNames()) { + names << p; + } + return names; +} + QDebug operator<<(QDebug dbg, const Kube::Context &context) { dbg << "Kube::Context {\n"; @@ -55,3 +83,9 @@ QDebug operator<<(QDebug dbg, const Kube::Context &context) dbg << "\n}"; return dbg; } + +QDebug operator<<(QDebug dbg, const Kube::ContextWrapper &context) +{ + dbg << context.context; + return dbg; +} diff --git a/framework/actions/context.h b/framework/actions/context.h index 42ae3a93..4207fe12 100644 --- a/framework/actions/context.h +++ b/framework/actions/context.h @@ -19,17 +19,20 @@ #pragma once #include - #define KUBE_CONTEXT_PROPERTY(TYPE, NAME, LOWERCASENAME) \ public: Q_PROPERTY(TYPE LOWERCASENAME MEMBER m##NAME NOTIFY LOWERCASENAME##Changed) \ + Q_SIGNALS: void LOWERCASENAME##Changed(); \ + private: TYPE m##NAME; + +#define KUBE_CONTEXTWRAPPER_PROPERTY(TYPE, NAME, LOWERCASENAME) \ + public: \ struct NAME { \ static constexpr const char *name = #LOWERCASENAME; \ typedef TYPE Type; \ }; \ - void set##NAME(const TYPE &value) { setProperty(NAME::name, QVariant::fromValue(value)); } \ - TYPE get##NAME() const { return m##NAME; } \ - Q_SIGNALS: void LOWERCASENAME##Changed(); \ - private: TYPE m##NAME; + void set##NAME(const TYPE &value) { context.setProperty(NAME::name, QVariant::fromValue(value)); } \ + void clear##NAME() { context.setProperty(NAME::name, QVariant{}); } \ + TYPE get##NAME() const { return context.property(NAME::name).value(); } \ namespace Kube { @@ -38,13 +41,27 @@ class Context : public QObject { Q_OBJECT public: Context(QObject *parent = 0); + Context(const Context &); + virtual ~Context(){}; + + Context &operator=(const Context &); + virtual void clear(); + + QSet availableProperties() const; +}; + +class ContextWrapper { +public: + ContextWrapper(Context &c) : context{c} {} + Context &context; }; } QDebug operator<<(QDebug dbg, const Kube::Context &); +QDebug operator<<(QDebug dbg, const Kube::ContextWrapper &); Q_DECLARE_METATYPE(Kube::Context*); diff --git a/framework/actions/tests/CMakeLists.txt b/framework/actions/tests/CMakeLists.txt new file mode 100644 index 00000000..af872a3b --- /dev/null +++ b/framework/actions/tests/CMakeLists.txt @@ -0,0 +1,6 @@ +include_directories(${CMAKE_CURRENT_BINARY_DIR}) +cmake_policy(SET CMP0063 NEW) +add_executable(actiontest actiontest.cpp) +add_test(actiontest sinkactiontest) +qt5_use_modules(actiontest Core Test) +target_link_libraries(actiontest actionplugin) diff --git a/framework/actions/tests/actiontest.cpp b/framework/actions/tests/actiontest.cpp new file mode 100644 index 00000000..a4ec4432 --- /dev/null +++ b/framework/actions/tests/actiontest.cpp @@ -0,0 +1,102 @@ +#include +#include +#include + +#include +#include +#include + +#include + +SINK_DEBUG_AREA("actiontest") + +class HandlerContext : public Kube::Context { + Q_OBJECT + KUBE_CONTEXT_PROPERTY(QString, Property1, property1) + KUBE_CONTEXT_PROPERTY(QString, Property2, property2) +}; + +class HandlerContextWrapper : public Kube::ContextWrapper { + using Kube::ContextWrapper::ContextWrapper; + KUBE_CONTEXTWRAPPER_PROPERTY(QString, Property1, property1) + KUBE_CONTEXTWRAPPER_PROPERTY(QString, Property2, property2) +}; + + + +class Handler : public Kube::ActionHandlerBase +{ +public: + Handler() : Kube::ActionHandlerBase{"org.kde.kube.test.action1"} + {} + + //TODO default implementation checks that all defined properties are available in the context + // bool isReady() override { + // auto accountId = context->property("accountId").value(); + // return !accountId.isEmpty(); + // } + + KAsync::Job execute(HandlerContextWrapper &context) + { + SinkLog() << "Executing action1"; + SinkLog() << context; + executions.append(context.context); + return KAsync::null(); + } + mutable QList executions; +}; + +class Context1 : public Kube::ContextWrapper { + using Kube::ContextWrapper::ContextWrapper; + KUBE_CONTEXTWRAPPER_PROPERTY(QString, Property1, property1) + KUBE_CONTEXTWRAPPER_PROPERTY(QByteArray, Property2, property2) +}; + +class Context2 : public Kube::ContextWrapper { + using Kube::ContextWrapper::ContextWrapper; + KUBE_CONTEXTWRAPPER_PROPERTY(QByteArray, Property2, property2) +}; + + +class ActionTest : public QObject +{ + Q_OBJECT +private slots: + + void initTestCase() + { + } + + void testActionExecution() + { + Handler actionHandler; + + HandlerContext context; + //Kube::Context context; + HandlerContextWrapper{context}.setProperty1(QString("property1")); + context.setProperty("property2", QVariant::fromValue(QString("property2"))); + auto future = Kube::Action("org.kde.kube.test.action1", context).executeWithResult(); + + QTRY_VERIFY(future.isDone()); + QVERIFY(!future.error()); + + QCOMPARE(actionHandler.executions.size(), 1); + QCOMPARE(actionHandler.executions.first().availableProperties().size(), 2); + } + + void testContextCasting() + { + Kube::Context c; + + Context1 context1{c}; + context1.setProperty1("property1"); + context1.setProperty2("property2"); + + auto context2 = Context2{c}; + QCOMPARE(context2.getProperty2(), QByteArray("property2")); + } + +}; + +QTEST_GUILESS_MAIN(ActionTest) +#include "actiontest.moc" -- cgit v1.2.3