Merge pull request #3589 from nextcloud/bugfix/warnings

Don't ignore setPinState() results and fix some other warnings

src/csync/vio/csync_vio_local_unix.cpp

@@ -124,7 +124,8 @@ std::unique_ptr<csync_file_stat_t> csync_vio_local_readdir(csync_vio_handle_t *h
   if (vfs) {
       // Directly modifies file_stat->type.
       // We can ignore the return value since we're done here anyway.
-      vfs->statTypeVirtualFile(file_stat.get(), &handle->path);
+      const auto result = vfs->statTypeVirtualFile(file_stat.get(), &handle->path);
+      Q_UNUSED(result)
   }
 
   return file_stat;

src/gui/accountsettings.cpp

@@ -826,7 +826,9 @@ void AccountSettings::slotEnableVfsCurrentFolder()
             folder->setRootPinState(PinState::Unspecified);
             for (const auto &entry : oldBlacklist) {
                 folder->journalDb()->schedulePathForRemoteDiscovery(entry);
-                folder->vfs().setPinState(entry, PinState::OnlineOnly);
+                if (!folder->vfs().setPinState(entry, PinState::OnlineOnly)) {
+                    qCWarning(lcAccountSettings) << "Could not set pin state of" << entry << "to online only";
+                }
             }
             folder->slotNextSyncFullLocalDiscovery();
 
@@ -932,7 +934,9 @@ void AccountSettings::slotSetSubFolderAvailability(Folder *folder, const QString
     Q_ASSERT(!path.endsWith('/'));
 
     // Update the pin state on all items
-    folder->vfs().setPinState(path, state);
+    if (!folder->vfs().setPinState(path, state)) {
+        qCWarning(lcAccountSettings) << "Could not set pin state of" << path << "to" << state;
+    }
 
     // Trigger sync
     folder->schedulePathForLocalDiscovery(path);

src/gui/application.cpp

@@ -478,7 +478,6 @@ void Application::slotCrash()
 
 void Application::slotownCloudWizardDone(int res)
 {
-    AccountManager *accountMan = AccountManager::instance();
     FolderMan *folderMan = FolderMan::instance();
 
     // During the wizard, scheduling of new syncs is disabled
@@ -491,7 +490,7 @@ void Application::slotownCloudWizardDone(int res)
 
         // If one account is configured: enable autostart
 #ifndef QT_DEBUG
-        bool shouldSetAutoStart = (accountMan->accounts().size() == 1);
+        bool shouldSetAutoStart = AccountManager::instance()->accounts().size() == 1;
 #else
         bool shouldSetAutoStart = false;
 #endif

src/gui/folder.cpp

@@ -636,7 +636,9 @@ void Folder::implicitlyHydrateFile(const QString &relativepath)
     // (suffix-virtual file's pin state is stored at the hydrated path)
     const auto pin = _vfs->pinState(relativepath);
     if (pin && *pin == PinState::OnlineOnly) {
-        _vfs->setPinState(relativepath, PinState::Unspecified);
+        if (!_vfs->setPinState(relativepath, PinState::Unspecified)) {
+            qCWarning(lcFolder) << "Could not set pin state of" << relativepath << "to unspecified";
+        }
     }
 
     // Add to local discovery
@@ -675,7 +677,9 @@ void Folder::setVirtualFilesEnabled(bool enabled)
 
 void Folder::setRootPinState(PinState state)
 {
-    _vfs->setPinState(QString(), state);
+    if (!_vfs->setPinState(QString(), state)) {
+        qCWarning(lcFolder) << "Could not set root pin state of" << _definition.alias;
+    }
 
     // We don't actually need discovery, but it's important to recurse
     // into all folders, so the changes can be applied.

src/gui/folderman.cpp

@@ -1075,7 +1075,7 @@ Folder *FolderMan::addFolder(AccountState *accountState, const FolderDefinition
     // Migration: The first account that's configured for a local folder shall
     // be saved in a backwards-compatible way.
     const auto folderList = FolderMan::instance()->map();
-    const auto oneAccountOnly = std::none_of(folderList.cbegin(), folderList.cend(), [this, folder](const auto *other) {
+    const auto oneAccountOnly = std::none_of(folderList.cbegin(), folderList.cend(), [folder](const auto *other) {
         return other != folder && other->cleanPath() == folder->cleanPath();
     });
 

src/gui/socketapi.cpp

@@ -796,7 +796,9 @@ void SocketApi::command_MAKE_AVAILABLE_LOCALLY(const QString &filesArg, SocketLi
             continue;
 
         // Update the pin state on all items
-        data.folder->vfs().setPinState(data.folderRelativePath, PinState::AlwaysLocal);
+        if (!data.folder->vfs().setPinState(data.folderRelativePath, PinState::AlwaysLocal)) {
+            qCWarning(lcSocketApi) << "Could not set pin state of" << data.folderRelativePath << "to always local";
+        }
 
         // Trigger sync
         data.folder->schedulePathForLocalDiscovery(data.folderRelativePath);
@@ -815,7 +817,9 @@ void SocketApi::command_MAKE_ONLINE_ONLY(const QString &filesArg, SocketListener
             continue;
 
         // Update the pin state on all items
-        data.folder->vfs().setPinState(data.folderRelativePath, PinState::OnlineOnly);
+        if (!data.folder->vfs().setPinState(data.folderRelativePath, PinState::OnlineOnly)) {
+            qCWarning(lcSocketApi) << "Could not set pin state of" << data.folderRelativePath << "to online only";
+        }
 
         // Trigger sync
         data.folder->schedulePathForLocalDiscovery(data.folderRelativePath);

src/gui/tray/UserModel.cpp

@@ -79,7 +79,7 @@ void User::showDesktopNotification(const QString &title, const QString &message)
     }
 
     // after one hour, clear the gui log notification store
-    constexpr quint64 clearGuiLogInterval = 60 * 60 * 1000;
+    constexpr qint64 clearGuiLogInterval = 60 * 60 * 1000;
     if (_guiLogTimer.elapsed() > clearGuiLogInterval) {
         _notificationCache.clear();
     }

src/gui/userstatus.cpp

@@ -45,23 +45,6 @@ namespace {
         // it matches _preDefinedStatus, otherwise the default is online (0)
         return preDefinedStatus.value(status.toLower(), UserStatus::Status::Online);
     }
-    
-    QString enumToString(UserStatus::Status status) 
-    {
-        switch (status) {
-        case UserStatus::Status::Away:
-            return QObject::tr("Away");
-        case UserStatus::Status::DoNotDisturb:
-            return QObject::tr("Do not disturb");
-        case UserStatus::Status::Invisible:
-        case UserStatus::Status::Offline:
-            return QObject::tr("Offline");
-        case UserStatus::Status::Online:
-            return QObject::tr("Online");
-        }
-        
-        Q_UNREACHABLE();
-    }
 }
 
 UserStatus::UserStatus(QObject *parent)

src/libsync/clientsideencryption.cpp

@@ -1009,7 +1009,7 @@ void ClientSideEncryption::writePrivateKey(const AccountPtr &account)
     job->setInsecureFallback(false);
     job->setKey(kck);
     job->setBinaryData(_privateKey);
-    connect(job, &WritePasswordJob::finished, [this](Job *incoming) {
+    connect(job, &WritePasswordJob::finished, [](Job *incoming) {
         Q_UNUSED(incoming);
         qCInfo(lcCse()) << "Private key stored in keychain";
     });
@@ -1028,7 +1028,7 @@ void ClientSideEncryption::writeCertificate(const AccountPtr &account)
     job->setInsecureFallback(false);
     job->setKey(kck);
     job->setBinaryData(_certificate.toPem());
-    connect(job, &WritePasswordJob::finished, [this](Job *incoming) {
+    connect(job, &WritePasswordJob::finished, [](Job *incoming) {
         Q_UNUSED(incoming);
         qCInfo(lcCse()) << "Certificate stored in keychain";
     });

src/libsync/propagatedownload.cpp

@@ -1079,15 +1079,21 @@ void PropagateDownloadFile::downloadFinished()
             // Move the pin state to the new location
             auto pin = propagator()->_journal->internalPinStates().rawForPath(virtualFile.toUtf8());
             if (pin && *pin != PinState::Inherited) {
-                vfs->setPinState(_item->_file, *pin);
-                vfs->setPinState(virtualFile, PinState::Inherited);
+                if (!vfs->setPinState(_item->_file, *pin)) {
+                    qCWarning(lcPropagateDownload) << "Could not set pin state of" << _item->_file;
+                }
+                if (!vfs->setPinState(virtualFile, PinState::Inherited)) {
+                    qCWarning(lcPropagateDownload) << "Could not set pin state of" << virtualFile << " to inherited";
+                }
             }
         }
 
         // Ensure the pin state isn't contradictory
         auto pin = vfs->pinState(_item->_file);
         if (pin && *pin == PinState::OnlineOnly)
-            vfs->setPinState(_item->_file, PinState::Unspecified);
+            if (!vfs->setPinState(_item->_file, PinState::Unspecified)) {
+                qCWarning(lcPropagateDownload) << "Could not set pin state of" << _item->_file << "to unspecified";
+            }
     }
 
     updateMetadata(isConflict);

src/libsync/propagateremotemove.cpp

@@ -239,7 +239,9 @@ void PropagateRemoteMove::finalize()
 
     // Delete old db data.
     propagator()->_journal->deleteFileRecord(_item->_originalFile);
-    vfs->setPinState(_item->_originalFile, PinState::Inherited);
+    if (!vfs->setPinState(_item->_originalFile, PinState::Inherited)) {
+        qCWarning(lcPropagateRemoteMove) << "Could not set pin state of" << _item->_originalFile << "to inherited";
+    }
 
     SyncFileItem newItem(*_item);
     newItem._type = _item->_type;

src/libsync/propagateupload.cpp

@@ -781,7 +781,9 @@ void PropagateUploadFileCommon::finalize()
         auto &vfs = propagator()->syncOptions()._vfs;
         const auto pin = vfs->pinState(_item->_file);
         if (pin && *pin == PinState::OnlineOnly) {
-            vfs->setPinState(_item->_file, PinState::Unspecified);
+            if (!vfs->setPinState(_item->_file, PinState::Unspecified)) {
+                qCWarning(lcPropagateUpload) << "Could not set pin state of" << _item->_file << "to unspecified";
+            }
         }
     }
 

src/libsync/propagatorjobs.cpp

@@ -248,7 +248,9 @@ void PropagateLocalRename::start()
 
     auto &vfs = propagator()->syncOptions()._vfs;
     auto pinState = vfs->pinState(_item->_originalFile);
-    vfs->setPinState(_item->_originalFile, PinState::Inherited);
+    if (!vfs->setPinState(_item->_originalFile, PinState::Inherited)) {
+        qCWarning(lcPropagateLocalRename) << "Could not set pin state of" << _item->_originalFile << "to inherited";
+    }
 
     const auto oldFile = _item->_file;
 

test/CMakeLists.txt

@@ -9,6 +9,7 @@ add_library(testutils
   syncenginetestutils.cpp
   pushnotificationstestutils.cpp
   themeutils.cpp
+  testhelper.cpp
 )
 
 target_link_libraries(testutils PUBLIC ${APPLICATION_EXECUTABLE}sync Qt5::Test)

test/pushnotificationstestutils.cpp

@@ -1,6 +1,7 @@
 #include <QLoggingCategory>
 #include <QSignalSpy>
 #include <QTest>
+#include <cstdint>
 #include <functional>
 
 #include "pushnotificationstestutils.h"
@@ -116,15 +117,15 @@ uint32_t FakeWebSocketServer::textMessagesCount() const
     return _processTextMessageSpy->count();
 }
 
-QString FakeWebSocketServer::textMessage(uint32_t messageNumber) const
+QString FakeWebSocketServer::textMessage(int messageNumber) const
 {
-    Q_ASSERT(messageNumber < _processTextMessageSpy->count());
+    Q_ASSERT(0 <= messageNumber && messageNumber < _processTextMessageSpy->count());
     return _processTextMessageSpy->at(messageNumber).at(1).toString();
 }
 
-QWebSocket *FakeWebSocketServer::socketForTextMessage(uint32_t messageNumber) const
+QWebSocket *FakeWebSocketServer::socketForTextMessage(int messageNumber) const
 {
-    Q_ASSERT(messageNumber < _processTextMessageSpy->count());
+    Q_ASSERT(0 <= messageNumber && messageNumber < _processTextMessageSpy->count());
     return _processTextMessageSpy->at(messageNumber).at(0).value<QWebSocket *>();
 }
 

test/pushnotificationstestutils.h

@@ -40,9 +40,9 @@ public:
 
     uint32_t textMessagesCount() const;
 
-    QString textMessage(uint32_t messageNumber) const;
+    QString textMessage(int messageNumber) const;
 
-    QWebSocket *socketForTextMessage(uint32_t messageNumber) const;
+    QWebSocket *socketForTextMessage(int messageNumber) const;
 
     void clearTextMessages();
 

test/testhelper.cpp

@@ -0,0 +1,10 @@
+#include "testhelper.h"
+
+OCC::FolderDefinition folderDefinition(const QString &path)
+{
+    OCC::FolderDefinition d;
+    d.localPath = path;
+    d.targetPath = path;
+    d.alias = path;
+    return d;
+}

test/testhelper.h

@@ -1,12 +1,11 @@
 #ifndef TESTHELPER_H
 #define TESTHELPER_H
 
-#include "folder.h"
+#include "gui/folder.h"
 #include "creds/httpcredentials.h"
 
-using namespace OCC;
-
-class HttpCredentialsTest : public HttpCredentials {
+class HttpCredentialsTest : public OCC::HttpCredentials
+{
 public:
     HttpCredentialsTest(const QString& user, const QString& password)
     : HttpCredentials(user, password)
@@ -17,12 +16,6 @@ public:
     }
 };
 
-static FolderDefinition folderDefinition(const QString &path) {
-    FolderDefinition d;
-    d.localPath = path;
-    d.targetPath = path;
-    d.alias = path;
-    return d;
-}
+OCC::FolderDefinition folderDefinition(const QString &path);
 
 #endif // TESTHELPER_H

test/testsyncvirtualfiles.cpp

@@ -1211,8 +1211,8 @@ private slots:
         QCOMPARE(*vfs->availability("local"), VfsItemAvailability::Mixed);
         QCOMPARE(*vfs->availability("online"), VfsItemAvailability::Mixed);
 
-        vfs->setPinState("local", PinState::AlwaysLocal);
-        vfs->setPinState("online", PinState::OnlineOnly);
+        QVERIFY(vfs->setPinState("local", PinState::AlwaysLocal));
+        QVERIFY(vfs->setPinState("online", PinState::OnlineOnly));
         QVERIFY(fakeFolder.syncOnce());
 
         QCOMPARE(*vfs->availability("online"), VfsItemAvailability::OnlineOnly);
@@ -1293,13 +1293,13 @@ private slots:
         QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename" DVSUFFIX), PinState::OnlineOnly);
 
         // When a file is hydrated or dehydrated due to pin state it retains its pin state
-        vfs->setPinState("onlinerenamed2/file1rename" DVSUFFIX, PinState::AlwaysLocal);
+        QVERIFY(vfs->setPinState("onlinerenamed2/file1rename" DVSUFFIX, PinState::AlwaysLocal));
         QVERIFY(fakeFolder.syncOnce());
         QVERIFY(fakeFolder.currentLocalState().find("onlinerenamed2/file1rename"));
         QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::AlwaysLocal);
 
-        vfs->setPinState("onlinerenamed2", PinState::Unspecified);
-        vfs->setPinState("onlinerenamed2/file1rename", PinState::OnlineOnly);
+        QVERIFY(vfs->setPinState("onlinerenamed2", PinState::Unspecified));
+        QVERIFY(vfs->setPinState("onlinerenamed2/file1rename", PinState::OnlineOnly));
         QVERIFY(fakeFolder.syncOnce());
         QVERIFY(fakeFolder.currentLocalState().find("onlinerenamed2/file1rename" DVSUFFIX));
         QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename" DVSUFFIX), PinState::OnlineOnly);
@@ -1376,7 +1376,7 @@ private slots:
 
         cleanup();
         // Dehydrate
-        vfs->setPinState(QString(), PinState::OnlineOnly);
+        QVERIFY(vfs->setPinState(QString(), PinState::OnlineOnly));
         QVERIFY(!fakeFolder.syncOnce());
 
         QVERIFY(itemInstruction(completeSpy, "A/igno" DVSUFFIX, CSYNC_INSTRUCTION_IGNORE));

test/testsyncxattr.cpp

@@ -956,8 +956,8 @@ private slots:
         QCOMPARE(*vfs->availability("local"), VfsItemAvailability::Mixed);
         QCOMPARE(*vfs->availability("online"), VfsItemAvailability::Mixed);
 
-        vfs->setPinState("local", PinState::AlwaysLocal);
-        vfs->setPinState("online", PinState::OnlineOnly);
+        QVERIFY(vfs->setPinState("local", PinState::AlwaysLocal));
+        QVERIFY(vfs->setPinState("online", PinState::OnlineOnly));
         QVERIFY(fakeFolder.syncOnce());
 
         QCOMPARE(*vfs->availability("online"), VfsItemAvailability::OnlineOnly);
@@ -1037,13 +1037,13 @@ private slots:
         QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::OnlineOnly);
 
         // When a file is hydrated or dehydrated due to pin state it retains its pin state
-        vfs->setPinState("onlinerenamed2/file1rename", PinState::AlwaysLocal);
+        QVERIFY(vfs->setPinState("onlinerenamed2/file1rename", PinState::AlwaysLocal));
         QVERIFY(fakeFolder.syncOnce());
         QVERIFY(fakeFolder.currentLocalState().find("onlinerenamed2/file1rename"));
         QCOMPARE(*vfs->pinState("onlinerenamed2/file1rename"), PinState::AlwaysLocal);
 
-        vfs->setPinState("onlinerenamed2", PinState::Unspecified);
-        vfs->setPinState("onlinerenamed2/file1rename", PinState::OnlineOnly);
+        QVERIFY(vfs->setPinState("onlinerenamed2", PinState::Unspecified));
+        QVERIFY(vfs->setPinState("onlinerenamed2/file1rename", PinState::OnlineOnly));
         QVERIFY(fakeFolder.syncOnce());
 
         XAVERIFY_VIRTUAL(fakeFolder, "onlinerenamed2/file1rename");