Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable-3.15] fix regressions in automated tests for bulk upload #7588

Merged
merged 4 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libsync/bulkpropagatorjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/libsync/propagateupload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* for more details.
*/

#include "config.h"

Check failure on line 15 in src/libsync/propagateupload.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/propagateupload.cpp:15:10 [clang-diagnostic-error]

'config.h' file not found
#include "propagateupload.h"
#include "propagateuploadencrypted.h"
#include "owncloudpropagator_p.h"
Expand Down Expand Up @@ -532,7 +532,10 @@
}

auto c = _file.read(data, maxlen);
if (c < 0) {
if (c == 0) {
setErrorString({});
return c;
} else if (c < 0) {
setErrorString(_file.errorString());
return -1;
}
Expand Down
4 changes: 3 additions & 1 deletion src/libsync/vfs/cfapi/vfs_cfapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -382,6 +382,8 @@ Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath, const
}
}
return AvailabilityError::NoSuchItem;
} else {
return AvailabilityError::NoSuchItem;
}
}

Expand Down
57 changes: 50 additions & 7 deletions test/syncenginetestutils.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on test/syncenginetestutils.cpp

File test/syncenginetestutils.cpp does not conform to Custom style guidelines. (lines 555)
* This software is in the public domain, furnished "as is", without technical
* support, and with no warranty, express or implied, as to its usefulness for
* any purpose.
Expand Down Expand Up @@ -552,12 +552,12 @@
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);
Expand All @@ -568,7 +568,7 @@
// 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);
}
Expand Down Expand Up @@ -1052,13 +1052,13 @@
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);
}
}

Expand Down Expand Up @@ -1413,3 +1413,46 @@
xml.writeEndElement(); // prop
xml.writeEndDocument();
}

FakeJsonReply::FakeJsonReply(QNetworkAccessManager::Operation op,

Check warning on line 1417 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1417:1 [cppcoreguidelines-pro-type-member-init]

constructor does not initialize these fields: , _body

Check warning on line 1417 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1417:30 [bugprone-easily-swappable-parameters]

2 adjacent parameters of 'FakeJsonReply' of similar type are easily swapped by mistake

Check warning on line 1417 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1417:63 [readability-identifier-length]

parameter name 'op' is too short, expected at least 3 characters
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()

Check warning on line 1433 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1433:21 [readability-convert-member-functions-to-static]

method 'respond' can be made static
{
emit metaDataChanged();

Check warning on line 1435 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1435:10 [modernize-use-trailing-return-type]

use a trailing return type for this function
emit readyRead();

Check warning on line 1436 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1436:10 [modernize-use-trailing-return-type]

use a trailing return type for this function
// finishing can come strictly after readyRead was called
QTimer::singleShot(5, this, &FakeJsonReply::slotSetFinished);
}

void FakeJsonReply::slotSetFinished()

Check warning on line 1441 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1441:21 [readability-convert-member-functions-to-static]

method 'slotSetFinished' can be made static
{
setFinished(true);
emit finished();

Check warning on line 1444 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1444:10 [modernize-use-trailing-return-type]

use a trailing return type for this function
}

qint64 FakeJsonReply::readData(char *buf, qint64 max)

Check warning on line 1447 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1447:23 [modernize-use-trailing-return-type]

use a trailing return type for this function
{
max = qMin<qint64>(max, _body.size());
memcpy(buf, _body.constData(), max);
_body = _body.mid(max);
return max;
}

qint64 FakeJsonReply::bytesAvailable() const

Check warning on line 1455 in test/syncenginetestutils.cpp

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.cpp:1455:23 [modernize-use-trailing-return-type]

use a trailing return type for this function
{
return _body.size();
}
23 changes: 23 additions & 0 deletions test/syncenginetestutils.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in test/syncenginetestutils.h

View workflow job for this annotation

GitHub Actions / build

Run clang-format on test/syncenginetestutils.h

File test/syncenginetestutils.h does not conform to Custom style guidelines. (lines 233)
* This software is in the public domain, furnished "as is", without technical
* support, and with no warranty, express or implied, as to its usefulness for
* any purpose.
Expand All @@ -6,7 +6,7 @@
*/
#pragma once

#include "account.h"

Check failure on line 9 in test/syncenginetestutils.h

View workflow job for this annotation

GitHub Actions / build

test/syncenginetestutils.h:9:10 [clang-diagnostic-error]

'account.h' file not found
#include "common/result.h"
#include "creds/abstractcredentials.h"
#include "logger.h"
Expand Down Expand Up @@ -214,6 +214,29 @@
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
Expand Down
11 changes: 6 additions & 5 deletions test/testlocaldiscovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

#include <QtTest>

Check failure on line 8 in test/testlocaldiscovery.cpp

View workflow job for this annotation

GitHub Actions / build

test/testlocaldiscovery.cpp:8:10 [clang-diagnostic-error]

'QtTest' file not found
#include "syncenginetestutils.h"
#include <syncengine.h>
#include <localdiscoverytracker.h>
Expand Down Expand Up @@ -378,6 +378,7 @@
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);
Expand All @@ -386,7 +387,7 @@
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();
Expand All @@ -400,15 +401,15 @@
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);
Expand All @@ -417,7 +418,7 @@
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();

Expand All @@ -431,7 +432,7 @@
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
}

Expand Down
6 changes: 4 additions & 2 deletions test/testsyncconflictsmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* for more details.
*/

#include "gui/syncconflictsmodel.h"

Check failure on line 15 in test/testsyncconflictsmodel.cpp

View workflow job for this annotation

GitHub Actions / build

test/testsyncconflictsmodel.cpp:15:10 [clang-diagnostic-error]

'gui/syncconflictsmodel.h' file not found
#include "folderman.h"
#include "accountstate.h"
#include "configfile.h"
Expand All @@ -24,6 +24,7 @@
#include <QTest>
#include <QAbstractItemModelTester>
#include <QSignalSpy>
#include <QLocale>

namespace {

Expand All @@ -47,6 +48,7 @@
Q_OBJECT

private:
QLocale _locale;

private slots:
void initTestCase()
Expand Down Expand Up @@ -104,8 +106,8 @@

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())}));
Expand Down
12 changes: 9 additions & 3 deletions test/testsyncengine.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/*

Check notice on line 1 in test/testsyncengine.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on test/testsyncengine.cpp

File test/testsyncengine.cpp does not conform to Custom style guidelines. (lines 988, 990, 997)
* This software is in the public domain, furnished "as is", without technical
* support, and with no warranty, express or implied, as to its usefulness for
* any purpose.
*
*/

#include <QtTest>

Check failure on line 8 in test/testsyncengine.cpp

View workflow job for this annotation

GitHub Actions / build

test/testsyncengine.cpp:8:10 [clang-diagnostic-error]

'QtTest' file not found
#include <QTextCodec>

#include "syncenginetestutils.h"
Expand Down Expand Up @@ -984,15 +984,17 @@
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;
Expand All @@ -1005,7 +1007,11 @@
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;
}
Expand Down
Loading