Skip to content

Commit c946e5c

Browse files
erikjverikjv
committed
Fix check for creation of a spaces folder in a spaces root
Fixes: #11516
1 parent 8b25947 commit c946e5c

8 files changed

+142
-71
lines changed

‎src/gui/folderman.cpp

+58-24
Original file line numberDiff line numberDiff line change
@@ -720,22 +720,41 @@ static QString canonicalPath(const QString &path)
720720
return selFile.canonicalFilePath();
721721
}
722722

723-
static QString checkPathForSyncRootMarkingRecursive(const QString &path)
724-
{
725-
auto existingTag = Utility::getDirectorySyncRootMarking(path);
726-
if (!existingTag.isEmpty()) {
727-
return FolderMan::tr("Folder '%1' is already in use by application %2!").arg(path, existingTag);
723+
static QString checkPathForSyncRootMarkingRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
724+
{
725+
std::pair<QString, QUuid> existingTags = Utility::getDirectorySyncRootMarkings(path);
726+
if (!existingTags.first.isEmpty()) {
727+
if (folderType == FolderMan::NewFolderType::SpacesFolder) {
728+
if (existingTags.first == Theme::instance()->orgDomainName()) {
729+
// Looks good, our app, let's check the account:
730+
if (existingTags.second == accountUuid) {
731+
// Nice, that's what we like, the sync root for our account in our app. No error.
732+
return {};
733+
} else {
734+
return FolderMan::tr("Folder '%1' is already in use by another account.").arg(path);
735+
}
736+
} else {
737+
// this is handled below: another application uses this as spces root folder
738+
}
739+
}
740+
741+
if (existingTags.first == Theme::instance()->orgDomainName()) {
742+
// it's our application, so must be another space root
743+
return FolderMan::tr("Folder '%1' is already in use by another account.").arg(path);
744+
}
745+
// another application uses this as spaces root folder
746+
return FolderMan::tr("Folder '%1' is already in use by application %2!").arg(path, existingTags.first);
728747
}
729748

730749
QString parent = QFileInfo(path).path();
731750
if (parent == path) { // root dir, stop recursing
732751
return {};
733752
}
734753

735-
return checkPathForSyncRootMarkingRecursive(parent);
754+
return checkPathForSyncRootMarkingRecursive(parent, folderType, accountUuid);
736755
}
737756

738-
QString FolderMan::checkPathValidityRecursive(const QString &path)
757+
QString FolderMan::checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid)
739758
{
740759
if (path.isEmpty()) {
741760
return FolderMan::tr("No valid folder selected!");
@@ -750,31 +769,45 @@ QString FolderMan::checkPathValidityRecursive(const QString &path)
750769
return pathLenghtCheck.error();
751770
}
752771

753-
const QFileInfo selFile(path);
754-
if (numberOfSyncJournals(selFile.filePath()) != 0) {
755-
return FolderMan::tr("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(selFile.filePath()));
756-
}
757-
758-
if (!selFile.exists()) {
759-
const QString parentPath = selFile.path();
772+
const QFileInfo selectedPathInfo(path);
773+
if (!selectedPathInfo.exists()) {
774+
const QString parentPath = selectedPathInfo.path();
760775
if (parentPath != path) {
761-
return checkPathValidityRecursive(parentPath);
776+
return checkPathValidityRecursive(parentPath, folderType, accountUuid);
762777
}
763778
return FolderMan::tr("The selected path does not exist!");
764779
}
765780

766-
if (!selFile.isDir()) {
781+
if (numberOfSyncJournals(selectedPathInfo.filePath()) != 0) {
782+
return FolderMan::tr("The folder %1 is used in a folder sync connection!").arg(QDir::toNativeSeparators(selectedPathInfo.filePath()));
783+
}
784+
785+
// At this point we know there is no syncdb in the parent hyrarchy, check for spaces sync root.
786+
787+
if (!selectedPathInfo.isDir()) {
767788
return FolderMan::tr("The selected path is not a folder!");
768789
}
769790

770-
if (!selFile.isWritable()) {
791+
if (!selectedPathInfo.isWritable()) {
771792
return FolderMan::tr("You have no permission to write to the selected folder!");
772793
}
773794

774-
return checkPathForSyncRootMarkingRecursive(path);
795+
return checkPathForSyncRootMarkingRecursive(path, folderType, accountUuid);
775796
}
776797

777-
QString FolderMan::checkPathValidityForNewFolder(const QString &path) const
798+
/*
799+
* OC10 folder:
800+
* - sync root not in syncdb folder
801+
* - sync root not in spaces root
802+
* with spaces:
803+
* - spaces sync root not in syncdb folder
804+
* - spaces sync root not in another spaces sync root
805+
*
806+
* - space not in syncdb folder
807+
* - space *can* be in sync root
808+
* - space not in spaces sync root of other account (check with account uuid)
809+
*/
810+
QString FolderMan::checkPathValidityForNewFolder(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const
778811
{
779812
// check if the local directory isn't used yet in another ownCloud sync
780813
const auto cs = Utility::fsCaseSensitivity();
@@ -789,20 +822,20 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path) const
789822
}
790823
if (FileSystem::isChildPathOf(folderDir, userDir)) {
791824
return tr("The local folder %1 already contains a folder used in a folder sync connection. "
792-
"Please pick another one!")
825+
"Please pick another local folder!")
793826
.arg(QDir::toNativeSeparators(path));
794827
}
795828

796829
if (FileSystem::isChildPathOf(userDir, folderDir)) {
797830
return tr("The local folder %1 is already contained in a folder used in a folder sync connection. "
798-
"Please pick another one!")
831+
"Please pick another local folder!")
799832
.arg(QDir::toNativeSeparators(path));
800833
}
801834
}
802835

803-
const auto result = checkPathValidityRecursive(path);
836+
const auto result = checkPathValidityRecursive(path, folderType, accountUuid);
804837
if (!result.isEmpty()) {
805-
return tr("%1 Please pick another one!").arg(result);
838+
return tr("%1 Please pick another local folder!").arg(result);
806839
}
807840
return {};
808841
}
@@ -825,8 +858,9 @@ QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const Q
825858
{
826859
QString folder = normalisedPath;
827860
for (int attempt = 2; attempt <= 100; ++attempt) {
861+
// HvR: is this correct?
828862
if (!QFileInfo::exists(folder)
829-
&& FolderMan::instance()->checkPathValidityForNewFolder(folder).isEmpty()) {
863+
&& FolderMan::instance()->checkPathValidityForNewFolder(folder, FolderMan::NewFolderType::SpacesSyncRootOrOC10SyncRoot, {}).isEmpty()) {
830864
return canonicalPath(folder);
831865
}
832866
folder = normalisedPath + QStringLiteral(" (%1)").arg(attempt);

‎src/gui/folderman.h

+11-2
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,16 @@ class FolderMan : public QObject
8686
static QString suggestSyncFolder(const QUrl &server, const QString &displayName);
8787
[[nodiscard]] static bool prepareFolder(const QString &folder);
8888

89-
static QString checkPathValidityRecursive(const QString &path);
89+
/**
90+
* For a new folder, the type guides what kind of checks are done to ensure the new folder is not embedded in an existing one.
91+
* Or in case of a space folder, that if the new folder is in a Space sync root, it is the sync root of the same account.
92+
*/
93+
enum class NewFolderType {
94+
SpacesSyncRootOrOC10SyncRoot,
95+
SpacesFolder,
96+
};
97+
98+
static QString checkPathValidityRecursive(const QString &path, FolderMan::NewFolderType folderType, const QUuid &accountUuid);
9099

91100
static std::unique_ptr<FolderMan> createInstance();
92101
~FolderMan() override;
@@ -177,7 +186,7 @@ class FolderMan : public QObject
177186
*
178187
* @returns an empty string if it is allowed, or an error if it is not allowed
179188
*/
180-
QString checkPathValidityForNewFolder(const QString &path) const;
189+
QString checkPathValidityForNewFolder(const QString &path, NewFolderType folderType, const QUuid &accountUuid) const;
181190

182191
/**
183192
* Attempts to find a non-existing, acceptable path for creating a new sync folder.

‎src/gui/folderwizard/folderwizardlocalpath.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ QString FolderWizardLocalPath::localPath() const
6363

6464
bool FolderWizardLocalPath::isComplete() const
6565
{
66-
QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(localPath());
66+
// HvR: is this correct?
67+
auto folderType = folderWizardPrivate()->accountState()->supportsSpaces() ? FolderMan::NewFolderType::SpacesFolder
68+
: FolderMan::NewFolderType::SpacesSyncRootOrOC10SyncRoot;
69+
auto accountUuid = folderWizardPrivate()->accountState()->account()->uuid();
70+
QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(localPath(), folderType, accountUuid);
6771

6872
bool isOk = errorStr.isEmpty();
6973
QStringList warnStrings;

‎src/gui/guiutility.cpp

+31-9
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@ const QString dirTag()
3939
{
4040
return QStringLiteral("com.owncloud.spaces.app");
4141
}
42+
43+
const QString uuidTag()
44+
{
45+
return QStringLiteral("com.owncloud.spaces.account-uuid");
46+
}
4247
} // anonymous namespace
4348

4449
using namespace OCC;
@@ -104,25 +109,39 @@ bool Utility::internetConnectionIsMetered()
104109
return false;
105110
}
106111

107-
void Utility::markDirectoryAsSyncRoot(const QString &path)
112+
void Utility::markDirectoryAsSyncRoot(const QString &path, const QUuid &accountUuid)
108113
{
109-
Q_ASSERT(getDirectorySyncRootMarking(path).isEmpty());
114+
Q_ASSERT(getDirectorySyncRootMarkings(path).first.isEmpty());
115+
Q_ASSERT(getDirectorySyncRootMarkings(path).second.isNull());
110116

111-
auto result = FileSystem::Tags::set(path, dirTag(), Theme::instance()->orgDomainName().toUtf8());
112-
if (!result) {
113-
qCWarning(lcGuiUtility) << QStringLiteral("Failed to set tag on '%1': %2").arg(path, result.error())
117+
auto result1 = FileSystem::Tags::set(path, dirTag(), Theme::instance()->orgDomainName().toUtf8());
118+
if (!result1) {
119+
qCWarning(lcGuiUtility) << QStringLiteral("Failed to set tag on '%1': %2").arg(path, result1.error())
114120
#ifdef Q_OS_WIN
115121
<< QStringLiteral("(filesystem %1)").arg(FileSystem::fileSystemForPath(path))
116122
#endif // Q_OS_WIN
117123
;
124+
return;
125+
}
126+
127+
auto result2 = FileSystem::Tags::set(path, uuidTag(), accountUuid.toString().toUtf8());
128+
if (!result2) {
129+
qCWarning(lcGuiUtility) << QStringLiteral("Failed to set tag on '%1': %2").arg(path, result2.error())
130+
#ifdef Q_OS_WIN
131+
<< QStringLiteral("(filesystem %1)").arg(FileSystem::fileSystemForPath(path))
132+
#endif // Q_OS_WIN
133+
;
134+
return;
118135
}
119136
}
120137

121-
QString Utility::getDirectorySyncRootMarking(const QString &path)
138+
std::pair<QString, QUuid> Utility::getDirectorySyncRootMarkings(const QString &path)
122139
{
123-
auto existingValue = FileSystem::Tags::get(path, dirTag());
124-
if (existingValue.has_value()) {
125-
return QString::fromUtf8(existingValue.value());
140+
auto existingDirTag = FileSystem::Tags::get(path, dirTag());
141+
auto existingUuidTag = FileSystem::Tags::get(path, uuidTag());
142+
143+
if (existingDirTag.has_value() && existingUuidTag.has_value()) {
144+
return {QString::fromUtf8(existingDirTag.value()), QUuid::fromString(QString::fromUtf8(existingUuidTag.value()))};
126145
}
127146

128147
return {};
@@ -133,4 +152,7 @@ void Utility::unmarkDirectoryAsSyncRoot(const QString &path)
133152
if (!FileSystem::Tags::remove(path, dirTag())) {
134153
qCWarning(lcGuiUtility) << "Failed to remove tag on" << path;
135154
}
155+
if (!FileSystem::Tags::remove(path, uuidTag())) {
156+
qCWarning(lcGuiUtility) << "Failed to remove uuid tag on" << path;
157+
}
136158
}

‎src/gui/guiutility.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ namespace Utility {
5353

5454
bool internetConnectionIsMetered();
5555

56-
void markDirectoryAsSyncRoot(const QString &path);
57-
QString getDirectorySyncRootMarking(const QString &path);
56+
void markDirectoryAsSyncRoot(const QString &path, const QUuid &accountUuid);
57+
std::pair<QString, QUuid> getDirectorySyncRootMarkings(const QString &path);
5858
void unmarkDirectoryAsSyncRoot(const QString &path);
5959
} // namespace Utility
6060
} // namespace OCC

‎src/gui/newwizard/setupwizardaccountbuilder.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ AccountPtr SetupWizardAccountBuilder::build()
141141
if (!QFileInfo::exists(_defaultSyncTargetDir)) {
142142
OC_ASSERT(QDir().mkpath(_defaultSyncTargetDir));
143143
}
144-
Utility::markDirectoryAsSyncRoot(_defaultSyncTargetDir);
144+
Utility::markDirectoryAsSyncRoot(_defaultSyncTargetDir, newAccountPtr->uuid());
145145
}
146146

147147
return newAccountPtr;

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ void AccountConfiguredSetupWizardState::evaluatePage()
8686
return;
8787
}
8888

89-
QString invalidPathErrorMessage = FolderMan::checkPathValidityRecursive(syncTargetDir);
89+
QString invalidPathErrorMessage = FolderMan::checkPathValidityRecursive(syncTargetDir, FolderMan::NewFolderType::SpacesSyncRootOrOC10SyncRoot, {});
9090
if (!invalidPathErrorMessage.isEmpty()) {
9191
Q_EMIT evaluationFailed(errorMessageTemplate.arg(invalidPathErrorMessage));
9292
return;

0 commit comments

Comments
 (0)