TaskTree: Use DoneResult in Task done handler

Instead of using ambiguous bool.

Change-Id: I7eb8e97947f23161c5c2bf5e22575e49bea35d61
Reviewed-by: hjk <hjk@qt.io>
This commit is contained in:
Jarek Kobus
2023-11-06 18:52:19 +01:00
parent 7ff13f1eaa
commit 0f6e6cebf7
3 changed files with 72 additions and 66 deletions

View File

@@ -837,6 +837,11 @@ static SetupResult toSetupResult(bool success)
return success ? SetupResult::StopWithSuccess : SetupResult::StopWithError; return success ? SetupResult::StopWithSuccess : SetupResult::StopWithError;
} }
static DoneResult toDoneResult(DoneWith doneWith)
{
return doneWith == DoneWith::Success ? DoneResult::Success : DoneResult::Error;
}
bool TreeStorageBase::isValid() const bool TreeStorageBase::isValid() const
{ {
return m_storageData && m_storageData->m_constructor && m_storageData->m_destructor; return m_storageData && m_storageData->m_constructor && m_storageData->m_destructor;
@@ -1136,7 +1141,7 @@ public:
// in order to unwind properly. // in order to unwind properly.
SetupResult start(); SetupResult start();
void stop(); void stop();
bool invokeDoneHandler(DoneWith result); bool invokeDoneHandler(DoneWith doneWith);
bool isRunning() const { return m_task || m_container.isRunning(); } bool isRunning() const { return m_task || m_container.isRunning(); }
bool isTask() const { return bool(m_taskHandler.m_createHandler); } bool isTask() const { return bool(m_taskHandler.m_createHandler); }
int taskCount() const { return isTask() ? 1 : m_container.m_constData.m_taskCount; } int taskCount() const { return isTask() ? 1 : m_container.m_constData.m_taskCount; }
@@ -1507,13 +1512,13 @@ void TaskNode::stop()
m_task.reset(); m_task.reset();
} }
bool TaskNode::invokeDoneHandler(DoneWith result) bool TaskNode::invokeDoneHandler(DoneWith doneWith)
{ {
bool success = result == DoneWith::Success; DoneResult result = toDoneResult(doneWith);
if (m_taskHandler.m_doneHandler && shouldCall(m_taskHandler.m_callDoneIf, result)) if (m_taskHandler.m_doneHandler && shouldCall(m_taskHandler.m_callDoneIf, doneWith))
success = invokeHandler(parentContainer(), m_taskHandler.m_doneHandler, *m_task.get(), result); result = invokeHandler(parentContainer(), m_taskHandler.m_doneHandler, *m_task.get(), doneWith);
m_container.m_constData.m_taskTreePrivate->advanceProgress(1); m_container.m_constData.m_taskTreePrivate->advanceProgress(1);
return success; return result == DoneResult::Success;
} }
/*! /*!

View File

@@ -190,7 +190,7 @@ public:
// Called prior to task start, just after createHandler // Called prior to task start, just after createHandler
using TaskSetupHandler = std::function<SetupResult(TaskInterface &)>; using TaskSetupHandler = std::function<SetupResult(TaskInterface &)>;
// Called on task done, just before deleteLater // Called on task done, just before deleteLater
using TaskDoneHandler = std::function<bool(const TaskInterface &, DoneWith)>; using TaskDoneHandler = std::function<DoneResult(const TaskInterface &, DoneWith)>;
// Called when group entered, after group's storages are created // Called when group entered, after group's storages are created
using GroupSetupHandler = std::function<SetupResult()>; using GroupSetupHandler = std::function<SetupResult()>;
// Called when group done, before group's storages are deleted // Called when group done, before group's storages are deleted
@@ -423,7 +423,7 @@ public:
"The Adapter type for the CustomTask<Adapter> needs to be derived from " "The Adapter type for the CustomTask<Adapter> needs to be derived from "
"TaskAdapter<Task>."); "TaskAdapter<Task>.");
using SetupFunction = std::function<void(const Task &)>; using SetupFunction = std::function<void(const Task &)>;
using DoneFunction = std::function<bool(const Task &, DoneWith)>; using DoneFunction = std::function<DoneResult(const Task &, DoneWith)>;
static Adapter *createAdapter() { return new Adapter; } static Adapter *createAdapter() { return new Adapter; }
template <typename SetupHandler = SetupFunction, typename DoneHandler = DoneFunction> template <typename SetupHandler = SetupFunction, typename DoneHandler = DoneFunction>
@@ -463,38 +463,38 @@ private:
static GroupItem::TaskDoneHandler wrapDone(Handler &&handler) { static GroupItem::TaskDoneHandler wrapDone(Handler &&handler) {
if constexpr (std::is_same_v<Handler, DoneFunction>) if constexpr (std::is_same_v<Handler, DoneFunction>)
return {}; // When user passed {} for the done handler. return {}; // When user passed {} for the done handler.
// B, V, T, D stands for: [B]ool, [V]oid, [T]ask, [D]oneWith // D, V, T, W stands for: [D]oneResult, [V]oid, [T]ask, done[W]ith
static constexpr bool isBTD = isInvocable<bool, Handler, const Task &, DoneWith>(); static constexpr bool isDTW = isInvocable<DoneResult, Handler, const Task &, DoneWith>();
static constexpr bool isBT = isInvocable<bool, Handler, const Task &>(); static constexpr bool isDT = isInvocable<DoneResult, Handler, const Task &>();
static constexpr bool isBD = isInvocable<bool, Handler, DoneWith>(); static constexpr bool isDW = isInvocable<DoneResult, Handler, DoneWith>();
static constexpr bool isB = isInvocable<bool, Handler>(); static constexpr bool isD = isInvocable<DoneResult, Handler>();
static constexpr bool isVTD = isInvocable<void, Handler, const Task &, DoneWith>(); static constexpr bool isVTW = isInvocable<void, Handler, const Task &, DoneWith>();
static constexpr bool isVT = isInvocable<void, Handler, const Task &>(); static constexpr bool isVT = isInvocable<void, Handler, const Task &>();
static constexpr bool isVD = isInvocable<void, Handler, DoneWith>(); static constexpr bool isVW = isInvocable<void, Handler, DoneWith>();
static constexpr bool isV = isInvocable<void, Handler>(); static constexpr bool isV = isInvocable<void, Handler>();
static_assert(isBTD || isBT || isBD || isB || isVTD || isVT || isVD || isV, static_assert(isDTW || isDT || isDW || isD || isVTW || isVT || isVW || isV,
"Task done handler needs to take (const Task &, DoneWith), (const Task &), " "Task done handler needs to take (const Task &, DoneWith), (const Task &), "
"(DoneWith) or (void) as arguments and has to return void or bool. " "(DoneWith) or (void) as arguments and has to return void or DoneResult. "
"The passed handler doesn't fulfill these requirements."); "The passed handler doesn't fulfill these requirements.");
return [=](const TaskInterface &taskInterface, DoneWith result) { return [=](const TaskInterface &taskInterface, DoneWith result) {
const Adapter &adapter = static_cast<const Adapter &>(taskInterface); const Adapter &adapter = static_cast<const Adapter &>(taskInterface);
if constexpr (isBTD) if constexpr (isDTW)
return std::invoke(handler, *adapter.task(), result); return std::invoke(handler, *adapter.task(), result);
if constexpr (isBT) if constexpr (isDT)
return std::invoke(handler, *adapter.task()); return std::invoke(handler, *adapter.task());
if constexpr (isBD) if constexpr (isDW)
return std::invoke(handler, result); return std::invoke(handler, result);
if constexpr (isB) if constexpr (isD)
return std::invoke(handler); return std::invoke(handler);
if constexpr (isVTD) if constexpr (isVTW)
std::invoke(handler, *adapter.task(), result); std::invoke(handler, *adapter.task(), result);
else if constexpr (isVT) else if constexpr (isVT)
std::invoke(handler, *adapter.task()); std::invoke(handler, *adapter.task());
else if constexpr (isVD) else if constexpr (isVW)
std::invoke(handler, result); std::invoke(handler, result);
else if constexpr (isV) else if constexpr (isV)
std::invoke(handler); std::invoke(handler);
return result == DoneWith::Success; return result == DoneWith::Success ? DoneResult::Success : DoneResult::Error;
}; };
}; };
}; };

View File

@@ -372,32 +372,32 @@ GroupItem createBarrierAdvance(const TreeStorage<CustomStorage> &storage,
}); });
} }
static Handler resultToGroupHandler(DoneWith result) static Handler resultToGroupHandler(DoneWith doneWith)
{ {
switch (result) { switch (doneWith) {
case DoneWith::Success : return Handler::GroupSuccess; case DoneWith::Success: return Handler::GroupSuccess;
case DoneWith::Error : return Handler::GroupError; case DoneWith::Error: return Handler::GroupError;
case DoneWith::Cancel : return Handler::GroupCanceled; case DoneWith::Cancel: return Handler::GroupCanceled;
} }
return Handler::GroupCanceled; return Handler::GroupCanceled;
} }
static Handler setupToTweakHandler(SetupResult result) static Handler toTweakSetupHandler(SetupResult result)
{ {
switch (result) { switch (result) {
case SetupResult::Continue : return Handler::TweakSetupToContinue; case SetupResult::Continue: return Handler::TweakSetupToContinue;
case SetupResult::StopWithSuccess : return Handler::TweakSetupToSuccess; case SetupResult::StopWithSuccess: return Handler::TweakSetupToSuccess;
case SetupResult::StopWithError : return Handler::TweakSetupToError; case SetupResult::StopWithError: return Handler::TweakSetupToError;
} }
return Handler::TweakSetupToContinue; return Handler::TweakSetupToContinue;
} }
static Handler doneToTweakHandler(bool result) static Handler toTweakDoneHandler(bool success)
{ {
return result ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; return success ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError;
} }
static Handler doneToTweakHandler(DoneResult result) static Handler toTweakDoneHandler(DoneResult result)
{ {
return result == DoneResult::Success ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError; return result == DoneResult::Success ? Handler::TweakDoneToSuccess : Handler::TweakDoneToError;
} }
@@ -492,20 +492,21 @@ void tst_Tasking::testTree_data()
}; };
}; };
const auto setupTaskWithTweak = [storage](int taskId, SetupResult desiredResult) { const auto setupTaskWithTweak = [storage](int taskId, SetupResult result) {
return [storage, taskId, desiredResult](TaskObject &) { return [storage, taskId, result](TaskObject &) {
storage->m_log.append({taskId, Handler::Setup}); storage->m_log.append({taskId, Handler::Setup});
storage->m_log.append({taskId, setupToTweakHandler(desiredResult)}); storage->m_log.append({taskId, toTweakSetupHandler(result)});
return desiredResult; return result;
}; };
}; };
const auto setupDone = [storage](int taskId, bool desiredResult = true) { const auto setupDone = [storage](int taskId, DoneResult result = DoneResult::Success) {
return [storage, taskId, desiredResult](DoneWith result) { return [storage, taskId, result](DoneWith doneWith) {
const Handler handler = result == DoneWith::Cancel ? Handler::Canceled const Handler handler = doneWith == DoneWith::Cancel ? Handler::Canceled
: desiredResult ? Handler::Success : Handler::Error; : result == DoneResult::Success ? Handler::Success : Handler::Error;
storage->m_log.append({taskId, handler}); storage->m_log.append({taskId, handler});
return desiredResult && result != DoneWith::Cancel; return doneWith == DoneWith::Cancel ? DoneResult::Error
: result == DoneResult::Success ? DoneResult::Success : DoneResult::Error;
}; };
}; };
@@ -516,16 +517,16 @@ void tst_Tasking::testTree_data()
}; };
const auto createTask = [storage, setupTask, setupDone]( const auto createTask = [storage, setupTask, setupDone](
int taskId, bool successTask, milliseconds timeout = 0ms) { int taskId, DoneResult result, milliseconds timeout = 0ms) {
return TestTask(setupTask(taskId, timeout), setupDone(taskId, successTask)); return TestTask(setupTask(taskId, timeout), setupDone(taskId, result));
}; };
const auto createSuccessTask = [createTask](int taskId, milliseconds timeout = 0ms) { const auto createSuccessTask = [createTask](int taskId, milliseconds timeout = 0ms) {
return createTask(taskId, true, timeout); return createTask(taskId, DoneResult::Success, timeout);
}; };
const auto createFailingTask = [createTask](int taskId, milliseconds timeout = 0ms) { const auto createFailingTask = [createTask](int taskId, milliseconds timeout = 0ms) {
return createTask(taskId, false, timeout); return createTask(taskId, DoneResult::Error, timeout);
}; };
const auto createTaskWithSetupTweak = [storage, setupTaskWithTweak, setupDone]( const auto createTaskWithSetupTweak = [storage, setupTaskWithTweak, setupDone](
@@ -546,14 +547,14 @@ void tst_Tasking::testTree_data()
const auto groupSetupWithTweak = [storage](int taskId, SetupResult desiredResult) { const auto groupSetupWithTweak = [storage](int taskId, SetupResult desiredResult) {
return onGroupSetup([storage, taskId, desiredResult] { return onGroupSetup([storage, taskId, desiredResult] {
storage->m_log.append({taskId, Handler::GroupSetup}); storage->m_log.append({taskId, Handler::GroupSetup});
storage->m_log.append({taskId, setupToTweakHandler(desiredResult)}); storage->m_log.append({taskId, toTweakSetupHandler(desiredResult)});
return desiredResult; return desiredResult;
}); });
}; };
const auto groupDoneWithTweak = [storage](int taskId, bool desiredResult) { const auto groupDoneWithTweak = [storage](int taskId, bool desiredResult) {
return onGroupDone([storage, taskId, desiredResult](DoneWith result) { return onGroupDone([storage, taskId, desiredResult](DoneWith result) {
storage->m_log.append({taskId, resultToGroupHandler(result)}); storage->m_log.append({taskId, resultToGroupHandler(result)});
storage->m_log.append({taskId, doneToTweakHandler(desiredResult)}); storage->m_log.append({taskId, toTweakDoneHandler(desiredResult)});
return desiredResult; return desiredResult;
}); });
}; };
@@ -563,7 +564,7 @@ void tst_Tasking::testTree_data()
const auto createSyncWithTweak = [storage](int taskId, DoneResult result) { const auto createSyncWithTweak = [storage](int taskId, DoneResult result) {
return Sync([storage, taskId, result] { return Sync([storage, taskId, result] {
storage->m_log.append({taskId, Handler::Sync}); storage->m_log.append({taskId, Handler::Sync});
storage->m_log.append({taskId, doneToTweakHandler(result)}); storage->m_log.append({taskId, toTweakDoneHandler(result)});
return result; return result;
}); });
}; };
@@ -1525,22 +1526,22 @@ void tst_Tasking::testTree_data()
} }
{ {
const auto createRoot = [storage, createTask, groupDone]( const auto createRoot = [storage, createTask, groupDone](DoneResult firstResult,
bool firstSuccess, bool secondSuccess) { DoneResult secondResult) {
return Group { return Group {
parallel, parallel,
stopOnFinished, stopOnFinished,
Storage(storage), Storage(storage),
createTask(1, firstSuccess, 1000ms), createTask(1, firstResult, 1000ms),
createTask(2, secondSuccess, 1ms), createTask(2, secondResult, 1ms),
groupDone(0) groupDone(0)
}; };
}; };
const Group root1 = createRoot(true, true); const Group root1 = createRoot(DoneResult::Success, DoneResult::Success);
const Group root2 = createRoot(true, false); const Group root2 = createRoot(DoneResult::Success, DoneResult::Error);
const Group root3 = createRoot(false, true); const Group root3 = createRoot(DoneResult::Error, DoneResult::Success);
const Group root4 = createRoot(false, false); const Group root4 = createRoot(DoneResult::Error, DoneResult::Error);
const Log success { const Log success {
{1, Handler::Setup}, {1, Handler::Setup},
@@ -1610,18 +1611,18 @@ void tst_Tasking::testTree_data()
{ {
// This test checks whether group done handler's result is properly dispatched. // This test checks whether group done handler's result is properly dispatched.
const auto createRoot = [storage, createTask, groupDone, groupDoneWithTweak]( const auto createRoot = [storage, createTask, groupDone, groupDoneWithTweak](
bool successTask, bool desiredResult) { DoneResult firstResult, bool desiredResult) {
return Group { return Group {
Storage(storage), Storage(storage),
Group { Group {
createTask(1, successTask), createTask(1, firstResult),
groupDoneWithTweak(1, desiredResult) groupDoneWithTweak(1, desiredResult)
}, },
groupDone(0) groupDone(0)
}; };
}; };
const Group root1 = createRoot(true, true); const Group root1 = createRoot(DoneResult::Success, true);
const Log log1 { const Log log1 {
{1, Handler::Setup}, {1, Handler::Setup},
{1, Handler::Success}, {1, Handler::Success},
@@ -1632,7 +1633,7 @@ void tst_Tasking::testTree_data()
QTest::newRow("GroupDoneWithSuccessTweakToSuccess") QTest::newRow("GroupDoneWithSuccessTweakToSuccess")
<< TestData{storage, root1, log1, 1, OnDone::Success}; << TestData{storage, root1, log1, 1, OnDone::Success};
const Group root2 = createRoot(true, false); const Group root2 = createRoot(DoneResult::Success, false);
const Log log2 { const Log log2 {
{1, Handler::Setup}, {1, Handler::Setup},
{1, Handler::Success}, {1, Handler::Success},
@@ -1643,7 +1644,7 @@ void tst_Tasking::testTree_data()
QTest::newRow("GroupDoneWithSuccessTweakToError") QTest::newRow("GroupDoneWithSuccessTweakToError")
<< TestData{storage, root2, log2, 1, OnDone::Failure}; << TestData{storage, root2, log2, 1, OnDone::Failure};
const Group root3 = createRoot(false, true); const Group root3 = createRoot(DoneResult::Error, true);
const Log log3 { const Log log3 {
{1, Handler::Setup}, {1, Handler::Setup},
{1, Handler::Error}, {1, Handler::Error},
@@ -1654,7 +1655,7 @@ void tst_Tasking::testTree_data()
QTest::newRow("GroupDoneWithErrorTweakToSuccess") QTest::newRow("GroupDoneWithErrorTweakToSuccess")
<< TestData{storage, root3, log3, 1, OnDone::Success}; << TestData{storage, root3, log3, 1, OnDone::Success};
const Group root4 = createRoot(false, false); const Group root4 = createRoot(DoneResult::Error, false);
const Log log4 { const Log log4 {
{1, Handler::Setup}, {1, Handler::Setup},
{1, Handler::Error}, {1, Handler::Error},