Merge pull request #6573 from nextcloud/bugfix/fasterContextMenuWindo…

…wsVirtualFiles context menu: do not recursively check pin and availability states
nextcloud · Mar 27, 2024 · b6ff0c5 · b6ff0c5
2 parents 9715cb0 + 608a435
commit b6ff0c5
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 73 deletions.
diff --git a/src/common/vfs.h b/src/common/vfs.h
@@ -134,8 +134,15 @@ class OCSYNC_EXPORT Vfs : public QObject
         // Availability not available since the item doesn't exist
         NoSuchItem,
     };
+    Q_ENUM(AvailabilityError)
     using AvailabilityResult = Result<VfsItemAvailability, AvailabilityError>;
 
+    enum class AvailabilityRecursivity {
+        RecursiveAvailability,
+        NotRecursiveAvailability,
+    };
+    Q_ENUM(AvailabilityRecursivity)
+
 public:
     explicit Vfs(QObject* parent = nullptr);
     ~Vfs() override;
@@ -258,7 +265,7 @@ class OCSYNC_EXPORT Vfs : public QObject
      *
      * folderPath is relative to the sync folder. Can be "" for root folder.
      */
-    Q_REQUIRED_RESULT virtual AvailabilityResult availability(const QString &folderPath) = 0;
+    Q_REQUIRED_RESULT virtual AvailabilityResult availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck) = 0;
 
 public slots:
     /** Update in-sync state based on SyncFileStatusTracker signal.
@@ -320,15 +327,15 @@ class OCSYNC_EXPORT VfsOff : public Vfs
     Result<void, QString> updateMetadata(const QString &, time_t, qint64, const QByteArray &) override { return {}; }
     Result<void, QString> createPlaceholder(const SyncFileItem &) override { return {}; }
     Result<void, QString> dehydratePlaceholder(const SyncFileItem &) override { return {}; }
-    Result<ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &, const SyncFileItem &, const QString &, UpdateMetadataTypes) override { return ConvertToPlaceholderResult::Ok; }
+    Result<ConvertToPlaceholderResult, QString> convertToPlaceholder(const QString &, const SyncFileItem &, const QString &, const UpdateMetadataTypes) override { return ConvertToPlaceholderResult::Ok; }
 
     bool needsMetadataUpdate(const SyncFileItem &) override { return false; }
     bool isDehydratedPlaceholder(const QString &) override { return false; }
     bool statTypeVirtualFile(csync_file_stat_t *, void *) override { return false; }
 
     bool setPinState(const QString &, PinState) override { return true; }
     Optional<PinState> pinState(const QString &) override { return PinState::AlwaysLocal; }
-    AvailabilityResult availability(const QString &) override { return VfsItemAvailability::AlwaysLocal; }
+    AvailabilityResult availability(const QString &, const AvailabilityRecursivity) override { return VfsItemAvailability::AlwaysLocal; }
 
 public slots:
     void fileStatusChanged(const QString &, OCC::SyncFileStatus) override {}

diff --git a/src/gui/socketapi/socketapi.cpp b/src/gui/socketapi/socketapi.cpp
@@ -1459,7 +1459,7 @@ void SocketApi::command_GET_MENU_ITEMS(const QString &argument, OCC::SocketListe
         };
         for (const auto &file : files) {
             auto fileData = FileData::get(file);
-            auto availability = syncFolder->vfs().availability(fileData.folderRelativePath);
+            auto availability = syncFolder->vfs().availability(fileData.folderRelativePath, Vfs::AvailabilityRecursivity::NotRecursiveAvailability);
             if (!availability) {
                 if (availability.error() == Vfs::AvailabilityError::DbError)
                     availability = VfsItemAvailability::Mixed;

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
@@ -1290,7 +1290,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
         const bool isFilePlaceHolder = !localEntry.isDirectory && _discoveryData->_syncOptions._vfs->isDehydratedPlaceholder(_discoveryData->_localDir + path._local);
 
         // either correct availability, or a result with error if the folder is new or otherwise has no availability set yet
-        const auto folderPlaceHolderAvailability = localEntry.isDirectory ? _discoveryData->_syncOptions._vfs->availability(path._local) : Vfs::AvailabilityResult(Vfs::AvailabilityError::NoSuchItem);
+        const auto folderPlaceHolderAvailability = localEntry.isDirectory ? _discoveryData->_syncOptions._vfs->availability(path._local, Vfs::AvailabilityRecursivity::RecursiveAvailability) : Vfs::AvailabilityResult(Vfs::AvailabilityError::NoSuchItem);
 
         const auto folderPinState = localEntry.isDirectory ? _discoveryData->_syncOptions._vfs->pinState(path._local) : Optional<PinState>(PinState::Unspecified);
 

diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.cpp b/src/libsync/vfs/cfapi/vfs_cfapi.cpp
@@ -334,28 +334,47 @@ Optional<PinState> VfsCfApi::pinStateLocal(const QString &localPath) const
     return info.pinState();
 }
 
-Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath)
+Vfs::AvailabilityResult VfsCfApi::availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck)
 {
     const auto basePinState = pinState(folderPath);
-    const auto hydrationAndPinStates = computeRecursiveHydrationAndPinStates(folderPath, basePinState);
-
-    const auto pin = hydrationAndPinStates.pinState;
-    const auto hydrationStatus = hydrationAndPinStates.hydrationStatus;
-
-    if (hydrationStatus.hasDehydrated) {
-        if (hydrationStatus.hasHydrated)
-            return VfsItemAvailability::Mixed;
-        if (pin && *pin == PinState::OnlineOnly)
-            return VfsItemAvailability::OnlineOnly;
-        else
-            return VfsItemAvailability::AllDehydrated;
-    } else if (hydrationStatus.hasHydrated) {
-        if (pin && *pin == PinState::AlwaysLocal)
+    if (basePinState && recursiveCheck == Vfs::AvailabilityRecursivity::NotRecursiveAvailability) {
+        switch (*basePinState)
+        {
+        case OCC::PinState::AlwaysLocal:
             return VfsItemAvailability::AlwaysLocal;
-        else
-            return VfsItemAvailability::AllHydrated;
+            break;
+        case OCC::PinState::Excluded:
+            break;
+        case OCC::PinState::Inherited:
+            break;
+        case OCC::PinState::OnlineOnly:
+            return VfsItemAvailability::OnlineOnly;
+            break;
+        case OCC::PinState::Unspecified:
+            break;
+        };
+        return VfsItemAvailability::Mixed;
+    } else {
+        const auto hydrationAndPinStates = computeRecursiveHydrationAndPinStates(folderPath, basePinState);
+
+        const auto pin = hydrationAndPinStates.pinState;
+        const auto hydrationStatus = hydrationAndPinStates.hydrationStatus;
+
+        if (hydrationStatus.hasDehydrated) {
+            if (hydrationStatus.hasHydrated)
+                return VfsItemAvailability::Mixed;
+            if (pin && *pin == PinState::OnlineOnly)
+                return VfsItemAvailability::OnlineOnly;
+            else
+                return VfsItemAvailability::AllDehydrated;
+        } else if (hydrationStatus.hasHydrated) {
+            if (pin && *pin == PinState::AlwaysLocal)
+                return VfsItemAvailability::AlwaysLocal;
+            else
+                return VfsItemAvailability::AllHydrated;
+        }
+        return AvailabilityError::NoSuchItem;
     }
-    return AvailabilityError::NoSuchItem;
 }
 
 HydrationJob *VfsCfApi::findHydrationJob(const QString &requestId) const

diff --git a/src/libsync/vfs/cfapi/vfs_cfapi.h b/src/libsync/vfs/cfapi/vfs_cfapi.h
@@ -53,7 +53,7 @@ class VfsCfApi : public Vfs
 
     bool setPinState(const QString &folderPath, PinState state) override;
     Optional<PinState> pinState(const QString &folderPath) override;
-    AvailabilityResult availability(const QString &folderPath) override;
+    AvailabilityResult availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck) override;
 
     void cancelHydration(const QString &requestId, const QString &path);
 

diff --git a/src/libsync/vfs/suffix/vfs_suffix.cpp b/src/libsync/vfs/suffix/vfs_suffix.cpp
@@ -169,8 +169,9 @@ bool VfsSuffix::setPinState(const QString &folderPath, PinState state)
     return setPinStateInDb(folderPath, state);
 }
 
-Vfs::AvailabilityResult VfsSuffix::availability(const QString &folderPath)
+Vfs::AvailabilityResult VfsSuffix::availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck)
 {
+    Q_UNUSED(recursiveCheck)
     return availabilityInDb(folderPath);
 }
 

diff --git a/src/libsync/vfs/suffix/vfs_suffix.h b/src/libsync/vfs/suffix/vfs_suffix.h
@@ -51,7 +51,7 @@ class VfsSuffix : public Vfs
     bool setPinState(const QString &folderPath, PinState state) override;
     Optional<PinState> pinState(const QString &folderPath) override
     { return pinStateInDb(folderPath); }
-    AvailabilityResult availability(const QString &folderPath) override;
+    AvailabilityResult availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck) override;
 
 public slots:
     void fileStatusChanged(const QString &, OCC::SyncFileStatus) override {}

diff --git a/src/libsync/vfs/xattr/vfs_xattr.cpp b/src/libsync/vfs/xattr/vfs_xattr.cpp
@@ -188,8 +188,9 @@ Optional<PinState> VfsXAttr::pinState(const QString &folderPath)
     return pinStateInDb(folderPath);
 }
 
-Vfs::AvailabilityResult VfsXAttr::availability(const QString &folderPath)
+Vfs::AvailabilityResult VfsXAttr::availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck)
 {
+    Q_UNUSED(recursiveCheck)
     return availabilityInDb(folderPath);
 }
 

diff --git a/src/libsync/vfs/xattr/vfs_xattr.h b/src/libsync/vfs/xattr/vfs_xattr.h
@@ -53,7 +53,7 @@ class VfsXAttr : public Vfs
 
     bool setPinState(const QString &folderPath, PinState state) override;
     Optional<PinState> pinState(const QString &folderPath) override;
-    AvailabilityResult availability(const QString &folderPath) override;
+    AvailabilityResult availability(const QString &folderPath, const AvailabilityRecursivity recursiveCheck) override;
 
 public slots:
     void fileStatusChanged(const QString &systemFileName, OCC::SyncFileStatus fileStatus) override;

diff --git a/test/testsynccfapi.cpp b/test/testsynccfapi.cpp
@@ -1032,37 +1032,37 @@ private slots:
         QVERIFY(fakeFolder.syncOnce());
 
         // root is unspecified
-        QCOMPARE(*vfs->availability("file1"), VfsItemAvailability::AllDehydrated);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::AlwaysLocal);
-        QCOMPARE(*vfs->availability("local/file1"), VfsItemAvailability::AlwaysLocal);
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::OnlineOnly);
-        QCOMPARE(*vfs->availability("online/file1"), VfsItemAvailability::OnlineOnly);
-        QCOMPARE(*vfs->availability("unspec"), VfsItemAvailability::AllDehydrated);
-        QCOMPARE(*vfs->availability("unspec/file1"), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("file1", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AlwaysLocal);
+        QCOMPARE(*vfs->availability("local/file1", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AlwaysLocal);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::OnlineOnly);
+        QCOMPARE(*vfs->availability("online/file1", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::OnlineOnly);
+        QCOMPARE(*vfs->availability("unspec", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("unspec/file1", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
 
         // Subitem pin states can ruin "pure" availabilities
         ::setPinState(fakeFolder.localPath() + "local/sub", PinState::OnlineOnly, cfapi::NoRecurse);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::AllHydrated);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllHydrated);
         ::setPinState(fakeFolder.localPath() + "online/sub", PinState::Unspecified, cfapi::NoRecurse);
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
 
         triggerDownload(fakeFolder, "unspec/file1");
         ::setPinState(fakeFolder.localPath() + "local/file2", PinState::OnlineOnly, cfapi::NoRecurse);
         ::setPinState(fakeFolder.localPath() + "online/file2", PinState::AlwaysLocal, cfapi::NoRecurse);
         QVERIFY(fakeFolder.syncOnce());
 
-        QCOMPARE(*vfs->availability("unspec"), VfsItemAvailability::AllHydrated);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::Mixed);
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::Mixed);
+        QCOMPARE(*vfs->availability("unspec", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllHydrated);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::Mixed);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::Mixed);
 
         QVERIFY(vfs->setPinState("local", PinState::AlwaysLocal));
         QVERIFY(vfs->setPinState("online", PinState::OnlineOnly));
         QVERIFY(fakeFolder.syncOnce());
 
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::OnlineOnly);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::AlwaysLocal);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::OnlineOnly);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AlwaysLocal);
 
-        auto r = vfs->availability("nonexistent");
+        auto r = vfs->availability("nonexistent", Vfs::AvailabilityRecursivity::RecursiveAvailability);
         QVERIFY(!r);
         QCOMPARE(r.error(), Vfs::AvailabilityError::NoSuchItem);
     }

diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp
@@ -1365,37 +1365,37 @@ private slots:
         QVERIFY(fakeFolder.syncOnce());
 
         // root is unspecified
-        QCOMPARE(*vfs->availability("file1" DVSUFFIX), VfsItemAvailability::AllDehydrated);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::AlwaysLocal);
-        QCOMPARE(*vfs->availability("local/file1"), VfsItemAvailability::AlwaysLocal);
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::OnlineOnly);
-        QCOMPARE(*vfs->availability("online/file1" DVSUFFIX), VfsItemAvailability::OnlineOnly);
-        QCOMPARE(*vfs->availability("unspec"), VfsItemAvailability::AllDehydrated);
-        QCOMPARE(*vfs->availability("unspec/file1" DVSUFFIX), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("file1" DVSUFFIX, Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AlwaysLocal);
+        QCOMPARE(*vfs->availability("local/file1", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AlwaysLocal);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::OnlineOnly);
+        QCOMPARE(*vfs->availability("online/file1" DVSUFFIX, Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::OnlineOnly);
+        QCOMPARE(*vfs->availability("unspec", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("unspec/file1" DVSUFFIX, Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
 
         // Subitem pin states can ruin "pure" availabilities
         setPin("local/sub", PinState::OnlineOnly);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::AllHydrated);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllHydrated);
         setPin("online/sub", PinState::Unspecified);
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::AllDehydrated);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllDehydrated);
 
         triggerDownload(fakeFolder, "unspec/file1");
         setPin("local/file2", PinState::OnlineOnly);
         setPin("online/file2" DVSUFFIX, PinState::AlwaysLocal);
         QVERIFY(fakeFolder.syncOnce());
 
-        QCOMPARE(*vfs->availability("unspec"), VfsItemAvailability::AllHydrated);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::Mixed);
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::Mixed);
+        QCOMPARE(*vfs->availability("unspec", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AllHydrated);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::Mixed);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::Mixed);
 
         QVERIFY(vfs->setPinState("local", PinState::AlwaysLocal));
         QVERIFY(vfs->setPinState("online", PinState::OnlineOnly));
         QVERIFY(fakeFolder.syncOnce());
 
-        QCOMPARE(*vfs->availability("online"), VfsItemAvailability::OnlineOnly);
-        QCOMPARE(*vfs->availability("local"), VfsItemAvailability::AlwaysLocal);
+        QCOMPARE(*vfs->availability("online", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::OnlineOnly);
+        QCOMPARE(*vfs->availability("local", Vfs::AvailabilityRecursivity::RecursiveAvailability), VfsItemAvailability::AlwaysLocal);
 
-        auto r = vfs->availability("nonexistent");
+        auto r = vfs->availability("nonexistent", Vfs::AvailabilityRecursivity::RecursiveAvailability);
         QVERIFY(!r);
         QCOMPARE(r.error(), Vfs::AvailabilityError::NoSuchItem);
     }