Merge pull request #5 from KitsuneRal/basejob-improvement

Basejob improvement

7 files changed, 150 insertions, 112 deletions

diff --git a/connection.cpp b/connection.cpp
index e04de7aa..a219f1da 100644
--- a/connection.cpp
+++ b/connection.cpp
@@ -62,7 +62,13 @@ void Connection::resolveServer(QString domain)
 void Connection::connectToServer(QString user, QString password)
 {
     PasswordLogin* loginJob = new PasswordLogin(d->data, user, password);
-    connect( loginJob, &PasswordLogin::result, d, &ConnectionPrivate::connectDone );
+    connect( loginJob, &PasswordLogin::success, [=] () {
+        qDebug() << "Our user ID: " << loginJob->id();
+        connectWithToken(loginJob->id(), loginJob->token());
+    });
+    connect( loginJob, &PasswordLogin::failure, [=] () {
+        emit loginError(loginJob->errorString());
+    });
     loginJob->start();
     d->username = user; // to be able to reconnect
     d->password = password;
@@ -79,7 +85,14 @@ void Connection::connectWithToken(QString userId, QString token)
 void Connection::reconnect()
 {
     PasswordLogin* loginJob = new PasswordLogin(d->data, d->username, d->password );
-    connect( loginJob, &PasswordLogin::result, d, &ConnectionPrivate::reconnectDone );
+    connect( loginJob, &PasswordLogin::success, [=] () {
+        d->userId = loginJob->id();
+        emit reconnected();
+    });
+    connect( loginJob, &PasswordLogin::failure, [=] () {
+        emit loginError(loginJob->errorString());
+        d->isConnected = false;
+    });
     loginJob->start();
 }
 
@@ -89,7 +102,13 @@ SyncJob* Connection::sync(int timeout)
     SyncJob* syncJob = new SyncJob(d->data, d->data->lastEvent());
     syncJob->setFilter(filter);
     syncJob->setTimeout(timeout);
-    connect( syncJob, &SyncJob::result, d, &ConnectionPrivate::syncDone );
+    connect( syncJob, &SyncJob::success, [=] () {
+        d->data->setLastEvent(syncJob->nextBatch());
+        d->processRooms(syncJob->roomData());
+        emit syncDone();
+    });
+    connect( syncJob, &SyncJob::failure,
+             [=] () { emit connectionError(syncJob->errorString());});
     syncJob->start();
     return syncJob;
 }
@@ -110,7 +129,10 @@ PostReceiptJob* Connection::postReceipt(Room* room, Event* event)
 void Connection::joinRoom(QString roomAlias)
 {
     JoinRoomJob* job = new JoinRoomJob(d->data, roomAlias);
-    connect( job, &JoinRoomJob::result, d, &ConnectionPrivate::gotJoinRoom );
+    connect( job, &SyncJob::success, [=] () {
+        if ( Room* r = d->provideRoom(job->roomId()) )
+            emit joinedRoom(r);
+    });
     job->start();
 }
 
diff --git a/connectionprivate.cpp b/connectionprivate.cpp
index 851e3d58..bcca8744 100644
--- a/connectionprivate.cpp
+++ b/connectionprivate.cpp
@@ -116,67 +116,67 @@ Room* ConnectionPrivate::provideRoom(QString id)
     return room;
 }
 
-void ConnectionPrivate::connectDone(KJob* job)
-{
-    PasswordLogin* realJob = static_cast<PasswordLogin*>(job);
-    if( !realJob->error() )
-    {
-        isConnected = true;
-        userId = realJob->id();
-        qDebug() << "Our user ID: " << userId;
-        emit q->connected();
-    }
-    else {
-        emit q->loginError( job->errorString() );
-    }
-}
-
-void ConnectionPrivate::reconnectDone(KJob* job)
-{
-    PasswordLogin* realJob = static_cast<PasswordLogin*>(job);
-    if( !realJob->error() )
-    {
-        userId = realJob->id();
-        emit q->reconnected();
-    }
-    else {
-        emit q->loginError( job->errorString() );
-        isConnected = false;
-    }
-}
-
-void ConnectionPrivate::syncDone(KJob* job)
-{
-    SyncJob* syncJob = static_cast<SyncJob*>(job);
-    if( !syncJob->error() )
-    {
-        data->setLastEvent(syncJob->nextBatch());
-        processRooms(syncJob->roomData());
-        emit q->syncDone();
-    }
-    else {
-        if( syncJob->error() == BaseJob::NetworkError )
-            emit q->connectionError( syncJob->errorString() );
-        else
-            qDebug() << "syncJob failed, error:" << syncJob->error();
-    }
-}
-
-void ConnectionPrivate::gotJoinRoom(KJob* job)
-{
-    qDebug() << "gotJoinRoom";
-    JoinRoomJob* joinJob = static_cast<JoinRoomJob*>(job);
-    if( !joinJob->error() )
-    {
-        if ( Room* r = provideRoom(joinJob->roomId()) )
-            emit q->joinedRoom(r);
-    }
-    else
-    {
-        if( joinJob->error() == BaseJob::NetworkError )
-            emit q->connectionError( joinJob->errorString() );
-    }
-}
+//void ConnectionPrivate::connectDone(KJob* job)
+//{
+//    PasswordLogin* realJob = static_cast<PasswordLogin*>(job);
+//    if( !realJob->error() )
+//    {
+//        isConnected = true;
+//        userId = realJob->id();
+//        qDebug() << "Our user ID: " << userId;
+//        emit q->connected();
+//    }
+//    else {
+//        emit q->loginError( job->errorString() );
+//    }
+//}
+
+//void ConnectionPrivate::reconnectDone(KJob* job)
+//{
+//    PasswordLogin* realJob = static_cast<PasswordLogin*>(job);
+//    if( !realJob->error() )
+//    {
+//        userId = realJob->id();
+//        emit q->reconnected();
+//    }
+//    else {
+//        emit q->loginError( job->errorString() );
+//        isConnected = false;
+//    }
+//}
+
+//void ConnectionPrivate::syncDone(KJob* job)
+//{
+//    SyncJob* syncJob = static_cast<SyncJob*>(job);
+//    if( !syncJob->error() )
+//    {
+//        data->setLastEvent(syncJob->nextBatch());
+//        processRooms(syncJob->roomData());
+//        emit q->syncDone();
+//    }
+//    else {
+//        if( syncJob->error() == BaseJob::NetworkError )
+//            emit q->connectionError( syncJob->errorString() );
+//        else
+//            qDebug() << "syncJob failed, error:" << syncJob->error();
+//    }
+//}
+
+//void ConnectionPrivate::gotJoinRoom(KJob* job)
+//{
+//    qDebug() << "gotJoinRoom";
+//    JoinRoomJob* joinJob = static_cast<JoinRoomJob*>(job);
+//    if( !joinJob->error() )
+//    {
+//        if ( Room* r = provideRoom(joinJob->roomId()) )
+//            emit q->joinedRoom(r);
+//    }
+//    else
+//    {
+//        if( joinJob->error() == BaseJob::NetworkError )
+//            emit q->connectionError( joinJob->errorString() );
+//    }
+//}
 
 void ConnectionPrivate::gotRoomMembers(KJob* job)
 {
diff --git a/connectionprivate.h b/connectionprivate.h
index c538cef5..8e37a934 100644
--- a/connectionprivate.h
+++ b/connectionprivate.h
@@ -60,10 +60,10 @@ namespace QMatrixClient
             QString userId;
 
         public slots:
-            void connectDone(KJob* job);
-            void reconnectDone(KJob* job);
-            void syncDone(KJob* job);
-            void gotJoinRoom(KJob* job);
+//            void connectDone(KJob* job);
+//            void reconnectDone(KJob* job);
+//            void syncDone(KJob* job);
+//            void gotJoinRoom(KJob* job);
             void gotRoomMembers(KJob* job);
     };
 }
diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp
index 519e1517..cfdf8a28 100644
--- a/jobs/basejob.cpp
+++ b/jobs/basejob.cpp
@@ -42,12 +42,23 @@ class BaseJob::Private
 BaseJob::BaseJob(ConnectionData* connection, JobHttpType type, bool needsToken)
     : d(new Private(connection, type, needsToken))
 {
+    // Work around KJob inability to separate success and failure signals
+    connect(this, &BaseJob::result, [this]() {
+        if (error() == NoError)
+            emit success(this);
+        else
+            emit failure(this);
+    });
 }
 
 BaseJob::~BaseJob()
 {
     if( d->reply )
+    {
+        if( d->reply->isRunning() )
+            d->reply->abort();
         d->reply->deleteLater();
+    }
     delete d;
 }
 
@@ -108,6 +119,9 @@ void BaseJob::fail(int errorCode, QString errorString)
 {
     setError( errorCode );
     setErrorText( errorString );
+    if( d->reply->isRunning() )
+        d->reply->abort();
+    qWarning() << this << "failed:" << errorString;
     emitResult();
 }
 
@@ -125,7 +139,7 @@ void BaseJob::gotReply()
 {
     if( d->reply->error() != QNetworkReply::NoError )
     {
-        qDebug() << "NetworkError!!!" << d->reply->error();
+        qDebug() << "NetworkError:" << d->reply->error();
         fail( NetworkError, d->reply->errorString() );
         return;
     }
@@ -142,8 +156,7 @@ void BaseJob::gotReply()
 void BaseJob::timeout()
 {
     qDebug() << "Timeout!";
-    if( d->reply->isRunning() )
-        d->reply->abort();
+    fail( TimeoutError, "The job has timed out" );
 }
 
 void BaseJob::sslErrors(const QList<QSslError>& errors)
diff --git a/jobs/basejob.h b/jobs/basejob.h
index 95cf4232..f1ad66d1 100644
--- a/jobs/basejob.h
+++ b/jobs/basejob.h
@@ -45,8 +45,20 @@ namespace QMatrixClient
 
             void start() override;
 
-            enum ErrorCode { NetworkError = KJob::UserDefinedError, JsonParseError, UserDefinedError };
-            
+            enum ErrorCode { NetworkError = KJob::UserDefinedError,
+                             JsonParseError, TimeoutError, UserDefinedError };
+
+        signals:
+            /**
+             * Emitted together with KJob::result() but only if there's no error.
+             */
+            void success(BaseJob*);
+            /**
+             * Emitted together with KJob::result() if there's an error.
+             * Same as result(), this won't be emitted in case of kill(Quietly).
+             */
+            void failure(BaseJob*);
+
         protected:
             ConnectionData* connection() const;
 
diff --git a/room.cpp b/room.cpp
index ac4275af..81b64c54 100644
--- a/room.cpp
+++ b/room.cpp
@@ -73,7 +73,7 @@ class Room::Private: public QObject
         QList<User*> membersLeft;
         QHash<User*, QString> lastReadEvent;
         QString prevBatch;
-        bool gettingNewContent;
+        RoomMessagesJob* roomMessagesJob;
         
         // Convenience methods to work with the membersMap and usersLeft. addMember()
         // and removeMember() emit respective Room:: signals after a succesful
@@ -86,6 +86,8 @@ class Room::Private: public QObject
         void renameMember(User* u, QString oldName);
         void removeMember(User* u);
 
+        void getPreviousContent();
+
     private:
         QString calculateDisplayname() const;
         QString roomNameFromMemberNames(const QList<User*>& userlist) const;
@@ -100,7 +102,7 @@ Room::Room(Connection* connection, QString id)
     d->id = id;
     d->connection = connection;
     d->joinState = JoinState::Join;
-    d->gettingNewContent = false;
+    d->roomMessagesJob = nullptr;
     qDebug() << "New Room:" << id;
 
     //connection->getMembers(this); // I don't think we need this anymore in r0.0.1
@@ -373,31 +375,27 @@ void Room::updateData(const SyncRoomData& data)
 
 void Room::getPreviousContent()
 {
-    if( !d->gettingNewContent )
-    {
-        d->gettingNewContent = true;
-        RoomMessagesJob* job = d->connection->getMessages(this, d->prevBatch);
-        connect( job, &RoomMessagesJob::result, d, &Room::Private::gotMessages );
-    }
+    d->getPreviousContent();
 }
 
-void Room::Private::gotMessages(KJob* job)
+void Room::Private::getPreviousContent()
 {
-    RoomMessagesJob* realJob = static_cast<RoomMessagesJob*>(job);
-    if( realJob->error() )
-    {
-        qDebug() << realJob->errorString();
-    }
-    else
+    if( !roomMessagesJob )
     {
-        for( Event* event: realJob->events() )
-        {
-            q->processMessageEvent(event);
-            emit q->newMessage(event);
-        }
-        prevBatch = realJob->end();
+        roomMessagesJob = connection->getMessages(q, prevBatch);
+        connect( roomMessagesJob, &RoomMessagesJob::result, [=]() {
+            if( !roomMessagesJob->error() )
+            {
+                for( Event* event: roomMessagesJob->events() )
+                {
+                    q->processMessageEvent(event);
+                    emit q->newMessage(event);
+                }
+                prevBatch = roomMessagesJob->end();
+            }
+            roomMessagesJob = nullptr;
+        });
     }
-    gettingNewContent = false;
 }
 
 Connection* Room::connection()
diff --git a/user.cpp b/user.cpp
index f9529db3..5018fe83 100644
--- a/user.cpp
+++ b/user.cpp
@@ -28,7 +28,7 @@
 
 using namespace QMatrixClient;
 
-class User::Private: public QObject
+class User::Private
 {
     public:
         User* q;
@@ -45,8 +45,6 @@ class User::Private: public QObject
         QHash<QPair<int,int>,QPixmap> scaledMap;
 
         void requestAvatar();
-    public slots:
-        void gotAvatar(KJob* job);
 };
 
 User::User(QString userId, Connection* connection)
@@ -135,17 +133,12 @@ void User::Private::requestAvatar()
 {
     MediaThumbnailJob* job =
             connection->getThumbnail(avatarUrl, requestedWidth, requestedHeight);
-    connect( job, &MediaThumbnailJob::result, this, &User::Private::gotAvatar );
-}
-
-void User::Private::gotAvatar(KJob* job)
-{
-    avatarOngoingRequest = false;
-    avatarValid = true;
-    avatar =
-        static_cast<MediaThumbnailJob*>(job)->thumbnail()
-            .scaled(requestedWidth, requestedHeight,
-                    Qt::KeepAspectRatio, Qt::SmoothTransformation);
-    scaledMap.clear();
-    emit q->avatarChanged(q);
+    connect( job, &MediaThumbnailJob::success, [=]() {
+        avatarOngoingRequest = false;
+        avatarValid = true;
+        avatar = job->thumbnail().scaled(requestedWidth, requestedHeight,
+                        Qt::KeepAspectRatio, Qt::SmoothTransformation);
+        scaledMap.clear();
+        emit q->avatarChanged(q);
+    });
 }