Skip to content

Commit 9750a0b

Browse files
erikjverikjv
authored
fix: findGoodPathForNewSyncFolder (#11970)
The account UUID was not passed in, which resulted in a failed check when adding a space folder to a spaces root.
1 parent 1b05548 commit 9750a0b

File tree

7 files changed

+37
-26
lines changed

7 files changed

+37
-26
lines changed

‎src/gui/accountsettings.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -555,8 +555,8 @@ void AccountSettings::slotSpacesUpdated()
555555
qCInfo(lcAccountSettings) << "Adding sync connection for newly discovered space" << newSpace->displayName();
556556

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

561561
FolderMan::SyncConnectionDescription fwr;
562562
fwr.davUrl = QUrl(newSpace->drive().getRoot().getWebDavUrl());

‎src/gui/folderman.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -836,8 +836,11 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path, NewFolderT
836836
return {};
837837
}
838838

839-
QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType)
839+
QString FolderMan::findGoodPathForNewSyncFolder(
840+
const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
840841
{
842+
OC_ASSERT(!accountUuid.isNull() || folderType == FolderMan::NewFolderType::SpacesSyncRoot);
843+
841844
// reserve extra characters to allow appending of a number
842845
const QString normalisedPath = FileSystem::createPortableFileName(basePath, FileSystem::pathEscape(newFolder), std::string_view(" (100)").size());
843846

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

970-
QString FolderMan::suggestSyncFolder(NewFolderType folderType)
973+
QString FolderMan::suggestSyncFolder(NewFolderType folderType, const QUuid &accountUuid)
971974
{
972-
return FolderMan::instance()->findGoodPathForNewSyncFolder(QDir::homePath(), Theme::instance()->appName(), folderType);
975+
return FolderMan::instance()->findGoodPathForNewSyncFolder(QDir::homePath(), Theme::instance()->appName(), folderType, accountUuid);
973976
}
974977

975978
bool FolderMan::prepareFolder(const QString &folder)

‎src/gui/folderman.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject
130130
QSet<QString> selectiveSyncBlackList;
131131
};
132132

133-
static QString suggestSyncFolder(NewFolderType folderType);
133+
static QString suggestSyncFolder(NewFolderType folderType, const QUuid &accountUuid);
134134
[[nodiscard]] static bool prepareFolder(const QString &folder);
135135

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

240240
/**
241241
* While ignoring hidden files can theoretically be switched per folder,

‎src/gui/folderwizard/folderwizard.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ QString FolderWizardPrivate::defaultSyncRoot() const
6464
{
6565
if (!_account->account()->hasDefaultSyncRoot()) {
6666
const auto folderType = _account->supportsSpaces() ? FolderMan::NewFolderType::SpacesSyncRoot : FolderMan::NewFolderType::OC10SyncRoot;
67-
return FolderMan::suggestSyncFolder(folderType);
67+
return FolderMan::suggestSyncFolder(folderType, _account->account()->uuid());
6868
} else {
6969
return _account->account()->defaultSyncRoot();
7070
}
@@ -94,12 +94,13 @@ FolderWizardPrivate::FolderWizardPrivate(FolderWizard *q, const AccountStatePtr
9494
QString FolderWizardPrivate::initialLocalPath() const
9595
{
9696
if (_account->supportsSpaces()) {
97-
return FolderMan::findGoodPathForNewSyncFolder(defaultSyncRoot(), _spacesPage->currentSpace()->displayName(), FolderMan::NewFolderType::SpacesSyncRoot);
97+
return FolderMan::findGoodPathForNewSyncFolder(
98+
defaultSyncRoot(), _spacesPage->currentSpace()->displayName(), FolderMan::NewFolderType::SpacesSyncRoot, _account->account()->uuid());
9899
}
99100

100101
// Split default sync root:
101102
const QFileInfo path(defaultSyncRoot());
102-
return FolderMan::findGoodPathForNewSyncFolder(path.path(), path.fileName(), FolderMan::NewFolderType::OC10SyncRoot);
103+
return FolderMan::findGoodPathForNewSyncFolder(path.path(), path.fileName(), FolderMan::NewFolderType::OC10SyncRoot, _account->account()->uuid());
103104
}
104105

105106
QString FolderWizardPrivate::remotePath() const

‎src/gui/newwizard/states/accountconfiguredsetupwizardstate.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ AccountConfiguredSetupWizardState::AccountConfiguredSetupWizardState(SetupWizard
4444
}
4545

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

5051
if (syncTargetDir.isEmpty()) {

‎src/gui/owncloudgui.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ void setUpInitialSyncFolder(AccountStatePtr accountStatePtr, bool useVfs)
9494
Utility::setupFavLink(localDir);
9595
for (const auto *space : spaces) {
9696
const QString name = space->displayName();
97-
const QString folderName = FolderMan::instance()->findGoodPathForNewSyncFolder(localDir, name, FolderMan::NewFolderType::SpacesFolder);
97+
const QString folderName = FolderMan::instance()->findGoodPathForNewSyncFolder(
98+
localDir, name, FolderMan::NewFolderType::SpacesFolder, accountStatePtr->account()->uuid());
9899
auto folder = addFolder(folderName, {}, QUrl(space->drive().getRoot().getWebDavUrl()), space->drive().getRoot().getId(), name);
99100
folder->setPriority(space->priority());
100101
// save the new priority

‎test/testfolderman.cpp‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -186,29 +186,34 @@ private Q_SLOTS:
186186

187187
// TEST
188188
const auto folderType = FolderMan::NewFolderType::OC10SyncRoot;
189+
const auto uuid = QUuid::createUuid();
189190

190-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("oc"), folderType), dirPath + QStringLiteral("/oc"));
191-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType), dirPath + QStringLiteral("/ownCloud (3)"));
192-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType), dirPath + QStringLiteral("/ownCloud2 (2)"));
193-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType), dirPath + QStringLiteral("/ownCloud (2) (2)"));
194-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/foo"), folderType), dirPath + QStringLiteral("/ownCloud2_foo"));
195-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/bar"), folderType), dirPath + QStringLiteral("/ownCloud2_bar"));
196-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("sub"), folderType), dirPath + QStringLiteral("/sub (2)"));
191+
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("oc"), folderType, uuid), dirPath + QStringLiteral("/oc"));
192+
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType, uuid), dirPath + QStringLiteral("/ownCloud (3)"));
193+
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType, uuid), dirPath + QStringLiteral("/ownCloud2 (2)"));
194+
QCOMPARE(
195+
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType, uuid), dirPath + QStringLiteral("/ownCloud (2) (2)"));
196+
QCOMPARE(
197+
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/foo"), folderType, uuid), dirPath + QStringLiteral("/ownCloud2_foo"));
198+
QCOMPARE(
199+
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2/bar"), folderType, uuid), dirPath + QStringLiteral("/ownCloud2_bar"));
200+
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("sub"), folderType, uuid), dirPath + QStringLiteral("/sub (2)"));
197201

198202
// REMOVE ownCloud2 from the filesystem, but keep a folder sync'ed to it.
199203
// We should still not suggest this folder as a new folder.
200204
QDir(dirPath + QStringLiteral("/ownCloud (2)/")).removeRecursively();
201-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType), dirPath + QStringLiteral("/ownCloud (3)"));
202-
QCOMPARE(
203-
folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType), QString(dirPath + QStringLiteral("/ownCloud2 (2)")));
204-
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType),
205+
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud"), folderType, uuid), dirPath + QStringLiteral("/ownCloud (3)"));
206+
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud2"), folderType, uuid),
207+
QString(dirPath + QStringLiteral("/ownCloud2 (2)")));
208+
QCOMPARE(folderman->findGoodPathForNewSyncFolder(dirPath, QStringLiteral("ownCloud (2)"), folderType, uuid),
205209
QString(dirPath + QStringLiteral("/ownCloud (2) (2)")));
206210

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

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

0 commit comments

Comments
 (0)