Bugfix/folder conflict disappear

src/libsync/discovery.cpp

@@ -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;

src/libsync/owncloudpropagator.cpp

@@ -1,4 +1,4 @@
 /*
  * Copyright (C) by Olivier Goffart <[email protected]>
  * Copyright (C) by Klaas Freitag <[email protected]>
  *
@@ -1291,6 +1291,19 @@
     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::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 @@
                 }
             }
 #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");
+                }
             }
         }
     }

src/libsync/owncloudpropagator.h

@@ -15,7 +15,7 @@
 #ifndef OWNCLOUDPROPAGATOR_H
 #define OWNCLOUDPROPAGATOR_H
 
 #include <QHash>
 #include <QObject>
 #include <QMap>
 #include <QElapsedTimer>
@@ -246,6 +246,8 @@
     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)

src/libsync/propagatorjobs.cpp

@@ -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);

src/libsync/syncfileitem.h

@@ -15,7 +15,7 @@
 #ifndef SYNCFILEITEM_H
 #define SYNCFILEITEM_H
 
 #include <QVector>
 #include <QString>
 #include <QDateTime>
 #include <QMetaType>
@@ -335,6 +335,9 @@
     bool _isFileDropDetected = false;
 
     bool _isEncryptedMetadataNeedUpdate = false;
+
+    bool _isAnyInvalidCharChild = false;
+    bool _isAnyCaseClashChild = false;
 };
 
 inline bool operator<(const SyncFileItemPtr &item1, const SyncFileItemPtr &item2)

test/syncenginetestutils.cpp

@@ -258,7 +258,22 @@
     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 };
     FileInfo *parent = findInvalidatingEtags(pathComponents.parentDirComponents());

test/syncenginetestutils.h

@@ -6,7 +6,7 @@
  */
 #pragma once
 
 #include "account.h"
 #include "common/result.h"
 #include "creds/abstractcredentials.h"
 #include "logger.h"
@@ -147,6 +147,7 @@
     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);
 

test/testsyncengine.cpp

@@ -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";