From 4cb50d69d891c854a9162939e1a674f7ce6fbec4 Mon Sep 17 00:00:00 2001
From: Aleix Pol <aleixpol@kde.org>
Date: Wed, 22 Jun 2016 12:40:14 +0200
Subject: [PATCH] Fix some issues in the downloadjob and its test

Test on KJob abstraction, this way we can check if it has an error when it
breaks.
Handle errors in a different branch of the code.
---
 core/backends/lan/downloadjob.cpp | 20 ++++------
 core/backends/lan/downloadjob.h   |  3 +-
 tests/downloadjobtest.cpp         | 63 ++++++++++---------------------
 3 files changed, 29 insertions(+), 57 deletions(-)

diff --git a/core/backends/lan/downloadjob.cpp b/core/backends/lan/downloadjob.cpp
index f23eb06ea..3e086e106 100644
--- a/core/backends/lan/downloadjob.cpp
+++ b/core/backends/lan/downloadjob.cpp
@@ -36,7 +36,7 @@ DownloadJob::DownloadJob(const QHostAddress &address, const QVariantMap &transfe
     : KJob()
     , mAddress(address)
     , mPort(transferInfo["port"].toInt())
-    , mSocket(new QSslSocket)
+    , mSocket(new QSslSocket(this))
 {
     // Setting ssl related properties for socket when using ssl
     mSocket->setLocalCertificate(KdeConnectConfig::instance()->certificate());
@@ -56,24 +56,20 @@ DownloadJob::~DownloadJob()
 void DownloadJob::start()
 {
     //TODO: Timeout?
-    connect(mSocket.data(), &QAbstractSocket::disconnected, this, &DownloadJob::done);
-    connect(mSocket.data(), SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(done()));
-    //connect(mSocket.data(), &QAbstractSocket::connected, [=](){ qDebug() << "Connected"; });
+    connect(mSocket.data(), &QAbstractSocket::disconnected, this, &DownloadJob::emitResult);
+    connect(mSocket.data(), SIGNAL(error(QAbstractSocket::SocketError)), this, SLOT(socketFailed(QAbstractSocket::SocketError)));
+//     connect(mSocket.data(), &QAbstractSocket::stateChanged, [](QAbstractSocket::SocketState state){ qDebug() << "statechange" << state; });
 
     // Cannot use read only, might be due to ssl handshake, getting QIODevice::ReadOnly error and no connection
     mSocket->connectToHostEncrypted(mAddress.toString(), mPort, QIODevice::ReadWrite);
-    mSocket->waitForEncrypted();
-
-    //mSocket->open(QIODevice::ReadOnly);
 }
 
-void DownloadJob::done()
+void DownloadJob::socketFailed(QAbstractSocket::SocketError error)
 {
-    if (mSocket->error()) {
-        qWarning(KDECONNECT_CORE) << mSocket->errorString();
-    }
+    qWarning(KDECONNECT_CORE) << "error..." << mSocket->errorString();
+    setError(error + 1);
+    setErrorText(mSocket->errorString());
     emitResult();
-    deleteLater();
 }
 
 QSharedPointer<QIODevice> DownloadJob::getPayload()
diff --git a/core/backends/lan/downloadjob.h b/core/backends/lan/downloadjob.h
index d73b496be..6100e4176 100644
--- a/core/backends/lan/downloadjob.h
+++ b/core/backends/lan/downloadjob.h
@@ -43,13 +43,12 @@ public:
     QSharedPointer<QIODevice> getPayload();
 
 private:
-    bool useSsl;
     QHostAddress mAddress;
     qint16 mPort;
     QSharedPointer<QSslSocket> mSocket;
 
 private Q_SLOTS:
-    void done();
+    void socketFailed(QAbstractSocket::SocketError error);
 
 };
 
diff --git a/tests/downloadjobtest.cpp b/tests/downloadjobtest.cpp
index 312769b66..55576fce3 100644
--- a/tests/downloadjobtest.cpp
+++ b/tests/downloadjobtest.cpp
@@ -26,6 +26,7 @@
 #include <QTimer>
 #include <QHostAddress>
 #include <KJob>
+#include <QSignalSpy>
 #include <iostream>
 
 class DownloadJobTest : public QObject
@@ -35,70 +36,46 @@ class DownloadJobTest : public QObject
 private Q_SLOTS:
     void failToConnectShouldDestroyTheJob();
     void closingTheConnectionShouldDestroyTheJob();
-private:
 
+private:
     void initServer();
     void initDownloadJob();
     void awaitToBeDestroyedOrTimeOut();
     void stopServer();
 
-    QTimer mTimer;
-    QEventLoop mLoop;
-    DownloadJob* test;
-    QTcpServer *mServer;
+    QPointer<DownloadJob> test;
+    QPointer<QTcpServer> mServer;
 };
 
 void DownloadJobTest::initServer()
 {
+    delete mServer;
     mServer = new QTcpServer(this);
     QVERIFY2(mServer->listen(QHostAddress::LocalHost, 8694), "Failed to create local tcp server");
 }
 
-void DownloadJobTest::stopServer()
-{
-    mServer->close();
-}
-
-void DownloadJobTest::initDownloadJob()
-{
-    QVariantMap transferInfo;
-    transferInfo["port"]= 8694;
-    test = new DownloadJob(QHostAddress::LocalHost, transferInfo);
-    test->start();
-}
-
-void DownloadJobTest::awaitToBeDestroyedOrTimeOut()
-{
-    //Either the job is destroyed
-    connect(test, &QObject::destroyed, &mLoop, &QEventLoop::quit);
-
-    //Or we time out
-    mTimer.setInterval(2000);
-    mTimer.setSingleShot(true);
-    connect(&mTimer, &QTimer::timeout, [this]() {
-        mLoop.quit();
-        QFAIL("Test timed out");
-    });
-    mTimer.start();
-
-    //We wait
-    mLoop.exec();
-
-    mTimer.stop();
-}
-
 void DownloadJobTest::failToConnectShouldDestroyTheJob()
 {
-    initDownloadJob();
-    awaitToBeDestroyedOrTimeOut();
+    // no initServer
+    test = new DownloadJob(QHostAddress::LocalHost, {{"port", 8694}});
+
+    QSignalSpy spy(test, &KJob::finished);
+    test->start();
+
+    QVERIFY(spy.count() || spy.wait());
+
+    QCOMPARE(test->error(), 1);
 }
 
 void DownloadJobTest::closingTheConnectionShouldDestroyTheJob()
 {
     initServer();
-    initDownloadJob();
-    stopServer();
-    awaitToBeDestroyedOrTimeOut();
+    test = new DownloadJob(QHostAddress::LocalHost, {{"port", 8694}});
+    QSignalSpy spy(test, &KJob::finished);
+    test->start();
+    mServer->close();
+
+    QVERIFY(spy.count() || spy.wait(2000));
 }
 
 QTEST_GUILESS_MAIN(DownloadJobTest)