Add a test for resuming downloads and extract the <message> for the error message

src/libsync/propagatedownload.cpp

@@ -589,7 +589,7 @@ void PropagateDownloadFile::slotGetFinished()
                 &propagator()->_anotherSyncNeeded);
         }
 
-        done(status, job->errorString());
+        done(status,_item->_httpErrorCode >= 400 ? job->errorStringParsingBody() : job->errorString());
         return;
     }
 

test/CMakeLists.txt

@@ -45,6 +45,7 @@ owncloud_add_test(SyncEngine "syncenginetestutils.h")
 owncloud_add_test(SyncMove "syncenginetestutils.h")
 owncloud_add_test(SyncConflict "syncenginetestutils.h")
 owncloud_add_test(SyncFileStatusTracker "syncenginetestutils.h")
+owncloud_add_test(Download "syncenginetestutils.h")
 owncloud_add_test(ChunkingNg "syncenginetestutils.h")
 owncloud_add_test(UploadReset "syncenginetestutils.h")
 owncloud_add_test(AllFilesDeleted "syncenginetestutils.h")

test/syncenginetestutils.h

@@ -715,8 +715,8 @@ class FakeErrorReply : public QNetworkReply
     Q_OBJECT
 public:
     FakeErrorReply(QNetworkAccessManager::Operation op, const QNetworkRequest &request,
-                   QObject *parent, int httpErrorCode)
-    : QNetworkReply{parent}, _httpErrorCode(httpErrorCode) {
+                   QObject *parent, int httpErrorCode, const QByteArray &body = QByteArray())
+    : QNetworkReply{parent}, _httpErrorCode(httpErrorCode), _body(body) {
         setRequest(request);
         setUrl(request.url());
         setOperation(op);
@@ -732,9 +732,15 @@ class FakeErrorReply : public QNetworkReply
     }
 
     void abort() override { }
-    qint64 readData(char *, qint64) override { return 0; }
+    qint64 readData(char *buf, qint64 max) override {
+        max = qMin<qint64>(max, _body.size());
+        memcpy(buf, _body.constData(), max);
+        _body = _body.mid(max);
+        return max;
+    }
 
     int _httpErrorCode;
+    QByteArray _body;
 };
 
 // A reply that never responds

test/testdownload.cpp

@@ -0,0 +1,122 @@
+/*
+ *    This software is in the public domain, furnished "as is", without technical
+ *    support, and with no warranty, express or implied, as to its usefulness for
+ *    any purpose.
+ *
+ */
+
+#include <QtTest>
+#include "syncenginetestutils.h"
+#include <syncengine.h>
+
+using namespace OCC;
+
+static constexpr quint64 stopAfter = 3'123'668;
+
+/* A FakeGetReply that sends max 'fakeSize' bytes, but whose ContentLength has the corect size */
+class BrokenFakeGetReply : public FakeGetReply
+{
+    Q_OBJECT
+public:
+    using FakeGetReply::FakeGetReply;
+    int fakeSize = stopAfter;
+
+    qint64 bytesAvailable() const override
+    {
+        if (aborted)
+            return 0;
+        return std::min(size, fakeSize) + QIODevice::bytesAvailable();
+    }
+
+    qint64 readData(char *data, qint64 maxlen) override
+    {
+        qint64 len = std::min(qint64{ fakeSize }, maxlen);
+        std::fill_n(data, len, payload);
+        size -= len;
+        fakeSize -= len;
+        return len;
+    }
+};
+
+
+SyncFileItemPtr getItem(const QSignalSpy &spy, const QString &path)
+{
+    for (const QList<QVariant> &args : spy) {
+        auto item = args[0].value<SyncFileItemPtr>();
+        if (item->destination() == path)
+            return item;
+    }
+    return {};
+}
+
+
+class TestDownload : public QObject
+{
+    Q_OBJECT
+
+private slots:
+
+    void testResume()
+    {
+        FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+        auto size = 30 * 1000 * 1000;
+        fakeFolder.remoteModifier().insert("A/a0", size);
+
+        // First, download only the first 3 MB of the file
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * {
+            if (op == QNetworkAccessManager::GetOperation && request.url().path().endsWith("A/a0")) {
+                return new BrokenFakeGetReply(fakeFolder.remoteModifier(), op, request, this);
+            }
+            return nullptr;
+        });
+
+        QVERIFY(!fakeFolder.syncOnce()); // The sync must fail because not all the file was downloaded
+        QCOMPARE(getItem(completeSpy, "A/a0")->_status, SyncFileItem::SoftError);
+        QCOMPARE(getItem(completeSpy, "A/a0")->_errorString, QString("The file could not be downloaded completely."));
+        QVERIFY(fakeFolder.syncEngine().isAnotherSyncNeeded());
+
+        // Now, we need to restart, this time, it should resume.
+        QByteArray ranges;
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * {
+            if (op == QNetworkAccessManager::GetOperation && request.url().path().endsWith("A/a0")) {
+                ranges = request.rawHeader("Range");
+            }
+            return nullptr;
+        });
+        QVERIFY(fakeFolder.syncOnce()); // now this succeeds
+        QCOMPARE(ranges, QByteArray("bytes=" + QByteArray::number(stopAfter) + "-"));
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
+    void testErrorMessage () {
+        // This test's main goal is to test that the error string from the server is shown in the UI
+
+        FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
+        auto size = 3'500'000;
+        fakeFolder.remoteModifier().insert("A/broken", size);
+
+        QByteArray serverMessage = "The file was not downloaded because the tests wants so!";
+
+        // First, download only the first 3 MB of the file
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * {
+            if (op == QNetworkAccessManager::GetOperation && request.url().path().endsWith("A/broken")) {
+                return new FakeErrorReply(op, request, this, 400,
+                    "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+                    "<d:error xmlns:d=\"DAV:\" xmlns:s=\"http://sabredav.org/ns\">\n"
+                    "<s:exception>Sabre\\DAV\\Exception\\Forbidden</s:exception>\n"
+                    "<s:message>"+serverMessage+"</s:message>\n"
+                    "</d:error>");
+            }
+            return nullptr;
+        });
+
+        QVERIFY(!fakeFolder.syncOnce());  // Fail because A/broken
+        QCOMPARE(getItem(completeSpy, "A/broken")->_status, SyncFileItem::NormalError);
+        QVERIFY(getItem(completeSpy, "A/broken")->_errorString.contains(serverMessage));
+    }
+};
+
+QTEST_GUILESS_MAIN(TestDownload)
+#include "testdownload.moc"