[Telepathy-commits] [telepathy-qt4/master] PendingOperation: adjust API as per Olli's review

Simon McVittie simon.mcvittie at collabora.co.uk
Wed Dec 3 03:27:54 PST 2008


* parent can be any QObject, although it will often be a DBusProxy
* replace setError() with setFinishedWithError() (which is called instead
  of setFinished() rather than in addition)
* delay emission of finished() to guarantee that it takes at least one
  main loop invocation in all cases
* warn if the parent is destroyed before the pending operation finishes
---
 TelepathyQt4/cli-connection.cpp        |    6 +-
 TelepathyQt4/cli-pending-operation.cpp |   83 +++++++++++++++++++------------
 TelepathyQt4/cli-pending-operation.h   |   63 ++++++++++++++++++------
 3 files changed, 101 insertions(+), 51 deletions(-)

diff --git a/TelepathyQt4/cli-connection.cpp b/TelepathyQt4/cli-connection.cpp
index 7ae50b2..d322117 100644
--- a/TelepathyQt4/cli-connection.cpp
+++ b/TelepathyQt4/cli-connection.cpp
@@ -499,7 +499,7 @@ PendingChannel::~PendingChannel()
 
 Connection* PendingChannel::connection() const
 {
-    return qobject_cast<Connection*>(proxy());
+    return qobject_cast<Connection*>(parent());
 }
 
 const QString& PendingChannel::channelType() const
@@ -539,14 +539,14 @@ void PendingChannel::onCallFinished(QDBusPendingCallWatcher* watcher)
     QDBusPendingReply<QDBusObjectPath> reply = *watcher;
 
     debug() << "Received reply to RequestChannel";
-    setFinished();
 
     if (!reply.isError()) {
         debug() << " Success: object path" << reply.value().path();
         mPriv->objectPath = reply.value();
+        setFinished();
     } else {
         debug().nospace() << " Failure: error " << reply.error().name() << ": " << reply.error().message();
-        setError(reply.error());
+        setFinishedWithError(reply.error());
     }
 
     debug() << " Emitting finished()";
diff --git a/TelepathyQt4/cli-pending-operation.cpp b/TelepathyQt4/cli-pending-operation.cpp
index 181c2fe..b834939 100644
--- a/TelepathyQt4/cli-pending-operation.cpp
+++ b/TelepathyQt4/cli-pending-operation.cpp
@@ -21,6 +21,8 @@
 
 #include "cli-pending-operation.h"
 
+#include <QTimer>
+
 #include <QDBusPendingCall>
 #include <QDBusPendingCallWatcher>
 
@@ -47,12 +49,12 @@ struct PendingOperation::Private
 };
 
 
-PendingOperation::PendingOperation(DBusProxy* proxy)
-  : QObject(proxy),
+PendingOperation::PendingOperation(QObject* parent)
+  : QObject(parent),
     mPriv(new Private())
 {
-    connect(this, SIGNAL(destroyed(QObject*)),
-        this, SLOT(selfDestroyed(QObject*)));
+    connect(parent, SIGNAL(destroyed(QObject*)),
+        this, SLOT(parentDestroyed(QObject*)));
 }
 
 
@@ -62,36 +64,59 @@ PendingOperation::~PendingOperation()
 }
 
 
-void PendingOperation::setFinished()
+void PendingOperation::emitFinished()
 {
-    mPriv->finished = true;
+    Q_ASSERT(mPriv->finished);
+    emit finished(this);
+    deleteLater();
 }
 
 
-void PendingOperation::setError(const QString& name, const QString& message)
+void PendingOperation::setFinished()
 {
-    Q_ASSERT(!name.isEmpty());
-    mPriv->errorName = name;
-    mPriv->errorMessage = message;
+    if (mPriv->finished) {
+        qWarning() << this << "already finished";
+        return;
+    }
+
+    mPriv->finished = true;
+    Q_ASSERT(isValid());
+    QTimer::singleShot(0, this, SLOT(emitFinished()));
 }
 
 
-void PendingOperation::setError(const QDBusError& error)
+void PendingOperation::setFinishedWithError(const QString& name,
+        const QString& message)
 {
-    setError(error.name(), error.message());
+    if (mPriv->finished) {
+        qWarning() << this << "already finished";
+        return;
+    }
+
+    if (name.isEmpty()) {
+        qWarning() << this << "should be given a non-empty error name";
+        mPriv->errorName = "org.freedesktop.Telepathy.Qt4.ErrorHandlingError";
+    }
+    else {
+        mPriv->errorName = name;
+    }
+
+    mPriv->errorMessage = message;
+    mPriv->finished = true;
+    Q_ASSERT(isError());
+    QTimer::singleShot(0, this, SLOT(emitFinished()));
 }
 
 
-DBusProxy* PendingOperation::proxy() const
+void PendingOperation::setFinishedWithError(const QDBusError& error)
 {
-    return QObject::parent();
+    setFinishedWithError(error.name(), error.message());
 }
 
 
 bool PendingOperation::isValid() const
 {
-    Q_ASSERT(mPriv->finished);
-    return (mPriv->errorName.isEmpty());
+    return (mPriv->finished && mPriv->errorName.isEmpty());
 }
 
 
@@ -103,33 +128,33 @@ bool PendingOperation::isFinished() const
 
 bool PendingOperation::isError() const
 {
-    Q_ASSERT(mPriv->finished);
-    return (!mPriv->errorName.isEmpty());
+    return (mPriv->finished && !mPriv->errorName.isEmpty());
 }
 
 
 QString PendingOperation::errorName() const
 {
-    Q_ASSERT(mPriv->finished);
     return mPriv->errorName;
 }
 
 
 QString PendingOperation::errorMessage() const
 {
-    Q_ASSERT(mPriv->finished);
     return mPriv->errorMessage;
 }
 
 
-void PendingOperation::selfDestroyed(QObject* self)
+void PendingOperation::parentDestroyed(QObject* parent)
 {
-    // FIXME: signal finished with a synthetic error here?
-    // need to work out exactly what the life-cycle is first
+    if (!mPriv->finished) {
+        qWarning() << parent
+                << "still pending when its parent was deleted - finished will "
+                   "never be emitted";
+    }
 }
 
 
-PendingVoidMethodCall::PendingVoidMethodCall(DBusProxy* proxy,
+PendingVoidMethodCall::PendingVoidMethodCall(QObject* proxy,
     QDBusPendingCall call)
   : PendingOperation(proxy),
     mPriv(0)
@@ -143,20 +168,14 @@ PendingVoidMethodCall::PendingVoidMethodCall(DBusProxy* proxy,
 
 void PendingVoidMethodCall::watcherFinished(QDBusPendingCallWatcher* watcher)
 {
-    setFinished();
-
     if (watcher->isError())
     {
-        setError(watcher->error());
-        Q_ASSERT(isError());
+        setFinishedWithError(watcher->error());
     }
     else
     {
-        Q_ASSERT(isValid());
+        setFinished();
     }
-
-    emit finished(this);
-    deleteLater();
 }
 
 
diff --git a/TelepathyQt4/cli-pending-operation.h b/TelepathyQt4/cli-pending-operation.h
index b370841..6c70aa7 100644
--- a/TelepathyQt4/cli-pending-operation.h
+++ b/TelepathyQt4/cli-pending-operation.h
@@ -33,9 +33,6 @@ namespace Telepathy
 namespace Client
 {
 
-// TODO: remove this when we have the DBusProxy class
-typedef QObject DBusProxy;
-
 /**
  * Abstract base class for pending asynchronous operations.
  *
@@ -67,14 +64,6 @@ public:
     virtual ~PendingOperation();
 
     /**
-     * Returns the object through which the pending operation was requested
-     * (a %Connection, %Channel etc.)
-     *
-     * \return A pointer to a D-Bus proxy object
-     */
-    DBusProxy* proxy() const;
-
-    /**
      * Returns whether or not the request has finished processing. #finished()
      * is emitted when this changes from <code>false</code> to
      * <code>true</code>.
@@ -142,13 +131,39 @@ Q_SIGNALS:
     void finished(Telepathy::Client::PendingOperation* operation);
 
 protected:
-    PendingOperation(DBusProxy* proxy);
+    /**
+     * Protected constructor. Only subclasses of this class may be constructed
+     *
+     * \param parent The object on which this pending operation takes place
+     */
+    PendingOperation(QObject* parent);
+
+    /**
+     * Record that this pending operation has finished successfully, and
+     * emit the #finished() signal next time the event loop runs.
+     */
     void setFinished();
-    void setError(const QString& name, const QString& message);
-    void setError(const QDBusError& error);
+
+    /**
+     * Record that this pending operation has finished with an error, and
+     * emit the #finished() signal next time the event loop runs.
+     *
+     * \param name A D-Bus error name, which must be non-empty
+     * \param message A debugging message
+     */
+    void setFinishedWithError(const QString& name, const QString& message);
+
+    /**
+     * Record that this pending operation has finished with an error, and
+     * emit the #finished() signal next time the event loop runs.
+     *
+     * \param error A QtDBus error
+     */
+    void setFinishedWithError(const QDBusError& error);
 
 private Q_SLOTS:
-    void selfDestroyed(QObject* self);
+    void parentDestroyed(QObject* parent);
+    void emitFinished();
 
 private:
     struct Private;
@@ -156,12 +171,28 @@ private:
 };
 
 
+/**
+ * Generic subclass of %PendingOperation representing a pending D-Bus method
+ * call that does not return anything (or returns a result that is not
+ * interesting).
+ *
+ * Objects of this class indicate the success or failure of the method call,
+ * but if the method call succeeds, no additional information is available.
+ */
 class PendingVoidMethodCall : public PendingOperation
 {
     Q_OBJECT
 
 public:
-    PendingVoidMethodCall(DBusProxy* proxy, QDBusPendingCall call);
+    /**
+     * Constructor.
+     *
+     * \param parent The object on which this pending operation takes place
+     * \param call A pending call as returned by the auto-generated low level
+     *             Telepathy API; if the method returns anything, the return
+     *             value(s) will be ignored
+     */
+    PendingVoidMethodCall(QObject* parent, QDBusPendingCall call);
 
 private Q_SLOTS:
     void watcherFinished(QDBusPendingCallWatcher*);
-- 
1.5.6.5




More information about the Telepathy-commits mailing list