Navigation Menu

Skip to content

Commit

Permalink
[buteo-sync-plugin-caldav] Always log calendar changes, don't count e…
Browse files Browse the repository at this point in the history 
…rrors.
  • Loading branch information
@dcaliste
dcaliste committed Oct 8, 2020
1 parent a8f695b commit a03fb87
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 86 deletions.
7 changes: 2 additions & 5 deletions src/caldavclient.cpp
View file
Expand Up @@ -749,25 +749,22 @@ void CalDavClient::notebookSyncFinished()
if (!mNotebookSyncAgents[i]->applyRemoteChanges()) {
LOG_WARNING("Unable to write notebook changes for notebook at index:" << i);
hasDatabaseErrors = true;
} else if (!mNotebookSyncAgents[i]->result().targetName().isEmpty()) {
mResults.addTargetResults(mNotebookSyncAgents[i]->result());
}
if (mNotebookSyncAgents[i]->isDeleted()) {
deletedNotebooks += mNotebookSyncAgents[i]->path();
} else {
mResults.addTargetResults(mNotebookSyncAgents[i]->result());
}
mNotebookSyncAgents[i]->finalize();
}
removeAccountCalendars(deletedNotebooks);
if (hasDownloadErrors) {
mResults = Buteo::SyncResults();
syncFinished(Buteo::SyncResults::CONNECTION_ERROR,
QLatin1String("unable to fetch all upstream changes"));
} else 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 apply all remote changes"));
} else {
Expand Down
153 changes: 75 additions & 78 deletions src/notebooksyncagent.cpp
View file
Expand Up @@ -62,6 +62,10 @@ namespace {
uri = QUrl::fromPercentEncoding(uri.toUtf8());
LOG_DEBUG("URI comment was percent encoded:" << comment << ", returning uri:" << uri);
}
if (uri.isEmpty() && uriNeedsFilling) {
LOG_WARNING("Stored uri was empty for:" << incidence->uid() << incidence->recurrenceId().toString());
return remoteCalendarPath + incidence->uid() + ".ics";
}
return uri;
}
}
Expand Down Expand Up @@ -158,6 +162,32 @@ namespace {
&& (incidence->recurs()
|| incidence->dateTime(KCalCore::Incidence::RoleDisplayEnd).dateTime() >= from);
}

unsigned int countSuccess(const QSet<QString> &failingHrefs,
const KCalCore::Incidence::List &incidences,
const QString &remotePath = QString())
{
unsigned int success = incidences.size();
for (int i = 0; i < incidences.size(); i++) {
bool doit = true;
const QString href = incidenceHrefUri(incidences[i], remotePath, remotePath.isEmpty() ? 0 : &doit);
if (failingHrefs.contains(href)) {
success -= 1;
}
}
return success;
}
unsigned int countSuccess(const QSet<QString> &failingHrefs,
const QList<QString> &incidences)
{
unsigned int success = incidences.size();
for (int i = 0; i < incidences.size(); i++) {
if (failingHrefs.contains(incidences[i])) {
success -= 1;
}
}
return success;
}
}


Expand All @@ -178,12 +208,9 @@ NotebookSyncAgent::NotebookSyncAgent(mKCal::ExtendedCalendar::Ptr calendar,
, mSyncMode(NoSyncMode)
, mRetriedReport(false)
, mNotebookNeedsDeletion(false)
, mResults(QString(), Buteo::ItemCounts(), Buteo::ItemCounts())
, 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 @@ -345,7 +372,7 @@ void NotebookSyncAgent::reportRequestFinished(const QString &uri)

Report *report = qobject_cast<Report*>(sender());
if (!report) {
mHasDownloadErrors = true;
mFailingUpdates.insert(uri);
clearRequests();
emit finished();
return;
Expand All @@ -358,19 +385,8 @@ void NotebookSyncAgent::reportRequestFinished(const QString &uri)
// 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 = report->receivedCalendarResources().constBegin(); it != report->receivedCalendarResources().constEnd(); ++it) {
count += it->incidences.count();
}
LOG_DEBUG("Report request finished: received:"
<< report->receivedCalendarResources().length() << "iCal blobs containing a total of"
<< count << "incidences");

if (mSyncMode == SlowSync) {
mResults = Buteo::TargetResults(mNotebook->name().toHtmlEscaped(),
Buteo::ItemCounts(count, 0, 0),
Buteo::ItemCounts());
}
<< report->receivedCalendarResources().length() << "iCal blobs");
} else if (mSyncMode == SlowSync
&& report->networkError() == QNetworkReply::AuthenticationRequiredError
&& !mRetriedReport) {
Expand All @@ -387,7 +403,8 @@ void NotebookSyncAgent::reportRequestFinished(const QString &uri)
mNotebookNeedsDeletion = true;
LOG_DEBUG("calendar" << uri << "was deleted remotely, skipping sync locally.");
} else {
mHasDownloadErrors = true;
mFailingUpdates += QSet<QString>::fromList(report->fetchedUris());
mFailingUpdates.insert(uri);
}

requestFinished(report);
Expand All @@ -399,7 +416,7 @@ void NotebookSyncAgent::processETags(const QString &uri)

Report *report = qobject_cast<Report*>(sender());
if (!report) {
mHasDownloadErrors = true;
mFailingUpdates.insert(uri);
clearRequests();
emit finished();
return;
Expand All @@ -414,7 +431,7 @@ void NotebookSyncAgent::processETags(const QString &uri)
report->receivedCalendarResources()) {
if (!resource.href.contains(mRemoteCalendarPath)) {
LOG_WARNING("href does not contain server path:" << resource.href << ":" << mRemoteCalendarPath);
mHasDownloadErrors = true;
mFailingUpdates.insert(uri);
clearRequests();
emit finished();
return;
Expand All @@ -431,21 +448,13 @@ void NotebookSyncAgent::processETags(const QString &uri)
&mRemoteModifications,
&mRemoteDeletions)) {
LOG_WARNING("unable to calculate the sync delta for:" << mRemoteCalendarPath);
mHasDownloadErrors = true;
mFailingUpdates.insert(uri);
clearRequests();
emit finished();
return;
}
mResults = Buteo::TargetResults
(mNotebook->name().toHtmlEscaped(),
Buteo::ItemCounts(mRemoteAdditions.size(),
mRemoteDeletions.size(),
mRemoteModifications.size()),
Buteo::ItemCounts(mLocalAdditions.size(),
mLocalDeletions.size(),
mLocalModifications.size()));

QStringList fetchRemoteHrefUris = mRemoteAdditions + mRemoteModifications;

const QStringList fetchRemoteHrefUris = mRemoteAdditions + mRemoteModifications;
if (mEnableDownsync && !fetchRemoteHrefUris.isEmpty()) {
// some incidences have changed on the server, so fetch the new details
sendReportRequest(fetchRemoteHrefUris);
Expand All @@ -464,7 +473,7 @@ void NotebookSyncAgent::processETags(const QString &uri)
mNotebookNeedsDeletion = true;
LOG_DEBUG("calendar" << uri << "was deleted remotely, marking for deletion locally:" << mNotebook->name());
} else {
mHasDownloadErrors = true;
mFailingUpdates.insert(uri);
}

requestFinished(report);
Expand All @@ -474,6 +483,8 @@ void NotebookSyncAgent::sendLocalChanges()
{
NOTEBOOK_FUNCTION_CALL_TRACE;

mFailingUploads.clear();

if (!mLocalAdditions.count() && !mLocalModifications.count() && !mLocalDeletions.count()) {
// no local changes to upsync.
// we're finished syncing.
Expand Down Expand Up @@ -532,11 +543,6 @@ void NotebookSyncAgent::sendLocalChanges()
for (int i = 0; i < toUpload.count(); i++) {
bool create = false;
QString href = incidenceHrefUri(toUpload[i], mRemoteCalendarPath, &create);
if (href.isEmpty()) {
LOG_WARNING("Unable to determine remote uri for incidence:" << toUpload[i]->uid());
mHasUploadErrors = true;
continue;
}
if (mSentUids.contains(href)) {
LOG_DEBUG("Already handled upload" << i << "via series update");
continue; // already handled this one, as a result of a previous update of another occurrence in the series.
Expand All @@ -559,7 +565,7 @@ void NotebookSyncAgent::sendLocalChanges()
}
if (icsData.isEmpty()) {
LOG_DEBUG("Skipping upload of broken incidence:" << i << ":" << toUpload[i]->uid());
mHasUploadErrors = true;
mFailingUploads.insert(href);
} else {
LOG_DEBUG("Uploading incidence" << i << "via PUT for uid:" << toUpload[i]->uid());
Put *put = new Put(mNetworkManager, mSettings);
Expand All @@ -577,13 +583,14 @@ void NotebookSyncAgent::nonReportRequestFinished(const QString &uri)

Request *request = qobject_cast<Request*>(sender());
if (!request) {
mHasUploadErrors = true;
mFailingUploads.insert(uri);
clearRequests();
emit finished();
return;
}
mHasUploadErrors = mHasUploadErrors
|| (request->errorCode() != Buteo::SyncResults::NO_ERROR);
if (request->errorCode() != Buteo::SyncResults::NO_ERROR) {
mFailingUploads.insert(uri);
}

Put *putRequest = qobject_cast<Put*>(request);
if (putRequest) {
Expand Down Expand Up @@ -642,47 +649,14 @@ void NotebookSyncAgent::finalizeSendingLocalChanges()
// been updated with new etag value. Just remains the ones
// that requires additional retrieval to get etag values.
if (!mSentUids.isEmpty()) {
Report *report = new Report(mNetworkManager, mSettings);
mRequests.insert(report);
connect(report, &Report::finished, this, &NotebookSyncAgent::additionalReportRequestFinished);
report->multiGetEvents(mRemoteCalendarPath, mSentUids.keys());
}
}

void NotebookSyncAgent::additionalReportRequestFinished(const QString &uri)
{
Q_UNUSED(uri);
NOTEBOOK_FUNCTION_CALL_TRACE;

// The server did not originally respond with the update ETAG values after
// our initial PUT/UPDATE so we had to do an addition report request.
// This response will contain the new ETAG values for any resource we
// upsynced (ie, a local modification/addition) and also the incidence
// as it may have been modified by the server.

Report *report = qobject_cast<Report*>(sender());

if (report->errorCode() == Buteo::SyncResults::NO_ERROR) {
LOG_DEBUG("Additional report request finished: received:"
<< report->receivedCalendarResources().count() << "incidences");
mReceivedCalendarResources += report->receivedCalendarResources();
} else {
LOG_WARNING("Additional report request finished with error.");
mHasDownloadErrors = true;
sendReportRequest(mSentUids.keys());
}

requestFinished(report);
}

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;
Expand All @@ -708,7 +682,7 @@ bool NotebookSyncAgent::applyRemoteChanges()
notebook = mNotebook;
}

bool success = !mHasDownloadErrors;
bool success = true;
// Make notebook writable for the time of the modifications.
notebook->setIsReadOnly(false);
if ((mEnableDownsync || mSyncMode == SlowSync)
Expand Down Expand Up @@ -743,7 +717,26 @@ bool NotebookSyncAgent::applyRemoteChanges()

Buteo::TargetResults NotebookSyncAgent::result() const
{
return mResults;
if (mSyncMode == SlowSync) {
unsigned int count = 0;
for (QList<Reader::CalendarResource>::ConstIterator it = mReceivedCalendarResources.constBegin(); it != mReceivedCalendarResources.constEnd(); ++it) {
if (!mFailingUpdates.contains(it->href)) {
count += it->incidences.count();
}
}
return Buteo::TargetResults(mNotebook->name().toHtmlEscaped(),
Buteo::ItemCounts(count, 0, 0),
Buteo::ItemCounts());
} else {
return Buteo::TargetResults
(mNotebook->name().toHtmlEscaped(),
Buteo::ItemCounts(countSuccess(mFailingUpdates, mRemoteAdditions),
countSuccess(mFailingUpdates, mRemoteDeletions),
countSuccess(mFailingUpdates, mRemoteModifications)),
Buteo::ItemCounts(countSuccess(mFailingUploads, mLocalAdditions, mRemoteCalendarPath),
countSuccess(mFailingUploads, mLocalDeletions),
countSuccess(mFailingUploads, mLocalModifications)));
}
}

void NotebookSyncAgent::requestFinished(Request *request)
Expand Down Expand Up @@ -775,12 +768,12 @@ bool NotebookSyncAgent::isDeleted() const

bool NotebookSyncAgent::hasDownloadErrors() const
{
return mHasDownloadErrors;
return !mFailingUpdates.isEmpty();
}

bool NotebookSyncAgent::hasUploadErrors() const
{
return mHasUploadErrors;
return !mFailingUploads.isEmpty();
}

const QString& NotebookSyncAgent::path() const
Expand Down Expand Up @@ -1178,6 +1171,7 @@ bool NotebookSyncAgent::updateIncidences(const QList<Reader::CalendarResource> &
}
if (!localBaseIncidence) {
LOG_WARNING("Error saving base incidence of resource" << resource.href);
mFailingUpdates.insert(resource.href);
success = false;
continue; // don't return false and block the entire sync cycle, just ignore this event.
}
Expand All @@ -1198,6 +1192,7 @@ bool NotebookSyncAgent::updateIncidences(const QList<Reader::CalendarResource> &
updateIncidence(remoteInstance, localInstance);
} else if (!addException(remoteInstance, localBaseIncidence, parentIndex == -1)) {
LOG_WARNING("Error saving updated persistent occurrence of resource" << resource.href << ":" << remoteInstance->recurrenceId().toString());
mFailingUpdates.insert(resource.href);
success = false;
continue; // don't return false and block the entire sync cycle, just ignore this event.
}
Expand All @@ -1210,6 +1205,7 @@ bool NotebookSyncAgent::updateIncidences(const QList<Reader::CalendarResource> &
LOG_DEBUG("Now removing remotely-removed persistent occurrence:" << localInstance->recurrenceId().toString());
if (!mCalendar->deleteIncidence(localInstance)) {
LOG_WARNING("Error removing remotely deleted persistent occurrence of resource" << resource.href << ":" << localInstance->recurrenceId().toString());
mFailingUpdates.insert(resource.href);
// don't return here and block the entire sync cycle.
success = false;
}
Expand All @@ -1228,6 +1224,7 @@ bool NotebookSyncAgent::deleteIncidences(const KCalCore::Incidence::List deleted
mStorage->load(doomed->uid(), doomed->recurrenceId());
if (!mCalendar->deleteIncidence(mCalendar->incidence(doomed->uid(), doomed->recurrenceId()))) {
LOG_WARNING("Unable to delete incidence: " << doomed->uid() << doomed->recurrenceId().toString());
mFailingUpdates.insert(incidenceHrefUri(doomed));
success = false;
} else {
LOG_DEBUG("Deleted incidence: " << doomed->uid() << doomed->recurrenceId().toString());
Expand Down
5 changes: 2 additions & 3 deletions src/notebooksyncagent.h
View file
Expand Up @@ -83,7 +83,6 @@ class NotebookSyncAgent : public QObject

private slots:
void reportRequestFinished(const QString &uri);
void additionalReportRequestFinished(const QString &uri);
void nonReportRequestFinished(const QString &uri);
void processETags(const QString &uri);
private:
Expand Down Expand Up @@ -129,10 +128,8 @@ private slots:
SyncMode mSyncMode; // quick (etag-based delta detection) or slow (full report) sync
bool mRetriedReport; // some servers will fail the first request but succeed on second
bool mNotebookNeedsDeletion; // if the calendar was deleted remotely, we will need to delete it locally.
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 All @@ -146,6 +143,8 @@ private slots:
QHash<QString, QString> mSentUids; // Dictionnary of sent (href, uid) made from
// local additions, modifications.
bool mHasUpdatedEtags;
QSet<QString> mFailingUploads; // List of hrefs with upload errors.
QSet<QString> mFailingUpdates; // List of hrefs from which incidences failed to update.

// received remote incidence resource data
QList<Reader::CalendarResource> mReceivedCalendarResources;
Expand Down

0 comments on commit a03fb87

Please sign in to comment.