ChunkingNG: remove stale chunks when cleaning the uploadInfo table

Stale chunks might be there because a file was removed or would just not
be uploaded, for any reason.
We just start the DeleteJob but we don't care if it success or not.

Relates to owncloud/core#26981

One of the test is testing the case where the file is modified on the server
during the upload. So this test the precondition failed error.
The FakeGetReply logic was modified because resizing a 150MB big QByteArray
by increment of 16k just did not scale when downloading a big file.

src/libsync/syncengine.cpp

@@ -23,6 +23,7 @@
 #include "syncfilestatus.h"
 #include "csync_private.h"
 #include "filesystem.h"
+#include "propagateremotedelete.h"
 
 #ifdef Q_OS_WIN
 #include <windows.h>
@@ -301,7 +302,16 @@ void SyncEngine::deleteStaleUploadInfos()
     }
 
     // Delete from journal.
-    _journal->deleteStaleUploadInfos(upload_file_paths);
+    auto ids = _journal->deleteStaleUploadInfos(upload_file_paths);
+
+    // Delete the stales chunk on the server.
+    if (account()->capabilities().chunkingNg()) {
+        foreach (uint transferId, ids) {
+            QUrl url = Utility::concatUrlPath(account()->url(), QLatin1String("remote.php/dav/uploads/")
+                + account()->davUser() + QLatin1Char('/') + QString::number(transferId));
+            (new DeleteJob(account(), url, this))->start();
+        }
+    }
 }
 
 void SyncEngine::deleteStaleErrorBlacklistEntries()

src/libsync/syncjournaldb.cpp

@@ -1370,21 +1370,22 @@ void SyncJournalDb::setUploadInfo(const QString& file, const SyncJournalDb::Uplo
     }
 }
 
-bool SyncJournalDb::deleteStaleUploadInfos(const QSet<QString> &keep)
+QVector<uint> SyncJournalDb::deleteStaleUploadInfos(const QSet<QString> &keep)
 {
     QMutexLocker locker(&_mutex);
+    QVector<uint> ids;
 
     if (!checkConnect()) {
-        return false;
+        return ids;
     }
 
     SqlQuery query(_db);
-    query.prepare("SELECT path FROM uploadinfo");
+    query.prepare("SELECT path,transferid FROM uploadinfo");
 
     if (!query.exec()) {
         QString err = query.error();
         qDebug() << "Error creating prepared statement: " << query.lastQuery() << ", Error:" << err;
-        return false;
+        return ids;
     }
 
     QStringList superfluousPaths;
@@ -1393,10 +1394,12 @@ bool SyncJournalDb::deleteStaleUploadInfos(const QSet<QString> &keep)
         const QString file = query.stringValue(0);
         if (!keep.contains(file)) {
             superfluousPaths.append(file);
+            ids.append(query.intValue(1));
         }
     }
 
-    return deleteBatch(*_deleteUploadInfoQuery, superfluousPaths, "uploadinfo");
+    deleteBatch(*_deleteUploadInfoQuery, superfluousPaths, "uploadinfo");
+    return ids;
 }
 
 SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry( const QString& file )

src/libsync/syncjournaldb.h

@@ -105,7 +105,8 @@ class OWNCLOUDSYNC_EXPORT SyncJournalDb : public QObject
 
     UploadInfo getUploadInfo(const QString &file);
     void setUploadInfo(const QString &file, const UploadInfo &i);
-    bool deleteStaleUploadInfos(const QSet<QString>& keep);
+    // Return the list of transfer ids that were removed.
+    QVector<uint> deleteStaleUploadInfos(const QSet<QString>& keep);
 
     SyncJournalErrorBlacklistRecord errorBlacklistEntry( const QString& );
     bool deleteStaleErrorBlacklistEntries(const QSet<QString>& keep);

test/syncenginetestutils.h

@@ -524,7 +524,8 @@ class FakeGetReply : public QNetworkReply
     Q_OBJECT
 public:
     const FileInfo *fileInfo;
-    QByteArray payload;
+    char payload;
+    int size;
 
     FakeGetReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, QObject *parent)
     : QNetworkReply{parent} {
@@ -540,8 +541,9 @@ class FakeGetReply : public QNetworkReply
     }
 
     Q_INVOKABLE void respond() {
-        payload.fill(fileInfo->contentChar, fileInfo->size);
-        setHeader(QNetworkRequest::ContentLengthHeader, payload.size());
+        payload = fileInfo->contentChar;
+        size = fileInfo->size;
+        setHeader(QNetworkRequest::ContentLengthHeader, size);
         setAttribute(QNetworkRequest::HttpStatusCodeAttribute, 200);
         setRawHeader("OC-ETag", fileInfo->etag.toLatin1());
         setRawHeader("ETag", fileInfo->etag.toLatin1());
@@ -553,12 +555,12 @@ class FakeGetReply : public QNetworkReply
     }
 
     void abort() override { }
-    qint64 bytesAvailable() const override { return payload.size() + QIODevice::bytesAvailable(); }
+    qint64 bytesAvailable() const override { return size + QIODevice::bytesAvailable(); }
 
     qint64 readData(char *data, qint64 maxlen) override {
-        qint64 len = std::min(qint64{payload.size()}, maxlen);
-        strncpy(data, payload.constData(), len);
-        payload.remove(0, len);
+        qint64 len = std::min(qint64{size}, maxlen);
+        std::fill_n(data, len, payload);
+        size -= len;
         return len;
     }
 };
@@ -607,7 +609,12 @@ class FakeChunkMoveReply : public QNetworkReply
         Q_ASSERT(!fileName.isEmpty());
 
         if ((fileInfo = remoteRootFileInfo.find(fileName))) {
-            QCOMPARE(request.rawHeader("If"), QByteArray("<" + request.rawHeader("Destination") + "> ([\"" + fileInfo->etag.toLatin1() + "\"])"));
+            QVERIFY(request.hasRawHeader("If")); // The client should put this header
+            if (request.rawHeader("If") != QByteArray("<" + request.rawHeader("Destination") +
+                                                "> ([\"" + fileInfo->etag.toLatin1() + "\"])")) {
+                QMetaObject::invokeMethod(this, "respondPreconditionFailed", Qt::QueuedConnection);
+                return;
+            }
             fileInfo->size = size;
             fileInfo->contentChar = payload;
         } else {
@@ -632,6 +639,13 @@ class FakeChunkMoveReply : public QNetworkReply
         emit finished();
     }
 
+    Q_INVOKABLE void respondPreconditionFailed() {
+        setAttribute(QNetworkRequest::HttpStatusCodeAttribute, 412);
+        setError(InternalServerError, "Precondition Failed");
+        emit metaDataChanged();
+        emit finished();
+    }
+
     void abort() override { }
     qint64 readData(char *, qint64) override { return 0; }
 };

test/testchunkingng.cpp

@@ -86,7 +86,7 @@ private slots:
     }
 
     // We modify the file locally after it has been partially uploaded
-    void testRemoveStale() {
+    void testRemoveStale1() {
 
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } });
@@ -107,6 +107,77 @@ private slots:
         QCOMPARE(fakeFolder.uploadState().children.count(), 1);
         QVERIFY(fakeFolder.uploadState().children.first().name != chunkingId);
     }
+
+    // We remove the file locally after it has been partially uploaded
+    void testRemoveStale2() {
+
+        FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
+        fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } });
+        const int size = 300 * 1000 * 1000; // 300 MB
+        partialUpload(fakeFolder, "A/a0", size);
+        QCOMPARE(fakeFolder.uploadState().children.count(), 1);
+
+        fakeFolder.localModifier().remove("A/a0");
+
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.uploadState().children.count(), 0);
+    }
+
+
+    void testCreateConflictWhileSyncing() {
+        FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
+        fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } });
+        const int size = 150 * 1000 * 1000; // 150 MB
+
+        // Put a file on the server and download it.
+        fakeFolder.remoteModifier().insert("A/a0", size);
+        QVERIFY(fakeFolder.syncOnce());
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        // Modify the file localy and start the upload
+        fakeFolder.localModifier().setContents("A/a0", 'B');
+        fakeFolder.localModifier().appendByte("A/a0");
+
+        // But in the middle of the sync, modify the file on the server
+        QMetaObject::Connection con = QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::transmissionProgress,
+                                    [&](const ProgressInfo &progress) {
+                if (progress.completedSize() > (progress.totalSize() / 2 )) {
+                    fakeFolder.remoteModifier().setContents("A/a0", 'C');
+                    QObject::disconnect(con);
+                }
+        });
+
+        QVERIFY(!fakeFolder.syncOnce());
+        // There was a precondition failed error, this means wen need to sync again
+        QCOMPARE(fakeFolder.syncEngine().isAnotherSyncNeeded(), ImmediateFollowUp);
+
+        QCOMPARE(fakeFolder.uploadState().children.count(), 1); // We did not clean the chunks at this point
+
+        // Now we will download the server file and create a conflict
+        QVERIFY(fakeFolder.syncOnce());
+        auto localState = fakeFolder.currentLocalState();
+
+        // A0 is the one from the server
+        QCOMPARE(localState.find("A/a0")->size, size);
+        QCOMPARE(localState.find("A/a0")->contentChar, 'C');
+
+        // There is a conflict file with our version
+        auto &stateAChildren = localState.find("A")->children;
+        auto it = std::find_if(stateAChildren.cbegin(), stateAChildren.cend(), [&](const FileInfo &fi) {
+            return fi.name.startsWith("a0_conflict");
+        });
+        QVERIFY(it != stateAChildren.cend());
+        QCOMPARE(it->contentChar, 'B');
+        QCOMPARE(it->size, size+1);
+
+        // Remove the conflict file so the comparison works!
+        fakeFolder.localModifier().remove("A/" + it->name);
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        QCOMPARE(fakeFolder.uploadState().children.count(), 0); // The last sync cleaned the chunks
+    }
+
 };
 
 QTEST_GUILESS_MAIN(TestChunkingNG)