FolderWatcher (linux): optimize slotAddFolderRecursive

- Instead of putting all the pathes in a big QStringList and then process them,
  do the processing as we iterate with QDirIterator

- Since we know that the parent directory is not ignored, we can simplify the
  ignore check by not checking the part before (i.e: we call
  ExcludedFiles::isExcluded with the parent 'path' as base path) This is what
  speeds up considerably the function.

Note that the OWNCLOUD_TEST check is already there in FolderWatcher::pathIsIgnored
so the behavior is not changed

admin/win/nsi/l10n/Dutch.nsh

@@ -1,5 +1,5 @@
 # Auto-generated - do not modify
-StrCpy $MUI_FINISHPAGE_SHOWREADME_TEXT_STRING "Toon opmerkingen bij deze uitgave"
+StrCpy $MUI_FINISHPAGE_SHOWREADME_TEXT_STRING "Toon opmerkingen bij deze versie"
 StrCpy $ConfirmEndProcess_MESSAGEBOX_TEXT "Gevonden ${APPLICATION_EXECUTABLE} proces(sen) moet worden gestopt.$\nWilt u dat het installatieprogramma dat voor u doet?"
 StrCpy $ConfirmEndProcess_KILLING_PROCESSES_TEXT "Stoppen ${APPLICATION_EXECUTABLE} processen."
 StrCpy $ConfirmEndProcess_KILL_NOT_FOUND_TEXT "Het te stoppen proces is niet gevonden!"

csync/src/csync_rename.cc

@@ -20,6 +20,7 @@
 
 extern "C" {
 #include "csync_private.h"
+#include "csync_rename.h"
 }
 
 #include <map>
@@ -93,5 +94,9 @@ char* csync_rename_adjust_path_source(CSYNC* ctx, const char* path)
     return c_strdup(path);
 }
 
+bool csync_rename_count(CSYNC *ctx) {
+    csync_rename_s* d = csync_rename_s::get(ctx);
+    return d->folder_renamed_from.size();
+}
 
 }

csync/src/csync_rename.h

@@ -32,6 +32,8 @@ char OCSYNC_EXPORT *csync_rename_adjust_path(CSYNC *ctx, const char *path);
 char OCSYNC_EXPORT *csync_rename_adjust_path_source(CSYNC *ctx, const char *path);
 void OCSYNC_EXPORT csync_rename_destroy(CSYNC *ctx);
 void OCSYNC_EXPORT csync_rename_record(CSYNC *ctx, const char *from, const char *to);
+/*  Return the amount of renamed item recorded */
+bool OCSYNC_EXPORT csync_rename_count(CSYNC *ctx);
 
 #ifdef __cplusplus
 }

doc/architecture.rst

@@ -199,12 +199,6 @@ the database by comparing the files and their modification times. This process
 ensures that both server and client are synchronized using the appropriate NTP
 time before restarting the client following a database removal.
 
-Pressing ``F5`` while in the Account Settings Dialog enables you to "reset" the
-journal. This function can be used to recreate the journal database.
-
-.. note:: We recommend that you use this function only when advised to do so by
-   ownCloud support staff.
-
 Custom WebDAV Properties
 ------------------------
 

src/gui/accountmanager.cpp

@@ -209,6 +209,13 @@ void AccountManager::saveAccountHelper(Account* acc, QSettings& settings, bool s
 
 AccountPtr AccountManager::loadAccountHelper(QSettings& settings)
 {
+    auto urlConfig = settings.value(QLatin1String(urlC));
+    if (!urlConfig.isValid()) {
+        // No URL probably means a corrupted entry in the account settings
+        qDebug() << "No URL for account " << settings.group();
+        return AccountPtr();
+    }
+
     auto acc = createAccount();
 
     QString authType = settings.value(QLatin1String(authTypeC)).toString();
@@ -220,7 +227,7 @@ AccountPtr AccountManager::loadAccountHelper(QSettings& settings)
         acc->setUrl(overrideUrl);
         authType = forceAuth;
     } else {
-        acc->setUrl(settings.value(QLatin1String(urlC)).toUrl());
+        acc->setUrl(urlConfig.toUrl());
     }
     acc->_serverVersion = settings.value(QLatin1String(serverVersionC)).toString();
 

src/gui/application.cpp

@@ -336,8 +336,13 @@ void Application::slotownCloudWizardDone( int res )
         _checkConnectionTimer.start();
         slotCheckConnection();
 
-        // The very first time an account is configured: enabled autostart
-        // TODO: Doing this every time the account wizard finishes will annoy users.
+        // If one account is configured: enable autostart
+        bool shouldSetAutoStart = (accountMan->accounts().size() == 1);
+#ifdef Q_OS_MAC
+        // Don't auto start when not being 'installed'
+        shouldSetAutoStart = shouldSetAutoStart
+                && QCoreApplication::applicationDirPath().startsWith("/Applications/");
+#endif
         Utility::setLaunchOnStartup(_theme->appName(), _theme->appNameGUI(), true);
     }
 }

src/gui/folder.cpp

@@ -188,7 +188,7 @@ QString Folder::shortGuiLocalPath() const
 }
 
 
-bool Folder::ignoreHiddenFiles()
+bool Folder::ignoreHiddenFiles() const
 {
     bool re(_definition.ignoreHiddenFiles);
     return re;

src/gui/folder.h

@@ -167,7 +167,7 @@ public:
       * Ignore syncing of hidden files or not. This is defined in the
       * folder definition
       */
-     bool ignoreHiddenFiles();
+     bool ignoreHiddenFiles() const;
      void setIgnoreHiddenFiles(bool ignore);
 
      // Used by the Socket API

src/gui/folderwatcher_linux.cpp

@@ -23,6 +23,7 @@
 #include <QStringList>
 #include <QObject>
 #include <QVarLengthArray>
+#include <syncengine.h>
 
 namespace OCC {
 
@@ -48,31 +49,6 @@ FolderWatcherPrivate::~FolderWatcherPrivate()
 
 }
 
-// attention: result list passed by reference!
-bool FolderWatcherPrivate::findFoldersBelow( const QDir& dir, QStringList& fullList )
-{
-    bool ok = true;
-    if( !(dir.exists() && dir.isReadable()) ) {
-        qDebug() << "Non existing path coming in: " << dir.absolutePath();
-        ok = false;
-    } else {
-        QStringList nameFilter;
-        nameFilter << QLatin1String("*");
-        QDir::Filters filter = QDir::Dirs | QDir::NoDotAndDotDot | QDir::NoSymLinks | QDir::Hidden;
-        const QStringList pathes = dir.entryList(nameFilter, filter);
-
-        QStringList::const_iterator constIterator;
-        for (constIterator = pathes.constBegin(); constIterator != pathes.constEnd();
-               ++constIterator) {
-            const QString fullPath(dir.path()+QLatin1String("/")+(*constIterator));
-            fullList.append(fullPath);
-            ok = findFoldersBelow(QDir(fullPath), fullList);
-        }
-    }
-
-    return ok;
-}
-
 void FolderWatcherPrivate::inotifyRegisterPath(const QString& path)
 {
     if( !path.isEmpty()) {
@@ -88,41 +64,47 @@ void FolderWatcherPrivate::inotifyRegisterPath(const QString& path)
 
 void FolderWatcherPrivate::slotAddFolderRecursive(const QString &path)
 {
-    int subdirs = 0;
     qDebug() << "(+) Watcher:" << path;
-
-    QDir inPath(path);
-    inotifyRegisterPath(inPath.absolutePath());
-
     const QStringList watchedFolders = _watches.values();
-
-    QStringList allSubfolders;
-    if( !findFoldersBelow(QDir(path), allSubfolders)) {
-        qDebug() << "Could not traverse all sub folders";
-    }
-    // qDebug() << "currently watching " << watchedFolders;
-    QStringListIterator subfoldersIt(allSubfolders);
-    while (subfoldersIt.hasNext()) {
-        QString subfolder = subfoldersIt.next();
-        // qDebug() << "  (**) subfolder: " << subfolder;
-        QDir folder (subfolder);
-        if (folder.exists() && !watchedFolders.contains(folder.absolutePath())) {
-            subdirs++;
-            if( _parent->pathIsIgnored(subfolder) ) {
-                qDebug() << "* Not adding" << folder.path();
-                continue;
-            }
-            inotifyRegisterPath(folder.absolutePath());
-        } else {
-            qDebug() << "    `-> discarded:" << folder.path();
-        }
-    }
-
+    int subdirs = addFolderRecursiveHelper(path, watchedFolders.toSet());
     if (subdirs >0) {
         qDebug() << "    `-> and" << subdirs << "subdirectories";
     }
 }
 
+int FolderWatcherPrivate::addFolderRecursiveHelper(const QString &path, const QSet<QString> &watchedFolders)
+{
+    QDir dir(path);
+    if( !(dir.exists() && dir.isReadable()) ) {
+        qDebug() << "Non existing path coming in: " << dir.absolutePath();
+        return 0;
+    }
+    int subdirs = 1;
+
+    inotifyRegisterPath(dir.absolutePath());
+
+    QDir::Filters filter = QDir::Dirs | QDir::NoDotAndDotDot | QDir::NoSymLinks | QDir::Hidden;
+    const QStringList pathes = dir.entryList(filter);
+    for (auto constIterator = pathes.constBegin(); constIterator != pathes.constEnd(); ++constIterator) {
+        const QString subfolder = path + QLatin1String("/") + (*constIterator);
+        QDir folder(subfolder);
+        if (folder.exists() && !watchedFolders.contains(subfolder)) {
+#ifndef OWNCLOUD_TEST // InotifyWatcherTest is not interested in ignored files and does not link against the folder
+            if( _parent->_folder->syncEngine().excludedFiles().isExcluded(
+                    subfolder, path, _parent->_folder->ignoreHiddenFiles())) {
+                qDebug() << "* Not adding" << folder.path();
+                continue;
+            }
+#endif
+            subdirs += addFolderRecursiveHelper(subfolder, watchedFolders);
+        } else {
+            qDebug() << "    `-> discarded:" << folder.path();
+        }
+    }
+    return subdirs;
+}
+
+
 void FolderWatcherPrivate::slotReceivedNotification(int fd)
 {
     int len;

src/gui/folderwatcher_linux.h

@@ -33,7 +33,6 @@ class FolderWatcherPrivate : public QObject
 {
     Q_OBJECT
 public:
-    FolderWatcherPrivate() { }
     FolderWatcherPrivate(FolderWatcher *p, const QString &path);
     ~FolderWatcherPrivate();
 
@@ -45,10 +44,9 @@ protected slots:
     void slotAddFolderRecursive(const QString &path);
 
 protected:
-    bool findFoldersBelow( const QDir& dir, QStringList& fullList );
+    int addFolderRecursiveHelper(const QString &path, const QSet<QString> &watchedFolders);
     void inotifyRegisterPath(const QString& path);
 
-private:
     FolderWatcher *_parent;
 
     QString _folder;

src/gui/owncloudgui.cpp

@@ -67,7 +67,7 @@ ownCloudGui::ownCloudGui(Application *parent) :
     _tray->setParent(this);
 
     // for the beginning, set the offline icon until the account was verified
-    _tray->setIcon( Theme::instance()->folderOfflineIcon(true));
+    _tray->setIcon( Theme::instance()->folderOfflineIcon(/*systray?*/ true, /*currently visible?*/ false));
 
     connect(_tray.data(), SIGNAL(activated(QSystemTrayIcon::ActivationReason)),
             SLOT(slotTrayClicked(QSystemTrayIcon::ActivationReason)));
@@ -262,7 +262,7 @@ void ownCloudGui::slotComputeOverallSyncStatus()
     }
 
     if (!problemAccounts.empty()) {
-        _tray->setIcon(Theme::instance()->folderOfflineIcon(true));
+        _tray->setIcon(Theme::instance()->folderOfflineIcon(true, contextMenuVisible()));
 #ifdef Q_OS_WIN
         // Windows has a 128-char tray tooltip length limit.
         QStringList accountNames;
@@ -289,11 +289,11 @@ void ownCloudGui::slotComputeOverallSyncStatus()
     }
 
     if (allSignedOut) {
-        _tray->setIcon(Theme::instance()->folderOfflineIcon(true));
+        _tray->setIcon(Theme::instance()->folderOfflineIcon(true, contextMenuVisible()));
         _tray->setToolTip(tr("Please sign in"));
         return;
     } else if (allPaused) {
-        _tray->setIcon(Theme::instance()->syncStateIcon(SyncResult::Paused, true));
+        _tray->setIcon(Theme::instance()->syncStateIcon(SyncResult::Paused, true, contextMenuVisible()));
         _tray->setToolTip(tr("Account synchronization is disabled"));
         return;
     }
@@ -323,12 +323,12 @@ void ownCloudGui::slotComputeOverallSyncStatus()
             trayMessage = tr("No sync folders configured.");
         }
 
-        QIcon statusIcon = Theme::instance()->syncStateIcon( overallResult.status(), true);
+        QIcon statusIcon = Theme::instance()->syncStateIcon( overallResult.status(), true, contextMenuVisible());
         _tray->setIcon( statusIcon );
         _tray->setToolTip(trayMessage);
     } else {
         // undefined because there are no folders.
-        QIcon icon = Theme::instance()->syncStateIcon(SyncResult::Problem, true);
+        QIcon icon = Theme::instance()->syncStateIcon(SyncResult::Problem, true, contextMenuVisible());
         _tray->setIcon( icon );
         _tray->setToolTip(tr("There are no sync folders configured."));
     }
@@ -405,6 +405,9 @@ void ownCloudGui::slotContextMenuAboutToShow()
     // For some reason on OS X _contextMenu->isVisible returns always false
     qDebug() << "";
     _contextMenuVisibleOsx = true;
+
+    // Update icon in sys tray, as it might change depending on the context menu state
+    slotComputeOverallSyncStatus();
 }
 
 void ownCloudGui::slotContextMenuAboutToHide()
@@ -412,6 +415,9 @@ void ownCloudGui::slotContextMenuAboutToHide()
     // For some reason on OS X _contextMenu->isVisible returns always false
     qDebug() << "";
     _contextMenuVisibleOsx = false;
+
+    // Update icon in sys tray, as it might change depending on the context menu state
+    slotComputeOverallSyncStatus();
 }
 
 bool ownCloudGui::contextMenuVisible() const

src/gui/wizard/owncloudadvancedsetuppage.ui

@@ -267,6 +267,12 @@
          <layout class="QHBoxLayout" name="horizontalLayout">
           <item>
            <widget class="QRadioButton" name="rSelectiveSync">
+            <property name="sizePolicy">
+             <sizepolicy hsizetype="Minimum" vsizetype="Minimum">
+              <horstretch>0</horstretch>
+              <verstretch>0</verstretch>
+             </sizepolicy>
+            </property>
             <property name="text">
              <string/>
             </property>

src/libsync/discoveryphase.cpp

@@ -14,6 +14,7 @@
 
 #include "discoveryphase.h"
 #include <csync_private.h>
+#include <csync_rename.h>
 #include <qdebug.h>
 
 #include <QUrl>
@@ -51,7 +52,7 @@ static bool findPathInList(const QStringList &list, const QString &path)
     return pathSlash.startsWith(*it);
 }
 
-bool DiscoveryJob::isInSelectiveSyncBlackList(const QString& path) const
+bool DiscoveryJob::isInSelectiveSyncBlackList(const char *path) const
 {
     if (_selectiveSyncBlackList.isEmpty()) {
         // If there is no black list, everything is allowed
@@ -59,13 +60,25 @@ bool DiscoveryJob::isInSelectiveSyncBlackList(const QString& path) const
     }
 
     // Block if it is in the black list
-    return findPathInList(_selectiveSyncBlackList, path);
+    if (findPathInList(_selectiveSyncBlackList, QString::fromUtf8(path))) {
+        return true;
+    }
 
+    // Also try to adjust the path if there was renames
+    if (csync_rename_count(_csync_ctx)) {
+        QScopedPointer<char, QScopedPointerPodDeleter> adjusted(
+            csync_rename_adjust_path_source(_csync_ctx, path));
+        if (strcmp(adjusted.data(), path) != 0) {
+            return findPathInList(_selectiveSyncBlackList, QString::fromUtf8(adjusted.data()));
+        }
+    }
+
+    return false;
 }
 
 int DiscoveryJob::isInSelectiveSyncBlackListCallback(void *data, const char *path)
 {
-    return static_cast<DiscoveryJob*>(data)->isInSelectiveSyncBlackList(QString::fromUtf8(path));
+    return static_cast<DiscoveryJob*>(data)->isInSelectiveSyncBlackList(path);
 }
 
 bool DiscoveryJob::checkSelectiveSyncNewFolder(const QString& path)
@@ -192,7 +205,8 @@ int get_errno_from_http_errcode( int err, const QString & reason ) {
         new_errno = EIO;
         break;
     case 503:           /* Service Unavailable */
-        if (reason == "Storage not available") {
+        // https://github.com/owncloud/core/pull/26145/files
+        if (reason == "Storage not available" || reason == "Storage is temporarily not available") {
             new_errno = ERRNO_STORAGE_UNAVAILABLE;
         } else {
             new_errno = ERRNO_SERVICE_UNAVAILABLE;

src/libsync/discoveryphase.h

@@ -174,7 +174,7 @@ class DiscoveryJob : public QObject {
      * return true if the given path should be ignored,
      * false if the path should be synced
      */
-    bool isInSelectiveSyncBlackList(const QString &path) const;
+    bool isInSelectiveSyncBlackList(const char* path) const;
     static int isInSelectiveSyncBlackListCallback(void *, const char *);
     bool checkSelectiveSyncNewFolder(const QString &path);
     static int checkSelectiveSyncNewFolderCallback(void*, const char*);

src/libsync/ownsql.cpp

@@ -230,8 +230,12 @@ bool SqlQuery::isPragma()
 
 bool SqlQuery::exec()
 {
+    if (!_stmt) {
+        return false;
+    }
+
     // Don't do anything for selects, that is how we use the lib :-|
-    if(_stmt && !isSelect() && !isPragma() ) {
+    if( !isSelect() && !isPragma() ) {
         int rc, n = 0;
         do {
             rc = sqlite3_step(_stmt);
@@ -376,8 +380,10 @@ void SqlQuery::finish()
 
 void SqlQuery::reset_and_clear_bindings()
 {
-    SQLITE_DO(sqlite3_reset(_stmt));
-    SQLITE_DO(sqlite3_clear_bindings(_stmt));
+    if (_stmt) {
+        SQLITE_DO(sqlite3_reset(_stmt));
+        SQLITE_DO(sqlite3_clear_bindings(_stmt));
+    }
 }
 
 } // namespace OCC

src/libsync/propagateremotemove.cpp

@@ -13,6 +13,7 @@
  */
 
 #include "propagateremotemove.h"
+#include "propagatorjobs.h"
 #include "owncloudpropagator_p.h"
 #include "account.h"
 #include "syncjournalfilerecord.h"
@@ -175,10 +176,45 @@ void PropagateRemoteMove::finalize()
         done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
         return;
     }
+
+    if (_item->_isDirectory) {
+        if (!adjustSelectiveSync(_propagator->_journal, _item->_file, _item->_renameTarget)) {
+            done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
+            return;
+        }
+    }
+
     _propagator->_journal->commit("Remote Rename");
     done(SyncFileItem::Success);
 }
 
+bool PropagateRemoteMove::adjustSelectiveSync(SyncJournalDb *journal, const QString &from_, const QString &to_)
+{
+    bool ok;
+    // We only care about preserving the blacklist.   The white list should anyway be empty.
+    // And the undecided list will be repopulated on the next sync, if there is anything too big.
+    QStringList list = journal->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, &ok);
+    if (!ok)
+        return false;
+
+    bool changed = false;
+    Q_ASSERT(!from_.endsWith(QLatin1String("/")));
+    Q_ASSERT(!to_.endsWith(QLatin1String("/")));
+    QString from = from_ + QLatin1String("/");
+    QString to = to_ + QLatin1String("/");
+
+    for (auto it = list.begin(); it != list.end(); ++it) {
+        if (it->startsWith(from)) {
+            *it = it->replace(0, from.size(), to);
+            changed = true;
+        }
+    }
+
+    if (changed) {
+        journal->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList, list);
+    }
+    return true;
+}
 
 }
 

src/libsync/propagateremotemove.h

@@ -51,6 +51,12 @@ public:
     void start() Q_DECL_OVERRIDE;
     void abort() Q_DECL_OVERRIDE;
     JobParallelism parallelism() Q_DECL_OVERRIDE { return OCC::PropagatorJob::WaitForFinishedInParentDirectory; }
+
+    /**
+     * Rename the directory in the selective sync list
+     */
+    static bool adjustSelectiveSync(SyncJournalDb *journal, const QString &from, const QString &to);
+
 private slots:
     void slotMoveJobFinished();
     void finalize();

src/libsync/propagatorjobs.cpp

@@ -15,7 +15,7 @@
 
 #include "propagatorjobs.h"
 #include "owncloudpropagator_p.h"
-
+#include "propagateremotemove.h"
 #include "utility.h"
 #include "syncjournaldb.h"
 #include "syncjournalfilerecord.h"
@@ -231,6 +231,7 @@ void PropagateLocalRename::start()
     _propagator->_journal->deleteFileRecord(_item->_originalFile);
 
     // store the rename file name in the item.
+    const auto oldFile = _item->_file;
     _item->_file = _item->_renameTarget;
 
     SyncJournalFileRecord record(*_item, targetFile);
@@ -245,9 +246,14 @@ void PropagateLocalRename::start()
             done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
             return;
         }
+    } else {
+        if (!PropagateRemoteMove::adjustSelectiveSync(_propagator->_journal, oldFile, _item->_renameTarget)) {
+            done(SyncFileItem::FatalError, tr("Error writing metadata to the database"));
+            return;
+        }
     }
-    _propagator->_journal->commit("localRename");
 
+    _propagator->_journal->commit("localRename");
 
     done(SyncFileItem::Success);
 }

src/libsync/syncengine.h

@@ -202,7 +202,6 @@ private:
     SyncJournalDb *_journal;
     QPointer<DiscoveryMainThread> _discoveryMainThread;
     QSharedPointer <OwncloudPropagator> _propagator;
-    QString _lastDeleted; // if the last item was a path and it has been deleted
 
     // After a sync, only the syncdb entries whose filenames appear in this
     // set will be kept. See _temporarilyUnavailablePaths.

src/libsync/syncjournaldb.cpp

@@ -318,7 +318,9 @@ bool SyncJournalDb::checkConnect()
         createQuery.bindValue(2, MIRALL_VERSION_MINOR);
         createQuery.bindValue(3, MIRALL_VERSION_PATCH);
         createQuery.bindValue(4, MIRALL_VERSION_BUILD);
-        createQuery.exec();
+        if (!createQuery.exec()) {
+            return sqlFail("Update version", createQuery);
+        }
 
     } else {
         int major = versionQuery.intValue(0);
@@ -367,60 +369,84 @@ bool SyncJournalDb::checkConnect()
     }
 
     _getFileRecordQuery.reset(new SqlQuery(_db));
-    _getFileRecordQuery->prepare(
+    if (_getFileRecordQuery->prepare(
             "SELECT path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize,"
             "  ignoredChildrenRemote, contentChecksum, contentchecksumtype.name"
             " FROM metadata"
             "  LEFT JOIN checksumtype as contentchecksumtype ON metadata.contentChecksumTypeId == contentchecksumtype.id"
-            " WHERE phash=?1" );
+            " WHERE phash=?1" )) {
+        return sqlFail("prepare _getFileRecordQuery", *_getFileRecordQuery);
+    }
 
     _setFileRecordQuery.reset(new SqlQuery(_db) );
-    _setFileRecordQuery->prepare("INSERT OR REPLACE INTO metadata "
+    if (_setFileRecordQuery->prepare("INSERT OR REPLACE INTO metadata "
                                  "(phash, pathlen, path, inode, uid, gid, mode, modtime, type, md5, fileid, remotePerm, filesize, ignoredChildrenRemote, contentChecksum, contentChecksumTypeId) "
-                                 "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7,  ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16);" );
+                                 "VALUES (?1 , ?2, ?3 , ?4 , ?5 , ?6 , ?7,  ?8 , ?9 , ?10, ?11, ?12, ?13, ?14, ?15, ?16);" )) {
+        return sqlFail("prepare _setFileRecordQuery", *_setFileRecordQuery);
+    }
 
     _setFileRecordChecksumQuery.reset(new SqlQuery(_db) );
-    _setFileRecordChecksumQuery->prepare(
+    if (_setFileRecordChecksumQuery->prepare(
             "UPDATE metadata"
             " SET contentChecksum = ?2, contentChecksumTypeId = ?3"
-            " WHERE phash == ?1;");
+            " WHERE phash == ?1;")) {
+        return sqlFail("prepare _setFileRecordChecksumQuery", *_setFileRecordChecksumQuery);
+    }
 
     _setFileRecordLocalMetadataQuery.reset(new SqlQuery(_db));
-    _setFileRecordLocalMetadataQuery->prepare(
+    if (_setFileRecordLocalMetadataQuery->prepare(
             "UPDATE metadata"
             " SET inode=?2, modtime=?3, filesize=?4"
-            " WHERE phash == ?1;");
+            " WHERE phash == ?1;")) {
+        return sqlFail("prepare _setFileRecordLocalMetadataQuery", *_setFileRecordLocalMetadataQuery);
+    }
  
     _getDownloadInfoQuery.reset(new SqlQuery(_db) );
-    _getDownloadInfoQuery->prepare( "SELECT tmpfile, etag, errorcount FROM "
-                                    "downloadinfo WHERE path=?1" );
+    if (_getDownloadInfoQuery->prepare( "SELECT tmpfile, etag, errorcount FROM "
+                                    "downloadinfo WHERE path=?1" )) {
+        return sqlFail("prepare _getDownloadInfoQuery", *_getDownloadInfoQuery);
+    }
 
     _setDownloadInfoQuery.reset(new SqlQuery(_db) );
-    _setDownloadInfoQuery->prepare( "INSERT OR REPLACE INTO downloadinfo "
+    if (_setDownloadInfoQuery->prepare( "INSERT OR REPLACE INTO downloadinfo "
                                     "(path, tmpfile, etag, errorcount) "
-                                    "VALUES ( ?1 , ?2, ?3, ?4 )" );
+                                    "VALUES ( ?1 , ?2, ?3, ?4 )" )) {
+        return sqlFail("prepare _setDownloadInfoQuery", *_setDownloadInfoQuery);
+    }
 
     _deleteDownloadInfoQuery.reset(new SqlQuery(_db) );
-    _deleteDownloadInfoQuery->prepare( "DELETE FROM downloadinfo WHERE path=?1" );
+    if (_deleteDownloadInfoQuery->prepare( "DELETE FROM downloadinfo WHERE path=?1" )) {
+        return sqlFail("prepare _deleteDownloadInfoQuery", *_deleteDownloadInfoQuery);
+    }
 
     _getUploadInfoQuery.reset(new SqlQuery(_db));
-    _getUploadInfoQuery->prepare( "SELECT chunk, transferid, errorcount, size, modtime FROM "
-                                  "uploadinfo WHERE path=?1" );
+    if (_getUploadInfoQuery->prepare( "SELECT chunk, transferid, errorcount, size, modtime FROM "
+                                  "uploadinfo WHERE path=?1" )) {
+        return sqlFail("prepare _getUploadInfoQuery", *_getUploadInfoQuery);
+    }
 
     _setUploadInfoQuery.reset(new SqlQuery(_db));
-    _setUploadInfoQuery->prepare( "INSERT OR REPLACE INTO uploadinfo "
+    if (_setUploadInfoQuery->prepare( "INSERT OR REPLACE INTO uploadinfo "
                                   "(path, chunk, transferid, errorcount, size, modtime) "
-                                  "VALUES ( ?1 , ?2, ?3 , ?4 ,  ?5, ?6 )");
+                                  "VALUES ( ?1 , ?2, ?3 , ?4 ,  ?5, ?6 )")) {
+        return sqlFail("prepare _setUploadInfoQuery", *_setUploadInfoQuery);
+    }
 
     _deleteUploadInfoQuery.reset(new SqlQuery(_db));
-    _deleteUploadInfoQuery->prepare("DELETE FROM uploadinfo WHERE path=?1" );
+    if (_deleteUploadInfoQuery->prepare("DELETE FROM uploadinfo WHERE path=?1" )) {
+        return sqlFail("prepare _deleteUploadInfoQuery", *_deleteUploadInfoQuery);
+    }
 
 
     _deleteFileRecordPhash.reset(new SqlQuery(_db));
-    _deleteFileRecordPhash->prepare("DELETE FROM metadata WHERE phash=?1");
+    if (_deleteFileRecordPhash->prepare("DELETE FROM metadata WHERE phash=?1")) {
+        return sqlFail("prepare _deleteFileRecordPhash", *_deleteFileRecordPhash);
+    }
 
     _deleteFileRecordRecursively.reset(new SqlQuery(_db));
-    _deleteFileRecordRecursively->prepare("DELETE FROM metadata WHERE path LIKE(?||'/%')");
+    if (_deleteFileRecordRecursively->prepare("DELETE FROM metadata WHERE path LIKE(?||'/%')")) {
+        return sqlFail("prepare _deleteFileRecordRecursively", *_deleteFileRecordRecursively);
+    }
 
     QString sql( "SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget "
                  "FROM blacklist WHERE path=?1");
@@ -430,32 +456,50 @@ bool SyncJournalDb::checkConnect()
         sql += QLatin1String(" COLLATE NOCASE");
     }
     _getErrorBlacklistQuery.reset(new SqlQuery(_db));
-    _getErrorBlacklistQuery->prepare(sql);
+    if (_getErrorBlacklistQuery->prepare(sql)) {
+        return sqlFail("prepare _getErrorBlacklistQuery", *_getErrorBlacklistQuery);
+    }
 
     _setErrorBlacklistQuery.reset(new SqlQuery(_db));
-    _setErrorBlacklistQuery->prepare("INSERT OR REPLACE INTO blacklist "
+    if (_setErrorBlacklistQuery->prepare("INSERT OR REPLACE INTO blacklist "
                                 "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget) "
-                                "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)");
+                                "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)")) {
+        return sqlFail("prepare _setErrorBlacklistQuery", *_setErrorBlacklistQuery);
+    }
 
     _getSelectiveSyncListQuery.reset(new SqlQuery(_db));
-    _getSelectiveSyncListQuery->prepare("SELECT path FROM selectivesync WHERE type=?1");
+    if (_getSelectiveSyncListQuery->prepare("SELECT path FROM selectivesync WHERE type=?1")) {
+        return sqlFail("prepare _getSelectiveSyncListQuery", *_getSelectiveSyncListQuery);
+    }
 
     _getChecksumTypeIdQuery.reset(new SqlQuery(_db));
-    _getChecksumTypeIdQuery->prepare("SELECT id FROM checksumtype WHERE name=?1");
+    if (_getChecksumTypeIdQuery->prepare("SELECT id FROM checksumtype WHERE name=?1")) {
+        return sqlFail("prepare _getChecksumTypeIdQuery", *_getChecksumTypeIdQuery);
+    }
 
     _getChecksumTypeQuery.reset(new SqlQuery(_db));
-    _getChecksumTypeQuery->prepare("SELECT name FROM checksumtype WHERE id=?1");
+    if (_getChecksumTypeQuery->prepare("SELECT name FROM checksumtype WHERE id=?1")) {
+        return sqlFail("prepare _getChecksumTypeQuery", *_getChecksumTypeQuery);
+    }
 
     _insertChecksumTypeQuery.reset(new SqlQuery(_db));
-    _insertChecksumTypeQuery->prepare("INSERT OR IGNORE INTO checksumtype (name) VALUES (?1)");
+    if (_insertChecksumTypeQuery->prepare("INSERT OR IGNORE INTO checksumtype (name) VALUES (?1)")) {
+        return sqlFail("prepare _insertChecksumTypeQuery", *_insertChecksumTypeQuery);
+    }
 
     _getDataFingerprintQuery.reset(new SqlQuery(_db));
-    _getDataFingerprintQuery->prepare("SELECT fingerprint FROM datafingerprint");
+    if (_getDataFingerprintQuery->prepare("SELECT fingerprint FROM datafingerprint")) {
+        return sqlFail("prepare _getDataFingerprintQuery", *_getDataFingerprintQuery);
+    }
 
     _setDataFingerprintQuery1.reset(new SqlQuery(_db));
-    _setDataFingerprintQuery1->prepare("DELETE FROM datafingerprint;");
+    if (_setDataFingerprintQuery1->prepare("DELETE FROM datafingerprint;")) {
+        return sqlFail("prepare _setDataFingerprintQuery1", *_setDataFingerprintQuery1);
+    }
     _setDataFingerprintQuery2.reset(new SqlQuery(_db));
-    _setDataFingerprintQuery2->prepare("INSERT INTO datafingerprint (fingerprint) VALUES (?1);");
+    if (_setDataFingerprintQuery2->prepare("INSERT INTO datafingerprint (fingerprint) VALUES (?1);")) {
+        return sqlFail("prepare _setDataFingerprintQuery2", *_setDataFingerprintQuery2);
+    }
 
     // don't start a new transaction now
     commitInternal(QString("checkConnect End"), false);

src/libsync/theme.cpp

@@ -115,11 +115,11 @@ QIcon Theme::trayFolderIcon( const QString& backend ) const
  * helper to load a icon from either the icon theme the desktop provides or from
  * the apps Qt resources.
  */
-QIcon Theme::themeIcon( const QString& name, bool sysTray ) const
+QIcon Theme::themeIcon( const QString& name, bool sysTray, bool sysTrayMenuVisible ) const
 {
     QString flavor;
     if (sysTray) {
-        flavor = systrayIconFlavor(_mono);
+        flavor = systrayIconFlavor(_mono, sysTrayMenuVisible);
     } else {
         flavor = QLatin1String("colored");
     }
@@ -158,6 +158,14 @@ QIcon Theme::themeIcon( const QString& name, bool sysTray ) const
         }
     }
 
+    #ifdef Q_OS_MAC
+    #if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)
+    // This defines the icon as a template and enables automatic macOS color handling
+    // See https://bugreports.qt.io/browse/QTBUG-42109
+    cached.setIsMask(_mono && sysTray && !sysTrayMenuVisible);
+    #endif
+    #endif
+
     return cached;
 }
 
@@ -227,11 +235,18 @@ QString Theme::defaultClientFolder() const
     return appName();
 }
 
-QString Theme::systrayIconFlavor(bool mono) const
+QString Theme::systrayIconFlavor(bool mono, bool sysTrayMenuVisible ) const
 {
+    Q_UNUSED(sysTrayMenuVisible)
     QString flavor;
     if (mono) {
         flavor = Utility::hasDarkSystray() ? QLatin1String("white") : QLatin1String("black");
+
+        #ifdef Q_OS_MAC
+        if (sysTrayMenuVisible) {
+            flavor = QLatin1String("white");
+        }
+        #endif
     } else {
         flavor = QLatin1String("colored");
     }
@@ -331,7 +346,7 @@ QVariant Theme::customMedia( CustomMediaType type )
     return re;
 }
 
-QIcon Theme::syncStateIcon( SyncResult::Status status, bool sysTray ) const
+QIcon Theme::syncStateIcon( SyncResult::Status status, bool sysTray, bool sysTrayMenuVisible ) const
 {
     // FIXME: Mind the size!
     QString statusIcon;
@@ -363,7 +378,7 @@ QIcon Theme::syncStateIcon( SyncResult::Status status, bool sysTray ) const
         statusIcon = QLatin1String("state-error");
     }
 
-    return themeIcon( statusIcon, sysTray );
+    return themeIcon( statusIcon, sysTray, sysTrayMenuVisible );
 }
 
 QIcon Theme::folderDisabledIcon( ) const
@@ -371,9 +386,9 @@ QIcon Theme::folderDisabledIcon( ) const
     return themeIcon( QLatin1String("state-pause") );
 }
 
-QIcon Theme::folderOfflineIcon(bool systray) const
+QIcon Theme::folderOfflineIcon(bool sysTray, bool sysTrayMenuVisible ) const
 {
-    return themeIcon( QLatin1String("state-offline"), systray );
+    return themeIcon( QLatin1String("state-offline"), sysTray, sysTrayMenuVisible );
 }
 
 QColor Theme::wizardHeaderTitleColor() const

src/libsync/theme.h

@@ -97,10 +97,10 @@ public:
     /**
       * get an sync state icon
       */
-    virtual QIcon   syncStateIcon( SyncResult::Status, bool sysTray = false ) const;
+    virtual QIcon   syncStateIcon( SyncResult::Status, bool sysTray = false, bool sysTrayMenuVisible = false) const;
 
     virtual QIcon   folderDisabledIcon() const;
-    virtual QIcon   folderOfflineIcon(bool systray = false) const;
+    virtual QIcon   folderOfflineIcon(bool sysTray = false, bool sysTrayMenuVisible = false) const;
     virtual QIcon   applicationIcon() const = 0;
 #endif
 
@@ -152,7 +152,7 @@ public:
     virtual QString enforcedLocale() const { return QString::null; }
 
     /** colored, white or black */
-    QString systrayIconFlavor(bool mono) const;
+    QString systrayIconFlavor(bool mono, bool sysTrayMenuVisible = false) const;
 
 #ifndef TOKEN_AUTH_ONLY
     /**
@@ -304,7 +304,7 @@ public:
 
 protected:
 #ifndef TOKEN_AUTH_ONLY
-    QIcon themeIcon(const QString& name, bool sysTray = false) const;
+    QIcon themeIcon(const QString& name, bool sysTray = false, bool sysTrayMenuVisible = false) const;
 #endif
     Theme();
 

test/syncenginetestutils.h

@@ -28,6 +28,7 @@ inline QByteArray generateFileId() {
 
 class PathComponents : public QStringList {
 public:
+    PathComponents(const char *path) : PathComponents{QString::fromUtf8(path)} {}
     PathComponents(const QString &path) : QStringList{path.split('/', QString::SkipEmptyParts)} { }
     PathComponents(const QStringList &pathComponents) : QStringList{pathComponents} { }
 
@@ -90,8 +91,9 @@ public:
     void mkdir(const QString &relativePath) override {
         _rootDir.mkpath(relativePath);
     }
-    void rename(const QString &, const QString &) override {
-        Q_ASSERT(!"not implemented");
+    void rename(const QString &from, const QString &to) override {
+        QVERIFY(_rootDir.exists(from));
+        QVERIFY(_rootDir.rename(from, to));
     }
 };
 
@@ -457,6 +459,36 @@ public:
     qint64 readData(char *, qint64) override { return 0; }
 };
 
+class FakeMoveReply : public QNetworkReply
+{
+    Q_OBJECT
+public:
+    FakeMoveReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, QObject *parent)
+    : QNetworkReply{parent} {
+        setRequest(request);
+        setUrl(request.url());
+        setOperation(op);
+        open(QIODevice::ReadOnly);
+
+        Q_ASSERT(request.url().path().startsWith(sRootUrl.path()));
+        QString fileName = request.url().path().mid(sRootUrl.path().length());
+        QString destPath = request.rawHeader("Destination");
+        Q_ASSERT(destPath.startsWith(sRootUrl.path()));
+        QString dest = destPath.mid(sRootUrl.path().length());
+        remoteRootFileInfo.rename(fileName, dest);
+        QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection);
+    }
+
+    Q_INVOKABLE void respond() {
+        setAttribute(QNetworkRequest::HttpStatusCodeAttribute, 201);
+        emit metaDataChanged();
+        emit finished();
+    }
+
+    void abort() override { }
+    qint64 readData(char *, qint64) override { return 0; }
+};
+
 class FakeGetReply : public QNetworkReply
 {
     Q_OBJECT
@@ -552,6 +584,8 @@ protected:
             return new FakeMkcolReply{_remoteRootFileInfo, op, request, this};
         else if (verb == QLatin1String("DELETE"))
             return new FakeDeleteReply{_remoteRootFileInfo, op, request, this};
+        else if (verb == QLatin1String("MOVE"))
+            return new FakeMoveReply{_remoteRootFileInfo, op, request, this};
         else {
             qDebug() << verb << outgoingData;
             Q_UNREACHABLE();

test/testfolderman.cpp

@@ -15,6 +15,7 @@
 #include "folderman.h"
 #include "account.h"
 #include "accountstate.h"
+#include "configfile.h"
 
 using namespace OCC;
 
@@ -39,6 +40,7 @@ private slots:
     {
 #if QT_VERSION >= QT_VERSION_CHECK(5, 1, 0)
         QTemporaryDir dir;
+        ConfigFile::setConfDir(dir.path()); // we don't want to pollute the user's config file
         QVERIFY(dir.isValid());
         QDir dir2(dir.path());
         QVERIFY(dir2.mkpath("sub/ownCloud1/folder/f"));

test/testinotifywatcher.cpp

@@ -11,18 +11,23 @@
 
 using namespace OCC;
 
-class TestInotifyWatcher: public FolderWatcherPrivate
+
+struct FriendlyFolderWatcherPrivate : FolderWatcherPrivate
+{
+    using FolderWatcherPrivate::FolderWatcherPrivate;
+    friend class TestInotifyWatcher;
+};
+
+
+class TestInotifyWatcher : public QObject
 {
     Q_OBJECT
-
-private:
-    QString _root;
-
 private slots:
-    void initTestCase() {
-        qsrand(QTime::currentTime().msec());
+    // Test the recursive path listing function lists everything
+    void testAddFolderRecursiveHelper() {
+        QTemporaryDir tmpDir;
 
-        _root = QDir::tempPath() + "/" + "test_" + QString::number(qrand());
+        QString _root = tmpDir.path();
         qDebug() << "creating test directory tree in " << _root;
         QDir rootDir(_root);
 
@@ -32,13 +37,13 @@ private slots:
         rootDir.mkpath(_root + "/a1/b3/c3");
         rootDir.mkpath(_root + "/a2/b3/c3");
 
-    }
+        FriendlyFolderWatcherPrivate watcher(0, _root);
+        QVERIFY(watcher._fd >= 0);
+        QCoreApplication::processEvents(); // Let the slotAddFolderRecursive slot run;
+        QStringList dirs = watcher._watches.values();
 
-    // Test the recursive path listing function findFoldersBelow
-    void testDirsBelowPath() {
-        QStringList dirs;
+        QVERIFY( dirs.indexOf(_root)>-1);
 
-        bool ok = findFoldersBelow(QDir(_root), dirs);
         QVERIFY( dirs.indexOf(_root + "/a1")>-1);
         QVERIFY( dirs.indexOf(_root + "/a1/b1")>-1);
         QVERIFY( dirs.indexOf(_root + "/a1/b1/c1")>-1);
@@ -53,20 +58,13 @@ private slots:
         QVERIFY( dirs.indexOf(_root + "/a1/b3")>-1);
         QVERIFY( dirs.indexOf(_root + "/a1/b3/c3")>-1);
 
-        QVERIFY( dirs.indexOf(_root + "/a2"));
-        QVERIFY( dirs.indexOf(_root + "/a2/b3"));
-        QVERIFY( dirs.indexOf(_root + "/a2/b3/c3"));
+        QVERIFY( dirs.contains(_root + "/a2"));
+        QVERIFY( dirs.contains(_root + "/a2/b3"));
+        QVERIFY( dirs.contains(_root + "/a2/b3/c3"));
 
-        QVERIFY2(dirs.count() == 11, "Directory count wrong.");
-
-        QVERIFY2(ok, "findFoldersBelow failed.");
+        QCOMPARE(dirs.count(), 12);
     }
 
-    void cleanupTestCase() {
-        if( _root.startsWith(QDir::tempPath() )) {
-           system( QString("rm -rf %1").arg(_root).toLocal8Bit() );
-        }
-    }
 };
 
 QTEST_APPLESS_MAIN(TestInotifyWatcher)

test/testsyncengine.cpp

@@ -7,6 +7,7 @@
 
 #include <QtTest>
 #include "syncenginetestutils.h"
+#include <syncengine.h>
 
 using namespace OCC;
 
@@ -137,8 +138,8 @@ private slots:
         fakeFolder.syncOnce();
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
         auto oldState = fakeFolder.currentLocalState();
-        QVERIFY(oldState.find(PathComponents("folder/folderB/folderA/file.txt")));
-        QVERIFY(!oldState.find(PathComponents("folder/folderA/file.txt")));
+        QVERIFY(oldState.find("folder/folderB/folderA/file.txt"));
+        QVERIFY(!oldState.find("folder/folderA/file.txt"));
 
         // This sync should not remove the file
         fakeFolder.syncOnce();
@@ -146,6 +147,72 @@ private slots:
         QCOMPARE(fakeFolder.currentLocalState(), oldState);
 
     }
+
+    void testSelectiveSyncModevFolder() {
+        // issue #5224
+        FakeFolder fakeFolder{FileInfo{ QString(), {
+            FileInfo { QStringLiteral("parentFolder"), {
+                FileInfo{ QStringLiteral("subFolderA"), { { QStringLiteral("fileA.txt"), 400 } } },
+                FileInfo{ QStringLiteral("subFolderB"), { { QStringLiteral("fileB.txt"), 400 } } }
+            }
+        }}}};
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        auto expectedServerState = fakeFolder.currentRemoteState();
+
+        // Remove subFolderA with selectiveSync:
+        fakeFolder.syncEngine().journal()->setSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList,
+                                                                {"parentFolder/subFolderA/"});
+        fakeFolder.syncEngine().journal()->avoidReadFromDbOnNextSync("parentFolder/subFolderA/");
+
+        fakeFolder.syncOnce();
+
+        {
+            // Nothing changed on the server
+            QCOMPARE(fakeFolder.currentRemoteState(), expectedServerState);
+            // The local state should not have subFolderA
+            auto remoteState = fakeFolder.currentRemoteState();
+            remoteState.remove("parentFolder/subFolderA");
+            QCOMPARE(fakeFolder.currentLocalState(), remoteState);
+        }
+
+        // Rename parentFolder on the server
+        fakeFolder.remoteModifier().rename("parentFolder", "parentFolderRenamed");
+        expectedServerState = fakeFolder.currentRemoteState();
+        fakeFolder.syncOnce();
+
+        {
+            QCOMPARE(fakeFolder.currentRemoteState(), expectedServerState);
+            auto remoteState = fakeFolder.currentRemoteState();
+            // The subFolderA should still be there on the server.
+            QVERIFY(remoteState.find("parentFolderRenamed/subFolderA/fileA.txt"));
+            // But not on the client because of the selective sync
+            remoteState.remove("parentFolderRenamed/subFolderA");
+            QCOMPARE(fakeFolder.currentLocalState(), remoteState);
+        }
+
+        // Rename it again, locally this time.
+        fakeFolder.localModifier().rename("parentFolderRenamed", "parentThirdName");
+        fakeFolder.syncOnce();
+
+        {
+            auto remoteState = fakeFolder.currentRemoteState();
+            // The subFolderA should still be there on the server.
+            QVERIFY(remoteState.find("parentThirdName/subFolderA/fileA.txt"));
+            // But not on the client because of the selective sync
+            remoteState.remove("parentThirdName/subFolderA");
+            QCOMPARE(fakeFolder.currentLocalState(), remoteState);
+
+            expectedServerState = fakeFolder.currentRemoteState();
+            QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItem &, const PropagatorJob &)));
+            fakeFolder.syncOnce(); // This sync should do nothing
+            QCOMPARE(completeSpy.count(), 0);
+
+            QCOMPARE(fakeFolder.currentRemoteState(), expectedServerState);
+            QCOMPARE(fakeFolder.currentLocalState(), remoteState);
+        }
+    }
+
 };
 
 QTEST_GUILESS_MAIN(TestSyncEngine)