Enforce Chunk V2 API chunk size limits at the sync options level

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

src/gui/folder.cpp

@@ -1037,9 +1037,9 @@ SyncOptions Folder::initializeSyncOptions() const
     opt._parallelNetworkJobs = _accountState->account()->isHttp2Supported() ? 20 : 6;
 
     // Chunk V2: Size of chunks must be between 5MB and 5GB, except for the last chunk which can be smaller
-    opt._minChunkSize = qMin(cfgFile.minChunkSize(), 5LL * 1000LL * 1000LL);
-    opt._maxChunkSize = qMax(cfgFile.maxChunkSize(), 5LL * 1000LL * 1000LL);
-    opt._initialChunkSize = qBound(opt._minChunkSize, cfgFile.chunkSize(), opt._maxChunkSize);
+    opt.setMinChunkSize(cfgFile.minChunkSize());
+    opt.setMaxChunkSize(cfgFile.maxChunkSize());
+    opt._initialChunkSize = ::qBound(opt.minChunkSize(), cfgFile.chunkSize(), opt.maxChunkSize());
     opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration();
 
     opt.fillFromEnvironmentVariables();

src/libsync/owncloudpropagator.cpp

@@ -1046,7 +1046,8 @@ bool OwncloudPropagator::isDelayedUploadItem(const SyncFileItemPtr &item) const
         return true;
     };
 
-    return account()->capabilities().bulkUpload() && !_scheduleDelayedTasks && !item->isEncrypted() && _syncOptions._minChunkSize > item->_size && !isInBulkUploadBlackList(item->_file) && !checkFileShouldBeEncrypted(item);
+    return account()->capabilities().bulkUpload() && !_scheduleDelayedTasks && !item->isEncrypted() && _syncOptions.minChunkSize() > item->_size
+        && !isInBulkUploadBlackList(item->_file) && !checkFileShouldBeEncrypted(item);
 }
 
 void OwncloudPropagator::setScheduleDelayedTasks(bool active)

src/libsync/propagateuploadng.cpp

@@ -427,10 +427,7 @@ void PropagateUploadFileNG::slotPutFinished()
         qint64 targetSize = propagator()->_chunkSize / 2 + predictedGoodSize / 2;
 
         // Adjust the dynamic chunk size _chunkSize used for sizing of the item's chunks to be send
-        propagator()->_chunkSize = qBound(
-            propagator()->syncOptions()._minChunkSize,
-            targetSize,
-            propagator()->syncOptions()._maxChunkSize);
+        propagator()->_chunkSize = ::qBound(propagator()->syncOptions().minChunkSize(), targetSize, propagator()->syncOptions().maxChunkSize());
 
         qCInfo(lcPropagateUploadNG) << "Chunked upload of" << _currentChunkSize << "bytes took" << uploadTime.count()
                                   << "ms, desired is" << targetDuration.count() << "ms, expected good chunk size is"

src/libsync/syncoptions.cpp

@@ -26,6 +26,26 @@ SyncOptions::SyncOptions()
 
 SyncOptions::~SyncOptions() = default;
 
+qint64 SyncOptions::minChunkSize() const
+{
+    return _minChunkSize;
+}
+
+void SyncOptions::setMinChunkSize(const qint64 minChunkSize)
+{
+    _minChunkSize = ::qBound(_minChunkSize, minChunkSize, _maxChunkSize);
+}
+
+qint64 SyncOptions::maxChunkSize() const
+{
+    return _maxChunkSize;
+}
+
+void SyncOptions::setMaxChunkSize(const qint64 maxChunkSize)
+{
+    _maxChunkSize = ::qBound(_minChunkSize, maxChunkSize, _maxChunkSize);
+}
+
 void SyncOptions::fillFromEnvironmentVariables()
 {
     QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE");

src/libsync/syncoptions.h

@@ -57,12 +57,6 @@ public:
      */
     qint64 _initialChunkSize = 10 * 1000 * 1000; // 10MB
 
-    /** The minimum chunk size in bytes for chunked uploads */
-    qint64 _minChunkSize = 1 * 1000 * 1000; // 1MB
-
-    /** The maximum chunk size in bytes for chunked uploads */
-    qint64 _maxChunkSize = 1000 * 1000 * 1000; // 1000MB
-
     /** The target duration of chunk uploads for dynamic chunk sizing.
      *
      * Set to 0 it will disable dynamic chunk sizing.
@@ -72,6 +66,17 @@ public:
     /** The maximum number of active jobs in parallel  */
     int _parallelNetworkJobs = 6;
 
+    static constexpr auto chunkV2MinChunkSize = 5LL * 1000LL * 1000LL; // 5 MB
+    static constexpr auto chunkV2MaxChunkSize = 5LL * 1000LL * 1000LL * 1000LL; // 5 GB
+
+    /** The minimum chunk size in bytes for chunked uploads */
+    qint64 minChunkSize() const;
+    void setMinChunkSize(const qint64 minChunkSize);
+
+    /** The maximum chunk size in bytes for chunked uploads */
+    qint64 maxChunkSize() const;
+    void setMaxChunkSize(const qint64 maxChunkSize);
+
     /** Reads settings from env vars where available.
      *
      * Currently reads _initialChunkSize, _minChunkSize, _maxChunkSize,
@@ -110,6 +115,9 @@ private:
      * Invalid pattern by default.
      */
     QRegularExpression _fileRegex = QRegularExpression(QStringLiteral("("));
+
+    qint64 _minChunkSize = chunkV2MinChunkSize;
+    qint64 _maxChunkSize = chunkV2MaxChunkSize;
 };
 
 }

test/testasyncop.cpp

@@ -54,7 +54,7 @@ private slots:
         fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ { "chunking", "1.0" } } } });
         // Reduce max chunk size a bit so we get more chunks
         SyncOptions options;
-        options._maxChunkSize = 20 * 1000;
+        options.setMaxChunkSize(20 * 1000);
         fakeFolder.syncEngine().setSyncOptions(options);
         int nGET = 0;
 
@@ -144,16 +144,15 @@ private slots:
             testCases[file] = { std::move(cb) };
         };
         fakeFolder.localModifier().mkdir("success");
-        insertFile("success/chunked_success", options._maxChunkSize * 3, successCallback);
+        insertFile("success/chunked_success", options.maxChunkSize() * 3, successCallback);
         insertFile("success/single_success", 300, successCallback);
-        insertFile("success/chunked_patience", options._maxChunkSize * 3,
-            waitAndChain(waitAndChain(successCallback)));
+        insertFile("success/chunked_patience", options.maxChunkSize() * 3, waitAndChain(waitAndChain(successCallback)));
         insertFile("success/single_patience", 300,
             waitAndChain(waitAndChain(successCallback)));
         fakeFolder.localModifier().mkdir("err");
-        insertFile("err/chunked_error", options._maxChunkSize * 3, errorCallback);
+        insertFile("err/chunked_error", options.maxChunkSize() * 3, errorCallback);
         insertFile("err/single_error", 300, errorCallback);
-        insertFile("err/chunked_error2", options._maxChunkSize * 3, waitAndChain(errorCallback));
+        insertFile("err/chunked_error2", options.maxChunkSize() * 3, waitAndChain(errorCallback));
         insertFile("err/single_error2", 300, waitAndChain(errorCallback));
 
         // First sync should finish by itself.
@@ -171,11 +170,10 @@ private slots:
         fakeFolder.localModifier().mkdir("waiting");
         insertFile("waiting/small", 300, waitForeverCallback);
         insertFile("waiting/willNotConflict", 300, waitForeverCallback);
-        insertFile("waiting/big", options._maxChunkSize * 3,
-            waitAndChain(waitAndChain([&](TestCase *tc, const QNetworkRequest &request) {
-                QTimer::singleShot(0, &fakeFolder.syncEngine(), &SyncEngine::abort);
-                return waitAndChain(waitForeverCallback)(tc, request);
-            })));
+        insertFile("waiting/big", options.maxChunkSize() * 3, waitAndChain(waitAndChain([&](TestCase *tc, const QNetworkRequest &request) {
+                       QTimer::singleShot(0, &fakeFolder.syncEngine(), &SyncEngine::abort);
+                       return waitAndChain(waitForeverCallback)(tc, request);
+                   })));
 
         QVERIFY(fakeFolder.syncJournal().wipeErrorBlacklist() != -1);
 

test/testchunkingng.cpp

@@ -43,9 +43,9 @@ static void partialUpload(FakeFolder &fakeFolder, const QString &name, qint64 si
 static void setChunkSize(SyncEngine &engine, qint64 size)
 {
     SyncOptions options;
-    options._maxChunkSize = size;
+    options.setMaxChunkSize(size);
+    options.setMinChunkSize(size);
     options._initialChunkSize = size;
-    options._minChunkSize = size;
     engine.setSyncOptions(options);
 }
 
@@ -607,6 +607,13 @@ private slots:
     void testVeryBigFiles() {
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } });
+
+        // Dynamic chunk sizing tries to go for biggest chunks possible depending on upload speed.
+        // In the tests this is immediate, so we need to give a file larger than the max chunk size
+        // and cap the max chunk size a bit
+        auto opts = fakeFolder.syncEngine().syncOptions();
+        opts.setMaxChunkSize(500LL * 1000LL * 1000LL); // 500MB
+        fakeFolder.syncEngine().setSyncOptions(opts);
         const qint64 size = 2.5 * 1024 * 1024 * 1024; // 2.5 GiB
 
         // Partial upload of big files
@@ -623,7 +630,6 @@ private slots:
         QCOMPARE(fakeFolder.uploadState().children.count(), 1);
         QCOMPARE(fakeFolder.uploadState().children.first().name, chunkingId);
 
-
         // Upload another file again, this time without interruption
         fakeFolder.localModifier().appendByte("A/a0");
         QVERIFY(fakeFolder.syncOnce());

test/testsyncengine.cpp

@@ -851,8 +851,8 @@ private slots:
         FakeFolder fakeFolder{ FileInfo{} };
         SyncOptions options;
         options._initialChunkSize = 10;
-        options._maxChunkSize = 10;
-        options._minChunkSize = 10;
+        options.setMaxChunkSize(10);
+        options.setMinChunkSize(10);
         fakeFolder.syncEngine().setSyncOptions(options);
 
         QObject parent;