Upload: Applied suggested changes

src/libsync/account.h

@@ -253,11 +253,11 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
     void setHttp2Supported(bool value) { _http2Supported = value; }
 
     /** Max allowed request size in bytes that the server accepts. Is <= 0 if not limited */
-    qint64 getMaxRequestSize() { return _maxRequestSize; }
+    qint64 getMaxRequestSize() const { return _maxRequestSize; }
     void setMaxRequestSize(qint64 value) { _maxRequestSize = value; }
     void setMaxRequestSizeIfLower(qint64 value) {
         while (true) {
-            auto size = _maxRequestSize;
+            const auto size = _maxRequestSize;
 
             if (size > 0 && value >= size)
                 break;

src/libsync/configfile.cpp

@@ -15,6 +15,7 @@
 #include "config.h"
 
 #include "configfile.h"
+#include "syncoptions.h"
 #include "theme.h"
 #include "version.h"
 #include "common/utility.h"
@@ -237,25 +238,25 @@ int ConfigFile::timeout() const
 qint64 ConfigFile::chunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return settings.value(QLatin1String(chunkSizeC), 10 * 1000 * 1000).toLongLong(); // default to 10 MB
+    return settings.value(QLatin1String(chunkSizeC), DefaultInitialChunkSize).toLongLong(); // default to 10 MB
 }
 
 qint64 ConfigFile::maxChunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return settings.value(QLatin1String(maxChunkSizeC), 1000 * 1000 * 1000).toLongLong(); // default to 1000 MB
+    return settings.value(QLatin1String(maxChunkSizeC), DefaultMaxChunkSize).toLongLong(); // default to 1000 MB
 }
 
 qint64 ConfigFile::failsafeMaxChunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return settings.value(QLatin1String(failsafeMaxChunkSizeC), 100 * 1000 * 1000).toLongLong(); // default to 100 MB
+    return settings.value(QLatin1String(failsafeMaxChunkSizeC), DefaultFailsafeMaxChunkSize).toLongLong(); // default to 100 MB
 }
 
 qint64 ConfigFile::minChunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);
-    return settings.value(QLatin1String(minChunkSizeC), 1000 * 1000).toLongLong(); // default to 1 MB
+    return settings.value(QLatin1String(minChunkSizeC), DefaultMinChunkSize).toLongLong(); // default to 1 MB
 }
 
 chrono::milliseconds ConfigFile::targetChunkUploadDuration() const

src/libsync/propagateupload.cpp

@@ -676,42 +676,45 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)
     SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode,
         &propagator()->_anotherSyncNeeded, replyContent);
 
-    // Remote host closed the connection after an upload larger than _failsafeMaxChunkSize
-    if (job->reply()->error() == QNetworkReply::RemoteHostClosedError 
-        && _item->_requestBodySize > propagator()->syncOptions()._failsafeMaxChunkSize) {
-        // treat it as if the server returned HTTP-413
-        qCInfo(lcPropagateUpload()) << "Remote host closed the connection when uploading" 
-                                    << Utility::octetsToString(_item->_requestBodySize) << "."
-                                    << "Treating the error like HTTP-413 (request entity to large)";
-
-        _item->_httpErrorCode = 413;
-    }
-
     // Request entity to large: Chunk size is larger than supported by the server
-    if (_item->_httpErrorCode == 413 && _item->_requestBodySize > 0 && status == SyncFileItem::NormalError) {
-        auto opts = propagator()->syncOptions();
-        auto requestSize = _item->_requestBodySize;
-
-        // Set new bound as account upload limit (we need to guess here, as sent bytes are no good indicator what is allowed)
-        auto maxRequestSize = requestSize > opts._failsafeMaxChunkSize
-            ? opts._failsafeMaxChunkSize
-            : qMax(opts._minChunkSize, requestSize / 2);
-
-        propagator()->account()->setMaxRequestSizeIfLower(maxRequestSize);
-
-        // Apply to this propagator (limit is applied in setSyncOptions())
-        propagator()->setSyncOptions(opts);
-        opts = propagator()->syncOptions();
-
-        // Trigger retry
-        if (opts._maxChunkSize < requestSize) {
-            propagator()->_anotherSyncNeeded = true;
-            status = SyncFileItem::SoftError;
+    if (status == SyncFileItem::NormalError && _item->_requestBodySize > 0) {
+        const auto opts = propagator()->syncOptions();
+        const auto requestSize = _item->_requestBodySize;
+
+        // Check if remote host closed the connection after an upload larger than _failsafeMaxChunkSize
+        const bool isConnectionClosedOnLargeUpload = job->reply()->error() == QNetworkReply::RemoteHostClosedError
+                                                     && requestSize > opts._failsafeMaxChunkSize;
+
+        if (isConnectionClosedOnLargeUpload) {
+            qCInfo(lcPropagateUpload()) << "Remote host closed the connection when uploading" 
+                                        << Utility::octetsToString(requestSize) << "."
+                                        << "Treating the error like HTTP-413 (request entity to large)";
         }
 
-        errorString = tr("Upload of %1 exceeds the max upload size allowed by the server. Reducing max upload size to %2")
-                        .arg(Utility::octetsToString(requestSize))
-                        .arg(Utility::octetsToString(opts._maxChunkSize));
+        // Calculate new upload limit and set it in propagator()->account() 
+        // Sent bytes is no indicator how much is allowed by the server as requests must be consumed fully.
+        if (_item->_httpErrorCode == 413 || isConnectionClosedOnLargeUpload) {
+
+            const auto maxRequestSize = requestSize > opts._failsafeMaxChunkSize
+                ? opts._failsafeMaxChunkSize
+                : qMax(opts._minChunkSize, requestSize / 2);
+
+            propagator()->account()->setMaxRequestSizeIfLower(maxRequestSize);
+
+            // Apply to this propagator (limit is applied in setSyncOptions())
+            propagator()->setSyncOptions(opts);
+            const auto adjustedOpts = propagator()->syncOptions();
+
+            // Trigger retry
+            if (adjustedOpts._maxChunkSize < requestSize) {
+                propagator()->_anotherSyncNeeded = true;
+                status = SyncFileItem::SoftError;
+            }
+
+            errorString = tr("Upload of %1 exceeds the max upload size allowed by the server. Reducing max upload size to %2")
+                            .arg(Utility::octetsToString(requestSize))
+                            .arg(Utility::octetsToString(adjustedOpts._maxChunkSize));
+        }
     }
 
     // Insufficient remote storage.

src/libsync/syncoptions.cpp

@@ -19,9 +19,6 @@
 
 using namespace OCC;
 
-// limit cannot be lower than 512KB
-const qint64 lowestPossibleChunkSize = 512 * 1024;
-
 SyncOptions::SyncOptions()
     : _vfs(new VfsOff)
 {
@@ -64,10 +61,10 @@ void SyncOptions::verifyChunkSizes(qint64 maxUpperLimit)
         _maxChunkSize = qMin(maxUpperLimit, _maxChunkSize);
     }
 
-    _initialChunkSize = qMax(lowestPossibleChunkSize, _initialChunkSize);
-    _minChunkSize = qMax(lowestPossibleChunkSize, _minChunkSize);
-    _maxChunkSize = qMax(lowestPossibleChunkSize, _maxChunkSize);
-    _failsafeMaxChunkSize = qMax(lowestPossibleChunkSize, _failsafeMaxChunkSize);
+    _initialChunkSize = qMax(LowestPossibleChunkSize, _initialChunkSize);
+    _minChunkSize = qMax(LowestPossibleChunkSize, _minChunkSize);
+    _maxChunkSize = qMax(LowestPossibleChunkSize, _maxChunkSize);
+    _failsafeMaxChunkSize = qMax(LowestPossibleChunkSize, _failsafeMaxChunkSize);
 
     _minChunkSize = qMin(_minChunkSize, _initialChunkSize);
     _maxChunkSize = qMax(_maxChunkSize, _initialChunkSize);

src/libsync/syncoptions.h

@@ -26,6 +26,21 @@
 
 namespace OCC {
 
+/** Default value for SyncOptions::_initialChunkSize     (  10MB) */
+const qint64 DefaultInitialChunkSize = 10 * 1000 * 1000;
+
+/** Default value for SyncOptions::_minChunkSize         (   1MB) */
+const qint64 DefaultMinChunkSize = 1 * 1000 * 1000;
+
+/** Default value for SyncOptions::_maxChunkSize         (1000MB) */
+const qint64 DefaultMaxChunkSize = 1000 * 1000 * 1000;
+
+/** Default value for SyncOptions::_failsafeMaxChunkSize ( 100MB) */
+const qint64 DefaultFailsafeMaxChunkSize = 100 * 1000 * 1000;
+
+/** No chunk size can be set to a value lower than this  ( 512KB) */
+const qint64 LowestPossibleChunkSize = 512 * 1000;
+
 /**
  * Value class containing the options given to the sync engine
  */
@@ -55,18 +70,18 @@ class OWNCLOUDSYNC_EXPORT SyncOptions
      * starting value and is then gradually adjusted within the
      * minChunkSize / maxChunkSize bounds.
      */
-    qint64 _initialChunkSize = 10 * 1000 * 1000; // 10MB
+    qint64 _initialChunkSize = DefaultInitialChunkSize;
 
     /** The minimum chunk size in bytes for chunked uploads */
-    qint64 _minChunkSize = 1 * 1000 * 1000; // 1MB
+    qint64 _minChunkSize = DefaultMinChunkSize;
 
     /** The maximum chunk size in bytes for chunked uploads */
-    qint64 _maxChunkSize = 1000 * 1000 * 1000; // 1000MB
+    qint64 _maxChunkSize = DefaultMaxChunkSize;
 
     /** Failsafe configuration of the maximum chunk size in bytes
      *  Applies when a chunked upload caused HTTP-413 or RemoteHostClosedError
      */
-    qint64 _failsafeMaxChunkSize = 100 * 1000 * 1000; // 100MB
+    qint64 _failsafeMaxChunkSize = DefaultFailsafeMaxChunkSize;
 
     /** The target duration of chunk uploads for dynamic chunk sizing.
      *

test/testnextcloudpropagator.cpp

@@ -10,6 +10,7 @@
 #include "account.h"
 #include "propagatedownload.h"
 #include "owncloudpropagator_p.h"
+#include "syncoptions.h"
 
 using namespace OCC;
 namespace OCC {
@@ -81,7 +82,7 @@ private slots:
     void testRespectsLowestPossibleChunkSize()
     {
         QSet<QString> __blacklist;
-        OwncloudPropagator propagator(OCC::Account::create(), "", "", nullptr, __blacklist);
+        OwncloudPropagator propagator(Account::create(), "", "", nullptr, __blacklist);
         auto opts = propagator.syncOptions();
 
         opts._minChunkSize = 
@@ -91,15 +92,15 @@ private slots:
         propagator.setSyncOptions(opts);
 
         opts = propagator.syncOptions();
-        QCOMPARE( opts._minChunkSize, 512 * 1024 );
-        QCOMPARE( opts._initialChunkSize, 512 * 1024 );
-        QCOMPARE( opts._maxChunkSize, 512 * 1024 );
+        QCOMPARE( opts._minChunkSize, LowestPossibleChunkSize );
+        QCOMPARE( opts._initialChunkSize, LowestPossibleChunkSize );
+        QCOMPARE( opts._maxChunkSize, LowestPossibleChunkSize );
     }
 
     void testLimitsMaxChunkSizeByAccount()
     {
         QSet<QString> __blacklist;
-        OwncloudPropagator propagator(OCC::Account::create(), "", "", nullptr, __blacklist);
+        OwncloudPropagator propagator(Account::create(), "", "", nullptr, __blacklist);
         auto opts = propagator.syncOptions();
 
         SyncOptions defaultOpts;

test/testsyncengine.cpp

@@ -8,6 +8,7 @@
 #include <QtTest>
 #include "syncenginetestutils.h"
 #include <syncengine.h>
+#include <syncoptions.h>
 #include <propagatorjobs.h>
 
 using namespace OCC;
@@ -599,7 +600,7 @@ private slots:
     {
         FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
 
-        const int minRequestSize = 512 * 1024;
+        const int minRequestSize = LowestPossibleChunkSize;
         int maxAllowedRequestSize = 4 * minRequestSize;
 
         SyncOptions options;
@@ -911,7 +912,7 @@ private slots:
         SyncOptions options;
         options._initialChunkSize =
             options._maxChunkSize = 
-            options._minChunkSize = 1000 * 1000;
+            options._minChunkSize = LowestPossibleChunkSize;
 
         fakeFolder.syncEngine().setSyncOptions(options);