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

Emit finished signal if a sync was aborted #11396

Merged
merged 3 commits into from
Nov 24, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/11396
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: Schedule syncs after an abort

We fixed a bug where the client stopped scheduling syncs after a sync was aborted.

https://github.com/owncloud/client/pull/11396
14 changes: 11 additions & 3 deletions src/libsync/abstractnetworkjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,11 @@ void AbstractNetworkJob::sendRequest(const QByteArray &verb,
void AbstractNetworkJob::adoptRequest(QPointer<QNetworkReply> reply)
{
std::swap(_reply, reply);
delete reply;
if (reply) {
reply->disconnect();
reply->abort();
reply->deleteLater();
}

_request = _reply->request();

Expand Down Expand Up @@ -301,8 +305,12 @@ AbstractNetworkJob::~AbstractNetworkJob()
if (!_finished && !_aborted && !_timedout) {
qCCritical(lcNetworkJob) << "Deleting running job" << this;
}
delete _reply;
_reply = nullptr;
if (_reply) {
_reply->disconnect();
_reply->abort();
_reply->deleteLater();
_reply.clear();
}
}

void AbstractNetworkJob::start()
Expand Down
11 changes: 6 additions & 5 deletions src/libsync/syncengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -857,18 +857,19 @@ bool SyncEngine::shouldDiscoverLocally(const QString &path) const

void SyncEngine::abort()
{
if (_propagator)
qCInfo(lcEngine) << "Aborting sync";

bool aborting = false;
if (_propagator) {
aborting = true;
// If we're already in the propagation phase, aborting that is sufficient
_propagator->abort();
} else if (_discoveryPhase) {
aborting = true;
// Delete the discovery and all child jobs after ensuring
// it can't finish and start the propagator
disconnect(_discoveryPhase.get(), nullptr, this, nullptr);
_discoveryPhase.release()->deleteLater();
TheOneRing marked this conversation as resolved.
Show resolved Hide resolved

}
if (aborting) {
qCInfo(lcEngine) << "Aborting sync";
if (!_goingDown) {
Q_EMIT syncError(tr("Aborted"));
}
Expand Down
19 changes: 17 additions & 2 deletions test/testchunkingng.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,10 @@ private slots:
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
}

// Check what happens when we abort during the final MOVE and the
// the final MOVE is short enough for the abort-delay to help
// Check what happens when we abort during the final MOVE.
// The move succeeds on the server but as we abort it we won't realize in time.
// This means that the current sync fails, as it is aborted, but the MOVE was still performed on the server.
// Resulting in the local state being the same as the remote state.
void testLateAbortRecoverable()
{
FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
Expand All @@ -344,18 +346,31 @@ private slots:
fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * {
if (request.attribute(QNetworkRequest::CustomVerbAttribute).toByteArray() == "MOVE") {
QTimer::singleShot(50ms, &parent, [&]() { fakeFolder.syncEngine().abort(); });
// while the response is delayed, the move is performed in the constructor, thus it happens immediately
return new DelayedReply<FakeChunkMoveReply>(responseDelay, fakeFolder.uploadState(), fakeFolder.remoteModifier(), op, request, &parent);
}
return nullptr;
});

// Test 1: NEW file aborted
fakeFolder.localModifier().insert(QStringLiteral("A/a0"), size);
// the sync will be aborted and thus fail
QVERIFY(!fakeFolder.applyLocalModificationsAndSync());
// the move was still performed
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
fmoc marked this conversation as resolved.
Show resolved Hide resolved

// update the meta data after the aborted sync
QVERIFY(fakeFolder.applyLocalModificationsAndSync());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

// Test 2: modified file upload aborted
fakeFolder.localModifier().appendByte(QStringLiteral("A/a0"));
// the sync will be aborted and thus fail
QVERIFY(!fakeFolder.applyLocalModificationsAndSync());
// the move was still performed
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

// update the meta data after the aborted sync
QVERIFY(fakeFolder.applyLocalModificationsAndSync());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
}
Expand Down