From f1ce0a1cf353438270e07dd8dd54b9e125b05df8 Mon Sep 17 00:00:00 2001 From: Markus Goetz Date: Mon, 13 Oct 2014 17:23:42 +0200 Subject: [PATCH 1/3] SocketAPI: Use non-recursive (heuristic) computation of icons --- src/mirall/folder.cpp | 94 ++++++++++++++++++++++ src/mirall/folder.h | 16 ++-- src/mirall/folderman.cpp | 21 +++-- src/mirall/folderwatcher.cpp | 2 +- src/mirall/socketapi.cpp | 148 ++++++++++++++--------------------- src/mirall/syncengine.cpp | 14 +++- src/mirall/syncengine.h | 7 ++ src/mirall/syncfileitem.h | 7 ++ src/mirall/utility.cpp | 12 +++ src/mirall/utility.h | 4 + 10 files changed, 225 insertions(+), 100 deletions(-) diff --git a/src/mirall/folder.cpp b/src/mirall/folder.cpp index cf85dc5e6d..1e3816cb61 100644 --- a/src/mirall/folder.cpp +++ b/src/mirall/folder.cpp @@ -512,11 +512,90 @@ QString Folder::configFile() return _configFile; } +static void addErroredSyncItemPathsToList(const SyncFileItemVector& items, QSet* set) { + Q_FOREACH(const SyncFileItem &item, items) { + if (item.hasErrorStatus()) { + set->insert(item._file); + } + } +} + void Folder::slotThreadTreeWalkResult(const SyncFileItemVector& items) { + addErroredSyncItemPathsToList(items, &this->_stateLastSyncItemsWithError); _syncResult.setSyncFileItemVector(items); } +void Folder::slotAboutToPropagate(const SyncFileItemVector& items) +{ + // empty the tainted list since the status generation code will use the _syncedItems + // (which imply the folder) to generate the syncing state icon now. + _stateTaintedFolders.clear(); + + addErroredSyncItemPathsToList(items, &this->_stateLastSyncItemsWithError); +} + + +bool Folder::estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s) +{ + if (t == CSYNC_FTW_TYPE_DIR) { + qDebug() << Q_FUNC_INFO << "ASKING ERROR FOLDERS" << fn; + if (Utility::doesSetContainPrefix(_stateLastSyncItemsWithError, fn)) { + s->set(SyncFileStatus::STATUS_ERROR); + return true; + } + // If sync is running, check _syncedItems, possibly give it STATUS_EVAL (=syncing down) + if (!_engine.isNull()) { + qDebug() << Q_FUNC_INFO << "SYNC IS RUNNING, asking SyncEngine" << fn; + if (_engine->estimateState(fn, t, s)) { + return true; + } + } + qDebug() << Q_FUNC_INFO << "ASKING TAINTED FOLDERS" << fn; + if (Utility::doesSetContainPrefix(_stateTaintedFolders, fn)) { + qDebug() << Q_FUNC_INFO << "Folder is tainted, EVAL!" << fn; + s->set(SyncFileStatus::STATUS_EVAL); + return true; + } + return false; + } else if ( t== CSYNC_FTW_TYPE_FILE) { + // check if errorList has the directory/file + if (Utility::doesSetContainPrefix(_stateLastSyncItemsWithError, fn)) { + s->set(SyncFileStatus::STATUS_ERROR); + return true; + } + // If sync running: _syncedItems -> SyncingState + if (!_engine.isNull()) { + if (_engine->estimateState(fn, t, s)) { + return true; + } + } + } + return false; +} + +void Folder::watcherSlot(QString fn) +{ + // FIXME: On OS X we could not do this "if" since on OS X the file watcher ignores events for ourselves + // however to have the same behaviour atm on all platforms, we don't do it + if (!_engine.isNull()) { + qDebug() << Q_FUNC_INFO << "Sync running, IGNORE event for " << fn; + return; + } + QFileInfo fi(fn); + if (fi.isFile()) { + fn = fi.path(); // depending on OS, file watcher might be for dir or file + } + // Make it a relative path depending on the folder + QString relativePath = fn.remove(0, path().length()); + qDebug() << Q_FUNC_INFO << fi.canonicalFilePath() << fn << relativePath; + _stateTaintedFolders.insert(relativePath); + + // Notify the SocketAPI? +} + + + void Folder::slotTerminateSync() { qDebug() << "folder " << alias() << " Terminating!"; @@ -639,6 +718,8 @@ void Folder::startSync(const QStringList &pathList) connect( _engine.data(), SIGNAL(treeWalkResult(const SyncFileItemVector&)), this, SLOT(slotThreadTreeWalkResult(const SyncFileItemVector&)), Qt::QueuedConnection); + connect( _engine.data(), SIGNAL(aboutToPropagate(const SyncFileItemVector&)), + this, SLOT(slotAboutToPropagate(const SyncFileItemVector&)), Qt::QueuedConnection); connect(_engine.data(), SIGNAL(started()), SLOT(slotSyncStarted()), Qt::QueuedConnection); connect(_engine.data(), SIGNAL(finished()), SLOT(slotSyncFinished()), Qt::QueuedConnection); @@ -720,6 +801,15 @@ void Folder::slotSyncFinished() // _watcher->setEventsEnabledDelayed(2000); + // This is for sync state calculation + _stateLastSyncItemsWithError = _stateLastSyncItemsWithErrorNew; + _stateLastSyncItemsWithErrorNew.clear(); + _stateTaintedFolders.clear(); // heuristic: assume the sync had been done, new file watches needed to taint dirs + if (_csyncError || _csyncUnavail) { + // Taint the whole sync dir, we cannot give reliable state information + _stateTaintedFolders.insert(QLatin1String("/")); + } + if (_csyncError) { _syncResult.setStatus(SyncResult::Error); @@ -786,6 +876,10 @@ void Folder::slotTransmissionProgress(const Progress::Info &pi) // a job is completed: count the errors and forward to the ProgressDispatcher void Folder::slotJobCompleted(const SyncFileItem &item) { + if (item.hasErrorStatus()) { + _stateLastSyncItemsWithError.insert(item._file); + } + if (Progress::isWarningKind(item._status)) { // Count all error conditions. _syncResult.setWarnCount(_syncResult.warnCount()+1); diff --git a/src/mirall/folder.h b/src/mirall/folder.h index af96511983..c09863e778 100644 --- a/src/mirall/folder.h +++ b/src/mirall/folder.h @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -34,15 +35,12 @@ #include #include -class QFileSystemWatcher; class QThread; namespace Mirall { class SyncEngine; -class FolderWatcher; - class Folder : public QObject { Q_OBJECT @@ -119,12 +117,12 @@ public: // Used by the Socket API SyncJournalDb *journalDb() { return &_journal; } - CSYNC *csyncContext() { return _csync_ctx; } QStringList selectiveSyncBlackList() { return _selectiveSyncBlackList; } void setSelectiveSyncBlackList(const QStringList &blackList) { _selectiveSyncBlackList = blackList; } + bool estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s); signals: void syncStateChange(); @@ -170,10 +168,13 @@ private slots: void etagRetreived(const QString &); void slotNetworkUnavailable(); - void slotThreadTreeWalkResult(const SyncFileItemVector& ); + void slotAboutToPropagate(const SyncFileItemVector& ); + void slotThreadTreeWalkResult(const SyncFileItemVector& ); // after sync is done void slotEmitFinishedDelayed(); + void watcherSlot(QString); + private: bool init(); @@ -204,6 +205,11 @@ private: QElapsedTimer _timeSinceLastSync; bool _forceSyncOnPollTimeout; + // For the SocketAPI folder states + QSet _stateLastSyncItemsWithErrorNew; // gets moved to _stateLastSyncItemsWithError at end of sync + QSet _stateLastSyncItemsWithError; + QSet _stateTaintedFolders; + SyncJournalDb _journal; ClientProxy _clientProxy; diff --git a/src/mirall/folderman.cpp b/src/mirall/folderman.cpp index 5c3a9394f8..6e7b8dbd0f 100644 --- a/src/mirall/folderman.cpp +++ b/src/mirall/folderman.cpp @@ -147,6 +147,9 @@ void FolderMan::registerFolderMonitor( Folder *folder ) connect(fw, SIGNAL(folderChanged(QString)), _folderWatcherSignalMapper, SLOT(map())); _folderWatcherSignalMapper->setMapping(fw, folder->alias()); _folderWatchers.insert(folder->alias(), fw); + + // This is at the moment only for the behaviour of the SocketApi. + connect(fw, SIGNAL(folderChanged(QString)), folder, SLOT(watcherSlot(QString))); } // register the folder with the socket API @@ -437,19 +440,27 @@ void FolderMan::slotScheduleAllFolders() */ void FolderMan::slotScheduleSync( const QString& alias ) { + if( alias.isEmpty() || ! _folderMap.contains(alias) ) { + qDebug() << "Not scheduling sync for empty or unknown folder" << alias; + return; + } + // The folder watcher fires a lot of bogus notifications during // a sync operation, both for actual user files and the database // and log. Never enqueue a folder for sync while it is syncing. // We lose some genuine sync requests that way, but that can't be // helped. + // ^^ FIXME: Note that this is not the case on OS X if( _currentSyncFolder == alias ) { qDebug() << "folder " << alias << " is currently syncing. NOT scheduling."; return; } - if( alias.isEmpty() || ! _folderMap.contains(alias) ) { - qDebug() << "Not scheduling sync for empty or unknown folder" << alias; - return; + if( _socketApi ) { + // We want the SocketAPI to already now update so that it can show the EVAL icon + // for files/folders. Only do this when not syncing, else we might get a lot + // of those notifications. + _socketApi->slotUpdateFolderView(alias); } qDebug() << "Schedule folder " << alias << " to sync!"; @@ -578,11 +589,11 @@ Folder *FolderMan::folderForPath(const QString &path) const QString folderPath = QDir::cleanPath(folder->path())+QLatin1Char('/'); if(absolutePath.startsWith(folderPath)) { - qDebug() << "found folder: " << folder->path() << " for " << absolutePath; + //qDebug() << "found folder: " << folder->path() << " for " << absolutePath; return folder; } } - + qDebug() << "ERROR: could not find folder for " << absolutePath; return 0; } diff --git a/src/mirall/folderwatcher.cpp b/src/mirall/folderwatcher.cpp index 7813a1e7f2..303534cb02 100644 --- a/src/mirall/folderwatcher.cpp +++ b/src/mirall/folderwatcher.cpp @@ -81,7 +81,7 @@ bool FolderWatcher::pathIsIgnored( const QString& path ) QFileInfo fInfo(path); if( fInfo.isHidden() ) { - qDebug() << "* Discarded as is hidden!"; + qDebug() << "* Discarded as is hidden!" << fInfo.filePath(); return true; } diff --git a/src/mirall/socketapi.cpp b/src/mirall/socketapi.cpp index 05b70af39a..c98ec36217 100644 --- a/src/mirall/socketapi.cpp +++ b/src/mirall/socketapi.cpp @@ -71,50 +71,6 @@ namespace SocketApiHelper { SyncFileStatus fileStatus(Folder *folder, const QString& systemFileName, c_strlist_t *excludes ); -/** - * @brief recursiveFolderStatus - * @param fileName - the relative file name to examine - * @return the resulting status - * - * The resulting status can only be either SYNC which means all files - * are in sync, ERROR if an error occured, or EVAL if something needs - * to be synced underneath this dir. - */ -// compute the file status of a directory recursively. It returns either -// "all in sync" or "needs update" or "error", no more details. -SyncFileStatus recursiveFolderStatus(Folder *folder, const QString& fileName, c_strlist_t *excludes ) -{ - QDir dir(folder->path() + fileName); - - const QStringList dirEntries = dir.entryList( QDir::AllEntries | QDir::NoDotAndDotDot ); - - SyncFileStatus result(SyncFileStatus::STATUS_SYNC); - - foreach( const QString entry, dirEntries ) { - QString normalizedFile = QString(fileName + QLatin1Char('/') + entry).normalized(QString::NormalizationForm_C); - QFileInfo fi(entry); - SyncFileStatus sfs; - - if( fi.isDir() ) { - sfs = recursiveFolderStatus(folder, normalizedFile, excludes ); - } else { - QString fs( normalizedFile ); - if( fileName.isEmpty() ) { - // toplevel, no slash etc. needed. - fs = entry.normalized(QString::NormalizationForm_C); - } - sfs = fileStatus(folder, fs, excludes); - } - - if( sfs.tag() == SyncFileStatus::STATUS_STAT_ERROR || sfs.tag() == SyncFileStatus::STATUS_ERROR ) { - return SyncFileStatus::STATUS_ERROR; - } else if( sfs.tag() == SyncFileStatus::STATUS_EVAL || sfs.tag() == SyncFileStatus::STATUS_NEW) { - result.set(SyncFileStatus::STATUS_EVAL); - } - } - return result; -} - SyncJournalFileRecord dbFileRecord_capi( Folder *folder, QString fileName ) { @@ -211,7 +167,8 @@ SyncFileStatus fileStatus(Folder *folder, const QString& systemFileName, c_strli if( fi.isSymLink() ) { return SyncFileStatus(SyncFileStatus::STATUS_IGNORE); } - int type = CSYNC_FTW_TYPE_FILE; + + csync_ftw_type_e type = CSYNC_FTW_TYPE_FILE; if( fi.isDir() ) { type = CSYNC_FTW_TYPE_DIR; } @@ -224,55 +181,67 @@ SyncFileStatus fileStatus(Folder *folder, const QString& systemFileName, c_strli return SyncFileStatus(SyncFileStatus::STATUS_IGNORE); } - // Problem: for the sync dir itself we do not have a record in the sync journal - // so the next check must not be used for the sync root folder. - SyncJournalFileRecord rec = dbFileRecord_capi(folder, unixFileName ); - if( !isSyncRootFolder && !rec.isValid() ) { - // check the parent folder if it is shared and if it is allowed to create a file/dir within - QDir d( fi.path() ); - QString parentPath = d.path(); - SyncJournalFileRecord dirRec = dbFileRecord_capi(folder, parentPath); - while( !d.isRoot() && !(d.exists() && dirRec.isValid()) ) { - d.cdUp(); // returns true if the dir exists. - - parentPath = d.path(); - // cut the folder path - dirRec = dbFileRecord_capi(folder, parentPath); - } - if( dirRec.isValid() ) { - if( dirRec._type == CSYNC_FTW_TYPE_DIR ) { - if( !dirRec._remotePerm.contains("K") ) { - return SyncFileStatus::STATUS_ERROR; - } - } else { - if( !dirRec._remotePerm.contains("C") ) { - return SyncFileStatus::STATUS_ERROR; - } - } - } - return SyncFileStatus(SyncFileStatus::STATUS_NEW); - } SyncFileStatus status(SyncFileStatus::STATUS_NONE); - if( type == CSYNC_FTW_TYPE_DIR ) { - // compute recursive status of the directory - status = recursiveFolderStatus( folder, fileName, excludes ); - } else if( FileSystem::getModTime(fi.absoluteFilePath()) != Utility::qDateTimeToTime_t(rec._modtime) ) { - // file was locally modified. - status.set(SyncFileStatus::STATUS_EVAL); - } else { - status.set(SyncFileStatus::STATUS_SYNC); - } - - if (rec._remotePerm.contains("S")) { - status.setSharedWithMe(true); + if (type == CSYNC_FTW_TYPE_DIR) { + if (folder->estimateState(fileName, type, &status)) { + qDebug() << Q_FUNC_INFO << "Folder estimated status for" << fileName << "to" << status.toSocketAPIString(); + return status; + } + if (fileName == "") { + // sync folder itself + if (folder->syncResult().status() == SyncResult::Undefined + || folder->syncResult().status() == SyncResult::NotYetStarted + || folder->syncResult().status() == SyncResult::SyncPrepare + || folder->syncResult().status() == SyncResult::SyncRunning + || folder->syncResult().status() == SyncResult::Paused) { + status.set(SyncFileStatus::STATUS_EVAL); + return status; + } else if (folder->syncResult().status() == SyncResult::Success + || folder->syncResult().status() == SyncResult::Problem) { + status.set(SyncFileStatus::STATUS_SYNC); + return status; + } else if (folder->syncResult().status() == SyncResult::Error + || folder->syncResult().status() == SyncResult::SetupError + || folder->syncResult().status() == SyncResult::SyncAbortRequested) { + status.set(SyncFileStatus::STATUS_ERROR); + return status; + } + } + SyncJournalFileRecord rec = dbFileRecord_capi(folder, unixFileName ); + if (rec.isValid()) { + status.set(SyncFileStatus::STATUS_SYNC); + if (rec._remotePerm.contains("S")) { + status.setSharedWithMe(true); + } + } else { + status.set(SyncFileStatus::STATUS_EVAL); + } + } else if (type == CSYNC_FTW_TYPE_FILE) { + if (folder->estimateState(fileName, type, &status)) { + return status; + } + SyncJournalFileRecord rec = dbFileRecord_capi(folder, unixFileName ); + if (rec.isValid()) { + if (rec._remotePerm.contains("S")) { + status.setSharedWithMe(true); + } + if( FileSystem::getModTime(fi.absoluteFilePath()) == Utility::qDateTimeToTime_t(rec._modtime) ) { + status.set(SyncFileStatus::STATUS_SYNC); + return status; + } else { + status.set(SyncFileStatus::STATUS_EVAL); + return status; + } + } + status.set(SyncFileStatus::STATUS_NEW); + return status; } return status; } - -} +} // namespace SocketApi::SocketApi(QObject* parent) : QObject(parent) @@ -450,6 +419,8 @@ void SocketApi::slotUpdateFolderView(const QString& alias) } else { broadcastMessage(QLatin1String("UPDATE_VIEW"), f->path() ); } + } else { + qDebug() << "Not sending UPDATE_VIEW for" << alias << "because status() is" << f->syncResult().status(); } } } @@ -521,7 +492,8 @@ void SocketApi::broadcastMessage( const QString& verb, const QString& path, cons msg.append(QDir::toNativeSeparators(path)); } - DEBUG << "Broadcasting to" << _listeners.count() << "listeners: " << msg; + // sendMessage already has a debug output + //DEBUG << "Broadcasting to" << _listeners.count() << "listeners: " << msg; foreach(SocketType *socket, _listeners) { sendMessage(socket, msg, doWait); } diff --git a/src/mirall/syncengine.cpp b/src/mirall/syncengine.cpp index ae0edf5765..6c24d050f7 100644 --- a/src/mirall/syncengine.cpp +++ b/src/mirall/syncengine.cpp @@ -22,6 +22,7 @@ #include "discoveryphase.h" #include "creds/abstractcredentials.h" #include "csync_util.h" +#include "mirall/syncfilestatus.h" #ifdef Q_OS_WIN #include @@ -1069,7 +1070,18 @@ void SyncEngine::setSelectiveSyncBlackList(const QStringList& list) } } - +bool SyncEngine::estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s) +{ + Q_FOREACH(const SyncFileItem &item, _syncedItems) { + //qDebug() << Q_FUNC_INFO << fn << item._file << fn.startsWith(item._file) << item._file.startsWith(fn); + if (item._file.startsWith(fn)) { + qDebug() << Q_FUNC_INFO << "Setting" << fn << " to STATUS_EVAL"; + s->set(SyncFileStatus::STATUS_EVAL); + return true; + } + } + return false; +} void SyncEngine::abort() { diff --git a/src/mirall/syncengine.h b/src/mirall/syncengine.h index 0b25a43f9c..e37ce7c6f5 100644 --- a/src/mirall/syncengine.h +++ b/src/mirall/syncengine.h @@ -33,6 +33,7 @@ #include "mirall/syncfileitem.h" #include "mirall/progressdispatcher.h" #include "mirall/utility.h" +#include "mirall/syncfilestatus.h" class QProcess; @@ -66,6 +67,8 @@ public: /* Return true if we detected that another sync is needed to complete the sync */ bool isAnotherSyncNeeded() { return _anotherSyncNeeded; } + bool estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s); + signals: void csyncError( const QString& ); void csyncUnavailable(); @@ -123,7 +126,11 @@ private: void finalize(); static bool _syncRunning; //true when one sync is running somewhere (for debugging) + QMap _syncItemMap; + + // should be called _syncItems (present tense). It's the items from the _syncItemMap but + // sorted and re-adjusted based on permissions. SyncFileItemVector _syncedItems; CSYNC *_csync_ctx; diff --git a/src/mirall/syncfileitem.h b/src/mirall/syncfileitem.h index fccc91a06f..22a7581593 100644 --- a/src/mirall/syncfileitem.h +++ b/src/mirall/syncfileitem.h @@ -72,6 +72,13 @@ public: return _file.isEmpty(); } + bool hasErrorStatus() const { + return _status == SyncFileItem::SoftError + || _status == SyncFileItem::NormalError + || _status == SyncFileItem::FatalError + || !_errorString.isEmpty(); + } + // Variables usefull for everybody QString _file; QString _renameTarget; diff --git a/src/mirall/utility.cpp b/src/mirall/utility.cpp index 4e38a8d576..63d6feee5e 100644 --- a/src/mirall/utility.cpp +++ b/src/mirall/utility.cpp @@ -224,6 +224,18 @@ QString Utility::toCSyncScheme(const QString &urlStr) return url.toString(); } +bool Utility::doesSetContainPrefix(QSet &l, QString &p) { + + Q_FOREACH (const QString &setPath, l) { + //qDebug() << Q_FUNC_INFO << p << setPath << setPath.startsWith(p); + if (setPath.startsWith(p)) { + return true; + } + } + //qDebug() << "-> NOOOOO!!!" << p << l.count(); + return false; +} + QString Utility::escape(const QString &in) { #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) diff --git a/src/mirall/utility.h b/src/mirall/utility.h index ac451cf975..32eab30fa4 100644 --- a/src/mirall/utility.h +++ b/src/mirall/utility.h @@ -42,6 +42,10 @@ namespace Utility OWNCLOUDSYNC_EXPORT QString toCSyncScheme(const QString &urlStr); /** Like QLocale::toString(double, 'f', prec), but drops trailing zeros after the decimal point */ + OWNCLOUDSYNC_EXPORT bool doesSetContainPrefix(QSet &l, QString &p); + + + /** * @brief compactFormatDouble - formats a double value human readable. * From 4d3020421a2cdfbb5bac28e48fcf500e5603eca6 Mon Sep 17 00:00:00 2001 From: Markus Goetz Date: Mon, 13 Oct 2014 17:25:13 +0200 Subject: [PATCH 2/3] SocketAPI: Remove old dbFileRecord function We will use the _capi functions --- src/mirall/socketapi.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/mirall/socketapi.cpp b/src/mirall/socketapi.cpp index c98ec36217..231dc24be5 100644 --- a/src/mirall/socketapi.cpp +++ b/src/mirall/socketapi.cpp @@ -124,19 +124,6 @@ SyncJournalFileRecord dbFileRecord_capi( Folder *folder, QString fileName ) return rec; } -SyncJournalFileRecord dbFileRecord( Folder *folder, QString fileName ) -{ - if( !folder ) { - return SyncJournalFileRecord(); - } - - QFileInfo fi(fileName); - if( fi.isAbsolute() ) { - fileName.remove(0, folder->path().length()); - } - return( folder->journalDb()->getFileRecord(fileName) ); -} - /** * Get status about a single file. */ From eb60aca34f343fc567118c3dd2741f16c2bcf10b Mon Sep 17 00:00:00 2001 From: Markus Goetz Date: Mon, 13 Oct 2014 17:28:47 +0200 Subject: [PATCH 3/3] SyncJournalDB: Clarify debug message --- src/mirall/syncjournaldb.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mirall/syncjournaldb.cpp b/src/mirall/syncjournaldb.cpp index 71dd8494f3..be8ce41825 100644 --- a/src/mirall/syncjournaldb.cpp +++ b/src/mirall/syncjournaldb.cpp @@ -1064,7 +1064,7 @@ void SyncJournalDb::commit(const QString& context, bool startTrans) void SyncJournalDb::commitInternal(const QString& context, bool startTrans ) { - qDebug() << "Transaction Start " << context; + qDebug() << Q_FUNC_INFO << "Transaction commit " << context << (startTrans ? "and starting new transaction" : ""); commitTransaction(); if( startTrans ) {