From 6312f6dddeae5c773b4516f92d3c157404951c76 Mon Sep 17 00:00:00 2001 From: allexzander Date: Tue, 8 Jun 2021 14:56:43 +0300 Subject: [PATCH 1/2] Wipe empty folder when moving a VFS placeholder. Keep a folder if there are hydrated items inside. Signed-off-by: allexzander --- src/libsync/discovery.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 530f67a51..2f52dbcde 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -894,13 +894,24 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( _childModified = true; auto postProcessLocalNew = [item, localEntry, path, this]() { - if (localEntry.isVirtualFile) { + if (localEntry.isVirtualFile || localEntry.isDirectory) { const bool isPlaceHolder = _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local); - if (isPlaceHolder) { + + const bool isNonHydratedVfsFolder = (localEntry.isDirectory + && _discoveryData->_syncOptions._vfs->mode() != Vfs::Off + && *_discoveryData->_syncOptions._vfs->availability(path._local) == VfsItemAvailability::OnlineOnly); + + // must be file placeholder or an online-only folder + if (isPlaceHolder || isNonHydratedVfsFolder) { qCWarning(lcDisco) << "Wiping virtual file without db entry for" << path._local; item->_instruction = CSYNC_INSTRUCTION_REMOVE; item->_direction = SyncFileItem::Down; + _childModified = false; } else { + if (localEntry.isDirectory && !isNonHydratedVfsFolder) { + qCWarning(lcDisco) << "Virtual directory without db entry for" << path._local << "but it is half-hydrated, so, keeping it."; + return; + } qCWarning(lcDisco) << "Virtual file without db entry for" << path._local << "but looks odd, keeping"; item->_instruction = CSYNC_INSTRUCTION_IGNORE; From a3d12a616b589571e5189a97d061c8039484c87b Mon Sep 17 00:00:00 2001 From: allexzander Date: Tue, 8 Jun 2021 17:20:07 +0300 Subject: [PATCH 2/2] Add error message to GUI. Signed-off-by: allexzander --- src/gui/folder.cpp | 7 ++++ src/gui/folder.h | 2 ++ src/gui/tray/ActivityData.h | 1 + src/gui/tray/UserModel.cpp | 60 ++++++++++++++++++++++++++++++++ src/gui/tray/UserModel.h | 3 ++ src/libsync/discovery.cpp | 39 +++++++++++++++------ src/libsync/discoveryphase.h | 2 ++ src/libsync/progressdispatcher.h | 9 +++++ src/libsync/syncengine.cpp | 1 + src/libsync/syncengine.h | 2 ++ 10 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 8ea4ac092..475c313d0 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -105,6 +105,8 @@ Folder::Folder(const FolderDefinition &definition, this, &Folder::slotLogPropagationStart); connect(_engine.data(), &SyncEngine::syncError, this, &Folder::slotSyncError); + connect(_engine.data(), &SyncEngine::addErrorToGui, this, &Folder::slotAddErrorToGui); + _scheduleSelfTimer.setSingleShot(true); _scheduleSelfTimer.setInterval(SyncEngine::minimumFileAgeForUpload); connect(&_scheduleSelfTimer, &QTimer::timeout, @@ -936,6 +938,11 @@ void Folder::slotSyncError(const QString &message, ErrorCategory category) emit ProgressDispatcher::instance()->syncError(alias(), message, category); } +void Folder::slotAddErrorToGui(SyncFileItem::Status status, const QString &errorMessage, const QString &subject) +{ + emit ProgressDispatcher::instance()->addErrorToGui(alias(), status, errorMessage, subject); +} + void Folder::slotSyncStarted() { qCInfo(lcFolder) << "#### Propagation start ####################################################"; diff --git a/src/gui/folder.h b/src/gui/folder.h index f5ca8c2e0..809ca85e7 100644 --- a/src/gui/folder.h +++ b/src/gui/folder.h @@ -370,6 +370,8 @@ private slots: */ void slotSyncError(const QString &message, ErrorCategory category = ErrorCategory::Normal); + void slotAddErrorToGui(SyncFileItem::Status status, const QString &errorMessage, const QString &subject = {}); + void slotTransmissionProgress(const ProgressInfo &pi); void slotItemCompleted(const SyncFileItemPtr &); diff --git a/src/gui/tray/ActivityData.h b/src/gui/tray/ActivityData.h index f8bbef101..f5d31e32c 100644 --- a/src/gui/tray/ActivityData.h +++ b/src/gui/tray/ActivityData.h @@ -71,6 +71,7 @@ public: QString _file; QUrl _link; QDateTime _dateTime; + qint64 _expireAtMsecs = -1; QString _accName; QString _icon; QString _iconData; diff --git a/src/gui/tray/UserModel.cpp b/src/gui/tray/UserModel.cpp index 49b38bc27..aa36c0d9b 100644 --- a/src/gui/tray/UserModel.cpp +++ b/src/gui/tray/UserModel.cpp @@ -24,6 +24,11 @@ // refreshes of the notifications #define NOTIFICATION_REQUEST_FREE_PERIOD 15000 +namespace { + constexpr qint64 expiredActivitiesCheckIntervalMsecs = 1000 * 60; + constexpr qint64 activityDefaultExpirationTimeMsecs = 1000 * 60 * 10; +} + namespace OCC { User::User(AccountStatePtr &account, const bool &isCurrent, QObject *parent) @@ -39,10 +44,15 @@ User::User(AccountStatePtr &account, const bool &isCurrent, QObject *parent) this, &User::slotItemCompleted); connect(ProgressDispatcher::instance(), &ProgressDispatcher::syncError, this, &User::slotAddError); + connect(ProgressDispatcher::instance(), &ProgressDispatcher::addErrorToGui, + this, &User::slotAddErrorToGui); connect(&_notificationCheckTimer, &QTimer::timeout, this, &User::slotRefresh); + connect(&_expiredActivitiesCheckTimer, &QTimer::timeout, + this, &User::slotCheckExpiredActivities); + connect(_account.data(), &AccountState::stateChanged, [=]() { if (isConnected()) {slotRefreshImmediately();} }); connect(_account.data(), &AccountState::stateChanged, this, &User::accountStateChanged); @@ -145,6 +155,19 @@ void User::slotReceivedPushActivity(Account *account) } } +void User::slotCheckExpiredActivities() +{ + for (const Activity &activity : _activityModel->errorsList()) { + if (activity._expireAtMsecs > 0 && QDateTime::currentDateTime().toMSecsSinceEpoch() >= activity._expireAtMsecs) { + _activityModel->removeActivityFromActivityList(activity); + } + } + + if (_activityModel->errorsList().size() == 0) { + _expiredActivitiesCheckTimer.stop(); + } +} + void User::connectPushNotifications() const { connect(_account->account().data(), &Account::pushNotificationsDisabled, this, &User::slotDisconnectPushNotifications, Qt::UniqueConnection); @@ -328,6 +351,10 @@ void User::slotProgressInfo(const QString &folder, const ProgressInfo &progress) const auto &engine = f->syncEngine(); const auto style = engine.lastLocalDiscoveryStyle(); foreach (Activity activity, _activityModel->errorsList()) { + if (activity._expireAtMsecs != -1) { + // we process expired activities in a different slot + continue; + } if (activity._folder != folder) { continue; } @@ -417,6 +444,39 @@ void User::slotAddError(const QString &folderAlias, const QString &message, Erro } } +void User::slotAddErrorToGui(const QString &folderAlias, SyncFileItem::Status status, const QString &errorMessage, const QString &subject) +{ + const auto folderInstance = FolderMan::instance()->folder(folderAlias); + if (!folderInstance) { + return; + } + + if (folderInstance->accountState() == _account.data()) { + qCWarning(lcActivity) << "Item " << folderInstance->shortGuiLocalPath() << " retrieved resulted in " << errorMessage; + + Activity activity; + activity._type = Activity::SyncFileItemType; + activity._status = status; + const auto currentDateTime = QDateTime::currentDateTime(); + activity._dateTime = QDateTime::fromString(currentDateTime.toString(), Qt::ISODate); + activity._expireAtMsecs = currentDateTime.addMSecs(activityDefaultExpirationTimeMsecs).toMSecsSinceEpoch(); + activity._subject = !subject.isEmpty() ? subject : folderInstance->shortGuiLocalPath(); + activity._message = errorMessage; + activity._link = folderInstance->shortGuiLocalPath(); + activity._accName = folderInstance->accountState()->account()->displayName(); + activity._folder = folderAlias; + + // add 'other errors' to activity list + _activityModel->addErrorToActivityList(activity); + + showDesktopNotification(activity._subject, activity._message); + + if (!_expiredActivitiesCheckTimer.isActive()) { + _expiredActivitiesCheckTimer.start(expiredActivitiesCheckIntervalMsecs); + } + } +} + bool User::isActivityOfCurrentAccount(const Folder *folder) const { return folder->accountState() == _account.data(); diff --git a/src/gui/tray/UserModel.h b/src/gui/tray/UserModel.h index ef3b1058c..2e57d0844 100644 --- a/src/gui/tray/UserModel.h +++ b/src/gui/tray/UserModel.h @@ -75,6 +75,7 @@ public slots: void slotItemCompleted(const QString &folder, const SyncFileItemPtr &item); void slotProgressInfo(const QString &folder, const ProgressInfo &progress); void slotAddError(const QString &folderAlias, const QString &message, ErrorCategory category); + void slotAddErrorToGui(const QString &folderAlias, SyncFileItem::Status status, const QString &errorMessage, const QString &subject = {}); void slotNotificationRequestFinished(int statusCode); void slotNotifyNetworkError(QNetworkReply *reply); void slotEndNotificationRequest(int replyCode); @@ -94,6 +95,7 @@ private: void slotDisconnectPushNotifications(); void slotReceivedPushNotification(Account *account); void slotReceivedPushActivity(Account *account); + void slotCheckExpiredActivities(); void connectPushNotifications() const; bool checkPushNotificationsAreReady() const; @@ -109,6 +111,7 @@ private: ActivityListModel *_activityModel; ActivityList _blacklistedNotifications; + QTimer _expiredActivitiesCheckTimer; QTimer _notificationCheckTimer; QHash _timeSinceLastCheck; diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 2f52dbcde..cb9fb7ab3 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -894,22 +894,41 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo( _childModified = true; auto postProcessLocalNew = [item, localEntry, path, this]() { - if (localEntry.isVirtualFile || localEntry.isDirectory) { - const bool isPlaceHolder = _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local); + // TODO: We may want to execute the same logic for non-VFS mode, as, moving/renaming the same folder by 2 or more clients at the same time is not possible in Web UI. + // Keeping it like this (for VFS files and folders only) just to fix a user issue. + const auto isVfsEnabled = _discoveryData->_syncOptions._vfs && _discoveryData->_syncOptions._vfs->mode() != Vfs::Off; + if (localEntry.isVirtualFile || (localEntry.isDirectory && isVfsEnabled)) { + // must be a dehydrated placeholder + const bool isFilePlaceHolder = !localEntry.isDirectory && _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local); - const bool isNonHydratedVfsFolder = (localEntry.isDirectory - && _discoveryData->_syncOptions._vfs->mode() != Vfs::Off - && *_discoveryData->_syncOptions._vfs->availability(path._local) == VfsItemAvailability::OnlineOnly); + // a folder must be online-only (no files should be hydrated) + const bool isFolderPlaceholder = localEntry.isDirectory && *_discoveryData->_syncOptions._vfs->availability(path._local) == VfsItemAvailability::OnlineOnly; - // must be file placeholder or an online-only folder - if (isPlaceHolder || isNonHydratedVfsFolder) { - qCWarning(lcDisco) << "Wiping virtual file without db entry for" << path._local; + Q_ASSERT(item->_instruction == CSYNC_INSTRUCTION_NEW); + if (item->_instruction != CSYNC_INSTRUCTION_NEW) { + qCWarning(lcDisco) << "Wiping virtual file without db entry for" << path._local << ". But, item->_instruction is" << item->_instruction; + } + + // must be a file placeholder or an online-only folder placeholder + if (isFilePlaceHolder || isFolderPlaceholder) { + if (isFolderPlaceholder) { + qCInfo(lcDisco) << "Wiping virtual folder without db entry for" << path._local; + } else { + qCInfo(lcDisco) << "Wiping virtual file without db entry for" << path._local; + } item->_instruction = CSYNC_INSTRUCTION_REMOVE; item->_direction = SyncFileItem::Down; + // this flag needs to be unset, otherwise a folder would get marked as new in the processSubJobs _childModified = false; + if (isFolderPlaceholder && _discoveryData) { + emit _discoveryData->addErrorToGui(SyncFileItem::SoftError, tr("Conflict when uploading a folder. It's going to get cleared!"), path._local); + } } else { - if (localEntry.isDirectory && !isNonHydratedVfsFolder) { - qCWarning(lcDisco) << "Virtual directory without db entry for" << path._local << "but it is half-hydrated, so, keeping it."; + if (localEntry.isDirectory && !isFolderPlaceholder) { + qCInfo(lcDisco) << "Virtual directory without db entry for" << path._local << "but it contains hydrated file(s), so let's keep it and reupload."; + if (_discoveryData) { + emit _discoveryData->addErrorToGui(SyncFileItem::SoftError, tr("Conflict when uploading some files to a folder. Those, conflicted, are going to get cleared!"), path._local); + } return; } qCWarning(lcDisco) << "Virtual file without db entry for" << path._local diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h index 7e31389c2..9868e3ec6 100644 --- a/src/libsync/discoveryphase.h +++ b/src/libsync/discoveryphase.h @@ -279,6 +279,8 @@ signals: * The path is relative to the sync folder, similar to item->_file */ void silentlyExcluded(const QString &folderPath); + + void addErrorToGui(SyncFileItem::Status status, const QString &errorMessage, const QString &subject); }; /// Implementation of DiscoveryPhase::adjustRenamedPath diff --git a/src/libsync/progressdispatcher.h b/src/libsync/progressdispatcher.h index f2e7ae626..99415d544 100644 --- a/src/libsync/progressdispatcher.h +++ b/src/libsync/progressdispatcher.h @@ -290,6 +290,15 @@ signals: */ void syncError(const QString &folder, const QString &message, ErrorCategory category); + /** + * @brief Emitted when an error needs to be added into GUI + * @param[out] folder The folder which is being processed + * @param[out] status of the error + * @param[out] full error message + * @param[out] subject (optional) + */ + void addErrorToGui(const QString &folder, SyncFileItem::Status status, const QString &errorMessage, const QString &subject); + /** * @brief Emitted for a folder when a sync is done, listing all pending conflicts */ diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 0751a7b66..7f480913d 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -592,6 +592,7 @@ void SyncEngine::startSync() _discoveryPhase.data(), PinState::AlwaysLocal, _journal->keyValueStoreGetInt("last_sync", 0), _discoveryPhase.data()); _discoveryPhase->startJob(discoveryJob); connect(discoveryJob, &ProcessDirectoryJob::etag, this, &SyncEngine::slotRootEtagReceived); + connect(_discoveryPhase.data(), &DiscoveryPhase::addErrorToGui, this, &SyncEngine::addErrorToGui); } void SyncEngine::slotFolderDiscovered(bool local, const QString &folder) diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h index 5bc8dd802..a9e50a197 100644 --- a/src/libsync/syncengine.h +++ b/src/libsync/syncengine.h @@ -151,6 +151,8 @@ signals: /// We've produced a new sync error of a type. void syncError(const QString &message, ErrorCategory category = ErrorCategory::Normal); + void addErrorToGui(SyncFileItem::Status status, const QString &errorMessage, const QString &subject); + void finished(bool success); void started();