FolderWatcher: Detect own changes. #2297

src/gui/folder.cpp

@@ -540,6 +540,37 @@ int Folder::slotWipeBlacklist()
     return _journal.wipeBlacklist();
 }
 
+void Folder::slotWatchedPathChanged(const QString& path)
+{
+    // When no sync is running or it's in the prepare phase, we can
+    // always schedule a new sync.
+    if (! _engine || _syncResult.status() == SyncResult::SyncPrepare) {
+        emit scheduleToSync(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. Therefore we check notifications against operations
+    // the sync is doing to filter out our own changes.
+    bool ownChange = false;
+#ifdef Q_OS_MAC
+    // On OSX the folder watcher does not report changes done by our
+    // own process. Therefore nothing needs to be done here!
+#else
+    // Use the path to figure out whether it was our own change
+    const auto maxNotificationDelay = 15*1000;
+    qint64 time = _engine->timeSinceFileTouched(path);
+    if (time != -1 && time < maxNotificationDelay) {
+        ownChange = true;
+    }
+#endif
+
+    if (! ownChange) {
+        emit scheduleToSync(alias());
+    }
+}
+
 void Folder::setConfigFile( const QString& file )
 {
     _configFile = file;

src/gui/folder.h

@@ -154,6 +154,13 @@ public slots:
       int slotWipeBlacklist();
       int blackListEntryCount();
 
+      /**
+       * Triggered by the folder watcher when a file/dir in this folder
+       * changes. Needs to check whether this change should trigger a new
+       * sync run to be scheduled.
+       */
+      void slotWatchedPathChanged(const QString& path);
+
 private slots:
     void slotSyncStarted();
     void slotSyncError(const QString& );

src/gui/folderman.cpp

@@ -55,10 +55,6 @@ FolderMan::FolderMan(QObject *parent) :
     connect(_folderChangeSignalMapper, SIGNAL(mapped(const QString &)),
             this, SIGNAL(folderSyncStateChange(const QString &)));
 
-    _folderWatcherSignalMapper = new QSignalMapper(this);
-    connect(_folderWatcherSignalMapper, SIGNAL(mapped(const QString&)),
-            this, SLOT(slotScheduleSync(const QString&)));
-
     ne_sock_init();
     Q_ASSERT(!_instance);
     _instance = this;
@@ -100,8 +96,6 @@ void FolderMan::unloadFolder( const QString& alias )
 
         _folderChangeSignalMapper->removeMappings(f);
         if( _folderWatchers.contains(alias)) {
-            FolderWatcher *fw = _folderWatchers[alias];
-            _folderWatcherSignalMapper->removeMappings(fw);
             _folderWatchers.remove(alias);
         }
         _folderMap.remove( alias );
@@ -135,7 +129,7 @@ void FolderMan::registerFolderMonitor( Folder *folder )
     if( !folder ) return;
 
     if( !_folderWatchers.contains(folder->alias() ) ) {
-        FolderWatcher *fw = new FolderWatcher(folder->path(), this);
+        FolderWatcher *fw = new FolderWatcher(folder->path(), folder);
         MirallConfigFile cfg;
         fw->addIgnoreListFile( cfg.excludeFile(MirallConfigFile::SystemScope) );
         fw->addIgnoreListFile( cfg.excludeFile(MirallConfigFile::UserScope) );
@@ -143,8 +137,7 @@ void FolderMan::registerFolderMonitor( Folder *folder )
         // Connect the pathChanged signal, which comes with the changed path,
         // to the signal mapper which maps to the folder alias. The changed path
         // is lost this way, but we do not need it for the current implementation.
-        connect(fw, SIGNAL(pathChanged(QString)), _folderWatcherSignalMapper, SLOT(map()));
-        _folderWatcherSignalMapper->setMapping(fw, folder->alias());
+        connect(fw, SIGNAL(pathChanged(QString)), folder, SLOT(slotWatchedPathChanged(QString)));
         _folderWatchers.insert(folder->alias(), fw);
 
         // This is at the moment only for the behaviour of the SocketApi.
@@ -444,20 +437,6 @@ void FolderMan::slotScheduleSync( const QString& alias )
         return;
     }
 
-#ifndef Q_OS_MAC
-    // 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.
-    // This works fine on OSX, where the folder watcher does not
-    // report changes done by our own process.
-    if( _currentSyncFolder == alias ) {
-        qDebug() << "folder " << alias << " is currently syncing. NOT scheduling.";
-        return;
-    }
-#endif
-
     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

src/gui/folderman.h

@@ -150,7 +150,6 @@ private:
     Folder::Map    _folderMap;
     QString        _folderConfigPath;
     QSignalMapper *_folderChangeSignalMapper;
-    QSignalMapper *_folderWatcherSignalMapper;
     QString        _currentSyncFolder;
     bool           _syncEnabled;
     QMap<QString, FolderWatcher*> _folderWatchers;

src/libsync/owncloudpropagator.cpp

@@ -457,6 +457,26 @@ void OwncloudPropagator::scheduleNextJob()
     }
 }
 
+void OwncloudPropagator::addTouchedFile(const QString& fn)
+{
+    QString file = QDir::cleanPath(fn);
+
+    QElapsedTimer timer;
+    timer.start();
+
+    QMutexLocker lock(&_touchedFilesMutex);
+    _touchedFiles.insert(file, timer);
+}
+
+qint64 OwncloudPropagator::timeSinceFileTouched(const QString& fn) const
+{
+    QMutexLocker lock(&_touchedFilesMutex);
+    if (! _touchedFiles.contains(fn)) {
+        return -1;
+    }
+
+    return _touchedFiles[fn].elapsed();
+}
 
 // ================================================================================
 

src/libsync/owncloudpropagator.h

@@ -24,6 +24,7 @@
 #include <QTimer>
 #include <QPointer>
 #include <QIODevice>
+#include <QMutex>
 
 #include "syncfileitem.h"
 #include "syncjournaldb.h"
@@ -291,6 +292,19 @@ public:
     // timeout in seconds
     static int httpTimeout();
 
+    /** Records that a file was touched by a job.
+     *
+     * Thread-safe.
+     */
+    void addTouchedFile(const QString& fn);
+
+    /** Get the ms since a file was touched, or -1 if it wasn't.
+     *
+     * Thread-safe.
+     */
+    qint64 timeSinceFileTouched(const QString& fn) const;
+
+
 private slots:
 
     /** Emit the finished signal and make sure it is only emit once */
@@ -312,6 +326,11 @@ signals:
      */
     void adjustTotalTransmissionSize( qint64 adjust );
 
+private:
+
+    /** Stores the time since a job touched a file. */
+    QHash<QString, QElapsedTimer> _touchedFiles;
+    mutable QMutex _touchedFilesMutex;
 };
 
 // Job that wait for all the poll jobs to be completed

src/libsync/propagatedownload.cpp

@@ -476,6 +476,7 @@ void PropagateDownloadFileQNAM::downloadFinished()
     FileSystem::setFileHidden(_tmpFile.fileName(), false);
 
     QString error;
+    _propagator->addTouchedFile(fn);
     if (!FileSystem::renameReplace(_tmpFile.fileName(), fn, &error)) {
         // If we moved away the original file due to a conflict but can't
         // put the downloaded file in its place, we are in a bad spot:

src/libsync/propagateremotemove.cpp

@@ -58,13 +58,18 @@ void PropagateRemoteMove::start()
 
     qDebug() << Q_FUNC_INFO << _item._file << _item._renameTarget;
 
+    QString targetFile(_propagator->getFilePath(_item._renameTarget));
+
     if (_item._file == _item._renameTarget) {
         // The parents has been renamed already so there is nothing more to do.
         finalize();
         return;
     } else if (AbstractNetworkJob::preOc7WasDetected && _item._file == QLatin1String("Shared") ) {
         // Check if it is the toplevel Shared folder and do not propagate it.
-        if( QFile::rename(  _propagator->_localDir + _item._renameTarget, _propagator->_localDir + QLatin1String("Shared")) ) {
+        QString originalFile(_propagator->getFilePath(QLatin1String("Shared")));
+        _propagator->addTouchedFile(originalFile);
+        _propagator->addTouchedFile(targetFile);
+        if( QFile::rename( targetFile, originalFile) ) {
             done(SyncFileItem::NormalError, tr("This folder must not be renamed. It is renamed back to its original name."));
         } else {
             done(SyncFileItem::NormalError, tr("This folder must not be renamed. Please name it back to Shared."));
@@ -131,7 +136,7 @@ void PropagateRemoteMove::slotMoveJobFinished()
 void PropagateRemoteMove::finalize()
 {
     _propagator->_journal->deleteFileRecord(_item._originalFile);
-    SyncJournalFileRecord record(_item, _propagator->_localDir + _item._renameTarget);
+    SyncJournalFileRecord record(_item, _propagator->getFilePath(_item._renameTarget));
     record._path = _item._renameTarget;
 
     _propagator->_journal->setFileRecord(record);

src/libsync/propagator_legacy.cpp

@@ -712,6 +712,7 @@ void PropagateDownloadFileLegacy::start()
     FileSystem::setFileHidden(tmpFile.fileName(), false);
 
     QString error;
+    _propagator->addTouchedFile(fn);
     if (!FileSystem::renameReplace(tmpFile.fileName(), fn, &error)) {
         done(SyncFileItem::NormalError, error);
         return;

src/libsync/propagatorjobs.cpp

@@ -131,12 +131,14 @@ void PropagateLocalRename::start()
     if (_propagator->_abortRequested.fetchAndAddRelaxed(0))
         return;
 
+    QString existingFile = _propagator->getFilePath(_item._file);
+    QString targetFile = _propagator->getFilePath(_item._renameTarget);
+
     // if the file is a file underneath a moved dir, the _item.file is equal
     // to _item.renameTarget and the file is not moved as a result.
     if (_item._file != _item._renameTarget) {
         emit progress(_item, 0);
-        qDebug() << "MOVE " << _propagator->_localDir + _item._file << " => " << _propagator->_localDir + _item._renameTarget;
-        QFile file(_propagator->_localDir + _item._file);
+        qDebug() << "MOVE " << existingFile << " => " << targetFile;
 
         if (QString::compare(_item._file, _item._renameTarget, Qt::CaseInsensitive) != 0
                 && _propagator->localFileNameClash(_item._renameTarget)) {
@@ -149,7 +151,11 @@ void PropagateLocalRename::start()
                  .arg(QDir::toNativeSeparators(_item._file)).arg(QDir::toNativeSeparators(_item._renameTarget)) );
             return;
         }
-        if (!file.rename(_propagator->_localDir + _item._file, _propagator->_localDir + _item._renameTarget)) {
+
+        _propagator->addTouchedFile(existingFile);
+        _propagator->addTouchedFile(targetFile);
+        QFile file(existingFile);
+        if (!file.rename(targetFile)) {
             done(SyncFileItem::NormalError, file.errorString());
             return;
         }
@@ -160,7 +166,7 @@ void PropagateLocalRename::start()
     // store the rename file name in the item.
     _item._file = _item._renameTarget;
 
-    SyncJournalFileRecord record(_item, _propagator->_localDir + _item._renameTarget);
+    SyncJournalFileRecord record(_item, targetFile);
     record._path = _item._renameTarget;
 
     if (!_item._isDirectory) { // Directory are saved at the end

src/libsync/syncengine.cpp

@@ -716,9 +716,6 @@ void SyncEngine::slotDiscoveryJobFinished(int discoveryResult)
         }
     }
 
-    if (_needsUpdate)
-        emit(started());
-
     ne_session_s *session = 0;
     // that call to set property actually is a get which will return the session
     csync_set_module_property(_csync_ctx, "get_dav_session", &session);
@@ -756,6 +753,10 @@ void SyncEngine::slotDiscoveryJobFinished(int discoveryResult)
     deleteStaleBlacklistEntries();
     _journal->commit("post stale entry removal");
 
+    // Emit the started signal only after the propagator has been set up.
+    if (_needsUpdate)
+        emit(started());
+
     _propagator->start(_syncedItems);
 }
 
@@ -842,9 +843,11 @@ void SyncEngine::finalize()
     qDebug() << "CSync run took " << _stopWatch.addLapTime(QLatin1String("Sync Finished"));
     _stopWatch.stop();
 
-    _propagator.reset(0);
     _syncRunning = false;
     emit finished();
+
+    // Delete the propagator only after emitting the signal.
+    _propagator.reset();
 }
 
 void SyncEngine::slotProgress(const SyncFileItem& item, quint64 current)
@@ -1130,6 +1133,16 @@ bool SyncEngine::estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s
     return false;
 }
 
+qint64 SyncEngine::timeSinceFileTouched(const QString& fn) const
+{
+    // This copy is essential for thread safety.
+    QSharedPointer<OwncloudPropagator> prop = _propagator;
+    if (prop) {
+        return prop->timeSinceFileTouched(fn);
+    }
+    return -1;
+}
+
 void SyncEngine::abort()
 {
     csync_request_abort(_csync_ctx);

src/libsync/syncengine.h

@@ -69,6 +69,12 @@ public:
 
     bool estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s);
 
+    /** Get the ms since a file was touched, or -1 if it wasn't.
+     *
+     * Thread-safe.
+     */
+    qint64 timeSinceFileTouched(const QString& fn) const;
+
 signals:
     void csyncError( const QString& );
     void csyncUnavailable();
@@ -140,7 +146,7 @@ private:
     QString _remoteUrl;
     QString _remotePath;
     SyncJournalDb *_journal;
-    QScopedPointer <OwncloudPropagator> _propagator;
+    QSharedPointer <OwncloudPropagator> _propagator;
     QString _lastDeleted; // if the last item was a path and it has been deleted
     QSet<QString> _seenFiles;
     QThread _thread;