mirrors/nextcloud-desktop
1
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2024-11-25 06:25:56 +03:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #4914 from nextcloud/bugfix/conflict-resolution-when-selecting-folder

Browse source 
Bugfix/conflict resolution when selecting folder
This commit is contained in:
Matthieu Gallien 2022-09-10 11:11:50 +02:00 committed by GitHub
commit 9803607916
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 81 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/common/syncjournaldb.cpp
View file

@ -2425,7 +2425,9 @@ void SyncJournalDb::commitInternal(const QString &context, bool startTrans)
SyncJournalDb::~SyncJournalDb() SyncJournalDb::~SyncJournalDb()
{ {
close(); if (isOpen()) {
close();
}
} }

8
src/gui/folder.cpp
View file

@ -302,14 +302,6 @@ void Folder::setSyncPaused(bool paused)
emit canSyncChanged(); emit canSyncChanged();
} }
void Folder::onAssociatedAccountRemoved()
{
if (_vfs) {
_vfs->stop();
_vfs->unregisterFolder();
}
}
void Folder::setSyncState(SyncResult::Status state) void Folder::setSyncState(SyncResult::Status state)
{ {
_syncResult.setStatus(state); _syncResult.setStatus(state);

2
src/gui/folder.h
View file

@ -206,8 +206,6 @@ public:
*/ */
virtual void wipeForRemoval(); virtual void wipeForRemoval();
void onAssociatedAccountRemoved();
void setSyncState(SyncResult::Status state); void setSyncState(SyncResult::Status state);
void setDirtyNetworkLimits(); void setDirtyNetworkLimits();

36
src/gui/folderman.cpp
View file

@ -945,11 +945,15 @@ void FolderMan::runEtagJobIfPossible(Folder *folder)
void FolderMan::slotAccountRemoved(AccountState *accountState) void FolderMan::slotAccountRemoved(AccountState *accountState)
{ {
QVector<Folder *> foldersToRemove;
for (const auto &folder : qAsConst(_folderMap)) { for (const auto &folder : qAsConst(_folderMap)) {
if (folder->accountState() == accountState) { if (folder->accountState() == accountState) {
folder->onAssociatedAccountRemoved(); foldersToRemove.push_back(folder);
} }
} }
for (const auto &folder : qAsConst(foldersToRemove)) {
removeFolder(folder);
}
} }
void FolderMan::slotRemoveFoldersForAccount(AccountState *accountState) void FolderMan::slotRemoveFoldersForAccount(AccountState *accountState)
@ -1659,12 +1663,16 @@ static QString canonicalPath(const QString &path)
return selFile.canonicalFilePath(); return selFile.canonicalFilePath();
} }
QString FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl) const QPair<FolderMan::PathValidityResult, QString> FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl) const
{ {
QString recursiveValidity = checkPathValidityRecursive(path); QPair<FolderMan::PathValidityResult, QString> result;
const auto recursiveValidity = checkPathValidityRecursive(path);
if (!recursiveValidity.isEmpty()) { if (!recursiveValidity.isEmpty()) {
qCDebug(lcFolderMan) << path << recursiveValidity; qCDebug(lcFolderMan) << path << recursiveValidity;
return recursiveValidity; result.first = FolderMan::PathValidityResult::ErrorRecursiveValidity;
result.second = recursiveValidity;
return result;
} }
// check if the local directory isn't used yet in another ownCloud sync // check if the local directory isn't used yet in another ownCloud sync
@ -1680,15 +1688,19 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl
bool differentPaths = QString::compare(folderDir, userDir, cs) != 0; bool differentPaths = QString::compare(folderDir, userDir, cs) != 0;
if (differentPaths && folderDir.startsWith(userDir, cs)) { if (differentPaths && folderDir.startsWith(userDir, cs)) {
return tr("The local folder %1 already contains a folder used in a folder sync connection. " result.first = FolderMan::PathValidityResult::ErrorContainsFolder;
"Please pick another one!") result.second = tr("The local folder %1 already contains a folder used in a folder sync connection. "
.arg(QDir::toNativeSeparators(path)); "Please pick another one!")
.arg(QDir::toNativeSeparators(path));
return result;
} }
if (differentPaths && userDir.startsWith(folderDir, cs)) { if (differentPaths && userDir.startsWith(folderDir, cs)) {
return tr("The local folder %1 is already contained in a folder used in a folder sync connection. " result.first = FolderMan::PathValidityResult::ErrorContainedInFolder;
result.second = tr("The local folder %1 is already contained in a folder used in a folder sync connection. "
"Please pick another one!") "Please pick another one!")
.arg(QDir::toNativeSeparators(path)); .arg(QDir::toNativeSeparators(path));
return result;
} }
// if both pathes are equal, the server url needs to be different // if both pathes are equal, the server url needs to be different
@ -1700,13 +1712,15 @@ QString FolderMan::checkPathValidityForNewFolder(const QString &path, const QUrl
folderUrl.setUserName(user); folderUrl.setUserName(user);
if (serverUrl == folderUrl) { if (serverUrl == folderUrl) {
return tr("There is already a sync from the server to this local folder. " result.first = FolderMan::PathValidityResult::ErrorNonEmptyFolder;
result.second = tr("There is already a sync from the server to this local folder. "
"Please pick another local folder!"); "Please pick another local folder!");
return result;
} }
} }
} }
return QString(); return result;
} }
QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QUrl &serverUrl) const QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const QUrl &serverUrl) const
@ -1728,7 +1742,7 @@ QString FolderMan::findGoodPathForNewSyncFolder(const QString &basePath, const Q
forever { forever {
const bool isGood = const bool isGood =
!QFileInfo(folder).exists() !QFileInfo(folder).exists()
&& FolderMan::instance()->checkPathValidityForNewFolder(folder, serverUrl).isEmpty(); && FolderMan::instance()->checkPathValidityForNewFolder(folder, serverUrl).second.isEmpty();
if (isGood) { if (isGood) {
break; break;
} }

12
src/gui/folderman.h
View file

@ -63,6 +63,14 @@ class FolderMan : public QObject
{ {
Q_OBJECT Q_OBJECT
public: public:
enum class PathValidityResult {
Valid,
ErrorRecursiveValidity,
ErrorContainsFolder,
ErrorContainedInFolder,
ErrorNonEmptyFolder
};
~FolderMan() override; ~FolderMan() override;
static FolderMan *instance(); static FolderMan *instance();
@ -134,9 +142,9 @@ public:
* *
* Note that different accounts are allowed to sync to the same folder. * Note that different accounts are allowed to sync to the same folder.
* *
* @returns an empty string if it is allowed, or an error if it is not allowed * @returns an empty string and PathValidityResult::Valid if it is allowed, or an error if it is not allowed
*/ */
QString checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl = QUrl()) const; QPair<PathValidityResult, QString> checkPathValidityForNewFolder(const QString &path, const QUrl &serverUrl = QUrl()) const;
/** /**
* Attempts to find a non-existing, acceptable path for creating a new sync folder. * Attempts to find a non-existing, acceptable path for creating a new sync folder.

4
src/gui/folderwizard.cpp
View file

@ -98,8 +98,8 @@ bool FolderWizardLocalPath::isComplete() const
QUrl serverUrl = _account->url(); QUrl serverUrl = _account->url();
serverUrl.setUserName(_account->credentials()->user()); serverUrl.setUserName(_account->credentials()->user());
QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder( const auto errorStr = FolderMan::instance()->checkPathValidityForNewFolder(
QDir::fromNativeSeparators(_ui.localFolderLineEdit->text()), serverUrl); QDir::fromNativeSeparators(_ui.localFolderLineEdit->text()), serverUrl).second;
bool isOk = errorStr.isEmpty(); bool isOk = errorStr.isEmpty();

5
src/gui/wizard/owncloudadvancedsetuppage.cpp
View file

@ -258,8 +258,9 @@ void OwncloudAdvancedSetupPage::updateStatus()
const QString locFolder = localFolder(); const QString locFolder = localFolder();
// check if the local folder exists. If so, and if its not empty, show a warning. // check if the local folder exists. If so, and if its not empty, show a warning.
QString errorStr = FolderMan::instance()->checkPathValidityForNewFolder(locFolder, serverUrl()); const auto pathValidityCheckResult = FolderMan::instance()->checkPathValidityForNewFolder(locFolder, serverUrl());
_localFolderValid = errorStr.isEmpty(); auto errorStr = pathValidityCheckResult.second;
_localFolderValid = errorStr.isEmpty() || pathValidityCheckResult.first == FolderMan::PathValidityResult::ErrorNonEmptyFolder;
QString t; QString t;

76
test/testfolderman.cpp
View file

@ -65,17 +65,17 @@ private slots:
// those should be allowed // those should be allowed
// QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory) // QString FolderMan::checkPathValidityForNewFolder(const QString& path, const QUrl &serverUrl, bool forNewDirectory).second
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/free"), QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/free").second, QString());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/free2/"), QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/free2/").second, QString());
// Not an existing directory -> Ok // Not an existing directory -> Ok
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu"), QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu").second, QString());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/free/bliblablu"), QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/free/bliblablu").second, QString());
// QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu/some/more"), QString()); // QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/bliblablu/some/more").second, QString());
// A file -> Error // A file -> Error
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/file.txt").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/file.txt").second.isNull());
// There are folders configured in those folders, url needs to be taken into account: -> ERROR // There are folders configured in those folders, url needs to be taken into account: -> ERROR
QUrl url2(url); QUrl url2(url);
@ -83,18 +83,18 @@ private slots:
url2.setUserName(user); url2.setUserName(user);
// The following both fail because they refer to the same account (user and url) // The following both fail because they refer to the same account (user and url)
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url2).isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url2).second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url2).isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url2).second.isNull());
// Now it will work because the account is different // Now it will work because the account is different
QUrl url3("http://anotherexample.org"); QUrl url3("http://anotherexample.org");
url3.setUserName("dummy"); url3.setUserName("dummy");
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3), QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1", url3).second, QString());
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url3), QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/", url3).second, QString());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath).isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath).second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/f").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/f").second.isNull());
#ifndef Q_OS_WIN // no links on windows, no permissions #ifndef Q_OS_WIN // no links on windows, no permissions
// make a bunch of links // make a bunch of links
@ -104,56 +104,56 @@ private slots:
QVERIFY(QFile::link(dirPath + "/sub/ownCloud1/folder", dirPath + "/link4")); QVERIFY(QFile::link(dirPath + "/sub/ownCloud1/folder", dirPath + "/link4"));
// Ok // Ok
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1").isNull()); QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1").second.isNull());
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free").isNull()); QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free").second.isNull());
// Not Ok // Not Ok
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link2").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link2").second.isNull());
// link 3 points to an existing sync folder. To make it fail, the account must be the same // link 3 points to an existing sync folder. To make it fail, the account must be the same
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3", url2).isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3", url2).second.isNull());
// while with a different account, this is fine // while with a different account, this is fine
QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3), QString()); QCOMPARE(folderman->checkPathValidityForNewFolder(dirPath + "/link3", url3).second, QString());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link4").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link4").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder").second.isNull());
// test some non existing sub path (error) // test some non existing sub path (error)
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/some/sub/path").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/some/sub/path").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/g/h").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/sub/ownCloud1/folder/g/h").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder/neu_folder").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/link3/folder/neu_folder").second.isNull());
// Subfolder of links // Subfolder of links
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1/subfolder").isNull()); QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link1/subfolder").second.isNull());
QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free/subfolder").isNull()); QVERIFY(folderman->checkPathValidityForNewFolder(dirPath + "/link2/free/subfolder").second.isNull());
// Should not have the rights // Should not have the rights
QVERIFY(!folderman->checkPathValidityForNewFolder("/").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder("/").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder("/usr/bin/somefolder").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder("/usr/bin/somefolder").second.isNull());
#endif #endif
#ifdef Q_OS_WIN // drive-letter tests #ifdef Q_OS_WIN // drive-letter tests
if (!QFileInfo("v:/").exists()) { if (!QFileInfo("v:/").exists()) {
QVERIFY(!folderman->checkPathValidityForNewFolder("v:").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder("v:").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder("v:/").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder("v:/").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder("v:/foo").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder("v:/foo").second.isNull());
} }
if (QFileInfo("c:/").isWritable()) { if (QFileInfo("c:/").isWritable()) {
QVERIFY(folderman->checkPathValidityForNewFolder("c:").isNull()); QVERIFY(folderman->checkPathValidityForNewFolder("c:").second.isNull());
QVERIFY(folderman->checkPathValidityForNewFolder("c:/").isNull()); QVERIFY(folderman->checkPathValidityForNewFolder("c:/").second.isNull());
QVERIFY(folderman->checkPathValidityForNewFolder("c:/foo").isNull()); QVERIFY(folderman->checkPathValidityForNewFolder("c:/foo").second.isNull());
} }
#endif #endif
// Invalid paths // Invalid paths
QVERIFY(!folderman->checkPathValidityForNewFolder("").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder("").second.isNull());
// REMOVE ownCloud2 from the filesystem, but keep a folder sync'ed to it. // REMOVE ownCloud2 from the filesystem, but keep a folder sync'ed to it.
QDir(dirPath + "/ownCloud2/").removeRecursively(); QDir(dirPath + "/ownCloud2/").removeRecursively();
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/blublu").second.isNull());
QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/sub/subsub/sub").isNull()); QVERIFY(!folderman->checkPathValidityForNewFolder(dirPath + "/ownCloud2/sub/subsub/sub").second.isNull());
} }
void testFindGoodPathForNewSyncFolder() void testFindGoodPathForNewSyncFolder()