From 9be169c8561d454b356b4bff6331ac4e6e0f6d9e Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 27 Nov 2024 16:26:00 +0100 Subject: [PATCH 1/4] fix regressions in automated tests for bulk upload Signed-off-by: Matthieu Gallien --- src/libsync/bulkpropagatorjob.cpp | 1 + src/libsync/propagateupload.cpp | 5 ++- test/syncenginetestutils.cpp | 57 +++++++++++++++++++++++++++---- test/syncenginetestutils.h | 23 +++++++++++++ test/testsyncengine.cpp | 12 +++++-- 5 files changed, 87 insertions(+), 11 deletions(-) diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp index 58e1ab82f0c95..2d06298fe1088 100644 --- 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 index d486f861da03f..16e68c96b5ac6 100644 --- 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/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp index 39edfc6537abd..d3f351f1c725d 100644 --- a/test/syncenginetestutils.cpp +++ b/test/syncenginetestutils.cpp @@ -552,12 +552,12 @@ QVector FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot auto onePartBody = onePart.mid(headerEndPosition + 4, onePart.size() - headerEndPosition - 6); auto onePartHeaders = onePartHeaderPart.split(QStringLiteral("\r\n")); QMap 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 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 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(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 index dca0eb02b92a4..fee77f1494c89 100644 --- 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/testsyncengine.cpp b/test/testsyncengine.cpp index c46a9f7ab0143..c745ddcad4ffe 100644 --- 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 &allHeaders) -> QJsonObject { + auto hasAnError = false; + auto jsonReplyObject = fakeFolder.forEachReplyPart(outgoingData, contentType, [&hasAnError] (const QMap &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; } From f38c2b7f51de0512a77d4ad814067f8f5b8f21b6 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 28 Nov 2024 11:07:50 +0100 Subject: [PATCH 2/4] fix local discovery test for file names with spaces on windows Signed-off-by: Matthieu Gallien --- test/testlocaldiscovery.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/testlocaldiscovery.cpp b/test/testlocaldiscovery.cpp index bb6bcdc01f374..95c691b75e364 100644 --- 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,7 +401,7 @@ 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); @@ -408,7 +409,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::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 } From 60b1091e138c1fc84789bc99c8e3a7c9852a3127 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 28 Nov 2024 14:09:05 +0100 Subject: [PATCH 3/4] fix TestSyncConflictsModel by using locale aware test data should ensure comparisons of locale aware text is compared against locale aware test data will remove portability issues especially on Windows Signed-off-by: Matthieu Gallien --- test/testsyncconflictsmodel.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/testsyncconflictsmodel.cpp b/test/testsyncconflictsmodel.cpp index fe7ed88e1bc70..fb7b94ab67ff2 100644 --- a/test/testsyncconflictsmodel.cpp +++ b/test/testsyncconflictsmodel.cpp @@ -24,6 +24,7 @@ #include #include #include +#include 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(SyncConflictsModel::SyncConflictRoles::ExistingFileName)), QString{"a2"}); - QCOMPARE(model.data(model.index(0), static_cast(SyncConflictsModel::SyncConflictRoles::ExistingSize)), QString{"6 bytes"}); - QCOMPARE(model.data(model.index(0), static_cast(SyncConflictsModel::SyncConflictRoles::ConflictSize)), QString{"5 bytes"}); + QCOMPARE(model.data(model.index(0), static_cast(SyncConflictsModel::SyncConflictRoles::ExistingSize)), _locale.formattedDataSize(6)); + QCOMPARE(model.data(model.index(0), static_cast(SyncConflictsModel::SyncConflictRoles::ConflictSize)), _locale.formattedDataSize(5)); QVERIFY(!model.data(model.index(0), static_cast(SyncConflictsModel::SyncConflictRoles::ExistingDate)).toString().isEmpty()); QVERIFY(!model.data(model.index(0), static_cast(SyncConflictsModel::SyncConflictRoles::ConflictDate)).toString().isEmpty()); QCOMPARE(model.data(model.index(0), static_cast(SyncConflictsModel::SyncConflictRoles::ExistingPreviewUrl)), QVariant::fromValue(QUrl{QStringLiteral("image://tray-image-provider/:/fileicon%1A/a2").arg(fakeFolder.localPath())})); From 49267b894b9b7e1942fca8342d04d66c3dceeaaa Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 28 Nov 2024 14:38:21 +0100 Subject: [PATCH 4/4] fir regression with VFS availability test on Windows ensure we report the expected availability erro for non existing files Signed-off-by: Matthieu Gallien --- src/libsync/vfs/cfapi/vfs_cfapi.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp index 64779407f2a9f..27f8f945792af 100644 --- 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; } }