From f6c77fad17060bf25f4e6591360340670ba2dd1c Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Fri, 20 Jan 2017 12:06:20 +0100 Subject: [PATCH] ChunkingNG: delete stale chunks if the file was changed locally Relates to https://github.com/owncloud/core/issues/26981 We do not track the success or error of the DeleteJob because it does not matter. If it fails, it might be because the chunks were already removed. If not, the chunks will be stale, but the server must anyway do a few cleanup from time to time because we do not always remove the chunks --- src/libsync/propagateuploadng.cpp | 6 +++ test/syncenginetestutils.h | 3 +- test/testchunkingng.cpp | 81 ++++++++++++++++++++++--------- 3 files changed, 66 insertions(+), 24 deletions(-) diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 09891b65b..b4191a216 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -97,6 +97,12 @@ void PropagateUploadFileNG::doStartUpload() this, SLOT(slotPropfindIterate(QString,QMap))); job->start(); return; + } else if (progressInfo._valid) { + // The upload info is stale. remove the stale chunks on the server + _transferId = progressInfo._transferid; + // Fire and forget. Any error will be ignored. + (new DeleteJob(propagator()->account(), chunkUrl(), this))->start(); + // startNewUpload will reset the _transferId and the UploadInfo in the db. } startNewUpload(); diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index 665dc2016..b2e52444b 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -586,7 +586,7 @@ public: Q_ASSERT(sourceFolder->isDir); int count = 0; int size = 0; - char payload = '*'; + char payload = '\0'; do { if (!sourceFolder->children.contains(QString::number(count))) @@ -595,6 +595,7 @@ public: Q_ASSERT(!x.isDir); Q_ASSERT(x.size > 0); // There should not be empty chunks size += x.size; + Q_ASSERT(!payload || payload == x.contentChar); payload = x.contentChar; ++count; } while(true); diff --git a/test/testchunkingng.cpp b/test/testchunkingng.cpp index 21225c210..b038737d6 100644 --- a/test/testchunkingng.cpp +++ b/test/testchunkingng.cpp @@ -11,6 +11,36 @@ using namespace OCC; +/* Upload a 1/3 of a file of given size. + * fakeFolder needs to be synchronized */ +static void partialUpload(FakeFolder &fakeFolder, const QString &name, int size) +{ + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QCOMPARE(fakeFolder.uploadState().children.count(), 0); // The state should be clean + + fakeFolder.localModifier().insert(name, size); + // Abort when the upload is at 1/3 + int sizeWhenAbort = -1; + auto con = QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::transmissionProgress, + [&](const ProgressInfo &progress) { + if (progress.completedSize() > (progress.totalSize() /3 )) { + sizeWhenAbort = progress.completedSize(); + fakeFolder.syncEngine().abort(); + } + }); + + QVERIFY(!fakeFolder.syncOnce()); // there should have been an error + QObject::disconnect(con); + QVERIFY(sizeWhenAbort > 0); + QVERIFY(sizeWhenAbort < size); + + QCOMPARE(fakeFolder.uploadState().children.count(), 1); // the transfer was done with chunking + auto upStateChildren = fakeFolder.uploadState().children.first().children; + QCOMPARE(sizeWhenAbort, std::accumulate(upStateChildren.cbegin(), upStateChildren.cend(), 0, + [](int s, const FileInfo &i) { return s + i.size; })); +} + + class TestChunkingNG : public QObject { Q_OBJECT @@ -40,37 +70,42 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); const int size = 300 * 1000 * 1000; // 300 MB - fakeFolder.localModifier().insert("A/a0", size); - - // Abort when the upload is at 1/3 - int sizeWhenAbort = -1; - auto con = QObject::connect(&fakeFolder.syncEngine(), &SyncEngine::transmissionProgress, - [&](const ProgressInfo &progress) { - if (progress.completedSize() > (progress.totalSize() /3 )) { - sizeWhenAbort = progress.completedSize(); - fakeFolder.syncEngine().abort(); - } - }); - - QVERIFY(!fakeFolder.syncOnce()); // there should have been an error - QObject::disconnect(con); - QVERIFY(sizeWhenAbort > 0); - QVERIFY(sizeWhenAbort < size); - QCOMPARE(fakeFolder.uploadState().children.count(), 1); // the transfer was done with chunking - auto upStateChildren = fakeFolder.uploadState().children.first().children; - QCOMPARE(sizeWhenAbort, std::accumulate(upStateChildren.cbegin(), upStateChildren.cend(), 0, - [](int s, const FileInfo &i) { return s + i.size; })); - + partialUpload(fakeFolder, "A/a0", size); + QCOMPARE(fakeFolder.uploadState().children.count(), 1); + auto chunkingId = fakeFolder.uploadState().children.first().name; // Add a fake file to make sure it gets deleted fakeFolder.uploadState().children.first().insert("10000", size); QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QCOMPARE(fakeFolder.currentRemoteState().find("A/a0")->size, size); + // The same chunk id was re-used + QCOMPARE(fakeFolder.uploadState().children.count(), 1); + QCOMPARE(fakeFolder.uploadState().children.first().name, chunkingId); + } + // We modify the file locally after it has been partially uploaded + void testRemoveStale() { + + 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); + auto chunkingId = fakeFolder.uploadState().children.first().name; + + + fakeFolder.localModifier().setContents("A/a0", 'B'); + fakeFolder.localModifier().appendByte("A/a0"); + + QVERIFY(fakeFolder.syncOnce()); QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); - QCOMPARE(fakeFolder.uploadState().children.count(), 1); // The same chunk id was re-used - QCOMPARE(fakeFolder.currentRemoteState().find("A/a0")->size, size); + QCOMPARE(fakeFolder.currentRemoteState().find("A/a0")->size, size + 1); + // A different chunk id was used, and the previous one is removed + QCOMPARE(fakeFolder.uploadState().children.count(), 1); + QVERIFY(fakeFolder.uploadState().children.first().name != chunkingId); } }; -- 2.30.2