Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

context menu: do not recursively check pin and availability states #6573

Merged
merged 1 commit into from
Mar 27, 2024
Merged

context menu: do not recursively check pin and availability states #6573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/common/vfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/common/vfs.h

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/common/vfs.h 

File src/common/vfs.h does not conform to Custom style guidelines. (lines 330, 338)
* Copyright (C) by Christian Kamm <mail@ckamm.de>
*
* This program is free software; you can redistribute it and/or modify
Expand All @@ -13,7 +13,7 @@
*/
#pragma once

#include "ocsynclib.h"

Check failure on line 16 in src/common/vfs.h

View workflow job for this annotation

GitHub Actions / build

src/common/vfs.h:16:10 [clang-diagnostic-error] 

'ocsynclib.h' file not found
#include "result.h"
#include "syncfilestatus.h"
#include "pinstate.h"
Expand Down Expand Up @@ -134,8 +134,15 @@
// 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 @@
*
* 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.
Expand Down Expand Up @@ -320,15 +327,15 @@
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 {}
Expand Down
2 changes: 1 addition & 1 deletion src/gui/socketapi/socketapi.cpp
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/discovery.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/discovery.cpp 

File src/libsync/discovery.cpp does not conform to Custom style guidelines. (lines 1293)
* Copyright (C) by Olivier Goffart <ogoffart@woboq.com>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -1290,7 +1290,7 @@
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/vfs/cfapi/vfs_cfapi.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/vfs/cfapi/vfs_cfapi.cpp 

File src/libsync/vfs/cfapi/vfs_cfapi.cpp does not conform to Custom style guidelines. (lines 341)
* Copyright (C) by Kevin Ottens <kevin.ottens@nextcloud.com>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -334,28 +334,47 @@
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
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/cfapi/vfs_cfapi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
#pragma once

#include <QObject>

Check failure on line 16 in src/libsync/vfs/cfapi/vfs_cfapi.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/vfs/cfapi/vfs_cfapi.h:16:10 [clang-diagnostic-error] 

'QObject' file not found
#include <QScopedPointer>

#include "common/vfs.h"
Expand Down Expand Up @@ -53,7 +53,7 @@

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);

Expand Down
3 changes: 2 additions & 1 deletion src/libsync/vfs/suffix/vfs_suffix.cpp
View file
Open in desktop
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, const AvailabilityRecursivity recursiveCheck)
{
Q_UNUSED(recursiveCheck)
return availabilityInDb(folderPath);
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/suffix/vfs_suffix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
#pragma once

#include <QObject>

Check failure on line 16 in src/libsync/vfs/suffix/vfs_suffix.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/vfs/suffix/vfs_suffix.h:16:10 [clang-diagnostic-error] 

'QObject' file not found
#include <QScopedPointer>

#include "common/vfs.h"
Expand Down Expand Up @@ -51,7 +51,7 @@
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 {}
Expand Down
3 changes: 2 additions & 1 deletion src/libsync/vfs/xattr/vfs_xattr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsync/vfs/xattr/vfs_xattr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
#pragma once

#include <QObject>

Check failure on line 16 in src/libsync/vfs/xattr/vfs_xattr.h

View workflow job for this annotation

GitHub Actions / build

src/libsync/vfs/xattr/vfs_xattr.h:16:10 [clang-diagnostic-error] 

'QObject' file not found
#include <QScopedPointer>

#include "common/vfs.h"
Expand Down Expand Up @@ -53,7 +53,7 @@

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;
Expand Down
30 changes: 15 additions & 15 deletions test/testsynccfapi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

#include <QtTest>

Check failure on line 8 in test/testsynccfapi.cpp

View workflow job for this annotation

GitHub Actions / build

test/testsynccfapi.cpp:8:10 [clang-diagnostic-error] 

'QtTest' file not found
#include "syncenginetestutils.h"
#include "common/vfs.h"
#include "config.h"
Expand Down Expand Up @@ -1032,37 +1032,37 @@
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
*/

#include <QtTest>

Check failure on line 8 in test/testsyncvirtualfiles.cpp

View workflow job for this annotation

GitHub Actions / build

test/testsyncvirtualfiles.cpp:8:10 [clang-diagnostic-error] 

'QtTest' file not found
#include "syncenginetestutils.h"
#include "common/vfs.h"
#include "config.h"
Expand Down Expand Up @@ -1365,37 +1365,37 @@
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
Loading
Loading