OpenVFS fix pin state

src/libsync/discoveryphase.h

@@ -63,8 +63,7 @@ class DiscoverySingleLocalDirectoryJob : public QObject, public QRunnable
 private:
     QString _localPath;
     AccountPtr _account;
-    OCC::Vfs* _vfs;
-public:
+    OCC::Vfs *_vfs;
 };
 
 

src/plugins/vfs/xattr/vfs_xattr.cpp

@@ -190,31 +190,38 @@ void VfsXAttr::startImpl(const VfsSetupParams &params)
     qCDebug(lcVfsXAttr, "Start XAttr VFS");
 
     // Lets claim the sync root directory for us
-    const auto path = params.root();
     // set the owner to opencloud to claim it
-    if (!FileSystem::Xattr::setxattr(path, QString::fromUtf8(OpenVfsConstants::XAttributeNames::Owner), xattrOwnerString(params.account->uuid()))) {
+    if (!FileSystem::Xattr::setxattr(params.root(), QString::fromUtf8(OpenVfsConstants::XAttributeNames::Owner), xattrOwnerString(params.account->uuid()))) {
         Q_EMIT error(tr("Unable to claim the sync root for files on demand"));
         return;
     }
 
-    auto vfsProcess = new QProcess(this);
+    qCDebug(lcVfsXAttr) << "Mounting" << openVFSExePath() << params.root().toString();
+    _openVfsProcess = new QProcess(this);
     // merging the channels and piping the output to our log lead to deadlocks
-    vfsProcess->setProcessChannelMode(QProcess::ForwardedChannels);
-    const auto logPrefix = [vfsProcess, path = params.root().toString()] { return u"[%1 %2] "_s.arg(QString::number(vfsProcess->processId()), path); };
-    connect(vfsProcess, &QProcess::finished, vfsProcess, [logPrefix, vfsProcess] {
-        qCInfo(lcVfsXAttr) << logPrefix() << "finished" << vfsProcess->exitCode();
-        vfsProcess->deleteLater();
+    _openVfsProcess->setProcessChannelMode(QProcess::ForwardedChannels);
+    const auto logPrefix = [path = params.root().toString(), this] { return u"[%1 %2] "_s.arg(QString::number(_openVfsProcess->processId()), path); };
+    connect(_openVfsProcess, &QProcess::finished, this, [logPrefix, this] {
+        qCInfo(lcVfsXAttr) << logPrefix() << "finished" << _openVfsProcess->exitCode();
+        _openVfsProcess->deleteLater();
     });
-    connect(vfsProcess, &QProcess::started, this, [logPrefix, this] {
+    connect(_openVfsProcess, &QProcess::started, this, [logPrefix, this] {
         qCInfo(lcVfsXAttr) << logPrefix() << u"started";
-        Q_EMIT started();
+        // TODO:
+        // give it time to mount
+        QTimer::singleShot(1s, this, &Vfs::started);
     });
-    connect(vfsProcess, &QProcess::errorOccurred, this, [logPrefix, vfsProcess] { qCWarning(lcVfsXAttr) << logPrefix() << vfsProcess->errorString(); });
-    vfsProcess->start(openVFSExePath().toString(), {u"-d"_s, u"-i"_s, openVFSConfigFilePath().toString(), params.root().toString()}, QIODevice::ReadOnly);
+    connect(_openVfsProcess, &QProcess::errorOccurred, this, [logPrefix, this] { qCWarning(lcVfsXAttr) << logPrefix() << _openVfsProcess->errorString(); });
+    _openVfsProcess->start(openVFSExePath().toString(), {u"-d"_s, u"-i"_s, openVFSConfigFilePath().toString(), params.root().toString()}, QIODevice::ReadOnly);
 }
 
 void VfsXAttr::stop()
 {
+    if (_openVfsProcess) {
+        _openVfsProcess->terminate();
+        _openVfsProcess->waitForFinished();
+        _openVfsProcess->deleteLater();
+    }
 }
 
 void VfsXAttr::unregisterFolder()
@@ -496,12 +503,17 @@ bool VfsXAttr::setPinState(const QString &folderPath, PinState state)
 
 Optional<PinState> VfsXAttr::pinState(const QString &folderPath)
 {
-    const auto attribs = placeHolderAttributes(params().root() / folderPath);
-    if (!attribs) {
-        qCDebug(lcVfsXAttr) << u"Couldn't find pin state for regular non-placeholder file" << folderPath;
-        return {};
+    for (auto relativePath = FileSystem::Path::relative(folderPath).get();; relativePath = relativePath.parent_path()) {
+        const auto attributes = placeHolderAttributes(params().root() / relativePath);
+        if (!attributes) {
+            qCDebug(lcVfsXAttr) << "Couldn't find pin state for placeholder file" << folderPath;
+            return {};
+        }
+        // if the state is inherited and we still have a parent path, retreive that instead.
+        if (attributes.pinState != OpenVfsConstants::PinStates::Inherited || !relativePath.has_relative_path()) {
+            return convertPinState(attributes.pinState);
+        }
     }
-    return convertPinState(attribs.pinState);
 }
 
 Vfs::AvailabilityResult VfsXAttr::availability(const QString &folderPath)

src/plugins/vfs/xattr/vfs_xattr.h

@@ -15,6 +15,8 @@
 #include "common/plugin.h"
 #include "common/result.h"
 
+#include <QProcess>
+
 
 namespace OCC {
 class HydrationJob;
@@ -62,6 +64,7 @@ public Q_SLOTS:
 
 private:
     QMap<QByteArray, HydrationJob*> _hydrationJobs;
+    QPointer<QProcess> _openVfsProcess;
 };
 
 class XattrVfsPluginFactory : public QObject, public DefaultPluginFactory<VfsXAttr>

test/testblacklist.cpp

@@ -28,6 +28,11 @@ private Q_SLOTS:
 
         QTest::newRow("Vfs::Mode::Off") << Vfs::Mode::Off << false;
 
+        if (VfsPluginManager::instance().isVfsPluginAvailable(Vfs::Mode::XAttr)) {
+            QTest::newRow("Vfs::Mode::Xattr dehydrdeated") << Vfs::Mode::XAttr << false;
+            QTest::newRow("Vfs::Mode::Xattr hydrated") << Vfs::Mode::XAttr << true;
+        }
+
         if (VfsPluginManager::instance().isVfsPluginAvailable(Vfs::Mode::WindowsCfApi)) {
             QTest::newRow("Vfs::Mode::WindowsCfApi dehydrated") << Vfs::Mode::WindowsCfApi << true;
 
@@ -82,7 +87,7 @@ private Q_SLOTS:
         modifier.insert(testFileName);
         fakeFolder.serverErrorPaths().append(testFileName, 500); // will be blacklisted
         const bool syncResult = fakeFolder.applyLocalModificationsAndSync();
-        if (vfsMode == Vfs::Mode::WindowsCfApi && filesAreDehydrated && remote) {
+        if (filesAreDehydrated && remote) {
             // With dehydrated files, only a PROPFIND is done, but not a GET request.
             // And it is the GET request that fails, and causes a blacklist entry, all "syncs" will succeed.
             QVERIFY(syncResult);

test/testdatabaseerror.cpp

@@ -24,11 +24,14 @@ private Q_SLOTS:
 
         QTest::newRow("Vfs::Mode::Off") << Vfs::Mode::Off << false;
 
+        if (VfsPluginManager::instance().isVfsPluginAvailable(Vfs::Mode::XAttr)) {
+            QTest::newRow("Vfs::Mode::Xattr dehydrdeated") << Vfs::Mode::XAttr << false;
+            QTest::newRow("Vfs::Mode::Xattr hydrated") << Vfs::Mode::XAttr << true;
+        }
+
         if (VfsPluginManager::instance().isVfsPluginAvailable(Vfs::Mode::WindowsCfApi)) {
             QTest::newRow("Vfs::Mode::WindowsCfApi dehydrated") << Vfs::Mode::WindowsCfApi << true;
-
-            // TODO: the hydrated version will fail due to an issue in the winvfs plugin, so leave it disabled for now.
-            // QTest::newRow("Vfs::Mode::WindowsCfApi hydrated") << Vfs::Mode::WindowsCfApi << false;
+            QTest::newRow("Vfs::Mode::WindowsCfApi hydrated") << Vfs::Mode::WindowsCfApi << false;
         } else if (Utility::isWindows()) {
             qWarning("Skipping Vfs::Mode::WindowsCfApi");
         }
@@ -43,8 +46,8 @@ private Q_SLOTS:
         QFETCH_GLOBAL(Vfs::Mode, vfsMode);
         QFETCH_GLOBAL(bool, filesAreDehydrated);
 
-        if (vfsMode == Vfs::Mode::WindowsCfApi) {
-            QSKIP("Known to be broken, see https://github.com/owncloud/client-desktop-vfs-win/issues/22");
+        if (filesAreDehydrated) {
+            QSKIP("Appending to a virtual file the client doesn't know about can never work");
         }
 
         FileInfo finalState;

test/testdownload.cpp

@@ -73,6 +73,11 @@ private Q_SLOTS:
 
         QTest::newRow("Vfs::Mode::Off") << Vfs::Mode::Off << false;
 
+        if (VfsPluginManager::instance().isVfsPluginAvailable(Vfs::Mode::XAttr)) {
+            QTest::newRow("Vfs::Mode::Xattr dehydrdeated") << Vfs::Mode::XAttr << false;
+            QTest::newRow("Vfs::Mode::Xattr hydrated") << Vfs::Mode::XAttr << true;
+        }
+
         if (VfsPluginManager::instance().isVfsPluginAvailable(Vfs::Mode::WindowsCfApi)) {
             QTest::newRow("Vfs::Mode::WindowsCfApi dehydrated") << Vfs::Mode::WindowsCfApi << true;
 

test/testutils/syncenginetestutils.cpp

@@ -865,13 +865,14 @@ FakeFolder::FakeFolder(const FileInfo &fileTemplate, OCC::Vfs::Mode vfsMode, boo
         Q_ASSERT(vfs);
     }
 
-    // Ensure we have a valid Vfs instance "running"
-    switchToVfs(vfs);
-
     if (vfsMode != OCC::Vfs::Mode::Off) {
-        const auto pinState = filesAreDehydrated ? OCC::PinState::OnlineOnly : OCC::PinState::AlwaysLocal;
-        OC_ENFORCE(vfs->setPinState(QString(), pinState));
+        connect(vfs.data(), &OCC::Vfs::started, this, [vfs, filesAreDehydrated] {
+            const auto pinState = filesAreDehydrated ? OCC::PinState::OnlineOnly : OCC::PinState::AlwaysLocal;
+            OC_ENFORCE(vfs->setPinState(QString(), pinState));
+        });
     }
+    // Ensure we have a valid Vfs instance "running"
+    switchToVfs(vfs);
 
     // A new folder will update the local file state database on first sync.
     // To have a state matching what users will encounter, we have to a sync