Merge pull request #6655 from nextcloud/bugfix/folder-conflict-disappear

Bugfix/folder conflict disappear
This commit is contained in:
allexzander 2024-04-23 12:18:25 +02:00 committed by GitHub
commit f40b8ae198
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 204 additions and 10 deletions

View file

@ -404,6 +404,11 @@ bool ProcessDirectoryJob::handleExcluded(const QString &path, const Entries &ent
}
}
if (_dirItem) {
_dirItem->_isAnyInvalidCharChild = _dirItem->_isAnyInvalidCharChild || item->_status == SyncFileItem::FileNameInvalid;
_dirItem->_isAnyCaseClashChild = _dirItem->_isAnyCaseClashChild || item->_status == SyncFileItem::FileNameClash;
}
_childIgnored = true;
emit _discoveryData->itemDiscovered(item);
return true;

View file

@ -1291,6 +1291,19 @@ void PropagatorCompositeJob::slotSubJobFinished(SyncFileItem::Status status)
auto *subJob = dynamic_cast<PropagatorJob *>(sender());
ASSERT(subJob);
if (!_isAnyInvalidCharChild || !_isAnyCaseClashChild) {
SyncFileItemPtr childDirItem;
if (const auto propagateDirectoryjob = qobject_cast<PropagateDirectory *>(subJob)) {
childDirItem = propagateDirectoryjob->_item;
} else if (const auto propagateIgnoreJob = qobject_cast<PropagateIgnoreJob *>(subJob)) {
childDirItem = propagateIgnoreJob->_item;
}
if (childDirItem) {
_isAnyCaseClashChild = _isAnyCaseClashChild || childDirItem->_status == SyncFileItem::FileNameClash || childDirItem->_isAnyCaseClashChild;
_isAnyInvalidCharChild = _isAnyInvalidCharChild || childDirItem->_status == SyncFileItem::FileNameInvalid || childDirItem->_isAnyInvalidCharChild;
}
}
// Delete the job and remove it from our list of jobs.
subJob->deleteLater();
int i = _runningJobs.indexOf(subJob);
@ -1407,6 +1420,8 @@ void PropagateDirectory::slotFirstJobFinished(SyncFileItem::Status status)
void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
{
if (!_item->isEmpty() && status == SyncFileItem::Success) {
_item->_isAnyCaseClashChild = _item->_isAnyCaseClashChild || _subJobs._isAnyCaseClashChild;
_item->_isAnyInvalidCharChild = _item->_isAnyInvalidCharChild || _subJobs._isAnyInvalidCharChild;
// If a directory is renamed, recursively delete any stale items
// that may still exist below the old path.
if (_item->_instruction == CSYNC_INSTRUCTION_RENAME && _item->_originalFile != _item->_renameTarget) {
@ -1496,15 +1511,16 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
}
}
#endif
const auto result = propagator()->updateMetadata(*_item);
if (!result) {
status = _item->_status = SyncFileItem::FatalError;
_item->_errorString = tr("Error updating metadata: %1").arg(result.error());
qCWarning(lcDirectory) << "Error writing to the database for file" << _item->_file << "with" << result.error();
} else if (*result == Vfs::ConvertToPlaceholderResult::Locked) {
_item->_status = SyncFileItem::SoftError;
_item->_errorString = tr("File is currently in use");
if (!_item->_isAnyCaseClashChild && !_item->_isAnyInvalidCharChild) {
const auto result = propagator()->updateMetadata(*_item);
if (!result) {
status = _item->_status = SyncFileItem::FatalError;
_item->_errorString = tr("Error updating metadata: %1").arg(result.error());
qCWarning(lcDirectory) << "Error writing to the database for file" << _item->_file << "with" << result.error();
} else if (*result == Vfs::ConvertToPlaceholderResult::Locked) {
_item->_status = SyncFileItem::SoftError;
_item->_errorString = tr("File is currently in use");
}
}
}
}

View file

@ -246,6 +246,8 @@ public:
QVector<PropagatorJob *> _runningJobs;
SyncFileItem::Status _hasError = SyncFileItem::NoStatus; // NoStatus, or NormalError / SoftError if there was an error
quint64 _abortsCount = 0;
bool _isAnyCaseClashChild = false;
bool _isAnyInvalidCharChild = false;
explicit PropagatorCompositeJob(OwncloudPropagator *propagator)
: PropagatorJob(propagator)

View file

@ -302,7 +302,17 @@ void PropagateLocalRename::start()
if (QString::compare(_item->_file, _item->_renameTarget, Qt::CaseInsensitive) != 0
&& propagator()->localFileNameClash(_item->_renameTarget)) {
qCInfo(lcPropagateLocalRename) << "renaming a case clashed file" << _item->_file << _item->_renameTarget;
qCInfo(lcPropagateLocalRename) << "renaming a case clashed item" << _item->_file << _item->_renameTarget;
if (_item->isDirectory()) {
// #HotFix
// fix a crash (we can not create a conflicted copy for folders)
// right now, the conflict resolution will not even work for this scenario with folders,
// but, let's fix it step by step, this will be a second stage
done(SyncFileItem::FileNameClash,
tr("Folder %1 cannot be renamed because of a local file or folder name clash!").arg(_item->_file),
ErrorCategory::GenericError);
return;
}
const auto caseClashConflictResult = propagator()->createCaseClashConflict(_item, existingFile);
if (caseClashConflictResult) {
done(SyncFileItem::SoftError, *caseClashConflictResult, ErrorCategory::GenericError);

View file

@ -335,6 +335,9 @@ public:
bool _isFileDropDetected = false;
bool _isEncryptedMetadataNeedUpdate = false;
bool _isAnyInvalidCharChild = false;
bool _isAnyCaseClashChild = false;
};
inline bool operator<(const SyncFileItemPtr &item1, const SyncFileItemPtr &item2)

View file

@ -258,6 +258,21 @@ FileInfo *FileInfo::find(PathComponents pathComponents, const bool invalidateEta
return nullptr;
}
FileInfo FileInfo::findRecursive(PathComponents pathComponents, const bool invalidateEtags)
{
auto result = find({pathComponents.takeFirst()}, invalidateEtags);
if (!result) {
return *result;
}
for (const auto &pathComponent : pathComponents) {
if (!result) {
break;
}
result = result->find({pathComponent});
}
return *result;
}
FileInfo *FileInfo::createDir(const QString &relativePath)
{
const PathComponents pathComponents { relativePath };

View file

@ -147,6 +147,7 @@ public:
void setE2EE(const QString &relativepath, const bool enabled) override;
FileInfo *find(PathComponents pathComponents, const bool invalidateEtags = false);
FileInfo findRecursive(PathComponents pathComponents, const bool invalidateEtags = false);
FileInfo *createDir(const QString &relativePath);

View file

@ -1703,6 +1703,148 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
}
void testServer_caseClash_createDiverseConflictsInsideOneFolderAndSolveThem()
{
FakeFolder fakeFolder{FileInfo{}};
const QStringList conflictsFolderPathComponents = {"Documents", "DiverseConflicts"};
QString diverseConflictsFolderPath;
for (const auto &conflictsFolderPathComponent : conflictsFolderPathComponents) {
if (diverseConflictsFolderPath.isEmpty()) {
diverseConflictsFolderPath += conflictsFolderPathComponent;
} else {
diverseConflictsFolderPath += "/" + conflictsFolderPathComponent;
}
fakeFolder.remoteModifier().mkdir(diverseConflictsFolderPath);
}
constexpr auto testLowerCaseFile = "testfile";
constexpr auto testUpperCaseFile = "TESTFILE";
constexpr auto testLowerCaseFolder = "testfolder";
constexpr auto testUpperCaseFolder = "TESTFOLDER";
constexpr auto testInvalidCharFolder = "Really?";
fakeFolder.remoteModifier().insert(diverseConflictsFolderPath + "/" + testLowerCaseFile);
fakeFolder.remoteModifier().insert(diverseConflictsFolderPath + "/" + testUpperCaseFile);
fakeFolder.remoteModifier().mkdir(diverseConflictsFolderPath + "/" + testLowerCaseFolder);
fakeFolder.remoteModifier().mkdir(diverseConflictsFolderPath + "/" + testUpperCaseFolder);
fakeFolder.remoteModifier().mkdir(diverseConflictsFolderPath + "/" + testInvalidCharFolder);
#if defined Q_OS_LINUX
constexpr auto shouldHaveCaseClashConflict = false;
#else
constexpr auto shouldHaveCaseClashConflict = true;
#endif
if (shouldHaveCaseClashConflict) {
ItemCompletedSpy completeSpy(fakeFolder);
fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem);
QVERIFY(fakeFolder.syncOnce());
// verify the parent of a folder where caseclash and invalidchar conflicts were found, has corresponding flags set (conflict info must get
// propagated to the very top)
const auto diverseConflictsFolderParent = completeSpy.findItem(conflictsFolderPathComponents.first());
QVERIFY(diverseConflictsFolderParent);
QVERIFY(diverseConflictsFolderParent->_isAnyCaseClashChild);
QVERIFY(diverseConflictsFolderParent->_isAnyInvalidCharChild);
completeSpy.clear();
auto diverseConflictsFolderInfo = fakeFolder.currentLocalState().findRecursive(diverseConflictsFolderPath);
QVERIFY(!diverseConflictsFolderInfo.name.isEmpty());
auto conflictsFile = findCaseClashConflicts(diverseConflictsFolderInfo);
QCOMPARE(conflictsFile.size(), shouldHaveCaseClashConflict ? 1 : 0);
const auto hasFileCaseClashConflict = expectConflict(diverseConflictsFolderInfo, testLowerCaseFile);
QCOMPARE(hasFileCaseClashConflict, shouldHaveCaseClashConflict ? true : false);
fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem);
QVERIFY(fakeFolder.syncOnce());
diverseConflictsFolderInfo = fakeFolder.currentLocalState().findRecursive(diverseConflictsFolderPath);
QVERIFY(!diverseConflictsFolderInfo.name.isEmpty());
conflictsFile = findCaseClashConflicts(diverseConflictsFolderInfo);
QCOMPARE(conflictsFile.size(), shouldHaveCaseClashConflict ? 1 : 0);
const auto conflictFileName = QString{conflictsFile.constFirst()};
qDebug() << conflictFileName;
CaseClashConflictSolver conflictSolver(fakeFolder.localPath() + diverseConflictsFolderPath + "/" + testLowerCaseFile,
fakeFolder.localPath() + conflictFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
fakeFolder.account(),
&fakeFolder.syncJournal());
QSignalSpy conflictSolverDone(&conflictSolver, &CaseClashConflictSolver::done);
conflictSolver.solveConflict("testfile2");
QVERIFY(conflictSolverDone.wait());
QVERIFY(fakeFolder.syncOnce());
diverseConflictsFolderInfo = fakeFolder.currentLocalState().findRecursive(diverseConflictsFolderPath);
QVERIFY(!diverseConflictsFolderInfo.name.isEmpty());
conflictsFile = findCaseClashConflicts(diverseConflictsFolderInfo);
QCOMPARE(conflictsFile.size(), 0);
fakeFolder.syncEngine().setLocalDiscoveryOptions(OCC::LocalDiscoveryStyle::DatabaseAndFilesystem);
QVERIFY(fakeFolder.syncOnce());
// After solving file conflict, verify that we did not lose conflict detection of caseclash and invalidchar for folders
for (auto it = completeSpy.begin(); it != completeSpy.end(); ++it) {
auto item = (*it).first().value<OCC::SyncFileItemPtr>();
item = nullptr;
}
auto invalidFilenameConflictFolderItem = completeSpy.findItem(diverseConflictsFolderPath + "/" + testInvalidCharFolder);
QVERIFY(invalidFilenameConflictFolderItem);
QVERIFY(invalidFilenameConflictFolderItem->_status == SyncFileItem::FileNameInvalid);
auto caseClashConflictFolderItemLower = completeSpy.findItem(diverseConflictsFolderPath + "/" + testLowerCaseFolder);
auto caseClashConflictFolderItemUpper = completeSpy.findItem(diverseConflictsFolderPath + "/" + testUpperCaseFolder);
completeSpy.clear();
// we always create UPPERCASE folder in current syncengine logic for now and then create a conflict for a lowercase folder, but this may change, so
// keep this check more future proof
const auto upperOrLowerCaseFolderCaseClashFound =
(caseClashConflictFolderItemLower && caseClashConflictFolderItemLower->_status == SyncFileItem::FileNameClash)
|| (caseClashConflictFolderItemUpper && caseClashConflictFolderItemUpper->_status == SyncFileItem::FileNameClash);
QVERIFY(upperOrLowerCaseFolderCaseClashFound);
// solve case clash folders conflict
CaseClashConflictSolver conflictSolverCaseClashForFolder(fakeFolder.localPath() + diverseConflictsFolderPath + "/" + testLowerCaseFolder,
fakeFolder.localPath() + diverseConflictsFolderPath + "/" + testUpperCaseFolder,
QStringLiteral("/"),
fakeFolder.localPath(),
fakeFolder.account(),
&fakeFolder.syncJournal());
QSignalSpy conflictSolverCaseClashForFolderDone(&conflictSolverCaseClashForFolder, &CaseClashConflictSolver::done);
conflictSolverCaseClashForFolder.solveConflict("testfolder1");
QVERIFY(conflictSolverCaseClashForFolderDone.wait());
QVERIFY(fakeFolder.syncOnce());
// verify no case clash conflicts folder items are found
caseClashConflictFolderItemLower = completeSpy.findItem(diverseConflictsFolderPath + "/" + testLowerCaseFolder);
caseClashConflictFolderItemUpper = completeSpy.findItem(diverseConflictsFolderPath + "/" + testUpperCaseFolder);
QVERIFY((!caseClashConflictFolderItemLower || caseClashConflictFolderItemLower->_file.isEmpty())
&& (!caseClashConflictFolderItemUpper || caseClashConflictFolderItemUpper->_file.isEmpty()));
// veriy invalid filename conflict folder item is still present
invalidFilenameConflictFolderItem = completeSpy.findItem(diverseConflictsFolderPath + "/" + testInvalidCharFolder);
completeSpy.clear();
QVERIFY(invalidFilenameConflictFolderItem);
QVERIFY(invalidFilenameConflictFolderItem->_status == SyncFileItem::FileNameInvalid);
}
}
void testExistingFolderBecameBig()
{
constexpr auto testFolder = "folder";