Skip to content

Commit

Permalink
fix: findGoodPathForNewSyncFolder (#11970)
Browse files Browse the repository at this point in the history 
The account UUID was not passed in, which resulted in a failed check
when adding a space folder to a spaces root.
  • Loading branch information
@erikjv
erikjv authored Nov 11, 2024
1 parent 1b05548 commit 9750a0b
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 26 deletions.
4 changes: 2 additions & 2 deletions src/gui/accountsettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,8 @@ void AccountSettings::slotSpacesUpdated()
qCInfo(lcAccountSettings) << "Adding sync connection for newly discovered space" << newSpace->displayName();

const QString localDir(accountsState()->account()->defaultSyncRoot());
const QString folderName =
FolderMan::instance()->findGoodPathForNewSyncFolder(localDir, newSpace->displayName(), FolderMan::NewFolderType::SpacesFolder);
const QString folderName = FolderMan::instance()->findGoodPathForNewSyncFolder(
localDir, newSpace->displayName(), FolderMan::NewFolderType::SpacesFolder, accountStatePtr->account()->uuid());

FolderMan::SyncConnectionDescription fwr;
fwr.davUrl = QUrl(newSpace->drive().getRoot().getWebDavUrl());
Expand Down
11 changes: 7 additions & 4 deletions src/gui/folderman.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -836,8 +836,11 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path, NewFolderT
return {};
}

QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType)
QString FolderMan::findGoodPathForNewSyncFolder(
const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
{
OC_ASSERT(!accountUuid.isNull() || folderType == FolderMan::NewFolderType::SpacesSyncRoot);

// reserve extra characters to allow appending of a number
const QString normalisedPath = FileSystem::createPortableFileName(basePath, FileSystem::pathEscape(newFolder), std::string_view(" (100)").size());

Expand All @@ -854,7 +857,7 @@ QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const Q
{
QString folder = normalisedPath;
for (int attempt = 2; attempt <= 100; ++attempt) {
if (!QFileInfo::exists(folder) && FolderMan::instance()->checkPathValidityForNewFolder(folder, folderType, {}).isEmpty()) {
if (!QFileInfo::exists(folder) && FolderMan::instance()->checkPathValidityForNewFolder(folder, folderType, accountUuid).isEmpty()) {
return canonicalPath(folder);
}
folder = normalisedPath + QStringLiteral(" (%1)").arg(attempt);
Expand Down Expand Up @@ -967,9 +970,9 @@ Folder *FolderMan::addFolderFromFolderWizardResult(const AccountStatePtr &accoun
return f;
}

QString FolderMan::suggestSyncFolder(NewFolderType folderType)
QString FolderMan::suggestSyncFolder(NewFolderType folderType, const QUuid &accountUuid)
{
return FolderMan::instance()->findGoodPathForNewSyncFolder(QDir::homePath(), Theme::instance()->appName(), folderType);
return FolderMan::instance()->findGoodPathForNewSyncFolder(QDir::homePath(), Theme::instance()->appName(), folderType, accountUuid);
}

bool FolderMan::prepareFolder(const QString &folder)
Expand Down
4 changes: 2 additions & 2 deletions src/gui/folderman.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
QSet<QString> selectiveSyncBlackList;
};

static QString suggestSyncFolder(NewFolderType folderType);
static QString suggestSyncFolder(NewFolderType folderType, const QUuid &accountUuid);
[[nodiscard]] static bool prepareFolder(const QString &folder);

static QString checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid);
Expand Down Expand Up @@ -235,7 +235,7 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
* subfolder of ~ would be a good candidate. When that happens \a basePath
* is returned.
*/
static QString findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, NewFolderType folderType);
static QString findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, NewFolderType folderType, const QUuid &accountUuid);

/**
* While ignoring hidden files can theoretically be switched per folder,
Expand Down
7 changes: 4 additions & 3 deletions src/gui/folderwizard/folderwizard.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ QString FolderWizardPrivate::defaultSyncRoot() const
{
if (!_account->account()->hasDefaultSyncRoot()) {
const auto folderType = _account->supportsSpaces() ? FolderMan::NewFolderType::SpacesSyncRoot : FolderMan::NewFolderType::OC10SyncRoot;
return FolderMan::suggestSyncFolder(folderType);
return FolderMan::suggestSyncFolder(folderType, _account->account()->uuid());
} else {
return _account->account()->defaultSyncRoot();
}
Expand Down Expand Up @@ -94,12 +94,13 @@ FolderWizardPrivate::FolderWizardPrivate(FolderWizard *q, const AccountStatePtr
QString FolderWizardPrivate::initialLocalPath() const
{
if (_account->supportsSpaces()) {
return FolderMan::findGoodPathForNewSyncFolder(defaultSyncRoot(), _spacesPage->currentSpace()->displayName(), FolderMan::NewFolderType::SpacesSyncRoot);
return FolderMan::findGoodPathForNewSyncFolder(
defaultSyncRoot(), _spacesPage->currentSpace()->displayName(), FolderMan::NewFolderType::SpacesSyncRoot, _account->account()->uuid());
}

// Split default sync root:
const QFileInfo path(defaultSyncRoot());
return FolderMan::findGoodPathForNewSyncFolder(path.path(), path.fileName(), FolderMan::NewFolderType::OC10SyncRoot);
return FolderMan::findGoodPathForNewSyncFolder(path.path(), path.fileName(), FolderMan::NewFolderType::OC10SyncRoot, _account->account()->uuid());
}

QString FolderWizardPrivate::remotePath() const
Expand Down
3 changes: 2 additions & 1 deletion src/gui/newwizard/states/accountconfiguredsetupwizardstate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ AccountConfiguredSetupWizardState::AccountConfiguredSetupWizardState(SetupWizard
}

// We need some sync root, either for spaces, or for OC10. It's never a Space folder.
const QString defaultSyncTargetDir = FolderMan::suggestSyncFolder(FolderMan::NewFolderType::SpacesSyncRoot);
// We pass an invalid UUID, because we don't "own" a syncroot yet, and all checks against UUIDs should fail.
const QString defaultSyncTargetDir = FolderMan::suggestSyncFolder(FolderMan::NewFolderType::SpacesSyncRoot, {});
QString syncTargetDir = _context->accountBuilder().syncTargetDir();

if (syncTargetDir.isEmpty()) {
Expand Down
3 changes: 2 additions & 1 deletion src/gui/owncloudgui.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ void setUpInitialSyncFolder(AccountStatePtr accountStatePtr, bool useVfs)
Utility::setupFavLink(localDir);
for (const auto *space : spaces) {
const QString name = space->displayName();
const QString folderName = FolderMan::instance()->findGoodPathForNewSyncFolder(localDir, name, FolderMan::NewFolderType::SpacesFolder);
const QString folderName = FolderMan::instance()->findGoodPathForNewSyncFolder(
localDir, name, FolderMan::NewFolderType::SpacesFolder, accountStatePtr->account()->uuid());
auto folder = addFolder(folderName, {}, QUrl(space->drive().getRoot().getWebDavUrl()), space->drive().getRoot().getId(), name);
folder->setPriority(space->priority());
// save the new priority
Expand Down
31 changes: 18 additions & 13 deletions test/testfolderman.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,29 +186,34 @@ private Q_SLOTS:

// TEST
const auto folderType = FolderMan::NewFolderType::OC10SyncRoot;
const auto uuid = QUuid::createUuid();

QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("oc"), folderType), dirPath + QStringLiteral("/oc"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType), dirPath + QStringLiteral("/ownCloud (3)"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType), dirPath + QStringLiteral("/ownCloud2 (2)"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType), dirPath + QStringLiteral("/ownCloud (2) (2)"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/foo"), folderType), dirPath + QStringLiteral("/ownCloud2_foo"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/bar"), folderType), dirPath + QStringLiteral("/ownCloud2_bar"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("sub"), folderType), dirPath + QStringLiteral("/sub (2)"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("oc"), folderType, uuid), dirPath + QStringLiteral("/oc"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType, uuid), dirPath + QStringLiteral("/ownCloud (3)"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType, uuid), dirPath + QStringLiteral("/ownCloud2 (2)"));
QCOMPARE(
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType, uuid), dirPath + QStringLiteral("/ownCloud (2) (2)"));
QCOMPARE(
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/foo"), folderType, uuid), dirPath + QStringLiteral("/ownCloud2_foo"));
QCOMPARE(
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/bar"), folderType, uuid), dirPath + QStringLiteral("/ownCloud2_bar"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("sub"), folderType, uuid), dirPath + QStringLiteral("/sub (2)"));

// REMOVE ownCloud2 from the filesystem, but keep a folder sync'ed to it.
// We should still not suggest this folder as a new folder.
QDir(dirPath + QStringLiteral("/ownCloud (2)/")).removeRecursively();
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType), dirPath + QStringLiteral("/ownCloud (3)"));
QCOMPARE(
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType), QString(dirPath + QStringLiteral("/ownCloud2 (2)")));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType),
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType, uuid), dirPath + QStringLiteral("/ownCloud (3)"));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType, uuid),
QString(dirPath + QStringLiteral("/ownCloud2 (2)")));
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType, uuid),
QString(dirPath + QStringLiteral("/ownCloud (2) (2)")));

// make sure people can't do evil stuff
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("../../../Bo/b"), folderType), QString(dirPath + QStringLiteral("/___Bo_b")));
QCOMPARE(
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("../../../Bo/b"), folderType, uuid), QString(dirPath + QStringLiteral("/___Bo_b")));

// normalise the name
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral(" Bo:*<>!b "), folderType),
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral(" Bo:*<>!b "), folderType, uuid),
QString(dirPath + QStringLiteral("/Bo____!b")));
}

Expand Down

0 comments on commit 9750a0b

Please sign in to comment.