StatusTracker: Fix different case paths not matching (#5981)

Use a custom std::map comparator functor to do all comparisons
on contained QStrings using Qt::CaseInsensitive on macOS and Windows.

Issue #5257
This commit is contained in:
Jocelyn Turcotte 2017-08-31 13:32:00 +02:00 committed by GitHub
parent 839361594d
commit 48d2fc1599
3 changed files with 59 additions and 6 deletions

View File

@ -25,20 +25,49 @@ namespace OCC {
Q_LOGGING_CATEGORY(lcStatusTracker, "sync.statustracker", QtInfoMsg)
static SyncFileStatus::SyncFileStatusTag lookupProblem(const QString &pathToMatch, const std::map<QString, SyncFileStatus::SyncFileStatusTag> &problemMap)
static int pathCompare( const QString& lhs, const QString& rhs )
{
// Should match Utility::fsCasePreserving, we want don't want to pay for the runtime check on every comparison.
return lhs.compare(rhs,
#if defined(Q_OS_WIN) || defined(Q_OS_MAC)
Qt::CaseInsensitive
#else
Qt::CaseSensitive
#endif
);
}
static bool pathStartsWith( const QString& lhs, const QString& rhs )
{
return lhs.startsWith(rhs,
#if defined(Q_OS_WIN) || defined(Q_OS_MAC)
Qt::CaseInsensitive
#else
Qt::CaseSensitive
#endif
);
}
bool SyncFileStatusTracker::PathComparator::operator()( const QString& lhs, const QString& rhs ) const
{
// This will make sure that the std::map is ordered and queried case-insensitively on macOS and Windows.
return pathCompare(lhs, rhs) < 0;
}
SyncFileStatus::SyncFileStatusTag SyncFileStatusTracker::lookupProblem(const QString &pathToMatch, const SyncFileStatusTracker::ProblemsMap &problemMap)
{
auto lower = problemMap.lower_bound(pathToMatch);
for (auto it = lower; it != problemMap.cend(); ++it) {
const QString &problemPath = it->first;
SyncFileStatus::SyncFileStatusTag severity = it->second;
if (problemPath == pathToMatch) {
if (pathCompare(problemPath, pathToMatch) == 0) {
return severity;
} else if (severity == SyncFileStatus::StatusError
&& problemPath.startsWith(pathToMatch)
&& pathStartsWith(problemPath, pathToMatch)
&& (pathToMatch.isEmpty() || problemPath.at(pathToMatch.size()) == '/')) {
return SyncFileStatus::StatusWarning;
} else if (!problemPath.startsWith(pathToMatch)) {
} else if (!pathStartsWith(problemPath, pathToMatch)) {
// Starting at lower_bound we get the first path that is not smaller,
// since: "a/" < "a/aa" < "a/aa/aaa" < "a/ab/aba"
// If problemMap keys are ["a/aa/aaa", "a/ab/aba"] and pathToMatch == "a/aa",
@ -182,7 +211,7 @@ void SyncFileStatusTracker::slotAboutToPropagate(SyncFileItemVector &items)
{
ASSERT(_syncCount.isEmpty());
std::map<QString, SyncFileStatus::SyncFileStatusTag> oldProblems;
ProblemsMap oldProblems;
std::swap(_syncProblems, oldProblems);
foreach (const SyncFileItemPtr &item, items) {

View File

@ -51,6 +51,12 @@ private slots:
void slotSyncEngineRunningChanged();
private:
struct PathComparator {
bool operator()( const QString& lhs, const QString& rhs ) const;
};
typedef std::map<QString, SyncFileStatus::SyncFileStatusTag, PathComparator> ProblemsMap;
SyncFileStatus::SyncFileStatusTag lookupProblem(const QString &pathToMatch, const ProblemsMap &problemMap);
enum SharedFlag { UnknownShared,
NotShared,
Shared };
@ -65,7 +71,7 @@ private:
SyncEngine *_syncEngine;
std::map<QString, SyncFileStatus::SyncFileStatusTag> _syncProblems;
ProblemsMap _syncProblems;
QSet<QString> _dirtyPaths;
// Counts the number direct children currently being synced (has unfinished propagation jobs).
// We'll show a file/directory as SYNC as long as its sync count is > 0.

View File

@ -260,6 +260,24 @@ private slots:
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
}
void warningStatusForExcludedFile_CasePreserving() {
FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
fakeFolder.syncEngine().excludedFiles().addExcludeExpr("B");
fakeFolder.serverErrorPaths().append("A/a1");
fakeFolder.localModifier().appendByte("A/a1");
fakeFolder.syncOnce();
QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus(""), SyncFileStatus(SyncFileStatus::StatusWarning));
QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("A"), SyncFileStatus(SyncFileStatus::StatusWarning));
QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("A/a1"), SyncFileStatus(SyncFileStatus::StatusError));
QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("B"), SyncFileStatus(SyncFileStatus::StatusWarning));
// Should still get the status for different casing on macOS and Windows.
QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("a"), SyncFileStatus(Utility::fsCasePreserving() ? SyncFileStatus::StatusWarning : SyncFileStatus::StatusNone));
QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("A/A1"), SyncFileStatus(Utility::fsCasePreserving() ? SyncFileStatus::StatusError : SyncFileStatus::StatusNone));
QCOMPARE(fakeFolder.syncEngine().syncFileStatusTracker().fileStatus("b"), SyncFileStatus(Utility::fsCasePreserving() ? SyncFileStatus::StatusWarning : SyncFileStatus::StatusNone));
}
void parentsGetWarningStatusForError() {
FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
fakeFolder.serverErrorPaths().append("A/a1");