Skip to content

Commit

Permalink
[buteo-sync-plugin-caldav] Try to complete all successful operations …
Browse files Browse the repository at this point in the history 
…before reporting a failure.
  • Loading branch information
@dcaliste
dcaliste committed Oct 8, 2020
1 parent 70a126e commit cee7059
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 39 deletions.
10 changes: 8 additions & 2 deletions src/caldavclient.cpp
View file
Expand Up @@ -740,8 +740,10 @@ void CalDavClient::notebookSyncFinished(int errorCode, const QString &errorStrin
}
if (finished) {
bool hasDatabaseErrors = false;
bool hasUploadErrors = false;
QStringList deletedNotebooks;
for (int i=0; i<mNotebookSyncAgents.count(); i++) {
hasUploadErrors = hasUploadErrors || mNotebookSyncAgents[i]->hasUploadErrors();
if (!mNotebookSyncAgents[i]->applyRemoteChanges()) {
LOG_WARNING("Unable to write notebook changes for notebook at index:" << i);
hasDatabaseErrors = true;
Expand All @@ -755,10 +757,14 @@ void CalDavClient::notebookSyncFinished(int errorCode, const QString &errorStrin
mNotebookSyncAgents[i]->finalize();
}
removeAccountCalendars(deletedNotebooks);
if (hasDatabaseErrors) {
if (hasUploadErrors) {
mResults = Buteo::SyncResults();
syncFinished(Buteo::SyncResults::CONNECTION_ERROR,
QLatin1String("unable to upsync all local changes"));
} else if (hasDatabaseErrors) {
mResults = Buteo::SyncResults();
syncFinished(Buteo::SyncResults::DATABASE_FAILURE,
QLatin1String("unable to write notebook changes"));
QLatin1String("unable to apply all remote changes"));
} else {
LOG_DEBUG("Calendar storage saved successfully after writing notebook changes!");
syncFinished(Buteo::SyncResults::NO_ERROR);
Expand Down
105 changes: 68 additions & 37 deletions src/notebooksyncagent.cpp
View file
Expand Up @@ -183,6 +183,8 @@ NotebookSyncAgent::NotebookSyncAgent(mKCal::ExtendedCalendar::Ptr calendar,
, mEnableUpsync(true)
, mEnableDownsync(true)
, mReadOnlyFlag(readOnlyFlag)
, mHasUploadErrors(false)
, mHasDownloadErrors(false)
, mHasUpdatedEtags(false)
{
// the calendar path may be percent-encoded. Return UTF-8 QString.
Expand Down Expand Up @@ -348,6 +350,10 @@ void NotebookSyncAgent::reportRequestFinished(const QString &uri)
report->deleteLater();

if (report->errorCode() == Buteo::SyncResults::NO_ERROR) {
// NOTE: we don't store the remote artifacts yet
// Instead, we just emit finished (for this notebook)
// Once ALL notebooks are finished, then we apply the remote changes.
// This prevents the worst partial-sync issues.
mReceivedCalendarResources = report->receivedCalendarResources();
unsigned int count = 0;
for (QList<Reader::CalendarResource>::ConstIterator it = mReceivedCalendarResources.constBegin(); it != mReceivedCalendarResources.constEnd(); ++it) {
Expand All @@ -357,19 +363,11 @@ void NotebookSyncAgent::reportRequestFinished(const QString &uri)
<< report->receivedCalendarResources().length() << "iCal blobs containing a total of"
<< count << "incidences");

if (mSyncMode == QuickSync) {
sendLocalChanges();
return;
if (mSyncMode == SlowSync) {
mResults = Buteo::TargetResults(mNotebook->name().toHtmlEscaped(),
Buteo::ItemCounts(count, 0, 0),
Buteo::ItemCounts());
}

// NOTE: we don't store the remote artifacts yet
// Instead, we just emit finished (for this notebook)
// Once ALL notebooks are finished, then we apply the remote changes.
// This prevents the worst partial-sync issues.
mResults = Buteo::TargetResults(mNotebook->name().toHtmlEscaped(),
Buteo::ItemCounts(count, 0, 0),
Buteo::ItemCounts());

} else if (mSyncMode == SlowSync
&& report->networkError() == QNetworkReply::AuthenticationRequiredError
&& !mRetriedReport) {
Expand All @@ -390,8 +388,15 @@ void NotebookSyncAgent::reportRequestFinished(const QString &uri)
return;
}

LOG_DEBUG("emitting report request finished with result:" << report->errorCode() << report->errorString());
emitFinished(report->errorCode(), report->errorString());
LOG_DEBUG("report request finished with result:" << report->errorCode() << report->errorString());
mHasDownloadErrors = (report->errorCode() != Buteo::SyncResults::NO_ERROR);
if (mSyncMode == QuickSync) {
sendLocalChanges();
return;
}

// Let caller decide what to do if mHasDownloadErrors is true.
emitFinished(Buteo::SyncResults::NO_ERROR);
}

void NotebookSyncAgent::processETags(const QString &uri)
Expand Down Expand Up @@ -489,11 +494,11 @@ void NotebookSyncAgent::sendLocalChanges()
return;
} else if (!mEnableUpsync) {
LOG_DEBUG("Not upsyncing local changes, upsync disable in profile.");
emitFinished(Buteo::SyncResults::NO_ERROR, QString());
emitFinished(Buteo::SyncResults::NO_ERROR);
return;
} else if (mReadOnlyFlag) {
LOG_DEBUG("Not upsyncing local changes, upstream read only calendar.");
emitFinished(Buteo::SyncResults::NO_ERROR, QString());
emitFinished(Buteo::SyncResults::NO_ERROR);
return;
} else {
LOG_DEBUG("upsyncing local changes: A/M/R:" << mLocalAdditions.count() << "/" << mLocalModifications.count() << "/" << mLocalDeletions.count());
Expand Down Expand Up @@ -599,26 +604,41 @@ void NotebookSyncAgent::nonReportRequestFinished(const QString &uri)
QLatin1String("Invalid request object"));
return;
}
request->deleteLater();
mRequests.remove(request);

if (request->errorCode() != Buteo::SyncResults::NO_ERROR) {
LOG_WARNING("Aborting sync," << request->command() << "failed" << request->errorString() << "for notebook" << mRemoteCalendarPath << "of account:" << mNotebook->account());
emitFinished(request->errorCode(), request->errorString());
} else {
Put *putRequest = qobject_cast<Put*>(request);
if (putRequest) {
// Apply Etag and Href changes immediately since incidences are now
// for sure on server.
for (QHash<QString, QString>::ConstIterator
it = putRequest->updatedETags().constBegin();
it != putRequest->updatedETags().constEnd(); ++it) {
if (mSentUids.contains(it.key())) {
updateHrefETag(mSentUids.take(it.key()), it.key(), it.value());
mHasUpdatedEtags = true;
mHasUploadErrors = mHasUploadErrors
|| (request->errorCode() != Buteo::SyncResults::NO_ERROR);

Put *putRequest = qobject_cast<Put*>(request);
if (putRequest) {
if (request->errorCode() == Buteo::SyncResults::NO_ERROR) {
const QString &etag = putRequest->updatedETag(uri);
if (!etag.isEmpty()) {
// Apply Etag and Href changes immediately since incidences are now
// for sure on server.
updateHrefETag(mSentUids.take(uri), uri, etag);
mHasUpdatedEtags = true;
}
} else {
// Don't try to get etag later for a failed upload.
mSentUids.remove(uri);
}
}
Delete *deleteRequest = qobject_cast<Delete*>(request);
if (deleteRequest) {
if (request->errorCode() != Buteo::SyncResults::NO_ERROR) {
// Don't purge yet the locally deleted incidence.
KCalCore::Incidence::List::Iterator it = mPurgeList.begin();
while (it != mPurgeList.end()) {
if (incidenceHrefUri(*it) == uri) {
it = mPurgeList.erase(it);
} else {
++it;
}
}
}
}

if (mRequests.isEmpty()) {
finalizeSendingLocalChanges();
}
Expand Down Expand Up @@ -646,6 +666,7 @@ void NotebookSyncAgent::finalizeSendingLocalChanges()
connect(report, &Report::finished, this, &NotebookSyncAgent::additionalReportRequestFinished);
report->multiGetEvents(mRemoteCalendarPath, mSentUids.keys());
} else {
// Let the caller decide what to do if mHasUploadErrors is true
emitFinished(Buteo::SyncResults::NO_ERROR);
}
}
Expand All @@ -669,22 +690,27 @@ void NotebookSyncAgent::additionalReportRequestFinished(const QString &uri)
LOG_DEBUG("Additional report request finished: received:"
<< report->receivedCalendarResources().count() << "incidences");
mReceivedCalendarResources += report->receivedCalendarResources();
emitFinished(Buteo::SyncResults::NO_ERROR);
} else {
LOG_WARNING("Additional report request finished with error, aborting sync of notebook:" << mRemoteCalendarPath);
emitFinished(report->errorCode(), report->errorString());
LOG_WARNING("Additional report request finished with error.");
mHasDownloadErrors = true;
}
// Let the caller decide what to do if mHasDownloadErrors is true
emitFinished(Buteo::SyncResults::NO_ERROR);
}

bool NotebookSyncAgent::applyRemoteChanges()
{
NOTEBOOK_FUNCTION_CALL_TRACE;

if (mHasDownloadErrors && !mReceivedCalendarResources.size()) {
LOG_DEBUG("Download had errors, nothing to apply.");
return false;
}

if (!mNotebook) {
LOG_DEBUG("Missing notebook in apply changes.");
return false;
}

// mNotebook may not exist in mStorage, because it is new, or
// database has been modified and notebooks been reloaded.
mKCal::Notebook::Ptr notebook(mStorage->notebook(mNotebook->uid()));
Expand All @@ -693,7 +719,7 @@ bool NotebookSyncAgent::applyRemoteChanges()
if (notebook && !mStorage->deleteNotebook(notebook)) {
LOG_WARNING("Cannot delete notebook" << notebook->name() << "from storage.");
}
return true;
return false;
}

// If current notebook is not already in storage, we add it.
Expand All @@ -705,7 +731,7 @@ bool NotebookSyncAgent::applyRemoteChanges()
notebook = mNotebook;
}

bool success = true;
bool success = !mHasDownloadErrors;
// Make notebook writable for the time of the modifications.
notebook->setIsReadOnly(false);
if ((mEnableDownsync || mSyncMode == SlowSync)
Expand Down Expand Up @@ -772,6 +798,11 @@ bool NotebookSyncAgent::isDeleted() const
return (mEnableDownsync && mNotebookNeedsDeletion);
}

bool NotebookSyncAgent::hasUploadErrors() const
{
return mHasUploadErrors;
}

const QString& NotebookSyncAgent::path() const
{
return mEncodedRemotePath;
Expand Down
2 changes: 2 additions & 0 deletions src/notebooksyncagent.h
View file
Expand Up @@ -73,6 +73,7 @@ class NotebookSyncAgent : public QObject

bool isFinished() const;
bool isDeleted() const;
bool hasUploadErrors() const;

const QString& path() const;

Expand Down Expand Up @@ -132,6 +133,7 @@ private slots:
Buteo::TargetResults mResults;
bool mEnableUpsync, mEnableDownsync;
bool mReadOnlyFlag;
bool mHasUploadErrors, mHasDownloadErrors;

// these are used only in quick-sync mode.
// delta detection and change data
Expand Down

0 comments on commit cee7059

Please sign in to comment.