Merge pull request #7588 from nextcloud/backport/7580/stable-3.15

[stable-3.15] fix regressions in automated tests for bulk upload
nextcloud · Nov 29, 2024 · f6a91ce · f6a91ce
2 parents 79af26a + e502dfa
commit f6a91ce
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 19 deletions.
diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp
@@ -520,6 +520,7 @@ void BulkPropagatorJob::finalize(const QJsonObject &fullReply)
         }
         if (!singleFile._item->hasErrorStatus()) {
             finalizeOneFile(singleFile);
+            singleFile._item->_status = OCC::SyncFileItem::Success;
         }
 
         done(singleFile._item, singleFile._item->_status, {}, ErrorCategory::GenericError);

diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
@@ -532,7 +532,10 @@ qint64 UploadDevice::readData(char *data, qint64 maxlen)
     }
 
     auto c = _file.read(data, maxlen);
-    if (c < 0) {
+    if (c == 0) {
+        setErrorString({});
+        return c;
+    } else if (c < 0) {
         setErrorString(_file.errorString());
         return -1;
     }

diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp
@@ -354,7 +354,7 @@ Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath, const
             break;
         };
         return VfsItemAvailability::Mixed;
-    } else {
+    } else if (basePinState) {
         const auto hydrationAndPinStates = computeRecursiveHydrationAndPinStates(folderPath, basePinState);
 
         const auto pin = hydrationAndPinStates.pinState;
@@ -382,6 +382,8 @@ Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath, const
             }
         }
         return AvailabilityError::NoSuchItem;
+    } else {
+        return AvailabilityError::NoSuchItem;
     }
 }
 

diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp
@@ -552,12 +552,12 @@ QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot
         auto onePartBody = onePart.mid(headerEndPosition + 4, onePart.size() - headerEndPosition - 6);
         auto onePartHeaders = onePartHeaderPart.split(QStringLiteral("\r\n"));
         QMap<QString, QString> allHeaders;
-        for(auto oneHeader : onePartHeaders) {
+        for(const auto &oneHeader : onePartHeaders) {
             auto headerParts = oneHeader.split(QStringLiteral(": "));
-            allHeaders[headerParts.at(0)] = headerParts.at(1);
+            allHeaders[headerParts.at(0).toLower()] = headerParts.at(1);
         }
-        const auto fileName = allHeaders[QStringLiteral("X-File-Path")];
-        const auto modtime = allHeaders[QByteArrayLiteral("X-File-Mtime")].toLongLong();
+        const auto fileName = allHeaders[QStringLiteral("x-file-path")];
+        const auto modtime = allHeaders[QByteArrayLiteral("x-file-mtime")].toLongLong();
         Q_ASSERT(!fileName.isEmpty());
         Q_ASSERT(modtime > 0);
         FileInfo *fileInfo = remoteRootFileInfo.find(fileName);
@@ -568,7 +568,7 @@ QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot
             // Assume that the file is filled with the same character
             fileInfo = remoteRootFileInfo.create(fileName, onePartBody.size(), onePartBody.at(0).toLatin1());
         }
-        fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(request.rawHeader("X-OC-Mtime").toLongLong());
+        fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(request.rawHeader("x-oc-mtime").toLongLong());
         remoteRootFileInfo.find(fileName, /*invalidateEtags=*/true);
         result.push_back(fileInfo);
     }
@@ -1052,13 +1052,13 @@ QJsonObject FakeQNAM::forEachReplyPart(QIODevice *outgoingData,
         QMap<QString, QByteArray> allHeaders;
         for(const auto &oneHeader : qAsConst(onePartHeaders)) {
             auto headerParts = oneHeader.split(QStringLiteral(": "));
-            allHeaders[headerParts.at(0)] = headerParts.at(1).toLatin1();
+            allHeaders[headerParts.at(0).toLower()] = headerParts.at(1).toLatin1();
         }
 
         auto reply = replyFunction(allHeaders);
         if (reply.contains(QStringLiteral("error")) &&
                 reply.contains(QStringLiteral("etag"))) {
-            fullReply.insert(allHeaders[QStringLiteral("X-File-Path")], reply);
+            fullReply.insert(allHeaders[QStringLiteral("x-file-path")], reply);
         }
     }
 
@@ -1413,3 +1413,46 @@ FakeFileLockReply::FakeFileLockReply(FileInfo &remoteRootFileInfo,
     xml.writeEndElement(); // prop
     xml.writeEndDocument();
 }
+
+FakeJsonReply::FakeJsonReply(QNetworkAccessManager::Operation op,
+                             const QNetworkRequest &request,
+                             QObject *parent,
+                             int httpReturnCode,
+                             const QJsonDocument &reply)
+    : FakeReply{parent}
+    , _body{reply.toJson()}
+{
+    setRequest(request);
+    setUrl(request.url());
+    setOperation(op);
+    open(QIODevice::ReadOnly);
+    setAttribute(QNetworkRequest::HttpStatusCodeAttribute, httpReturnCode);
+    QMetaObject::invokeMethod(this, &FakeJsonReply::respond, Qt::QueuedConnection);
+}
+
+void FakeJsonReply::respond()
+{
+    emit metaDataChanged();
+    emit readyRead();
+    // finishing can come strictly after readyRead was called
+    QTimer::singleShot(5, this, &FakeJsonReply::slotSetFinished);
+}
+
+void FakeJsonReply::slotSetFinished()
+{
+    setFinished(true);
+    emit finished();
+}
+
+qint64 FakeJsonReply::readData(char *buf, qint64 max)
+{
+    max = qMin<qint64>(max, _body.size());
+    memcpy(buf, _body.constData(), max);
+    _body = _body.mid(max);
+    return max;
+}
+
+qint64 FakeJsonReply::bytesAvailable() const
+{
+    return _body.size();
+}
diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h
@@ -214,6 +214,29 @@ class FakeReply : public QNetworkReply
     using QNetworkReply::setRawHeader;
 };
 
+class FakeJsonReply : public FakeReply
+{
+    Q_OBJECT
+public:
+    FakeJsonReply(QNetworkAccessManager::Operation op,
+                  const QNetworkRequest &request,
+                  QObject *parent,
+                  int httpReturnCode,
+                  const QJsonDocument &reply = QJsonDocument());
+
+    Q_INVOKABLE virtual void respond();
+
+public slots:
+    void slotSetFinished();
+
+public:
+    void abort() override { }
+    qint64 readData(char *buf, qint64 max) override;
+    [[nodiscard]] qint64 bytesAvailable() const override;
+
+    QByteArray _body;
+};
+
 class FakePropfindReply : public FakeReply
 {
     Q_OBJECT

diff --git a/test/testlocaldiscovery.cpp b/test/testlocaldiscovery.cpp
@@ -378,6 +378,7 @@ private slots:
         const QString fileWithSpaces4("A/ foo");
         const QString fileWithSpaces5("A/ bar ");
         const QString fileWithSpaces6("A/bla ");
+        const auto extraFileNameWithSpaces = QStringLiteral(" with spaces ");
 
         fakeFolder.localModifier().insert(fileWithSpaces1);
         fakeFolder.localModifier().insert(fileWithSpaces2);
@@ -386,7 +387,7 @@ private slots:
         fakeFolder.localModifier().insert(fileWithSpaces4);
         fakeFolder.localModifier().insert(fileWithSpaces5);
         fakeFolder.localModifier().insert(fileWithSpaces6);
-        fakeFolder.localModifier().mkdir(QStringLiteral(" with spaces "));
+        fakeFolder.localModifier().mkdir(extraFileNameWithSpaces);
 
         ItemCompletedSpy completeSpy(fakeFolder);
         completeSpy.clear();
@@ -400,15 +401,15 @@ private slots:
         QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::FileNameInvalid);
         QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::FileNameInvalid);
         QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::FileNameInvalid);
-        QCOMPARE(completeSpy.findItem(QStringLiteral(" with spaces "))->_status, SyncFileItem::Status::FileNameInvalid);
+        QCOMPARE(completeSpy.findItem(extraFileNameWithSpaces)->_status, SyncFileItem::Status::FileNameInvalid);
 #else
         QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::Success);
         QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::Success);
         QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::Success);
         QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::Success);
         QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::Success);
         QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::Success);
-        QCOMPARE(completeSpy.findItem(QStringLiteral(" with spaces "))->_status, SyncFileItem::Status::Success);
+        QCOMPARE(completeSpy.findItem(extraFileNameWithSpaces)->_status, SyncFileItem::Status::Success);
 #endif
 
         fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces1);
@@ -417,7 +418,7 @@ private slots:
         fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces4);
         fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces5);
         fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + fileWithSpaces6);
-        fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + QStringLiteral(" with spaces "));
+        fakeFolder.syncEngine().addAcceptedInvalidFileName(fakeFolder.localPath() + extraFileNameWithSpaces);
 
         completeSpy.clear();
 
@@ -431,7 +432,7 @@ private slots:
         QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::Success);
         QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::Success);
         QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::Success);
-        QCOMPARE(completeSpy.findItem(QStringLiteral(" with spaces "))->_status, SyncFileItem::Status::NormalError);
+        QCOMPARE(completeSpy.findItem(extraFileNameWithSpaces)->_status, SyncFileItem::Status::Success);
 #endif
     }
 

diff --git a/test/testsyncconflictsmodel.cpp b/test/testsyncconflictsmodel.cpp
@@ -24,6 +24,7 @@
 #include <QTest>
 #include <QAbstractItemModelTester>
 #include <QSignalSpy>
+#include <QLocale>
 
 namespace {
 
@@ -47,6 +48,7 @@ class TestSyncConflictsModel : public QObject
     Q_OBJECT
 
 private:
+    QLocale _locale;
 
 private slots:
     void initTestCase()
@@ -104,8 +106,8 @@ private slots:
 
         QCOMPARE(model.rowCount(), 1);
         QCOMPARE(model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ExistingFileName)), QString{"a2"});
-        QCOMPARE(model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ExistingSize)), QString{"6 bytes"});
-        QCOMPARE(model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ConflictSize)), QString{"5 bytes"});
+        QCOMPARE(model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ExistingSize)), _locale.formattedDataSize(6));
+        QCOMPARE(model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ConflictSize)), _locale.formattedDataSize(5));
         QVERIFY(!model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ExistingDate)).toString().isEmpty());
         QVERIFY(!model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ConflictDate)).toString().isEmpty());
         QCOMPARE(model.data(model.index(0), static_cast<int>(SyncConflictsModel::SyncConflictRoles::ExistingPreviewUrl)), QVariant::fromValue(QUrl{QStringLiteral("image://tray-image-provider/:/fileicon%1A/a2").arg(fakeFolder.localPath())}));

diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
@@ -984,15 +984,17 @@ private slots:
             if (op == QNetworkAccessManager::PostOperation) {
                 ++nPOST;
                 if (contentType.startsWith(QStringLiteral("multipart/related; boundary="))) {
-                    auto jsonReplyObject = fakeFolder.forEachReplyPart(outgoingData, contentType, [] (const QMap<QString, QByteArray> &allHeaders) -> QJsonObject {
+                    auto hasAnError = false;
+                    auto jsonReplyObject = fakeFolder.forEachReplyPart(outgoingData, contentType, [&hasAnError] (const QMap<QString, QByteArray> &allHeaders) -> QJsonObject {
                         auto reply = QJsonObject{};
-                        const auto fileName = allHeaders[QStringLiteral("X-File-Path")];
+                        const auto fileName = allHeaders[QStringLiteral("x-file-path")];
                         if (fileName.endsWith("A/big2") ||
                                 fileName.endsWith("A/big3") ||
                                 fileName.endsWith("A/big4") ||
                                 fileName.endsWith("A/big5") ||
                                 fileName.endsWith("A/big7") ||
                                 fileName.endsWith("B/big8")) {
+                            hasAnError = true;
                             reply.insert(QStringLiteral("error"), true);
                             reply.insert(QStringLiteral("etag"), {});
                             return reply;
@@ -1005,7 +1007,11 @@ private slots:
                     if (jsonReplyObject.size()) {
                         auto jsonReply = QJsonDocument{};
                         jsonReply.setObject(jsonReplyObject);
-                        return new FakeJsonErrorReply{op, request, this, 200, jsonReply};
+                        if (hasAnError) {
+                            return new FakeJsonErrorReply{op, request, this, 200, jsonReply};
+                        } else {
+                            return new FakeJsonReply{op, request, this, 200, jsonReply};
+                        }
                     }
                     return  nullptr;
                 }