Refactor the overlay icon logic to show errors as a warning for parent folders #3634

This also remove all smartness from the SocketApi about the status
of a file and solely use info from the current and last sync.
This simplifies the logic a lot and prevents any discrepancy between
the status shown in the activity log and the one displayed on the
overlay icon of a file.

The main benefit of the additional simplicity is that we are able
to push all new status of a file reliably (including warnings for
parent folders) to properly update the icon on overlay implementations
that don't allow us invalidating the status cache, like on OS X.

Both errors and warning from the last sync are now kept in a set,
which is used to also affect parent folders of an error.

To make sure that errors don't become warning icons on a second
sync, SyncFileItem::_hasBlacklistEntry is also interpreted as an error.
This also renames StatusIgnore to StatusWarning to match this semantic.

SyncEngine::aboutToPropagate is used in favor of SyncEngine::syncItemDiscovered
since the latter is emitted before file permission warnings are set on the
SyncFileItem. SyncEngine::finished is not used since we have all the
needed information in SyncEngine::itemCompleted.
This commit is contained in:
Jocelyn Turcotte 2016-03-24 17:42:04 +01:00
parent 69aa39f1f6
commit 82190eaa81
8 changed files with 128 additions and 234 deletions

View file

@ -1306,33 +1306,14 @@ void SyncEngine::restoreOldFiles()
}
}
bool SyncEngine::estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s)
SyncFileItem* SyncEngine::findSyncItem(const QString &fileName) const
{
Q_UNUSED(t);
QString pat(fn);
if( t == CSYNC_FTW_TYPE_DIR && ! fn.endsWith(QLatin1Char('/'))) {
pat.append(QLatin1Char('/'));
}
Q_FOREACH(const SyncFileItemPtr &item, _syncedItems) {
//qDebug() << Q_FUNC_INFO << fn << item->_status << item->_file << fn.startsWith(item->_file) << item->_file.startsWith(fn);
if (item->_file.startsWith(pat) ||
item->_file == fn || item->_renameTarget == fn /* the same directory or file */) {
if (item->_status == SyncFileItem::NormalError
|| item->_status == SyncFileItem::FatalError)
s->set(SyncFileStatus::StatusError);
else if (item->_status == SyncFileItem::FileIgnored)
s->set(SyncFileStatus::StatusIgnore);
else if (item->_status == SyncFileItem::Success)
s->set(SyncFileStatus::StatusUpToDate);
else
s->set(SyncFileStatus::StatusSync);
qDebug() << Q_FUNC_INFO << "Setting" << fn << "to" << s->toSocketAPIString();
return true;
}
// Directories will appear in this list as well, and will get their status set once all children have been propagated
if ((item->_file == fileName || item->_renameTarget == fileName))
return item.data();
}
return false;
return 0;
}
qint64 SyncEngine::timeSinceFileTouched(const QString& fn) const

View file

@ -85,7 +85,7 @@ public:
/* Return true if we detected that another sync is needed to complete the sync */
bool isAnotherSyncNeeded() { return _anotherSyncNeeded; }
bool estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s);
SyncFileItem* findSyncItem(const QString &fileName) const;
/** Get the ms since a file was touched, or -1 if it wasn't.
*

View file

@ -59,7 +59,8 @@ QString SyncFileStatus::toSocketAPIString() const
case StatusSync:
statusString = QLatin1String("SYNC");
break;
case StatusIgnore:
case StatusWarning:
// The protocol says IGNORE, but all implementations show a yellow warning sign.
statusString = QLatin1String("IGNORE");
break;
case StatusUpToDate:

View file

@ -30,7 +30,7 @@ public:
enum SyncFileStatusTag {
StatusNone,
StatusSync,
StatusIgnore,
StatusWarning,
StatusUpToDate,
StatusError,
};

View file

@ -1,5 +1,6 @@
/*
* Copyright (C) by Klaas Freitag <freitag@owncloud.com>
* Copyright (C) by Jocelyn Turcotte <jturcotte@woboq.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@ -12,17 +13,25 @@
*/
#include "syncfilestatustracker.h"
#include "filesystem.h"
#include "syncengine.h"
#include "syncjournaldb.h"
#include "syncjournalfilerecord.h"
#include "utility.h"
#include <QDir>
#include <QFileInfo>
namespace OCC {
static SyncFileStatus::SyncFileStatusTag lookupProblem(const QString &pathToMatch, const std::set<Problem> &set)
{
for (auto it = set.cbegin(); it != set.cend(); ++it) {
qDebug() << Q_FUNC_INFO << pathToMatch << it->severity << it->path;
auto problemPath = it->path;
if (problemPath == pathToMatch)
return it->severity;
else if (it->severity == SyncFileStatus::StatusError && problemPath.startsWith(pathToMatch))
return SyncFileStatus::StatusWarning;
}
return SyncFileStatus::StatusNone;
}
/**
* Whether this item should get an ERROR icon through the Socket API.
*
@ -31,229 +40,143 @@ namespace OCC {
* icon as the problem is most likely going to resolve itself quickly and
* automatically.
*/
static bool showErrorInSocketApi(const SyncFileItem& item)
static inline bool showErrorInSocketApi(const SyncFileItem& item)
{
const auto status = item._status;
return status == SyncFileItem::NormalError
|| status == SyncFileItem::FatalError;
|| status == SyncFileItem::FatalError
|| item._hasBlacklistEntry;
}
static void addErroredSyncItemPathsToList(const SyncFileItemVector& items, QSet<QString>* set) {
foreach (const SyncFileItemPtr &item, items) {
if (showErrorInSocketApi(*item)) {
set->insert(item->_file);
}
}
static inline bool showWarningInSocketApi(const SyncFileItem& item)
{
const auto status = item._status;
return status == SyncFileItem::FileIgnored
|| status == SyncFileItem::Conflict
|| status == SyncFileItem::Restoration;
}
SyncFileStatusTracker::SyncFileStatusTracker(SyncEngine *syncEngine)
: _syncEngine(syncEngine)
{
connect(syncEngine, SIGNAL(treeWalkResult(const SyncFileItemVector&)),
this, SLOT(slotThreadTreeWalkResult(const SyncFileItemVector&)));
connect(syncEngine, SIGNAL(aboutToPropagate(SyncFileItemVector&)),
this, SLOT(slotAboutToPropagate(SyncFileItemVector&)));
connect(syncEngine, SIGNAL(finished(bool)), SLOT(slotSyncFinished()));
connect(syncEngine, SIGNAL(itemCompleted(const SyncFileItem&, const PropagatorJob&)),
this, SLOT(slotItemCompleted(const SyncFileItem&)));
connect(syncEngine, SIGNAL(syncItemDiscovered(const SyncFileItem&)),
this, SLOT(slotItemDiscovered(const SyncFileItem&)));
}
bool SyncFileStatusTracker::estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s)
{
if (t == CSYNC_FTW_TYPE_DIR) {
if (Utility::doesSetContainPrefix(_stateLastSyncItemsWithError, fn)) {
qDebug() << Q_FUNC_INFO << "Folder has error" << fn;
s->set(SyncFileStatus::StatusError);
return true;
}
// If sync is running, check _syncedItems, possibly give it StatusSync
if (_syncEngine->isSyncRunning()) {
if (_syncEngine->estimateState(fn, t, s)) {
return true;
}
}
return false;
} else if ( t== CSYNC_FTW_TYPE_FILE) {
// check if errorList has the directory/file
if (Utility::doesSetContainPrefix(_stateLastSyncItemsWithError, fn)) {
s->set(SyncFileStatus::StatusError);
return true;
}
// If sync running: _syncedItems -> SyncingState
if (_syncEngine->isSyncRunning()) {
if (_syncEngine->estimateState(fn, t, s)) {
return true;
}
}
}
return false;
}
/**
* Get status about a single file.
*/
SyncFileStatus SyncFileStatusTracker::fileStatus(const QString& systemFileName)
{
QString file = _syncEngine->localPath();
QString fileName = systemFileName.normalized(QString::NormalizationForm_C);
QString fileNameSlash = fileName;
if(fileName != QLatin1String("/") && !fileName.isEmpty()) {
file += fileName;
}
if( fileName.endsWith(QLatin1Char('/')) ) {
fileName.truncate(fileName.length()-1);
qDebug() << "Removed trailing slash: " << fileName;
} else {
fileNameSlash += QLatin1Char('/');
}
const QFileInfo fi(file);
if( !FileSystem::fileExists(file, fi) ) {
qDebug() << "OO File " << file << " is not existing";
return SyncFileStatus(SyncFileStatus::StatusError);
}
SyncFileItem* item = _syncEngine->findSyncItem(fileName);
if (item)
return fileStatus(*item);
// file is ignored?
// Qt considers .lnk files symlinks on Windows so we need to work
// around that here.
if( fi.isSymLink()
#ifdef Q_OS_WIN
&& fi.suffix() != "lnk"
#endif
) {
return SyncFileStatus(SyncFileStatus::StatusIgnore);
}
// If we're not currently syncing that file, look it up in the database to know if it's shared
SyncJournalFileRecord rec = _syncEngine->journal()->getFileRecord(fileName);
if (rec.isValid())
return fileStatus(rec.toSyncFileItem());
csync_ftw_type_e type = CSYNC_FTW_TYPE_FILE;
if( fi.isDir() ) {
type = CSYNC_FTW_TYPE_DIR;
}
// Is it excluded?
if( _syncEngine->excludedFiles().isExcluded(file, _syncEngine->localPath(), _syncEngine->ignoreHiddenFiles()) ) {
return SyncFileStatus(SyncFileStatus::StatusIgnore);
}
// Error if it is in the selective sync blacklist
foreach(const auto &s, _syncEngine->journal()->getSelectiveSyncList(SyncJournalDb::SelectiveSyncBlackList)) {
if (fileNameSlash.startsWith(s)) {
return SyncFileStatus(SyncFileStatus::StatusError);
}
}
SyncFileStatus status(SyncFileStatus::StatusNone);
SyncJournalFileRecord rec = _syncEngine->journal()->getFileRecord(fileName );
if (estimateState(fileName, type, &status)) {
qDebug() << "Folder estimated status for" << fileName << "to" << status.toSocketAPIString();
} else if (fileName == "") {
// sync folder itself
// FIXME: The new parent folder logic should take over this, treating the root the same as any folder.
} else if (type == CSYNC_FTW_TYPE_DIR) {
if (rec.isValid()) {
status.set(SyncFileStatus::StatusUpToDate);
} else {
qDebug() << "Could not determine state for folder" << fileName << "will set StatusSync";
status.set(SyncFileStatus::StatusSync);
}
} else if (type == CSYNC_FTW_TYPE_FILE) {
if (rec.isValid()) {
if( FileSystem::getModTime(fi.absoluteFilePath()) == Utility::qDateTimeToTime_t(rec._modtime) ) {
status.set(SyncFileStatus::StatusUpToDate);
} else {
if (rec._remotePerm.isNull() || rec._remotePerm.contains("W") ) {
status.set(SyncFileStatus::StatusSync);
} else {
status.set(SyncFileStatus::StatusError);
}
}
} else {
qDebug() << "Could not determine state for file" << fileName << "will set StatusSync";
status.set(SyncFileStatus::StatusSync);
}
}
if (rec.isValid() && rec._remotePerm.contains("S"))
status.setSharedWithMe(true);
// FIXME: Wrong, but will go away
if (status.tag() == SyncFileStatus::StatusSync) {
// check the parent folder if it is shared and if it is allowed to create a file/dir within
QDir d( fi.path() );
auto parentPath = d.path();
auto dirRec = _syncEngine->journal()->getFileRecord(parentPath);
bool isDir = type == CSYNC_FTW_TYPE_DIR;
while( !d.isRoot() && !(d.exists() && dirRec.isValid()) ) {
d.cdUp(); // returns true if the dir exists.
parentPath = d.path();
// cut the folder path
dirRec = _syncEngine->journal()->getFileRecord(parentPath);
isDir = true;
}
if( dirRec.isValid() && !dirRec._remotePerm.isNull()) {
if( (isDir && !dirRec._remotePerm.contains("K"))
|| (!isDir && !dirRec._remotePerm.contains("C")) ) {
status.set(SyncFileStatus::StatusError);
}
}
}
return status;
}
void SyncFileStatusTracker::slotThreadTreeWalkResult(const SyncFileItemVector& items)
{
addErroredSyncItemPathsToList(items, &_stateLastSyncItemsWithErrorNew);
// Must be a new file, wait for the filesystem watcher to trigger a sync
return SyncFileStatus();
}
void SyncFileStatusTracker::slotAboutToPropagate(SyncFileItemVector& items)
{
addErroredSyncItemPathsToList(items, &_stateLastSyncItemsWithErrorNew);
}
std::set<Problem> oldProblems;
std::swap(_syncProblems, oldProblems);
void SyncFileStatusTracker::slotSyncFinished()
{
_stateLastSyncItemsWithError = _stateLastSyncItemsWithErrorNew;
_stateLastSyncItemsWithErrorNew.clear();
foreach (const SyncFileItemPtr &item, items) {
qDebug() << Q_FUNC_INFO << "Investigating" << item->destination() << item->_status;
if (showErrorInSocketApi(*item))
_syncProblems.insert({item->_file, SyncFileStatus::StatusError});
else if (showWarningInSocketApi(*item))
_syncProblems.insert({item->_file, SyncFileStatus::StatusWarning});
QString systemFileName = _syncEngine->localPath() + item->destination();
// the trailing slash for directories must be appended as the filenames coming in
// from the plugins have that too. Otherwise the matching entry item is not found
// in the plugin.
if( item->_type == SyncFileItem::Type::Directory )
systemFileName += QLatin1Char('/');
emit fileStatusChanged(systemFileName, fileStatus(*item));
}
// Make sure to push any status that might have been resolved indirectly since the last sync
// (like an error file being deleted from disk)
for (auto it = _syncProblems.begin(); it != _syncProblems.end(); ++it)
oldProblems.erase(*it);
for (auto it = oldProblems.begin(); it != oldProblems.end(); ++it) {
if (it->severity == SyncFileStatus::StatusError)
invalidateParentPaths(it->path);
emit fileStatusChanged(_syncEngine->localPath() + it->path, fileStatus(it->path));
}
}
void SyncFileStatusTracker::slotItemCompleted(const SyncFileItem &item)
{
qDebug() << Q_FUNC_INFO << item.destination() << item._status;
if (showErrorInSocketApi(item)) {
_stateLastSyncItemsWithErrorNew.insert(item._file);
_syncProblems.insert({item._file, SyncFileStatus::StatusError});
invalidateParentPaths(item.destination());
} else if (showWarningInSocketApi(item)) {
_syncProblems.insert({item._file, SyncFileStatus::StatusWarning});
} else {
// There is currently no situation where an error status set during discovery/update is fixed by propagation.
Q_ASSERT(_syncProblems.find(item._file) == _syncProblems.end());
}
QString systemFileName = _syncEngine->localPath() + item.destination();
// the trailing slash for directories must be appended as the filenames coming in
// from the plugins have that too. Otherwise the matching entry item is not found
// in the plugin.
if( item._type == SyncFileItem::Type::Directory ) {
systemFileName += QLatin1Char('/');
}
auto status = fileStatus(item.destination());
emit fileStatusChanged(systemFileName, status);
emit fileStatusChanged(systemFileName, fileStatus(item));
}
void SyncFileStatusTracker::slotItemDiscovered(const SyncFileItem &item)
SyncFileStatus SyncFileStatusTracker::fileStatus(const SyncFileItem& item)
{
QString systemFileName = _syncEngine->localPath() + item.destination();
// Hack to know if the item was taken from the sync engine (Sync), or from the database (UpToDate)
bool waitingForPropagation = item._direction != SyncFileItem::None && item._status == SyncFileItem::NoStatus;
// the trailing slash for directories must be appended as the filenames coming in
// from the plugins have that too. Otherwise the matching entry item is not found
// in the plugin.
if( item._type == SyncFileItem::Type::Directory ) {
systemFileName += QLatin1Char('/');
SyncFileStatus status(SyncFileStatus::StatusUpToDate);
if (waitingForPropagation) {
status.set(SyncFileStatus::StatusSync);
} else if (showErrorInSocketApi(item)) {
status.set(SyncFileStatus::StatusError);
} else if (showWarningInSocketApi(item)) {
status.set(SyncFileStatus::StatusWarning);
} else {
// After a sync finished, we need to show the users issues from that last sync like the activity list does.
// Also used for parent directories showing a warning for an error child.
SyncFileStatus::SyncFileStatusTag problemStatus = lookupProblem(item.destination(), _syncProblems);
if (problemStatus != SyncFileStatus::StatusNone)
status.set(problemStatus);
}
emit fileStatusChanged(systemFileName, SyncFileStatus(SyncFileStatus::StatusSync));
if (item._remotePerm.contains("S"))
status.setSharedWithMe(true);
return status;
}
void SyncFileStatusTracker::invalidateParentPaths(const QString& path)
{
QStringList splitPath = path.split('/', QString::SkipEmptyParts);
for (int i = 0; i < splitPath.size(); ++i) {
QString parentPath = splitPath.mid(0, i).join('/');
emit fileStatusChanged(_syncEngine->localPath() + parentPath, fileStatus(parentPath));
}
}
}

View file

@ -1,5 +1,6 @@
/*
* Copyright (C) by Klaas Freitag <freitag@owncloud.com>
* Copyright (C) by Jocelyn Turcotte <jturcotte@woboq.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@ -17,38 +18,43 @@
#include "ownsql.h"
#include "syncfileitem.h"
#include "syncfilestatus.h"
#include <QSet>
#include <csync.h>
#include <set>
namespace OCC {
class SyncEngine;
struct Problem {
QString path;
SyncFileStatus::SyncFileStatusTag severity = SyncFileStatus::StatusError;
Problem(const QString &path) : path(path) { }
Problem(const QString &path, SyncFileStatus::SyncFileStatusTag severity) : path(path), severity(severity) { }
// Assume that each path will only have one severity, don't care about it in sorting
bool operator<(const Problem &other) const { return path < other.path; }
bool operator==(const Problem &other) const { return path == other.path; }
};
class OWNCLOUDSYNC_EXPORT SyncFileStatusTracker : public QObject
{
Q_OBJECT
public:
SyncFileStatusTracker(SyncEngine *syncEngine);
SyncFileStatusTracker(SyncEngine* syncEngine);
SyncFileStatus fileStatus(const QString& systemFileName);
signals:
void fileStatusChanged(const QString& systemFileName, SyncFileStatus fileStatus);
private slots:
void slotThreadTreeWalkResult(const SyncFileItemVector& items);
void slotAboutToPropagate(SyncFileItemVector& items);
void slotSyncFinished();
void slotItemCompleted(const SyncFileItem &item);
void slotItemDiscovered(const SyncFileItem &item);
void slotItemCompleted(const SyncFileItem& item);
private:
bool estimateState(QString fn, csync_ftw_type_e t, SyncFileStatus* s);
SyncEngine *_syncEngine;
// SocketAPI: Cache files and folders that had errors so that they can
// get a red ERROR icon.
QSet<QString> _stateLastSyncItemsWithErrorNew; // gets moved to _stateLastSyncItemsWithError at end of sync
QSet<QString> _stateLastSyncItemsWithError;
SyncFileStatus fileStatus(const SyncFileItem& item);
void invalidateParentPaths(const QString& path);
SyncEngine* _syncEngine;
std::set<Problem> _syncProblems;
};
}

View file

@ -236,18 +236,6 @@ QString Utility::toCSyncScheme(const QString &urlStr)
return url.toString();
}
bool Utility::doesSetContainPrefix(const QSet<QString> &l, const QString &p) {
Q_FOREACH (const QString &setPath, l) {
//qDebug() << Q_FUNC_INFO << p << setPath << setPath.startsWith(p);
if (setPath.startsWith(p)) {
return true;
}
}
//qDebug() << "-> NOOOOO!!!" << p << l.count();
return false;
}
QString Utility::escape(const QString &in)
{
#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0)

View file

@ -40,11 +40,6 @@ namespace Utility
OWNCLOUDSYNC_EXPORT void setLaunchOnStartup(const QString &appName, const QString& guiName, bool launch);
OWNCLOUDSYNC_EXPORT qint64 freeDiskSpace(const QString &path);
OWNCLOUDSYNC_EXPORT QString toCSyncScheme(const QString &urlStr);
/** Like QLocale::toString(double, 'f', prec), but drops trailing zeros after the decimal point */
OWNCLOUDSYNC_EXPORT bool doesSetContainPrefix(const QSet<QString> &l, const QString &p);
/**
* @brief compactFormatDouble - formats a double value human readable.