TaskTree: Introduce Storage item

The Storage item makes it possible to define the whole
subtree as a self-contained, full-functional recipe, without a need
for passing (together with recipe) a one-use only (disposable)
pointer to storage object.

That's the last closing element of making the idea of pure,
value-based recipe real. It makes the TaskTree machinery ultimately
powerful.

Change-Id: Icd81bdd3e94251e8b241b2b550957d566fa4ab75
Reviewed-by: hjk <hjk@qt.io>

src/libs/utils/tasktree.cpp

@@ -117,6 +117,9 @@ void TaskItem::addChildren(const QList<TaskItem> &children)
             if (child.m_groupHandler.m_dynamicSetupHandler)
                 m_groupHandler.m_dynamicSetupHandler = child.m_groupHandler.m_dynamicSetupHandler;
             break;
+        case Type::Storage:
+            m_storageList.append(child.m_storageList);
+            break;
         }
     }
 }
@@ -144,11 +147,18 @@ public:
     void resetSuccessBit();
     void updateSuccessBit(bool success);
 
+    void createStorages();
+    void deleteStorages();
+    void activateStorages();
+    void deactivateStorages();
+
     TaskTreePrivate *m_taskTreePrivate = nullptr;
     TaskContainer *m_parentContainer = nullptr;
     const ExecuteMode m_executeMode = ExecuteMode::Parallel;
     WorkflowPolicy m_workflowPolicy = WorkflowPolicy::StopOnError;
     const TaskItem::GroupHandler m_groupHandler;
+    QList<TreeStorageBase> m_storageList;
+    QList<int> m_storageIdList;
     int m_taskCount = 0;
     GroupConfig m_groupConfig;
     QList<TaskNode *> m_children;
@@ -157,6 +167,22 @@ public:
     bool m_successBit = true;
 };
 
+class StorageActivator
+{
+public:
+    StorageActivator(TaskContainer &container)
+        : m_container(container)
+    {
+        m_container.activateStorages();
+    }
+    ~StorageActivator()
+    {
+        m_container.deactivateStorages();
+    }
+private:
+    TaskContainer &m_container;
+};
+
 class TaskNode : public QObject
 {
 public:
@@ -244,6 +270,7 @@ TaskContainer::TaskContainer(TaskTreePrivate *taskTreePrivate, TaskContainer *pa
     , m_executeMode(task.executeMode())
     , m_workflowPolicy(task.workflowPolicy())
     , m_groupHandler(task.groupHandler())
+    , m_storageList(task.storageList())
 {
     const QList<TaskItem> &children = task.children();
     for (const TaskItem &child : children) {
@@ -264,6 +291,9 @@ void TaskContainer::start()
     m_groupConfig = {};
     m_selectedChildren.clear();
 
+    createStorages();
+    {
+        StorageActivator activator(*this);
         if (m_groupHandler.m_setupHandler) {
             GuardLocker locker(m_taskTreePrivate->m_guard);
             m_groupHandler.m_setupHandler();
@@ -273,6 +303,7 @@ void TaskContainer::start()
             GuardLocker locker(m_taskTreePrivate->m_guard);
             m_groupConfig = m_groupHandler.m_dynamicSetupHandler();
         }
+    }
 
     if (m_groupConfig.action == GroupAction::StopWithDone || m_groupConfig.action == GroupAction::StopWithError) {
         const bool success = m_groupConfig.action == GroupAction::StopWithDone;
@@ -374,6 +405,8 @@ void TaskContainer::invokeEndHandler(bool success, bool propagateToParent)
 {
     m_currentIndex = -1;
     m_successBit = success;
+    {
+        StorageActivator activator(*this);
         if (success && m_groupHandler.m_doneHandler) {
             GuardLocker locker(m_taskTreePrivate->m_guard);
             m_groupHandler.m_doneHandler();
@@ -381,6 +414,8 @@ void TaskContainer::invokeEndHandler(bool success, bool propagateToParent)
             GuardLocker locker(m_taskTreePrivate->m_guard);
             m_groupHandler.m_errorHandler();
         }
+    }
+    deleteStorages();
 
     if (!propagateToParent)
         return;
@@ -420,6 +455,46 @@ void TaskContainer::updateSuccessBit(bool success)
     }
 }
 
+void TaskContainer::createStorages()
+{
+    // TODO: Don't create new storage for already created storages with the same shared pointer.
+
+    for (int i = 0; i < m_storageList.size(); ++i)
+        m_storageIdList << m_storageList[i].createStorage();
+}
+
+void TaskContainer::deleteStorages()
+{
+    // TODO: Do the opposite
+
+    for (int i = 0; i < m_storageList.size(); ++i) // iterate in reverse order?
+        m_storageList[i].deleteStorage(m_storageIdList.value(i));
+
+    m_storageIdList.clear();
+}
+
+void TaskContainer::activateStorages()
+{
+    // TODO: check if the same shared storage was already activated. Don't activate recursively.
+
+    if (m_parentContainer)
+        m_parentContainer->activateStorages();
+
+    for (int i = 0; i < m_storageList.size(); ++i)
+        m_storageList[i].activateStorage(m_storageIdList.value(i));
+}
+
+void TaskContainer::deactivateStorages()
+{
+    // TODO: Do the opposite
+
+    for (int i = 0; i < m_storageList.size(); ++i) // iterate in reverse order?
+        m_storageList[i].activateStorage(0);
+
+    if (m_parentContainer)
+        m_parentContainer->deactivateStorages();
+}
+
 bool TaskNode::start()
 {
     if (!isTask()) {
@@ -428,10 +503,13 @@ bool TaskNode::start()
     }
     m_task.reset(m_taskHandler.m_createHandler());
     {
+        StorageActivator activator(m_container);
         GuardLocker locker(m_container.m_taskTreePrivate->m_guard);
         m_taskHandler.m_setupHandler(*m_task.get());
     }
     connect(m_task.get(), &TaskInterface::done, this, [this](bool success) {
+        {
+            StorageActivator activator(m_container);
             if (success && m_taskHandler.m_doneHandler) {
                 GuardLocker locker(m_container.m_taskTreePrivate->m_guard);
                 m_taskHandler.m_doneHandler(*m_task.get());
@@ -439,6 +517,7 @@ bool TaskNode::start()
                 GuardLocker locker(m_container.m_taskTreePrivate->m_guard);
                 m_taskHandler.m_errorHandler(*m_task.get());
             }
+        }
         m_container.m_taskTreePrivate->advanceProgress(1);
 
         m_task.release()->deleteLater();
@@ -465,6 +544,7 @@ void TaskNode::stop()
     // TODO: cancelHandler?
     // TODO: call TaskInterface::stop() ?
     if (m_taskHandler.m_errorHandler) {
+        StorageActivator activator(m_container);
         GuardLocker locker(m_container.m_taskTreePrivate->m_guard);
         m_taskHandler.m_errorHandler(*m_task.get());
     }

src/libs/utils/tasktree.h

@@ -143,15 +143,16 @@ public:
     TaskHandler taskHandler() const { return m_taskHandler; }
     GroupHandler groupHandler() const { return m_groupHandler; }
     QList<TaskItem> children() const { return m_children; }
+    QList<TreeStorageBase> storageList() const { return m_storageList; }
 
 protected:
     enum class Type {
         Group,
+        Storage,
         Mode,
         Policy,
         TaskHandler,
         GroupHandler
-        // TODO: Add Cond type (with CondHandler and True and False branches)?
     };
 
     TaskItem() = default;
@@ -167,6 +168,9 @@ protected:
     TaskItem(const GroupHandler &handler)
         : m_type(Type::GroupHandler)
         , m_groupHandler(handler) {}
+    TaskItem(const TreeStorageBase &storage)
+        : m_type(Type::Storage)
+        , m_storageList{storage} {}
     void addChildren(const QList<TaskItem> &children);
 
 private:
@@ -175,6 +179,7 @@ private:
     WorkflowPolicy m_workflowPolicy = WorkflowPolicy::StopOnError;
     TaskHandler m_taskHandler;
     GroupHandler m_groupHandler;
+    QList<TreeStorageBase> m_storageList;
     QList<TaskItem> m_children;
 };
 
@@ -185,6 +190,12 @@ public:
     Group(std::initializer_list<TaskItem> children) { addChildren(children); }
 };
 
+class QTCREATOR_UTILS_EXPORT Storage : public TaskItem
+{
+public:
+    Storage(const TreeStorageBase &storage) : TaskItem(storage) { }
+};
+
 class QTCREATOR_UTILS_EXPORT Execute : public TaskItem
 {
 public:

tests/auto/utils/tasktree/tst_tasktree.cpp

@@ -496,46 +496,72 @@ void tst_TaskTree::processTree()
     QCOMPARE(errorCount, expectedErrorCount);
 }
 
+struct CustomStorage
+{
+    CustomStorage() { ++s_count; }
+    ~CustomStorage() { --s_count; }
+    Log m_log;
+    static int instanceCount() { return s_count; }
+private:
+    static int s_count;
+};
+
+int CustomStorage::s_count = 0;
+static Log s_log;
+
 void tst_TaskTree::storage_data()
 {
     using namespace Tasking;
     using namespace std::placeholders;
 
     QTest::addColumn<Group>("root");
-    QTest::addColumn<std::shared_ptr<Log>>("storageLog");
+    QTest::addColumn<TreeStorage<CustomStorage>>("storageLog");
     QTest::addColumn<Log>("expectedLog");
     QTest::addColumn<bool>("runningAfterStart");
     QTest::addColumn<bool>("success");
 
-    // TODO: Much better approach would be that the TaskTree, when started, creates a
+    TreeStorage<CustomStorage> storageLog;
-    //       storage dynamically (then Group should be a template with storage type as a
-    //       template parameter) and a pointer (or reference) to the storage is being passed
-    //       into handlers (? how ?).
-    std::shared_ptr<Log> log(new Log);
 
-    const auto setupProcessHelper = [this, log](QtcProcess &process, const QStringList &args, int processId) {
+    const auto setupProcessHelper = [storageLog, testAppPath = m_testAppPath]
-        process.setCommand(CommandLine(m_testAppPath, args));
+            (QtcProcess &process, const QStringList &args, int processId) {
+        process.setCommand(CommandLine(testAppPath, args));
         process.setProperty(s_processIdProperty, processId);
-        log->append({processId, Handler::Setup});
+        storageLog->m_log.append({processId, Handler::Setup});
     };
     const auto setupProcess = [setupProcessHelper](QtcProcess &process, int processId) {
         setupProcessHelper(process, {"-return", "0"}, processId);
     };
-    const auto readResult = [log](const QtcProcess &process) {
+    const auto readResult = [storageLog](const QtcProcess &process) {
         const int processId = process.property(s_processIdProperty).toInt();
-        log->append({processId, Handler::Done});
+        storageLog->m_log.append({processId, Handler::Done});
     };
-    const auto groupSetup = [log](int processId) {
+    const auto groupSetup = [storageLog](int processId) {
-        log->append({processId, Handler::GroupSetup});
+        storageLog->m_log.append({processId, Handler::GroupSetup});
     };
-    const auto groupDone = [log](int processId) {
+    const auto groupDone = [storageLog](int processId) {
-        log->append({processId, Handler::GroupDone});
+        storageLog->m_log.append({processId, Handler::GroupDone});
     };
-    const auto rootDone = [log] {
+    const auto rootDone = [storageLog] {
-        log->append({-1, Handler::GroupDone});
+        storageLog->m_log.append({-1, Handler::GroupDone});
+        s_log = storageLog->m_log;
     };
 
-    const Group nestedRoot {
+    const Log expectedLog{{1, Handler::GroupSetup},
+                          {2, Handler::GroupSetup},
+                          {3, Handler::GroupSetup},
+                          {4, Handler::GroupSetup},
+                          {5, Handler::GroupSetup},
+                          {5, Handler::Setup},
+                          {5, Handler::Done},
+                          {5, Handler::GroupDone},
+                          {4, Handler::GroupDone},
+                          {3, Handler::GroupDone},
+                          {2, Handler::GroupDone},
+                          {1, Handler::GroupDone},
+                          {-1, Handler::GroupDone}};
+
+    const Group root {
+        Storage(storageLog),
         Group {
             Group {
                 Group {
@@ -560,20 +586,7 @@ void tst_TaskTree::storage_data()
         OnGroupDone(rootDone)
     };
 
-    const Log nestedLog{{1, Handler::GroupSetup},
+    QTest::newRow("Storage") << root << storageLog << expectedLog << true << true;
-                        {2, Handler::GroupSetup},
-                        {3, Handler::GroupSetup},
-                        {4, Handler::GroupSetup},
-                        {5, Handler::GroupSetup},
-                        {5, Handler::Setup},
-                        {5, Handler::Done},
-                        {5, Handler::GroupDone},
-                        {4, Handler::GroupDone},
-                        {3, Handler::GroupDone},
-                        {2, Handler::GroupDone},
-                        {1, Handler::GroupDone},
-                        {-1, Handler::GroupDone}};
-    QTest::newRow("Nested") << nestedRoot << log << nestedLog << true << true;
 }
 
 void tst_TaskTree::storage()
@@ -581,11 +594,16 @@ void tst_TaskTree::storage()
     using namespace Tasking;
 
     QFETCH(Group, root);
-    QFETCH(std::shared_ptr<Log>, storageLog);
+    QFETCH(TreeStorage<CustomStorage>, storageLog);
     QFETCH(Log, expectedLog);
     QFETCH(bool, runningAfterStart);
     QFETCH(bool, success);
 
+    s_log.clear();
+
+    QVERIFY(storageLog.isValid());
+    QCOMPARE(CustomStorage::instanceCount(), 0);
+
     QEventLoop eventLoop;
     TaskTree processTree(root);
     int doneCount = 0;
@@ -593,6 +611,7 @@ void tst_TaskTree::storage()
     connect(&processTree, &TaskTree::done, this, [&doneCount, &eventLoop] { ++doneCount; eventLoop.quit(); });
     connect(&processTree, &TaskTree::errorOccurred, this, [&errorCount, &eventLoop] { ++errorCount; eventLoop.quit(); });
     processTree.start();
+    QCOMPARE(CustomStorage::instanceCount(), 1);
     QCOMPARE(processTree.isRunning(), runningAfterStart);
 
     QTimer timer;
@@ -603,7 +622,8 @@ void tst_TaskTree::storage()
     eventLoop.exec();
 
     QVERIFY(!processTree.isRunning());
-    QCOMPARE(*storageLog, expectedLog);
+    QCOMPARE(s_log, expectedLog);
+    QCOMPARE(CustomStorage::instanceCount(), 0);
 
     const int expectedDoneCount = success ? 1 : 0;
     const int expectedErrorCount = success ? 0 : 1;