From 7b0cba5b9dab86d38de189091dfc954a4ad0ed8d Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Sat, 28 Apr 2018 17:54:09 +0200 Subject: [PATCH] Allow QSharedPointer as rejection reason Embed the promise fulfillment value and rejection reason in respectively PromiseValue and PromiseError private wrappers, both storing the data in a shared pointer (QPromiseError is now deprecated). --- src/qtpromise/qpromise.h | 1 + src/qtpromise/qpromise_p.h | 105 ++++++++++-------- src/qtpromise/qpromiseerror.h | 54 ++------- tests/auto/qtpromise/qpromise/qpromise.pro | 1 + .../auto/qtpromise/qpromise/reject/reject.pro | 4 + .../qtpromise/qpromise/reject/tst_reject.cpp | 74 ++++++++++++ .../qpromise/resolve/tst_resolve.cpp | 53 ++++++++- 7 files changed, 195 insertions(+), 97 deletions(-) create mode 100644 tests/auto/qtpromise/qpromise/reject/reject.pro create mode 100644 tests/auto/qtpromise/qpromise/reject/tst_reject.cpp diff --git a/src/qtpromise/qpromise.h b/src/qtpromise/qpromise.h index e11b42c..2516689 100644 --- a/src/qtpromise/qpromise.h +++ b/src/qtpromise/qpromise.h @@ -3,6 +3,7 @@ // QtPromise #include "qpromise_p.h" +#include "qpromiseerror.h" #include "qpromiseglobal.h" // Qt diff --git a/src/qtpromise/qpromise_p.h b/src/qtpromise/qpromise_p.h index 7611ef1..6d75d24 100644 --- a/src/qtpromise/qpromise_p.h +++ b/src/qtpromise/qpromise_p.h @@ -2,7 +2,6 @@ #define QTPROMISE_QPROMISE_P_H // QtPromise -#include "qpromiseerror.h" #include "qpromiseglobal.h" // Qt @@ -72,6 +71,43 @@ static void qtpromise_defer(F&& f) qtpromise_defer(std::forward(f), QThread::currentThread()); } +template +class PromiseValue +{ +public: + PromiseValue() { } + PromiseValue(const T& data) : m_data(new T(data)) { } + PromiseValue(T&& data) : m_data(new T(std::move(data))) { } + bool isNull() const { return m_data.isNull(); } + const T& data() const { return *m_data; } + +private: + QSharedPointer m_data; +}; + +class PromiseError +{ +public: + template + PromiseError(const T& value) + { + try { + throw value; + } catch (...) { + m_data = std::current_exception(); + } + } + + PromiseError() { } + PromiseError(const std::exception_ptr& exception) : m_data(exception) { } + void rethrow() const { std::rethrow_exception(m_data); } + bool isNull() const { return m_data == nullptr; } + +private: + // NOTE(SB) std::exception_ptr is already a shared pointer + std::exception_ptr m_data; +}; + template struct PromiseDeduce { @@ -306,12 +342,12 @@ struct PromiseCatcher using ResType = typename std::result_of::type; template - static std::function create( + static std::function create( const THandler& handler, const TResolve& resolve, const TReject& reject) { - return [=](const QtPromise::QPromiseError& error) { + return [=](const PromiseError& error) { try { error.rethrow(); } catch (const TArg& error) { @@ -329,12 +365,12 @@ struct PromiseCatcher using ResType = typename std::result_of::type; template - static std::function create( + static std::function create( const THandler& handler, const TResolve& resolve, const TReject& reject) { - return [=](const QtPromise::QPromiseError& error) { + return [=](const PromiseError& error) { try { error.rethrow(); } catch (...) { @@ -348,12 +384,12 @@ template struct PromiseCatcher { template - static std::function create( + static std::function create( std::nullptr_t, const TResolve&, const TReject& reject) { - return [=](const QtPromise::QPromiseError& error) { + return [=](const PromiseError& error) { // 2.2.7.4. If onRejected is not a function and promise1 is rejected, // promise2 must be rejected with the same reason as promise1 reject(error); @@ -367,9 +403,8 @@ template class PromiseDataBase : public QSharedData { public: - using Error = QtPromise::QPromiseError; using Handler = std::pair, std::function>; - using Catcher = std::pair, std::function>; + using Catcher = std::pair, std::function>; virtual ~PromiseDataBase() {} @@ -395,29 +430,22 @@ public: m_handlers.append({QThread::currentThread(), std::move(handler)}); } - void addCatcher(std::function catcher) + void addCatcher(std::function catcher) { QWriteLocker lock(&m_lock); m_catchers.append({QThread::currentThread(), std::move(catcher)}); } - void reject(Error error) + template + void reject(E&& error) { Q_ASSERT(isPending()); Q_ASSERT(m_error.isNull()); - m_error.reset(new Error(std::move(error))); + m_error = PromiseError(std::forward(error)); setSettled(); } - void reject(const QSharedPointer& error) - { - Q_ASSERT(isPending()); - Q_ASSERT(m_error.isNull()); - m_error = error; - this->setSettled(); - } - - const QSharedPointer& error() const + const PromiseError& error() const { Q_ASSERT(isRejected()); return m_error; @@ -446,13 +474,13 @@ public: return; } - QSharedPointer error = m_error; + PromiseError error(m_error); Q_ASSERT(!error.isNull()); for (const auto& catcher: catchers) { const auto& fn = catcher.second; qtpromise_defer([=]() { - fn(*error); + fn(error); }, catcher.first); } } @@ -473,7 +501,7 @@ private: bool m_settled = false; QVector m_handlers; QVector m_catchers; - QSharedPointer m_error; + PromiseError m_error; }; template @@ -482,31 +510,16 @@ class PromiseData : public PromiseDataBase using Handler = typename PromiseDataBase::Handler; public: - void resolve(T&& value) + template + void resolve(V&& value) { Q_ASSERT(this->isPending()); Q_ASSERT(m_value.isNull()); - m_value.reset(new T(std::move(value))); + m_value = PromiseValue(std::forward(value)); this->setSettled(); } - void resolve(const T& value) - { - Q_ASSERT(this->isPending()); - Q_ASSERT(m_value.isNull()); - m_value.reset(new T(value)); - this->setSettled(); - } - - void resolve(const QSharedPointer& value) - { - Q_ASSERT(this->isPending()); - Q_ASSERT(m_value.isNull()); - m_value = value; - this->setSettled(); - } - - const QSharedPointer& value() const + const PromiseValue& value() const { Q_ASSERT(this->isFulfilled()); return m_value; @@ -514,19 +527,19 @@ public: void notify(const QVector& handlers) Q_DECL_OVERRIDE { - QSharedPointer value(m_value); + PromiseValue value(m_value); Q_ASSERT(!value.isNull()); for (const auto& handler: handlers) { const auto& fn = handler.second; qtpromise_defer([=]() { - fn(*value); + fn(value.data()); }, handler.first); } } private: - QSharedPointer m_value; + PromiseValue m_value; }; template <> diff --git a/src/qtpromise/qpromiseerror.h b/src/qtpromise/qpromiseerror.h index 2392c97..b2ab9a4 100644 --- a/src/qtpromise/qpromiseerror.h +++ b/src/qtpromise/qpromiseerror.h @@ -2,6 +2,7 @@ #define QTPROMISE_QPROMISEERROR_H // QtPromise +#include "qpromise_p.h" #include "qpromiseglobal.h" // Qt @@ -9,53 +10,6 @@ namespace QtPromise { -class QPromiseError -{ -public: - template - QPromiseError(const T& value) - { - try { - throw value; - } catch (...) { - m_exception = std::current_exception(); - } - } - - QPromiseError(const std::exception_ptr& exception) - : m_exception(exception) - { } - - QPromiseError(const QPromiseError& error) - : m_exception(error.m_exception) - { } - - QPromiseError(QPromiseError&& other) - : m_exception(nullptr) - { - swap(other); - } - - QPromiseError& operator=(QPromiseError other) - { - swap(other); - return *this; - } - - void swap(QPromiseError& other) - { - qSwap(m_exception, other.m_exception); - } - - void rethrow() const - { - std::rethrow_exception(m_exception); - } - -private: - std::exception_ptr m_exception; -}; - class QPromiseTimeoutException : public QException { public: @@ -66,6 +20,12 @@ public: } }; +// QPromiseError is provided for backward compatibility and will be +// removed in the next major version: it wasn't intended to be used +// directly and thus should not be part of the public API. +// TODO Remove QPromiseError at version 1.0 +using QPromiseError = QtPromisePrivate::PromiseError; + } // namespace QtPromise #endif // QTPROMISE_QPROMISEERROR_H diff --git a/tests/auto/qtpromise/qpromise/qpromise.pro b/tests/auto/qtpromise/qpromise/qpromise.pro index e0c90bd..814ad7d 100644 --- a/tests/auto/qtpromise/qpromise/qpromise.pro +++ b/tests/auto/qtpromise/qpromise/qpromise.pro @@ -6,6 +6,7 @@ SUBDIRS += \ fail \ finally \ operators \ + reject \ resolve \ tap \ tapfail \ diff --git a/tests/auto/qtpromise/qpromise/reject/reject.pro b/tests/auto/qtpromise/qpromise/reject/reject.pro new file mode 100644 index 0000000..0c1caae --- /dev/null +++ b/tests/auto/qtpromise/qpromise/reject/reject.pro @@ -0,0 +1,4 @@ +TARGET = tst_qpromise_reject +SOURCES += $$PWD/tst_reject.cpp + +include(../../qtpromise.pri) diff --git a/tests/auto/qtpromise/qpromise/reject/tst_reject.cpp b/tests/auto/qtpromise/qpromise/reject/tst_reject.cpp new file mode 100644 index 0000000..f33777e --- /dev/null +++ b/tests/auto/qtpromise/qpromise/reject/tst_reject.cpp @@ -0,0 +1,74 @@ +// Tests +#include "../../shared/utils.h" + +// QtPromise +#include + +// Qt +#include + +// STL +#include + +using namespace QtPromise; + +class tst_qpromise_reject : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void rejectWithValue(); + void rejectWithQSharedPtr(); + void rejectWithStdSharedPtr(); +}; + +QTEST_MAIN(tst_qpromise_reject) +#include "tst_reject.moc" + +void tst_qpromise_reject::rejectWithValue() +{ + auto p = QPromise::reject(42); + + QCOMPARE(p.isRejected(), true); + QCOMPARE(waitForError(p, -1), 42); +} + +// https://github.com/simonbrunel/qtpromise/issues/6 +void tst_qpromise_reject::rejectWithQSharedPtr() +{ + QWeakPointer wptr; + + { + QSharedPointer sptr(new int(42)); + auto p = QPromise::reject(sptr); + + QCOMPARE(waitForError(p, QSharedPointer()), sptr); + + wptr = sptr; + sptr.reset(); + + QCOMPARE(wptr.isNull(), false); // "p" still holds a reference + } + + QCOMPARE(wptr.isNull(), true); +} + +// https://github.com/simonbrunel/qtpromise/issues/6 +void tst_qpromise_reject::rejectWithStdSharedPtr() +{ + std::weak_ptr wptr; + + { + std::shared_ptr sptr(new int(42)); + auto p = QPromise::reject(sptr); + + QCOMPARE(waitForError(p, std::shared_ptr()), sptr); + + wptr = sptr; + sptr.reset(); + + QCOMPARE(wptr.use_count(), 1l); // "p" still holds a reference + } + + QCOMPARE(wptr.use_count(), 0l); +} diff --git a/tests/auto/qtpromise/qpromise/resolve/tst_resolve.cpp b/tests/auto/qtpromise/qpromise/resolve/tst_resolve.cpp index 85991c7..3aec756 100644 --- a/tests/auto/qtpromise/qpromise/resolve/tst_resolve.cpp +++ b/tests/auto/qtpromise/qpromise/resolve/tst_resolve.cpp @@ -7,6 +7,9 @@ // Qt #include +// STL +#include + using namespace QtPromise; class tst_qpromise_resolve : public QObject @@ -14,14 +17,16 @@ class tst_qpromise_resolve : public QObject Q_OBJECT private Q_SLOTS: - void value(); - void empty_void(); + void resolveWithValue(); + void resolveWithNoValue(); + void resolveWithQSharedPtr(); + void resolveWithStdSharedPtr(); }; QTEST_MAIN(tst_qpromise_resolve) #include "tst_resolve.moc" -void tst_qpromise_resolve::value() +void tst_qpromise_resolve::resolveWithValue() { const int value = 42; auto p0 = QPromise::resolve(value); @@ -33,9 +38,49 @@ void tst_qpromise_resolve::value() QCOMPARE(waitForValue(p1, -1), 43); } -void tst_qpromise_resolve::empty_void() +void tst_qpromise_resolve::resolveWithNoValue() { auto p = QPromise::resolve(); QCOMPARE(p.isFulfilled(), true); } + +// https://github.com/simonbrunel/qtpromise/issues/6 +void tst_qpromise_resolve::resolveWithQSharedPtr() +{ + QWeakPointer wptr; + + { + QSharedPointer sptr(new int(42)); + auto p = QPromise>::resolve(sptr); + + QCOMPARE(waitForValue(p, QSharedPointer()), sptr); + + wptr = sptr; + sptr.reset(); + + QCOMPARE(wptr.isNull(), false); // "p" still holds a reference + } + + QCOMPARE(wptr.isNull(), true); +} + +// https://github.com/simonbrunel/qtpromise/issues/6 +void tst_qpromise_resolve::resolveWithStdSharedPtr() +{ + std::weak_ptr wptr; + + { + std::shared_ptr sptr(new int(42)); + auto p = QPromise>::resolve(sptr); + + QCOMPARE(waitForValue(p, std::shared_ptr()), sptr); + + wptr = sptr; + sptr.reset(); + + QCOMPARE(wptr.use_count(), 1l); // "p" still holds a reference + } + + QCOMPARE(wptr.use_count(), 0l); +}