Skip to content

Commit

Permalink
context menu: do not recursively check pin and availability states
Browse files Browse the repository at this point in the history
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
  • Loading branch information
mgallien committed Mar 22, 2024
1 parent 0312a1f commit 8f3633c
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 55 deletions.
11 changes: 9 additions & 2 deletions src/common/vfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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, AvailabilityRecursivity recursiveCheck) = 0;

public slots:
/** Update in-sync state based on SyncFileStatusTracker signal.
Expand Down Expand Up @@ -328,7 +335,7 @@ class OCSYNC_EXPORT VfsOff : public Vfs

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 &, AvailabilityRecursivity) override { return VfsItemAvailability::AlwaysLocal; }

public slots:
void fileStatusChanged(const QString &, OCC::SyncFileStatus) override {}
Expand Down
2 changes: 1 addition & 1 deletion src/gui/socketapi/socketapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
55 changes: 37 additions & 18 deletions src/libsync/vfs/cfapi/vfs_cfapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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
if (basePinState && recursiveCheck == Vfs::AvailabilityRecursivity::NotRecursiveAvailability) {
switch (*basePinState)
{
case OCC::PinState::AlwaysLocal:
return VfsItemAvailability::AllDehydrated;
} else if (hydrationStatus.hasHydrated) {
if (pin && *pin == 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
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/cfapi/vfs_cfapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -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, AvailabilityRecursivity recursiveCheck) override;

void cancelHydration(const QString &requestId, const QString &path);

Expand Down
3 changes: 2 additions & 1 deletion src/libsync/vfs/suffix/vfs_suffix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, AvailabilityRecursivity recursiveCheck)
{
Q_UNUSED(recursiveCheck)
return availabilityInDb(folderPath);
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/suffix/vfs_suffix.h
Original file line number Diff line number Diff line change
Expand Up @@ -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, AvailabilityRecursivity recursiveCheck) override;

public slots:
void fileStatusChanged(const QString &, OCC::SyncFileStatus) override {}
Expand Down
30 changes: 15 additions & 15 deletions test/testsynccfapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
30 changes: 15 additions & 15 deletions test/testsyncvirtualfiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 8f3633c

Please sign in to comment.